PR23938, should not free memory alloced in obstack by free()

[Alan Modra]

This removes ineffectual and wrong code caching section names in
gas/stabs.c. Code like

seg = subseg_new (name, 0);
...
if (seg->name == name)
seg->name = xstrdup (name);

with the idea of being able to unconditionally free "name" later no
longer works. "name" is referenced by the section hash table as well
as in the section->name field. It would be possible to use
"bfd_rename_section (stdoutput, seg, xstrdup (name))", but instead I
opted for a fairly straight-forward approach of adding extra
parameters to two functions to indicate section name strings should be
freed if possible.

PR 23938
* read.h (get_stab_string_offset): Update prototype.
* stabs.c (get_stab_string_offset): Add free_stabstr_secname
parameter. Free stabstr_secname if unused as section name.
Don't xstrdup name when used.
(s_stab_generic): Remove forward declaration. Add
stab_secname_obstack_end param. Reference notes obstack via
macros. Delete cached_secname. Adjust get_stab_string_offset
call. Free stab_secname if unused as section name.
(s_stab): Adjust s_stab_generic call.
(s_xstab): Likewise. Delete saved_secname and saved_strsecname.
* config/obj-elf.c (obj_elf_init_stab_section): Adjust
get_stab_string_offset call.
* config/obj-coff.c (obj_coff_init_stab_section): Likewise.
* config/obj-som.c (obj_som_init_stab_section): Likewise.
* testsuite/gas/all/pr23938.s: New test.
* testsuite/gas/all/gas.exp: Run it.

diff --git a/gas/ChangeLog b/gas/ChangeLog
index a9c972db79..f497b578f4 100644
--- a/gas/ChangeLog
+++ b/gas/ChangeLog
@@ -1,3 +1,23 @@
[phone] Alan Modra <***@gmail.com>
+
+ PR 23938
+ * read.h (get_stab_string_offset): Update prototype.
+ * stabs.c (get_stab_string_offset): Add free_stabstr_secname
+ parameter. Free stabstr_secname if unused as section name.
+ Don't xstrdup name when used.
+ (s_stab_generic): Remove forward declaration. Add
+ stab_secname_obstack_end param. Reference notes obstack via
+ macros. Delete cached_secname. Adjust get_stab_string_offset
+ call. Free stab_secname if unused as section name.
+ (s_stab): Adjust s_stab_generic call.
+ (s_xstab): Likewise. Delete saved_secname and saved_strsecname.
+ * config/obj-elf.c (obj_elf_init_stab_section): Adjust
+ get_stab_string_offset call.
+ * config/obj-coff.c (obj_coff_init_stab_section): Likewise.
+ * config/obj-som.c (obj_som_init_stab_section): Likewise.
+ * testsuite/gas/all/pr23938.s: New test.
+ * testsuite/gas/all/gas.exp: Run it.
+
2018-11-30 Fredrik Noring <***@nocrew.org>

* config/tc-mips.c (mips_fix_r5900, mips_fix_r5900_explicit):
diff --git a/gas/config/obj-coff.c b/gas/config/obj-coff.c
index 945b4ecd0b..2a5b5440a2 100644
--- a/gas/config/obj-coff.c
+++ b/gas/config/obj-coff.c
@@ -1802,7 +1802,7 @@ obj_coff_init_stab_section (segT seg)
memset (p, 0, 12);
file = as_where ((unsigned int *) NULL);
stabstr_name = concat (seg->name, "str", (char *) NULL);
- stroff = get_stab_string_offset (file, stabstr_name);
+ stroff = get_stab_string_offset (file, stabstr_name, TRUE);
know (stroff == 1);
md_number_to_chars (p, stroff, 4);
}
diff --git a/gas/config/obj-elf.c b/gas/config/obj-elf.c
index a674c1b8bb..3ec6b599f4 100644
--- a/gas/config/obj-elf.c
+++ b/gas/config/obj-elf.c
@@ -2127,7 +2127,7 @@ obj_elf_init_stab_section (segT seg)
memset (p, 0, 12);
file = as_where (NULL);
stabstr_name = concat (segment_name (seg), "str", (char *) NULL);
- stroff = get_stab_string_offset (file, stabstr_name);
+ stroff = get_stab_string_offset (file, stabstr_name, TRUE);
know (stroff == 1 || (stroff == 0 && file[0] == '\0'));
md_number_to_chars (p, stroff, 4);
seg_info (seg)->stabu.p = p;
diff --git a/gas/config/obj-som.c b/gas/config/obj-som.c
index 3f2e27b067..491bef806b 100644
--- a/gas/config/obj-som.c
+++ b/gas/config/obj-som.c
@@ -243,7 +243,7 @@ obj_som_init_stab_section (segT seg)
p = frag_more (12);
memset (p, 0, 12);
file = as_where ((unsigned int *) NULL);
- stroff = get_stab_string_offset (file, "$GDB_STRINGS$");
+ stroff = get_stab_string_offset (file, "$GDB_STRINGS$", FALSE);
know (stroff == 1);
md_number_to_chars (p, stroff, 4);
seg_info (seg)->stabu.p = p;
diff --git a/gas/read.h b/gas/read.h
index 352b802d4a..3ae447af56 100644
--- a/gas/read.h
+++ b/gas/read.h
@@ -114,7 +114,7 @@ extern char original_case_string[];

extern void pop_insert (const pseudo_typeS *);
extern unsigned int get_stab_string_offset
- (const char *string, const char *stabstr_secname);
+ (const char *, const char *, bfd_boolean);
extern void aout_process_stab (int, const char *, int, int, int);
extern char *demand_copy_string (int *lenP);
extern char *demand_copy_C_string (int *len_pointer);
diff --git a/gas/stabs.c b/gas/stabs.c
index 6ddbdada15..ed1f6adcef 100644
--- a/gas/stabs.c
+++ b/gas/stabs.c
@@ -34,7 +34,6 @@

int outputting_stabs_line_debug = 0;

-static void s_stab_generic (int, const char *, const char *);
static void generate_asm_file (int, const char *);

/* Allow backends to override the names used for the stab sections. */
@@ -80,7 +79,8 @@ static const char *current_function_label;
#endif

unsigned int
-get_stab_string_offset (const char *string, const char *stabstr_secname)
+get_stab_string_offset (const char *string, const char *stabstr_secname,
+ bfd_boolean free_stabstr_secname)
{
unsigned int length;
unsigned int retval;
@@ -97,8 +97,10 @@ get_stab_string_offset (const char *string, const char *stabstr_secname)
save_seg = now_seg;
save_subseg = now_subseg;

- /* Create the stab string section. */
+ /* Create the stab string section, if it doesn't already exist. */
seg = subseg_new (stabstr_secname, 0);
+ if (free_stabstr_secname && seg->name != stabstr_secname)
+ free ((char *) stabstr_secname);

retval = seg_info (seg)->stabu.stab_string_size;
if (retval <= 0)
@@ -108,8 +110,6 @@ get_stab_string_offset (const char *string, const char *stabstr_secname)
*p = 0;
retval = seg_info (seg)->stabu.stab_string_size = 1;
bfd_set_section_flags (stdoutput, seg, SEC_READONLY | SEC_DEBUGGING);
- if (seg->name == stabstr_secname)
- seg->name = xstrdup (stabstr_secname);
}

if (length > 0)
@@ -170,12 +170,15 @@ aout_process_stab (int what, const char *string, int type, int other, int desc)
#endif

/* This can handle different kinds of stabs (s,n,d) and different
- kinds of stab sections. */
+ kinds of stab sections. If STAB_SECNAME_OBSTACK_END is non-NULL,
+ then STAB_SECNAME and STABSTR_SECNAME will be freed if possible
+ before this function returns (the former by obstack_free). */

