[PATCH] ld: ensure build-id is placed near ELF headers

Andrew Burgess aburgess@redhat.com
Mon Sep 2 10:26:49 GMT 2024


Nick Clifton <nickc@redhat.com> writes:

> Hi Andrew,
>
>    [Sorry for being so late in responding to your email]
>
>> When an executable is built with the --rosegment option GDB is no
>> longer able to find the build-id of the executable from a core file.
>
> For problems like this it really helps if you create a bug report
> using the Sourceware bugzilla system.  This allows us to track the
> problem and any discussion surrounding it.  It also means that we
> can refer to the bug ID in comments in the code.
>
> In order to save you time however, I have gone ahead and created
> a bug report for you:
>
>    https://sourceware.org/bugzilla/show_bug.cgi?id=32100
>
>> This patch aims to fix this by placing the build-id first.  The
>> build-id will then be included within the same LOAD-able segment as
>> the executable content, just as the ELF headers are.  With this patch
>> in place GDB is once again able to find the build-id from a core
>> file.
>
> Unfortunately that patch breaks the intention of the --rosegment
> option by restoring two loadable, read-only segments to the executable.
>
> Here is an example:
>
>    $ cat hello.c
>
>    extern int printf (const char *, ...);
>    int i = 42;
>    const int * j = & i;
>    int main (void) { return printf ("hello world %d\n", * j); }
>
>    $ gcc -fPIC -Wl,-z,noseparate-code hello.c
>    $ readelf -lW a.out | grep LOAD
>
>    LOAD           0x000000 0x0000000000400000 0x0000000000400000 0x0006d4 0x0006d4 R E 0x1000
>    LOAD           0x000df8 0x0000000000401df8 0x0000000000401df8 0x000228 0x000230 RW  0x1000
>
>    $ gcc -fPIX -Wl,-z,separate-code hello.c
>    $ readelf -lW a.out | grep LOAD
>
>    LOAD           0x000000 0x0000000000400000 0x0000000000400000 0x000510 0x000510 R   0x1000
>    LOAD           0x001000 0x0000000000401000 0x0000000000401000 0x000155 0x000155 R E 0x1000
>    LOAD           0x002000 0x0000000000402000 0x0000000000402000 0x0000dc 0x0000dc R   0x1000
>    LOAD           0x002df8 0x0000000000403df8 0x0000000000403df8 0x000228 0x000230 RW  0x1000
>
>    $ gcc -fPIX -Wl,-z,separate-code -Wl,--rosegment hello.c
>    $ readelf -lW a.out | grep LOAD
>
>    LOAD           0x000000 0x0000000000400000 0x0000000000400000 0x00115d 0x00115d R E 0x1000
>    LOAD           0x002000 0x0000000000402000 0x0000000000402000 0x0002d4 0x0002d4 R   0x1000
>    LOAD           0x002df8 0x0000000000403df8 0x0000000000403df8 0x000220 0x000228 RW  0x1000
>
>    $ gcc -fPIX -Wl,-z,separate-code -Wl,--rosegment hello.c -fuse-ld=patched-linker
>    $ readelf -lW a.out | grep LOAD
>
>    LOAD           0x000000 0x0000000000400000 0x0000000000400000 0x00039c 0x00039c R   0x1000
>    LOAD           0x001000 0x0000000000401000 0x0000000000401000 0x00015d 0x00015d R E 0x1000
>    LOAD           0x002000 0x0000000000402000 0x0000000000402000 0x00024c 0x00024c R   0x1000
>    LOAD           0x002df8 0x0000000000403df8 0x0000000000403df8 0x000220 0x000228 RW  0x1000
>
>    (I invented the "-fuse-ld=patched-linker" option to keep the presentation simple.  In
>    reality I used a full path to a linker built with your proposed patch applied).
>
> The section to segment mapping is changed by using --rosegment, but with your patch
> applied we still get an early read-only segment containing the note sections.
>
> I think that what is needed is a patch to move all of the read-only segments to
> before the read-execute segment.  But this weill need testing.

Hi Nick,

First, what you've written above is 100% correct, and the patch I posted
did break the "only one r/o segment" goal.  However, I think there are
some additional, interesting observations to be made from the experiment
you performed.  I'll discuss these results below.

Second, I've read the link that Fangrui Song posted[1], which is yet
another voice arguing for r/o content to be placed before the text
content.  However, the initial suggestion is that this would be opt in,
which would still leave GDB's build-id lookup broken by default when
--rosegment was used, which is less than ideal.

Third, I've read up on bug PR ld/30907[2] which seems to be the
motivator for the addition of --rosegment.  The initial complaint
appears to be about file size rather than the number of segments.  It's
just that reducing the number of segments helps reduce the file size.
The description of --rosegment does mention creating a single r/o
segment, which is fine, but, I think, we should keep the original goal
(file size) in mind.

