[clang] [clang][analyzer] Suppress false positives in std::stable_sort and st… (PR #177804)

[Balázs Benics via cfe-commits]

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