static void
-s_stab_generic (int what,
- const char * stab_secname,
- const char * stabstr_secname)
+s_stab_generic (int what,
+ const char *stab_secname,
+ const char *stabstr_secname,
+ const char *stab_secname_obstack_end)
{
long longint;
const char *string;
@@ -211,7 +214,7 @@ s_stab_generic (int what,
/* FIXME: We should probably find some other temporary storage
for string, rather than leaking memory if someone else
happens to use the notes obstack. */
- saved_string_obstack_end = notes.next_free;
+ saved_string_obstack_end = obstack_next_free (&notes);
SKIP_WHITESPACE ();
if (*input_line_pointer == ',')
input_line_pointer++;
@@ -313,7 +316,6 @@ s_stab_generic (int what,
char *p;

static segT cached_sec;
- static char *cached_secname;

dot = frag_now_fix ();

@@ -321,7 +323,7 @@ s_stab_generic (int what,
md_flush_pending_output ();
#endif

- if (cached_secname && !strcmp (cached_secname, stab_secname))
+ if (cached_sec && strcmp (cached_sec->name, stab_secname) == 0)
{
seg = cached_sec;
subseg_set (seg, 0);
@@ -329,9 +331,6 @@ s_stab_generic (int what,
else
{
seg = subseg_new (stab_secname, 0);
- if (cached_secname)
- free (cached_secname);
- cached_secname = xstrdup (stab_secname);
cached_sec = seg;
}

@@ -345,13 +344,19 @@ s_stab_generic (int what,
seg_info (seg)->hadone = 1;
}

- stroff = get_stab_string_offset (string, stabstr_secname);
- if (what == 's')
- {
- /* Release the string, if nobody else has used the obstack. */
- if (saved_string_obstack_end == notes.next_free)
- obstack_free (&notes, string);
- }
+ stroff = get_stab_string_offset (string, stabstr_secname,
+ stab_secname_obstack_end != NULL);
+
+ /* Release the string, if nobody else has used the obstack. */
+ if (saved_string_obstack_end != NULL
+ && saved_string_obstack_end == obstack_next_free (&notes))
+ obstack_free (&notes, string);
+ /* Similarly for the section name. This must be done before
+ creating symbols below, which uses the notes obstack. */
+ if (seg->name != stab_secname
+ && stab_secname_obstack_end != NULL
+ && stab_secname_obstack_end == obstack_next_free (&notes))
+ obstack_free (&notes, stab_secname);

/* At least for now, stabs in a special stab section are always
output as 12 byte blocks of information. */
@@ -390,6 +395,12 @@ s_stab_generic (int what,
}
else
{
+ if (stab_secname_obstack_end != NULL)
+ {
+ free ((char *) stabstr_secname);
+ if (stab_secname_obstack_end == obstack_next_free (&notes))
+ obstack_free (&notes, stab_secname);
+ }
#ifdef OBJ_PROCESS_STAB
OBJ_PROCESS_STAB (0, what, string, type, other, desc);
#else
@@ -405,7 +416,7 @@ s_stab_generic (int what,
void
s_stab (int what)
{
- s_stab_generic (what, STAB_SECTION_NAME, STAB_STRING_SECTION_NAME);
+ s_stab_generic (what, STAB_SECTION_NAME, STAB_STRING_SECTION_NAME, NULL);
}

/* "Extended stabs", used in Solaris only now. */
@@ -414,13 +425,10 @@ void
s_xstab (int what)
{
int length;
- char *stab_secname, *stabstr_secname;
- static char *saved_secname, *saved_strsecname;
+ char *stab_secname, *stabstr_secname, *stab_secname_obstack_end;

- /* @@ MEMORY LEAK: This allocates a copy of the string, but in most
- cases it will be the same string, so we could release the storage
- back to the obstack it came from. */
stab_secname = demand_copy_C_string (&length);
+ stab_secname_obstack_end = obstack_next_free (&notes);
SKIP_WHITESPACE ();
if (*input_line_pointer == ',')
input_line_pointer++;
@@ -433,18 +441,9 @@ s_xstab (int what)

/* To get the name of the stab string section, simply add "str" to
the stab section name. */
- if (saved_secname == 0 || strcmp (saved_secname, stab_secname))
- {
- stabstr_secname = concat (stab_secname, "str", (char *) NULL);
- if (saved_secname)
- {
- free (saved_secname);
- free (saved_strsecname);
- }
- saved_secname = stab_secname;
- saved_strsecname = stabstr_secname;
- }
- s_stab_generic (what, saved_secname, saved_strsecname);
+ stabstr_secname = concat (stab_secname, "str", (char *) NULL);
+ s_stab_generic (what, stab_secname, stabstr_secname,
+ stab_secname_obstack_end);
}

#ifdef S_SET_DESC
diff --git a/gas/testsuite/gas/all/gas.exp b/gas/testsuite/gas/all/gas.exp
index 7c28f43cc5..c247cb3692 100644
--- a/gas/testsuite/gas/all/gas.exp
+++ b/gas/testsuite/gas/all/gas.exp
@@ -472,3 +472,5 @@ run_dump_test "org-5"
run_dump_test "org-6"

run_dump_test "fill-1"
+
+gas_test "pr23938.s" "" "" ".xstabs"
diff --git a/gas/testsuite/gas/all/pr23938.s b/gas/testsuite/gas/all/pr23938.s
new file mode 100644
index 0000000000..5c1a3938e6
--- /dev/null
+++ b/gas/testsuite/gas/all/pr23938.s
@@ -0,0 +1,2 @@
+ .xstabs ".stab.index","main",36,0,0,0
+ .xstabs ".stab.foo","main",36,0,0,0