]> granicus.if.org Git - clang/commitdiff
[analyzer] pr38668: Do not attempt to cast loaded values of non-scalar types.
authorArtem Dergachev <artem.dergachev@gmail.com>
Wed, 19 Dec 2018 23:48:44 +0000 (23:48 +0000)
committerArtem Dergachev <artem.dergachev@gmail.com>
Wed, 19 Dec 2018 23:48:44 +0000 (23:48 +0000)
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 a float and load an integer, you won't have your float
rounded to an integer; instead, you will have garbage.

Admit that we cannot perform the cast as long as types we're dealing with are
non-trivial (neither integers, nor pointers).

Of course, if the cast is not necessary (eg, T1 == T2), we can still load the
value just fine.

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

rdar://problem/45062567

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@349701 91177308-0d34-0410-b5e6-96231b3b80d8

lib/StaticAnalyzer/Core/Store.cpp
test/Analysis/casts.c
test/Analysis/pointer-to-member.cpp

index 794fd843647b8bf994e2b84bc9f32e667973693c..ac9cb4bc425df08fd60a4afe113f359430a9ec6a 100644 (file)
@@ -394,14 +394,28 @@ SVal StoreManager::attemptDownCast(SVal Base, QualType TargetType,
   return UnknownVal();
 }
 
+static bool isScalarEnoughToAttemptACast(QualType T) {
+  return T->isIntegralOrEnumerationType() || T->isAnyPointerType() ||
+         T->isReferenceType();
+}
+
 /// CastRetrievedVal - Used by subclasses of StoreManager to implement
 ///  implicit casts that arise from loads from regions that are reinterpreted
 ///  as another region.
 SVal StoreManager::CastRetrievedVal(SVal V, const TypedValueRegion *R,
-                                    QualType castTy) {
-  if (castTy.isNull() || V.isUnknownOrUndef())
+                                    QualType CastTy) {
+  if (CastTy.isNull() || V.isUnknownOrUndef())
     return V;
 
+  QualType OrigTy = R->getValueType();
+
+  if (!isScalarEnoughToAttemptACast(OrigTy) ||
+      !isScalarEnoughToAttemptACast(CastTy)) {
+    if (OrigTy.getUnqualifiedType() == CastTy.getUnqualifiedType())
+      return V;
+    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
@@ -410,13 +424,13 @@ SVal StoreManager::CastRetrievedVal(SVal V, const TypedValueRegion *R,
   // We might need to do that for non-void pointers as well.
   // FIXME: We really need a single good function to perform casts for us
   // correctly every time we need it.
-  if (castTy->isPointerType() && !castTy->isVoidPointerType())
+  if (CastTy->isPointerType() && !CastTy->isVoidPointerType())
     if (const auto *SR = dyn_cast_or_null<SymbolicRegion>(V.getAsRegion()))
       if (SR->getSymbol()->getType().getCanonicalType() !=
-          castTy.getCanonicalType())
-        return loc::MemRegionVal(castRegion(SR, castTy));
+          CastTy.getCanonicalType())
+        return loc::MemRegionVal(castRegion(SR, CastTy));
 
-  return svalBuilder.dispatchCast(V, castTy);
+  return svalBuilder.dispatchCast(V, CastTy);
 }
 
 SVal StoreManager::getLValueFieldOrIvar(const Decl *D, SVal Base) {
index 45ce1940dfaebbb68ba13248e908402e3fe92491..dd14b8a6e30cf199675ee767857e5a344eb238ea 100644 (file)
@@ -213,3 +213,14 @@ 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);
+}
index 65882527d2da2574757970a0dac4d52d9061c87c..7f1965584002b773ce007c788a738b88816d27e4 100644 (file)
@@ -253,11 +253,10 @@ void test() {
   clang_analyzer_eval(&A::y); // expected-warning{{TRUE}}
   clang_analyzer_eval(&A::z); // expected-warning{{TRUE}}
 
-  // FIXME: These should be true.
   int A::*l = &A::x, A::*m = &A::y, A::*n = &A::z;
-  clang_analyzer_eval(l); // expected-warning{{UNKNOWN}}
-  clang_analyzer_eval(m); // expected-warning{{UNKNOWN}}
-  clang_analyzer_eval(n); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(l); // expected-warning{{TRUE}}
+  clang_analyzer_eval(m); // expected-warning{{TRUE}}
+  clang_analyzer_eval(n); // expected-warning{{TRUE}}
 
   // FIXME: These should be true as well.
   A a;