This is an archive of the discontinued LLVM Phabricator instance.

[ELF] --emit-relocs: adjust offsets of .rel[a].eh_frame relocations
ClosedPublic

Authored by ayrtonm on Mar 24 2022, 10:00 PM.

Details

Summary

Two code paths may reach the EHFrame case in SectionBase::getOffset:

  • .eh_frame reference
  • relocation copy for --emit-relocs

The first may be used by clang_rt.crtbegin.o and GCC crtbegin*.o to get the
start address of the output .eh_frame. The relocation has an offset of 0 or
(x86-64 PC-relative leaq) -4. The current code just returns offset, which
handles this case well.

The second is related to InputSection::copyRelocations on .eh_frame (used by
--emit-relocs and -r). .eh_frame pieces may be dropped due to GC/ICF, so
we should convert the input offset to the output offset. Use the same way as
MergeInputSection with a special case handling outSecOff==-1 for an invalid
piece (see eh-frame-marker.s).

This exposes an issue in mips64-eh-abs-reloc.s that we don't reliably
handle anyway. Just add --no-check-dynamic-relocations to paper over it.

Original patch by Ayrton Muñoz

Diff Detail

Event Timeline

ayrtonm created this revision.Mar 24 2022, 10:00 PM
Herald added a project: Restricted Project. · View Herald Transcript
ayrtonm requested review of this revision.Mar 24 2022, 10:00 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 24 2022, 10:00 PM
MaskRay added a comment.EditedMar 25 2022, 4:25 PM

[ELF] Fix relocations against .eh_frame

[ELF] --emit-relocs: fix relocations in .rel[a].eh_frame.

I think this is --emit-relocs specific and the subject should mention this.

lld/ELF/InputSection.cpp
1366

Avoid used-once variable.

lld/ELF/InputSection.h
333

Only one overload is used. Remove the other one.

