[Clang][IA] support -generate-unused-section-symbols={yes|no}

[nickdesaulniers]

Summary

GNU binutils has added support for explicitly retaining (yes option) or
dropping (no option) STT_SECTION symbols from the symbols table via
assembler option -generate-unused-section-symbols={yes|no}.

Add support to clang (-Wa,-generate-unused-section-symbols={yes|no})
and llvm-mc.

This feature might be used by the Linux kernel.
https://lore.kernel.org/linux-toolchains/[e-mail]/
This has been supported by GNU binutils since 2.36.
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=d1bcae833b32f1408485ce69f844dcd7ded093a8

[MaskRay]

I wish we don't do this.
https://sourceware.org/pipermail/binutils/2022-March/119940.html (RFC: GAS: Add option to generate unused section symbols):

This problem was fixed in the kernel (and backported by distros to their kernels if binutils was updated); it's fairly simplistic changes.

I don't see the need for returning (optionally) to the old behaviour again. I mean, at the time, when 2.36 came out, sure, the patch might have made sense, but now?

LLVM integrated assembler has stayed the current behavior for many years.
Having the feature would only benefit the next major release 15.0.0. Versions 11~14 are still supported, so some conditional compilation is needed, making the feature less useful.

I am not familiar with the tool mentioned on The trouble with __weak and objtool got worse, but I think making the tool work with discarded STT_SECTION symbols isn't likely to cause much trouble.

[MaskRay]

Add support to clang (-Wa,-generate-unused-section-symbols={yes|no}) and llvm-mc.

This feature might be used by the Linux kernel. https://lore.kernel.org/linux-toolchains/[e-mail]/

This has been supported by GNU binutils since 2.36. https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=d1bcae833b32f1408485ce69f844dcd7ded093a8

AFAICT GNU as never gets an option named -generate-unused-section-symbols=yes.
It just has a quirk that interprets nearly all -g* options as -g, which indicates generation of some compile units for ELF symbols with the help of .size directives.
It's very different from generating unused section symbols.

[MaskRay]

In D123874#3454723, @MaskRay wrote:

Add support to clang (-Wa,-generate-unused-section-symbols={yes|no}) and llvm-mc.

This feature might be used by the Linux kernel. https://lore.kernel.org/linux-toolchains/[e-mail]/

This has been supported by GNU binutils since 2.36. https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=d1bcae833b32f1408485ce69f844dcd7ded093a8

AFAICT GNU as never gets an option named -generate-unused-section-symbols=yes.
It just has a quirk that interprets nearly all -g* options as -g, which indicates generation of some compile units for ELF symbols with the help of .size directives.
It's very different from generating unused section symbols.

Latest GNU as added an error for an unknown -g option.

clang/include/clang/Basic/CodeGenOptions.def

 #  define ENUM_CODEGENOPT(Name, Type, Bits, Default) \
 CODEGENOPT(Name, Bits, Default)
 #endif
 
 CODEGENOPT(DisableIntegratedAS, 1, 0) ///< -no-integrated-as
 ENUM_CODEGENOPT(CompressDebugSections, llvm::DebugCompressionType, 2,
                 llvm::DebugCompressionType::None)
 CODEGENOPT(RelaxELFRelocations, 1, 0) ///< -Wa,--mrelax-relocations
+CODEGENOPT(KeepSectionSymbols, 1, 0) ///< -Wa,-generate-unused-section-symbols=yes
 CODEGENOPT(AsmVerbose        , 1, 0) ///< -dA, -fverbose-asm.
 CODEGENOPT(Dwarf64           , 1, 0) ///< -gdwarf64.
 CODEGENOPT(Dwarf32           , 1, 1) ///< -gdwarf32.
 CODEGENOPT(PreserveAsmComments, 1, 1) ///< -dA, -fno-preserve-as-comments.
 CODEGENOPT(AssumeSaneOperatorNew , 1, 1) ///< implicit __attribute__((malloc)) operator new
 CODEGENOPT(Autolink          , 1, 1) ///< -fno-autolink
 CODEGENOPT(ObjCAutoRefCountExceptions , 1, 0) ///< Whether ARC should be EH-safe.
 CODEGENOPT(Backchain         , 1, 0) ///< -mbackchain

