[ELF] - Change way how we handle --noinhibit-exec

[grimar]

Summary

It is splitted from D35724, which uses this change,

Previously we handled this option implicitly, only for infering
unresolved symbols handling policy.
ld man says: "--noinhibit-exec Retain the executable output file whenever
it is still usable", we we may want to handle other cases too.

[ruiu]

ELF/Relocations.cpp

913–916

This is the only place you are using errorOrWarn, so don't add that function just for -noinbhiti-exec.

[ruiu]

I think preventing almost all errors is too much. Most errors are still checked even if --noinhibit-exec is specified. For example, if you give nonexistent file to the linker, it is still an error.

[grimar]

• Addressed comments.

ELF/Relocations.cpp

913–916

Added more places using errorOrWarn

[ruiu]

Yes, this looks fine. LGTM

ELF/Driver.cpp

646–648

Sort

[grimar]

ELF/Driver.cpp

646–648

Done (I hope I got sorting rule correctly now).
But isn't it easier/more usefu(for search) just to ignore character upper/lower cases
when sorting this list ?

Not sure it makes sense to have current:

Config->NoGnuUnique = Args.hasArg(OPT_no_gnu_unique);
Config->NoUndefinedVersion = Args.hasArg(OPT_no_undefined_version);
Config->NoinhibitExec = Args.hasArg(OPT_noinhibit_exec);
Config->Nostdlib = Args.hasArg(OPT_nostdlib);

instead of straightforward:

Config->NoGnuUnique = Args.hasArg(OPT_no_gnu_unique);
Config->NoinhibitExec = Args.hasArg(OPT_noinhibit_exec);
Config->Nostdlib = Args.hasArg(OPT_nostdlib);
Config->NoUndefinedVersion = Args.hasArg(OPT_no_undefined_version);

[ruiu]

lld/trunk/ELF/Config.h

131 ↗

