C++: Barrier guards API for indirect expressions

[MathiasVP]

Background

In the rewrite to use-use flow for C/C++ we added a concept of an indirect node which represents "the value you'd get by dereferencing the value of the expression represented by this node". These are the nodes accessible using the Node::asIndirectExpr() predicate which returns an IndirectExprNode (instead of an ExprNode).

The problem

These new IndirectExprNodes couldn't be marked as barriers with a barrier guard because the predicate offered by the BarrierGuard parameterized module had the signature:

ExprNode getABarrierNode();

which meant that an IndirectExprNodes was never the result of this predicate.

This PR

To resolve the above issue, I've added a new predicate to the BarrierGuard parameterized module with the signature:

IndirectExprNode getAnIndirectBarrierNode();

the implementation is syntactically identical: the only change is the return type.

Commit-by-commit review recommended.

• The first commit changes our dataflow tests to use the expression-based BarrierGuard API (instead of the Instruction-based one). I don't think we want users to use the Instruction-based one anyway, and we should test the APIs we tell people to use.
• The second commit adds a test that needs indirect expressions to be marked by a barrier guard.
• The final commit adds the new API and accepts the test changes 🎉

[aschackmull]

Why not just change ExprNode getABarrierNode(); to Node getABarrierNode();?

[MathiasVP]

Why not just change ExprNode getABarrierNode(); to Node getABarrierNode();?

That would also be fine, but then users would have to add an additional check in their isBarrier predicate if they want to restrict the barrier to one of the two cases.

I could imagine both situations, really. Most of the time users probably don't care which one they sanitize because they just want to remove some FP they're seeing. But I personally like to be able to distinguish the two cases, and still keep the pretty one-liner that you get from writing

barrier = DataFlow::BarrierGuard<testBarrierGuard/3>::getAnIndirectBarrierNode()

(😍)

[aschackmull]

But I personally like to be able to distinguish the two cases

Right, if you imagine use-cases where you'll want to only have one or the other then it makes sense, but I just thought that you'd always want both.

[aschackmull]

If indeed there are such use-cases that merit a split like this, then the qldoc probably needs some elaboration, because it's not at all clear that such use-cases exist, so that'll probably make it hard for users to figure out when to use one or the other or both.

[MathiasVP]

If indeed there are such use-cases that merit a split like this, then the qldoc probably needs some elaboration, because it's not at all clear that such use-cases exist, so that'll probably make it hard for users to figure out when to use one or the other or both.

Good point. I'll add some QLDoc. The concept of "indirect expression" is explained in some other QLDoc, but it can't hurt to add a more specific explanation within the context of barrier guards 👍.

[aschackmull]

Good point. I'll add some QLDoc

👍 Nicely elaborate.

[jketema]

LGTM, one small comment. I assume we can skip any DCA experiment, because the new predicate is not used in any query?

[jketema]

Do we want to expose the indirection index here as an argument of getAnIndirectBarrierNode?

[MathiasVP]

We could do that, yeah. And then implement a getAnIndirectBarrierNode with no parameters as getAnIndirectBarrierNode(_)?

[jketema]

Yup.

[MathiasVP]

Fixed in 2d98fb7.

[MathiasVP]

LGTM, one small comment. I assume we can skip any DCA experiment, because the new predicate is not used in any query?

Yeah, I don't see any reason to run DCA here 👍.

[jketema]

LGTM