[llvm] e01c666 - Revert D76519 "[NFC] Refactor how CFI section types are represented in AsmPrinter"

Fangrui Song via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 26 15:17:32 PDT 2021


Author: Fangrui Song
Date: 2021-04-26T15:17:28-07:00
New Revision: e01c666b136e3f97587b22a2029f31e5c36a0e71

URL: https://github.com/llvm/llvm-project/commit/e01c666b136e3f97587b22a2029f31e5c36a0e71
DIFF: https://github.com/llvm/llvm-project/commit/e01c666b136e3f97587b22a2029f31e5c36a0e71.diff

LOG: Revert D76519 "[NFC] Refactor how CFI section types are represented in AsmPrinter"

This reverts commit 0ce723cb228bc1d1a0f5718f3862fb836145a333.

D76519 was not quite NFC. If we see a CFISection::Debug function before a
CFISection::EH one (-fexceptions -fno-asynchronous-unwind-tables), we may
incorrectly pick CFISection::Debug and emit a `.cfi_sections .debug_frame`.
We should use .eh_frame instead.

This scenario is untested.

Added: 
    

Modified: 
    llvm/include/llvm/CodeGen/AsmPrinter.h
    llvm/lib/CodeGen/AsmPrinter/ARMException.cpp
    llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
    llvm/lib/CodeGen/AsmPrinter/DwarfCFIException.cpp
    llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/CodeGen/AsmPrinter.h b/llvm/include/llvm/CodeGen/AsmPrinter.h
index 2c65a344e3848..52b5ecd1a3bbf 100644
--- a/llvm/include/llvm/CodeGen/AsmPrinter.h
+++ b/llvm/include/llvm/CodeGen/AsmPrinter.h
@@ -157,13 +157,6 @@ class AsmPrinter : public MachineFunctionPass {
           TimerGroupDescription(TimerGroupDescription) {}
   };
 
-  // Flags representing which CFI section is required for a function/module.
-  enum class CFISection : unsigned {
-    None = 0, ///< Do not emit either .eh_frame or .debug_frame
-    EH = 1,   ///< Emit .eh_frame
-    Debug = 2 ///< Emit .debug_frame
-  };
-
 private:
   MCSymbol *CurrentFnEnd = nullptr;
 
