public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] x86-64: Allocate input section memory if needed
@ 2022-12-27 19:47 H.J. Lu
  2022-12-28  1:16 ` Alan Modra
  2023-01-03 21:59 ` H.J. Lu
  0 siblings, 2 replies; 7+ messages in thread
From: H.J. Lu @ 2022-12-27 19:47 UTC (permalink / raw)
  To: binutils

When --no-keep-memory is used, the input section memory may not be cached.
Allocate input section memory for -z pack-relative-relocs if needed.

bfd/

	PR ld/29939
	* elfxx-x86.c (elf_x86_size_or_finish_relative_reloc): Allocate
	input section memory if needed.

ld/

	PR ld/29939
	* testsuite/ld-elf/dt-relr-2i.d: New test.
---
 bfd/elfxx-x86.c                  | 25 +++++++++++++++++++++++--
 ld/testsuite/ld-elf/dt-relr-2i.d | 17 +++++++++++++++++
 2 files changed, 40 insertions(+), 2 deletions(-)
 create mode 100644 ld/testsuite/ld-elf/dt-relr-2i.d

diff --git a/bfd/elfxx-x86.c b/bfd/elfxx-x86.c
index 2ddca340473..ec86e75eba9 100644
--- a/bfd/elfxx-x86.c
+++ b/bfd/elfxx-x86.c
@@ -1541,12 +1541,33 @@ elf_x86_size_or_finish_relative_reloc
 		    }
 		  else
 		    {
+		      bfd_byte *contents;
+
 		      if (rel.r_offset >= sec->size)
 			abort ();
+
+		      if (elf_section_data (sec)->this_hdr.contents
+			  != NULL)
+			contents
+			  = elf_section_data (sec)->this_hdr.contents;
+		      else
+			{
+			  if (!bfd_malloc_and_get_section (sec->owner,
+							   sec,
+							   &contents))
+			    info->callbacks->einfo
+			      /* xgettext:c-format */
+			      (_("%F%P: %pB: failed to allocate memory for section `%pA'\n"),
+			       info->output_bfd, sec);
+
+			  /* Cache the section contents for
+			     elf_link_input_bfd.  */
+			  elf_section_data (sec)->this_hdr.contents
+			    = contents;
+			}
 		      htab->elf_write_addend
 			(info->output_bfd, outrel->r_addend,
-			 (elf_section_data (sec)->this_hdr.contents
-			  + rel.r_offset));
+			 contents + rel.r_offset);
 		    }
 		}
 	    }
diff --git a/ld/testsuite/ld-elf/dt-relr-2i.d b/ld/testsuite/ld-elf/dt-relr-2i.d
new file mode 100644
index 00000000000..ed0ef9ccded
--- /dev/null
+++ b/ld/testsuite/ld-elf/dt-relr-2i.d
@@ -0,0 +1,17 @@
+#source: dt-relr-2.s
+#ld: -e _start -pie --no-keep-memory $DT_RELR_LDFLAGS
+#readelf: -rW -d
+#target: [supports_dt_relr]
+
+#...
+ 0x[0-9a-f]+ \(RELR\)    +0x[0-9a-f]+
+ 0x[0-9a-f]+ \(RELRSZ\)  +(8|16) \(bytes\)
+ 0x[0-9a-f]+ \(RELRENT\) +(4|8) \(bytes\)
+#...
+Relocation section '\.rel(a|)\.dyn' at offset 0x[0-9a-f]+ contains 1 entry:
+#...
+[0-9a-f]+ +[0-9a-f]+ +R_.*_(RELATIVE|UADDR.*) .*
+#...
+Relocation section '\.relr\.dyn' at offset 0x[0-9a-f]+ contains 2 entries:
+  4 offsets
+#pass
-- 
2.38.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] x86-64: Allocate input section memory if needed
  2022-12-27 19:47 [PATCH] x86-64: Allocate input section memory if needed H.J. Lu
@ 2022-12-28  1:16 ` Alan Modra
  2022-12-28 17:23   ` H.J. Lu
  2023-01-03 21:59 ` H.J. Lu
  1 sibling, 1 reply; 7+ messages in thread
From: Alan Modra @ 2022-12-28  1:16 UTC (permalink / raw)
  To: H.J. Lu; +Cc: binutils

On Tue, Dec 27, 2022 at 11:47:56AM -0800, H.J. Lu via Binutils wrote:
> +			  /* Cache the section contents for
> +			     elf_link_input_bfd.  */
> +			  elf_section_data (sec)->this_hdr.contents
> +			    = contents;

You shouldn't really be caching unaltered section contents when
!info->keep_memory.  I'm not saying the patch is wrong, but please fix
this when you have some time.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] x86-64: Allocate input section memory if needed
  2022-12-28  1:16 ` Alan Modra
