C++: Fix dataflow node <> expression problem on prvalues #15918
Conversation
…e expression is a prvalue.
MathiasVP
force-pushed
the
fix-as-expr-for-temps
branch
from
88a31fd to
a839c92
Compare
cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll
Outdated
Show resolved
Hide resolved
cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll
Outdated
Show resolved
Hide resolved
| @@ -1533,6 +1552,12 @@ private class IndirectArgumentOutExprNode extends ExprNodeBase, IndirectArgument | |||
| final override Expr getConvertedExpr(int n) { exprNodeShouldBeIndirectOutNode(this, result, n) } | |||
| } | |||
|
|
|||
| private class IndirectTemporaryExpr extends ExprNodeBase instanceof IndirectOperand { | |||
| IndirectTemporaryExpr() { exprNodeShouldBeIndirectOperand(this, _, _) } | |||
There was a problem hiding this comment.
I feel like there's a mismatch between this class, which claims that the node is a temporary expr, with the QLDoc on exprNodeShouldBeIndirectOperand, which only claims the node is an IndirectOperand (that should have a .asExpr()). Its probably just be me, missing a connection here.
There was a problem hiding this comment.
That's fair. I'll rename the class to something more generic. It's currently true because exprNodeShouldBeIndirectOperand restricts the node to be an indirection of an operand whose definition is related to a temporary IR variable. But that's not guaranteed to be true if we find other uses of this class.
If it's okay with you, I'll do this in a subsequent PR so that you can get this change into #15371 without waiting for another round of CI
This fixes the two surprising missing results from #15371 that @geoffw0 was seeing when he switched from interpreting
Argument[-1]asthisto*this.I didn't bother adding a testcase since @geoffw0 PR will contain such tests anyway