[clang] [WIP] [analyzer] Refactor MallocChecker to use `BindExpr` in `evalCall` (PR #106081)

[Donát Nagy via cfe-commits]

================
@@ -1854,28 +1945,27 @@ static ProgramStateRef MallocUpdateRefState(CheckerContext &C, const Expr *E,
 
   // Get the return value.
   if (!RetVal)
-    RetVal = C.getSVal(E);
+    RetVal = State->getSVal(E, C.getLocationContext());
 
   // We expect the malloc functions to return a pointer.
   if (!RetVal->getAs<Loc>())
     return nullptr;
 
   SymbolRef Sym = RetVal->getAsLocSymbol();
 
-  // This is a return value of a function that was not inlined, such as malloc()
-  // or new(). We've checked that in the caller. Therefore, it must be a symbol.
-  assert(Sym);
-  // FIXME: In theory this assertion should fail for `alloca()` calls (because
-  // `AllocaRegion`s are not symbolic); but in practice this does not happen.
+  // FIXME: Following if fails for `alloca()` calls (because
+  // `AllocaRegion`s are not symbolic);
   // As the current code appears to work correctly, I'm not touching this issue
   // now, but it would be good to investigate and clarify this.
   // Also note that perhaps the special `AllocaRegion` should be replaced by
   // `SymbolicRegion` (or turned into a subclass of `SymbolicRegion`) to enable
   // proper tracking of memory allocated by `alloca()` -- and after that change
   // this assertion would become valid again.
----------------
NagyDonat wrote:

```suggestion
  // NOTE: If this was an `alloca()` call, then `RetVal` holds an
  // `AllocaRegion`, so `Sym` will be a nullpointer because `AllocaRegion`s do
  // not have an associated symbol. However, this distinct region type means
  // that we don't need to store anything about them in `RegionState`.
```
I'm happy to see that this FIXME comment (which was added by one of my commits a few months ago) can be finally eliminated :relieved:. At that time I was confused to see that `assert(Sym);` was did not crash after `alloca()` calls, but your refactoring experimentally proves that this was caused by the invariant-breaking logic that used `BindExpr()` from the `postCall` callback.

My suggestion rewords and mostly removes the leftover parts of my old comment block, because I'm satisfied with their resolution and no "FIXME" remains after your change.

I thought about keeping the remark that "perhaps the special `AllocaRegion` should be replaced by `SymbolicRegion` (or turned into a subclass of `SymbolicRegion`)", but I decided to remove it, because it would be completely off-topic here. (It would have provided a workaround to dodge the "why is this assert not triggered?" mystery, but now there is no motivation to mention it.)

I still have an unrelated motivation for replacing `AllocaRegion`s with regular `SymbolicRegion`s (it would improve diagnostics of some `alpha.core.ArrayBoundV2` reports, because I could place a note tag on the `alloca()` call), but there is no reason to mention that (low priority) plan here.

https://github.com/llvm/llvm-project/pull/106081