steakhal wrote:
> I agree with @steakhal that it would be better to eagerly avoid entering
> these confusing STL library functions instead of the visitor-based lazy
> solution (which is proposed in this patch and is already applied to for
> suppressing other STL functions) which wastes time on analyzing these
> branches _then_ discards the results found on them. In fact I think it would
> be nice and not too difficult to also migrate the existing lazy suppression
> to an eager path pruning solution.
>
> However this patch is **a** good solution for fixing this bug, so I'm also
> not opposed to just merging it. (And then we could create a single commit
> that migrates all the suppressions to an eager model later.
>
> > > FYI checkers cannot force call semantics unfortunately, so this could be
> > > only achieved from the engine right now.
> >
> >
> > Thanks for pointing this out, so this means to me (if I understand
> > correctly), that the `evalCall` of a checker is not even a robust option.
>
> @gamesh411 I think you are misunderstanding this -- I think @steakhal only
> wanted to say that there is no callback where the checker can return a
> boolean which controls `true` = try inlining vs `false` = don't inline this
> function.
You are right. I was overcomplicating this by immediately overgeneralising,
being able to express that a function must be always inlined. This can't be
described by our current evalCall mechanism. (For example, we always want to
inline member functions of view-like objects, such as string_view or span).
> I'm pretty confident that the `evalCall` of a checker is a robust option: if
> the `evalCall` callback of a certain checker returns `true` for a certain
> `CallEvent` then (if there are no conflicting `evalCall` checkers) it is
> guaranteed to "own" that call and there won't be any alternative code paths
> where that call is handled in other ways (inlining or conservative
> evaluation).
>
> IIRC there are a few checkers that use `evalCall` to simulate something very
> similar to a conservatively evaluated call: they manually invalidate the
> arguments/globals/etc. and conjure an unknown return value. IIRC this is
> usually done in order to add some "extra sauce" that wouldn't be done by the
> normal conservative evaluation (e.g. conjuring the return value in a slightly
> unusual way) -- but theoretically it would be possible to write an `evalCall`
> that is equivalent to conservative evaluation and use this as a "never inline
> this" marker.
Yes. EvalCalling means that you model every side-effects by hand. In this case,
if the fn writes to somewhere, we need to escape them to erase any information
we gathered about them to make the analysis sound.
> A naive implementation of this approach would be problematic due to code
> duplication (the handmade fake-conservative eval must be kept synced with the
> real one), but I think it wouldn't be too difficult to give the checkers an
> API which can be used in trivial `evalCall` callbacks like
>
> ```c++
> bool MyChecker::evalCall(const CallEvent &Call, CheckerContext &C) const {
> if (thisCallIsRelevantForMe(Call)) {
> justPerformConservativeEvaluation(Call, C);
> return true;
> }
> }
> ```
>
> If we implement this `justPerformConservativeEvaluation()` (and find a
> non-joke name for it) then there is no need to introduce a new separate
> callback that would control/prevent inlining -- so we can just write a hidden
> modeling PruneConfusingPathsInSTLChecker that uses an `evalCall` callback to
> force conservative evaluation of the problematic implementation detail
> functions that are confusing for the analyzer.
Invalidation is nothing complicated. Enumerate the arguments and escape them.
This is all the default eval call does anyway.
```c++
bool evalCall(const CallEvent &Call, CheckerContext &C) const {
ProgramStateRef State = C.getState();
State = Call.invalidateRegions(C.getBlockID(), State);
// Of course, use the checker's getDebugName(), and have this as a field of
the checker.
static SimpleProgramPointTag Tag("ForcedOpaqueCallChecker", "Forced Opaque
Call");
C.addTransition(State, C.getPredecessor(), &Tag);
return true;
}
```
https://github.com/llvm/llvm-project/pull/177804
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
