llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Yao Qi (qiyao) <details> <summary>Changes</summary> `Evaluate_DW_OP_convert` dereferenced `eval_ctx.dwarf_cu` (the `DWARFExpression` Delegate) whenever the operand DIE offset was non-zero, and unconditionally read `eval_ctx.stack.back()`. When a DWARF expression is evaluated without a DWARF unit (as the lldb-dwarf-expression-fuzzer does), two operand shapes crash: - `DW_OP_convert` with a non-zero offset calls `dwarf_cu->GetDIEBitSizeAndSign(...)` on a null Delegate. - `DW_OP_convert` with nothing on the stack reads the back of an empty vector. The unit test feeds both with `dwarf_cu == nullptr` and crashes: ``` [ RUN ] DWARFExpression.DW_OP_convert #<!-- -->2 SignalHandler(int, __siginfo*, void*) #<!-- -->4 DWARFExpression::Evaluate(...) #<!-- -->5 Evaluate(ArrayRef<unsigned char>, ...) ``` (SIGSEGV, the process aborts.) Bail out with an error when the stack is empty, and when a non-zero DIE offset is requested without a DWARF unit, instead of crashing. Extends `DWARFExpression.DW_OP_convert` with these two cases, which crash without the fix. --- Full diff: https://github.com/llvm/llvm-project/pull/207008.diff 2 Files Affected: - (modified) lldb/source/Expression/DWARFExpression.cpp (+6) - (modified) lldb/unittests/Expression/DWARFExpressionTest.cpp (+14) ``````````diff diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp index 5a3c99caeb1bb..aa4fd62f313d7 100644 --- a/lldb/source/Expression/DWARFExpression.cpp +++ b/lldb/source/Expression/DWARFExpression.cpp @@ -1201,6 +1201,9 @@ static llvm::Error Evaluate_DW_OP_piece(EvalContext &eval_ctx, static llvm::Error Evaluate_DW_OP_convert(EvalContext &eval_ctx, uint64_t relative_die_offset) { + if (eval_ctx.stack.empty()) + return llvm::createStringError("DW_OP_convert needs an argument"); + uint64_t bit_size; bool sign; if (relative_die_offset == 0) { @@ -1214,6 +1217,9 @@ static llvm::Error Evaluate_DW_OP_convert(EvalContext &eval_ctx, if (!bit_size) return llvm::createStringError("unspecified architecture"); } else { + if (!eval_ctx.dwarf_cu) + return llvm::createStringError( + "DW_OP_convert with a DIE offset requires a DWARF unit"); auto bit_size_sign_or_err = eval_ctx.dwarf_cu->GetDIEBitSizeAndSign(relative_die_offset); if (!bit_size_sign_or_err) diff --git a/lldb/unittests/Expression/DWARFExpressionTest.cpp b/lldb/unittests/Expression/DWARFExpressionTest.cpp index 305c4af2582db..2bb939778d7e4 100644 --- a/lldb/unittests/Expression/DWARFExpressionTest.cpp +++ b/lldb/unittests/Expression/DWARFExpressionTest.cpp @@ -574,6 +574,20 @@ TEST(DWARFExpression, DW_OP_convert) { EXPECT_THAT_ERROR( t.Eval({DW_OP_const1s, 'X', DW_OP_convert, 0x1d}).takeError(), llvm::Failed()); + + // A non-zero DIE offset with no DWARF unit must report an error rather than + // dereferencing a null Delegate (caught by lldb-dwarf-expression-fuzzer). + EXPECT_THAT_ERROR( + Evaluate({DW_OP_const1s, 'X', DW_OP_convert, 0x01}, nullptr, nullptr) + .takeError(), + llvm::Failed()); + + // DW_OP_convert with an empty stack must report an error rather than + // accessing the back of an empty stack (caught by + // lldb-dwarf-expression-fuzzer). + EXPECT_THAT_ERROR( + Evaluate({DW_OP_convert, 0x00}, nullptr, nullptr).takeError(), + llvm::Failed()); } TEST(DWARFExpression, DW_OP_stack_value) { `````````` </details> https://github.com/llvm/llvm-project/pull/207008 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
