[RFA/gas] stabs.c (stabs_generate_asm_file): Free malloced 'dir'.

[RFA/gas] stabs.c (stabs_generate_asm_file): Free malloced 'dir'.

[Michael Snyder]

[-- Attachment #1: Type: text/plain, Size: 5 bytes --]

OK?


[-- Attachment #2: stabs.txt --]
[-- Type: text/plain, Size: 609 bytes --]

2011-03-09  Michael Snyder  <msnyder@vmware.com>

	* stabs.c (stabs_generate_asm_file): Free malloced 'dir'.

Index: stabs.c
===================================================================
RCS file: /cvs/src/src/gas/stabs.c,v
retrieving revision 1.34
diff -u -p -r1.34 stabs.c
--- stabs.c	28 Feb 2011 18:32:52 -0000	1.34
+++ stabs.c	9 Mar 2011 20:56:43 -0000
@@ -502,6 +502,7 @@ stabs_generate_asm_file (void)
       dir2 = (char *) alloca (strlen (dir) + 2);
       sprintf (dir2, "%s%s", dir, "/");
       generate_asm_file (N_SO, dir2);
+      xfree (dir);
     }
   generate_asm_file (N_SO, file);
 }

Re: [RFA/gas] stabs.c (stabs_generate_asm_file): Free malloced 'dir'.

[Steve Ellcey]

Michael,

I believe this patch is causing the GNU assembler to abort on my
ia64-hp-hpux11.23 platform.  When I remove the xfree calls you added the
abort goes away.  I don't know if anyone else is seeing this but here is
what I think is happening:  The xfree calls are freeing the pointer
returned from remap_debug_filename and remap_debug_filename may allocate
new memory or it may just return the pointer that was passed in to it.
In some cases the pointer passed in to remap_debug_filename comes from a
getpwd call.  Now getpwd calls getcwd and keeps the return value of
getcwd in a static pointer for reuse.  I think we are calling getpwd
once, setting the static variable, using that pointer in a
remap_debug_filename call (which returns the same pointer) and then
freeing that allocated space.  But that doesn't change the value of the
static pointer in getpwd, so the next time we call getpwd we get a
pointer to freed memory and try to use (and then free) that already
freed memory.   I think that is where the abort is happening.

Steve Ellcey
sje@cup.hp.com

Re: [RFA/gas] stabs.c (stabs_generate_asm_file): Free malloced 'dir'.

[Michael Snyder]

Steve Ellcey wrote:
> Michael,
> 
> I believe this patch is causing the GNU assembler to abort on my
> ia64-hp-hpux11.23 platform.  When I remove the xfree calls you added the
> abort goes away.  I don't know if anyone else is seeing this but here is
> what I think is happening:  The xfree calls are freeing the pointer
> returned from remap_debug_filename and remap_debug_filename may allocate
> new memory or it may just return the pointer that was passed in to it.
> In some cases the pointer passed in to remap_debug_filename comes from a
> getpwd call.  Now getpwd calls getcwd and keeps the return value of
> getcwd in a static pointer for reuse.  I think we are calling getpwd
> once, setting the static variable, using that pointer in a
> remap_debug_filename call (which returns the same pointer) and then
> freeing that allocated space.  But that doesn't change the value of the
> static pointer in getpwd, so the next time we call getpwd we get a
> pointer to freed memory and try to use (and then free) that already
> freed memory.   I think that is where the abort is happening.
> 
> Steve Ellcey
> sje@cup.hp.com

Thanks for the excellent analysis.

However, I never checked in the change you are referring to.
Are you sure you're not thinking of the similar change that I
checked in to dwarf2dbg.c?

Re: [RFA/gas] stabs.c (stabs_generate_asm_file): Free malloced 'dir'.

[Steve Ellcey]

On Thu, 2011-03-10 at 10:27 -0800, Michael Snyder wrote:

> Thanks for the excellent analysis.
> 
> However, I never checked in the change you are referring to.
> Are you sure you're not thinking of the similar change that I
> checked in to dwarf2dbg.c?

Yes, the change to dwarf2dbg.c is what I was refering too.  Looks like
I got confused and followed up to the wrong email.  The problem
started with this patch:


2011-03-09  Michael Snyder  <msnyder@vmware.com>

        * dwarf2dbg.c (out_file_list): Free malloced 'dir'.
        (out_debug_info): Free malloced 'dirname' and 'comp_dir'.
        (emit_fixed_inc_line_addr): Assign instead of conditional
        in assert.


I am thinking the best fix might be this change to remap_debug_filename
so that it always allocates the return value and can thus that value can
always be freed.

Index: remap.c
===================================================================
RCS file: /cvs/src/src/gas/remap.c,v
retrieving revision 1.3
diff -r1.3 remap.c
83c83
<     return filename;
---
>     return xstrdup (filename);


It seems to fix my GCC build.

Steve Ellcey
sje@cup.hp.com