llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: firmiana402 <details> <summary>Changes</summary> ## 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. --- Full diff: https://github.com/llvm/llvm-project/pull/201288.diff 1 Files Affected: - (modified) lldb/source/Expression/DWARFExpression.cpp (+90) ``````````diff diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp index 106e3b4d59283..c04fcc34b7dfa 100644 --- a/lldb/source/Expression/DWARFExpression.cpp +++ b/lldb/source/Expression/DWARFExpression.cpp @@ -1291,6 +1291,32 @@ 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) { + // 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)); + + // Floating scalars do not have integer signedness, so only compare + // signedness after establishing that both operands are integers. + if (lhs.GetType() == Scalar::e_int && 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, @@ -1470,12 +1496,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)) + 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)) + return err; tmp = stack.back(); if (tmp.GetScalar().IsZero()) return llvm::createStringError("divide by zero"); @@ -1493,12 +1527,20 @@ llvm::Expected<Value> DWARFExpression::Evaluate( } break; case DW_OP_minus: + if (llvm::Error err = CheckScalarOperandsHaveSameType( + stack[stack.size() - 2].GetScalar(), stack.back().GetScalar(), + opcode)) + 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)) + return err; tmp = stack.back(); stack.pop_back(); stack.back().GetScalar() = stack.back().GetScalar() % tmp.GetScalar(); @@ -1521,12 +1563,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)) + 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)) + return err; tmp = stack.back(); stack.pop_back(); stack.back().GetScalar() += tmp.GetScalar(); @@ -1541,12 +1591,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)) + 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)) + return err; tmp = stack.back(); stack.pop_back(); if (!stack.back().GetScalar().ShiftRightLogical(tmp.GetScalar())) @@ -1554,12 +1612,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)) + 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)) + return err; tmp = stack.back(); stack.pop_back(); stack.back().GetScalar() = stack.back().GetScalar() ^ tmp.GetScalar(); @@ -1601,36 +1667,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)) + 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)) + 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)) + 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)) + 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)) + 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)) + return err; tmp = stack.back(); stack.pop_back(); stack.back().GetScalar() = stack.back().GetScalar() != tmp.GetScalar(); `````````` </details> https://github.com/llvm/llvm-project/pull/201288 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