OK, so the analysis.  I created four binaries using the same approach
Nick took (this experiment was done on AArch64, not x86-64):

  $ cat hello.c
  extern int printf (const char *, ...);
  int i = 42;
  const int * j = & i;
  int main (void) { return printf ("hello world %d\n", * j); }
  $ cat Makefile
  CC := gcc
  
  LD_NEW := /tmp/binutils-gdb/install.patched/bin
  LD_OLD := /tmp/binutils-gdb/install.master/bin
  
  all:
          $(CC) -B$(LD_OLD) -fPIC -Wl,-z,noseparate-code hello.c               -o hello.nosep-code.x
          $(CC) -B$(LD_OLD) -fPIC -Wl,-z,separate-code hello.c                 -o hello.sep-code.x
          $(CC) -B$(LD_OLD) -fPIC -Wl,-z,separate-code -Wl,--rosegment hello.c -o hello.roseg.x
          $(CC) -B$(LD_NEW) -fPIC -Wl,-z,separate-code -Wl,--rosegment hello.c -o hello.patched.x
  
  sizes:
          ls -l hello.*.x
  
  segments:
          for F in $$(ls -1 hello.*.x); do echo $$F; readelf -lW $$F | grep LOAD; done

After building the binaries using `make`, I then looked at the results:

  $ make segments
  for F in $(ls -1 hello.*.x); do echo $F; readelf -lW $F | grep LOAD; done
  hello.nosep-code.x
    LOAD           0x000000 0x0000000000400000 0x0000000000400000 0x00083c 0x00083c R E 0x10000
    LOAD           0x00fde0 0x000000000041fde0 0x000000000041fde0 0x000250 0x000258 RW  0x10000
  hello.patched.x
    LOAD           0x000000 0x0000000000400000 0x0000000000400000 0x0002ec 0x0002ec R   0x10000
    LOAD           0x010000 0x0000000000410000 0x0000000000410000 0x0001f4 0x0001f4 R E 0x10000
    LOAD           0x020000 0x0000000000420000 0x0000000000420000 0x00039c 0x00039c R   0x10000
    LOAD           0x02fde0 0x000000000043fde0 0x000000000043fde0 0x000250 0x000258 RW  0x10000
  hello.roseg.x
    LOAD           0x000000 0x0000000000400000 0x0000000000400000 0x0101f4 0x0101f4 R E 0x10000
    LOAD           0x020000 0x0000000000420000 0x0000000000420000 0x0003dc 0x0003dc R   0x10000
    LOAD           0x02fde0 0x000000000043fde0 0x000000000043fde0 0x000250 0x000258 RW  0x10000
  hello.sep-code.x
    LOAD           0x000000 0x0000000000400000 0x0000000000400000 0x000540 0x000540 R   0x10000
    LOAD           0x010000 0x0000000000410000 0x0000000000410000 0x0001f4 0x0001f4 R E 0x10000
    LOAD           0x020000 0x0000000000420000 0x0000000000420000 0x000144 0x000144 R   0x10000
    LOAD           0x02fde0 0x000000000043fde0 0x000000000043fde0 0x000250 0x000258 RW  0x10000

This confirms what Nick saw, the patched binary does have an extra LOAD
segment.  Next I tried this:

  $ make sizes
  ls -l hello.*.x
  -rwxr-xr-x. 1 andrew andrew  71512 Sep  2 05:39 hello.nosep-code.x
  -rwxr-xr-x. 1 andrew andrew 202584 Sep  2 05:39 hello.patched.x
  -rwxr-xr-x. 1 andrew andrew 202584 Sep  2 05:39 hello.roseg.x
  -rwxr-xr-x. 1 andrew andrew 202584 Sep  2 05:39 hello.sep-code.x

Which, I think, is interesting.  Despite having the extra segment, the
patched linker didn't trigger an increase in file size.  So what's going
on?

Taking a look at the 'make segments' output for 'hello.patched.x' and
'hello.roseg.x' we see this, I've added some labels on the left to make
it easier to reference the lines:

  hello.patched.x
  [1]  LOAD           0x000000 0x0000000000400000 0x0000000000400000 0x0002ec 0x0002ec R   0x10000
  [2]  LOAD           0x010000 0x0000000000410000 0x0000000000410000 0x0001f4 0x0001f4 R E 0x10000
  [3]  LOAD           0x020000 0x0000000000420000 0x0000000000420000 0x00039c 0x00039c R   0x10000
  [4]  LOAD           0x02fde0 0x000000000043fde0 0x000000000043fde0 0x000250 0x000258 RW  0x10000
  hello.roseg.x
  [5]  LOAD           0x000000 0x0000000000400000 0x0000000000400000 0x0101f4 0x0101f4 R E 0x10000
  [6]  LOAD           0x020000 0x0000000000420000 0x0000000000420000 0x0003dc 0x0003dc R   0x10000
  [7]  LOAD           0x02fde0 0x000000000043fde0 0x000000000043fde0 0x000250 0x000258 RW  0x10000

