MCRegisterInfo: Merge getLLVMRegNum and getLLVMRegNumFromEH

Summary:
The functions different in two ways:
- getLLVMRegNum could return both "eh" and "other" dwarf register
  numbers, while getLLVMRegNumFromEH only returned the "eh" number.
- getLLVMRegNum asserted if the register was not found, while the second
  function returned -1.

The second distinction was pretty important, but it was very hard to
infer that from the function name. Aditionally, for the use case of
dumping dwarf expressions, we needed a function which can work with both
kinds of number, but does not assert.

This patch solves both of these issues by merging the two functions into
one, returning an Optional<unsigned> value. While the same thing could
be achieved by adding an "IsEH" argument to the (renamed)
getLLVMRegNumFromEH function, it seemed better to avoid the confusion of
two functions and put the choice of asserting into the hands of the
caller -- if he checks the Optional value, he can safely process
"untrusted" input, and if he blindly dereferences the Optional, he gets
the assertion.

I've updated all call sites to the new API, choosing between the two
options according to the function they were calling originally, except
that I've updated the usage in DWARFExpression.cpp to use the "safe"
method instead, and added a test case which would have previously
triggered an assertion failure when processing (incorrect?) dwarf
expressions.

Reviewers: dsanders, arsenm, JDevlieghere

Subscribers: wdng, aprantl, javed.absar, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D67154

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@372710 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/MC/MCRegisterInfo.h b/include/llvm/MC/MCRegisterInfo.h
index 33ce3bd6aaf07db7a552a8b4059f8f5285b0ef03..c7dc56ea588e93c8f9239f959ec4a8a5b215b1e8 100644 (file)
--- a/include/llvm/MC/MCRegisterInfo.h
+++ b/include/llvm/MC/MCRegisterInfo.h
@@ -395,13 +395,9 @@ public:
   /// debugging info.
   int getDwarfRegNum(MCRegister RegNum, bool isEH) const;
 
-  /// Map a dwarf register back to a target register.
-  int getLLVMRegNum(unsigned RegNum, bool isEH) const;
-
-  /// Map a DWARF EH register back to a target register (same as
-  /// getLLVMRegNum(RegNum, true)) but return -1 if there is no mapping,
-  /// rather than asserting that there must be one.
-  int getLLVMRegNumFromEH(unsigned RegNum) const;
+  /// Map a dwarf register back to a target register. Returns None is there is
+  /// no mapping.
+  Optional<unsigned> getLLVMRegNum(unsigned RegNum, bool isEH) const;
 
   /// Map a target EH register number to an equivalent DWARF register
   /// number.

diff --git a/lib/CodeGen/MachineOperand.cpp b/lib/CodeGen/MachineOperand.cpp
index c7e804f6260a660e7a94265f3c888b81e8f940f4..5ec891b7aa0cae2726eb81eb6956f3ac2c71c495 100644 (file)
--- a/lib/CodeGen/MachineOperand.cpp
+++ b/lib/CodeGen/MachineOperand.cpp
@@ -429,12 +429,10 @@ static void printCFIRegister(unsigned DwarfReg, raw_ostream &OS,
     return;
   }
 
