llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: MrEven132 <details> <summary>Changes</summary> DWARF v5 requires relational comparisons whose operands have the generic type to be performed as signed operations. LLDB currently dispatches `DW_OP_lt`, `DW_OP_le`, `DW_OP_gt`, and `DW_OP_ge` directly to `Scalar` comparisons, whose C-like integer promotion makes an equal-width unsigned operand win. As a result, the generic comparison of an all-one address-sized value with zero can be evaluated as `UINT64_MAX < 0` instead of `-1 < 0`. Reuse the evaluator's existing approximation for potentially generic integer operands and mark temporary operand copies signed before evaluating the four ordering operations. This keeps the change local to DWARF expression semantics, without changing general `Scalar` comparisons or `DW_OP_eq` and `DW_OP_ne`. The expression stack does not yet preserve genericness explicitly, so this uses the same address-width approximation as the existing operand-type check. Precisely distinguishing generic values from explicit address-sized base types requires the larger typed-stack work already noted in the source. Add a unit test using the issue's operands and cover all four affected ordering opcodes. The focused test, all `DWARFExpression` tests, and the complete `ExpressionTests` binary pass locally. Component-level `check-lldb-unit` also passes. Fixes #<!-- -->202878 --- Full diff: https://github.com/llvm/llvm-project/pull/218335.diff 2 Files Affected: - (modified) lldb/source/Expression/DWARFExpression.cpp (+18) - (modified) lldb/unittests/Expression/DWARFExpressionTest.cpp (+39) ``````````diff diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp index 91c47546eeac7..a07bf5166ef51 100644 --- a/lldb/source/Expression/DWARFExpression.cpp +++ b/lldb/source/Expression/DWARFExpression.cpp @@ -1380,6 +1380,12 @@ static llvm::Error CheckScalarOperandsHaveSameType(const Scalar &lhs, return llvm::Error::success(); } +static bool IsPotentiallyGenericIntegerOperand(const Scalar &operand, + size_t address_size) { + return address_size != 0 && operand.GetType() == Scalar::e_int && + operand.GetByteSize() == address_size; +} + llvm::Expected<Value> DWARFExpression::Evaluate( ExecutionContext *exe_ctx, RegisterContext *reg_ctx, lldb::ModuleSP module_sp, const DataExtractor &opcodes, @@ -1419,6 +1425,14 @@ llvm::Expected<Value> DWARFExpression::Evaluate( !is_signed)); }; + auto make_generic_operands_signed = [&](Scalar &lhs, Scalar &rhs) { + if (IsPotentiallyGenericIntegerOperand(lhs, address_size) && + IsPotentiallyGenericIntegerOperand(rhs, address_size)) { + lhs.MakeSigned(); + rhs.MakeSigned(); + } + }; + llvm::DWARFExpression::iterator op = expr.begin(), op_end = expr.end(); while (op != op_end) { const uint64_t op_offset = op.getOffset(); @@ -1745,6 +1759,7 @@ llvm::Expected<Value> DWARFExpression::Evaluate( return err; tmp = stack.back(); stack.pop_back(); + make_generic_operands_signed(stack.back().GetScalar(), tmp.GetScalar()); stack.back().GetScalar() = to_generic(stack.back().GetScalar() >= tmp.GetScalar()); break; @@ -1756,6 +1771,7 @@ llvm::Expected<Value> DWARFExpression::Evaluate( return err; tmp = stack.back(); stack.pop_back(); + make_generic_operands_signed(stack.back().GetScalar(), tmp.GetScalar()); stack.back().GetScalar() = to_generic(stack.back().GetScalar() > tmp.GetScalar()); break; @@ -1767,6 +1783,7 @@ llvm::Expected<Value> DWARFExpression::Evaluate( return err; tmp = stack.back(); stack.pop_back(); + make_generic_operands_signed(stack.back().GetScalar(), tmp.GetScalar()); stack.back().GetScalar() = to_generic(stack.back().GetScalar() <= tmp.GetScalar()); break; @@ -1778,6 +1795,7 @@ llvm::Expected<Value> DWARFExpression::Evaluate( return err; tmp = stack.back(); stack.pop_back(); + make_generic_operands_signed(stack.back().GetScalar(), tmp.GetScalar()); stack.back().GetScalar() = to_generic(stack.back().GetScalar() < tmp.GetScalar()); break; diff --git a/lldb/unittests/Expression/DWARFExpressionTest.cpp b/lldb/unittests/Expression/DWARFExpressionTest.cpp index 094cfaf790a9a..50bdc96be7c93 100644 --- a/lldb/unittests/Expression/DWARFExpressionTest.cpp +++ b/lldb/unittests/Expression/DWARFExpressionTest.cpp @@ -766,6 +766,45 @@ TEST(DWARFExpression, RelationalOpsProduceGenericResult) { } } +TEST(DWARFExpression, GenericRelationalOpsUseSignedComparison) { + struct TestCase { + uint8_t opcode; + uint8_t expected; + }; + constexpr TestCase test_cases[] = { + {DW_OP_lt, 1}, {DW_OP_le, 1}, {DW_OP_gt, 0}, {DW_OP_ge, 0}}; + + for (const TestCase &test : test_cases) { + // Generic relational operands are compared as signed values, so the + // unsigned encoding of all-one bits below represents -1 for comparison. + const std::vector<uint8_t> expr = { + DW_OP_const8u, + 0xff, + 0xff, + 0xff, + 0xff, + 0xff, + 0xff, + 0xff, + 0xff, + DW_OP_consts, + 0x00, + test.opcode, + DW_OP_stack_value, + }; + DataExtractor extractor(expr.data(), expr.size(), lldb::eByteOrderLittle, + /*addr_size=*/8); + + EXPECT_THAT_EXPECTED( + DWARFExpression::Evaluate( + /*exe_ctx=*/nullptr, /*reg_ctx=*/nullptr, /*module_sp=*/{}, + extractor, /*unit=*/nullptr, lldb::eRegisterKindLLDB, + /*initial_value_ptr=*/nullptr, /*object_address_ptr=*/nullptr), + ExpectScalar(64, test.expected, false)) + << "opcode 0x" << llvm::utohexstr(test.opcode); + } +} + TEST(DWARFExpression, DW_OP_stack_value) { EXPECT_THAT_EXPECTED(Evaluate({DW_OP_stack_value}), llvm::Failed()); } `````````` </details> https://github.com/llvm/llvm-project/pull/218335 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