@@ -206,8 +199,8 @@ class AsmPrinter : public MachineFunctionPass {
   /// context.
   PseudoProbeHandler *PP = nullptr;
 
-  /// CFISection type the module needs i.e. either .eh_frame or .debug_frame.
-  CFISection ModuleCFISection = CFISection::None;
+  /// If the current module uses dwarf CFI annotations strictly for debugging.
+  bool isCFIMoveForDebugging = false;
 
 protected:
   explicit AsmPrinter(TargetMachine &TM, std::unique_ptr<MCStreamer> Streamer);
@@ -364,14 +357,12 @@ class AsmPrinter : public MachineFunctionPass {
 
   void emitRemarksSection(remarks::RemarkStreamer &RS);
 
-  /// Get the CFISection type for a function.
-  CFISection getFunctionCFISectionType(const Function &F) const;
-
-  /// Get the CFISection type for a function.
-  CFISection getFunctionCFISectionType(const MachineFunction &MF) const;
+  enum CFIMoveType { CFI_M_None, CFI_M_EH, CFI_M_Debug };
+  CFIMoveType needsCFIMoves() const;
 
-  /// Get the CFISection type for the module.
-  CFISection getModuleCFISectionType() const { return ModuleCFISection; }
+  /// Returns false if needsCFIMoves() == CFI_M_EH for any function
+  /// in the module.
+  bool needsOnlyDebugCFIMoves() const { return isCFIMoveForDebugging; }
 
   bool needsSEHMoves();
 

diff  --git a/llvm/lib/CodeGen/AsmPrinter/ARMException.cpp b/llvm/lib/CodeGen/AsmPrinter/ARMException.cpp
index db4215e92d44f..b634b24377fe4 100644
--- a/llvm/lib/CodeGen/AsmPrinter/ARMException.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/ARMException.cpp
@@ -39,13 +39,13 @@ void ARMException::beginFunction(const MachineFunction *MF) {
   if (Asm->MAI->getExceptionHandlingType() == ExceptionHandling::ARM)
     getTargetStreamer().emitFnStart();
   // See if we need call frame info.
-  AsmPrinter::CFISection CFISecType = Asm->getFunctionCFISectionType(*MF);
-  assert(CFISecType != AsmPrinter::CFISection::EH &&
+  AsmPrinter::CFIMoveType MoveType = Asm->needsCFIMoves();
+  assert(MoveType != AsmPrinter::CFI_M_EH &&
          "non-EH CFI not yet supported in prologue with EHABI lowering");
 
-  if (CFISecType == AsmPrinter::CFISection::Debug) {
+  if (MoveType == AsmPrinter::CFI_M_Debug) {
     if (!hasEmittedCFISections) {
-      if (Asm->getModuleCFISectionType() == AsmPrinter::CFISection::Debug)
+      if (Asm->needsOnlyDebugCFIMoves())
         Asm->OutStreamer->emitCFISections(false, true);
       hasEmittedCFISections = true;
     }

diff  --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index eb7c7e2797d4e..f12fd14b5c28b 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -351,18 +351,21 @@ bool AsmPrinter::doInitialization(Module &M) {
   case ExceptionHandling::SjLj:
   case ExceptionHandling::DwarfCFI:
   case ExceptionHandling::ARM:
+    isCFIMoveForDebugging = true;
+    if (MAI->getExceptionHandlingType() != ExceptionHandling::DwarfCFI)
+      break;
     for (auto &F : M.getFunctionList()) {
-      // If any function needsUnwindTableEntry(), it needs .eh_frame and hence
-      // the module needs .eh_frame. If we have found that case, we are done.
-      if (ModuleCFISection == AsmPrinter::CFISection::EH)
+      // If the module contains any function with unwind data,
+      // .eh_frame has to be emitted.
+      // Ignore functions that won't get emitted.
+      if (!F.isDeclarationForLinker() && F.needsUnwindTableEntry()) {
+        isCFIMoveForDebugging = false;
         break;
-      if (ModuleCFISection == AsmPrinter::CFISection::None)
-        ModuleCFISection = getFunctionCFISectionType(F);
+      }
     }
-    assert(MAI->getExceptionHandlingType() == ExceptionHandling::DwarfCFI ||
-           ModuleCFISection != AsmPrinter::CFISection::EH);
     break;
   default:
+    isCFIMoveForDebugging = false;
     break;
   }
 
@@ -1034,25 +1037,15 @@ static bool emitDebugLabelComment(const MachineInstr *MI, AsmPrinter &AP) {
   return true;
 }
 
-AsmPrinter::CFISection
-AsmPrinter::getFunctionCFISectionType(const Function &F) const {
-  // Ignore functions that won't get emitted.
-  if (F.isDeclarationForLinker())
-    return CFISection::None;
-
+AsmPrinter::CFIMoveType AsmPrinter::needsCFIMoves() const {
   if (MAI->getExceptionHandlingType() == ExceptionHandling::DwarfCFI &&
-      F.needsUnwindTableEntry())
-    return CFISection::EH;
+      MF->getFunction().needsUnwindTableEntry())
+    return CFI_M_EH;
 
-  if (MMI->hasDebugInfo() || TM.Options.ForceDwarfFrameSection)
-    return CFISection::Debug;
-
-  return CFISection::None;
-}
+  if (MMI->hasDebugInfo() || MF->getTarget().Options.ForceDwarfFrameSection)
+    return CFI_M_Debug;
 
-AsmPrinter::CFISection
-AsmPrinter::getFunctionCFISectionType(const MachineFunction &MF) const {
-  return getFunctionCFISectionType(MF.getFunction());
+  return CFI_M_None;
 }
 
 bool AsmPrinter::needsSEHMoves() {
@@ -1065,7 +1058,7 @@ void AsmPrinter::emitCFIInstruction(const MachineInstr &MI) {
       ExceptionHandlingType != ExceptionHandling::ARM)
     return;
 
-  if (getFunctionCFISectionType(*MF) == AsmPrinter::CFISection::None)
+  if (needsCFIMoves() == CFI_M_None)
     return;
 
   // If there is no "real" instruction following this CFI instruction, skip

diff  --git a/llvm/lib/CodeGen/AsmPrinter/DwarfCFIException.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfCFIException.cpp
index ae1c80141ff91..4b08e7d284686 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfCFIException.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfCFIException.cpp
@@ -93,8 +93,9 @@ void DwarfCFIException::beginFunction(const MachineFunction *MF) {
   bool hasLandingPads = !MF->getLandingPads().empty();
 
   // See if we need frame move info.
-  bool shouldEmitMoves =
-      Asm->getFunctionCFISectionType(*MF) != AsmPrinter::CFISection::None;
+  AsmPrinter::CFIMoveType MoveType = Asm->needsCFIMoves();
+
+  bool shouldEmitMoves = MoveType != AsmPrinter::CFI_M_None;
 
   const TargetLoweringObjectFile &TLOF = Asm->getObjFileLowering();
   unsigned PerEncoding = TLOF.getPersonalityEncoding();
@@ -131,14 +132,10 @@ void DwarfCFIException::beginFragment(const MachineBasicBlock *MBB,
     return;
 
   if (!hasEmittedCFISections) {
-    AsmPrinter::CFISection CFISecType = Asm->getModuleCFISectionType();
-    // If we don't say anything it implies `.cfi_sections .eh_frame`, so we
-    // chose not to be verbose in that case. And with `ForceDwarfFrameSection`,
-    // we should always emit .debug_frame.
-    if (CFISecType == AsmPrinter::CFISection::Debug ||
-        Asm->TM.Options.ForceDwarfFrameSection)
-      Asm->OutStreamer->emitCFISections(
-          CFISecType == AsmPrinter::CFISection::EH, true);
+    if (Asm->needsOnlyDebugCFIMoves())
+      Asm->OutStreamer->emitCFISections(false, true);
+    else if (Asm->TM.Options.ForceDwarfFrameSection)
+      Asm->OutStreamer->emitCFISections(true, true);
     hasEmittedCFISections = true;
   }
 

diff  --git a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
index b7b5bf33b9e67..3373e6c91b7fd 100644
--- a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
+++ b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
@@ -1229,7 +1229,7 @@ void AArch64AsmPrinter::emitInstruction(const MachineInstr *MI) {
           ExceptionHandlingType != ExceptionHandling::ARM)
         return;
 
-      if (getFunctionCFISectionType(*MF) == CFISection::None)
+      if (needsCFIMoves() == CFI_M_None)
         return;
 
       OutStreamer->emitCFIBKeyFrame();


        


More information about the llvm-commits mailing list