-  int Reg = TRI->getLLVMRegNum(DwarfReg, true);
-  if (Reg == -1) {
+  if (Optional<unsigned> Reg = TRI->getLLVMRegNum(DwarfReg, true))
+    OS << printReg(*Reg, TRI);
+  else
     OS << "<badreg>";
-    return;
-  }
-  OS << printReg(Reg, TRI);
 }
 
 static void printIRBlockReference(raw_ostream &OS, const BasicBlock &BB,

diff --git a/lib/CodeGen/StackMaps.cpp b/lib/CodeGen/StackMaps.cpp
index f4e6aa2471c8a13370a093db77647e889b4c16bf..383c91259ffc8b7b9170e0b192c1f339198afa00 100644 (file)
--- a/lib/CodeGen/StackMaps.cpp
+++ b/lib/CodeGen/StackMaps.cpp
@@ -155,7 +155,7 @@ StackMaps::parseOperand(MachineInstr::const_mop_iterator MOI,
 
     unsigned Offset = 0;
     unsigned DwarfRegNum = getDwarfRegNum(MOI->getReg(), TRI);
-    unsigned LLVMRegNum = TRI->getLLVMRegNum(DwarfRegNum, false);
+    unsigned LLVMRegNum = *TRI->getLLVMRegNum(DwarfRegNum, false);
     unsigned SubRegIdx = TRI->getSubRegIndex(LLVMRegNum, MOI->getReg());
     if (SubRegIdx)
       Offset = TRI->getSubRegIdxOffset(SubRegIdx);

diff --git a/lib/DebugInfo/DWARF/DWARFExpression.cpp b/lib/DebugInfo/DWARF/DWARFExpression.cpp
index e897a0eadbf8132eb1ac6312888100f66ed2fcf9..5009b1b7b4127df7304a78f712f69bcda276cf4a 100644 (file)
--- a/lib/DebugInfo/DWARF/DWARFExpression.cpp
+++ b/lib/DebugInfo/DWARF/DWARFExpression.cpp
@@ -218,9 +218,8 @@ static bool prettyPrintRegisterOp(raw_ostream &OS, uint8_t Opcode,
   else
     DwarfRegNum = Opcode - DW_OP_reg0;
 
-  int LLVMRegNum = MRI->getLLVMRegNum(DwarfRegNum, isEH);
-  if (LLVMRegNum >= 0) {
-    if (const char *RegName = MRI->getName(LLVMRegNum)) {
+  if (Optional<unsigned> LLVMRegNum = MRI->getLLVMRegNum(DwarfRegNum, isEH)) {
+    if (const char *RegName = MRI->getName(*LLVMRegNum)) {
       if ((Opcode >= DW_OP_breg0 && Opcode <= DW_OP_breg31) ||
           Opcode == DW_OP_bregx)
         OS << format(" %s%+" PRId64, RegName, Operands[OpNum]);

diff --git a/lib/MC/MCAsmStreamer.cpp b/lib/MC/MCAsmStreamer.cpp
index d2f99efef22abae78d6e4c2362c8b2709f57a8e8..55bcc914ea32d0a0f63327921e29b425ac6243f8 100644 (file)
--- a/lib/MC/MCAsmStreamer.cpp
+++ b/lib/MC/MCAsmStreamer.cpp
@@ -1533,9 +1533,8 @@ void MCAsmStreamer::EmitRegisterName(int64_t Register) {
     // just ones that map to LLVM register numbers and have known names.
     // Fall back to using the original number directly if no name is known.
     const MCRegisterInfo *MRI = getContext().getRegisterInfo();
-    int LLVMRegister = MRI->getLLVMRegNumFromEH(Register);
-    if (LLVMRegister != -1) {
-      InstPrinter->printRegName(OS, LLVMRegister);
+    if (Optional<unsigned> LLVMRegister = MRI->getLLVMRegNum(Register, true)) {
+      InstPrinter->printRegName(OS, *LLVMRegister);
       return;
     }
   }

diff --git a/lib/MC/MCRegisterInfo.cpp b/lib/MC/MCRegisterInfo.cpp
index 7c7376d906f7d9d9742186b8a480148f7543b6ac..d491c0eb7e060f3e9e826b79d2edc04c7c0be0af 100644 (file)
--- a/lib/MC/MCRegisterInfo.cpp
+++ b/lib/MC/MCRegisterInfo.cpp
@@ -78,29 +78,18 @@ int MCRegisterInfo::getDwarfRegNum(MCRegister RegNum, bool isEH) const {
   return I->ToReg;
 }
 
-int MCRegisterInfo::getLLVMRegNum(unsigned RegNum, bool isEH) const {
+Optional<unsigned> MCRegisterInfo::getLLVMRegNum(unsigned RegNum,
+                                                 bool isEH) const {
   const DwarfLLVMRegPair *M = isEH ? EHDwarf2LRegs : Dwarf2LRegs;
   unsigned Size = isEH ? EHDwarf2LRegsSize : Dwarf2LRegsSize;
 
   if (!M)
-    return -1;
-  DwarfLLVMRegPair Key = { RegNum, 0 };
-  const DwarfLLVMRegPair *I = std::lower_bound(M, M+Size, Key);
-  assert(I != M+Size && I->FromReg == RegNum && "Invalid RegNum");
-  return I->ToReg;
-}
-
-int MCRegisterInfo::getLLVMRegNumFromEH(unsigned RegNum) const {
-  const DwarfLLVMRegPair *M = EHDwarf2LRegs;
-  unsigned Size = EHDwarf2LRegsSize;
-
-  if (!M)
-    return -1;
+    return None;
   DwarfLLVMRegPair Key = { RegNum, 0 };
   const DwarfLLVMRegPair *I = std::lower_bound(M, M+Size, Key);
-  if (I == M+Size || I->FromReg != RegNum)
-    return -1;
-  return I->ToReg;
+  if (I != M + Size && I->FromReg == RegNum)
+    return I->ToReg;
+  return None;
 }
 
 int MCRegisterInfo::getDwarfRegNumFromDwarfEHRegNum(unsigned RegNum) const {
@@ -112,9 +101,8 @@ int MCRegisterInfo::getDwarfRegNumFromDwarfEHRegNum(unsigned RegNum) const {
   // a corresponding LLVM register number at all.  So if we can't map the
   // EH register number to an LLVM register number, assume it's just a
   // valid DWARF register number as is.
-  int LRegNum = getLLVMRegNumFromEH(RegNum);
-  if (LRegNum != -1)
-    return getDwarfRegNum(LRegNum, false);
+  if (Optional<unsigned> LRegNum = getLLVMRegNum(RegNum, true))
+    return getDwarfRegNum(*LRegNum, false);
   return RegNum;
 }
 

diff --git a/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp b/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp
index e7593be35247c3d61fabfa7ed1ccf1ea5ec401ea..21ce5785ea5e12b1a9451c29f178330f92f08a40 100644 (file)
--- a/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp
+++ b/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp
@@ -573,7 +573,7 @@ public:
       case MCCFIInstruction::OpDefCfa: {
         // Defines a frame pointer.
         unsigned XReg =
-            getXRegFromWReg(MRI.getLLVMRegNum(Inst.getRegister(), true));
+            getXRegFromWReg(*MRI.getLLVMRegNum(Inst.getRegister(), true));
 
         // Other CFA registers than FP are not supported by compact unwind.
         // Fallback on DWARF.
@@ -592,8 +592,8 @@ public:
         assert(FPPush.getOperation() == MCCFIInstruction::OpOffset &&
                "Frame pointer not pushed!");
 
-        unsigned LRReg = MRI.getLLVMRegNum(LRPush.getRegister(), true);
-        unsigned FPReg = MRI.getLLVMRegNum(FPPush.getRegister(), true);
+        unsigned LRReg = *MRI.getLLVMRegNum(LRPush.getRegister(), true);
+        unsigned FPReg = *MRI.getLLVMRegNum(FPPush.getRegister(), true);
 
         LRReg = getXRegFromWReg(LRReg);
         FPReg = getXRegFromWReg(FPReg);
@@ -614,14 +614,14 @@ public:
       case MCCFIInstruction::OpOffset: {
         // Registers are saved in pairs. We expect there to be two consecutive
         // `.cfi_offset' instructions with the appropriate registers specified.
-        unsigned Reg1 = MRI.getLLVMRegNum(Inst.getRegister(), true);
+        unsigned Reg1 = *MRI.getLLVMRegNum(Inst.getRegister(), true);
         if (i + 1 == e)
           return CU::UNWIND_ARM64_MODE_DWARF;
 
         const MCCFIInstruction &Inst2 = Instrs[++i];
         if (Inst2.getOperation() != MCCFIInstruction::OpOffset)
           return CU::UNWIND_ARM64_MODE_DWARF;
-        unsigned Reg2 = MRI.getLLVMRegNum(Inst2.getRegister(), true);
+        unsigned Reg2 = *MRI.getLLVMRegNum(Inst2.getRegister(), true);
 
         // N.B. The encodings must be in register number order, and the X
         // registers before the D registers.

diff --git a/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp b/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
index 27141f4fc6797153c3ad9ce1eee8958c881fafca..6196881a9b8f883d8e3be0c2dd8a5e0d306e0b44 100644 (file)
--- a/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
+++ b/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
@@ -1105,28 +1105,28 @@ uint32_t ARMAsmBackendDarwin::generateCompactUnwindEncoding(
   if (Instrs.empty())
     return 0;
   // Start off assuming CFA is at SP+0.
-  int CFARegister = ARM::SP;
+  unsigned CFARegister = ARM::SP;
   int CFARegisterOffset = 0;
   // Mark savable registers as initially unsaved
   DenseMap<unsigned, int> RegOffsets;
   int FloatRegCount = 0;
   // Process each .cfi directive and build up compact unwind info.
   for (size_t i = 0, e = Instrs.size(); i != e; ++i) {
-    int Reg;
+    unsigned Reg;
     const MCCFIInstruction &Inst = Instrs[i];
     switch (Inst.getOperation()) {
     case MCCFIInstruction::OpDefCfa: // DW_CFA_def_cfa
       CFARegisterOffset = -Inst.getOffset();
-      CFARegister = MRI.getLLVMRegNum(Inst.getRegister(), true);
+      CFARegister = *MRI.getLLVMRegNum(Inst.getRegister(), true);
       break;
     case MCCFIInstruction::OpDefCfaOffset: // DW_CFA_def_cfa_offset
       CFARegisterOffset = -Inst.getOffset();
       break;
     case MCCFIInstruction::OpDefCfaRegister: // DW_CFA_def_cfa_register
-      CFARegister = MRI.getLLVMRegNum(Inst.getRegister(), true);
+      CFARegister = *MRI.getLLVMRegNum(Inst.getRegister(), true);
       break;
     case MCCFIInstruction::OpOffset: // DW_CFA_offset
-      Reg = MRI.getLLVMRegNum(Inst.getRegister(), true);
+      Reg = *MRI.getLLVMRegNum(Inst.getRegister(), true);
       if (ARMMCRegisterClasses[ARM::GPRRegClassID].contains(Reg))
         RegOffsets[Reg] = Inst.getOffset();
       else if (ARMMCRegisterClasses[ARM::DPRRegClassID].contains(Reg)) {

diff --git a/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp b/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
index ff42996bd67971787646bbbf8c3bd84458e37c22..f08fcb575bf007d08990573ba49c1d133b5af534 100644 (file)
--- a/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
+++ b/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
@@ -557,7 +557,7 @@ protected:
 
         // If the frame pointer is other than esp/rsp, we do not have a way to
         // generate a compact unwinding representation, so bail out.
-        if (MRI.getLLVMRegNum(Inst.getRegister(), true) !=
+        if (*MRI.getLLVMRegNum(Inst.getRegister(), true) !=
             (Is64Bit ? X86::RBP : X86::EBP))
           return 0;
 
@@ -605,7 +605,7 @@ protected:
           // unwind encoding.
           return CU::UNWIND_MODE_DWARF;
 
-        unsigned Reg = MRI.getLLVMRegNum(Inst.getRegister(), true);
+        unsigned Reg = *MRI.getLLVMRegNum(Inst.getRegister(), true);
         SavedRegs[SavedRegIdx++] = Reg;
         StackAdjust += OffsetSize;
         InstrOffset += PushInstrSize(Reg);

diff --git a/test/DebugInfo/X86/dwarfdump-debug-loc-error-cases.s b/test/DebugInfo/X86/dwarfdump-debug-loc-error-cases.s
index 78196bfecf46b278be5bfe66bc67ae7d3aaacea1..1d43037fd6eeabb11231b9b7fde79fdbdcf679ca 100644 (file)
--- a/test/DebugInfo/X86/dwarfdump-debug-loc-error-cases.s
+++ b/test/DebugInfo/X86/dwarfdump-debug-loc-error-cases.s
@@ -16,8 +16,13 @@
 # RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE6=0 -o %t6.o
 # RUN: llvm-dwarfdump -debug-loc %t6.o 2>&1 | FileCheck %s
 
+# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE7=0 -o %t7.o
+# RUN: llvm-dwarfdump -debug-loc %t7.o 2>&1 | FileCheck %s --check-prefix=UNKNOWN-REG
+
 # CHECK: error: unexpected end of data
 
+# UNKNOWN-REG: [0x0000000000000000,  0x0000000000000001): DW_OP_regx 0xdeadbeef
+
 .section  .debug_loc,"",@progbits
 .ifdef CASE1
   .byte  1                       # bogus
@@ -45,6 +50,17 @@
   .quad  1                       # ending offset
   .word  0xffff                  # Loc expr size
 .endif
+.ifdef CASE7
+  .quad  0                       # starting offset
+  .quad  1                       # ending offset
+  .word  2f-1f                   # Loc expr size
+1:
+  .byte  0x90                    # DW_OP_regx
+  .uleb128 0xdeadbeef
+2:
+  .quad  0                       # starting offset
+  .quad  0                       # ending offset
+.endif
 
 # A minimal compile unit is needed to deduce the address size of the location
 # lists