COFF: Handle references from LTO object to lazy symbols correctly.

[pcc]

Summary

The code generator may create references to runtime library symbols such as
__chkstk which were not visible via LTOModule. Handle these cases by loading
the object file from the library, but abort if we end up having loaded any
bitcode objects.

Because loading the object file may have introduced new undefined references,
call reportRemainingUndefines again to detect and report them.

[ruiu]

__chkstk is a special symbol. Does the code generator introduce arbitrary
symbols? If not (if it could introduce only a limited set of symbols), we
may want to resolve them unconditionally as if they were specified with
/include.
2015/06/08 午後6:00 "Peter Collingbourne" <[e-mail]>:

Hi ruiu,

The code generator may create references to runtime library symbols such as
__chkstk which were not visible via LTOModule. Handle these cases by
loading
the object file from the library, but abort if we end up having loaded any
bitcode objects.

Because loading the object file may have introduced new undefined
references,
call reportRemainingUndefines again to detect and report them.

http://reviews.llvm.org/D10332

Files:

COFF/SymbolTable.cpp
test/COFF/Inputs/lto-chkstk-chkstk.s
test/COFF/Inputs/lto-chkstk-foo.s
test/COFF/lto-chkstk.ll

Index: COFF/SymbolTable.cpp

• COFF/SymbolTable.cpp

+++ COFF/SymbolTable.cpp
@@ -307,8 +307,27 @@

    return make_error_code(LLDError::BrokenFile);
  }
}

+
+ We may see new references to runtime library symbols such as
__chkstk
+ here. These symbols must be wholly defined in non-bitcode files.
+ if (auto *B = dyn_cast<Lazy>(Sym->Body)) {
+ unsigned NumBitcodeFiles = BitcodeFiles.size();
+ auto EC = addMemberFile(B);
+ if (EC)
+ return EC;
+ if (BitcodeFiles.size() != NumBitcodeFiles) {
+ llvm::errs()
+ << "LTO: late loaded symbol created new bitcode reference: "
<< Name
+ << "\n";
+ return make_error_code(LLDError::BrokenFile);
+ }
+ }

}

+ // New runtime library symbol references may have created undefined
references.
+ if (reportRemainingUndefines())
+ return make_error_code(LLDError::BrokenFile);
+

return std::error_code();

}

Index: test/COFF/Inputs/lto-chkstk-chkstk.s

• /dev/null

+++ test/COFF/Inputs/lto-chkstk-chkstk.s
@@ -0,0 +1,3 @@
+.globl chkstk
+chkstk:
+ret

Index: test/COFF/Inputs/lto-chkstk-foo.s

• /dev/null

+++ test/COFF/Inputs/lto-chkstk-foo.s
@@ -0,0 +1,3 @@
+.globl foo
+foo:
+ret

Index: test/COFF/lto-chkstk.ll

• /dev/null

+++ test/COFF/lto-chkstk.ll
@@ -0,0 +1,17 @@
+; RUN: llvm-as -o %t.obj %s
+; RUN: llvm-mc -triple=x86_64-pc-windows-msvc -filetype=obj -o
%T/lto-chkstk-foo.obj %S/Inputs/lto-chkstk-foo.s
+; RUN: llvm-mc -triple=x86_64-pc-windows-msvc -filetype=obj -o
%T/lto-chkstk-chkstk.obj %S/Inputs/lto-chkstk-chkstk.s
+; RUN: llvm-ar cru %t.lib %T/lto-chkstk-chkstk.obj
+; RUN: lld -flavor link2 /out:%t.exe /entry:main /subsystem:console
%t.obj %T/lto-chkstk-foo.obj %t.lib
+
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc"
+
+define void @main() {
+entry:
+ %array4096 = alloca [4096 x i8]
+ call void @foo([4096 x i8]* %array4096)
+ ret void
+}
+
+declare void @foo([4096 x i8]*)

EMAIL PREFERENCES

http://reviews.llvm.org/settings/panel/emailpreferences/

[ruiu]

LGTM

Thank you for the explanation. My concern was that before this patch, adding a combined compiled bitcode file is really a final operation in the linker, but after this patch, a new call of addMember() can read arbitrary number of new files. That would increase complexity. That being said, this change seems reasonable. It's probably better than maintaining a list of symbols to be linked unconditionally for LTO.

COFF/SymbolTable.cpp

314 ↗

(On Diff #27354)

size_t

317 ↗

(On Diff #27354)

if (auto EC = ...)
  return EC

329 ↗

(On Diff #27354)

Can we call reportRemainingUndefines after Symtab.addCombinedLTOObject in the Driver to eliminate this call from here?

[pcc]

COFF/SymbolTable.cpp

329 ↗

(On Diff #27354)

My reasoning was that because LTO is generally an expensive operation, we'll want to report undefined symbols as soon as possible, rather than wasting time building the LTO object file.

lld/trunk/COFF/SymbolTable.cpp

for (SymbolBody *Body : Obj->getSymbols()) {
     if (isa<DefinedBitcode>(Sym->Body)) {
       // The symbol should now be defined.
       if (!isa<Defined>(Body)) {
         llvm::errs() << "LTO: undefined symbol: " << Name << '\n';
         return make_error_code(LLDError::BrokenFile);
       }
       Sym->Body = Body;
     }
+
+    // We may see new references to runtime library symbols such as __chkstk
+    // here. These symbols must be wholly defined in non-bitcode files.
+    if (auto *B = dyn_cast<Lazy>(Sym->Body)) {
+      size_t NumBitcodeFiles = BitcodeFiles.size();
+      if (auto EC = addMemberFile(B))
+        return EC;
+      if (BitcodeFiles.size() != NumBitcodeFiles) {
+        llvm::errs()
+            << "LTO: late loaded symbol created new bitcode reference: " << Name
+            << "\n";
+        return make_error_code(LLDError::BrokenFile);
+      }
     }
+  }
+
+  // New runtime library symbol references may have created undefined references.
+  if (reportRemainingUndefines())
+    return make_error_code(LLDError::BrokenFile);
 
   return std::error_code();
 }
 
 } // namespace coff
 } // namespace lld

lld/trunk/test/COFF/Inputs/lto-chkstk-chkstk.s

+.globl __chkstk
+__chkstk:
+ret

lld/trunk/test/COFF/Inputs/lto-chkstk-foo.s

+.globl foo
+foo:
+ret

lld/trunk/test/COFF/lto-chkstk.ll

+; RUN: llvm-as -o %t.obj %s
+; RUN: llvm-mc -triple=x86_64-pc-windows-msvc -filetype=obj -o %T/lto-chkstk-foo.obj %S/Inputs/lto-chkstk-foo.s
+; RUN: llvm-mc -triple=x86_64-pc-windows-msvc -filetype=obj -o %T/lto-chkstk-chkstk.obj %S/Inputs/lto-chkstk-chkstk.s
+; RUN: llvm-ar cru %t.lib %T/lto-chkstk-chkstk.obj
+; RUN: lld -flavor link2 /out:%t.exe /entry:main /subsystem:console %t.obj %T/lto-chkstk-foo.obj %t.lib
+
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc"
+
+define void @main() {
+entry:
+  %array4096 = alloca [4096 x i8]
+  call void @foo([4096 x i8]* %array4096)
+  ret void
+}
+
+declare void @foo([4096 x i8]*)