[ELF] - Introduce StripPolicy instead of Config->StripAll/StripDebug flags.

[grimar]

Summary

This approach is not only consistent with UnresolvedPolicy,
but also should help to solve a problem
of options with opposing meanings, mentioned in PR28843

[ruiu]

ELF/Config.h

35

Needs a comment.

ELF/Driver.cpp

351

Add {}.

398

I'd make it explicit that the function depends on Config->Relocatable and move it where the previous assignment to Strip{All,Debug} was.

if (!Config->Relocatable)
  Config->Strip = getStripOption(Args);

Otherwise, it seems to happened to be here because of the alphabetical order.

[grimar]

• Addressed review comments.

ELF/Config.h

35

Done.

ELF/Driver.cpp

351

Done.

398

Done.

[ruiu]

LGTM

ELF/Config.h

enum ELFKind {
   ELF32LEKind,
   ELF32BEKind,
   ELF64LEKind,
   ELF64BEKind
 };
 
 enum class BuildIdKind { None, Fnv1, Md5, Sha1, Hexstring };
 
+enum class StripPolicy { None, All, Debug };
+
 enum class UnresolvedPolicy { NoUndef, Error, Warn, Ignore };
 
 struct SymbolVersion {
   llvm::StringRef Name;
   bool IsExternCpp;
 };
 
 // This struct contains symbols version definition that
struct Configuration {
   bool Pic;
   bool Pie;
   bool PrintGcSections;
   bool Rela;
   bool Relocatable;
   bool SaveTemps;
   bool Shared;
   bool Static = false;
-  bool StripAll;
-  bool StripDebug;
   bool SysvHash = true;
   bool Target1Rel;
   bool Threads;
   bool Trace;
   bool Verbose;
   bool WarnCommon;
   bool ZCombreloc;
   bool ZExecStack;
   bool ZNodelete;
   bool ZNow;
   bool ZOrigin;
   bool ZRelro;
+  StripPolicy Strip;
   UnresolvedPolicy UnresolvedSymbols;
   BuildIdKind BuildId = BuildIdKind::None;
   ELFKind EKind = ELFNoneKind;
   uint16_t DefaultSymbolVersion = llvm::ELF::VER_NDX_GLOBAL;
   uint16_t EMachine = llvm::ELF::EM_NONE;
   uint64_t EntryAddr = -1;
   uint64_t ImageBase;
   uint64_t ZStackSize = -1;

ELF/Driver.cpp

if (auto *Arg = Args.getLastArg(OPT_oformat)) {
     StringRef S = Arg->getValue();
     if (S == "binary")
       return true;
     error("unknown --oformat value: " + S);
   }
   return false;
 }
 
