This is an archive of the discontinued LLVM Phabricator instance.

Differential D123985

[ELF] Assert on invalid GOT or PLT relocations
ClosedPublic

Authored by smeenai on Apr 18 2022, 11:40 PM.

Details

Reviewers
MaskRay
Commits
rG610a0e8b5368: [ELF] Assert on invalid GOT or PLT relocations
Summary

Because of https://llvm.org/PR50675, we can end up producing a PLT
relocation referencing a symbol that's dropped from the dynamic symbol
table, which in turn causes a crash at runtime. We ran into this again
recently, resulting in crashes for our users. A subsequent diff will fix
that issue, but add an assert to catch it if it happens again.

Diff Detail

Event Timeline

smeenai created this revision.Apr 18 2022, 11:40 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 18 2022, 11:40 PM
Herald added subscribers: StephenFan, arichardson, emaste. · View Herald Transcript
smeenai requested review of this revision.Apr 18 2022, 11:40 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 18 2022, 11:40 PM
Herald added a subscriber: llvm-commits. · View Herald Transcript
Harbormaster completed remote builds in B160177: Diff 423541.Apr 19 2022, 2:15 AM
smeenai updated this revision to Diff 423649.Apr 19 2022, 9:12 AM

Clean up test a bit

Harbormaster completed remote builds in B160255: Diff 423649.Apr 19 2022, 9:35 AM
smeenai updated this revision to Diff 423778.Apr 19 2022, 6:03 PM

Use generic ELF triple in test

Harbormaster completed remote builds in B160352: Diff 423778.Apr 19 2022, 6:15 PM
smeenai mentioned this in D124056: [ELF] Fix wrapping symbols produced during LTO codegen.Apr 19 2022, 6:26 PM
smeenai added a child revision: D124056: [ELF] Fix wrapping symbols produced during LTO codegen.Apr 19 2022, 6:26 PM
MaskRay added a comment.Apr 19 2022, 10:47 PM

I haven't read other patches yet. If the issue only happens due to a bug, I think guarding in #ifndef NDEBUG is probably more appropriate and goes in line with checkDynamicRelocsDefault.

MaskRay added inline comments.Apr 19 2022, 10:49 PM
lld/ELF/SyntheticSections.cpp
1592–1600
if (!needsDynSymIndex())
  return 0;
smeenai added a comment.Apr 19 2022, 10:56 PM
In D123985#3460991, @MaskRay wrote:

I haven't read other patches yet. If the issue only happens due to a bug, I think guarding in #ifndef NDEBUG is probably more appropriate and goes in line with checkDynamicRelocsDefault.

Yup, this is happening due to a bug which is fixed by the next patch.

By guarding with #ifndef NDEBUG, did you mean keeping the current error and guarding it, or just switching to an assert? In either case, should I keep the test in this patch? (The next patch which fixes the bug has to get rid of the test anyway, because I couldn't think of another way to trigger this bug.)

MaskRay added a comment.Apr 19 2022, 11:50 PM
In D123985#3461017, @smeenai wrote:
In D123985#3460991, @MaskRay wrote:

I haven't read other patches yet. If the issue only happens due to a bug, I think guarding in #ifndef NDEBUG is probably more appropriate and goes in line with checkDynamicRelocsDefault.

Yup, this is happening due to a bug which is fixed by the next patch.

By guarding with #ifndef NDEBUG, did you mean keeping the current error and guarding it, or just switching to an assert? In either case, should I keep the test in this patch? (The next patch which fixes the bug has to get rid of the test anyway, because I couldn't think of another way to trigger this bug.)

Switching to an assert looks good:) No needed for test then.

smeenai updated this revision to Diff 423827.Apr 20 2022, 12:08 AM

Switch to assert

smeenai edited the summary of this revision. (Show Details)Apr 20 2022, 12:08 AM
smeenai marked an inline comment as done.
smeenai retitled this revision from [ELF] Error on invalid GOT or PLT relocations to [ELF] Assert on invalid GOT or PLT relocations.Apr 20 2022, 12:12 AM
Harbormaster completed remote builds in B160397: Diff 423827.Apr 20 2022, 12:20 AM
MaskRay accepted this revision.Apr 20 2022, 12:37 AM

LGTM.

