[PATCH] D74755: [llvm-objcopy] Attribute an empty section to a segment ending at its address

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D74755#1956876 <https://reviews.llvm.org/D74755#1956876>, @jhenderson wrote:

> I've played around with the llvm-objcopy's with and without this patch to see the impact of the change. One situation I noticed is where using the `--remove-section` option, combined with the `--only-keep-debug` option. In the example I'm using, there is a PT_LOAD which contains a PT_GNU_RELRO, which contains, at its end, a PT_TLS, with another relro section before the TLS segment. (Note: the segment types, apart from the PT_LOAD are irrelevant here - a custom linker script could have all sorts of different segment types where this layout could happen). If the relro section is removed by objcopy, the output from only-keep-debug differs before and after this patch. I haven't experimented yet due to time constraints, but I wouldn't be entirely surprised if the same behaviour difference would be observed if --remove-section was performed in a prior run of llvm-objcopy, or if the section was replaced with segment padding of some variety.
>
> Here is the input source I used:
>
>   // test.cpp
>   thread_local int tdata = 42;
>   extern void (* const ptr)();
>   extern "C" void _start (){}
>   void (* const ptr)() = &_start;
>  
>   // test.script - derived from LLD's output without a linker script but with the relro and tls segments and sections swapped.
>   PHDRS
>   {
>       ph_phdr           PT_PHDR PHDRS   FLAGS (0x4);
>       ph_ro             PT_LOAD FILEHDR PHDRS FLAGS (0x4);
>       ph_text           PT_LOAD           FLAGS (0x1 | 0x4);
>       ph_data           PT_LOAD           FLAGS (0x2 | 0x4);
>       ph_relro          PT_GNU_RELRO      FLAGS (0x4);
>       ph_tls            PT_TLS            FLAGS (0x4);
>   }
>   SECTIONS
>   {
>       . = 0x200000 + SIZEOF_HEADERS;
>       .eh_frame : { *(.eh_frame) } : ph_ro
>       .text : { *(.text .text.*) } : ph_text
>       .data.rel.ro : { *(.data.rel.ro .data.rel.ro*) } : ph_data : ph_relro
>       .tdata : { *(.tdata .tdata.*) } : ph_data : ph_relro : ph_tls
>   }
>  
>   // Compile and link this using clang+LLD. I then ran the following commands:
>   C:\Work\D74755> .\patched-objcopy.exe test.elf test.okd-patched.elf --only-keep-debug --remove-section=.data.rel.ro
>   C:\Work\D74755> .\base-objcopy.exe test.elf test.okd-base.elf --only-keep-debug --remove-section=.data.rel.ro
>   C:\Work\D74755> C:\llvm\build\Debug\bin\llvm-readelf --sections -l .\test.okd-base.elf > 1.txt
>   C:\Work\D74755> C:\llvm\build\Debug\bin\llvm-readelf --sections -l .\test.okd-patched.elf > 2.txt
>
>
> 1.txt and 2.txt illustrate that the section header offsets have been modified, such that they do not match those of the equivalent ELF file (i.e. one only created with just --remove-section=.data.rel.ro applied). I do not know how debuggers use the only-keep-debug information, but if they use section offsets, the patched version of objcopy will break them, as they will no longer match up. I will look at the remaining differences another time.


Thanks for making the investigations!

I am not concerned about file offset changes with --only-keep-debug --remove-section. First, in an --only-keep-debug produced debug file, section contents other than .debug_* are not used by debuggers. Second, users will not combine the two options. File offsets are expected to be rewritten to keep the file size small.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74755/new/

https://reviews.llvm.org/D74755