[llvm] e1b3d7a - Account for memory locations in DIExpression::createFragmentExpression

via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 27 00:30:30 PDT 2022


Author: OCHyams
Date: 2022-10-27T08:29:39+01:00
New Revision: e1b3d7ab44e19d5a67f9d1a05ea503d23b4c0dd5

URL: https://github.com/llvm/llvm-project/commit/e1b3d7ab44e19d5a67f9d1a05ea503d23b4c0dd5
DIFF: https://github.com/llvm/llvm-project/commit/e1b3d7ab44e19d5a67f9d1a05ea503d23b4c0dd5.diff

LOG: Account for memory locations in DIExpression::createFragmentExpression

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.

Reviewed By: StephenTozer

Differential Revision: https://reviews.llvm.org/D136243

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/llvm/lib/IR/DebugInfoMetadata.cpp b/llvm/lib/IR/DebugInfoMetadata.cpp
index 99c445fc2842a..9b4f92a63c5e2 100644
--- a/llvm/lib/IR/DebugInfoMetadata.cpp
+++ b/llvm/lib/IR/DebugInfoMetadata.cpp
@@ -1637,6 +1637,9 @@ DIExpression *DIExpression::appendToStack(const DIExpression *Expr,
 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()) {
@@ -1654,7 +1657,23 @@ Optional<DIExpression *> DIExpression::createFragmentExpression(
         //
         // 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:
+      case dwarf::DW_OP_deref_size:
+      case dwarf::DW_OP_deref_type:
+      case dwarf::DW_OP_xderef:
+      case dwarf::DW_OP_xderef_size:
+      case dwarf::DW_OP_xderef_type:
+        // 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);
@@ -1669,6 +1688,7 @@ Optional<DIExpression *> DIExpression::createFragmentExpression(
       Op.appendToVector(Ops);
     }
   }
+  assert((!Expr->isImplicit() || CanSplitValue) && "Expr can't be split");
   assert(Expr && "Unknown DIExpression");
   Ops.push_back(dwarf::DW_OP_LLVM_fragment);
   Ops.push_back(OffsetInBits);

diff  --git a/llvm/unittests/IR/MetadataTest.cpp b/llvm/unittests/IR/MetadataTest.cpp
index 126ae03ff9877..72e89be3e3c69 100644
--- a/llvm/unittests/IR/MetadataTest.cpp
+++ b/llvm/unittests/IR/MetadataTest.cpp
@@ -3032,12 +3032,43 @@ TEST_F(DIExpressionTest, createFragmentExpression) {
   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);
+
+  // Check the other deref operations work in the same way.
+  EXPECT_VALID_FRAGMENT(0, 32, dwarf::DW_OP_plus_uconst, 6,
+                        dwarf::DW_OP_deref_size, 1);
+  EXPECT_VALID_FRAGMENT(0, 32, dwarf::DW_OP_plus_uconst, 6,
+                        dwarf::DW_OP_deref_type, 1, 1);
+  EXPECT_VALID_FRAGMENT(0, 32, dwarf::DW_OP_plus_uconst, 6,
+                        dwarf::DW_OP_xderef);
+  EXPECT_VALID_FRAGMENT(0, 32, dwarf::DW_OP_plus_uconst, 6,
+                        dwarf::DW_OP_xderef_size, 1);
+  EXPECT_VALID_FRAGMENT(0, 32, dwarf::DW_OP_plus_uconst, 6,
+                        dwarf::DW_OP_xderef_type, 1, 1);
+
+  // 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


        


More information about the llvm-commits mailing list