Ruby: Add post-update nodes for compound arguments

[hvitved]

Post-update nodes

Post-update nodes are used to enable flow out through arguments:

def taint_field x
  x.field = "taint"
end

taint_field(y)
sink(y.field)

There is flow from "taint" to the field field on post-update node for y in the call to taint_field, written (post) y [field], and from (post) y [field] in the call to taint_field to y [field] in the call to sink, and finally to y.field in the call to sink.

Compound arguments

However, when the argument is a compound expression, such as

taint_field(if b then x else y)
sink(y.field)

we fail to track flow, since there is no connection between (post) if b then x else y and the access to y in sink(y.field).

This PR fixes that, by also adding post-update nodes for x and y in if b then x else y, and let those nodes have two pre-update nodes:

1. the compound argument, if b then x else y; and
2. the underlying expressions, x and y, respectively.

This ensures that we get flow out of the call into both leafs (1), while still maintaining the invariant that the underlying expression is a pre-update node (2). Of course, compound expression can be nested, if b then x else if c then y else z, so we also need to handle that.

Common in Ruby (?)

While compound arguments such as if b then x else y may not be that common, it is easy to write an argument that is:

taint_field(x)
# vs
taint_field (x)

In the former call, x is just a normal argument, but in the latter the argument is a parenthesized expression with just one child expression, x.

[tausbn]

While compound arguments such as if b then x else y may not be that common, it is easy to write an argument that is:

taint_field(x)
# vs
taint_field (x)

In the former call, x is just a normal argument, but in the latter the argument is a parenthesized expression with just one child expression, x.

Fascinating, and a bit horrifying.

I wonder, though, could this be handled at the level of AST desugaring?

[tausbn]

Python changes look, well, harmless to me. 🙂

[MathiasVP]

C++ 👍. I just realized that I forgot to update identical-files.json when I added a couple of copies of the dataflow library into experimental last week. So if #10487 gets merged before this you might need to pull in main and update the identical files in C++'s experimental directory. Sorry!

[hvitved]

I wonder, though, could this be handled at the level of AST desugaring?

Yes, we could in principle desugar (x) to just x, but since we would have to handle (foo; x) as well anyway, it seems a bit pointless to special case.

[tausbn]

I wonder, though, could this be handled at the level of AST desugaring?

Yes, we could in principle desugar (x) to just x, but since we would have to handle (foo; x) as well anyway, it seems a bit pointless to special case.

For this case, definitely. I was just thinking more generally if it would be less surprising for users to have foo (x) represented in the same way as foo(x).

[hvitved]

I was just thinking more generally if it would be less surprising for users to have foo (x) represented in the same way as foo(x).

So, desugar (x) to x when it is the only argument of a call? I guess we could do that.

[atorralba]

Java looks harmless too 👍

[michaelnebel]

C# LGTM

[hvitved]

Rebased to resolve merge conflicts.

[michaelnebel]

C# still looks good to me.

[hvitved]

Had to fix another semantic merge conflict.

[aibaars]

Looks good to me.

[aibaars]

When reading the comment explaining this predicate , I had expected it to be recursive. I see you take the transitive closure later.