clang/include/clang/Driver/Options.td

 
 // Assemble Options
 def dwarf_debug_producer : Separate<["-"], "dwarf-debug-producer">,
   HelpText<"The string to embed in the Dwarf debug AT_producer record.">;
 
 def defsym : Separate<["-"], "defsym">,
   HelpText<"Define a value for a symbol">;
 
+def keep_section_symbols : Separate<["-"], "keep-section-symbols">,
+  HelpText<"(integrated-as) Retain section symbols">,
+  MarshallingInfoFlag<CodeGenOpts<"KeepSectionSymbols">>;
+
 } // let Flags = [CC1AsOption]
 
 //===----------------------------------------------------------------------===//
 // clang-cl Options
 //===----------------------------------------------------------------------===//
 
 def cl_Group : OptionGroup<"<clang-cl options>">, Flags<[CLOption]>,
   HelpText<"CL.EXE COMPATIBILITY OPTIONS">;

clang/lib/Driver/ToolChains/Clang.cpp

for (StringRef Value : A->getValues()) {
       } else if (Value.consume_front("-fdebug-compilation-dir=")) {
         // The flag is a -Wa / -Xassembler argument and Options doesn't
         // parse the argument, so this isn't automatically aliased to
         // -fdebug-compilation-dir (without '=') here.
         CmdArgs.push_back("-fdebug-compilation-dir");
         CmdArgs.push_back(Value.data());
       } else if (Value == "--version") {
         D.PrintVersion(C, llvm::outs());
+      } else if (Value.consume_front("-generate-unused-section-symbols=") &&
+                 (Value == "yes" || Value == "no")) {
+        if (Value == "yes")
+          CmdArgs.push_back("-keep-section-symbols");
       } else {
         D.Diag(diag::err_drv_unsupported_option_argument)
             << A->getOption().getName() << Value;
       }
     }
   }
   if (ImplicitIt.size())
     AddARMImplicitITArgs(Args, CmdArgs, ImplicitIt);

clang/test/Driver/as-options.s

 
 // But -m flags for the integrated assembler _should_ warn if the integrated
 // assembler is not in use.
 // RUN: %clang -mrelax-all -fintegrated-as -x c++ %s -S -o /dev/null 2>&1 \
 // RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
 // RUN: %clang -mrelax-all -fno-integrated-as -x c++ %s -S -o /dev/null 2>&1 \
 // RUN:   | FileCheck --check-prefix=WARN --allow-empty %s
 // WARN: unused
+
+// RUN: %clang -Wa,-generate-unused-section-symbols=yes %s -c -o /dev/null -v 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-KEEP
+// RUN: %clang -Wa,-generate-unused-section-symbols=no %s -c -o /dev/null -v 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-DROP
+// RUN: %clang %s -c -o /dev/null -v 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-DROP
+// CHECK-KEEP: -keep-section-symbols
+// CHECK-DROP-NOT: -keep-section-symbols

clang/test/Misc/cc1as-keep-section-symbols.s

+// REQUIRES: x86-registered-target
+
+// RUN: %clang -cc1as -triple x86_64-pc-linux-gnu -filetype obj -keep-section-symbols %s -o %t
+// RUN: llvm-readelf -s %t | FileCheck %s --check-prefix=CHECK-KEEP
+
+// RUN: %clang -cc1as -triple x86_64-pc-linux-gnu -filetype obj %s -o %t
+// RUN: llvm-readelf -s %t | FileCheck %s --check-prefix=CHECK-DROP
+
+// CHECK-KEEP:     SECTION LOCAL DEFAULT {{[0-9]+}} .text
+// CHECK-DROP-NOT: SECTION LOCAL DEFAULT {{[0-9]+}} .text
+.text
+  nop