This revision is now accepted and ready to land.Apr 20 2022, 12:37 AM
Closed by commit rG610a0e8b5368: [ELF] Assert on invalid GOT or PLT relocations (authored by smeenai). · Explain WhyApr 20 2022, 10:46 AM
This revision was automatically updated to reflect the committed changes.
smeenai added a commit: rG610a0e8b5368: [ELF] Assert on invalid GOT or PLT relocations.
nathanchance added subscribers: nickdesaulniers, nathanchance.Apr 20 2022, 7:17 PM

This assertion triggers when building the Linux kernel for arm64:

$ make -skj"$(nproc)" ARCH=arm64 LLVM=1 mrproper defconfig all
ld.lld: /home/nathan/cbl/src/llvm-project/lld/ELF/SyntheticSections.cpp:1597: uint32_t lld::elf::DynamicReloc::getSymIndex(lld::elf::SymbolTableBaseSection *) const: Assertion `(index != 0 || type != target->gotRel && type != target->pltRel) && "GOT or PLT relocation must refer to symbol in dynamic symbol table"' failed.
ld.lld: /home/nathan/cbl/src/llvm-project/lld/ELF/SyntheticSections.cpp:1597: uint32_t lld::elf::DynamicReloc::getSymIndex(lld::elf::SymbolTableBaseSection *) const: Assertion `(index != 0 || type != target->gotRel && type != target->pltRel) && "GOT or PLT relocation must refer to symbol in dynamic symbol table"' failed.
ld.lld: /home/nathan/cbl/src/llvm-project/lld/ELF/SyntheticSections.cpp:1597: uint32_t lld::elf::DynamicReloc::getSymIndex(lld::elf::SymbolTableBaseSection *) const: Assertion `(index != 0 || type != target->gotRel && type != target->pltRel) && "GOT or PLT relocation must refer to symbol in dynamic symbol table"' failed.
ld.lld: /home/nathan/cbl/src/llvm-project/lld/ELF/SyntheticSections.cpp:1597: uint32_t lld::elf::DynamicReloc::getSymIndex(lld::elf::SymbolTableBaseSection *) const: Assertion `(index != 0 || type != target->gotRel && type != target->pltRel) && "GOT or PLT relocation must refer to symbol in dynamic symbol table"' failed.
ld.lld: /home/nathan/cbl/src/llvm-project/lld/ELF/SyntheticSections.cpp:1597: uint32_t lld::elf::DynamicReloc::getSymIndex(lld::elf::SymbolTableBaseSection *) const: Assertion `(index != 0 || type != target->gotRel && type != target->pltRel) && "GOT or PLT relocation must refer to symbol in dynamic symbol table"' failed.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
ld.lld: /home/nathan/cbl/src/llvm-project/lld/ELF/SyntheticSections.cpp:1597: uint32_t lld::elf::DynamicReloc::getSymIndex(lld::elf::SymbolTableBaseSection *) const: Assertion `(index != 0 || type != target->gotRel && type != target->pltRel) && "GOT or PLT relocation must refer to symbol in dynamic symbol table"' failed.
ld.lld: /home/nathan/cbl/src/llvm-project/lld/ELF/SyntheticSections.cpp:1597: uint32_t lld::elf::DynamicReloc::getSymIndex(lld::elf::SymbolTableBaseSection *) const: Assertion `(index != 0 || type != target->gotRel && type != target->pltRel) && "GOT or PLT relocation must refer to symbol in dynamic symbol table"' failed.
ld.lld: /home/nathan/cbl/src/llvm-project/lld/ELF/SyntheticSections.cpp:1597: uint32_t lld::elf::DynamicReloc::getSymIndex(lld::elf::SymbolTableBaseSection *) const: Assertion `(index != 0 || type != target->gotRel && type != target->pltRel) && "GOT or PLT relocation must refer to symbol in dynamic symbol table"' failed.
scripts/link-vmlinux.sh: line 164: 2580614 Aborted                 (core dumped) ${ld} ${ldflags} -o ${output} ${wl}--whole-archive ${objs} ${wl}--no-whole-archive ${wl}--start-group ${libs} ${wl}--end-group $@ ${ldlibs}
make[1]: *** [Makefile:1158: vmlinux] Error 134

What can I provide to help investigate this further?

smeenai added a comment.Apr 20 2022, 9:13 PM
In D123985#3463657, @nathanchance wrote:

This assertion triggers when building the Linux kernel for arm64:

$ make -skj"$(nproc)" ARCH=arm64 LLVM=1 mrproper defconfig all
ld.lld: /home/nathan/cbl/src/llvm-project/lld/ELF/SyntheticSections.cpp:1597: uint32_t lld::elf::DynamicReloc::getSymIndex(lld::elf::SymbolTableBaseSection *) const: Assertion `(index != 0 || type != target->gotRel && type != target->pltRel) && "GOT or PLT relocation must refer to symbol in dynamic symbol table"' failed.
ld.lld: /home/nathan/cbl/src/llvm-project/lld/ELF/SyntheticSections.cpp:1597: uint32_t lld::elf::DynamicReloc::getSymIndex(lld::elf::SymbolTableBaseSection *) const: Assertion `(index != 0 || type != target->gotRel && type != target->pltRel) && "GOT or PLT relocation must refer to symbol in dynamic symbol table"' failed.
ld.lld: /home/nathan/cbl/src/llvm-project/lld/ELF/SyntheticSections.cpp:1597: uint32_t lld::elf::DynamicReloc::getSymIndex(lld::elf::SymbolTableBaseSection *) const: Assertion `(index != 0 || type != target->gotRel && type != target->pltRel) && "GOT or PLT relocation must refer to symbol in dynamic symbol table"' failed.
ld.lld: /home/nathan/cbl/src/llvm-project/lld/ELF/SyntheticSections.cpp:1597: uint32_t lld::elf::DynamicReloc::getSymIndex(lld::elf::SymbolTableBaseSection *) const: Assertion `(index != 0 || type != target->gotRel && type != target->pltRel) && "GOT or PLT relocation must refer to symbol in dynamic symbol table"' failed.
ld.lld: /home/nathan/cbl/src/llvm-project/lld/ELF/SyntheticSections.cpp:1597: uint32_t lld::elf::DynamicReloc::getSymIndex(lld::elf::SymbolTableBaseSection *) const: Assertion `(index != 0 || type != target->gotRel && type != target->pltRel) && "GOT or PLT relocation must refer to symbol in dynamic symbol table"' failed.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
ld.lld: /home/nathan/cbl/src/llvm-project/lld/ELF/SyntheticSections.cpp:1597: uint32_t lld::elf::DynamicReloc::getSymIndex(lld::elf::SymbolTableBaseSection *) const: Assertion `(index != 0 || type != target->gotRel && type != target->pltRel) && "GOT or PLT relocation must refer to symbol in dynamic symbol table"' failed.
ld.lld: /home/nathan/cbl/src/llvm-project/lld/ELF/SyntheticSections.cpp:1597: uint32_t lld::elf::DynamicReloc::getSymIndex(lld::elf::SymbolTableBaseSection *) const: Assertion `(index != 0 || type != target->gotRel && type != target->pltRel) && "GOT or PLT relocation must refer to symbol in dynamic symbol table"' failed.
ld.lld: /home/nathan/cbl/src/llvm-project/lld/ELF/SyntheticSections.cpp:1597: uint32_t lld::elf::DynamicReloc::getSymIndex(lld::elf::SymbolTableBaseSection *) const: Assertion `(index != 0 || type != target->gotRel && type != target->pltRel) && "GOT or PLT relocation must refer to symbol in dynamic symbol table"' failed.
scripts/link-vmlinux.sh: line 164: 2580614 Aborted                 (core dumped) ${ld} ${ldflags} -o ${output} ${wl}--whole-archive ${objs} ${wl}--no-whole-archive ${wl}--start-group ${libs} ${wl}--end-group $@ ${ldlibs}
make[1]: *** [Makefile:1158: vmlinux] Error 134

What can I provide to help investigate this further?

Hmm, this is interesting. I think the assertion should only fire when the linker would have otherwise produced an invalid binary, but maybe there's something in the Linux kernel which would make this work.

Could you create a tarball with --reproduce and upload that somewhere? Feel free to revert this in the meantime if it's blocking you.

MaskRay mentioned this in rGf4a3569d0ad6: [ELF] Fix spurious GOT/PLT assertion failure when .dynsym is discarded.Apr 20 2022, 10:49 PM
nathanchance added a comment.Apr 21 2022, 9:04 AM

