[Lldb-commits] [PATCH] D113498: [lldb] Constant-resolve operands to `getelementptr`

[Andy Yankovsky via Phabricator via lldb-commits]

werat updated this revision to Diff 387236.
werat added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113498

Files:
  lldb/source/Expression/IRInterpreter.cpp
  lldb/test/API/lang/cpp/static_members/TestCPPStaticMembers.py


Index: lldb/test/API/lang/cpp/static_members/TestCPPStaticMembers.py
===================================================================
--- lldb/test/API/lang/cpp/static_members/TestCPPStaticMembers.py
+++ lldb/test/API/lang/cpp/static_members/TestCPPStaticMembers.py
@@ -41,3 +41,31 @@
         self.createTestTarget()
         self.expect("expression s_c", error=True,
                     startstr="error: use of undeclared identifier 's_d'")
+
+    def test_no_crash_in_IR_arithmetic(self):
+        """
+        Test that LLDB doesn't crash on evaluating specific expression involving
+        pointer arithmetic and taking the address of a static class member.
+        See https://bugs.llvm.org/show_bug.cgi?id=52449
+        """
+        self.build()
+        lldbutil.run_to_source_breakpoint(self, "// stop in main", lldb.SBFileSpec("main.cpp"))
+
+        # This expression contains the following IR code:
+        # ... i64 ptrtoint (i32* @_ZN1A3s_cE to i64)) ...
+        expr = "(int*)100 + (long long)(&A::s_c)"
+
+        # The IR interpreter doesn't support non-const operands to the
+        # `GetElementPtr` IR instruction, so verify that it correctly fails to
+        # evaluate expression.
+        opts = lldb.SBExpressionOptions()
+        opts.SetAllowJIT(False)
+        value = self.target().EvaluateExpression(expr, opts)
+        self.assertTrue(value.GetError().Fail())
+        self.assertIn(
+            "Can't evaluate the expression without a running target",
+            value.GetError().GetCString())
+
+        # Evaluating the expression via JIT should work fine.
+        value = self.target().EvaluateExpression(expr)
+        self.assertSuccess(value.GetError())
Index: lldb/source/Expression/IRInterpreter.cpp
===================================================================
--- lldb/source/Expression/IRInterpreter.cpp
+++ lldb/source/Expression/IRInterpreter.cpp
@@ -282,12 +282,33 @@
           if (op_cursor == op_end)
             return true; // no offset to apply!
 
+          // DataLayout::getIndexedOffsetInType assumes the indices are
+          // instances of ConstantInt, so we need to resolve them.
           SmallVector<Value *, 8> indices(op_cursor, op_end);
+          SmallVector<Value *, 8> resolved_indices;
+
+          for (Value *idx : indices) {
+            Constant *constant_idx = dyn_cast<Constant>(idx);
+            if (!constant_idx)
+              return false;
+
+            ConstantInt *constant_int = dyn_cast<ConstantInt>(constant_idx);
+            if (!constant_int) {
+              APInt v;
+              if (!ResolveConstantValue(v, constant_idx))
+                return false;
+
+              constant_int =
+                  cast<ConstantInt>(ConstantInt::get(idx->getType(), v));
+            }
+
+            resolved_indices.push_back(constant_int);
+          }
 
           Type *src_elem_ty =
               cast<GEPOperator>(constant_expr)->getSourceElementType();
-          uint64_t offset =
-              m_target_data.getIndexedOffsetInType(src_elem_ty, indices);
+          uint64_t offset = m_target_data.getIndexedOffsetInType(
+              src_elem_ty, resolved_indices);
 
           const bool is_signed = true;
           value += APInt(value.getBitWidth(), offset, is_signed);
@@ -465,12 +486,17 @@
       case Instruction::BitCast:
         return CanResolveConstant(constant_expr->getOperand(0));
       case Instruction::GetElementPtr: {
-        ConstantExpr::const_op_iterator op_cursor = constant_expr->op_begin();
-        Constant *base = dyn_cast<Constant>(*op_cursor);
-        if (!base)
-          return false;
-
-        return CanResolveConstant(base);
+        // Check that all operands of `getelementptr` can be constant-resolved.
+        SmallVector<Value *, 8> operands(constant_expr->op_begin(),
+                                         constant_expr->op_end());
+        for (Value *op : operands) {
+          Constant *constant_op = dyn_cast<Constant>(op);
+          if (!constant_op)
+            return false;
+          if (!CanResolveConstant(constant_op))
+            return false;
+        }
+        return true;
       }
       }
     } else {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D113498.387236.patch
Type: text/x-patch
Size: 4202 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20211115/68930e55/attachment-0001.bin>