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

[Vedant Kumar via Phabricator via llvm-commits]

vsk added subscribers: dblaikie, jmorse.
vsk added inline comments.


================
Comment at: lib/CodeGen/AsmPrinter/DwarfExpression.cpp:418
+      assert(!isParameterValue() && "Parameter entry values should not be "
+                                    "dereferenced due to safety reasons.");
       if (!isMemoryLocation() && ::isMemoryLocation(ExprCursor))
----------------
vsk wrote:
> I think this makes sense as an interim solution, but I'm concerned that this will be too restrictive in the long run. For example, I think this rules out accessing spilled values when stopped in the callee. What does gcc do?
> 
> What I had in mind was an escape analysis for the MachineMemOperand. As I understand it, the mem operand can either be backed by an llvm::Value or a PseudoSourceValue. Perhaps the analysis could visit the uses of these values to check for stores of pointers which may-alias it, or calls. I know the analysis isn't sound, because the callee can technically reconstruct+clobber the pointer bit-by-bit in a way that's invisible to the analysis. However, that strikes me as something only an attacker would do, so perhaps it's not a concern. Wdyt? (Paging @aprantl @djtodoro @friss as well.)
A data point about this: in Darwin's xnu kernel (built for x86_64 with debug entry values enabled), only 9% of DW_AT_call_value's contain DW_OP_deref. Of those, 100% are of the form "DW_OP_fbreg <offset>, DW_OP_deref", i.e. it's all spill recovery.

I think we can draw two conclusions from this:

1) This patch as-written is a "90%" solution, and thus likely represents a good engineering tradeoff.
2) If we were to try and improve entry value support further, we probably just need to focus on values spilled in the caller.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67717/new/

https://reviews.llvm.org/D67717