[DebugInfo] Exclude memory location values as parameter entry values
authorDjordje Todorovic <djordje.todorovic@rt-rk.com>
Fri, 27 Sep 2019 13:52:43 +0000 (13:52 +0000)
committerDjordje Todorovic <djordje.todorovic@rt-rk.com>
Fri, 27 Sep 2019 13:52:43 +0000 (13:52 +0000)
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

llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp
llvm/lib/CodeGen/TargetInstrInfo.cpp
llvm/test/DebugInfo/MIR/X86/dbgcall-site-interpretation.mir

index 52e5109..ea42e60 100644 (file)
@@ -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.
index 0e9eb77..2108fc6 100644 (file)
@@ -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;
index 70c5501..f4857cc 100644 (file)
 # 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: