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 a9c972db79dc72458b1314f5d12dbc790412af02..f497b578f469143f2973cb62414da89660ce69c5 100644 (file)
--- a/gas/ChangeLog
+++ b/gas/ChangeLog
@@ -1,3 +1,23 @@
+2018-12-01  Alan Modra  <amodra@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  <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 945b4ecd0b004594a7f2fdf004e8685521fcfaa1..2a5b5440a270488967cddb94393b26b281707ddb 100644 (file)
--- 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 a674c1b8bb1866d31b1b82f6fe459a06dfab1208..3ec6b599f4a836e0fda112e79211e0b6f4d53894 100644 (file)
--- 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 3f2e27b067ac417009cf9dfa34f61ac1147a6060..491bef806bf354ae717adb0714a9f442652d01bb 100644 (file)
--- 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 352b802d4af94cc271ec81d9c732e641b55f0a3b..3ae447af56691f03315fc5352b8b1985375bbd34 100644 (file)
--- 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 6ddbdada15c3d4c7bf2fccbf8ca5e06eba10e662..ed1f6adceff6cc75eb42901901c7672f529bd03b 100644 (file)
--- 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 7c28f43cc588c797ee982318caafb7ccf2b5ab9c..c247cb3692d1611d559e2aaf19db6600f45fc0a2 100644 (file)
--- 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 (file)
index 0000000..5c1a393
--- /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