[analyzer] pr38668: Do not attempt to cast loaded integers to floats.

This patch is a different approach to landing the reverted r349701.

It is expected to have the same object (memory region) treated as if it has
different types in different program points. The correct behavior for
RegionStore when an object is stored as an object of type T1 but loaded as
an object of type T2 is to store the object as if it has type T1 but cast it
to T2 during load.

Note that the cast here is some sort of a "reinterpret_cast" (even in C). For
instance, if you store an integer and load a float, you won't get your integer
represented as a float; instead, you will get garbage.

Admit that we cannot perform the cast and return an unknown value.

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

rdar://problem/45062567

llvm-svn: 349984

6 files changed, 97 insertions, 11 deletions

diff --git a/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
index 742266bfe60..2553f54bbca 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
@@ -321,11 +321,6 @@ void ExprInspectionChecker::analyzerDenote(const CallExpr *CE,
     return;
   }
 
-  if (!isa<SymbolData>(Sym)) {
-    reportBug("Not an atomic symbol", C);
-    return;
-  }
-
   const auto *E = dyn_cast<StringLiteral>(CE->getArg(1)->IgnoreParenCasts());
   if (!E) {
     reportBug("Not a string literal", C);
@@ -345,7 +340,7 @@ class SymbolExpressor
 public:
   SymbolExpressor(ProgramStateRef State) : State(State) {}
 
-  Optional<std::string> VisitSymExpr(const SymExpr *S) {
+  Optional<std::string> lookup(const SymExpr *S) {
     if (const StringLiteral *const *SLPtr = State->get<DenotedSymbols>(S)) {
       const StringLiteral *SL = *SLPtr;
       return std::string(SL->getBytes());
@@ -353,8 +348,14 @@ public:
     return None;
   }
 
+  Optional<std::string> VisitSymExpr(const SymExpr *S) {
+    return lookup(S);
+  }
+
   Optional<std::string> VisitSymIntExpr(const SymIntExpr *S) {
-    if (auto Str = Visit(S->getLHS()))
+    if (Optional<std::string> Str = lookup(S))
+      return Str;
+    if (Optional<std::string> Str = Visit(S->getLHS()))
       return (*Str + " " + BinaryOperator::getOpcodeStr(S->getOpcode()) + " " +
               std::to_string(S->getRHS().getLimitedValue()) +
               (S->getRHS().isUnsigned() ? "U" : ""))
@@ -363,12 +364,22 @@ public:
   }
 
   Optional<std::string> VisitSymSymExpr(const SymSymExpr *S) {
-    if (auto Str1 = Visit(S->getLHS()))
-      if (auto Str2 = Visit(S->getRHS()))
+    if (Optional<std::string> Str = lookup(S))
+      return Str;
+    if (Optional<std::string> Str1 = Visit(S->getLHS()))
+      if (Optional<std::string> Str2 = Visit(S->getRHS()))
         return (*Str1 + " " + BinaryOperator::getOpcodeStr(S->getOpcode()) +
                 " " + *Str2).str();
     return None;
   }
+
+  Optional<std::string> VisitSymbolCast(const SymbolCast *S) {
+    if (Optional<std::string> Str = lookup(S))
+      return Str;
+    if (Optional<std::string> Str = Visit(S->getOperand()))
+      return (Twine("(") + S->getType().getAsString() + ")" + *Str).str();
+    return None;
+  }
 };
 } // namespace
 
diff --git a/clang/lib/StaticAnalyzer/Core/Store.cpp b/clang/lib/StaticAnalyzer/Core/Store.cpp
index 794fd843647..4fa937d9658 100644
--- a/clang/lib/StaticAnalyzer/Core/Store.cpp
+++ b/clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -402,6 +402,17 @@ SVal StoreManager::CastRetrievedVal(SVal V, const TypedValueRegion *R,
   if (castTy.isNull() || V.isUnknownOrUndef())
     return V;
 
+  // The dispatchCast() call below would convert the int into a float.
+  // What we want, however, is a bit-by-bit reinterpretation of the int
+  // as a float, which usually yields nothing garbage. For now skip casts
+  // from ints to floats.
+  // TODO: What other combinations of types are affected?
+  if (castTy->isFloatingType()) {
+    SymbolRef Sym = V.getAsSymbol();
+    if (Sym && !Sym->getType()->isFloatingType())
+      return UnknownVal();
+  }
+
   // When retrieving symbolic pointer and expecting a non-void pointer,
   // wrap them into element regions of the expected type if necessary.
   // SValBuilder::dispatchCast() doesn't do that, but it is necessary to
diff --git a/clang/test/Analysis/casts.c b/clang/test/Analysis/casts.c
index 45ce1940dfa..2d96aa3571d 100644
--- a/clang/test/Analysis/casts.c
+++ b/clang/test/Analysis/casts.c
@@ -213,3 +213,35 @@ void no_crash_on_symsym_cast_to_long() {
 }
 
 #endif
+
+char no_crash_SymbolCast_of_float_type_aux(int *p) {
+  *p += 1;
+  return *p;
+}
+
+void no_crash_SymbolCast_of_float_type() {
+  extern float x;
+  char (*f)() = no_crash_SymbolCast_of_float_type_aux;
+  f(&x);
+}
+
+double no_crash_reinterpret_double_as_int(double a) {
+  *(int *)&a = 1;
+  return a * a;
+}
+
+double no_crash_reinterpret_double_as_ptr(double a) {
+  *(void **)&a = 0;
+  return a * a;
+}
+
+double no_crash_reinterpret_double_as_sym_int(double a, int b) {
+  *(int *)&a = b;
+  return a * a;
+}
+
+double no_crash_reinterpret_double_as_sym_ptr(double a, void * b) {
+  *(void **)&a = b;
+  return a * a;
+}
+
diff --git a/clang/test/Analysis/casts.cpp b/clang/test/Analysis/casts.cpp
index e920bd96da0..aa2bd9c1fad 100644
--- a/clang/test/Analysis/casts.cpp
+++ b/clang/test/Analysis/casts.cpp
@@ -102,3 +102,15 @@ void foo(VeryOpaqueRef ORef) {
   castToDerived(reinterpret_cast<Transparent *>(ORef))->getNotInt();
 }
 } // namespace base_to_derived_opaque_class
+
+namespace bool_to_nullptr {
+struct S {
+  int *a[1];
+  bool b;
+};
+void foo(S s) {
+  s.b = true;
+  for (int i = 0; i < 2; ++i)
+    (void)(s.a[i] != nullptr); // no-crash
+}
+} // namespace bool_to_nullptr
diff --git a/clang/test/Analysis/expr-inspection.cpp b/clang/test/Analysis/expr-inspection.cpp
index 28f35b3eace..609b44ca6d4 100644
--- a/clang/test/Analysis/expr-inspection.cpp
+++ b/clang/test/Analysis/expr-inspection.cpp
@@ -24,7 +24,7 @@ void foo(int x, unsigned y) {
   clang_analyzer_denote(1, "$z");     // expected-warning{{Not a symbol}}
   clang_analyzer_express(1);     // expected-warning{{Not a symbol}}
 
-  clang_analyzer_denote(x + 1, "$w"); // expected-warning{{Not an atomic symbol}}
-  clang_analyzer_express(x + 1); // expected-warning{{$x + 1}}
+  clang_analyzer_denote(x + 1, "$w");
+  clang_analyzer_express(x + 1); // expected-warning{{$w}}
   clang_analyzer_express(y + 1); // expected-warning{{$y + 1U}}
 }
diff --git a/clang/test/Analysis/svalbuilder-float-cast.c b/clang/test/Analysis/svalbuilder-float-cast.c
new file mode 100644
index 00000000000..0f5760cbfca
--- /dev/null
+++ b/clang/test/Analysis/svalbuilder-float-cast.c
@@ -0,0 +1,20 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker debug.ExprInspection -verify %s
+void clang_analyzer_denote(int, const char *);
+void clang_analyzer_express(int);
+
+void SymbolCast_of_float_type_aux(int *p) {
+  *p += 0;
+  // FIXME: Ideally, all unknown values should be symbolicated.
+  clang_analyzer_denote(*p, "$x"); // expected-warning{{Not a symbol}}
+
+  *p += 1;
+  // This should NOT be (float)$x + 1. Symbol $x was never casted to float.
+  // FIXME: Ideally, this should be $x + 1.
+  clang_analyzer_express(*p); // expected-warning{{Not a symbol}}
+}
+
+void SymbolCast_of_float_type() {
+  extern float x;
+  void (*f)() = SymbolCast_of_float_type_aux;
+  f(&x);
+}