[analyzer] LValueToRValueBitCasts should evaluate to an r-value

[steakhal]

Summary

Previously LValueToRValueBitCasts were modeled in the same way how
a regular BitCast was. However, this should not produce an l-value.
Modeling bitcasts accurately is tricky, so it's probably better to
model this expression by binding a fresh conjured value.

The following code should not result in a diagnostic:

  __attribute__((always_inline))
  static inline constexpr unsigned int_castf32_u32(float __A) {
	return __builtin_bit_cast(unsigned int, __A); // no-warning
  }

Previously, it reported
Address of stack memory associated with local variable '__A' returned to caller [core.StackAddressEscape].

[NoQ]

Thanks, nice! More stuff to support.

clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp

307–308

How difficult would it be to handle the cast properly by adding it to this branch, except eventually passing the right type to getSVal(R, ...) inside evalLoad()?

544

You didn't start it but that's, ugh, not a great name for the function to handle the new cast.

I wouldn't mind eliminating the function entirely and inlining the implementation as it actually makes the code easier to read, for once.

[steakhal]

Evaluate LValueToRValueBitCasts the same way as we evaluate LValueToRValue casts.
Extended the tests accordingly.

Thanks for the hint @NoQ!
It looks much better now.

[steakhal]

clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp

544

I'm going to do that in an NFC follow-up patch.

[steakhal]

clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp

544

D105125

[NoQ]

So it worked out of the box? Great!

[steakhal]

In D105017#2848465, @NoQ wrote:

So it worked out of the box? Great!

Yes, it did.

[vsavchenko]

Oh, wow, that's awesome! Thanks!

clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp

 }
 
 void ExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex,
                            ExplodedNode *Pred, ExplodedNodeSet &Dst) {
 
   ExplodedNodeSet dstPreStmt;
   getCheckerManager().runCheckersForPreStmt(dstPreStmt, Pred, CastE, *this);
 
-  if (CastE->getCastKind() == CK_LValueToRValue) {
+  if (CastE->getCastKind() == CK_LValueToRValue ||
+      CastE->getCastKind() == CK_LValueToRValueBitCast) {
     for (ExplodedNodeSet::iterator I = dstPreStmt.begin(), E = dstPreStmt.end();
          I!=E; ++I) {
       ExplodedNode *subExprNode = *I;
       ProgramStateRef state = subExprNode->getState();
       const LocationContext *LCtx = subExprNode->getLocationContext();
       evalLoad(Dst, CastE, CastE, subExprNode, state, state->getSVal(Ex, LCtx));
     }
     return;
for (ExplodedNodeSet::iterator I = dstPreStmt.begin(), E = dstPreStmt.end();
        I != E; ++I) {
 
     Pred = *I;
     ProgramStateRef state = Pred->getState();
     const LocationContext *LCtx = Pred->getLocationContext();
 
     switch (CastE->getCastKind()) {
       case CK_LValueToRValue:
+      case CK_LValueToRValueBitCast:
         llvm_unreachable("LValueToRValue casts handled earlier.");
       case CK_ToVoid:
         continue;
         // The analyzer doesn't do anything special with these casts,
         // since it understands retain/release semantics already.
       case CK_ARCProduceObject:
       case CK_ARCConsumeObject:
       case CK_ARCReclaimReturnedObject:
switch (CastE->getCastKind()) {
         // Explicitly proceed with default handler for this case cascade.
         state =
             handleLValueBitCast(state, Ex, LCtx, T, ExTy, CastE, Bldr, Pred);
         continue;
       }
       case CK_Dependent:
       case CK_ArrayToPointerDecay:
       case CK_BitCast:
-      case CK_LValueToRValueBitCast:
       case CK_AddressSpaceConversion:
       case CK_BooleanToSignedIntegral:
       case CK_IntegralToPointer:
       case CK_PointerToIntegral: {
         SVal V = state->getSVal(Ex, LCtx);
         if (V.getAs<nonloc::PointerToMember>()) {
           state = state->BindExpr(CastE, LCtx, UnknownVal());
           Bldr.generateNode(CastE, Pred, state);
switch (CastE->getCastKind()) {
         }
         // Explicitly proceed with default handler for this case cascade.
         state = handleLVectorSplat(state, LCtx, CastE, Bldr, Pred);
         continue;
       }
       // Various C++ casts that are not handled yet.
       case CK_ToUnion:
       case CK_VectorSplat: {
         state = handleLVectorSplat(state, LCtx, CastE, Bldr, Pred);
         continue;
       }
       case CK_MatrixCast: {
         // TODO: Handle MatrixCast here.
         continue;
       }
     }
   }

clang/test/Analysis/builtin_bitcast.cpp

+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-unknown -verify %s \
+// RUN:   -analyzer-checker=core,debug.ExprInspection
+
+template <typename T> void clang_analyzer_dump(T);
+
+__attribute__((always_inline)) static inline constexpr unsigned int _castf32_u32(float __A) {
+  return __builtin_bit_cast(unsigned int, __A); // no-warning
+}
+
+void test(int i) {
+  _castf32_u32(42);
+
+  float f = 42;
+
+  // Loading from a floating point value results in unknown,
+  // which later materializes as a conjured value.
+  auto g = __builtin_bit_cast(unsigned int, f);
+  clang_analyzer_dump(g);
+  // expected-warning-re@-1 {{{{^conj_\$[0-9]+{unsigned int,}}}}
+
+  auto g2 = __builtin_bit_cast(unsigned int, 42.0f);
+  clang_analyzer_dump(g2);
+  // expected-warning-re@-1 {{{{^conj_\$[0-9]+{unsigned int,}}}}
+
+  auto g3 = __builtin_bit_cast(unsigned int, i);
+  clang_analyzer_dump(g3);
+  // expected-warning-re@-1 {{{{^reg_\$[0-9]+<int i>}}}}
+
+  auto g4 = __builtin_bit_cast(unsigned long, &i);
+  clang_analyzer_dump(g4);
+  // expected-warning@-1 {{&i [as 64 bit integer]}}
+}