[MC][ELF] Prevent globals with an explicit section from being mergeable by default and add a safe option to allow mergeable

[bd1976llvm]

Summary

Alternative to https://reviews.llvm.org/D68101

Prevents globals with an explicit section from being mergeable by default (as in D68101).
Adds a command line option to allow the explicit section to be mergeable; but, an error will be emitted if broken output would be created.
Users can use the command line option if the default behaviour results in a performance regression.

This is a partial fix for https://bugs.llvm.org/show_bug.cgi?id=43457.

clang/include/clang/Basic/CodeGenOptions.def

 CODEGENOPT(CoverageExtraChecksum, 1, 0) ///< Whether we need a second checksum for functions in GCNO files.
 CODEGENOPT(CoverageNoFunctionNamesInData, 1, 0) ///< Do not include function names in GCDA files.
 CODEGENOPT(CoverageExitBlockBeforeBody, 1, 0) ///< Whether to emit the exit block before the body blocks in GCNO files.
 CODEGENOPT(CXAAtExit         , 1, 1) ///< Use __cxa_atexit for calling destructors.
 CODEGENOPT(RegisterGlobalDtorsWithAtExit, 1, 1) ///< Use atexit or __cxa_atexit to register global destructors.
 CODEGENOPT(CXXCtorDtorAliases, 1, 0) ///< Emit complete ctors/dtors as linker
                                      ///< aliases to base ctors when possible.
 CODEGENOPT(DataSections      , 1, 0) ///< Set when -fdata-sections is enabled.
+CODEGENOPT(MergeableExplicitSections, 1, 0) ///< Set when -fmergeable-explict-sections is enabled.
 CODEGENOPT(UniqueSectionNames, 1, 1) ///< Set for -funique-section-names.
 ENUM_CODEGENOPT(FramePointer, FramePointerKind, 2, FramePointerKind::None) /// frame-pointer: all,non-leaf,none
 
 CODEGENOPT(DisableFree       , 1, 0) ///< Don't free memory.
 CODEGENOPT(DiscardValueNames , 1, 0) ///< Discard Value Names from the IR (LLVMContext flag)
 CODEGENOPT(DisableGCov       , 1, 0) ///< Don't run the GCov pass, for testing.
 CODEGENOPT(DisableLLVMPasses , 1, 0) ///< Don't run any LLVM IR passes to get
                                      ///< the pristine IR generated by the

clang/include/clang/Driver/Options.td

def ffunction_sections : Flag<["-"], "ffunction-sections">, Group<f_Group>,
   Flags<[CC1Option]>,
   HelpText<"Place each function in its own section (ELF Only)">;
 def fno_function_sections : Flag<["-"], "fno-function-sections">,
   Group<f_Group>, Flags<[CC1Option]>;
 def fdata_sections : Flag <["-"], "fdata-sections">, Group<f_Group>,
  Flags<[CC1Option]>, HelpText<"Place each data in its own section (ELF Only)">;
 def fno_data_sections : Flag <["-"], "fno-data-sections">, Group<f_Group>,
   Flags<[CC1Option]>;
+def fmergeable_explicit_sections : Flag <["-"], "fmergeable-explicit-sections">, Group<f_Group>,
+ Flags<[CC1Option]>, HelpText<"Attempts mark explicit sections as mergeable. May result in an error if incompatible symbols are assigned to a section.">;
+def fno_mergeable_explicit_sections : Flag <["-"], "fno-mergeable-explicit-sections">, Group<f_Group>,
+  Flags<[CC1Option]>;
 def fstack_size_section : Flag<["-"], "fstack-size-section">, Group<f_Group>, Flags<[CC1Option]>,
   HelpText<"Emit section containing metadata on function stack sizes">;
 def fno_stack_size_section : Flag<["-"], "fno-stack-size-section">, Group<f_Group>, Flags<[CC1Option]>,
   HelpText<"Don't emit section containing metadata on function stack sizes">;
 
 def funique_section_names : Flag <["-"], "funique-section-names">,
   Group<f_Group>, Flags<[CC1Option]>,
   HelpText<"Use unique names for text and data sections (ELF Only)">;

clang/lib/CodeGen/BackendUtil.cpp

