Author: firmiana Date: 2026-07-14T12:54:46-07:00 New Revision: fc298ccbc52a9199a2181d30fa6e7189c374c091
URL: https://github.com/llvm/llvm-project/commit/fc298ccbc52a9199a2181d30fa6e7189c374c091 DIFF: https://github.com/llvm/llvm-project/commit/fc298ccbc52a9199a2181d30fa6e7189c374c091.diff LOG: [lldb] Reject mixed typed DWARF binary operands (#201288) ## Summary LLDB currently accepts and evaluates some ill-typed DWARF typed binary operations whose two operands have different base types. DWARF v5 typed-expression rules require arithmetic/logical and relational binary operators to operate on operands of the same type, either the same base type or the generic type. (see [DWARF v5 doc](https://dwarfstd.org/doc/DWARF5.pdf) Section 2.5.1.4) This patch adds an explicit compatibility check before evaluating the affected binary operators. ## Example A DWARF expression illustrating the issue is: ```text DW_OP_constu 0xff DW_OP_convert <unsigned char> DW_OP_constu 0x1 DW_OP_convert <short unsigned int> DW_OP_plus DW_OP_stack_value ``` The left operand is typed as unsigned char, while the right operand is typed as short unsigned int. These are different DWARF base types, so `DW_OP_plus` should reject the expression instead of evaluating it. In differential testing, GDB rejects this kind of expression with: ```text Incompatible types on DWARF stack ``` Before this patch, LLDB continued evaluating the expression and produced a concrete result through the existing `Scalar` arithmetic path. ## Affected operations The issue is not specific to DW_OP_plus. The same acceptance pattern was observed for mixed-base-type typed operands across these binary operations: - Arithmetic: `DW_OP_plus`, `DW_OP_minus`, `DW_OP_div`, `DW_OP_mod` - Bitwise: `DW_OP_and`, `DW_OP_or`, `DW_OP_xor` - Shifts: `DW_OP_shl`, `DW_OP_shr`, `DW_OP_shra` - Relations: `DW_OP_lt`, `DW_OP_le`, `DW_OP_gt`, `DW_OP_ge`, `DW_OP_eq`, `DW_OP_ne` ## Implementation notes LLDB's DWARF expression stack currently stores `Value` objects, whose scalar payload is represented by `Scalar`. `Scalar` does not preserve the original DWARF base type DIE, but it does carry the pieces of base-type information used by the evaluator: - scalar kind - byte size - integer signedness This patch therefore checks those `Scalar` properties before dispatching each affected binary operation to the existing arithmetic/comparison logic. If the two operands do not match, evaluation now stops with an error. This **PR Text** was prepared with assistance from Codex. I reviewed the PR Text and take responsibility for the contribution. Added: Modified: lldb/source/Expression/DWARFExpression.cpp lldb/unittests/Expression/DWARFExpressionTest.cpp Removed: ################################################################################ diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp index c4c86b408accd..ea81d52219376 100644 --- a/lldb/source/Expression/DWARFExpression.cpp +++ b/lldb/source/Expression/DWARFExpression.cpp @@ -1324,6 +1324,46 @@ static llvm::Error Evaluate_DW_OP_call_frame_cfa(EvalContext &eval_ctx) { return llvm::Error::success(); } +static llvm::Error CheckScalarOperandsHaveSameType(const Scalar &lhs, + const Scalar &rhs, + LocationAtom opcode, + size_t address_size) { + // Scalar does not preserve the original DWARF DIE, but it does carry the + // pieces of base-type information used by the evaluator: kind, size, and + // integer signedness. + if (lhs.GetType() != rhs.GetType()) + return llvm::createStringError("%s requires operands to have the same type", + DW_OP_value_to_name(opcode)); + + if (lhs.GetByteSize() != rhs.GetByteSize()) + return llvm::createStringError("%s requires operands to have the same type", + DW_OP_value_to_name(opcode)); + + // Only integer scalars have signedness, so non-integer operands have no + // further scalar type information to compare after kind and size match. + if (lhs.GetType() != Scalar::e_int) + return llvm::Error::success(); + + // DWARF generic values are address-sized integers with unspecified + // signedness. LLDB does not explicitly preserve genericness on the expression + // stack, so treat integers at least as wide as the generic type as + // potentially generic to keep existing expressions compatible. For example, + // DW_OP_constu and DW_OP_consts currently do not always use to_generic due to + // https://github.com/llvm/llvm-project/issues/47431. A precise fix would + // require tracking genericness directly, which is a larger type-system + // change, so do not use signedness to reject these operands here. + if (address_size != 0 && lhs.GetByteSize() >= address_size) + return llvm::Error::success(); + + // For non-generic integer operands, signedness is part of the base-type + // information preserved by Scalar, so require it to match. + if (lhs.IsSigned() != rhs.IsSigned()) + return llvm::createStringError("%s requires operands to have the same type", + DW_OP_value_to_name(opcode)); + + return llvm::Error::success(); +} + llvm::Expected<Value> DWARFExpression::Evaluate( ExecutionContext *exe_ctx, RegisterContext *reg_ctx, lldb::ModuleSP module_sp, const DataExtractor &opcodes, @@ -1494,12 +1534,20 @@ llvm::Expected<Value> DWARFExpression::Evaluate( break; case DW_OP_and: + if (llvm::Error err = CheckScalarOperandsHaveSameType( + stack[stack.size() - 2].GetScalar(), stack.back().GetScalar(), + opcode, address_size)) + return err; tmp = stack.back(); stack.pop_back(); stack.back().GetScalar() = stack.back().GetScalar() & tmp.GetScalar(); break; case DW_OP_div: { + if (llvm::Error err = CheckScalarOperandsHaveSameType( + stack[stack.size() - 2].GetScalar(), stack.back().GetScalar(), + opcode, address_size)) + return err; tmp = stack.back(); if (tmp.GetScalar().IsZero()) return llvm::createStringError("divide by zero"); @@ -1517,18 +1565,30 @@ llvm::Expected<Value> DWARFExpression::Evaluate( } break; case DW_OP_minus: + if (llvm::Error err = CheckScalarOperandsHaveSameType( + stack[stack.size() - 2].GetScalar(), stack.back().GetScalar(), + opcode, address_size)) + return err; tmp = stack.back(); stack.pop_back(); stack.back().GetScalar() = stack.back().GetScalar() - tmp.GetScalar(); break; case DW_OP_mod: + if (llvm::Error err = CheckScalarOperandsHaveSameType( + stack[stack.size() - 2].GetScalar(), stack.back().GetScalar(), + opcode, address_size)) + return err; tmp = stack.back(); stack.pop_back(); stack.back().GetScalar() = stack.back().GetScalar() % tmp.GetScalar(); break; case DW_OP_mul: + if (llvm::Error err = CheckScalarOperandsHaveSameType( + stack[stack.size() - 2].GetScalar(), stack.back().GetScalar(), + opcode, address_size)) + return err; tmp = stack.back(); stack.pop_back(); stack.back().GetScalar() = stack.back().GetScalar() * tmp.GetScalar(); @@ -1545,12 +1605,20 @@ llvm::Expected<Value> DWARFExpression::Evaluate( break; case DW_OP_or: + if (llvm::Error err = CheckScalarOperandsHaveSameType( + stack[stack.size() - 2].GetScalar(), stack.back().GetScalar(), + opcode, address_size)) + return err; tmp = stack.back(); stack.pop_back(); stack.back().GetScalar() = stack.back().GetScalar() | tmp.GetScalar(); break; case DW_OP_plus: + if (llvm::Error err = CheckScalarOperandsHaveSameType( + stack[stack.size() - 2].GetScalar(), stack.back().GetScalar(), + opcode, address_size)) + return err; tmp = stack.back(); stack.pop_back(); stack.back().GetScalar() += tmp.GetScalar(); @@ -1565,12 +1633,20 @@ llvm::Expected<Value> DWARFExpression::Evaluate( } break; case DW_OP_shl: + if (llvm::Error err = CheckScalarOperandsHaveSameType( + stack[stack.size() - 2].GetScalar(), stack.back().GetScalar(), + opcode, address_size)) + return err; tmp = stack.back(); stack.pop_back(); stack.back().GetScalar() <<= tmp.GetScalar(); break; case DW_OP_shr: + if (llvm::Error err = CheckScalarOperandsHaveSameType( + stack[stack.size() - 2].GetScalar(), stack.back().GetScalar(), + opcode, address_size)) + return err; tmp = stack.back(); stack.pop_back(); if (!stack.back().GetScalar().ShiftRightLogical(tmp.GetScalar())) @@ -1578,12 +1654,20 @@ llvm::Expected<Value> DWARFExpression::Evaluate( break; case DW_OP_shra: + if (llvm::Error err = CheckScalarOperandsHaveSameType( + stack[stack.size() - 2].GetScalar(), stack.back().GetScalar(), + opcode, address_size)) + return err; tmp = stack.back(); stack.pop_back(); stack.back().GetScalar() >>= tmp.GetScalar(); break; case DW_OP_xor: + if (llvm::Error err = CheckScalarOperandsHaveSameType( + stack[stack.size() - 2].GetScalar(), stack.back().GetScalar(), + opcode, address_size)) + return err; tmp = stack.back(); stack.pop_back(); stack.back().GetScalar() = stack.back().GetScalar() ^ tmp.GetScalar(); @@ -1625,36 +1709,60 @@ llvm::Expected<Value> DWARFExpression::Evaluate( } break; case DW_OP_eq: + if (llvm::Error err = CheckScalarOperandsHaveSameType( + stack[stack.size() - 2].GetScalar(), stack.back().GetScalar(), + opcode, address_size)) + return err; tmp = stack.back(); stack.pop_back(); stack.back().GetScalar() = stack.back().GetScalar() == tmp.GetScalar(); break; case DW_OP_ge: + if (llvm::Error err = CheckScalarOperandsHaveSameType( + stack[stack.size() - 2].GetScalar(), stack.back().GetScalar(), + opcode, address_size)) + return err; tmp = stack.back(); stack.pop_back(); stack.back().GetScalar() = stack.back().GetScalar() >= tmp.GetScalar(); break; case DW_OP_gt: + if (llvm::Error err = CheckScalarOperandsHaveSameType( + stack[stack.size() - 2].GetScalar(), stack.back().GetScalar(), + opcode, address_size)) + return err; tmp = stack.back(); stack.pop_back(); stack.back().GetScalar() = stack.back().GetScalar() > tmp.GetScalar(); break; case DW_OP_le: + if (llvm::Error err = CheckScalarOperandsHaveSameType( + stack[stack.size() - 2].GetScalar(), stack.back().GetScalar(), + opcode, address_size)) + return err; tmp = stack.back(); stack.pop_back(); stack.back().GetScalar() = stack.back().GetScalar() <= tmp.GetScalar(); break; case DW_OP_lt: + if (llvm::Error err = CheckScalarOperandsHaveSameType( + stack[stack.size() - 2].GetScalar(), stack.back().GetScalar(), + opcode, address_size)) + return err; tmp = stack.back(); stack.pop_back(); stack.back().GetScalar() = stack.back().GetScalar() < tmp.GetScalar(); break; case DW_OP_ne: + if (llvm::Error err = CheckScalarOperandsHaveSameType( + stack[stack.size() - 2].GetScalar(), stack.back().GetScalar(), + opcode, address_size)) + return err; tmp = stack.back(); stack.pop_back(); stack.back().GetScalar() = stack.back().GetScalar() != tmp.GetScalar(); diff --git a/lldb/unittests/Expression/DWARFExpressionTest.cpp b/lldb/unittests/Expression/DWARFExpressionTest.cpp index f552f42dcba5c..eab7be9c2dbd1 100644 --- a/lldb/unittests/Expression/DWARFExpressionTest.cpp +++ b/lldb/unittests/Expression/DWARFExpressionTest.cpp @@ -609,6 +609,76 @@ TEST(DWARFExpression, DW_OP_convert) { "(stack has 0 entries)")); } +TEST(DWARFExpression, TypedBinaryOpsRejectMismatchedTypes) { + class TypedDwarfDelegate : public MockDwarfDelegate { + public: + enum : uint8_t { + UnsignedChar = 1, + SignedChar = 2, + UnsignedShort = 3, + }; + + llvm::Expected<std::pair<uint64_t, bool>> + GetDIEBitSizeAndSign(uint64_t relative_die_offset) const override { + switch (relative_die_offset) { + case UnsignedChar: + return std::pair<uint64_t, bool>{8, false}; + case SignedChar: + return std::pair<uint64_t, bool>{8, true}; + case UnsignedShort: + return std::pair<uint64_t, bool>{16, false}; + default: + return llvm::createStringError("unknown base type offset"); + } + } + }; + + TypedDwarfDelegate unit; + constexpr uint8_t opcodes[] = { + DW_OP_plus, DW_OP_minus, DW_OP_div, DW_OP_mod, DW_OP_mul, DW_OP_and, + DW_OP_or, DW_OP_xor, DW_OP_shl, DW_OP_shr, DW_OP_shra, DW_OP_lt, + DW_OP_le, DW_OP_gt, DW_OP_ge, DW_OP_eq, DW_OP_ne, + }; + + for (uint8_t opcode : opcodes) { + std::vector<uint8_t> expr = {DW_OP_constu, + 0xff, + 0x01, + DW_OP_convert, + TypedDwarfDelegate::UnsignedChar, + DW_OP_lit1, + DW_OP_convert, + TypedDwarfDelegate::UnsignedShort, + opcode, + DW_OP_stack_value}; + EXPECT_THAT_EXPECTED(Evaluate(expr, {}, &unit), llvm::Failed()) + << "opcode 0x" << llvm::utohexstr(opcode); + } + + EXPECT_THAT_EXPECTED( + Evaluate({DW_OP_constu, 0xff, 0x01, DW_OP_convert, + TypedDwarfDelegate::UnsignedChar, DW_OP_lit1, DW_OP_convert, + TypedDwarfDelegate::SignedChar, DW_OP_plus, DW_OP_stack_value}, + {}, &unit), + llvm::Failed()); +} + +TEST(DWARFExpression, GenericBinaryOpsAllowDifferentSignedness) { + // The DWARF generic type has unspecified signedness, so diff erently signed + // address-sized generic values are still compatible operands. + uint8_t expr[] = {DW_OP_lit8, DW_OP_consts, 4, DW_OP_minus, + DW_OP_stack_value}; + DataExtractor extractor(expr, sizeof(expr), 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(4)); +} + TEST(DWARFExpression, DW_OP_stack_value) { EXPECT_THAT_EXPECTED(Evaluate({DW_OP_stack_value}), llvm::Failed()); } _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
