[PATCH] D35793: [ELF] - Change way how we handle --noinhibit-exec

[Rafael Avila de Espindola via llvm-commits]

LGTM

Rui, what do you think?

Cheers,
Rafael

George Rimar via Phabricator <reviews at reviews.llvm.org> writes:

> grimar updated this revision to Diff 108022.
> grimar added a comment.
>
> - Addressed comments.
>
>
> https://reviews.llvm.org/D35793
>
> Files:
>   ELF/Config.h
>   ELF/Driver.cpp
>   ELF/Relocations.cpp
>   test/ELF/copy-errors.s
>   test/ELF/x86-64-dyn-rel-error.s
>
> Index: test/ELF/x86-64-dyn-rel-error.s
> ===================================================================
> --- test/ELF/x86-64-dyn-rel-error.s
> +++ test/ELF/x86-64-dyn-rel-error.s
> @@ -10,3 +10,5 @@
>          .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
> Index: test/ELF/copy-errors.s
> ===================================================================
> --- test/ELF/copy-errors.s
> +++ test/ELF/copy-errors.s
> @@ -9,6 +9,9 @@
>  // 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
> Index: ELF/Relocations.cpp
> ===================================================================
> --- ELF/Relocations.cpp
> +++ ELF/Relocations.cpp
> @@ -535,6 +535,13 @@
>    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,
> @@ -609,8 +616,8 @@
>      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;
>  }
>  
> @@ -691,12 +698,10 @@
>      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>
> @@ -905,9 +910,10 @@
>        // 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) +
> +            " cannot be used against shared object; recompile with -fPIC" +
> +            getLocation<ELFT>(Sec, Body, Offset));
>  
>        In<ELFT>::RelaDyn->addReloc(
>            {Target->getDynRel(Type), &Sec, Offset, false, &Body, Addend});
> Index: ELF/Driver.cpp
> ===================================================================
> --- ELF/Driver.cpp
> +++ ELF/Driver.cpp
> @@ -418,9 +418,6 @@
>  // 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;
>  
> @@ -647,6 +644,7 @@
>    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);
> Index: ELF/Config.h
> ===================================================================
> --- ELF/Config.h
> +++ ELF/Config.h
> @@ -44,7 +44,7 @@
>  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 };
> @@ -128,6 +128,7 @@
>    bool ICF;
>    bool MipsN32Abi = false;
>    bool NoGnuUnique;
> +  bool NoinhibitExec;
>    bool NoUndefinedVersion;
>    bool Nostdlib;
>    bool OFormatBinary;