Bug 23938 - should not free memory alloced in obstack by free()
Summary: should not free memory alloced in obstack by free()
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: gas (show other bugs)
Version: 2.32
: P2 normal
Target Milestone: 2.32
Assignee: Alan Modra
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-11-30 06:13 UTC by wuheng
Modified: 2018-12-01 06:18 UTC (History)
1 user (show)

See Also:
Host:
Target: sparc-sun-solaris2
Build:
Last reconfirmed: 2018-11-30 00:00:00


Attachments
free obstack memory use obstack_free() (227 bytes, patch)
2018-11-30 06:13 UTC, wuheng
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description wuheng 2018-11-30 06:13:45 UTC
Created attachment 11420 [details]
free obstack memory use obstack_free()

void
s_xstab (int what)
{
  int length;
  char *stab_secname, *stabstr_secname;
  static char *saved_secname, *saved_strsecname;

  /* @@ 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);
...
      if (saved_secname)
        {
          free (saved_secname);
          free (saved_strsecname);
        }
...
}

The var "saved_secname" in "s_xstab"  function which pointed to the memory alloced in obstack, should not be freed in line 441:"free (saved_secname);".
Comment 1 Alan Modra 2018-11-30 10:13:26 UTC
This bug has existed since 1996, git commit c7a89bde9b7.

You can't simply call obstack_free though, since obstack_free frees all more recently allocated memory on the obstack as well as its arg (and obstack_free takes an obstack arg too).
Comment 2 Alan Modra 2018-11-30 13:30:46 UTC
Testing a fix.
Comment 3 Sourceware Commits 2018-12-01 04:52:27 UTC
The master branch has been updated by Alan Modra <amodra@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=0acc7632bb09cce832a1b3756d31cc3fa93a724a

commit 0acc7632bb09cce832a1b3756d31cc3fa93a724a
Author: Alan Modra <amodra@gmail.com>
Date:   Sat Dec 1 09:37:48 2018 +1030

    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.
Comment 4 Alan Modra 2018-12-01 04:53:46 UTC
Fixed
Comment 5 wuheng 2018-12-01 06:18:17 UTC
(In reply to Alan Modra from comment #4)
> Fixed

thx