[Lldb-commits] [PATCH] D77108: [lldb/DWARF] Fix evaluator crash when accessing empty stack

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D77108#1951610 <https://reviews.llvm.org/D77108#1951610>, @aprantl wrote:

> This is obviously good! Do you think that a similar error handling bug might exist in other cases that depend top-of-stack?


Most DW_OP cases check their stack, but it's quite possible that others were missed too. It might be a nice cleanup to make a function like (`getMinimalStackSize(op)`) and move this check up in front of the big switch. That could not handle all operators, as for some of them, the value is not statically known, but it would handle the vast majority of them.



================
Comment at: lldb/unittests/Expression/DWARFExpressionTest.cpp:238
+TEST(DWARFExpression, DW_OP_stack_value) {
+  EXPECT_THAT_ERROR(Evaluate({DW_OP_stack_value}).takeError(), llvm::Failed());
+}
----------------
`EXPECT_THAT_EXPECTED(Evaluate(...), Failed())` is better, as it will produce an error (instead of a crash) in case the evaluation does succeed for any reason.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77108/new/

https://reviews.llvm.org/D77108