+static StripPolicy getStripOption(opt::InputArgList &Args) {
+  if (Config->Relocatable)
+    return StripPolicy::None;
+
+  if (auto *Arg = Args.getLastArg(OPT_strip_all, OPT_strip_debug))
+    if (Arg->getOption().getID() == OPT_strip_all)
+      return StripPolicy::All;
+    else
+      return StripPolicy::Debug;
+  return StripPolicy::None;
+}
+
 // Initializes Config members by the command line options.
 void LinkerDriver::readConfigs(opt::InputArgList &Args) {
   for (auto *Arg : Args.filtered(OPT_L))
     Config->SearchPaths.push_back(Arg->getValue());
 
   std::vector<StringRef> RPaths;
   for (auto *Arg : Args.filtered(OPT_rpath))
     RPaths.push_back(Arg->getValue());
void LinkerDriver::readConfigs(opt::InputArgList &Args) {
   Config->ICF = Args.hasArg(OPT_icf);
   Config->NoGnuUnique = Args.hasArg(OPT_no_gnu_unique);
   Config->NoUndefinedVersion = Args.hasArg(OPT_no_undefined_version);
   Config->Pie = Args.hasArg(OPT_pie);
   Config->PrintGcSections = Args.hasArg(OPT_print_gc_sections);
   Config->Relocatable = Args.hasArg(OPT_relocatable);
   Config->SaveTemps = Args.hasArg(OPT_save_temps);
   Config->Shared = Args.hasArg(OPT_shared);
-  Config->StripAll = Args.hasArg(OPT_strip_all);
-  Config->StripDebug = Args.hasArg(OPT_strip_debug);
+  Config->Strip = getStripOption(Args);
   Config->Target1Rel = Args.hasArg(OPT_target1_rel);
   Config->Threads = Args.hasArg(OPT_threads);
   Config->Trace = Args.hasArg(OPT_trace);
   Config->Verbose = Args.hasArg(OPT_verbose);
   Config->WarnCommon = Args.hasArg(OPT_warn_common);
 
   Config->DynamicLinker = getString(Args, OPT_dynamic_linker);
   Config->Entry = getString(Args, OPT_entry);
void LinkerDriver::readConfigs(opt::InputArgList &Args) {
   Config->ZNow = hasZOption(Args, "now");
   Config->ZOrigin = hasZOption(Args, "origin");
   Config->ZRelro = !hasZOption(Args, "norelro");
 
   if (Optional<StringRef> Value = getZOptionValue(Args, "stack-size"))
     if (Value->getAsInteger(0, Config->ZStackSize))
       error("invalid stack size: " + *Value);
 
-  if (Config->Relocatable)
-    Config->StripAll = false;
-
-  // --strip-all implies --strip-debug.
-  if (Config->StripAll)
-    Config->StripDebug = true;
-
   // Config->Pic is true if we are generating position-independent code.
   Config->Pic = Config->Pie || Config->Shared;
 
   if (auto *Arg = Args.getLastArg(OPT_hash_style)) {
     StringRef S = Arg->getValue();
     if (S == "gnu") {
       Config->GnuHash = true;
       Config->SysvHash = false;

ELF/InputFiles.cpp

elf::ObjectFile<ELFT>::createInputSection(const Elf_Shdr &Sec) {
   if (Name == ".note.GNU-stack")
     return &InputSection<ELFT>::Discarded;
 
   if (Name == ".note.GNU-split-stack") {
     error("objects using splitstacks are not supported");
     return &InputSection<ELFT>::Discarded;
   }
 
-  if (Config->StripDebug && Name.startswith(".debug"))
+  if (Config->Strip != StripPolicy::None && Name.startswith(".debug"))
     return &InputSection<ELFT>::Discarded;
 
   // The linker merges EH (exception handling) frames and creates a
   // .eh_frame_hdr section for runtime. So we handle them with a special
   // class. For relocatable outputs, they are just passed through.
   if (Name == ".eh_frame" && !Config->Relocatable)
     return new (EHAlloc.Allocate()) EhInputSection<ELFT>(this, &Sec);
 

ELF/Writer.cpp

template <class ELFT> void elf::writeResult() {
 
   if (Config->GnuHash)
     GnuHashTab.reset(new GnuHashTableSection<ELFT>);
   if (Config->SysvHash)
     HashTab.reset(new HashTableSection<ELFT>);
   StringRef S = Config->Rela ? ".rela.plt" : ".rel.plt";
   GotPlt.reset(new GotPltSection<ELFT>);
   RelaPlt.reset(new RelocationSection<ELFT>(S, false /*Sort*/));
-  if (!Config->StripAll) {
+  if (Config->Strip != StripPolicy::All) {
     StrTab.reset(new StringTableSection<ELFT>(".strtab", false));
     SymTabSec.reset(new SymbolTableSection<ELFT>(*StrTab));
   }
   if (Config->EMachine == EM_MIPS && !Config->Shared) {
     // This is a MIPS specific section to hold a space within the data segment
     // of executable file which is pointed to by the DT_MIPS_RLD_MAP entry.
     // See "Dynamic section" in Chapter 5 in the following document:
     // ftp://www.linux-mips.org/pub/linux/mips/doc/ABI/mipsabi.pdf