[lldb] Guard DW_OP_convert against null DWARF unit and empty stack

[qiyao]

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.

[llvmorg-github-actions]

@llvm/pr-subscribers-lldb

Author: Yao Qi (qiyao)

Changes

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&lt;unsigned char&gt;, ...)

(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 --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) {

[qiyao]

Thanks for reviewing it, @Michael137.

[github-actions]

🐧 Linux x64 Test Results

• 33754 tests passed
• 544 tests skipped

✅ The build succeeded and all tests passed.

[github-actions]

🪟 Windows x64 Test Results

• 33107 tests passed
• 902 tests skipped

✅ The build succeeded and all tests passed.

[felipepiovezan]

Merging for @qiyao