@ 2022-12-28 17:23   ` H.J. Lu
  2022-12-28 22:06     ` Alan Modra
  0 siblings, 1 reply; 7+ messages in thread
From: H.J. Lu @ 2022-12-28 17:23 UTC (permalink / raw)
  To: Alan Modra; +Cc: binutils

On Tue, Dec 27, 2022 at 5:16 PM Alan Modra <amodra@gmail.com> wrote:
>
> On Tue, Dec 27, 2022 at 11:47:56AM -0800, H.J. Lu via Binutils wrote:
> > +                       /* Cache the section contents for
> > +                          elf_link_input_bfd.  */
> > +                       elf_section_data (sec)->this_hdr.contents
> > +                         = contents;
>
> You shouldn't really be caching unaltered section contents when
> !info->keep_memory.  I'm not saying the patch is wrong, but please fix
> this when you have some time.
>
> --
> Alan Modra
> Australia Development Lab, IBM

elf_x86_64_scan_relocs has

  if (elf_section_data (sec)->this_hdr.contents != contents)
    {
      if (!converted && !_bfd_link_keep_memory (info))
        free (contents);
      else
        {
          /* Cache the section contents for elf_link_input_bfd if any
             load is converted or --no-keep-memory isn't used.  */
          elf_section_data (sec)->this_hdr.contents = contents;
          info->cache_size += sec->size;
        }
    }

Are you suggesting that it should be

  if (elf_section_data (sec)->this_hdr.contents != contents)
    {
      if (!converted)
        free (contents);
      else
        {
          /* Cache the section contents for elf_link_input_bfd if any
             load is converted.  */
          elf_section_data (sec)->this_hdr.contents = contents;
          info->cache_size += sec->size;
        }
    }

Thanks.

--
H.J.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] x86-64: Allocate input section memory if needed
  2022-12-28 17:23   ` H.J. Lu
@ 2022-12-28 22:06     ` Alan Modra
  2022-12-28 23:44       ` H.J. Lu
  0 siblings, 1 reply; 7+ messages in thread
From: Alan Modra @ 2022-12-28 22:06 UTC (permalink / raw)
  To: H.J. Lu; +Cc: binutils

On Wed, Dec 28, 2022 at 09:23:21AM -0800, H.J. Lu wrote:
> On Tue, Dec 27, 2022 at 5:16 PM Alan Modra <amodra@gmail.com> wrote:
> >
> > On Tue, Dec 27, 2022 at 11:47:56AM -0800, H.J. Lu via Binutils wrote:
> > > +                       /* Cache the section contents for
> > > +                          elf_link_input_bfd.  */
> > > +                       elf_section_data (sec)->this_hdr.contents
> > > +                         = contents;
> >
> > You shouldn't really be caching unaltered section contents when
> > !info->keep_memory.  I'm not saying the patch is wrong, but please fix
> > this when you have some time.
> >
> > --
> > Alan Modra
> > Australia Development Lab, IBM
> 
> elf_x86_64_scan_relocs has
> 
>   if (elf_section_data (sec)->this_hdr.contents != contents)
>     {
>       if (!converted && !_bfd_link_keep_memory (info))
>         free (contents);
>       else
>         {
>           /* Cache the section contents for elf_link_input_bfd if any
>              load is converted or --no-keep-memory isn't used.  */
>           elf_section_data (sec)->this_hdr.contents = contents;
>           info->cache_size += sec->size;
>         }
>     }
> 
> Are you suggesting that it should be

No, the code above is correct and how it should be in
elf_x86_size_or_finish_relative_reloc.  Of course it is a little more
complicated there due to handling multiple sections.

> 
>   if (elf_section_data (sec)->this_hdr.contents != contents)
>     {
>       if (!converted)
>         free (contents);
>       else
>         {
>           /* Cache the section contents for elf_link_input_bfd if any
>              load is converted.  */
>           elf_section_data (sec)->this_hdr.contents = contents;
>           info->cache_size += sec->size;
>         }
>     }

This would be wrong as it ignores info->keep_memory.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] x86-64: Allocate input section memory if needed
  2022-12-28 22:06     ` Alan Modra
@ 2022-12-28 23:44       ` H.J. Lu
  2022-12-29  0:57         ` Alan Modra
  0 siblings, 1 reply; 7+ messages in thread
From: H.J. Lu @ 2022-12-28 23:44 UTC (permalink / raw)
  To: Alan Modra; +Cc: binutils

On Wed, Dec 28, 2022 at 2:07 PM Alan Modra <amodra@gmail.com> wrote:
>
> On Wed, Dec 28, 2022 at 09:23:21AM -0800, H.J. Lu wrote:
> > On Tue, Dec 27, 2022 at 5:16 PM Alan Modra <amodra@gmail.com> wrote:
> > >
> > > On Tue, Dec 27, 2022 at 11:47:56AM -0800, H.J. Lu via Binutils wrote:
> > > > +                       /* Cache the section contents for
> > > > +                          elf_link_input_bfd.  */
> > > > +                       elf_section_data (sec)->this_hdr.contents
> > > > +                         = contents;
> > >
> > > You shouldn't really be caching unaltered section contents when
> > > !info->keep_memory.  I'm not saying the patch is wrong, but please fix
> > > this when you have some time.
> > >
> > > --
> > > Alan Modra
> > > Australia Development Lab, IBM
> >
> > elf_x86_64_scan_relocs has
> >
> >   if (elf_section_data (sec)->this_hdr.contents != contents)
> >     {
> >       if (!converted && !_bfd_link_keep_memory (info))
> >         free (contents);
> >       else
> >         {
> >           /* Cache the section contents for elf_link_input_bfd if any
> >              load is converted or --no-keep-memory isn't used.  */
> >           elf_section_data (sec)->this_hdr.contents = contents;
> >           info->cache_size += sec->size;
> >         }
> >     }
> >
> > Are you suggesting that it should be
>
> No, the code above is correct and how it should be in
> elf_x86_size_or_finish_relative_reloc.  Of course it is a little more

But elf_x86_size_or_finish_relative_reloc modifies the section
contents by storing r_addend in it.

> complicated there due to handling multiple sections.
>
> >
> >   if (elf_section_data (sec)->this_hdr.contents != contents)
> >     {
> >       if (!converted)
> >         free (contents);
> >       else
> >         {
> >           /* Cache the section contents for elf_link_input_bfd if any
> >              load is converted.  */
> >           elf_section_data (sec)->this_hdr.contents = contents;
> >           info->cache_size += sec->size;
> >         }
> >     }
>
> This would be wrong as it ignores info->keep_memory.
>
> --
> Alan Modra
> Australia Development Lab, IBM



-- 
H.J.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] x86-64: Allocate input section memory if needed
  2022-12-28 23:44       ` H.J. Lu
@ 2022-12-29  0:57         ` Alan Modra
  0 siblings, 0 replies; 7+ messages in thread
From: Alan Modra @ 2022-12-29  0:57 UTC (permalink / raw)
  To: H.J. Lu; +Cc: binutils

On Wed, Dec 28, 2022 at 03:44:55PM -0800, H.J. Lu wrote:
> On Wed, Dec 28, 2022 at 2:07 PM Alan Modra <amodra@gmail.com> wrote:
> >
> > On Wed, Dec 28, 2022 at 09:23:21AM -0800, H.J. Lu wrote:
> > > On Tue, Dec 27, 2022 at 5:16 PM Alan Modra <amodra@gmail.com> wrote:
> > > >
> > > > On Tue, Dec 27, 2022 at 11:47:56AM -0800, H.J. Lu via Binutils wrote:
> > > > > +                       /* Cache the section contents for
> > > > > +                          elf_link_input_bfd.  */
> > > > > +                       elf_section_data (sec)->this_hdr.contents
> > > > > +                         = contents;
> > > >
> > > > You shouldn't really be caching unaltered section contents when
> > > > !info->keep_memory.  I'm not saying the patch is wrong, but please fix
> > > > this when you have some time.
> 
> But elf_x86_size_or_finish_relative_reloc modifies the section
> contents by storing r_addend in it.

Oh, so it does.  I missed seeing that and thought contents were
unaltered.  Yes, you always need to cache contents in that case.
Sorry for the noise.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] x86-64: Allocate input section memory if needed
  2022-12-27 19:47 [PATCH] x86-64: Allocate input section memory if needed H.J. Lu
  2022-12-28  1:16 ` Alan Modra
@ 2023-01-03 21:59 ` H.J. Lu
  1 sibling, 0 replies; 7+ messages in thread
