From 70f11d7589738186588427ec8134e411ac43bad4 Mon Sep 17 00:00:00 2001 From: Rui Ueyama Date: Wed, 20 Nov 2013 20:51:55 +0000 Subject: [PATCH] Fix Weak External symbol handling. The fallback atom was used only when it's searching for a symbol in a library; if an undefined symbol was not found in a library, the LLD looked for its fallback symbol in the library. Although it worked in most cases, because symbols with fallbacks usually occur only in OLDNAMES.LIB (a standard library), that behavior was incompatible with link.exe. This patch fixes the issue so that the semantics is the same as MSVC's link.exe The new (and correct, I believe) behavior is this: - If there's no definition for an undefined atom, replace the undefined atom with its fallback and then proceed (e.g. look in the next file or stop linking as usual.) Weak External symbols are underspecified in the Microsoft PE/COFF spec. However, as long as I observed the behavior of link.exe, this seems to be what we want for compatibility. Differential Revision: http://llvm-reviews.chandlerc.com/D2162 llvm-svn: 195269 --- lld/lib/Core/Resolver.cpp | 19 ++++++++-------- lld/test/core/undef-fallback.objtxt | 29 ++++++++++++++++++++---- lld/test/core/undef-weak-coalesce.objtxt | 13 +---------- lld/test/pecoff/weak-external.test | 2 +- 4 files changed, 36 insertions(+), 27 deletions(-) diff --git a/lld/lib/Core/Resolver.cpp b/lld/lib/Core/Resolver.cpp index 5d9db38bec5f..cdacc64a7ebf 100644 --- a/lld/lib/Core/Resolver.cpp +++ b/lld/lib/Core/Resolver.cpp @@ -83,6 +83,16 @@ void Resolver::handleFile(const File &file) { for (const UndefinedAtom *undefAtom : file.undefined()) { doUndefinedAtom(*undefAtom); resolverState |= StateNewUndefinedAtoms; + + // If the undefined symbol has an alternative name, try to resolve the + // symbol with the name to give it a second chance. This feature is used for + // COFF "weak external" symbol. + if (!_symbolTable.isDefined(undefAtom->name())) { + if (const UndefinedAtom *fallbackAtom = undefAtom->fallback()) { + doUndefinedAtom(*fallbackAtom); + _symbolTable.addReplacement(undefAtom, fallbackAtom); + } + } } for (const SharedLibraryAtom *shlibAtom : file.sharedLibrary()) { doSharedLibraryAtom(*shlibAtom); @@ -108,15 +118,6 @@ Resolver::forEachUndefines(UndefCallback callback, bool searchForOverrides) { // load for previous undefine may also have loaded this undefine if (!_symbolTable.isDefined(undefName)) callback(undefName, false); - // If the undefined symbol has an alternative name, try to resolve the - // symbol with the name to give it a second chance. This feature is used - // for COFF "weak external" symbol. - if (!_symbolTable.isDefined(undefName)) { - if (const UndefinedAtom *fallbackUndefAtom = undefAtom->fallback()) { - _symbolTable.addReplacement(undefAtom, fallbackUndefAtom); - _symbolTable.add(*fallbackUndefAtom); - } - } } // search libraries for overrides of common symbols if (searchForOverrides) { diff --git a/lld/test/core/undef-fallback.objtxt b/lld/test/core/undef-fallback.objtxt index 8be4feef020b..8abaa93c8e8d 100644 --- a/lld/test/core/undef-fallback.objtxt +++ b/lld/test/core/undef-fallback.objtxt @@ -5,14 +5,33 @@ --- defined-atoms: - - name: bar - type: code + - name: def1 + scope: global undefined-atoms: - - name: foo + - name: undef1 fallback: - name: bar + name: fallback1 + - name: undef2 + fallback: + name: fallback2 +--- +defined-atoms: + - name: fallback1 + +undefined-atoms: + - name: def1 + fallback: + name: fallback3 ... # CHECK: defined-atoms: -# CHECK-NEXT: - name: bar +# CHECK-NEXT: - name: def1 +# CHECK-NEXT: scope: global +# CHECK-NEXT: - name: fallback1 +# CHECK-NEXT: ref-name: fallback1 +# CHECK-NEXT: undefined-atoms: +# CHECK-NEXT: - name: fallback1 +# CHECK-NEXT: - name: fallback2 + +# CHECK-NOT: - name: fallback3 diff --git a/lld/test/core/undef-weak-coalesce.objtxt b/lld/test/core/undef-weak-coalesce.objtxt index 45814535060a..97e0fd28500d 100644 --- a/lld/test/core/undef-weak-coalesce.objtxt +++ b/lld/test/core/undef-weak-coalesce.objtxt @@ -1,5 +1,4 @@ -# RUN: lld -core %s 2> %t.err | FileCheck %s -# RUN: FileCheck -check-prefix=ERROR %s < %t.err +# RUN: lld -core %s | FileCheck %s # # Test that undefined symbols preserve their attributes and merge properly @@ -29,12 +28,8 @@ undefined-atoms: can-be-null: never - name: bar8 can-be-null: at-runtime - fallback: - name: baz1 - name: bar9 can-be-null: at-buildtime - fallback: - name: baz2 --- undefined-atoms: - name: bar1 @@ -55,8 +50,6 @@ undefined-atoms: can-be-null: never - name: bar9 can-be-null: at-runtime - fallback: - name: baz3 ... # CHECK: - name: regular_func @@ -77,7 +70,3 @@ undefined-atoms: # CHECK-NEXT: - name: bar8 # CHECK-NEXT: - name: bar9 # CHECK-NEXT: can-be-null: at-runtime -# CHECK-NEXT: fallback: -# CHECK-NEXT: name: baz3 - -# ERROR: undefined symbol bar9 has different fallback: baz2 in and baz3 in diff --git a/lld/test/pecoff/weak-external.test b/lld/test/pecoff/weak-external.test index 2fb0c029084e..af2af9628aac 100644 --- a/lld/test/pecoff/weak-external.test +++ b/lld/test/pecoff/weak-external.test @@ -4,6 +4,6 @@ # RUN: /entry:fn -- %t.obj %p/Inputs/static.lib 2> %t2.out # RUN: FileCheck %s < %t2.out -CHECK-NOT: _no_such_symbol1 +CHECK: _no_such_symbol1 CHECK-NOT: _no_such_symbol2 CHECK: _no_such_symbol3