[all-commits] [llvm/llvm-project] aa12a4: [analyzer] Fix assertion failure with conflicting ...

[Balazs Benics via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: aa12a48c8223aafafa45fb1e6e9ea49dc18a62d2
      https://github.com/llvm/llvm-project/commit/aa12a48c8223aafafa45fb1e6e9ea49dc18a62d2
  Author: Balazs Benics <benicsbalazs at gmail.com>
  Date:   2022-10-26 (Wed, 26 Oct 2022)

  Changed paths:
    M clang/lib/StaticAnalyzer/Core/CallEvent.cpp
    M clang/test/Analysis/region-store.c

  Log Message:
  -----------
  [analyzer] Fix assertion failure with conflicting prototype calls

It turns out we can reach the `Init.castAs<nonlock::CompoundVal>()`
expression with other kinds of SVals. Such as by `nonloc::ConcreteInt`
in this example: https://godbolt.org/z/s4fdxrcs9

```lang=C++
int buffer[10];
void b();
void top() {
  b(&buffer);
}
void b(int *c) {
  *c = 42; // would crash
}
```
In this example, we try to store `42` to the `Elem{buffer, 0}`.

This situation can appear if the CallExpr refers to a function
declaration without prototype. In such cases, the engine will pick the
redecl of the referred function decl which has function body, hence has
a function prototype.

This weird situation will have an interesting effect to the AST, such as
the argument at the callsite will miss a cast, which would cast the
`int (*)[10]` expression into `int *`, which means that when we evaluate
the `*c = 42` expression, we want to bind `42` to an array, causing the
crash.

Look at the AST of the callsite with and without the function prototype:
https://godbolt.org/z/Gncebcbdb
The only difference is that without the proper function prototype, we
will not have the `ImplicitCastExpr` `BitCasting` from `int (*)[10]`
to `int *` to match the expected type of the parameter declaration.

In this patch, I'm proposing to emit a cast in the mentioned edge-case,
to bind the argument value of the expected type to the parameter.

I'm only proposing this if the runtime definition has exactly the same
number of parameters as the callsite feeds it by arguments.
If that's not the case, I believe, we are better off by binding `Unknown`
to those parameters.

Reviewed By: martong

Differential Revision: https://reviews.llvm.org/D136162