From 70c23e232e50b190c9e0d8e4b5b6a8ddfc19b80c Mon Sep 17 00:00:00 2001 From: Igor Kudrin Date: Thu, 6 May 2021 20:45:29 +0700 Subject: [PATCH] [LLD] Improve reporting unresolved symbols in shared libraries Currently, when reporting unresolved symbols in shared libraries, if an undefined symbol is firstly seen in a regular object file that shadows the reference for the same symbol in a shared object. As a result, the error for the unresolved symbol in the shared library is not reported. If referencing sections in regular object files are discarded because of '--gc-sections', no reports about such symbols are generated, and the linker finishes successfully, generating an output image that fails on the run. The patch fixes the issue by keeping symbols, which should be checked, for each shared library separately. Differential Revision: https://reviews.llvm.org/D101996 --- lld/ELF/InputFiles.cpp | 3 +++ lld/ELF/InputFiles.h | 7 ++++--- lld/ELF/Writer.cpp | 27 +++++++++++++-------------- lld/test/ELF/allow-shlib-undefined.s | 13 +++++++++++++ 4 files changed, 33 insertions(+), 17 deletions(-) diff --git a/lld/ELF/InputFiles.cpp b/lld/ELF/InputFiles.cpp index 268afae117cd..3e0c1d77823f 100644 --- a/lld/ELF/InputFiles.cpp +++ b/lld/ELF/InputFiles.cpp @@ -1567,6 +1567,9 @@ template void SharedFile::parse() { Symbol *s = symtab->addSymbol( Undefined{this, name, sym.getBinding(), sym.st_other, sym.getType()}); s->exportDynamic = true; + if (s->isUndefined() && !s->isWeak() && + config->unresolvedSymbolsInShlib != UnresolvedPolicy::Ignore) + requiredSymbols.push_back(s); continue; } diff --git a/lld/ELF/InputFiles.h b/lld/ELF/InputFiles.h index cdd6b5c2ce99..f66d7a25d954 100644 --- a/lld/ELF/InputFiles.h +++ b/lld/ELF/InputFiles.h @@ -381,12 +381,13 @@ public: template void parse(); - // Used for --no-allow-shlib-undefined. - bool allNeededIsKnown; - // Used for --as-needed bool isNeeded; + // Non-weak undefined symbols which are not yet resolved when the SO is + // parsed. Only filled for `--no-allow-shlib-undefined`. + std::vector requiredSymbols; + private: template std::vector parseVerneed(const llvm::object::ELFFile &obj, diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp index 2301a064e328..75ddd2dce1ea 100644 --- a/lld/ELF/Writer.cpp +++ b/lld/ELF/Writer.cpp @@ -2006,6 +2006,10 @@ template void Writer::finalizeSections() { in.iplt->addSymbols(); if (config->unresolvedSymbolsInShlib != UnresolvedPolicy::Ignore) { + auto diagnose = + config->unresolvedSymbolsInShlib == UnresolvedPolicy::ReportError + ? errorOrWarn + : warn; // Error on undefined symbols in a shared object, if all of its DT_NEEDED // entries are seen. These cases would otherwise lead to runtime errors // reported by the dynamic linker. @@ -2013,23 +2017,18 @@ template void Writer::finalizeSections() { // ld.bfd traces all DT_NEEDED to emulate the logic of the dynamic linker to // catch more cases. That is too much for us. Our approach resembles the one // used in ld.gold, achieves a good balance to be useful but not too smart. - for (SharedFile *file : sharedFiles) - file->allNeededIsKnown = + for (SharedFile *file : sharedFiles) { + bool allNeededIsKnown = llvm::all_of(file->dtNeeded, [&](StringRef needed) { return symtab->soNames.count(needed); }); - - for (Symbol *sym : symtab->symbols()) - if (sym->isUndefined() && !sym->isWeak()) - if (auto *f = dyn_cast_or_null(sym->file)) - if (f->allNeededIsKnown) { - auto diagnose = config->unresolvedSymbolsInShlib == - UnresolvedPolicy::ReportError - ? errorOrWarn - : warn; - diagnose(toString(f) + ": undefined reference to " + - toString(*sym) + " [--no-allow-shlib-undefined]"); - } + if (!allNeededIsKnown) + continue; + for (Symbol *sym : file->requiredSymbols) + if (sym->isUndefined() && !sym->isWeak()) + diagnose(toString(file) + ": undefined reference to " + + toString(*sym) + " [--no-allow-shlib-undefined]"); + } } { diff --git a/lld/test/ELF/allow-shlib-undefined.s b/lld/test/ELF/allow-shlib-undefined.s index 4994466ec5dc..4c0e06e5d009 100644 --- a/lld/test/ELF/allow-shlib-undefined.s +++ b/lld/test/ELF/allow-shlib-undefined.s @@ -25,9 +25,22 @@ # RUN: ld.lld -shared --allow-shlib-undefined %t1.o -o /dev/null # RUN: ld.lld -shared --no-allow-shlib-undefined %t1.o -o /dev/null +## Check that the error is reported if an unresolved symbol is first seen in a +## regular object file. +# RUN: echo 'callq _unresolved@PLT' | \ +# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux - -o %tref.o +# RUN: not ld.lld --gc-sections %t.o %tref.o %t.so -o /dev/null 2>&1 | FileCheck %s + +## Check that the error is reported for each shared library where the symbol +## is referenced. +# RUN: cp %t.so %t2.so +# RUN: not ld.lld %t.o %t.so %t2.so -o /dev/null 2>&1 | \ +# RUN: FileCheck %s --check-prefixes=CHECK,CHECK2 + .globl _start _start: callq _shared@PLT # CHECK: error: {{.*}}.so: undefined reference to _unresolved [--no-allow-shlib-undefined] +# CHECK2: error: {{.*}}2.so: undefined reference to _unresolved [--no-allow-shlib-undefined] # WARN: warning: {{.*}}.so: undefined reference to _unresolved [--no-allow-shlib-undefined]