Lines [3] and [6] are both the r/o data segment.  Notice that they both
start at file offset 0x20000.  This means that by this point in the ELF
the files have somehow synchronised.

Now the size of [3] is slightly smaller than [6] as some content has
moved into the new earlier segment [1], but despite this segments [4]
and [7] (the r/w data) still start at the same file offset, and not
surprisingly, are the same size.  And so, overall, the files are the
same size.

Given that [4] and [7] don't appear to have any particular alignment,
I'm a little surprised that [4] didn't move.  I haven't looked into this
at this point, because I don't think this is the most interesting part.

What is interesting is why [3] and [6] start at the same file offset.

If we look at [2] and [5] we can clearly see that [2] starts one page
later than [5], however, if we look at the file size of [2] you'll
notice that it is a page smaller than [5]!  What's going on here?

The answer is in the linker script.  Here's the unpatched script used,
on AArch64, when '-z separate-code --rosegment' is in use.  I've wrapped
some of the long lines at the ';' to make it more readable:

  SECTIONS
  {
    PROVIDE (__executable_start = SEGMENT_START("text-segment", 0x400000)); 
    . = SEGMENT_START("text-segment", 0x400000) + SIZEOF_HEADERS;
    . = ALIGN(CONSTANT (MAXPAGESIZE));
    .init           :
    {
      KEEP (*(SORT_NONE(.init)))
    } =0x1f2003d5
    .plt            : ALIGN(16) { *(.plt) *(.iplt) }
    .text           :
    {
      ... snip ...
    } =0x1f2003d5
  
    ... snip ...
  }

And here's the patched version of the same script:

  SECTIONS
  {
    PROVIDE (__executable_start = SEGMENT_START("text-segment", 0x400000));
    . = SEGMENT_START("text-segment", 0x400000) + SIZEOF_HEADERS;
    /* Place build-id as close to the ELF headers as possible.  This
       maximises the chance the build-id will be present in core files,
       which GDB can use to improve the debug experience.  */
    .note.gnu.build-id  : { *(.note.gnu.build-id) }
    . = ALIGN(CONSTANT (MAXPAGESIZE));
    .init           :
    {
      KEEP (*(SORT_NONE(.init)))
    } =0x1f2003d5
    .plt            : ALIGN(16) { *(.plt) *(.iplt) }
    .text           :
    {
      ... snip ...
    } =0x1f2003d5
  
    ... snip ...
  }

Notice that in both cases '.' is initialised to a non page aligned
value, unless overridden by the user, it will be:

  0x400000 + SIZEOF_HEADERS

This is then aligned up to a page boundary.

In both cases the linker is responsible for creating the section to
segment map.  In the unpatched case the linker will collect everything
from the start of the ELF (so the ELF headers) into the first segment,
which is the executable segment.  This also includes the gap that is
being left after the ELF headers and before the executable sections.

In the patched case the build-id easily fits into the gap between the
ELF headers and the aligned start of the code sections.  Even on targets
with 4k pages.  Of course, in the patched case, as the linker builds the
section to segment map, it figures that it should create two segments,
the first being r/o which holds the ELF headers and the build-id, and
the second being r/x to hold the code.  The linker can do this because
we still have the forced page alignment before the code.  But this has
no impact on file size .... which was the original goal of --rosegment.

Now, when looking at this, I did wonder, is that initial alignment still
needed?  I guess that it made sense when we have actual data content
before the text content.  But if all we have before the text is the ELF
headers, and if (apparently) we are OK having those headers mapped into
the r/x segment, then surely we could drop the alignment?

If we do that, but keep the rest of my patch (moving the build-id) then
we have a linker script that looks like this:

  SECTIONS
  {
    PROVIDE (__executable_start = SEGMENT_START("text-segment", 0x400000));
    . = SEGMENT_START("text-segment", 0x400000) + SIZEOF_HEADERS;
    /* Place build-id as close to the ELF headers as possible.  This
       maximises the chance the build-id will be present in core files,
       which GDB can use to improve the debug experience.  */
    .note.gnu.build-id  : { *(.note.gnu.build-id) }
    .init           :
    {
      KEEP (*(SORT_NONE(.init)))
    } =0x1f2003d5
    .plt            : ALIGN(16) { *(.plt) *(.iplt) }
    .text           :
    {
      ... snip ...
    } =0x1f2003d5
  
    ... snip ...
  }

