[ELF2] - add support of --noinhibit-exec flag

[grimar]

Summary

Support of --noinhibit-exec flag implemented.

[grimar]

Requires the
void warning(const Twine &Msg)
method from dependency.

[ruiu]

ELF/Writer.cpp

4–6

"A Twine is not intended for use directly and should not be stored, its implementation relies on the ability to store pointers to temporary stack objects which may be deallocated at the end of a statement" (http://llvm.org/docs/doxygen/html/classllvm_1_1Twine.html#details)

So make this an std::string rather than a Tiwne.

[grimar]

Review comments addressed.

ELF/Writer.cpp

288–290

Fixed. Sorry for that.

[ruiu]

BTW, is this guaranteed to be safe? I think we currently do not expect any undefined symbols to appear in Writer because we abort early if such symbol exists. This patch changes that assumption.

In COFF, we overwrite undefined symbols with absolute symbols with value 0 if /force (Windows option for -no-inhibit-exec). Maybe we should do that?

Rafael?

[• rafael]

Just a few fixes.

This should be fine from the writer perspective since we are already handling weak undefined symbols.

ELF/Writer.cpp

288

I think you can write this as

std::string Error = "undefined symbol: " + Sym.getName();
if (SymFile)

Error += " in " ...

test/elf2/no-inhibit-exec.s

3

Also run llvm-objdump -d to check what we produce.

12

You don't need this part

[grimar]

Review comments addressed

[grimar]

test/elf2/no-inhibit-exec.s

3

Done !

12

Agree. Fixed.

[denis-protivensky]

test/elf2/no-inhibit-exec.s

2

Maybe check that both runs issue the same message?

[grimar]

test/elf2/no-inhibit-exec.s

2

That might be useful. Can update the patch with that later.

[• rafael]

LGTM

[• rafael]

I tried to apply it but failed:

git apply -p0 ~/Downloads/D13138.diff.txt
/Users/espindola/Downloads/D13138.diff.txt:9: trailing whitespace.

bool NoInhibitExec = false;

/Users/espindola/Downloads/D13138.diff.txt:21: trailing whitespace.

if (Args.hasArg(OPT_noinhibit_exec))

/Users/espindola/Downloads/D13138.diff.txt:22: trailing whitespace.

Config->NoInhibitExec = true;

/Users/espindola/Downloads/D13138.diff.txt:23: trailing whitespace.

/Users/espindola/Downloads/D13138.diff.txt:35: trailing whitespace.
#include "Config.h"
error: patch failed: ELF/Config.h:24
error: ELF/Config.h: patch does not apply

....

Can you email me a git generated patch directly?

[grimar]

Can you email me a git generated patch directly?

Just sent it.

[grimar]

Commited as r248605. I will improve the test separately.

ELF/Config.h

struct Configuration {
   llvm::StringRef OutputFile;
   llvm::StringRef DynamicLinker;
   std::string RPath;
   bool Shared = false;
   bool DiscardAll = false;
   bool DiscardLocals = false;
   bool DiscardNone = false;
   bool ExportDynamic = false;
+  bool NoInhibitExec = false;
 };
 
 extern Configuration *Config;
 
 } // namespace elf2
 } // namespace lld
 
 #endif

ELF/Driver.cpp

if (Args.hasArg(OPT_discard_locals))
     Config->DiscardLocals = true;
 
   if (Args.hasArg(OPT_discard_none))
     Config->DiscardNone = true;
 
   if (Args.hasArg(OPT_export_dynamic))
     Config->ExportDynamic = true;
 
+  if (Args.hasArg(OPT_noinhibit_exec))
+    Config->NoInhibitExec = true;
+
   // Create a list of input files.
   std::vector<MemoryBufferRef> Inputs;
 
   for (auto *Arg : Args.filtered(OPT_INPUT)) {
     StringRef Path = Arg->getValue();
     Inputs.push_back(openFile(Path));
   }
 

