[IR][sanitizer] Add module flag "frame-pointer" and set it for cc1 -mframe-pointer={non-leaf,all}

[MaskRay]

Summary

The Linux kernel objtool diagnostic call without frame pointer save/setup
arise in multiple instrumentation passes (asan/tsan/gcov). With the mechanism
introduced in D100251, it's trivial to respect the command line
-m[no-]omit-leaf-frame-pointer/-f[no-]omit-frame-pointer, so let's do it.

Fix: https://github.com/ClangBuiltLinux/linux/issues/1236 (tsan)
Fix: https://github.com/ClangBuiltLinux/linux/issues/1238 (asan)

[nickdesaulniers]

Nice idea to create Function::createWithDefaultAttr then reuse for multiple purposes!

llvm/lib/IR/Function.cpp

333

we should call this from gcov too to solve https://github.com/ClangBuiltLinux/linux/issues/955.

[vitalybuka]

llvm/include/llvm/IR/Module.h

896

It should be enum, similar to PIELevel.

And probably needs IR documentation update?

[vitalybuka]

llvm/lib/IR/Function.cpp

346

Should these be added into Attribute:: ?

[MaskRay]

use enum.
add doc.

[MaskRay]

llvm/lib/IR/Function.cpp

333

Exactly:) Planned to do it after this patch https://github.com/ClangBuiltLinux/linux/issues/1238#issuecomment-824517092

346

The function attribute "frame-pointer" is already used, with a string value. A regular attribute cannot have a string value so it was created as the string form...

The documentation is long overdue, though. I have added a bit documentation.

[MaskRay]

llvm/docs/LangRef.rst

7019

I'll add:

These metadata correspond to a few function attributes with significant code generation behaviors. Function attributes with just optimization purposes should not be listed because the performance impact of these synthesized functions is small.

[ychen]

LGTM

[MaskRay]

Chatted offline, vitalybuka said I can go ahead with existing ack.

[MaskRay]

improve doc

clang/lib/CodeGen/CodeGenModule.cpp

if (getCodeGenOpts().CodeModel.size() > 0) {
     }
   }
 
   if (CodeGenOpts.NoPLT)
     getModule().setRtLibUseGOT();
   if (CodeGenOpts.UnwindTables)
     getModule().setUwtable();
 
+  switch (CodeGenOpts.getFramePointer()) {
+  case CodeGenOptions::FramePointerKind::None:
+    // 0 ("none") is the default.
+    break;
+  case CodeGenOptions::FramePointerKind::NonLeaf:
+    getModule().setFramePointer(llvm::FramePointerKind::NonLeaf);
+    break;
+  case CodeGenOptions::FramePointerKind::All:
+    getModule().setFramePointer(llvm::FramePointerKind::All);
+    break;
+  }
+
   SimplifyPersonality();
 
   if (getCodeGenOpts().EmitDeclMetadata)
     EmitDeclMetadata();
 
   if (getCodeGenOpts().EmitGcovArcs || getCodeGenOpts().EmitGcovNotes)
     EmitCoverageFile();
 

clang/test/CodeGen/asan-frame-pointer.cpp

+/// -mframe-pointer=none sets the module flag "frame-pointer" (merge behavior: max).
+/// asan synthesized ctor/dtor get the "frame-pointer" function attribute if not zero (default).
+// RUN: %clang_cc1 -emit-llvm -fsanitize=address -mframe-pointer=none %s -o - | FileCheck %s --check-prefix=NONE
+// RUN: %clang_cc1 -emit-llvm -fsanitize=address -mframe-pointer=non-leaf %s -o - | FileCheck %s --check-prefix=NONLEAF
+// RUN: %clang_cc1 -emit-llvm -fsanitize=address -mframe-pointer=all %s -o - | FileCheck %s --check-prefix=ALL
+
+int global;
+
+// NONE: define internal void @asan.module_ctor() #[[#ATTR:]] {
+// NONE: define internal void @asan.module_dtor() #[[#ATTR]] {
+// NONE: attributes #[[#ATTR]] = { nounwind }
+
+// NONLEAF: define internal void @asan.module_ctor() #[[#ATTR:]] {
+// NONLEAF: define internal void @asan.module_dtor() #[[#ATTR]] {
+// NONLEAF: attributes #[[#ATTR]] = { nounwind "frame-pointer"="non-leaf" }
+
+// ALL: define internal void @asan.module_ctor() #[[#ATTR:]] {
+// ALL: define internal void @asan.module_dtor() #[[#ATTR]] {
+// ALL: attributes #[[#ATTR]] = { nounwind "frame-pointer"="all" }

