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

[Peter Collingbourne]

In general, the optimizer or code generator could create a reference to any
C runtime library symbol, but the set of symbols is limited to those that it
knows about. I don't believe we maintain a list of such symbols anywhere,
and I'm a little concerned about the maintenance burden of such a list,
as it could easily get out of sync with what LLVM uses.

It could in principle also contain references to sanitizer runtime functions,
but I believe that no existing sanitizer will create symbol references at
LTO time.

That said, if I revert this patch, I find that I need to create artificial
references to __chkstk, _fltused, memcpy, memset and memmove in order
to successfully link llvm-tblgen.exe. These symbols mostly seem fine, but
_fltused is probably the most concerning, as I gather that it is part of an
mechanism that causes the C runtime's floating point support to be optionally
linked in if the program uses floating point arithmetic.

With all that in mind, I'd slightly prefer to follow the approach given in
my patch.

Peter

On Mon, Jun 08, 2015 at 06:27:46PM -0700, Rui Ueyama wrote:
> __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" <peter at pcc.me.uk>:
> 
> > 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/
> >

-- 
Peter