From 7c429fc37948fe5bd465b216a40e009026b82acb Mon Sep 17 00:00:00 2001 From: Djordje Todorovic Date: Fri, 27 Sep 2019 13:52:43 +0000 Subject: [PATCH] [DebugInfo] Exclude memory location values as parameter entry values Abandon describing of loaded values due to safety concerns. Loaded values are described as derefed memory location at caller point. At callee we can unintentionally change that memory location which would lead to different entry being printed value before and after the memory location clobbering. This problem is described in llvm.org/PR43343. Patch by Nikola Prica Differential Revision: https://reviews.llvm.org/D67717 llvm-svn: 373089 --- lib/CodeGen/AsmPrinter/DwarfExpression.cpp | 7 +++++-- lib/CodeGen/TargetInstrInfo.cpp | 12 ------------ .../MIR/X86/dbgcall-site-interpretation.mir | 6 ++---- 3 files changed, 7 insertions(+), 18 deletions(-) diff --git a/lib/CodeGen/AsmPrinter/DwarfExpression.cpp b/lib/CodeGen/AsmPrinter/DwarfExpression.cpp index 52e5109dbf9..ea42e60344d 100644 --- a/lib/CodeGen/AsmPrinter/DwarfExpression.cpp +++ b/lib/CodeGen/AsmPrinter/DwarfExpression.cpp @@ -246,8 +246,8 @@ bool DwarfExpression::addMachineRegExpression(const TargetRegisterInfo &TRI, // a call site parameter expression and if that expression is just a register // location, emit it with addBReg and offset 0, because we should emit a DWARF // expression representing a value, rather than a location. - if (!isMemoryLocation() && !HasComplexExpression && (!isParameterValue() || - isEntryValue())) { + if (!isMemoryLocation() && !HasComplexExpression && + (!isParameterValue() || isEntryValue())) { for (auto &Reg : DwarfRegs) { if (Reg.DwarfRegNo >= 0) addReg(Reg.DwarfRegNo, Reg.Comment); @@ -413,6 +413,9 @@ void DwarfExpression::addExpression(DIExpressionCursor &&ExprCursor, break; case dwarf::DW_OP_deref: assert(!isRegisterLocation()); + // For more detailed explanation see llvm.org/PR43343. + assert(!isParameterValue() && "Parameter entry values should not be " + "dereferenced due to safety reasons."); if (!isMemoryLocation() && ::isMemoryLocation(ExprCursor)) // Turning this into a memory location description makes the deref // implicit. diff --git a/lib/CodeGen/TargetInstrInfo.cpp b/lib/CodeGen/TargetInstrInfo.cpp index 0e9eb77ea26..2108fc6fecf 100644 --- a/lib/CodeGen/TargetInstrInfo.cpp +++ b/lib/CodeGen/TargetInstrInfo.cpp @@ -1133,18 +1133,6 @@ TargetInstrInfo::describeLoadedValue(const MachineInstr &MI) const { } else if (MI.isMoveImmediate()) { Op = &MI.getOperand(1); return ParamLoadedValue(*Op, Expr); - } else if (MI.hasOneMemOperand()) { - int64_t Offset; - const auto &TRI = MF->getSubtarget().getRegisterInfo(); - const auto &TII = MF->getSubtarget().getInstrInfo(); - const MachineOperand *BaseOp; - - if (!TII->getMemOperandWithOffset(MI, BaseOp, Offset, TRI)) - return None; - - Expr = DIExpression::prepend(Expr, DIExpression::DerefAfter, Offset); - Op = BaseOp; - return ParamLoadedValue(*Op, Expr); } return None; diff --git a/test/DebugInfo/MIR/X86/dbgcall-site-interpretation.mir b/test/DebugInfo/MIR/X86/dbgcall-site-interpretation.mir index 70c5501ff42..f4857ccd7e8 100644 --- a/test/DebugInfo/MIR/X86/dbgcall-site-interpretation.mir +++ b/test/DebugInfo/MIR/X86/dbgcall-site-interpretation.mir @@ -21,10 +21,8 @@ # CHECK-NEXT: DW_AT_low_pc # CHECK-EMPTY: # CHECK-NEXT: DW_TAG_GNU_call_site_parameter -# CHECK-NEXT: DW_AT_location (DW_OP_reg2 RCX) -# CHECK-NEXT: DW_AT_GNU_call_site_value (DW_OP_fbreg +8, DW_OP_deref) -# CHECK-EMPTY: -# CHECK-NEXT: DW_TAG_GNU_call_site_parameter +# RCX loads memory location. We can't rely that memory location won't be changed. +# CHECK-NOT: DW_AT_location (DW_OP_reg2 RCX) # CHECK-NEXT: DW_AT_location (DW_OP_reg4 RSI) # CHECK-NEXT: DW_AT_GNU_call_site_value (DW_OP_lit4) # CHECK-EMPTY: