x86-64: Allocate input section memory if needed
Checks
Commit Message
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
Comments
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.
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.
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.
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
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.
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.
@@ -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);
}
}
}
new file mode 100644
@@ -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