ELF/InputSection.cpp

 //===- InputSection.cpp ---------------------------------------------------===//
 //
 //                             The LLVM Linker
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "InputSection.h"
+#include "Config.h"
 #include "Error.h"
 #include "InputFiles.h"
 #include "OutputSections.h"
 #include "Target.h"
 
 using namespace llvm;
 using namespace llvm::ELF;
 using namespace llvm::object;
if (SymIndex < SymTab->sh_info) {
         if (Target->relocNeedsPlt(Type))
           Type = Target->getPCRelReloc();
         else if (Target->relocNeedsGot(Type))
           Type = Target->getPCRelReloc();
         else
           continue;
         break;
       case SymbolBody::UndefinedKind:
-        assert(Body->isWeak() && "Undefined symbol reached writer");
+        assert((Config->NoInhibitExec || Body->isWeak()) &&
+               "Undefined symbol reached writer");
         SymVA = 0;
         break;
       case SymbolBody::LazyKind:
         llvm_unreachable("Lazy symbol reached writer");
       }
 
       if (Target->relocNeedsPlt(OrigType))
         SymVA = PltSec.getEntryAddr(*Body);

ELF/Options.td

def alias_discard_all: Flag<["-"], "x">,
      Alias<discard_all>;
 
 def discard_locals : Flag<["-"], "discard-locals">,
      HelpText<"Delete temporary local symbols">;
 
 def alias_discard_locals: Flag<["-"], "X">,
      Alias<discard_locals>;
 
+def noinhibit_exec : Flag<["--"], "noinhibit-exec">,
+     HelpText<"Retain the executable output file whenever"
+              " it is still usable">;
+
 def discard_none : Flag<["-"], "discard-none">,
      HelpText<"Keep all symbols in the symbol table">;
 
 def export_dynamic : Flag<["--"], "export-dynamic">,
      HelpText<"Put symbols in the dynamic symbol table">;

ELF/Writer.cpp

 //===- Writer.cpp ---------------------------------------------------------===//
 //
 //                             The LLVM Linker
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "Writer.h"
 #include "Config.h"
 #include "OutputSections.h"
 #include "SymbolTable.h"
 #include "Target.h"
static void undefError(const SymbolTable &S, const SymbolBody &Sym) {
   ELFFileBase *SymFile = nullptr;
 
   for (const std::unique_ptr<ObjectFileBase> &F : S.getObjectFiles()) {
     const auto &File = cast<ObjectFile<ELFT>>(*F);
     Elf_Sym_Range Syms = File.getObj().symbols(File.getSymbolTable());
     if (&SymE > Syms.begin() && &SymE < Syms.end())
       SymFile = F.get();
   }
+
+  std::string Message = "undefined symbol: " + Sym.getName().str();
   if (SymFile)
-    error(Twine("undefined symbol: ") + Sym.getName() + " in " +
-          SymFile->getName());
+    Message += " in " + SymFile->getName().str();
+  if (Config->NoInhibitExec)
+    warning(Message);
   else
-    error(Twine("undefined symbol: ") + Sym.getName());
+    error(Message);
 }
 
 // Create output section objects and add them to OutputSections.
 template <class ELFT> void Writer<ELFT>::createSections() {
   SmallDenseMap<SectionKey<ELFT::Is64Bits>, OutputSection<ELFT> *> Map;
 
   OutputSections.push_back(&BssSec);
   Map[{BssSec.getName(), BssSec.getType(), BssSec.getFlags()}] = &BssSec;

test/elf2/no-inhibit-exec.s

+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
+# RUN: not lld -flavor gnu2 %t -o %t2
+# RUN: lld -flavor gnu2 %t --noinhibit-exec -o %t2
+# RUN: llvm-objdump -d %t2 | FileCheck %s
+# REQUIRES: x86
+
+# CHECK: Disassembly of section .text:
+# CHECK-NEXT: _start
+# CHECK-NEXT: 11000:   e8 fb ef fe ff   callq   -69637
+
+# next code will not link without noinhibit-exec flag 
+# because of undefined symbol _bar
+.globl _start;
+_start:
+  call _bar
+