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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
