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

[ayrtonm]

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

[MaskRay]

[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]

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.

[haowei]

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.

[ayrtonm]

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.

[MaskRay]

In D122459#3413092, @ayrtonm wrote:

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?

[haowei]

In D122459#3413101, @MaskRay wrote:

In D122459#3413092, @ayrtonm wrote:

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.

[MaskRay]

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]

Handle both cases

[MatzeB]

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

lld/ELF/InputSection.h

public:
   template <class ELFT> void split();
   template <class ELFT, class RelTy> void split(ArrayRef<RelTy> rels);
 
   // Splittable sections are handled as a sequence of data
   // rather than a single large blob of data.
   SmallVector<EhSectionPiece, 0> pieces;
 
   SyntheticSection *getParent() const;
+  uint64_t getParentOffset(uint64_t offset) const;
 };
 
 // This is a section that is added directly to an output section
 // instead of needing special combination via a synthetic section. This
 // includes all input sections with the exceptions of SHF_MERGE and
 // .eh_frame. It also includes the synthetic sections themselves.
 class InputSection : public InputSectionBase {
 public:
   InputSection(InputFile *f, uint64_t flags, uint32_t type, uint32_t alignment,
                ArrayRef<uint8_t> data, StringRef name, Kind k = Regular);
   template <class ELFT>
   InputSection(ObjFile<ELFT> &f, const typename ELFT::Shdr &header,
                StringRef name);

lld/ELF/InputSection.cpp

uint64_t SectionBase::getOffset(uint64_t offset) const {
   case Output: {
     auto *os = cast<OutputSection>(this);
     // For output sections we treat offset -1 as the end of the section.
     return offset == uint64_t(-1) ? os->size : offset;
   }
   case Regular:
   case Synthetic:
     return cast<InputSection>(this)->outSecOff + offset;
-  case EHFrame:
-    // The file crtbeginT.o has relocations pointing to the start of an empty
-    // .eh_frame that is known to be the first in the link. It does that to
-    // identify the start of the output .eh_frame.
+  case EHFrame: {
+    // Two code paths may reach here. First, clang_rt.crtbegin.o and GCC
+    // crtbegin*.o may reference the start of .eh_frame to identify the
+    // start of the output .eh_frame. The relocation has an offset of 0 or
+    // (x86-64 PC-relative leaq) -4. Just return offset.
+    //
+    // Second, InputSection::copyRelocations on .eh_frame. Some pieces may be
+    // discarded due to GC/ICF. We should compute the output section offset.
+    const EhInputSection *es = cast<EhInputSection>(this);
+    if (offset < es->rawData.size())
+      if (InputSection *isec = es->getParent())
+        return isec->outSecOff + es->getParentOffset(offset);
     return offset;
+  }
   case Merge:
     const MergeInputSection *ms = cast<MergeInputSection>(this);
     if (InputSection *isec = ms->getParent())
       return isec->outSecOff + ms->getParentOffset(offset);
     return ms->getParentOffset(offset);
   }
   llvm_unreachable("invalid section kind");
 }
while (!d.empty()) {
     pieces.emplace_back(off, this, size, getReloc(off, size, rels, relI));
     d = d.slice(size);
   }
   if (msg)
     errorOrWarn("corrupted .eh_frame: " + Twine(msg) + "\n>>> defined in " +
                 getObjMsg(d.data() - rawData.data()));
 }
 
+// Return the offset in an output section for a given input offset.
+uint64_t EhInputSection::getParentOffset(uint64_t offset) const {
+  const EhSectionPiece &piece = partition_point(
+      pieces, [=](EhSectionPiece p) { return p.inputOff <= offset; })[-1];
+  if (piece.outputOff == -1) // invalid piece
+    return offset - piece.inputOff;
+  return piece.outputOff + (offset - piece.inputOff);
+}
+
 static size_t findNull(StringRef s, size_t entSize) {
   for (unsigned i = 0, n = s.size(); i != n; i += entSize) {
     const char *b = s.begin() + i;
     if (std::all_of(b, b + entSize, [](char c) { return c == 0; }))
       return i;
   }
   llvm_unreachable("");
 }
 
 SyntheticSection *MergeInputSection::getParent() const {
   return cast_or_null<SyntheticSection>(parent);
 }
 
 // Split SHF_STRINGS section. Such section is a sequence of
 // null-terminated strings.
 void MergeInputSection::splitStrings(StringRef s, size_t entSize) {
   const bool live = !(flags & SHF_ALLOC) || !config->gcSections;
   const char *p = s.data(), *end = s.data() + s.size();
   if (!std::all_of(end - entSize, end, [](char c) { return c == 0; }))
     fatal(toString(this) + ": string is not null terminated");

lld/test/ELF/eh-frame-merge.s

 
 # CHECK:       Name      Type     Address              Off      Size   ES Flg Lk Inf Al
 # CHECK:      .eh_frame  PROGBITS [[#%x,]]             [[#%x,]] 000064 00   A  0   0  8
 # CHECK:      foo        PROGBITS {{0*}}[[#%x,FOO:]]   [[#%x,]] 000002 00  AX  0   0  1
 # CHECK-NEXT: bar        PROGBITS {{0*}}[[#%x,FOO+2]]  [[#%x,]] 000002 00  AX  0   0  1
 
 # RELOC:        Offset             Info     Type          Symbol's Value  Symbol's Name + Addend
 # RELOC-NEXT: {{0*}}[[#%x,OFF:]]   [[#%x,]] R_X86_64_PC32 [[#%x,]]        foo + 0
-# RELOC-NEXT: {{0*}}[[#%x,OFF+20]] [[#%x,]] R_X86_64_PC32 [[#%x,]]        bar + 0
-# RELOC-NEXT: {{0*}}[[#OFF]]       [[#%x,]] R_X86_64_PC32 [[#%x,]]        foo + 1
-# RELOC-NEXT: {{0*}}[[#OFF+20]]    [[#%x,]] R_X86_64_NONE 0{{$}}
+# RELOC-NEXT: {{0*}}[[#%x,OFF+24]] [[#%x,]] R_X86_64_PC32 [[#%x,]]        bar + 0
+# RELOC-NEXT: {{0*}}[[#OFF+48]]    [[#%x,]] R_X86_64_PC32 [[#%x,]]        foo + 1
+# RELOC-NEXT: {{0*}}[[#%x,OFF-24]] [[#%x,]] R_X86_64_NONE 0{{$}}
 
 # EH:          Format:                DWARF32
 # EH:        00000018 00000014 0000001c FDE cie=00000000 pc={{0*}}[[#%x,FOO:]]...
 # EH-SAME:   {{0*}}[[#%x,FOO+1]]
 # EH-COUNT-7:  DW_CFA_nop:
 # EH-EMPTY:  
 # EH:        00000030 00000014 00000034 FDE cie=00000000 pc={{0*}}[[#%x,FOO+2]]...{{0*}}[[#%x,FOO+4]]
 # EH-COUNT-7:  DW_CFA_nop:

lld/test/ELF/ehframe-relocation.s

 // CHECK-NEXT: Offset:
 // CHECK-NEXT: Size: 52
 // CHECK-NOT: .eh_frame
 
 // 0x200120 = 2097440
 // DISASM:      Disassembly of section .text:
 // DISASM-EMPTY:
 // DISASM-NEXT: <_start>:
-// DISASM-NEXT:   201154: {{.*}} movq 2097440, %rax
+// DISASM-NEXT:   movq 2097440, %rax
+// DISASM-NEXT:   leaq {{.*}}(%rip), %rax  # {{.*}} <__EH_FRAME_LIST__>
 
 .section .eh_frame,"a",@unwind
+__EH_FRAME_LIST__:
 
 .section .text
 .globl _start
 _start:
- movq .eh_frame, %rax
+ movq .eh_frame, %rax  # addend=0
+ leaq __EH_FRAME_LIST__(%rip), %rax  # addend=-4, used by libclang_rt.crtbegin.o

lld/test/ELF/mips64-eh-abs-reloc.s

 # REQUIRES: mips
 # Having an R_MIPS_64 relocation in eh_frame would previously crash LLD
 # RUN: llvm-mc -filetype=obj -triple=mips64-unknown-freebsd %s -o %t.o
 # RUN: llvm-readobj -r %t.o | FileCheck %s -check-prefix OBJ
-# RUN: ld.lld --eh-frame-hdr -shared -z notext -o %t.so %t.o
+# RUN: ld.lld --eh-frame-hdr -shared -z notext -o %t.so %t.o --no-check-dynamic-relocations
 # RUN: llvm-readobj -r %t.so | FileCheck %s -check-prefix PIC-RELOCS
 
 # Linking this as a PIE executable would also previously crash
 # RUN: llvm-mc -filetype=obj -triple=mips64-unknown-freebsd %S/Inputs/archive2.s -o %t-foo.o
 # -pie needs -z notext because of the R_MIPS_64 relocation
-# RUN: ld.lld --eh-frame-hdr -Bdynamic -pie -z notext -o %t-pie-dynamic.exe %t.o %t-foo.o
+# RUN: ld.lld --eh-frame-hdr -Bdynamic -pie -z notext -o %t-pie-dynamic.exe %t.o %t-foo.o --no-check-dynamic-relocations
 # RUN: llvm-readobj -r %t-pie-dynamic.exe | FileCheck %s -check-prefix PIC-RELOCS
 
 
 # OBJ:       Section ({{.*}}) .rela.text {
 # OBJ-NEXT:    0x0 R_MIPS_GPREL16/R_MIPS_SUB/R_MIPS_HI16 foo 0x0
 # OBJ-NEXT:  }
 # OBJ-NEXT:  Section ({{.*}}) .rela.eh_frame {
 # OBJ-NEXT:    0x1C R_MIPS_64/R_MIPS_NONE/R_MIPS_NONE .text 0x0