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

[Peter Collingbourne]

On Mon, Jun 08, 2015 at 07:57:59PM -0700, Reid Kleckner wrote:
> My understanding is that the gold plugin already solves this problem by
> maintaining a list of such symbols (memcpy, memset, etc).

I wasn't aware of anything like that. A quick grep didn't reveal anything
either. My understanding was that the existing linkers use a mechanism
like the one I implemented, but I could be wrong.

> _fltused is interesting, since we may end up linking the FP library when we
> otherwise wouldn't. We might be able to win it back with dead code
> stripping.
> 
> On Mon, Jun 8, 2015 at 7:25 PM, Peter Collingbourne <peter at pcc.me.uk> wrote:
> 
> > 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
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >

-- 
Peter