clang/tools/driver/cc1as_main.cpp

struct AssemblerInvocation {
   /// @{
 
   unsigned RelaxAll : 1;
   unsigned NoExecStack : 1;
   unsigned FatalWarnings : 1;
   unsigned NoWarn : 1;
   unsigned IncrementalLinkerCompatible : 1;
   unsigned EmbedBitcode : 1;
+  unsigned KeepSectionSymbols : 1;
 
   /// The name of the relocation model to use.
   std::string RelocationModel;
 
   /// The ABI targeted by the backend. Specified using -target-abi. Empty
   /// otherwise.
   std::string TargetABI;
 
bool AssemblerInvocation::CreateFromArgs(AssemblerInvocation &Opts,
   Opts.FatalWarnings = Args.hasArg(OPT_massembler_fatal_warnings);
   Opts.NoWarn = Args.hasArg(OPT_massembler_no_warn);
   Opts.RelocationModel =
       std::string(Args.getLastArgValue(OPT_mrelocation_model, "pic"));
   Opts.TargetABI = std::string(Args.getLastArgValue(OPT_target_abi));
   Opts.IncrementalLinkerCompatible =
       Args.hasArg(OPT_mincremental_linker_compatible);
   Opts.SymbolDefs = Args.getAllArgValues(OPT_defsym);
+  Opts.KeepSectionSymbols = Args.hasArg(OPT_keep_section_symbols);
 
   // EmbedBitcode Option. If -fembed-bitcode is enabled, set the flag.
   // EmbedBitcode behaves the same for all embed options for assembly files.
   if (auto *A = Args.getLastArg(OPT_fembed_bitcode_EQ)) {
     Opts.EmbedBitcode = llvm::StringSwitch<unsigned>(A->getValue())
                             .Case("all", 1)
                             .Case("bitcode", 1)
                             .Case("marker", 1)
std::unique_ptr<MCAsmInfo> MAI(
       TheTarget->createMCAsmInfo(*MRI, Opts.Triple, MCOptions));
   assert(MAI && "Unable to create target asm info!");
 
   // Ensure MCAsmInfo initialization occurs before any use, otherwise sections
   // may be created with a combination of default and explicit settings.
   MAI->setCompressDebugSections(Opts.CompressDebugSections);
 
   MAI->setRelaxELFRelocations(Opts.RelaxELFRelocations);
+  MAI->setKeepSectionSymbols(Opts.KeepSectionSymbols);
 
   bool IsBinary = Opts.OutputType == AssemblerInvocation::FT_Obj;
   if (Opts.OutputPath.empty())
     Opts.OutputPath = "-";
   std::unique_ptr<raw_fd_ostream> FDOS =
       getOutputStream(Opts.OutputPath, Diags, IsBinary);
   if (!FDOS)
     return true;

llvm/include/llvm/MC/MCAsmInfo.h

protected:
   /// True if the integrated assembler should interpret 'a >> b' constant
   /// expressions as logical rather than arithmetic.
   bool UseLogicalShr = true;
 
   // If true, emit GOTPCRELX/REX_GOTPCRELX instead of GOTPCREL, on
   // X86_64 ELF.
   bool RelaxELFRelocations = true;
 
+  // If true, emit ELF STT_SECTION symbols.
+  bool KeepSectionSymbols = false;
+
   // If true, then the lexer and expression parser will support %neg(),
   // %hi(), and similar unary operators.
   bool HasMipsExpressions = false;
 
   // If true, use Motorola-style integers in Assembly (ex. $0ac).
   bool UseMotorolaIntegers = false;
 
   // If true, emit function descriptor symbol on AIX.
public:
   void setCompressDebugSections(DebugCompressionType CompressDebugSections) {
     this->CompressDebugSections = CompressDebugSections;
   }
 
   bool shouldUseLogicalShr() const { return UseLogicalShr; }
 
   bool canRelaxRelocations() const { return RelaxELFRelocations; }
   void setRelaxELFRelocations(bool V) { RelaxELFRelocations = V; }
+  bool getKeepSectionSymbols() const { return KeepSectionSymbols; }
+  void setKeepSectionSymbols(bool V) { KeepSectionSymbols = V; }
   bool hasMipsExpressions() const { return HasMipsExpressions; }
   bool needsFunctionDescriptors() const { return NeedsFunctionDescriptors; }
   bool shouldUseMotorolaIntegers() const { return UseMotorolaIntegers; }
 };
 
 } // end namespace llvm
 