static void initTargetOptions(llvm::TargetOptions &Options,
 
   Options.NoInfsFPMath = CodeGenOpts.NoInfsFPMath;
   Options.NoNaNsFPMath = CodeGenOpts.NoNaNsFPMath;
   Options.NoZerosInBSS = CodeGenOpts.NoZeroInitializedInBSS;
   Options.UnsafeFPMath = CodeGenOpts.UnsafeFPMath;
   Options.StackAlignmentOverride = CodeGenOpts.StackAlignment;
   Options.FunctionSections = CodeGenOpts.FunctionSections;
   Options.DataSections = CodeGenOpts.DataSections;
+  Options.MergeableExplicitSections = CodeGenOpts.MergeableExplicitSections;
   Options.UniqueSectionNames = CodeGenOpts.UniqueSectionNames;
   Options.EmulatedTLS = CodeGenOpts.EmulatedTLS;
   Options.ExplicitEmulatedTLS = CodeGenOpts.ExplicitEmulatedTLS;
   Options.DebuggerTuning = CodeGenOpts.getDebuggerTuning();
   Options.EmitStackSizeSection = CodeGenOpts.StackSizeSection;
   Options.EmitAddrsig = CodeGenOpts.Addrsig;
   Options.EnableDebugEntryValues = CodeGenOpts.EnableDebugEntryValues;
 

clang/lib/Driver/ToolChains/Clang.cpp

if (Args.hasFlag(options::OPT_ffunction_sections,
     CmdArgs.push_back("-ffunction-sections");
   }
 
   if (Args.hasFlag(options::OPT_fdata_sections, options::OPT_fno_data_sections,
                    UseSeparateSections)) {
     CmdArgs.push_back("-fdata-sections");
   }
 
+  if (Args.hasFlag(options::OPT_fmergeable_explicit_sections, options::OPT_fno_mergeable_explicit_sections,
+                   false)) {
+    CmdArgs.push_back("-fmergeable-explicit-sections");
+  }
+
   if (!Args.hasFlag(options::OPT_funique_section_names,
                     options::OPT_fno_unique_section_names, true))
     CmdArgs.push_back("-fno-unique-section-names");
 
   Args.AddLastArg(CmdArgs, options::OPT_finstrument_functions,
                   options::OPT_finstrument_functions_after_inlining,
                   options::OPT_finstrument_function_entry_bare);
 

clang/lib/Driver/ToolChains/CommonArgs.cpp

if (Args.hasFlag(options::OPT_ffunction_sections,
     CmdArgs.push_back("-plugin-opt=-function-sections");
   }
 
   if (Args.hasFlag(options::OPT_fdata_sections, options::OPT_fno_data_sections,
                    UseSeparateSections)) {
     CmdArgs.push_back("-plugin-opt=-data-sections");
   }
 
+  if (Args.hasFlag(options::OPT_fmergeable_explicit_sections, options::OPT_fno_mergeable_explicit_sections,
+                   false)) {
+    CmdArgs.push_back("-plugin-opt=-mergeable-explicit-sections");
+  }
+
   if (Arg *A = getLastProfileSampleUseArg(Args)) {
     StringRef FName = A->getValue();
     if (!llvm::sys::fs::exists(FName))
       ToolChain.getDriver().Diag(diag::err_drv_no_such_file) << FName;
     else
       CmdArgs.push_back(
           Args.MakeArgString(Twine("-plugin-opt=sample-profile=") + FName));
   }

clang/lib/Frontend/CompilerInvocation.cpp

Diags.Report(diag::err_drv_invalid_value)
         << Opts.ThreadModel;
   Opts.TrapFuncName = Args.getLastArgValue(OPT_ftrap_function_EQ);
   Opts.UseInitArray = Args.hasArg(OPT_fuse_init_array);
 
   Opts.FunctionSections = Args.hasFlag(OPT_ffunction_sections,
                                        OPT_fno_function_sections, false);
   Opts.DataSections = Args.hasFlag(OPT_fdata_sections,
                                    OPT_fno_data_sections, false);
+  Opts.MergeableExplicitSections = Args.hasFlag(
+      OPT_fmergeable_explicit_sections, OPT_fmergeable_explicit_sections, false);
   Opts.StackSizeSection =
       Args.hasFlag(OPT_fstack_size_section, OPT_fno_stack_size_section, false);
   Opts.UniqueSectionNames = Args.hasFlag(OPT_funique_section_names,
                                          OPT_fno_unique_section_names, true);
 
   Opts.MergeFunctions = Args.hasArg(OPT_fmerge_functions);
 
   Opts.NoUseJumpTables = Args.hasArg(OPT_fno_jump_tables);

llvm/include/llvm/CodeGen/CommandFlags.inc

static cl::opt<bool> RelaxELFRelocations(
     "relax-elf-relocations",
     cl::desc("Emit GOTPCRELX/REX_GOTPCRELX instead of GOTPCREL on x86-64 ELF"),
     cl::init(false));
 
 static cl::opt<bool> DataSections("data-sections",
                                   cl::desc("Emit data into separate sections"),
                                   cl::init(false));
 
+static cl::opt<bool> MergeableExplicitSections ("mergeable-explicit-sections",
+                                  cl::desc("Attempts mark explicit sections as mergeable. May result in an error if incompatible symbols are assigned to a section."),
+                                  cl::init(false));
+
 static cl::opt<bool>
     FunctionSections("function-sections",
                      cl::desc("Emit functions into separate sections"),
                      cl::init(false));
 
 static cl::opt<bool> EmulatedTLS("emulated-tls",
                                  cl::desc("Use emulated TLS model"),
                                  cl::init(false));
if (FloatABIForCalls != FloatABI::Default)
     Options.FloatABIType = FloatABIForCalls;
   Options.NoZerosInBSS = DontPlaceZerosInBSS;
   Options.GuaranteedTailCallOpt = EnableGuaranteedTailCallOpt;
   Options.StackAlignmentOverride = OverrideStackAlignment;
   Options.StackSymbolOrdering = StackSymbolOrdering;
   Options.UseInitArray = !UseCtors;
   Options.RelaxELFRelocations = RelaxELFRelocations;
   Options.DataSections = DataSections;
+  Options.MergeableExplicitSections = MergeableExplicitSections;
   Options.FunctionSections = FunctionSections;
   Options.UniqueSectionNames = UniqueSectionNames;
   Options.EmulatedTLS = EmulatedTLS;
   Options.ExplicitEmulatedTLS = EmulatedTLS.getNumOccurrences() > 0;
   Options.ExceptionModel = ExceptionModel;
   Options.EmitStackSizeSection = EnableStackSizeSection;
   Options.EmitAddrsig = EnableAddrsig;
   Options.EnableDebugEntryValues = EnableDebugEntryValues;

llvm/include/llvm/Target/TargetMachine.h

public:
   }
 
   /// Return true if functions should be emitted into their own section,
   /// corresponding to -ffunction-sections.
   bool getFunctionSections() const {
     return Options.FunctionSections;
   }
 
+  bool getMergeableExplicitSections() const {
+    return Options.MergeableExplicitSections;
+  }
+
   /// Get a \c TargetIRAnalysis appropriate for the target.
   ///
   /// This is used to construct the new pass manager's target IR analysis pass,
   /// set up appropriately for this target machine. Even the old pass manager
   /// uses this to answer queries about the IR.
   TargetIRAnalysis getTargetIRAnalysis();
 
   /// Return a TargetTransformInfo for a given function.

llvm/include/llvm/Target/TargetOptions.h

TargetOptions()
         : PrintMachineCode(false), UnsafeFPMath(false), NoInfsFPMath(false),
           NoNaNsFPMath(false), NoTrappingFPMath(false),
           NoSignedZerosFPMath(false),
           HonorSignDependentRoundingFPMathOption(false), NoZerosInBSS(false),
           GuaranteedTailCallOpt(false), StackSymbolOrdering(true),
           EnableFastISel(false), EnableGlobalISel(false), UseInitArray(false),
           DisableIntegratedAS(false), RelaxELFRelocations(false),
           FunctionSections(false), DataSections(false),
+          MergeableExplicitSections(false),
           UniqueSectionNames(true), TrapUnreachable(false),
           NoTrapAfterNoreturn(false), EmulatedTLS(false),
           ExplicitEmulatedTLS(false), EnableIPRA(false),
           EmitStackSizeSection(false), EnableMachineOutliner(false),
           SupportsDefaultOutlining(false), EmitAddrsig(false),
           EnableDebugEntryValues(false) {}
 
     /// PrintMachineCode - This flag is enabled when the -print-machineinstrs
public:
     unsigned RelaxELFRelocations : 1;
 
     /// Emit functions into separate sections.
     unsigned FunctionSections : 1;
 
     /// Emit data into separate sections.
     unsigned DataSections : 1;
 
+    /// Emit globals with explicit sections into mergeable sections; May result
+    /// in an error if incompatible symbols are assigned to a section.
+    unsigned MergeableExplicitSections : 1;
+
     unsigned UniqueSectionNames : 1;
 
     /// Emit target-specific trap instruction for 'unreachable' IR instructions.
     unsigned TrapUnreachable : 1;
 
     /// Do not emit a trap instruction for 'unreachable' IR instructions behind
     /// noreturn calls, even if TrapUnreachable is true.
     unsigned NoTrapAfterNoreturn : 1;

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

MCSection *TargetLoweringObjectFileELF::getExplicitSectionGlobal(
 
   MCSectionELF *Section = getContext().getELFSection(
       SectionName, getELFSectionType(SectionName, Kind), Flags,
       getEntrySizeForKind(Kind), Group, UniqueID, AssociatedSymbol);
   // Make sure that we did not get some other section with incompatible sh_link.
   // This should not be possible due to UniqueID code above.
   assert(Section->getAssociatedSymbol() == AssociatedSymbol &&
          "Associated symbol mismatch between sections");
+
+  // If two globals with differing sizes end up in the same mergeable section
+  // that section can be assigned an incorrect entry size. Error rather than
+  // creating broken output.
+  if ((Section->getEntrySize() != 0) &&
+      (Section->getEntrySize() != getEntrySizeForKind(Kind)))
+    report_fatal_error(
+        "Symbol '" + GO->getName() + "' from module '" +
+        (GO->getParent() ? GO->getParent()->getSourceFileName() : "unknown") +
+        "' required a section with entry-size=" +
+        Twine(getEntrySizeForKind(Kind)) + " but was placed in section '" +
+        SectionName + "' with entry-size=" + Twine(Section->getEntrySize()) +
+        ": Explicit assignment by pragma or attribute of incompatible symbols "
+        "to a section?");
+
   return Section;
 }
 
 /// Return the section prefix name used by options FunctionsSections and
 /// DataSections.
 static StringRef getSectionPrefixForGlobal(SectionKind Kind) {
   if (Kind.isText())
     return ".text";

llvm/lib/Target/TargetLoweringObjectFile.cpp

MCSymbol *TargetLoweringObjectFile::getCFIPersonalitySymbol(
   return TM.getSymbol(GV);
 }
 
 void TargetLoweringObjectFile::emitPersonalityValue(MCStreamer &Streamer,
                                                     const DataLayout &,
                                                     const MCSymbol *Sym) const {
 }
 
+static bool inExplicitSection(const GlobalObject *GO, SectionKind Kind) {
+  if (GO->hasSection())
+    return true;
+
+  if (auto *GVar = dyn_cast<GlobalVariable>(GO)) {
+    auto Attrs = GVar->getAttributes();
+    if ((Attrs.hasAttribute("bss-section") && Kind.isBSS()) ||
+        (Attrs.hasAttribute("data-section") && Kind.isData()) ||
+        (Attrs.hasAttribute("rodata-section") && Kind.isReadOnly())) {
+      return true;
+    }
+  }
+
+  if (auto *F = dyn_cast<Function>(GO)) {
+    if (F->hasFnAttribute("implicit-section-name"))
+      return true;
+  }
+
+  return false;
+}
 
 /// getKindForGlobal - This is a top-level target-independent classifier for
 /// a global object.  Given a global variable and information from the TM, this
 /// function classifies the global in a target independent manner. This function
 /// may be overridden by the target implementation.
 SectionKind TargetLoweringObjectFile::getKindForGlobal(const GlobalObject *GO,
                                                        const TargetMachine &TM){
   assert(!GO->isDeclaration() && !GO->hasAvailableExternallyLinkage() &&
if (GVar->isConstant()) {
     const Constant *C = GVar->getInitializer();
     if (!C->needsRelocation()) {
       // If the global is required to have a unique address, it can't be put
       // into a mergable section: just drop it into the general read-only
       // section instead.
       if (!GVar->hasGlobalUnnamedAddr())
         return SectionKind::getReadOnly();
 
+      // If two globals with differing sizes end up in the same mergeable section that
+      // section can be assigned an incorrect entry size. To avoid this we do not put
+      // globals into a mergeable section if they have been explicitly assigned to a section.
+      // TODO: This is a pessimistic solution as we lose the benifits of mergeable sections. A
+      //       better solution might be to bin globals of the same size into different mergeable
+      //       sections with the same name.
+      if (inExplicitSection(GO, SectionKind::getReadOnly()) &&
+          !TM.getMergeableExplicitSections())
+        return SectionKind::getReadOnly();
+
       // If initializer is a null-terminated string, put it in a "cstring"
       // section of the right width.
       if (ArrayType *ATy = dyn_cast<ArrayType>(C->getType())) {
         if (IntegerType *ITy =
               dyn_cast<IntegerType>(ATy->getElementType())) {
           if ((ITy->getBitWidth() == 8 || ITy->getBitWidth() == 16 ||
                ITy->getBitWidth() == 32) &&
               IsNullTerminatedString(C)) {
SectionKind TargetLoweringObjectFile::getKindForGlobal(const GlobalObject *GO,
   return SectionKind::getData();
 }
 
 /// This method computes the appropriate section to emit the specified global
 /// variable or function definition.  This should not be passed external (or
 /// available externally) globals.
 MCSection *TargetLoweringObjectFile::SectionForGlobal(
     const GlobalObject *GO, SectionKind Kind, const TargetMachine &TM) const {
-  // Select section name.
-  if (GO->hasSection())
-    return getExplicitSectionGlobal(GO, Kind, TM);
-
-  if (auto *GVar = dyn_cast<GlobalVariable>(GO)) {
-    auto Attrs = GVar->getAttributes();
-    if ((Attrs.hasAttribute("bss-section") && Kind.isBSS()) ||
-        (Attrs.hasAttribute("data-section") && Kind.isData()) ||
-        (Attrs.hasAttribute("rodata-section") && Kind.isReadOnly()))  {
-       return getExplicitSectionGlobal(GO, Kind, TM);
-    }
-  }
-
-  if (auto *F = dyn_cast<Function>(GO)) {
-    if (F->hasFnAttribute("implicit-section-name"))
-      return getExplicitSectionGlobal(GO, Kind, TM);
-  }
-
+  if (inExplicitSection(GO, Kind))
   // Use default section depending on the 'type' of global
   return SelectSectionForGlobal(GO, Kind, TM);
 }
 
 MCSection *TargetLoweringObjectFile::getSectionForJumpTable(
     const Function &F, const TargetMachine &TM) const {
   unsigned Align = 0;
   return getSectionForConstant(F.getParent()->getDataLayout(),

llvm/test/CodeGen/X86/explict-section-mergeable.ll

+; RUN: llc < %s -mtriple=x86_64-linux-gnu | FileCheck %s --check-prefix=DEFAULT
+
+;; show that the implicitly generated sections are as expected
+@implicit1 = unnamed_addr constant [2 x i16] [i16 1, i16 1]
+; DEFAULT-CHECK: .section .rodata.cst4,"aM",@progbits,4
+@implicit2 = unnamed_addr constant [2 x i32] [i32 1, i32 1]
+; DEFAULT-CHECK: .section .rodata.cst8,"aM",@progbits,8
+@implicit3 = unnamed_addr constant [2 x i16] [i16 1, i16 0]
+; DEFAULT-CHECK: .section .rodata.str2.2,"aMS",@progbits,2
+
+;; Check that an explicitly created section is not SHF_MERGE and that
+;; it contains all three symbols.
+@explicit1 = unnamed_addr constant [2 x i16] [i16 1, i16 1] "rodata-section"=".explicit"
+@explicit2 = unnamed_addr constant [2 x i32] [i32 1, i32 1] "rodata-section"=".explicit"
+@explicit3 = unnamed_addr constant [2 x i16] [i16 1, i16 0] "rodata-section"=".explicit"
+; DEFAULT: .section .explicit,"a",@progbits
+; DEFAULT-NOT: .section
+; DEFAULT: explicit1:
+; DEFAULT-NOT: .section
+; DEFAULT: explicit2:
+; DEFAULT-NOT: .section
+; DEFAULT: explicit3:
+
+; RUN: not llc < %s -mtriple=x86_64-linux-gnu -mergeable-explicit-sections 2>&1 | FileCheck %s --check-prefix=MERGE
+
+; MERGE: Symbol 'explicit2' from module '<stdin>' required a section with entry-size=8 but was placed in section '.explicit' with entry-size=4: Explicit assignment by pragma or attribute of incompatible symbols to a section?
+

llvm/test/CodeGen/X86/section_mergeable_size.ll

-; RUN: llc -mtriple x86_64-linux-gnu < %s | FileCheck %s
+; RUN: llc -mtriple x86_64-linux-gnu -mergeable-explicit-sections < %s | FileCheck %s
 @a = internal unnamed_addr constant [1 x [1 x i32]] zeroinitializer, section ".init.rodata", align 4
 ; CHECK: .init.rodata,"aM",{{[@%]}}progbits,4