xazax.hun added inline comments.

================
Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:446
+    }
+    const bool BState = State->contains<CTUDispatchBifurcationSet>(D);
+    if (!BState) { // This is the first time we see this foreign function.
----------------
martong wrote:
> xazax.hun wrote:
> > So if we see the same foreign function called in multiple contexts, we will 
> > only queue one of the contexts for the CTU. Is this the intended design? 
> > 
> > So if I see:
> > ```
> > foreign(true);
> > foreign(false);
> > ```
> > 
> > The new CTU will only evaluate `foreign(true)` but not `foreign(false)`. 
> This is intentional.
> ```
> foreign(true);
> foreign(false);
> ```
> The new CTU will evaluate the following paths in the exploded graph:
> ```
> foreign(true); // conservative evaluated
> foreign(false); // conservative evaluated
> foreign(true); // inlined
> foreign(false); // inlined
> ```
> The point is to keep bifurcation to a minimum and avoid the exponential blow 
> up.
> So, we will never have a path like this:
> ```
> foreign(true); // conservative evaluated
> foreign(false); // inlined
> ```
> 
> Actually, this is the same strategy that we use during the dynamic dispatch 
> of C++ virtual calls. See `DynamicDispatchBifurcationMap`.
> 
> The conservative evaluation happens in the first phase, the inlining in the 
> second phase (assuming the phase1 inlining option is set to none).
> The new CTU will evaluate the following paths in the exploded graph:
> ```
> foreign(true); // conservative evaluated
> foreign(false); // conservative evaluated
> foreign(true); // inlined
> foreign(false); // inlined
> ```

When we encounter `foreign(true)`, we would add the decl to 
`CTUDispatchBifurcationSet`. So the second time we see a call to the function 
`foreign(false);`, we will just do the conservative evaluation and will not add 
the call to the CTU worklist. So how will `foreign(false);` be inlined in the 
second pass? Do I miss something? 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123773

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to