From be57c88ca7bc2438900b07aa09b8c8bee408eb13 Mon Sep 17 00:00:00 2001 From: Steven Ramirez Rosa Date: Fri, 22 May 2026 06:55:18 -0700 Subject: [PATCH] Make last conflicting option take precedence Update option handling so that the last occurrence of conflicting flags (e.g. --demangle/--no-demangle) takes precedence, matching standard command-line semantics and user expectations. Fix: qualcomm#1179 Signed-off-by: Steven Ramirez Rosa --- lib/LinkerWrapper/GnuLdDriver.cpp | 114 +++++++++--------- lib/LinkerWrapper/RISCVLinkDriver.cpp | 7 +- .../standalone/TLS_GD_ALIGN/GDAlign.test | 2 +- .../standalone/TLS_IE_ALIGN/IEAlign.test | 2 +- .../TLS_IE_GOT_Offsets.test | 2 +- .../FlagOverwrite/FlagOverwrite.test | 27 +++++ .../standalone/FlagOverwrite/Inputs/1.cpp | 1 + .../FlagOverwrite/FlagOverwrite.test | 16 +++ .../RISCV/standalone/FlagOverwrite/Inputs/1.s | 20 +++ .../standalone/FlagOverwrite/Inputs/script.t | 9 ++ 10 files changed, 135 insertions(+), 65 deletions(-) create mode 100644 test/Common/standalone/FlagOverwrite/FlagOverwrite.test create mode 100644 test/Common/standalone/FlagOverwrite/Inputs/1.cpp create mode 100644 test/RISCV/standalone/FlagOverwrite/FlagOverwrite.test create mode 100644 test/RISCV/standalone/FlagOverwrite/Inputs/1.s create mode 100644 test/RISCV/standalone/FlagOverwrite/Inputs/script.t diff --git a/lib/LinkerWrapper/GnuLdDriver.cpp b/lib/LinkerWrapper/GnuLdDriver.cpp index 8b34e5c2f..251d15c82 100644 --- a/lib/LinkerWrapper/GnuLdDriver.cpp +++ b/lib/LinkerWrapper/GnuLdDriver.cpp @@ -328,12 +328,9 @@ bool GnuLdDriver::processOptions(llvm::opt::InputArgList &Args) { if (llvm::opt::Arg *arg = Args.getLastArg(T::sysroot)) Config.setSysRoot(arg->getValue()); - // --fatal-warnings - Config.options().setFatalWarnings(Args.hasArg(T::fatal_warnings)); - - // --no-fatal-warnings - if (Args.hasArg(T::no_fatal_warnings)) - Config.options().setFatalWarnings(false); + // --[no-]fatal-warnings + Config.options().setFatalWarnings( + Args.hasFlag(T::fatal_warnings, T::no_fatal_warnings, /*default=*/false)); // --opt-record-file if (Args.hasArg(T::opt_record_file)) { @@ -469,8 +466,9 @@ bool GnuLdDriver::processOptions(llvm::opt::InputArgList &Args) { } } - // --export-dynamic, -E - Config.options().setExportDynamic(Args.hasArg(T::export_dynamic)); + // --[no-]export-dynamic, -E + Config.options().setExportDynamic( + Args.hasFlag(T::export_dynamic, T::no_export_dynamic, /*default=*/false)); // --export-dynamic-symbol for (auto *Arg : Args.filtered(T::export_dynamic_symbol)) @@ -572,18 +570,14 @@ bool GnuLdDriver::processOptions(llvm::opt::InputArgList &Args) { } } - // --warn-shared-textrel - if (Args.hasArg(T::warn_shared_textrel)) - Config.options().setWarnSharedTextrel(true); + // --[no-]warn-shared-textrel + Config.options().setWarnSharedTextrel(Args.hasFlag( + T::warn_shared_textrel, T::no_warn_shared_textrel, /*default=*/false)); // --warn-common if (Args.hasArg(T::warn_common)) Config.options().setWarnCommon(); - // --no-warn-shared_textrel - if (Args.hasArg(T::no_warn_shared_textrel)) - Config.options().setWarnSharedTextrel(false); - // --enable-newdtags, --disable-newdtags if (Args.hasArg(T::enable_newdtags) && Args.hasArg(T::disable_newdtags)) { errs() << "Cannot specify enable and disable DTAGS at same time!\n"; @@ -619,34 +613,31 @@ bool GnuLdDriver::processOptions(llvm::opt::InputArgList &Args) { T::record_command_line, T::no_record_command_line, /*default=*/false); Config.options().setRecordCommandLine(recordCommandLine); - // --emit-relocs - if (Args.hasArg(T::emit_relocs)) { - Config.options().setEmitGNUCompatRelocs(true); - Config.options().setEmitRelocs(true); + // --[no-]emit-relocs + bool emitRelocs = + Args.hasFlag(T::emit_relocs, T::no_emit_relocs, /*default=*/false); + Config.options().setEmitGNUCompatRelocs(emitRelocs); + Config.options().setEmitRelocs(emitRelocs); + if (emitRelocs) Config.addCommandLine(Table->getOptionName(T::emit_relocs), true); - } // --emit-relocs-llvm if (Args.hasArg(T::emit_relocs_llvm)) Config.options().setEmitRelocs(true); - // --no-emit-relocs - if (Args.hasArg(T::no_emit_relocs)) { - Config.options().setEmitGNUCompatRelocs(false); - Config.options().setEmitRelocs(false); - } - // --no-merge-strings Config.options().setMergeStrings(!Args.hasArg(T::no_merge_strings)); Config.addCommandLine(Table->getOptionName(T::no_merge_strings), Args.hasArg(T::no_merge_strings)); - // --{no-}warn-mismatch - if (Args.getLastArg(T::no_warn_mismatch)) - Config.options().setWarnMismatch(false); - - if (Args.getLastArg(T::warn_mismatch)) - Config.options().setWarnMismatch(true); + // --[no-]warn-mismatch + // This flag defaults to none so only set it if at least one of the two flags + // are present. + if (Args.getLastArg(T::warn_mismatch) || + Args.getLastArg(T::no_warn_mismatch)) { + Config.options().setWarnMismatch( + Args.hasFlag(T::warn_mismatch, T::no_warn_mismatch, /*default=*/true)); + } // --no-trampolines if (Args.hasArg(T::no_trampolines)) { @@ -687,9 +678,15 @@ bool GnuLdDriver::processOptions(llvm::opt::InputArgList &Args) { } Config.addCommandLine(Table->getOptionName(T::flto_options), ltoOptions); - // --no-align-segments - if (Args.hasArg(T::no_align_segments)) - Config.options().setAlignSegments(false); + // --[no-]align-segments + // This flag sets the same thing as --script, -T, --[no-]omagic, -N so only + // set it if at least one of the two flags are present. + if (Args.getLastArg(T::align_segments) || + Args.getLastArg(T::no_align_segments)) { + bool alignSegments = + Args.hasFlag(T::align_segments, T::no_align_segments, /*default=*/true); + Config.options().setAlignSegments(alignSegments); + } bool enableFatalInternalErrors = Args.hasFlag( T::fatal_internal_errors, T::no_fatal_internal_errors, /*default=*/false); @@ -1000,8 +997,9 @@ bool GnuLdDriver::processOptions(llvm::opt::InputArgList &Args) { // Disable --gc-sections, --print-gc-sections for Partial Linking. if (Config.codeGenType() != eld::LinkerConfig::Object) { - // --gc-sections - bool enableGC = Args.hasArg(T::gc_sections); + // --[no-]gc-sections + bool enableGC = + Args.hasFlag(T::gc_sections, T::no_gc_sections, /*default=*/false); Config.options().setGCSections(enableGC); Config.addCommandLine(Table->getOptionName(T::gc_sections), enableGC); // --print-gc-sections @@ -1028,12 +1026,11 @@ bool GnuLdDriver::processOptions(llvm::opt::InputArgList &Args) { // Thread Options. // - // --no-threads, --threads - if (!Args.getLastArg(T::no_threads) || Args.getLastArg(T::threads)) { + // --[no-]threads + if (Args.hasFlag(T::threads, T::no_threads, /*default=*/true)) { Config.options().enableThreads(); Config.addCommandLine(Table->getOptionName(T::threads), true); - } else if (Args.getLastArg(T::no_threads)) { - // --no-threads + } else { Config.options().disableThreads(); Config.options().setNumThreads(1); Config.addCommandLine(Table->getOptionName(T::threads), false); @@ -1112,13 +1109,15 @@ bool GnuLdDriver::processOptions(llvm::opt::InputArgList &Args) { } } - // --no-demangle - if (Args.getLastArg(T::no_demangle)) - Config.options().setDemangleStyle("none"); - - // --demangle - if (Args.getLastArg(T::demangle)) - Config.options().setDemangleStyle("demangle"); + // --[no-]demangle + // This flag sets the same thing as --demangle-style so only set it if at + // least one of the two flags are present. + if (Args.getLastArg(T::demangle) || Args.getLastArg(T::no_demangle)) { + if (Args.hasFlag(T::demangle, T::no_demangle, /*default=*/true)) + Config.options().setDemangleStyle("demangle"); + else + Config.options().setDemangleStyle("none"); + } /// --progress-bar if (Args.getLastArg(T::progress_bar)) @@ -1231,12 +1230,11 @@ bool GnuLdDriver::processOptions(llvm::opt::InputArgList &Args) { if (Args.hasArg(T::use_old_style_trampoline_name)) Config.setUseOldStyleTrampolineName(true); - // --check-sections - if (Args.hasArg(T::enable_overlap_checks)) + // --[no-]check-sections + if (Args.hasFlag(T::enable_overlap_checks, T::disable_overlap_checks, + /*default=*/true)) Config.options().setEnableCheckSectionOverlaps(); - - // --no-check-sections - if (Args.hasArg(T::disable_overlap_checks)) + else Config.options().setDisableCheckSectionOverlaps(); if (Args.hasArg(T::thin_archive_rule_matching_compatibility)) @@ -1279,13 +1277,11 @@ bool GnuLdDriver::processOptions(llvm::opt::InputArgList &Args) { if (Args.hasArg(T::noDefaultPlugins)) Config.options().setNoDefaultPlugins(); - // --no-omagic, --omagic, -N support - if (Args.hasArg(T::no_omagic)) - Config.options().setOMagic(false); - else if (Args.hasArg(T::omagic)) { + // --[no-]omagic, -N + bool omagic = Args.hasFlag(T::omagic, T::no_omagic, /*default=*/false); + Config.options().setOMagic(omagic); + if (omagic) Config.options().setAlignSegments(false); - Config.options().setOMagic(true); - } // --plugin-activity-file= if (llvm::opt::Arg *A = Args.getLastArg(T::PluginActivityFile)) { diff --git a/lib/LinkerWrapper/RISCVLinkDriver.cpp b/lib/LinkerWrapper/RISCVLinkDriver.cpp index 70e52de4e..734332f8b 100644 --- a/lib/LinkerWrapper/RISCVLinkDriver.cpp +++ b/lib/LinkerWrapper/RISCVLinkDriver.cpp @@ -114,9 +114,10 @@ RISCVLinkDriver::parseOptions(ArrayRef Args, return LINK_SUCCESS; } - // --no-relax - if (ArgList.hasArg(OPT_RISCVLinkOptTable::no_riscv_relax)) - Config.options().setRISCVRelax(false); + // --[no-]relax + Config.options().setRISCVRelax( + ArgList.hasFlag(OPT_RISCVLinkOptTable::riscv_relax, + OPT_RISCVLinkOptTable::no_riscv_relax, /*default=*/true)); // --no-relax-zero if (ArgList.hasArg(OPT_RISCVLinkOptTable::no_relax_zero)) diff --git a/test/AArch64/standalone/TLS_GD_ALIGN/GDAlign.test b/test/AArch64/standalone/TLS_GD_ALIGN/GDAlign.test index 5b0ad7beb..d063412cd 100644 --- a/test/AArch64/standalone/TLS_GD_ALIGN/GDAlign.test +++ b/test/AArch64/standalone/TLS_GD_ALIGN/GDAlign.test @@ -1,7 +1,7 @@ #---GDAlign.test----------------------------------------------------------# # Check that TLS GD GOT entries account for TLS block alignment padding. RUN: %clang %clangopts -target aarch64 -c %p/Inputs/gd-align.c -o %t.o -fPIC -ftls-model=global-dynamic -RUN: %link %linkopts -march aarch64 %t.o -o %t.out --no-threads +RUN: %link %linkopts -march aarch64 %t.o -o %t.out --thread-count=1 RUN: %readelf -x .got %t.out | %filecheck %s CHECK: .got diff --git a/test/AArch64/standalone/TLS_IE_ALIGN/IEAlign.test b/test/AArch64/standalone/TLS_IE_ALIGN/IEAlign.test index 34d637a56..6bf566046 100644 --- a/test/AArch64/standalone/TLS_IE_ALIGN/IEAlign.test +++ b/test/AArch64/standalone/TLS_IE_ALIGN/IEAlign.test @@ -1,7 +1,7 @@ #---IEAlign.test----------------------------------------------------------# # Check that TLS IE GOT entries account for TLS block alignment padding. RUN: %clang %clangopts -target aarch64 -c %p/Inputs/ie-align.c -o %t.o -fPIC -ftls-model=initial-exec -RUN: %link %linkopts -march aarch64 %t.o -o %t.out --no-threads +RUN: %link %linkopts -march aarch64 %t.o -o %t.out --thread-count=1 RUN: %readelf -x .got %t.out | %filecheck %s CHECK: .got diff --git a/test/AArch64/standalone/TLS_IE_GOT_Offsets/TLS_IE_GOT_Offsets.test b/test/AArch64/standalone/TLS_IE_GOT_Offsets/TLS_IE_GOT_Offsets.test index 54bdc8043..fb3320a1b 100644 --- a/test/AArch64/standalone/TLS_IE_GOT_Offsets/TLS_IE_GOT_Offsets.test +++ b/test/AArch64/standalone/TLS_IE_GOT_Offsets/TLS_IE_GOT_Offsets.test @@ -13,7 +13,7 @@ RUN: %clang %clangopts -target aarch64 -c %p/Inputs/hw1.c -o %t.o \ RUN: -fPIC -ftls-model=initial-exec -fdata-sections -RUN: %link %linkopts -march aarch64 %t.o -o %t.out --no-threads +RUN: %link %linkopts -march aarch64 %t.o -o %t.out --thread-count=1 RUN: %readelf -x .got %t.out | %filecheck %s # The .got section should contain two TLS IE entries with offsets 0x10 and 0x14 diff --git a/test/Common/standalone/FlagOverwrite/FlagOverwrite.test b/test/Common/standalone/FlagOverwrite/FlagOverwrite.test new file mode 100644 index 000000000..8859d0eaf --- /dev/null +++ b/test/Common/standalone/FlagOverwrite/FlagOverwrite.test @@ -0,0 +1,27 @@ +#---FlagOverwrite.test--------------------- Executable --------------------# +UNSUPPORTED: x86 +#BEGIN_COMMENT +# Check that the last option is the one being used for mutually exclusive flags. +#END_COMMENT +#START_TEST +RUN: %clang %clangopts -ffunction-sections -c %p/Inputs/1.cpp -o %t1.o +RUN: %link -o %t1.out -shared -z=notext %t1.o --no-warn-shared-textrel --warn-shared-textrel 2>&1 | %filecheck %s -check-prefix=TEXTREL +RUN: %link -o %t1.out -shared -z=notext %t1.o --warn-shared-textrel --no-warn-shared-textrel 2>&1 | %filecheck %s -check-prefix=NOTEXTREL -allow-empty +RUN: %not %link -o %t1.out -shared -z=notext %t1.o --warn-shared-textrel --no-fatal-warnings --fatal-warnings 2>&1 | %filecheck %s -check-prefix=FATAL +RUN: %link -o %t1.out -shared -z=notext %t1.o --warn-shared-textrel --fatal-warnings --no-fatal-warnings 2>&1 | %filecheck %s -check-prefix=NOFATAL +RUN: %link %linkopts -o %t1.out %t1.o --verbose --no-demangle --demangle 2>&1 | %filecheck %s -check-prefix=DEMANGLE +RUN: %link %linkopts -o %t1.out %t1.o --verbose --demangle --no-demangle 2>&1 | %filecheck %s -check-prefix=NODEMANGLE +RUN: %link %linkopts -o %t1.out %t1.o --no-omagic --omagic -z max-page-size=0x1000 +RUN: %readelf --elf-output-style LLVM -l -W %t1.out 2>&1 | %filecheck %s -check-prefix=OMAGIC +RUN: %link %linkopts -o %t1.out %t1.o --omagic -z max-page-size=0x1000 --no-omagic +RUN: %readelf --elf-output-style LLVM -l -W %t1.out 2>&1 | %filecheck %s -check-prefix=NOOMAGIC + +TEXTREL: Warning: Add DT_TEXTREL in a shared object! +NOTEXTREL-NOT: Warning: Add DT_TEXTREL in a shared object! +FATAL: Fatal: Linking had errors. +NOFATAL-NOT: Fatal: Linking had errors. +DEMANGLE: Verbose: Adding symbol foo() +NODEMANGLE: Verbose: Adding symbol _Z3foov +OMAGIC-NOT: 4096 +NOOMAGIC: 4096 +#END_TEST \ No newline at end of file diff --git a/test/Common/standalone/FlagOverwrite/Inputs/1.cpp b/test/Common/standalone/FlagOverwrite/Inputs/1.cpp new file mode 100644 index 000000000..05941aa02 --- /dev/null +++ b/test/Common/standalone/FlagOverwrite/Inputs/1.cpp @@ -0,0 +1 @@ +int foo() { return 0; } \ No newline at end of file diff --git a/test/RISCV/standalone/FlagOverwrite/FlagOverwrite.test b/test/RISCV/standalone/FlagOverwrite/FlagOverwrite.test new file mode 100644 index 000000000..d333d160c --- /dev/null +++ b/test/RISCV/standalone/FlagOverwrite/FlagOverwrite.test @@ -0,0 +1,16 @@ +#---FlagOverwrite.test--------------------- Executable------------------# +#BEGIN_COMMENT +# Check that the last option is the one being used for mutually exclusive flags. +# Currently, it checks for: +# --[no-]relax +#END_COMMENT +#START_TEST +RUN: %clang %clangopts -c %p/Inputs/1.s -o %t1.1.o +RUN: %link %linkopts --emit-relocs --no-relax --relax -T %p/Inputs/script.t %t1.1.o -o %t1.out +RUN: %objdump --no-print-imm-hex -d %t1.out | %filecheck %s -check-prefix=RELAX +RUN: %link %linkopts --emit-relocs --relax --no-relax -T %p/Inputs/script.t %t1.1.o -o %t1.out +RUN: %objdump --no-print-imm-hex -d %t1.out | %filecheck %s -check-prefix=NORELAX + +RELAX-NOT: auipc ra, 0 +NORELAX: auipc ra, 0 +#END_TEST \ No newline at end of file diff --git a/test/RISCV/standalone/FlagOverwrite/Inputs/1.s b/test/RISCV/standalone/FlagOverwrite/Inputs/1.s new file mode 100644 index 000000000..9fb94ed21 --- /dev/null +++ b/test/RISCV/standalone/FlagOverwrite/Inputs/1.s @@ -0,0 +1,20 @@ +.section .text.foo +.globl foo +.type foo, @function +foo: + lui a5,%hi(bar) + lw a5,%lo(bar)(a5) + beqz a5,.L4 + call baz +.L4: + nop + +.section .text.baz +.globl baz +.type baz, @function +baz: + nop +.data +.globl bar +bar: + .word 1 diff --git a/test/RISCV/standalone/FlagOverwrite/Inputs/script.t b/test/RISCV/standalone/FlagOverwrite/Inputs/script.t new file mode 100644 index 000000000..b8fe79646 --- /dev/null +++ b/test/RISCV/standalone/FlagOverwrite/Inputs/script.t @@ -0,0 +1,9 @@ +SECTIONS { + .text (0x1233000) : { + *(.text.foo) + *(.text.baz) + } + .data (0x12345600) : { + *(.data) + } +}