This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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 a9c972db79..f497b578f4 100644
--- 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 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

-- 
Alan Modra
Australia Development Lab, IBM


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]