And when we rebuild the test binaries, here are the two most interesting
results ('hello.roseg.x' is still using the unpatched, vanilla, upstream
linker):

  hello.patched.x
    LOAD           0x000000 0x0000000000400000 0x0000000000400000 0x0004b4 0x0004b4 R E 0x10000
    LOAD           0x010000 0x0000000000410000 0x0000000000410000 0x00039c 0x00039c R   0x10000
    LOAD           0x01fde0 0x000000000042fde0 0x000000000042fde0 0x000250 0x000258 RW  0x10000
  hello.roseg.x
    LOAD           0x000000 0x0000000000400000 0x0000000000400000 0x0101f4 0x0101f4 R E 0x10000
    LOAD           0x020000 0x0000000000420000 0x0000000000420000 0x0003dc 0x0003dc R   0x10000
    LOAD           0x02fde0 0x000000000043fde0 0x000000000043fde0 0x000250 0x000258 RW  0x10000

Better, we still have just 3 segments.  The first segment with
permissions 'R E' does contain the ELF headers in both cases, and in the
patched case it also contains the build-id note.

And as an added bonus, the file sizes:

  $ make sizes
  ls -l hello.*.x
  -rwxr-xr-x. 1 andrew andrew  71512 Sep  2 06:13 hello.nosep-code.x
  -rwxr-xr-x. 1 andrew andrew 137048 Sep  2 06:13 hello.patched.x
  -rwxr-xr-x. 1 andrew andrew 202584 Sep  2 06:13 hello.roseg.x
  -rwxr-xr-x. 1 andrew andrew 202584 Sep  2 06:13 hello.sep-code.x

The patched binary gets smaller due to the removal of the page
alignment.

Obviously, the removal of the alignment could be done without moving the
build-id note, which would provide file size improvements.

So, why am I still pushing this patch when "just moving the data before
the text" is the obviously better solution?  My concerns are mostly
those discussed in [2], `ld` has had text before data for so long, I'm
nervous proposing moving all that content just to fix this one issue.
Especially if, as in [2] it was done opt-out be default, that would
leave this feature broken in this case.  Placing the build-id separately
to the rest of the data content highlights its slightly special
placement requirements, and hopefully will stop it getting broken again
in the future.

The patch below is an RFC rather than a full patch, I've not update the
commit message to describe the changes to the alignment code yet, but I
wanted to share this so we could discuss it.

Thanks,
Andrew

[1] https://sourceware.org/pipermail/binutils/2024-June/134541.html
[2] https://sourceware.org/bugzilla/show_bug.cgi?id=30907

---

diff --git a/ld/scripttempl/elf.sc b/ld/scripttempl/elf.sc
index 54716110b61..b20c5ccc9a1 100644
--- a/ld/scripttempl/elf.sc
+++ b/ld/scripttempl/elf.sc
@@ -425,7 +425,6 @@ emit_early_ro()
 {
   cat <<EOF
   ${INITIAL_READONLY_SECTIONS}
-  .note.gnu.build-id ${RELOCATING-0}: { *(.note.gnu.build-id) }
 EOF
 }
 
@@ -436,6 +435,11 @@ cat <<EOF
   ${CREATE_SHLIB-${CREATE_PIE-${RELOCATING+PROVIDE (__executable_start = ${TEXT_START_ADDR}); . = ${TEXT_BASE_ADDRESS};}}}
   ${CREATE_SHLIB+${RELOCATING+. = ${SHLIB_TEXT_START_ADDR}${SIZEOF_HEADERS_CODE};}}
   ${CREATE_PIE+${RELOCATING+PROVIDE (__executable_start = ${SHLIB_TEXT_START_ADDR}); . = ${SHLIB_TEXT_START_ADDR}${SIZEOF_HEADERS_CODE};}}
+
+  /* Place build-id as close to the ELF headers as possible.  This
+     maximises the chance the build-id will be present in core files,
+     which GDB can use to improve the debug experience.  */
+  .note.gnu.build-id ${RELOCATING-0}: { *(.note.gnu.build-id) }
 EOF
 }
 
@@ -800,9 +804,12 @@ EOF
   if test -z "${ALL_TEXT_BEFORE_RO}"; then
     test -n "${SEPARATE_CODE}" || emit_early_ro
     test -n "${NON_ALLOC_DYN}${SEPARATE_CODE}" || emit_dyn
+
+    if test -n "${SEPARATE_CODE}" -o -n "${NON_ALLOC_DYN}${SEPARATE_CODE}"; then
+      align_text
+    fi
   fi
-  
-  align_text  
+
   emit_text
 
   align_rodata



More information about the Binutils mailing list