 #endif // LLVM_MC_MCASMINFO_H

llvm/lib/MC/ELFObjectWriter.cpp

if (Symbol.isVariable() && Symbol.isUndefined()) {
     Layout.getBaseSymbol(Symbol);
     return false;
   }
 
   if (Symbol.isTemporary())
     return false;
 
   if (Symbol.getType() == ELF::STT_SECTION)
-    return false;
+    return Layout.getAssembler()
+        .getContext()
+        .getAsmInfo()
+        ->getKeepSectionSymbols();
 
   return true;
 }
 
 void ELFWriter::computeSymbolTable(
     MCAssembler &Asm, const MCAsmLayout &Layout,
     const SectionIndexMapTy &SectionIndexMap, const RevGroupMapTy &RevGroupMap,
     SectionOffsetsTy &SectionOffsets) {

llvm/test/MC/X86/keep-section-symbols.s

+# RUN: llvm-mc -keep-section-symbols %s -triple x86_64-linux-gnu -filetype=obj -o %t
+# RUN: llvm-readelf -s %t | FileCheck %s --check-prefix=CHECK-KEEP
+
+# RUN: llvm-mc %s -triple x86_64-linux-gnu -filetype=obj -o %t
+# RUN: llvm-readelf -s %t | FileCheck %s --check-prefix=CHECK-DROP
+
+# CHECK-KEEP:     SECTION LOCAL DEFAULT {{[0-9]+}} .text
+# CHECK-DROP-NOT: SECTION LOCAL DEFAULT {{[0-9]+}} .text
+.text
+  nop

llvm/tools/llvm-mc/llvm-mc.cpp

 static cl::opt<bool> LexMotorolaIntegers(
     "motorola-integers",
     cl::desc("Enable binary and hex Motorola integers (%110 and $ABC)"),
     cl::cat(MCCategory));
 
 static cl::opt<bool> NoExecStack("no-exec-stack",
                                  cl::desc("File doesn't need an exec stack"),
                                  cl::cat(MCCategory));
+static cl::opt<bool>
+    KeepSectionSymbols("keep-section-symbols",
+                       cl::desc("Do not discard section symbols"),
+                       cl::cat(MCCategory));
 
 enum ActionType {
   AC_AsLex,
   AC_Assemble,
   AC_Disassemble,
   AC_MDisassemble,
 };
 
int main(int argc, char **argv) {
   std::unique_ptr<MCRegisterInfo> MRI(TheTarget->createMCRegInfo(TripleName));
   assert(MRI && "Unable to create target register info!");
 
   std::unique_ptr<MCAsmInfo> MAI(
       TheTarget->createMCAsmInfo(*MRI, TripleName, MCOptions));
   assert(MAI && "Unable to create target asm info!");
 
   MAI->setRelaxELFRelocations(RelaxELFRel);
+  MAI->setKeepSectionSymbols(KeepSectionSymbols);
 
   if (CompressDebugSections != DebugCompressionType::None) {
     if (!zlib::isAvailable()) {
       WithColor::error(errs(), ProgName)
           << "build tools with zlib to enable -compress-debug-sections";
       return 1;
     }
     MAI->setCompressDebugSections(CompressDebugSections);