[PATCH] ld: ensure build-id is placed near ELF headers
Andrew Burgess
aburgess@redhat.com
Tue Sep 10 15:38:50 GMT 2024
Nick Clifton <nickc@redhat.com> writes:
> Hi Andrew,
>
> [Sorry for the delay in replying - I have been on PTO...]
>
>> 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.
>
> Just as a matter of interest, have you checked lld's current behaviour ?
> (Ideally with and without --rosegment and/or -z separate-code).
Here are the results using lld 17.0.6:
$ gcc -fPIC -fuse-ld=lld -Wl,-z,noseparate-code -Wl,--no-rosegment hello.c -o hello.lld.nosep-code.x
$ gcc -fPIC -fuse-ld=lld -Wl,-z,separate-code -Wl,--no-rosegment hello.c -o hello.lld.sep-code.x
$ gcc -fPIC -fuse-ld=lld -Wl,-z,separate-code -Wl,--rosegment hello.c -o hello.lld.sep-code-roseg.x
$ ld.lld --version
LLD 17.0.6 (compatible with GNU linkers)
$ readelf -lW hello.lld.nosep-code.x | grep LOAD\\\|NOTE
LOAD 0x000000 0x0000000000200000 0x0000000000200000 0x000690 0x000690 R E 0x1000
LOAD 0x000690 0x0000000000201690 0x0000000000201690 0x0001a0 0x0001a0 RW 0x1000
LOAD 0x000830 0x0000000000202830 0x0000000000202830 0x000030 0x000031 RW 0x1000
NOTE 0x00028c 0x000000000020028c 0x000000000020028c 0x000038 0x000038 R 0x4
$ readelf -lW hello.lld.sep-code.x | grep LOAD\\\|NOTE
LOAD 0x000000 0x0000000000200000 0x0000000000200000 0x000690 0x000690 R E 0x1000
LOAD 0x001000 0x0000000000201000 0x0000000000201000 0x0001a0 0x0001a0 RW 0x1000
LOAD 0x0011a0 0x00000000002021a0 0x00000000002021a0 0x000030 0x000031 RW 0x1000
NOTE 0x00028c 0x000000000020028c 0x000000000020028c 0x000038 0x000038 R 0x4
$ readelf -lW hello.lld.sep-code-roseg.x | grep LOAD\\\|NOTE
LOAD 0x000000 0x0000000000200000 0x0000000000200000 0x000568 0x000568 R 0x1000
LOAD 0x001000 0x0000000000201000 0x0000000000201000 0x000160 0x000160 R E 0x1000
LOAD 0x002000 0x0000000000202000 0x0000000000202000 0x0001a0 0x0001a0 RW 0x1000
LOAD 0x0021a0 0x00000000002031a0 0x00000000002031a0 0x000030 0x000031 RW 0x1000
NOTE 0x0002c4 0x00000000002002c4 0x00000000002002c4 0x000038 0x000038 R 0x4
$ ls -l hello.lld.*.x
-rwxr-xr-x. 1 andrew andrew 6064 Sep 10 16:00 hello.lld.nosep-code.x
-rwxr-xr-x. 1 andrew andrew 12576 Sep 10 16:00 hello.lld.sep-code-roseg.x
-rwxr-xr-x. 1 andrew andrew 8480 Sep 10 16:00 hello.lld.sep-code.x
In every case the NOTE segment is placed towards the start of the ELF,
which is good. In the first two cases the r/o data is merged with the
code into a single segment. In the --rosegment case the r/o data is
moved earlier, before the code, into a single r/o segment which also
continues to hold the NOTE data.
I've repeated this experiment using lld 18.1.8 and there's no change in
how the segments are laid out, or the section to segment assignment.
I think this aligns with what Fangrui Song said in their emails:
https://inbox.sourceware.org/binutils/PH7PR06MB9078C0C0A8E4634A266A9436CB8E2@PH7PR06MB9078.namprd06.prod.outlook.com/
https://inbox.sourceware.org/binutils/20240605022136.pnkv3j5sgdx6waex@google.com/
Basically suggesting that the bfd linker should place r/o data before
the code. My concern is that if this was made optional, especially if
this wasn't the default in --rosegment mode, then this would leave some
loss of functionality in place, which I'd really like to avoid if
possible.
>
>> 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.
>
> Right.
>
>> 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.
>
> I totally agree.
>
>> 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?
>
> That makes sense.
>
>
>> 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.
>
> I think that the patch looks OK, apart from one possible typo...
>
>> @@ -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
>>
>
> The intent of this change is to only invoke the align_text operation if some
> data has been emitted before the start of the emit_text operation, yes ?
>
> But emit_early_ro and emit_dyn are only invoked if the corresponding test
> *fails*. So for example:
>
> test -n "${SEPARATE_CODE}" || emit_early_ro
>
> translates to:
>
> if ${SEPARATE_CODE} is not empty do nothing, otherwise invoke emit_early_ro
>
> So the test to invoke align_text ought to be:
>
> if test -z "${SEPARATE_CODE}" -o -z "${NON_ALLOC_DYN}${SEPARATE_CODE}"; then
> align_text
> fi
>
Ugh! You are absolutely correct. I updated my patch and posted it
below, just so we have something to discuss.
Thanks,
Andrew
---
commit 33c4f1e786b5e782dae2e80af2a3f63d43ca0a39
Author: Andrew Burgess <aburgess@redhat.com>
Date: Mon Aug 12 14:46:26 2024 +0100
ld: ensure build-id is placed near ELF headers
After the commit:
commit e8e10743f7b207b21a1efb0cc9e42487080db013
Date: Thu Jun 13 15:10:15 2024 +0100
Add --rosegment option to BFD linker to stop the '-z separate-code' from generating two read-only segments.
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.
The problem is that GDB depends on what I guess was a lucky
coincidence; that the build-id was placed close to the ELF headers,
such that the build-id can be found in the first page of the ELF.
When a Linux kernel produces a core file there is specific support in
place so that the kernel will emit the first page of any private file
mapping that looks like it might contain ELF headers. If the build-id
is placed in this first page then this means the build-id will be
included into the core file, and GDB can then find it.
Using the build-id GDB can validate that it has the correct executable
loaded to debug the core file, or in some cases GDB can use the
build-id to fetch the executable that matches the core file
automatically, e.g. using debuginfod.
The Linux kernel support for this feature has existed since 2007, see
this (kernel) commit:
commit 82df39738ba9e02c057fa99b7461a56117d36119
Date: Tue Oct 16 23:27:02 2007 -0700
Add MMF_DUMP_ELF_HEADERS
I ran into this issue while testing on the latest Fedora Rawhide
build, currently the linker on this system enables --rosegment by
default and as a result GDB is not able to find the build-id for any
core file.
What's happening is that with --rosegment the linker merges the
.note.gnu.build-id sections with all the other data content and places
this after the executable content. Without the --rosegment the
.note.gnu.build-id section is placed pretty much first before the
executable and data sections.
As a consequence, without --rosegment the build-id will usually be on
the same page as the ELF headers while with --rosegment on the
executable content is first and due to alignment directives in the
linker script the NOTE segment (including the build-id) will always
appear outside the first page.
In fact, when we look at the alignment directives we see that there is
an unnecessary alignment being generated before the .text content.
This means that when --rosegment is in use we leave an unnecessary gap
after the ELF headers and before the .text content. I've fixed this
in this commit by only emitting the alignment when we have first
emitted any data content before the .text.
With that fixed I have then placed the build-id immediately after the
ELF headers. The build-id will then be included within the same
LOAD-able segment as the executable content, just as the ELF headers
are. This maybe isn't ideal as the goal of --rosegment is to avoid
placing data into an executable segment, but we currently do include
the ELF headers, so adding the NOTES seems like a necessary evil at
this point.
The other option is to move all read-only data to live before the
.text content, thus creating a r/o segment as the first segment in the
ELF. This segment would hold the ELF headers, the NOTES, and then all
the other r/o data. This is what lld does. This might be a good long
term goal for the bfd linker to work towards.
With this patch in place GDB is once again able to find the build-id
from a core file.
diff --git a/ld/scripttempl/elf.sc b/ld/scripttempl/elf.sc
index 54716110b61..97b96fae85e 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 -z "${SEPARATE_CODE}" -o -z "${NON_ALLOC_DYN}${SEPARATE_CODE}"; then
+ align_text
+ fi
fi
-
- align_text
+
emit_text
align_rodata
More information about the Binutils
mailing list