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

[Rafael Avila de Espindola via llvm-commits]

The intention is to prevent almost all errors, no? So maybe you want to
check NoinhibitExec in error() and the places that do have to fail can use
fatal()?

Cheers,
Rafael


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

> grimar created this revision.
> Herald added a subscriber: emaste.
>
> It is splitted from https://reviews.llvm.org/D35724, whicch 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.
>
>
> https://reviews.llvm.org/D35793
>
> Files:
>   ELF/Config.h
>   ELF/Driver.cpp
>   ELF/Relocations.cpp
>   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: 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,
> @@ -691,7 +698,7 @@
>      Msg += Src + "\n>>>               ";
>    Msg += S.getObjMsg<ELFT>(Offset);
>  
> -  if (Config->UnresolvedSymbols == UnresolvedPolicy::WarnAll ||
> +  if (Config->NoinhibitExec ||
>        (Config->UnresolvedSymbols == UnresolvedPolicy::Warn && CanBeExternal)) {
>      warn(Msg);
>    } else {
> @@ -905,9 +912,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;
>
>
> 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: 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,
> @@ -691,7 +698,7 @@
>      Msg += Src + "\n>>>               ";
>    Msg += S.getObjMsg<ELFT>(Offset);
>  
> -  if (Config->UnresolvedSymbols == UnresolvedPolicy::WarnAll ||
> +  if (Config->NoinhibitExec ||
>        (Config->UnresolvedSymbols == UnresolvedPolicy::Warn && CanBeExternal)) {
>      warn(Msg);
>    } else {
> @@ -905,9 +912,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;