lld/test/ELF/mips-eh_frame-offset.s
1 ↗(On Diff #418130)

Perhaps mips-emit-relocs-eh-frame.s

This seems a generic problem, not specific to mips.
It seems to make sense to add emit-relocs-eh-frame.s for an x86-64 test.

5 ↗(On Diff #418130)
# RUN: llvm-mc -filetype=obj -triple=mips64-unknown-linux %s -o %t.o
# RUN: ld.lld --emit-relocs --entry=0 %t.o %t.o -o %t
10 ↗(On Diff #418130)

[[#%X,OFFSET:]]

11 ↗(On Diff #418130)

[[#%X,OFFSET+24]]

Prefer positive patterns to negative patterns.

MaskRay added a comment.EditedMar 28 2022, 10:04 AM

I've simplified MergeInputSection::getParentOffset and will push a version with similar changes to EhInputSection, if you haven't got around to update the patch.

This revision was not accepted when it landed; it landed in state Needs Review.Mar 28 2022, 4:23 PM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.

Hi, we are seeing build failures in our clang bots in Performing build step for 'runtimes-x86_64-unknown-linux-gnu' after this change lands, error message:

[533/1404] Linking CXX shared library /b/s/w/ir/x/w/staging/llvm_build/lib/clang/15.0.0/lib/x86_64-unknown-linux-gnu/libclang_rt.ubsan_minimal.so
FAILED: /b/s/w/ir/x/w/staging/llvm_build/lib/clang/15.0.0/lib/x86_64-unknown-linux-gnu/libclang_rt.ubsan_minimal.so 
: && /b/s/w/ir/x/w/staging/llvm_build/./bin/clang++ --target=x86_64-unknown-linux-gnu --sysroot=/b/s/w/ir/x/w/cipd/linux -fPIC --target=x86_64-unknown-linux-gnu -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -Wstring-conversion -Wmisleading-indentation -fdiagnostics-color -ffunction-sections -fdata-sections -ffile-prefix-map=/b/s/w/ir/x/w/staging/llvm_build/runtimes/runtimes-x86_64-unknown-linux-gnu-bins=../staging/llvm_build/runtimes/runtimes-x86_64-unknown-linux-gnu-bins -ffile-prefix-map=/b/s/w/ir/x/w/llvm-llvm-project/= -no-canonical-prefixes -Wall -std=c++14 -Wno-unused-parameter -O2 -g -DNDEBUG  -fuse-ld=lld  -Wl,-z,nodelete -fuse-ld=lld -Wl,--color-diagnostics   -nodefaultlibs -Wl,-z,text -nostdlib++ -shared -Wl,-soname,libclang_rt.ubsan_minimal.so -o /b/s/w/ir/x/w/staging/llvm_build/lib/clang/15.0.0/lib/x86_64-unknown-linux-gnu/libclang_rt.ubsan_minimal.so compiler-rt/lib/ubsan_minimal/CMakeFiles/RTUbsan_minimal.x86_64.dir/ubsan_minimal_handlers.cpp.o  -Wl,-rpath,"\$ORIGIN/../lib:/b/s/w/ir/x/w/staging/llvm_build/./lib"  -lc  /b/s/w/ir/x/w/staging/llvm_build/lib/clang/15.0.0/lib/x86_64-unknown-linux-gnu/libclang_rt.builtins.a && :
ld.lld: error: /b/s/w/ir/x/w/staging/llvm_build/./lib/clang/15.0.0/lib/x86_64-unknown-linux-gnu/clang_rt.crtbegin.o:(.eh_frame): offset is outside the section
ld.lld: error: /b/s/w/ir/x/w/staging/llvm_build/./lib/clang/15.0.0/lib/x86_64-unknown-linux-gnu/clang_rt.crtbegin.o:(.eh_frame): offset is outside the section
clang++: error: linker command failed with exit code 1 (use -v to see invocation)

We reproduced this issue by building revision 6faba31e0d88ce71e87567ddb51d2444524b8a81 on the same bot and it failed with the same error message: https://luci-milo.appspot.com/raw/build/logs.chromium.org/fuchsia/led/haowei_google.com/187f5403ad43469ef5de98fe44ea4170eb453eaedf1b0bda309062d30a114113/+/build.proto The same build passed on f7381a795ab235d34c94eaf01dc880eb5b89619d which is one patch ahead.

Could you take a look? If it takes some time to fix forward, could you revert this change first please? Thanks.

It looks like the relocations pointing to an empty .eh_frame in crtbegin.o have an offset greater than the size of the input section pieces. So in that case we need to revert to the old behavior of returning the offset unchanged instead of throwing a fatal error.

It looks like the relocations pointing to an empty .eh_frame in crtbegin.o have an offset greater than the size of the input section pieces. So in that case we need to revert to the old behavior of returning the offset unchanged instead of throwing a fatal error.

@ayrtonm @haowei Is there a --reproduce=/tmp/rep.tar file for the crtbegin.o with a relocation with r_offset larger than the input section piece?

It looks like the relocations pointing to an empty .eh_frame in crtbegin.o have an offset greater than the size of the input section pieces. So in that case we need to revert to the old behavior of returning the offset unchanged instead of throwing a fatal error.

@ayrtonm @haowei Is there a --reproduce=/tmp/rep.tar file for the crtbegin.o with a relocation with r_offset larger than the input section piece?

You know lld does not automatically create a reproducer. So we don't have that available. This is one of the case that an automatic lld crash reproducer would be very helpful.

I can manually create a reproducer package for you once I managed to reproduce this issue on my workstation.

I do not need a reproduce now. I can reproduce it by myself.

compiler-rt/lib/crt/crtbegin.c __do_fini has such usage:

32: 48 8d 3d 00 00 00 00          leaq    (%rip), %rdi            # 0x39 <__do_init+0x39>
          0000000000000035:  R_X86_64_PC32        .eh_frame-0x4

I'll think about it.

MaskRay reopened this revision.Mar 28 2022, 9:39 PM
MaskRay updated this revision to Diff 418779.Mar 28 2022, 9:39 PM
MaskRay retitled this revision from [ELF] Fix relocations against .eh_frame to [ELF] --emit-relocs: adjust offsets of .rel[a].eh_frame relocations.
MaskRay edited the summary of this revision. (Show Details)

Handle both cases

This revision was not accepted when it landed; it landed in state Needs Review.Mar 29 2022, 9:51 AM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
MatzeB added a subscriber: MatzeB.EditedJan 30 2023, 11:34 AM

We are seeing this change triggering exception handling unwind failures on linux/aarch64 in combination with -Wl,-z,notext. I filed a report with reproducer at https://github.com/llvm/llvm-project/issues/60392