[lld] r192771 - Propagate deadStripOptimize()'s failure to the caller.

[Rui Ueyama]

That is a good point. Looks like I forgot to
call addInitialUndefinedSymbol() in the link.exe-compatible driver. Only
addDeadStrip() is called on an entry point symbol, but
addInitialUndefinedSymbol() was never called on the same symbol.

This reminds me of the discussion that addInitialUndefinedSymbol() should
add a given symbol to the dead strip root. I remember I claimed it should
do but it was pushed back. It seems that not doing that does not make much
sense, and proven to be error prone :/, so we should do that now.


On Wed, Oct 16, 2013 at 10:26 AM, Nick Kledzik <kledzik at apple.com> wrote:

> The entry point should also be added to the list of initial undefines.  If
> it turns out to be never defined, then that undefined symbol will cause the
> link to error out.  Adding to the undefined list is also necessary if the
> entry is to be pulled out of an archive.
>
> Give that, is this change necessary?
>
> -Nick
>
> On Oct 15, 2013, at 10:03 PM, Rui Ueyama wrote:
> > Author: ruiu
> > Date: Wed Oct 16 00:03:39 2013
> > New Revision: 192771
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=192771&view=rev
> > Log:
> > Propagate deadStripOptimize()'s failure to the caller.
> >
> > We want to make the program to exit with non-zero exit code if there's
> an error
> > during dead stripping.
> >
> > Modified:
> >    lld/trunk/include/lld/Core/Resolver.h
> >    lld/trunk/lib/Core/Resolver.cpp
> >    lld/trunk/test/pecoff/entry.test
> >
> > Modified: lld/trunk/include/lld/Core/Resolver.h
> > URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/include/lld/Core/Resolver.h?rev=192771&r1=192770&r2=192771&view=diff
> >
> ==============================================================================
> > --- lld/trunk/include/lld/Core/Resolver.h (original)
> > +++ lld/trunk/include/lld/Core/Resolver.h Wed Oct 16 00:03:39 2013
> > @@ -71,7 +71,7 @@ private:
> >   /// \brief The main function that iterates over the files to resolve
> >   bool resolveUndefines();
> >   void updateReferences();
> > -  void deadStripOptimize();
> > +  bool deadStripOptimize();
> >   bool checkUndefines(bool final);
> >   void removeCoalescedAwayAtoms();
> >   void checkDylibSymbolCollisions();
> >
> > Modified: lld/trunk/lib/Core/Resolver.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/Core/Resolver.cpp?rev=192771&r1=192770&r2=192771&view=diff
> >
> ==============================================================================
> > --- lld/trunk/lib/Core/Resolver.cpp (original)
> > +++ lld/trunk/lib/Core/Resolver.cpp Wed Oct 16 00:03:39 2013
> > @@ -363,11 +363,11 @@ void Resolver::markLive(const Atom &atom
> >
> >
> > // remove all atoms not actually used
> > -void Resolver::deadStripOptimize() {
> > +bool Resolver::deadStripOptimize() {
> >   ScopedTask task(getDefaultDomain(), "deadStripOptimize");
> >   // only do this optimization with -dead_strip
> >   if (!_context.deadStrip())
> > -    return;
> > +    return true;
> >
> >   // clear liveness on all atoms
> >   _liveAtoms.clear();
> > @@ -386,10 +386,10 @@ void Resolver::deadStripOptimize() {
> >   // Or, use list of names that are dead stip roots.
> >   for (const StringRef &name : _context.deadStripRoots()) {
> >     const Atom *symAtom = _symbolTable.findByName(name);
> > -    if (symAtom->definition() == Atom::definitionUndefined) {
> > +    if (!symAtom || symAtom->definition() == Atom::definitionUndefined)
> {
> >       llvm::errs() << "Dead strip root '" << symAtom->name()
> >                    << "' is not defined\n";
> > -      return;
> > +      return false;
> >     }
> >     _deadStripRoots.insert(symAtom);
> >   }
> > @@ -402,6 +402,7 @@ void Resolver::deadStripOptimize() {
> >   // now remove all non-live atoms from _atoms
> >   _atoms.erase(std::remove_if(_atoms.begin(), _atoms.end(),
> >                               NotLive(_liveAtoms)), _atoms.end());
> > +  return true;
> > }
> >
> >
> > @@ -473,7 +474,8 @@ bool Resolver::resolve() {
> >   if (!this->resolveUndefines())
> >     return false;
> >   this->updateReferences();
> > -  this->deadStripOptimize();
> > +  if (!this->deadStripOptimize())
> > +    return false;
> >   if (this->checkUndefines(false)) {
> >     if (!_context.allowRemainingUndefines())
> >       return false;
> >
> > Modified: lld/trunk/test/pecoff/entry.test
> > URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/test/pecoff/entry.test?rev=192771&r1=192770&r2=192771&view=diff
> >
> ==============================================================================
> > --- lld/trunk/test/pecoff/entry.test (original)
> > +++ lld/trunk/test/pecoff/entry.test Wed Oct 16 00:03:39 2013
> > @@ -2,8 +2,15 @@
> > # Verify that entry atom will not be dead-stripped.
> >
> > # RUN: yaml2obj %p/Inputs/main.obj.yaml > %t.obj
> > -# RUN: lld -flavor link /mllvm:-debug-only=WriterPECOFF /out:%t.exe \
> > -# RUN:   /subsystem:console /entry:_main /force -- %t.obj >& %t.log
> > -# RUN: FileCheck %s < %t.log
> > +# RUN: lld -flavor link /mllvm:-debug-only=WriterPECOFF /out:%t1.exe \
> > +# RUN:   /subsystem:console /entry:main /force -- %t.obj >& %t1.log
> > +# RUN: FileCheck -check-prefix=CHECK %s < %t1.log
> >
> > CHECK: : _main
> > +
> > +
> > +# RUN: not lld -flavor link /out:%t2.exe /subsystem:console \
> > +# RUN:    /entry:no_such_symbol /force -- %t.obj >& %t2.log
> > +# RUN: FileCheck -check-prefix=FAIL %s < %t2.log
> > +
> > +FAIL: Dead strip root '_no_such_symbol' is not defined
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131016/5de73099/attachment.html>