Leak in i386_elf_section_change_hook

Leak in i386_elf_section_change_hook

[Alan Modra]

notes_alloc is perfect for assorted memory you can't free easily
and/or would rather leave freeing until just before exit.

	* config/tc-i386.c (i386_elf_section_change_hook): Use notes_alloc.

diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
index ed7c4a5ea03..c56ca4a2b4b 100644
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -17627,7 +17627,7 @@ i386_elf_section_change_hook (void)
       break;
   if (!curr)
     {
-      curr = XNEW (struct i386_segment_info);
+      curr = notes_alloc (sizeof (*curr));
       curr->subseg = info->subseg;
       curr->next = NULL;
       prev->next = curr;

-- 
Alan Modra
Australia Development Lab, IBM

Re: Leak in i386_elf_section_change_hook

[Jan Beulich]

On 21.02.2024 22:41, Alan Modra wrote:
> notes_alloc is perfect for assorted memory you can't free easily
> and/or would rather leave freeing until just before exit.
> 
> 	* config/tc-i386.c (i386_elf_section_change_hook): Use notes_alloc.
> 
> diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
> index ed7c4a5ea03..c56ca4a2b4b 100644
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -17627,7 +17627,7 @@ i386_elf_section_change_hook (void)
>        break;
>    if (!curr)
>      {
> -      curr = XNEW (struct i386_segment_info);
> +      curr = notes_alloc (sizeof (*curr));
>        curr->subseg = info->subseg;
>        curr->next = NULL;
>        prev->next = curr;
> 

Just for my understanding: Is it an unwritten(?) requirement then that
all XNEW()-ed (and alike) memory be freed before exiting? If so, why
would that be? I can certainly see that library code may not leak
memory, but something like gas, which is an isolated process, gives
up all resources anyway once finished with the (singular) task.

Jan

Re: Leak in i386_elf_section_change_hook

[Alan Modra]

On Thu, Feb 22, 2024 at 10:20:50AM +0100, Jan Beulich wrote:
> On 21.02.2024 22:41, Alan Modra wrote:
> > notes_alloc is perfect for assorted memory you can't free easily
> > and/or would rather leave freeing until just before exit.
> > 
> > 	* config/tc-i386.c (i386_elf_section_change_hook): Use notes_alloc.
> > 
> > diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
> > index ed7c4a5ea03..c56ca4a2b4b 100644
> > --- a/gas/config/tc-i386.c
> > +++ b/gas/config/tc-i386.c
> > @@ -17627,7 +17627,7 @@ i386_elf_section_change_hook (void)
> >        break;
> >    if (!curr)
> >      {
> > -      curr = XNEW (struct i386_segment_info);
> > +      curr = notes_alloc (sizeof (*curr));
> >        curr->subseg = info->subseg;
> >        curr->next = NULL;
> >        prev->next = curr;
> > 
> 
> Just for my understanding: Is it an unwritten(?) requirement then that
> all XNEW()-ed (and alike) memory be freed before exiting? If so, why
> would that be? I can certainly see that library code may not leak
> memory, but something like gas, which is an isolated process, gives
> up all resources anyway once finished with the (singular) task.

As you say, there is very little reason to free memory that holds
persistent data.  In fact, freeing memory slows down gas, ld, and the
binutils.  The only reason we bother is that many people using shiny
new tools that reports memory leaks, or automated checkers like
oss-fuzz, don't care or aren't able to distinguish between leaks that
matter and those that don't.  This leak obviously doesn't matter, but
a leak that occurred on every symbol evaluation would matter.
Hopefully by tidying memory before exit we'll only see reports of
leaks that matter.

-- 
Alan Modra
Australia Development Lab, IBM