[PATCH] D117754: [clang][dataflow] Intersect ExprToLoc when joining environments

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev marked an inline comment as done.
sgatev added inline comments.


================
Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:49
+  // in tests.
+  std::set<const CFGBlock *> Preds;
   Preds.insert(Block.pred_begin(), Block.pred_end());
----------------
xazax.hun wrote:
> xazax.hun wrote:
> > Are we sure that the memory addresses of CFGBlocks are stable enough for a deterministic order? Alternatively, we could use the block ids for the ordering.
> Also, could you describe where the flakiness is originated from? Naively, I'd expect that the order in which we process the predecessors should not change the results of the analysis.
You're right, using block ids for ordering is better. I updated the code.

> Also, could you describe where the flakiness is originated from?

Say we have a block `B1` with predecessors `B2` and `B3`. Let the environment of `B2` after evaluating all of its statements is `B2Env = { Expr1 -> Loc1 }` and the environment of `B3` after evaluating all of its statement is `B3Env = { Expr2 -> Loc2 }` where `ExprX -> LocX` refers to a particular mapping of storage locations to expressions.

What we want for the input environment of `B1` is `{}` because `B2Env` and `B3Env` do not contain common assignments of storage locations to expressions. What we got before this patch is either `B2Env.join(B3Env) = { Expr1 -> Loc1 }` or `B3Env.join(B2Env) = { Expr2 -> Loc2 }`.

Without deterministic ordering of predecessors the test that I'm introducing in this patch is flaky.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117754/new/

https://reviews.llvm.org/D117754