[PATCH] D136243: Account for memory locations in DIExpression::createFragmentExpression

[Orlando Cazalet-Hyams via Phabricator via llvm-commits]

Orlando created this revision.
Orlando added reviewers: StephenTozer, aprantl, jryans, djtodoro.
Orlando added a project: debug-info.
Herald added a subscriber: hiraditya.
Herald added a project: All.
Orlando requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

createFragmentExpression rejects expressions containing certain ops, like
DW_OP_plus, that may cause the expression to compute a value that can't be
split.

Teach createFragmentExpression that the value loaded from an address computed
using those ops is safe to split.

Update a unittest to account for and test this change.


https://reviews.llvm.org/D136243

Files:
  llvm/lib/IR/DebugInfoMetadata.cpp
  llvm/unittests/IR/MetadataTest.cpp


Index: llvm/unittests/IR/MetadataTest.cpp
===================================================================
--- llvm/unittests/IR/MetadataTest.cpp
+++ llvm/unittests/IR/MetadataTest.cpp
@@ -2932,12 +2932,30 @@
   EXPECT_VALID_FRAGMENT(16, 16, dwarf::DW_OP_LLVM_fragment, 0, 32);
 
   // Invalid fragment expressions (incompatible ops).
-  EXPECT_INVALID_FRAGMENT(0, 32, dwarf::DW_OP_constu, 6, dwarf::DW_OP_plus);
-  EXPECT_INVALID_FRAGMENT(0, 32, dwarf::DW_OP_constu, 14, dwarf::DW_OP_minus);
-  EXPECT_INVALID_FRAGMENT(0, 32, dwarf::DW_OP_constu, 16, dwarf::DW_OP_shr);
-  EXPECT_INVALID_FRAGMENT(0, 32, dwarf::DW_OP_constu, 16, dwarf::DW_OP_shl);
-  EXPECT_INVALID_FRAGMENT(0, 32, dwarf::DW_OP_constu, 16, dwarf::DW_OP_shra);
-  EXPECT_INVALID_FRAGMENT(0, 32, dwarf::DW_OP_plus_uconst, 6);
+  EXPECT_INVALID_FRAGMENT(0, 32, dwarf::DW_OP_constu, 6, dwarf::DW_OP_plus,
+                          dwarf::DW_OP_stack_value);
+  EXPECT_INVALID_FRAGMENT(0, 32, dwarf::DW_OP_constu, 14, dwarf::DW_OP_minus,
+                          dwarf::DW_OP_stack_value);
+  EXPECT_INVALID_FRAGMENT(0, 32, dwarf::DW_OP_constu, 16, dwarf::DW_OP_shr,
+                          dwarf::DW_OP_stack_value);
+  EXPECT_INVALID_FRAGMENT(0, 32, dwarf::DW_OP_constu, 16, dwarf::DW_OP_shl,
+                          dwarf::DW_OP_stack_value);
+  EXPECT_INVALID_FRAGMENT(0, 32, dwarf::DW_OP_constu, 16, dwarf::DW_OP_shra,
+                          dwarf::DW_OP_stack_value);
+  EXPECT_INVALID_FRAGMENT(0, 32, dwarf::DW_OP_plus_uconst, 6,
+                          dwarf::DW_OP_stack_value);
+
+  // Fragments can be created for expressions using DW_OP_plus to compute an
+  // address.
+  EXPECT_VALID_FRAGMENT(0, 32, dwarf::DW_OP_constu, 6, dwarf::DW_OP_plus);
+  EXPECT_VALID_FRAGMENT(0, 32, dwarf::DW_OP_plus_uconst, 6, dwarf::DW_OP_deref);
+  EXPECT_VALID_FRAGMENT(0, 32, dwarf::DW_OP_plus_uconst, 6, dwarf::DW_OP_deref,
+                        dwarf::DW_OP_stack_value);
+  // Fragments cannot be created for expressions using DW_OP_plus to compute an
+  // implicit value (check that this correctly fails even though there is a
+  // deref in the expression).
+  EXPECT_INVALID_FRAGMENT(0, 32, dwarf::DW_OP_deref, dwarf::DW_OP_plus_uconst,
+                          2, dwarf::DW_OP_stack_value);
 
 #undef EXPECT_VALID_FRAGMENT
 #undef EXPECT_INVALID_FRAGMENT
Index: llvm/lib/IR/DebugInfoMetadata.cpp
===================================================================
--- llvm/lib/IR/DebugInfoMetadata.cpp
+++ llvm/lib/IR/DebugInfoMetadata.cpp
@@ -1471,6 +1471,9 @@
 Optional<DIExpression *> DIExpression::createFragmentExpression(
     const DIExpression *Expr, unsigned OffsetInBits, unsigned SizeInBits) {
   SmallVector<uint64_t, 8> Ops;
+  // Track whether it's safe to split the value at the top of the DWARF stack,
+  // assuming that it'll be used as an implicit location value.
+  bool CanSplitValue = true;
   // Copy over the expression, but leave off any trailing DW_OP_LLVM_fragment.
   if (Expr) {
     for (auto Op : Expr->expr_ops()) {
@@ -1488,7 +1491,18 @@
         //
         // FIXME: We *could* preserve the lowest fragment of a constant offset
         // operation if the offset fits into SizeInBits.
-        return None;
+        CanSplitValue = false;
+        break;
+      case dwarf::DW_OP_deref:
+        // Preceeding arithmetic operations have been applied to compute an
+        // address. It's okay to split the value loaded from that address.
+        CanSplitValue = true;
+        break;
+      case dwarf::DW_OP_stack_value:
+        // Bail if this expression computes a value that cannot be split.
+        if (!CanSplitValue)
+          return None;
+        break;
       case dwarf::DW_OP_LLVM_fragment: {
         // Make the new offset point into the existing fragment.
         uint64_t FragmentOffsetInBits = Op.getArg(0);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D136243.468857.patch
Type: text/x-patch
Size: 3860 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20221019/4fa2a802/attachment.bin>