It looks like Fangrui already fixed this with f4a3569d0ad61907692fe10b1ffe1fa7763e9c26, thanks a lot! I can confirm that ARCH=arm64 defconfig and allmodconfig build fine now. I will test other architectures tonight and report back if there are any other issues I uncover.

smeenai added a comment.Apr 21 2022, 9:09 AM
In D123985#3464940, @nathanchance wrote:

It looks like Fangrui already fixed this with f4a3569d0ad61907692fe10b1ffe1fa7763e9c26, thanks a lot! I can confirm that ARCH=arm64 defconfig and allmodconfig build fine now. I will test other architectures tonight and report back if there are any other issues I uncover.

Great, thanks @MaskRay!

smeenai mentioned this in rG1af25a986069: [ELF] Fix wrapping symbols produced during LTO codegen.Apr 22 2022, 4:46 PM

Revision Contents

PathSize
lld/
ELF/
11 lines
test/
ELF/
32 lines

Diff 423649

lld/ELF/SyntheticSections.cpp

Show First 20 Lines • Show All 1,583 Lines • ▼ Show 20 Linesint64_t DynamicReloc::computeAddend() const {
case MipsMultiGotPage: case MipsMultiGotPage:
assert(sym == nullptr); assert(sym == nullptr);
return getMipsPageAddr(outputSec->addr) + addend; return getMipsPageAddr(outputSec->addr) + addend;
} }
llvm_unreachable("Unknown DynamicReloc::Kind enum"); llvm_unreachable("Unknown DynamicReloc::Kind enum");
} }
uint32_t DynamicReloc::getSymIndex(SymbolTableBaseSection *symTab) const { uint32_t DynamicReloc::getSymIndex(SymbolTableBaseSection *symTab) const {
if (needsDynSymIndex()) if (needsDynSymIndex()) {
return symTab->getSymbolIndex(sym); size_t index = symTab->getSymbolIndex(sym);
if (index == 0 && (type == target->gotRel || type == target->pltRel))
error(Twine(type == target->gotRel ? "GOT" : "PLT") +
" relocation references symbol not present in dynamic "
"symbol table: " +
toString(*sym));
return index;
}
MaskRayUnsubmitted
Done
ReplyInline Actions
if (!needsDynSymIndex())
  return 0;
MaskRay: ``` if (!needsDynSymIndex()) return 0; ```
return 0; return 0;
} }
RelocationBaseSection::RelocationBaseSection(StringRef name, uint32_t type, RelocationBaseSection::RelocationBaseSection(StringRef name, uint32_t type,
int32_t dynamicTag, int32_t dynamicTag,
int32_t sizeDynamicTag, int32_t sizeDynamicTag,
bool combreloc) bool combreloc)
: SyntheticSection(SHF_ALLOC, type, config->wordsize, name), : SyntheticSection(SHF_ALLOC, type, config->wordsize, name),
▲ Show 20 Lines • Show All 2,366 Lines • Show Last 20 Lines

lld/test/ELF/invalid-plt-relocation.test

  • This file was added.
# REQUIRES: x86
# RUN: rm -rf %t && split-file %s %t
# RUN: llvm-mc -triple x86_64-linux-gnu --filetype=obj -o %t/unwind.o %t/unwind.s
# RUN: ld.lld -shared -o %t/libunwind.so -soname libunwind.so %t/unwind.o
# RUN: llvm-as -o %t/resume.bc %t/resume.ll
# RUN: not ld.lld -shared -o %t/libresume.so %t/resume.bc %t/libunwind.so \
# RUN: --wrap _Unwind_Resume 2>&1 | FileCheck %s
# CHECK: error: PLT relocation references symbol not present in dynamic symbol table: __wrap__Unwind_Resume
#--- unwind.s
.globl _Unwind_Resume
.globl __wrap__Unwind_Resume
_Unwind_Resume:
__wrap__Unwind_Resume:
retq
#--- resume.ll
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
define dso_local void @_Z1fv() optnone noinline personality i8* bitcast (void ()* @throw to i8*) {
invoke void @throw()
to label %unreachable unwind label %lpad
lpad:
%1 = landingpad { i8*, i32 }
cleanup
resume { i8*, i32 } %1
unreachable:
unreachable
}
declare void @throw()