x86 relr memory leaks

x86 relr memory leaks

[Alan Modra]

From 2af61a5d22c550ac951fbbd2a606ae689657066a Mon Sep 17 00:00:00 2001
From: Alan Modra <amodra@gmail.com>
Date: Wed, 15 Jan 2025 23:12:52 +1030
Subject: 

This fixes some x86 memory leaks.  I think it would be possible to
free the relr data in _bfd_elf_x86_finish_relative_relocs if we
wanted to reclaim some memory earlier, but for tidying after errors we
likely would need to free in the hash_table_free function anyway.

_bfd_x86_elf_link_relax_section is called via bfd_relax_section,
ie. whenever relaxation is enabled.  This is a waste of time if
dt_relr relocs are not enabled since the function is there only to
handle relr.

Since I may have missed something with the relax_section change,
OK to install?

	* elfxx-x86.c (elf_x86_link_hash_table_free): Free relr data.
	(_bfd_x86_elf_link_relax_section): Return early
	if !info->enable_dt_relr.  Do set "again" false before early
	returns.

diff --git a/bfd/elfxx-x86.c b/bfd/elfxx-x86.c
index 7c164e9c131..cd47575f589 100644
--- a/bfd/elfxx-x86.c
+++ b/bfd/elfxx-x86.c
@@ -693,6 +693,9 @@ elf_x86_link_hash_table_free (bfd *obfd)
   struct elf_x86_link_hash_table *htab
     = (struct elf_x86_link_hash_table *) obfd->link.hash;
 
+  free (htab->dt_relr_bitmap.u.elf64);
+  free (htab->unaligned_relative_reloc.data);
+  free (htab->relative_reloc.data);
   if (htab->loc_hash_table)
     htab_delete (htab->loc_hash_table);
   if (htab->loc_hash_memory)
