Make section attribute and -ffunction-sections play nicely

[probinson]

Summary

People use -ffunction-sections to put each function into its own
object-file section; this makes linker garbage-collection simpler.
However, if there's an explicit attribute((section("name")))
on the function, the programmer is indicating the function has
some special purpose, and is not expected to be garbage collected.

In that case, add the function to the "llvm.used" list, which for ELF
adds SHF_GNU_RETAIN to the section flags, preventing GC.

[MaskRay]

the programmer is indicating the function has some special purpose, and is not expected to be garbage collected.

There are use cases that GC semantics are desired. They may place the address function in a variable.
I don't think changing the behavior by default is fine. Using an option may be fine, but it'd be nicer that GCC commits to do this as well.

[probinson]

I think the GC behavior with explicit section names is currently a little peculiar. For functions without a section name, -ffunction-sections allows GC to happen at the individual function level. With a section name, GC would happen at the level of all-or-nothing per input file, regardless of -ffunction-sections. That just seems unexpected and inconsistent to me.
But it is the current behavior, and given the objections it's not worth pursuing this.

[MaskRay]

The inconsistency stems from the fact that with the default .text, the compiler may generate .text.$suffix if -ffunction-sections is specified or if a COMDAT is needed.
While with an explicit section, there is no such suffix appending mechanism used together with -ffunction-sections. (By default the assembler will combine sections of the same name.)

Therefore, there is a missing feature and perhaps GCC wants to do something with it as well, but just changing the semantics silently may upset some users.

clang/lib/CodeGen/CodeGenModule.cpp

if (D) {
     }
 
     if (auto *F = dyn_cast<llvm::Function>(GO)) {
       if (D->hasAttr<RetainAttr>())
         addUsedGlobal(F);
       if (auto *SA = D->getAttr<PragmaClangTextSectionAttr>())
         if (!D->getAttr<SectionAttr>())
           F->addFnAttr("implicit-section-name", SA->getName());
+      // If we have -ffunction-sections, and an explicit section name,
+      // and the function is not COMDAT, explicitly retain it.
+      if (CodeGenOpts.FunctionSections && D->hasAttr<SectionAttr>() &&
+          !F->getComdat())
+        addUsedGlobal(F);
 
       llvm::AttrBuilder Attrs(F->getContext());
       if (GetCPUAndFeaturesAttributes(GD, Attrs)) {
         // We know that GetCPUAndFeaturesAttributes will always have the
         // newest set, since it has the newest possible FunctionDecl, so the
         // new ones should replace the old.
         llvm::AttributeMask RemoveAttrs;
         RemoveAttrs.addAttribute("target-cpu");

clang/test/CodeGen/section-retain.cpp

+// RUN: %clang_cc1 -triple x86_64-linux -emit-llvm -o - %s | \
+// RUN:     FileCheck %s --check-prefix=UNUSED
+// RUN: %clang_cc1 -triple x86_64-linux -emit-llvm -ffunction-sections -o - %s | \
+// RUN:     FileCheck %s --check-prefix=USED
+
+// template function = comdat always.
+template<typename T>
+__attribute__((section("foo"))) T ftemplate(T a) { return a + 1; }
+__attribute__((section("foo"))) int fglobal(int a) { return ftemplate(a) + 2; }
+
+// UNUSED-NOT: llvm.used
+// USED: @llvm.used = {{.*}} [ptr
+// USED-NOT: ftemplate
+// USED-SAME: fglobal
+// USED-NOT: ftemplate
+// USED-SAME: ]
+
+// USED-DAG: define {{.*}}ftemplate{{.*}} section "foo"
+// USED-DAG: define {{.*}}fglobal{{.*}} section "foo"