From: Alan Modra Date: Fri, 30 Nov 2018 23:07:48 +0000 (+1030) Subject: PR23938, should not free memory alloced in obstack by free() X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=0acc7632bb09cce832a1b3756d31cc3fa93a724a;p=binutils-gdb.git PR23938, should not free memory alloced in obstack by free() 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 a9c972db79d..f497b578f46 100644 --- a/gas/ChangeLog +++ b/gas/ChangeLog @@ -1,3 +1,23 @@ +2018-12-01 Alan Modra + + 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 * 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 945b4ecd0b0..2a5b5440a27 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 a674c1b8bb1..3ec6b599f4a 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 3f2e27b067a..491bef806bf 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 352b802d4af..3ae447af566 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 6ddbdada15c..ed1f6adceff 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 (¬es); 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 (¬es, 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 (¬es)) + obstack_free (¬es, 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 (¬es)) + obstack_free (¬es, 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 (¬es)) + obstack_free (¬es, 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 (¬es); 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 7c28f43cc58..c247cb3692d 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 00000000000..5c1a3938e68 --- /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