@@ -1089,13 +1092,16 @@ _bfd_x86_elf_link_relax_section (bfd *abfd ATTRIBUTE_UNUSED,
   bool return_status = false;
   bool keep_symbuf = false;
 
-  if (bfd_link_relocatable (info))
-    return true;
-
   /* Assume we're not going to change any sizes, and we'll only need
      one pass.  */
   *again = false;
 
+  if (bfd_link_relocatable (info))
+    return true;
+
+  if (!info->enable_dt_relr)
+    return true;
+
   bed = get_elf_backend_data (abfd);
   htab = elf_x86_hash_table (info, bed->target_id);
   if (htab == NULL)

-- 
Alan Modra

Re: x86 relr memory leaks

[H.J. Lu]

On Thu, Jan 16, 2025 at 6:56 AM Alan Modra <amodra@gmail.com> wrote:
>
> From 2af61a5d22c550ac951fbbd2a606ae689657066a Mon Sep 17 00:00:00 2001
> From: Alan Modra <amodra@gmail.com>
> Date: Wed, 15 Jan 2025 23:12:52 +1030
> Subject:
>
> This fixes some x86 memory leaks.  I think it would be possible to
> free the relr data in _bfd_elf_x86_finish_relative_relocs if we
> wanted to reclaim some memory earlier, but for tidying after errors we
> likely would need to free in the hash_table_free function anyway.
>
> _bfd_x86_elf_link_relax_section is called via bfd_relax_section,
> ie. whenever relaxation is enabled.  This is a waste of time if
> dt_relr relocs are not enabled since the function is there only to
> handle relr.
>
> Since I may have missed something with the relax_section change,
> OK to install?

OK if tests pass.

Thanks.

>
>         * elfxx-x86.c (elf_x86_link_hash_table_free): Free relr data.
>         (_bfd_x86_elf_link_relax_section): Return early
>         if !info->enable_dt_relr.  Do set "again" false before early
>         returns.
>
> diff --git a/bfd/elfxx-x86.c b/bfd/elfxx-x86.c
> index 7c164e9c131..cd47575f589 100644
> --- a/bfd/elfxx-x86.c
> +++ b/bfd/elfxx-x86.c
> @@ -693,6 +693,9 @@ elf_x86_link_hash_table_free (bfd *obfd)
>    struct elf_x86_link_hash_table *htab
>      = (struct elf_x86_link_hash_table *) obfd->link.hash;
>
> +  free (htab->dt_relr_bitmap.u.elf64);
> +  free (htab->unaligned_relative_reloc.data);
> +  free (htab->relative_reloc.data);
>    if (htab->loc_hash_table)
>      htab_delete (htab->loc_hash_table);
>    if (htab->loc_hash_memory)
> @@ -1089,13 +1092,16 @@ _bfd_x86_elf_link_relax_section (bfd *abfd ATTRIBUTE_UNUSED,
>    bool return_status = false;
>    bool keep_symbuf = false;
>
> -  if (bfd_link_relocatable (info))
> -    return true;
> -
>    /* Assume we're not going to change any sizes, and we'll only need
>       one pass.  */
>    *again = false;
>
> +  if (bfd_link_relocatable (info))
> +    return true;
> +
> +  if (!info->enable_dt_relr)
> +    return true;
> +
>    bed = get_elf_backend_data (abfd);
>    htab = elf_x86_hash_table (info, bed->target_id);
>    if (htab == NULL)
>
> --
> Alan Modra



-- 
H.J.

Re: x86 relr memory leaks

[Alan Modra]

On Thu, Jan 16, 2025 at 07:01:42AM +0800, H.J. Lu wrote:
> On Thu, Jan 16, 2025 at 6:56 AM Alan Modra <amodra@gmail.com> wrote:
> >
> > From 2af61a5d22c550ac951fbbd2a606ae689657066a Mon Sep 17 00:00:00 2001
> > From: Alan Modra <amodra@gmail.com>
> > Date: Wed, 15 Jan 2025 23:12:52 +1030
> > Subject:
> >
> > This fixes some x86 memory leaks.  I think it would be possible to
> > free the relr data in _bfd_elf_x86_finish_relative_relocs if we
> > wanted to reclaim some memory earlier, but for tidying after errors we
> > likely would need to free in the hash_table_free function anyway.
> >
> > _bfd_x86_elf_link_relax_section is called via bfd_relax_section,
> > ie. whenever relaxation is enabled.  This is a waste of time if
> > dt_relr relocs are not enabled since the function is there only to
> > handle relr.
> >
> > Since I may have missed something with the relax_section change,
> > OK to install?
> 
> OK if tests pass.

They do.  In fact with another 20 or so patches I have the x86_64 and
powerpc ld testsuites passing without seeing any memory leaks, except
for some in the gcc plugin.  I won't apply all of the patches before
Nick cuts the branch as there is some risk of introducing double frees
or attempted frees of contents that weren't malloced.

-- 
Alan Modra

Re: x86 relr memory leaks

[Alan Modra]

On Thu, Jan 16, 2025 at 09:52:23AM +1030, Alan Modra wrote:
> In fact with another 20 or so patches I have the x86_64 and
> powerpc ld testsuites passing without seeing any memory leaks, except
> for some in the gcc plugin.

Well that claim was premature.  After commit bea261b937df which runs
more of the ld tests I see many more leaks.  libctf leaks quite a bit
as shown by ld/testsuite/ld-ctf tests.

-- 
Alan Modra

Re: x86 relr memory leaks

[Nick Alcock]

On 21 Jan 2025, Alan Modra told this:

> On Thu, Jan 16, 2025 at 09:52:23AM +1030, Alan Modra wrote:
>> In fact with another 20 or so patches I have the x86_64 and
>> powerpc ld testsuites passing without seeing any memory leaks, except
>> for some in the gcc plugin.
>
> Well that claim was premature.  After commit bea261b937df which runs
> more of the ld tests I see many more leaks.  libctf leaks quite a bit
> as shown by ld/testsuite/ld-ctf tests.

Could you send me any more info on this? I run routine leak checks via
the valgrind leak checker and have squashed all I know about: so this is
presumably happening via some route I have not properly identified.

(Major changes are afoot right now for the BTF-superset CTFv4, and I'd
be happy to squash any leaks you find as part of that and backport the
fixes.)

-- 
NULL && (void)

Re: x86 relr memory leaks

[Nick Alcock]

On 22 Jan 2025, Nick Alcock spake thusly:

> On 21 Jan 2025, Alan Modra told this:
>
>> On Thu, Jan 16, 2025 at 09:52:23AM +1030, Alan Modra wrote:
>>> In fact with another 20 or so patches I have the x86_64 and
>>> powerpc ld testsuites passing without seeing any memory leaks, except
>>> for some in the gcc plugin.
>>
>> Well that claim was premature.  After commit bea261b937df which runs
>> more of the ld tests I see many more leaks.  libctf leaks quite a bit
>> as shown by ld/testsuite/ld-ctf tests.
>
> Could you send me any more info on this? I run routine leak checks via
> the valgrind leak checker and have squashed all I know about: so this is
> presumably happening via some route I have not properly identified.

Oh, never mind, I didn't look at the commit -- presumably this is
specific to -fsanitize=address,undefined? Builds without that, and with
just (phew) valgrind --quiet --leak-check=full --trace-children=yes
--trace-children-skip="*/gcc,*/cc1,*/cc1plus,*/lto1,*/debuginfod-find,*/sed,*/expr,*/libtool"
make -j 30 check-libctf check-ld

do not show unexpected leaks. I mean ld leaks, but I didn't see
*libctf*-attributable leaks, and check-libctf is leak-free: so the only
route is presumably libctf-allocated buffers not being freed by ld once
the link is done (like all its other buffers, as far as I could tell:
if that's changed, the libctf-written buffers can of course be freed at
the same time as all the rest).

-- 
NULL && (void)

Re: x86 relr memory leaks

[Alan Modra]

On Wed, Jan 22, 2025 at 03:24:16PM +0000, Nick Alcock wrote:
> On 22 Jan 2025, Nick Alcock spake thusly:
> 
> > On 21 Jan 2025, Alan Modra told this:
> >
> >> On Thu, Jan 16, 2025 at 09:52:23AM +1030, Alan Modra wrote:
> >>> In fact with another 20 or so patches I have the x86_64 and
> >>> powerpc ld testsuites passing without seeing any memory leaks, except
> >>> for some in the gcc plugin.
> >>
> >> Well that claim was premature.  After commit bea261b937df which runs
> >> more of the ld tests I see many more leaks.  libctf leaks quite a bit
> >> as shown by ld/testsuite/ld-ctf tests.
> >
> > Could you send me any more info on this? I run routine leak checks via

Sent privately.

-- 
Alan Modra

Re: x86 relr memory leaks

[Nick Alcock]

On 22 Jan 2025, Alan Modra verbalised:

> On Wed, Jan 22, 2025 at 03:24:16PM +0000, Nick Alcock wrote:
>> On 22 Jan 2025, Nick Alcock spake thusly:
>> 
>> > On 21 Jan 2025, Alan Modra told this:
>> >
>> >> On Thu, Jan 16, 2025 at 09:52:23AM +1030, Alan Modra wrote:
>> >>> In fact with another 20 or so patches I have the x86_64 and
>> >>> powerpc ld testsuites passing without seeing any memory leaks, except
>> >>> for some in the gcc plugin.
>> >>
>> >> Well that claim was premature.  After commit bea261b937df which runs
>> >> more of the ld tests I see many more leaks.  libctf leaks quite a bit
>> >> as shown by ld/testsuite/ld-ctf tests.
>> >
>> > Could you send me any more info on this? I run routine leak checks via
>
> Sent privately.

Thank you! I'll have a look. I can feel my test matrix expanding
already...

-- 
NULL && (void)

Re: x86 relr memory leaks

[Nick Alcock]

On 28 Jan 2025, Nick Alcock uttered the following:

> On 22 Jan 2025, Alan Modra verbalised:
>
>> On Wed, Jan 22, 2025 at 03:24:16PM +0000, Nick Alcock wrote:
>>> On 22 Jan 2025, Nick Alcock spake thusly:
>>> 
>>> > On 21 Jan 2025, Alan Modra told this:
>>> >
>>> >> On Thu, Jan 16, 2025 at 09:52:23AM +1030, Alan Modra wrote:
>>> >>> In fact with another 20 or so patches I have the x86_64 and
>>> >>> powerpc ld testsuites passing without seeing any memory leaks, except
>>> >>> for some in the gcc plugin.
>>> >>
>>> >> Well that claim was premature.  After commit bea261b937df which runs
>>> >> more of the ld tests I see many more leaks.  libctf leaks quite a bit
>>> >> as shown by ld/testsuite/ld-ctf tests.
>>> >
>>> > Could you send me any more info on this? I run routine leak checks via
>>
>> Sent privately.
>
> Thank you! I'll have a look. I can feel my test matrix expanding
> already...

Fix trivial, in the end (see attached). Will throw it at the trybots and
then push soon, but I'm going to use it as an excuse to revive and
improve my testing machinery so there might be another delay while I
hack at that :)

(It turns out I am already testing with the sanitizer flags in question.
Not sure why the leaks didn't show up...)

From f21a2f94c8587dbf5b86c951c4afcecdfdb9a653 Mon Sep 17 00:00:00 2001
From: Nick Alcock <nick.alcock@oracle.com>
Date: Mon, 10 Feb 2025 14:40:00 +0000
Subject: [PATCH] readelf, objdump: fix ctf dict leak

ctf_archive_next returns an opened dict, which must be closed by the caller.

Thanks to Alan Modra for spotting this.

binutils/
	* objdump.c (dump_ctf): Close dict.
	* readelf.c (dump_section_as_ctf): Likewise.
---
 binutils/objdump.c | 5 ++++-
 binutils/readelf.c | 5 ++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/binutils/objdump.c b/binutils/objdump.c
index 4980929d6ab..9b5b8faf397 100644
--- a/binutils/objdump.c
+++ b/binutils/objdump.c
@@ -4939,7 +4939,10 @@ dump_ctf (bfd *abfd, const char *sect_name, const char *parent_name,
   printf (_("Contents of CTF section %s:\n"), sanitize_string (sect_name));
 
   while ((fp = ctf_archive_next (ctfa, &i, &name, 0, &err)) != NULL)
-    dump_ctf_archive_member (fp, name, parent, member++);
+    {
+      dump_ctf_archive_member (fp, name, parent, member++);
+      ctf_dict_close (fp);
+    }
   if (err != ECTF_NEXT_END)
     {
       dump_ctf_errs (NULL);
diff --git a/binutils/readelf.c b/binutils/readelf.c
index 73163e0ee21..3c3acbc632a 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -17037,7 +17037,10 @@ dump_section_as_ctf (Elf_Internal_Shdr * section, Filedata * filedata)
 	    printable_section_name (filedata, section));
 
  while ((fp = ctf_archive_next (ctfa, &i, &name, 0, &err)) != NULL)
-    dump_ctf_archive_member (fp, name, parent, member++);
+   {
+     dump_ctf_archive_member (fp, name, parent, member++);
+     ctf_dict_close (fp);
+   }
  if (err != ECTF_NEXT_END)
    {
      dump_ctf_errs (NULL);
-- 
2.48.1.283.g18c60a128c

Re: x86 relr memory leaks

[Alan Modra]

On Mon, Feb 10, 2025 at 03:30:34PM +0000, Nick Alcock wrote:
> (It turns out I am already testing with the sanitizer flags in question.
> Not sure why the leaks didn't show up...)

ASAN_OPTIONS with detect_leaks=0 in environment?

> binutils/
> 	* objdump.c (dump_ctf): Close dict.
> 	* readelf.c (dump_section_as_ctf): Likewise.

With this applied I'm still seeing leaks.  eg.

/build/gas-san/all/ld/../binutils/objdump  --ctf tmpdir/dump > tmpdir/dump.out
Executing on host: sh -c {/build/gas-san/all/ld/../binutils/objdump  --ctf tmpdir/dump > tmpdir/dump.out 2>dump.tmp}  /dev/null  (timeout = 300)
spawn [open ...]
exited abnormally with 1, output:
=================================================================
==3929920==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 21 byte(s) in 1 object(s) allocated from:
    #0 0x7fe5316fc778 in realloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:85
    #1 0x623b91ec615f in ctf_str_append /home/alan/src/binutils-gdb/libctf/ctf-util.c:212
    #2 0x623b91ec630c in ctf_str_append_noerr /home/alan/src/binutils-gdb/libctf/ctf-util.c:228
    #3 0x623b91e70d6f in ctf_dump_objts /home/alan/src/binutils-gdb/libctf/ctf-dump.c:444
    #4 0x623b91e73716 in ctf_dump /home/alan/src/binutils-gdb/libctf/ctf-dump.c:738
    #5 0x623b9173128e in dump_ctf_archive_member /home/alan/src/binutils-gdb/binutils/objdump.c:4844
    #6 0x623b9173128e in dump_ctf /home/alan/src/binutils-gdb/binutils/objdump.c:4944
    #7 0x623b9173128e in dump_bfd /home/alan/src/binutils-gdb/binutils/objdump.c:5784
    #8 0x623b91733f4c in display_object_bfd /home/alan/src/binutils-gdb/binutils/objdump.c:5858
    #9 0x623b91733f4c in display_any_bfd /home/alan/src/binutils-gdb/binutils/objdump.c:5937
    #10 0x623b917341c2 in display_file /home/alan/src/binutils-gdb/binutils/objdump.c:5958
    #11 0x623b91714e9f in main /home/alan/src/binutils-gdb/binutils/objdump.c:6367
    #12 0x7fe530a2a1c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #13 0x7fe530a2a28a in __libc_start_main_impl ../csu/libc-start.c:360
    #14 0x623b9171c0c4 in _start (/build/gas-san/all/binutils/objdump+0x43050c4) (BuildId: 00679bb22e8b01be0368952c202b6fcc894d0f2c)

SUMMARY: AddressSanitizer: 21 byte(s) leaked in 1 allocation(s).

FAIL: Conflicted data syms, partially indexed, stripped, with variables

-- 
Alan Modra