[PATCH] D67717: [DebugInfo] Exclude memory location values as parameter entry values

[Nikola Prica via Phabricator via llvm-commits]

NikolaPrica created this revision.
NikolaPrica added reviewers: aprantl, vsk, dstenb.
NikolaPrica added a project: debug-info.

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.


https://reviews.llvm.org/D67717

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


Index: test/DebugInfo/MIR/X86/dbgcall-site-interpretation.mir
===================================================================
--- test/DebugInfo/MIR/X86/dbgcall-site-interpretation.mir
+++ 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:
Index: lib/CodeGen/TargetInstrInfo.cpp
===================================================================
--- lib/CodeGen/TargetInstrInfo.cpp
+++ lib/CodeGen/TargetInstrInfo.cpp
@@ -1133,18 +1133,6 @@
   } 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: lib/CodeGen/AsmPrinter/DwarfExpression.cpp
===================================================================
--- lib/CodeGen/AsmPrinter/DwarfExpression.cpp
+++ lib/CodeGen/AsmPrinter/DwarfExpression.cpp
@@ -246,8 +246,8 @@
   // 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 @@
       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.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D67717.220673.patch
Type: text/x-patch
Size: 2926 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190918/4f5c49d4/attachment.bin>