[PATCH] D104716: [analyzer] Fix assertion failure on code with transparent unions

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Thx a lot for the fix!



================
Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:482-483
+// them with parameters.  This function incapsulates such cases.
+static SVal process(SVal Value, const ParmVarDecl *Parameter,
+                    SValBuilder &SVB) {
+  QualType ParamType = Parameter->getType();
----------------
Maybe at least `processParameter`?


================
Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:496-497
+  if (isTransparentUnion(ParamType) &&
+      // Let's check that we indeed trying to bind different types.
+      !isTransparentUnion(Value, SVB.getContext())) {
+    BasicValueFactory &BVF = SVB.getBasicValueFactory();
----------------
I think this check could be implemented more reliably on the AST. I.e., ask what's the type of the argument expression instead.

It's typically better to query the AST types instead of `SVal` types because AST types are richer (cf. lvalue/rvalue confusion from D104550#inline-993348 - which is a very common source of bugs) and also significantly less likely to be buggy / improperly modeled. So i'm very much in favor of knowing in advance which type do we expect according to the AST and then later asserting that we got something compatible from the `SVal`.


================
Comment at: clang/test/Analysis/transparent_union_bug.c:5
+void clang_analyzer_warnIfReached();
+void clang_analyzer_printState();
+
----------------
Unused :P


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104716