(On Diff #108242)

Sort. In general, please sort in ASCIIbetical order unless there's other local rule there.

ELF/Config.h

 
 // For --discard-{all,locals,none}.
 enum class DiscardPolicy { Default, All, Locals, None };
 
 // For --strip-{all,debug}.
 enum class StripPolicy { None, All, Debug };
 
 // For --unresolved-symbols.
-enum class UnresolvedPolicy { ReportError, Warn, WarnAll, Ignore, IgnoreAll };
+enum class UnresolvedPolicy { ReportError, Warn, Ignore, IgnoreAll };
 
 // For --sort-section and linkerscript sorting rules.
 enum class SortSectionPolicy { Default, None, Alignment, Name, Priority };
 
 // For --target2
 enum class Target2Policy { Abs, Rel, GotRel };
 
 struct SymbolVersion {
struct Configuration {
   bool ExportDynamic;
   bool FatalWarnings;
   bool GcSections;
   bool GdbIndex;
   bool GnuHash;
   bool ICF;
   bool MipsN32Abi = false;
   bool NoGnuUnique;
+  bool NoinhibitExec;
   bool NoUndefinedVersion;
   bool Nostdlib;
   bool OFormatBinary;
   bool Omagic;
   bool OptRemarksWithHotness;
   bool Pie;
   bool PrintGcSections;
   bool Relocatable;

ELF/Driver.cpp

 static std::string getRpath(opt::InputArgList &Args) {
   std::vector<StringRef> V = getArgs(Args, OPT_rpath);
   return llvm::join(V.begin(), V.end(), ":");
 }
 
 // Determines what we should do if there are remaining unresolved
 // symbols after the name resolution.
 static UnresolvedPolicy getUnresolvedSymbolPolicy(opt::InputArgList &Args) {
-  // -noinhibit-exec or -r imply some default values.
-  if (Args.hasArg(OPT_noinhibit_exec))
-    return UnresolvedPolicy::WarnAll;
   if (Args.hasArg(OPT_relocatable))
     return UnresolvedPolicy::IgnoreAll;
 
   UnresolvedPolicy ErrorOrWarn = getArg(Args, OPT_error_unresolved_symbols,
                                         OPT_warn_unresolved_symbols, true)
                                      ? UnresolvedPolicy::ReportError
                                      : UnresolvedPolicy::Warn;
 
void LinkerDriver::readConfigs(opt::InputArgList &Args) {
   Config->GdbIndex = Args.hasArg(OPT_gdb_index);
   Config->ICF = Args.hasArg(OPT_icf);
   Config->Init = Args.getLastArgValue(OPT_init, "_init");
   Config->LTOAAPipeline = Args.getLastArgValue(OPT_lto_aa_pipeline);
   Config->LTONewPmPasses = Args.getLastArgValue(OPT_lto_newpm_passes);
   Config->LTOO = getInteger(Args, OPT_lto_O, 2);
   Config->LTOPartitions = getInteger(Args, OPT_lto_partitions, 1);
   Config->MapFile = Args.getLastArgValue(OPT_Map);
   Config->NoGnuUnique = Args.hasArg(OPT_no_gnu_unique);
+  Config->NoinhibitExec = Args.hasArg(OPT_noinhibit_exec);
   Config->NoUndefinedVersion = Args.hasArg(OPT_no_undefined_version);
   Config->Nostdlib = Args.hasArg(OPT_nostdlib);
   Config->OFormatBinary = isOutputFormatBinary(Args);
   Config->Omagic = Args.hasArg(OPT_omagic);
   Config->OptRemarksFilename = Args.getLastArgValue(OPT_opt_remarks_filename);
   Config->OptRemarksWithHotness = Args.hasArg(OPT_opt_remarks_with_hotness);
   Config->Optimize = getInteger(Args, OPT_O, 1);
   Config->OutputFile = Args.getLastArgValue(OPT_o);
   Config->Pie = getArg(Args, OPT_pie, OPT_nopie, false);

ELF/Relocations.cpp

for (SharedSymbol *Sym : getSymbolsAt<ELFT>(SS)) {
     Sym->CopyRelSec = Sec;
     Sym->CopyRelSecOff = Off;
     Sym->symbol()->IsUsedInRegularObj = true;
   }
 
   In<ELFT>::RelaDyn->addReloc({Target->CopyRel, Sec, Off, false, SS, 0});
 }
 
+static void errorOrWarn(const Twine &Msg) {
+  if (!Config->NoinhibitExec)
+    error(Msg);
+  else
+    warn(Msg);
+}
+
 template <class ELFT>
 static RelExpr adjustExpr(SymbolBody &Body, RelExpr Expr, uint32_t Type,
                           const uint8_t *Data, InputSectionBase &S,
                           typename ELFT::uint RelOff) {
   if (Body.isGnuIFunc()) {
     Expr = toPlt(Expr);
   } else if (!isPreemptible(Body, Type)) {
     if (needsPlt(Expr))
if (Body.isFunc()) {
     // pointer equality is maintained. To avoid an infinite loop, the only entry
     // that points to the real function is a dedicated got entry used by the
     // plt. That is identified by special relocation types (R_X86_64_JUMP_SLOT,
     // R_386_JMP_SLOT, etc).
     Body.NeedsPltAddr = true;
     return toPlt(Expr);
   }
 
-  error("symbol '" + toString(Body) + "' defined in " + toString(Body.File) +
-        " has no type");
+  errorOrWarn("symbol '" + toString(Body) + "' defined in " +
+              toString(Body.File) + " has no type");
   return Expr;
 }
 
 // Returns an addend of a given relocation. If it is RELA, an addend
 // is in a relocation itself. If it is REL, we need to read it from an
 // input section.
 template <class ELFT, class RelTy>
 static int64_t computeAddend(const RelTy &Rel, const uint8_t *Buf) {
static void reportUndefined(SymbolBody &Sym, InputSectionBase &S,
   std::string Msg =
       "undefined symbol: " + toString(Sym) + "\n>>> referenced by ";
 
   std::string Src = S.getSrcMsg<ELFT>(Offset);
   if (!Src.empty())
     Msg += Src + "\n>>>               ";
   Msg += S.getObjMsg<ELFT>(Offset);
 
-  if (Config->UnresolvedSymbols == UnresolvedPolicy::WarnAll ||
-      (Config->UnresolvedSymbols == UnresolvedPolicy::Warn && CanBeExternal)) {
+  if (Config->UnresolvedSymbols == UnresolvedPolicy::Warn && CanBeExternal)
     warn(Msg);
-  } else {
-    error(Msg);
-  }
+  else
+    errorOrWarn(Msg);
 }
 
 template <class RelTy>
 static std::pair<uint32_t, uint32_t>
 mergeMipsN32RelTypes(uint32_t Type, uint32_t Offset, RelTy *I, RelTy *E) {
   // MIPS N32 ABI treats series of successive relocations with the same offset
   // as a single relocation. The similar approach used by N64 ABI, but this ABI
   // packs all relocations into the single relocation record. Here we emulate
if (needsGot(Expr)) {
         addGotEntry<ELFT>(Body, Preemptible);
       }
     }
 
     if (!needsPlt(Expr) && !needsGot(Expr) && isPreemptible(Body, Type)) {
       // We don't know anything about the finaly symbol. Just ask the dynamic
       // linker to handle the relocation for us.
       if (!Target->isPicRel(Type))
-        error("relocation " + toString(Type) +
-              " cannot be used against shared object; recompile with -fPIC" +
-              getLocation<ELFT>(Sec, Body, Offset));
+        errorOrWarn(
+            "relocation " + toString(Type) +
 
       In<ELFT>::RelaDyn->addReloc(
           {Target->getDynRel(Type), &Sec, Offset, false, &Body, Addend});
 
       // MIPS ABI turns using of GOT and dynamic relocations inside out.
       // While regular ABI uses dynamic relocations to fill up GOT entries
       // MIPS ABI requires dynamic linker to fills up GOT entries using
       // specially sorted dynamic symbol table. This affects even dynamic

test/ELF/copy-errors.s

 // REQUIRES: x86
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %p/Inputs/protected-shared.s -o %t2.o
 // RUN: ld.lld %t2.o -o %t2.so -shared
 // RUN: not ld.lld %t.o %t2.so -o %t 2>&1 | FileCheck %s
 
 // CHECK: cannot preempt symbol: bar
 // CHECK: >>> defined in {{.*}}.so
 // CHECK: >>> referenced by {{.*}}.o:(.text+0x1)
 // CHECK: symbol 'zed' defined in {{.*}}.so has no type
 
+// RUN: not ld.lld --noinhibit-exec %t.o %t2.so -o %t 2>&1 | FileCheck %s --check-prefix=NOINHIBIT
+// NOINHIBIT: warning: symbol 'zed' defined in {{.*}}.so has no type
+
 .global _start
 _start:
 call bar
 call zed

test/ELF/x86-64-dyn-rel-error.s

 // REQUIRES: x86
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %p/Inputs/shared.s -o %t2.o
 // RUN: ld.lld %t2.o -shared -o %t2.so
 // RUN: not ld.lld %t.o %t2.so -o %t 2>&1 | FileCheck %s
 
         .global _start
 _start:
         .data
         .long bar
 
 // CHECK: relocation R_X86_64_32 cannot be used against shared object; recompile with -fPIC
+
+// RUN: ld.lld --noinhibit-exec %t.o %t2.so -o %t 2>&1 | FileCheck %s