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

Artem Dergachev via cfe-commits cfe-commits at lists.llvm.org
Fri Dec 21 18:06:52 PST 2018


Author: dergachev
Date: Fri Dec 21 18:06:51 2018
New Revision: 349984

URL: http://llvm.org/viewvc/llvm-project?rev=349984&view=rev
Log:
[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

Added:
    cfe/trunk/test/Analysis/svalbuilder-float-cast.c
Modified:
    cfe/trunk/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp
    cfe/trunk/test/Analysis/casts.c
    cfe/trunk/test/Analysis/casts.cpp
    cfe/trunk/test/Analysis/expr-inspection.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp?rev=349984&r1=349983&r2=349984&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp Fri Dec 21 18:06:51 2018
@@ -321,11 +321,6 @@ void ExprInspectionChecker::analyzerDeno
     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
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp?rev=349984&r1=349983&r2=349984&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp Fri Dec 21 18:06:51 2018
@@ -402,6 +402,17 @@ SVal StoreManager::CastRetrievedVal(SVal
   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

Modified: cfe/trunk/test/Analysis/casts.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/casts.c?rev=349984&r1=349983&r2=349984&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/casts.c (original)
+++ cfe/trunk/test/Analysis/casts.c Fri Dec 21 18:06:51 2018
@@ -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;
+}
+

Modified: cfe/trunk/test/Analysis/casts.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/casts.cpp?rev=349984&r1=349983&r2=349984&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/casts.cpp (original)
+++ cfe/trunk/test/Analysis/casts.cpp Fri Dec 21 18:06:51 2018
@@ -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

Modified: cfe/trunk/test/Analysis/expr-inspection.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/expr-inspection.cpp?rev=349984&r1=349983&r2=349984&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/expr-inspection.cpp (original)
+++ cfe/trunk/test/Analysis/expr-inspection.cpp Fri Dec 21 18:06:51 2018
@@ -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}}
 }

Added: cfe/trunk/test/Analysis/svalbuilder-float-cast.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/svalbuilder-float-cast.c?rev=349984&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/svalbuilder-float-cast.c (added)
+++ cfe/trunk/test/Analysis/svalbuilder-float-cast.c Fri Dec 21 18:06:51 2018
@@ -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);
+}




More information about the cfe-commits mailing list