From: H.J. Lu @ 2023-01-03 21:59 UTC (permalink / raw)
  To: binutils

On Tue, Dec 27, 2022 at 11:47 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> When --no-keep-memory is used, the input section memory may not be cached.
> Allocate input section memory for -z pack-relative-relocs if needed.
>
> bfd/
>
>         PR ld/29939
>         * elfxx-x86.c (elf_x86_size_or_finish_relative_reloc): Allocate
>         input section memory if needed.
>
> ld/
>
>         PR ld/29939
>         * testsuite/ld-elf/dt-relr-2i.d: New test.
> ---
>  bfd/elfxx-x86.c                  | 25 +++++++++++++++++++++++--
>  ld/testsuite/ld-elf/dt-relr-2i.d | 17 +++++++++++++++++
>  2 files changed, 40 insertions(+), 2 deletions(-)
>  create mode 100644 ld/testsuite/ld-elf/dt-relr-2i.d
>
> diff --git a/bfd/elfxx-x86.c b/bfd/elfxx-x86.c
> index 2ddca340473..ec86e75eba9 100644
> --- a/bfd/elfxx-x86.c
> +++ b/bfd/elfxx-x86.c
> @@ -1541,12 +1541,33 @@ elf_x86_size_or_finish_relative_reloc
>                     }
>                   else
>                     {
> +                     bfd_byte *contents;
> +
>                       if (rel.r_offset >= sec->size)
>                         abort ();
> +
> +                     if (elf_section_data (sec)->this_hdr.contents
> +                         != NULL)
> +                       contents
> +                         = elf_section_data (sec)->this_hdr.contents;
> +                     else
> +                       {
> +                         if (!bfd_malloc_and_get_section (sec->owner,
> +                                                          sec,
> +                                                          &contents))
> +                           info->callbacks->einfo
> +                             /* xgettext:c-format */
> +                             (_("%F%P: %pB: failed to allocate memory for section `%pA'\n"),
> +                              info->output_bfd, sec);
> +
> +                         /* Cache the section contents for
> +                            elf_link_input_bfd.  */
> +                         elf_section_data (sec)->this_hdr.contents
> +                           = contents;
> +                       }
>                       htab->elf_write_addend
>                         (info->output_bfd, outrel->r_addend,
> -                        (elf_section_data (sec)->this_hdr.contents
> -                         + rel.r_offset));
> +                        contents + rel.r_offset);
>                     }
>                 }
>             }
> diff --git a/ld/testsuite/ld-elf/dt-relr-2i.d b/ld/testsuite/ld-elf/dt-relr-2i.d
> new file mode 100644
> index 00000000000..ed0ef9ccded
> --- /dev/null
> +++ b/ld/testsuite/ld-elf/dt-relr-2i.d
> @@ -0,0 +1,17 @@
> +#source: dt-relr-2.s
> +#ld: -e _start -pie --no-keep-memory $DT_RELR_LDFLAGS
> +#readelf: -rW -d
> +#target: [supports_dt_relr]
> +
> +#...
> + 0x[0-9a-f]+ \(RELR\)    +0x[0-9a-f]+
> + 0x[0-9a-f]+ \(RELRSZ\)  +(8|16) \(bytes\)
> + 0x[0-9a-f]+ \(RELRENT\) +(4|8) \(bytes\)
> +#...
> +Relocation section '\.rel(a|)\.dyn' at offset 0x[0-9a-f]+ contains 1 entry:
> +#...
> +[0-9a-f]+ +[0-9a-f]+ +R_.*_(RELATIVE|UADDR.*) .*
> +#...
> +Relocation section '\.relr\.dyn' at offset 0x[0-9a-f]+ contains 2 entries:
> +  4 offsets
> +#pass
> --
> 2.38.1
>

I am backporting it to 2.39 branch.

-- 
H.J.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-01-03 22:00 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-27 19:47 [PATCH] x86-64: Allocate input section memory if needed H.J. Lu
2022-12-28  1:16 ` Alan Modra
2022-12-28 17:23   ` H.J. Lu
2022-12-28 22:06     ` Alan Modra
2022-12-28 23:44       ` H.J. Lu
2022-12-29  0:57         ` Alan Modra
2023-01-03 21:59 ` H.J. Lu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).