JS: Generalize global access paths to include local ones

[asger-semmle]

Renames the GlobalAccessPath module to AccessPath and generalizes it to deal with access paths relative to a given SourceNode root.

The predicates GlobalAccessPath::{fromReference,fromRhs} are helpful internally, but felt clunky as part of the public API so we now expose AccessPath::{getAReferenceTo,getAnAssignmentTo} instead.

Type tracking can step through these access paths. This is modelled as store/loads edges that step into/out of the root node using the whole access path as if it was a single property name.

This means we treat x.foo.bar and x["foo.bar"] as the same thing, which is a bit sleazy but probably fine for type tracking purposes. We could tag the strings to avoid this potential clash if we're willing to pay the overhead.

There is a modest performance cost and an equally modest number of new call edges.

[esbena]

Wow. That is a nice generalisation.
I only have two optimizer-related comments.

[esbena]

Does it make the optimizer unhappy if we use Expr::getUnderlyingValue instead of all these explicit cases? (I know this is just a port of some steps, feel free to ignore)

[asger-semmle]

I don't know about the optimizer, but it doesn't do quite the same thing. getUnderlyingValue goes all the way to the bottom in one step, whereas we generally want the individual steps here.

For example, suppose you add type casts as a sanitizer in a data flow query. In this case, getUnderlyingValue would simply skip over the type cast, bypassing the sanitizer.

[esbena]

Good point. I suppose we could look into letting Expr expose the non-recursive version as getImmediatelyUnderlyingValue in another PR.

[esbena]

Is not root.isGlobal() used to help the optimizer? Or is it just done to encourage not using DataFlow::globalAccessPathRootPseudoNode() explicitly?

[asger-semmle]

It's mainly to avoid exposing the pseudo-node, yes.

[esbena]

LGTM, but doesn't this require a full dist-compare? The type tracking improvements seem fundamental enough to me that they require a full evaluation.

[max-schaefer]

LGTM, but I agree with Esben about doing a full evaluation.

[asger-semmle]

There's an evaluation on default.slugs from a slightly earlier version but that's the security suite only. I'll do a full evaluation.

[max-schaefer]

@asger-semmle, I think you mentioned that there were new evaluation results? Or are you still rerunning?

[asger-semmle]

Here's the full evaluation. I messed up the re-run of the slowest slugs, a new one is underway.

[asger-semmle]

The re-run of the 10 slowest slugs looks much better.

[asger-semmle]

I'd like to merge this unless you have any further change requests @max-schaefer?