[analyzer] Fix assertion failure on code with transparent unions

[vsavchenko]

Summary

rdar://76948312

[NoQ]

Thx a lot for the fix!

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

482–483

Maybe at least processParameter?

496–497

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.

clang/test/Analysis/transparent_union_bug.c

6

Unused :P

[vsavchenko]

Use argument expression's type instead of SVal's type

[vsavchenko]

Remove printState from the test

[NoQ]

Thanks again!~

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

 #include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/Store.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/ImmutableList.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/PointerIntPair.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
bool CallEvent::isVariadic(const Decl *D) {
   if (const auto *MD = dyn_cast<ObjCMethodDecl>(D))
     return MD->isVariadic();
   if (const auto *BD = dyn_cast<BlockDecl>(D))
     return BD->isVariadic();
 
   llvm_unreachable("unknown callable kind");
 }
 
+static bool isTransparentUnion(QualType T) {
+  const RecordType *UT = T->getAsUnionType();
+  return UT && UT->getDecl()->hasAttr<TransparentUnionAttr>();
+}
+
+// In some cases, symbolic cases should be transformed before we associate
+// them with parameters.  This function incapsulates such cases.
+static SVal processArgument(SVal Value, const Expr *ArgumentExpr,
+                            const ParmVarDecl *Parameter, SValBuilder &SVB) {
+  QualType ParamType = Parameter->getType();
+  QualType ArgumentType = ArgumentExpr->getType();
+
+  // Transparent unions allow users to easily convert values of union field
+  // types into union-typed objects.
+  //
+  // Also, more importantly, they allow users to define functions with different
+  // different parameter types, substituting types matching transparent union
+  // field types with the union type itself.
+  //
+  // Here, we check specifically for latter cases and prevent binding
+  // field-typed values to union-typed regions.
+  if (isTransparentUnion(ParamType) &&
+      // Let's check that we indeed trying to bind different types.
+      !isTransparentUnion(ArgumentType)) {
+    BasicValueFactory &BVF = SVB.getBasicValueFactory();
+
+    llvm::ImmutableList<SVal> CompoundSVals = BVF.getEmptySValList();
+    CompoundSVals = BVF.prependSVal(Value, CompoundSVals);
+
+    // Wrap it with compound value.
+    return SVB.makeCompoundVal(ParamType, CompoundSVals);
+  }
+
+  return Value;
+}
+
 static void addParameterValuesToBindings(const StackFrameContext *CalleeCtx,
                                          CallEvent::BindingsTy &Bindings,
                                          SValBuilder &SVB,
                                          const CallEvent &Call,
                                          ArrayRef<ParmVarDecl*> parameters) {
   MemRegionManager &MRMgr = SVB.getRegionManager();
 
   // If the function has fewer parameters than the call has arguments, we simply
   // do not bind any values to them.
   unsigned NumArgs = Call.getNumArgs();
   unsigned Idx = 0;
   ArrayRef<ParmVarDecl*>::iterator I = parameters.begin(), E = parameters.end();
   for (; I != E && Idx < NumArgs; ++I, ++Idx) {
     assert(*I && "Formal parameter has no decl?");
 
     // TODO: Support allocator calls.
     if (Call.getKind() != CE_CXXAllocator)
       if (Call.isArgumentConstructedDirectly(Call.getASTArgumentIndex(Idx)))
         continue;
 
     // TODO: Allocators should receive the correct size and possibly alignment,
     // determined in compile-time but not represented as arg-expressions,
     // which makes getArgSVal() fail and return UnknownVal.
     SVal ArgVal = Call.getArgSVal(Idx);
+    const Expr *ArgExpr = Call.getArgExpr(Idx);
     if (!ArgVal.isUnknown()) {
       Loc ParamLoc = SVB.makeLoc(
           MRMgr.getParamVarRegion(Call.getOriginExpr(), Idx, CalleeCtx));
-      Bindings.push_back(std::make_pair(ParamLoc, ArgVal));
+      Bindings.push_back(
+          std::make_pair(ParamLoc, processArgument(ArgVal, ArgExpr, *I, SVB)));
     }
   }
 
   // FIXME: Variadic arguments are not handled at all right now.
 }
 
 const ConstructionContext *CallEvent::getConstructionContext() const {
   const StackFrameContext *StackFrame = getCalleeStackFrame(0);

clang/test/Analysis/transparent_union_bug.c

+// RUN: %clang_analyze_cc1 -analyze -triple x86_64-apple-darwin10 \
+// RUN:  -analyzer-checker=core,debug.ExprInspection -verify %s
+
+void clang_analyzer_warnIfReached();
+
+typedef struct {
+  int value;
+} Struct;
+
+typedef union {
+  Struct *ptr;
+  long num;
+} __attribute__((transparent_union)) Alias;
+
+void foo(Struct *x);
+void foo(Alias y) {
+  if (y.ptr == 0) {
+    // no-crash
+  }
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+void foobar(long z);
+void foobar(Alias z) {
+  if (z.num != 42) {
+    // no-crash
+  }
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void foobaz(Alias x) {
+  if (x.ptr == 0) {
+    // no-crash
+  }
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+void bar(Struct arg) {
+  foo(&arg);
+  foobar(42);
+  foobaz(&arg);
+}