From: Pavel Labath Date: Tue, 24 Sep 2019 09:31:02 +0000 (+0000) Subject: MCRegisterInfo: Merge getLLVMRegNum and getLLVMRegNumFromEH X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=d93e5a30d09891eb21efef56955030b14b342554;p=llvm 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 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 33ce3bd6aaf..c7dc56ea588 100644 --- 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 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 c7e804f6260..5ec891b7aa0 100644 --- 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 Reg = TRI->getLLVMRegNum(DwarfReg, true)) + OS << printReg(*Reg, TRI); + else OS << ""; - 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 f4e6aa2471c..383c91259ff 100644 --- 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 e897a0eadbf..5009b1b7b41 100644 --- 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 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 d2f99efef22..55bcc914ea3 100644 --- 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 LLVMRegister = MRI->getLLVMRegNum(Register, true)) { + InstPrinter->printRegName(OS, *LLVMRegister); return; } } diff --git a/lib/MC/MCRegisterInfo.cpp b/lib/MC/MCRegisterInfo.cpp index 7c7376d906f..d491c0eb7e0 100644 --- 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 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 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 e7593be3524..21ce5785ea5 100644 --- 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 27141f4fc67..6196881a9b8 100644 --- 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 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 ff42996bd67..f08fcb575bf 100644 --- 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 78196bfecf4..1d43037fd6e 100644 --- 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