[ELF] - Fix (partial) for bug 28843 - Make sure we handle options with opposing meanings.

[grimar]

Summary

As stated in PR28843:

we should handle command lines with

• -target1-rel -target1-abs
• --demangle --no-demangle

Patch implements this for specified options.
There are also others conflicting options, so fix is "partial".

[ruiu]

ELF/Driver.cpp

347–357 ↗

(On Diff #69226)

I can see the same pattern here. How about this?

static bool getArg(opt::InputArgList &Args, unsigned K1, unsigned K2, bool Default) {
  if (auto *Arg = Args.getLastArg(K1, K2))
    return Arg->getOption().getID() == K1;
  return Default;
}

[grimar]

ELF/Driver.cpp

347–357 ↗

(On Diff #69226)

I tried the same when wrote this. Finished with my version because find it slightly more readable (too much arguments imo).
No real preference here though. Lets go with your version if you prefer it.

[grimar]

• Addressed comments.

[grimar]

I think I still slightly prefer diff1 :)

[• rafael]

Second one LGTM. Maybe just rename K1 and K2 to more descriptive names.

[ruiu]

LGTM

lld/trunk/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 bool getArg(opt::InputArgList &Args, unsigned K1, unsigned K2,
+                   bool Default) {
+  if (auto *Arg = Args.getLastArg(K1, K2))
+    return Arg->getOption().getID() == K1;
+  return Default;
+}
+
 static DiscardPolicy getDiscardOption(opt::InputArgList &Args) {
   auto *Arg =
       Args.getLastArg(OPT_discard_all, OPT_discard_locals, OPT_discard_none);
   if (!Arg)
     return DiscardPolicy::Default;
 
   switch (Arg->getOption().getID()) {
   case OPT_discard_all:
     return DiscardPolicy::All;
   case OPT_discard_locals:
     return DiscardPolicy::Locals;
   case OPT_discard_none:
     return DiscardPolicy::None;
   default:
     llvm_unreachable("unknown discard option");
   }
 }
 
-
 static StripPolicy getStripOption(opt::InputArgList &Args) {
   if (auto *Arg = Args.getLastArg(OPT_strip_all, OPT_strip_debug)) {
     if (Arg->getOption().getID() == OPT_strip_all)
       return StripPolicy::All;
     return StripPolicy::Debug;
   }
   return StripPolicy::None;
 }
if (auto *Arg = Args.getLastArg(OPT_m)) {
     StringRef S = Arg->getValue();
     std::tie(Config->EKind, Config->EMachine) = parseEmulation(S);
     Config->Emulation = S;
   }
 
   Config->AllowMultipleDefinition = Args.hasArg(OPT_allow_multiple_definition);
   Config->Bsymbolic = Args.hasArg(OPT_Bsymbolic);
   Config->BsymbolicFunctions = Args.hasArg(OPT_Bsymbolic_functions);
-  Config->Demangle = !Args.hasArg(OPT_no_demangle);
+  Config->Demangle = getArg(Args, OPT_demangle, OPT_no_demangle, true);
   Config->DisableVerify = Args.hasArg(OPT_disable_verify);
   Config->Discard = getDiscardOption(Args);
   Config->EhFrameHdr = Args.hasArg(OPT_eh_frame_hdr);
   Config->EnableNewDtags = !Args.hasArg(OPT_disable_new_dtags);
   Config->ExportDynamic = Args.hasArg(OPT_export_dynamic);
   Config->FatalWarnings = Args.hasArg(OPT_fatal_warnings);
   Config->GcSections = Args.hasArg(OPT_gc_sections);
   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->Target1Rel = Args.hasArg(OPT_target1_rel);
+  Config->Target1Rel = getArg(Args, OPT_target1_rel, OPT_target1_abs, false);
   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);
   Config->Fini = getString(Args, OPT_fini, "_fini");

lld/trunk/test/ELF/arm-target1.s

 // REQUIRES: arm
 // RUN: llvm-mc -filetype=obj -triple=armv7a-none-linux-gnueabi %s -o %t.o
 // RUN: llvm-readobj -r %t.o | FileCheck %s --check-prefix=RELOC
 // RUN: ld.lld -shared %t.o -o %t2.so --target1-rel
 // RUN: llvm-objdump -t -d %t2.so | FileCheck %s \
 // RUN:   --check-prefix=RELATIVE
 // RUN: not ld.lld -shared %t.o -o %t3.so 2>&1 | FileCheck %s \
 // RUN:   --check-prefix=ABS
 
+// RUN: ld.lld -shared %t.o -o %t2.so --target1-abs --target1-rel
+// RUN: llvm-objdump -t -d %t2.so | FileCheck %s \
+// RUN:   --check-prefix=RELATIVE
+// RUN: not ld.lld -shared %t.o -o %t3.so --target1-rel --target1-abs 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=ABS
+
 // RELOC: Relocations [
 // RELOC:   .rel.text {
 // RELOC:     0x0 R_ARM_TARGET1 patatino 0x0
 // RELOC:   }
 // RELOC: ]
 
 .text
   .word patatino(target1)

lld/trunk/test/ELF/conflict.s

 # REQUIRES: x86, shell
 
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t1.o
 # RUN: not ld.lld %t1.o %t1.o -o %t2 2>&1 | FileCheck -check-prefix=DEMANGLE %s
 
 # DEMANGLE:    duplicate symbol: mul(double, double) in
 # DEMANGLE:    duplicate symbol: foo in
 
 # RUN: not ld.lld %t1.o %t1.o -o %t2 --no-demangle 2>&1 | \
 # RUN:   FileCheck -check-prefix=NO_DEMANGLE %s
 
 # NO_DEMANGLE: duplicate symbol: _Z3muldd in
 # NO_DEMANGLE: duplicate symbol: foo in
 
+# RUN: not ld.lld %t1.o %t1.o -o %t2 --demangle --no-demangle 2>&1 | \
+# RUN:   FileCheck -check-prefix=NO_DEMANGLE %s
+# RUN: not ld.lld %t1.o %t1.o -o %t2 --no-demangle --demangle 2>&1 | \
+# RUN:   FileCheck -check-prefix=DEMANGLE %s
+
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %S/Inputs/conflict.s -o %t2.o
 # RUN: llvm-ar rcs %t3.a %t2.o
 # RUN: not ld.lld %t1.o %t3.a -u baz -o %t2 2>&1 | FileCheck -check-prefix=ARCHIVE %s
 
 # ARCHIVE: duplicate symbol: foo in {{.*}}1.o and {{.*}}3.a({{.*}}2.o)
 
 .globl _Z3muldd, foo
 _Z3muldd:
 foo:
   mov $60, %rax
   mov $42, %rdi
   syscall