llvm/docs/LangRef.rst

``convergent``
     that we should treat the call as though we're calling a convergent
     function.  This is particularly useful on indirect calls; without this we
     may treat such calls as though the target is non-convergent.
 
     The optimizer may remove the ``convergent`` attribute on functions when it
     can prove that the function does not execute any convergent operations.
     Similarly, the optimizer may remove ``convergent`` on calls/invokes when it
     can prove that the call/invoke cannot call a convergent function.
+``"frame-pointer"``
+    This attribute tells the code generator whether the function
+    should keep the frame pointer. The code generator may emit the frame pointer
+    even if this attribute says the frame pointer can be eliminated.
+    The allowed string values are:
+
+     * ``"none"`` (default) - the frame pointer can be eliminated.
+     * ``"non-leaf"`` - the frame pointer should be kept if the function calls
+       other functions.
+     * ``"all"`` - the frame pointer should be kept.
 ``hot``
     This attribute indicates that this function is a hot spot of the program
     execution. The function will be optimized more aggressively and will be
     placed into special subsection of the text section to improving locality.
 
     When profile feedback is enabled, this attribute has the precedence over
     the profile information. By marking a function ``hot``, users can work
     around the cases where the training input does not have good coverage
-  Metadata ``!3`` has the ID ``!"qux"`` and the value:
    ::
 
        !{ !"foo", i32 1 }
 
    The behavior is to emit an error if the ``llvm.module.flags`` does not
    contain a flag with the ID ``!"foo"`` that has the value '1' after linking is
    performed.
 
+Synthesized Functions Module Flags Metadata
+-------------------------------------------
+
+These metadata specify the default attributes synthesized functions should have.
+These metadata are currently respected by a few instrumentation passes, such as
+sanitizers.
+
+These metadata correspond to a few function attributes with significant code
+generation behaviors. Function attributes with just optimization purposes
+should not be listed because the performance impact of these synthesized
+functions is small.
+
+- "frame-pointer": **Max**. The value can be 0, 1, or 2. A synthesized function
+  will get the "frame-pointer" function attribute, with value being "none",
+  "non-leaf", or "all", respectively.
+- "uwtable": **Max**. The value can be 0 or 1. If the value is 1, a synthesized
+  function will get the ``uwtable`` function attribute.
+
 Objective-C Garbage Collection Module Flags Metadata
 ----------------------------------------------------
 
 On the Mach-O platform, Objective-C stores metadata about garbage
 collection in a special section called "image info". The metadata
 consists of a version number and a bitmask specifying what types of
 garbage collection are supported (if any) by the file. If two or more
 modules are linked together their garbage collection metadata needs to

llvm/include/llvm/CodeGen/CommandFlags.h

 
 llvm::ExceptionHandling getExceptionModel();
 
 CodeGenFileType getFileType();
 Optional<CodeGenFileType> getExplicitFileType();
 
 CodeGenFileType getFileType();
 
-llvm::FramePointer::FP getFramePointerUsage();
+FramePointerKind getFramePointerUsage();
 
 bool getEnableUnsafeFPMath();
 
 bool getEnableNoInfsFPMath();
 
 bool getEnableNoNaNsFPMath();
 
 bool getEnableNoSignedZerosFPMath();

llvm/include/llvm/IR/Module.h

/// @}
 
   /// Set that PLT should be avoid for RTLib calls.
   void setRtLibUseGOT();
 
   /// Get/set whether synthesized functions should get the uwtable attribute.
   bool getUwtable() const;
   void setUwtable();
 
+  /// Get/set whether synthesized functions should get the "frame-pointer"
+  /// attribute.
+  FramePointerKind getFramePointer() const;
+  void setFramePointer(FramePointerKind Kind);
+
   /// @name Utility functions for querying and setting the build SDK version
   /// @{
 
   /// Attach a build SDK version metadata to this module.
   void setSDKVersion(const VersionTuple &V);
 
   /// Get the build SDK version metadata.
   ///

llvm/include/llvm/Support/CodeGen.h

namespace llvm {
   /// what type of file to emit, and returned by it to indicate what type of
   /// file could actually be made.
   enum CodeGenFileType {
     CGFT_AssemblyFile,
     CGFT_ObjectFile,
     CGFT_Null         // Do not emit any output.
   };
 
-  // Specify effect of frame pointer elimination optimization.
-  namespace FramePointer {
-    enum FP {All, NonLeaf, None};
-  }
+  // Specify what functions should keep the frame pointer.
+  enum class FramePointerKind { None, NonLeaf, All };
 
 }  // end llvm namespace
 
 #endif

llvm/lib/CodeGen/CommandFlags.cpp

 CGOPT(std::string, MArch)
 CGOPT(std::string, MCPU)
 CGLIST(std::string, MAttrs)
 CGOPT_EXP(Reloc::Model, RelocModel)
 CGOPT(ThreadModel::Model, ThreadModel)
 CGOPT_EXP(CodeModel::Model, CodeModel)
 CGOPT(ExceptionHandling, ExceptionModel)
 CGOPT_EXP(CodeGenFileType, FileType)
-CGOPT(FramePointer::FP, FramePointerUsage)
+CGOPT(FramePointerKind, FramePointerUsage)
 CGOPT(bool, EnableUnsafeFPMath)
 CGOPT(bool, EnableNoInfsFPMath)
 CGOPT(bool, EnableNoNaNsFPMath)
 CGOPT(bool, EnableNoSignedZerosFPMath)
 CGOPT(bool, EnableNoTrappingFPMath)
 CGOPT(bool, EnableAIXExtendedAltivecABI)
 CGOPT(DenormalMode::DenormalModeKind, DenormalFPMath)
 CGOPT(DenormalMode::DenormalModeKind, DenormalFP32Math)
static cl::opt<CodeGenFileType> FileType(
       cl::values(
           clEnumValN(CGFT_AssemblyFile, "asm", "Emit an assembly ('.s') file"),
           clEnumValN(CGFT_ObjectFile, "obj",
                      "Emit a native object ('.o') file"),
           clEnumValN(CGFT_Null, "null",
                      "Emit nothing, for performance testing")));
   CGBINDOPT(FileType);
 
-  static cl::opt<FramePointer::FP> FramePointerUsage(
+  static cl::opt<FramePointerKind> FramePointerUsage(
       "frame-pointer",
       cl::desc("Specify frame pointer elimination optimization"),
-      cl::init(FramePointer::None),
+      cl::init(FramePointerKind::None),
       cl::values(
-          clEnumValN(FramePointer::All, "all",
+          clEnumValN(FramePointerKind::All, "all",
                      "Disable frame pointer elimination"),
-          clEnumValN(FramePointer::NonLeaf, "non-leaf",
+          clEnumValN(FramePointerKind::NonLeaf, "non-leaf",
                      "Disable frame pointer elimination for non-leaf frame"),
-          clEnumValN(FramePointer::None, "none",
+          clEnumValN(FramePointerKind::None, "none",
                      "Enable frame pointer elimination")));
   CGBINDOPT(FramePointerUsage);
 
   static cl::opt<bool> EnableUnsafeFPMath(
       "enable-unsafe-fp-math",
       cl::desc("Enable optimizations that may decrease FP precision"),
       cl::init(false));
   CGBINDOPT(EnableUnsafeFPMath);
else {
       SmallString<256> Appended(OldFeatures);
       Appended.push_back(',');
       Appended.append(Features);
       NewAttrs.addAttribute("target-features", Appended);
     }
   }
   if (FramePointerUsageView->getNumOccurrences() > 0 &&
       !F.hasFnAttribute("frame-pointer")) {
-    if (getFramePointerUsage() == FramePointer::All)
+    if (getFramePointerUsage() == FramePointerKind::All)
       NewAttrs.addAttribute("frame-pointer", "all");
-    else if (getFramePointerUsage() == FramePointer::NonLeaf)
+    else if (getFramePointerUsage() == FramePointerKind::NonLeaf)
       NewAttrs.addAttribute("frame-pointer", "non-leaf");
-    else if (getFramePointerUsage() == FramePointer::None)
+    else if (getFramePointerUsage() == FramePointerKind::None)
       NewAttrs.addAttribute("frame-pointer", "none");
   }
   if (DisableTailCallsView->getNumOccurrences() > 0)
     NewAttrs.addAttribute("disable-tail-calls",
                           toStringRef(getDisableTailCalls()));
   if (getStackRealign())
     NewAttrs.addAttribute("stackrealign");
 

llvm/lib/IR/Function.cpp

unsigned Function::getInstructionCount() const {
   return NumInstrs;
 }
 
 Function *Function::Create(FunctionType *Ty, LinkageTypes Linkage,
                            const Twine &N, Module &M) {
   return Create(Ty, Linkage, M.getDataLayout().getProgramAddressSpace(), N, &M);
 }
 
 Function *Function::createWithDefaultAttr(FunctionType *Ty,
                                           LinkageTypes Linkage,
                                           unsigned AddrSpace, const Twine &N,
                                           Module *M) {
   auto *F = new Function(Ty, Linkage, AddrSpace, N, M);
+  AttrBuilder B;
   if (M->getUwtable())
-    F->addAttribute(AttributeList::FunctionIndex, Attribute::UWTable);
+    B.addAttribute(Attribute::UWTable);
+  switch (M->getFramePointer()) {
+  case FramePointerKind::None:
+    // 0 ("none") is the default.
+    break;
+  case FramePointerKind::NonLeaf:
+    B.addAttribute("frame-pointer", "non-leaf");
+    break;
+  case FramePointerKind::All:
+    B.addAttribute("frame-pointer", "all");
+    break;
+  }
+  F->addAttributes(AttributeList::FunctionIndex, B);
   return F;
 }
 
 void Function::removeFromParent() {
   getParent()->getFunctionList().remove(getIterator());
 }
 
 void Function::eraseFromParent() {

llvm/lib/IR/Module.cpp

 
 bool Module::getUwtable() const {
   auto *Val = cast_or_null<ConstantAsMetadata>(getModuleFlag("uwtable"));
   return Val && (cast<ConstantInt>(Val->getValue())->getZExtValue() > 0);
 }
 
 void Module::setUwtable() { addModuleFlag(ModFlagBehavior::Max, "uwtable", 1); }
 
+FramePointerKind Module::getFramePointer() const {
+  auto *Val = cast_or_null<ConstantAsMetadata>(getModuleFlag("frame-pointer"));
+  return static_cast<FramePointerKind>(
+      Val ? cast<ConstantInt>(Val->getValue())->getZExtValue() : 0);
+}
+
+void Module::setFramePointer(FramePointerKind Kind) {
+  addModuleFlag(ModFlagBehavior::Max, "frame-pointer", static_cast<int>(Kind));
+}
+
 void Module::setSDKVersion(const VersionTuple &V) {
   SmallVector<unsigned, 3> Entries;
   Entries.push_back(V.getMajor());
   if (auto Minor = V.getMinor()) {
     Entries.push_back(*Minor);
     if (auto Subminor = V.getSubminor())
       Entries.push_back(*Subminor);
     // Ignore the 'build' component as it can't be represented in the object

llvm/test/Instrumentation/AddressSanitizer/module-flags.ll

 ; RUN: opt < %s -passes='asan-pipeline' -S | FileCheck %s
 
 target triple = "x86_64-unknown-linux-gnu"
 
 @g = dso_local global i32 0, align 4
 
 define i32 @test_load() sanitize_address {
 entry:
   %tmp = load i32, i32* @g, align 4
   ret i32 %tmp
 }
 
-!llvm.module.flags = !{!0}
+!llvm.module.flags = !{!0, !1}
 
 ;; Due to -fasynchronous-unwind-tables.
 !0 = !{i32 7, !"uwtable", i32 1}
 
+;; Due to -fno-omit-frame-pointer.
+!1 = !{i32 7, !"frame-pointer", i32 2}
+
 ;; Set the uwtable attribute on ctor/dtor.
 ; CHECK: define internal void @asan.module_ctor() #[[#ATTR:]]
 ; CHECK: define internal void @asan.module_dtor() #[[#ATTR]]
-; CHECK: attributes #[[#ATTR]] = { nounwind uwtable }
+; CHECK: attributes #[[#ATTR]] = { nounwind uwtable "frame-pointer"="all" }