[analyzer] cover more cases where a Loc can be bound to constants

[r.stahl]

Summary

• SValBuilder::getConstantVal stopped processing an initialization if there are intermediate casts. Now additionally handles CStyleCastExpr, CXXConstCastExpr, CXXReinterpretCastExpr, CXXStaticCastExpr, CXXFunctionalCastExpr. Additional CastKinds are CK_NoOp and CK_IntegralToPointer - the others are already handled by EvaluateAsInt.
• RegionStoreManager::getBindingForElement now resolves a constant array initialization.
• RegionStoreManager::getBindingForField now resolves constant in-class initializers and constant element-wise initialization.
• Added tests for all new cases.

[r.stahl]

Not sure how to proceed about these:

• CXXDynamicCastExpr: I don't think evalCast would handle this correctly, does it?
• ObjCBridgedCastExpr: I don't know ObjectiveC
• CastKinds for floating point: Floats cannot yet be handled by the analyzer, right?

[MTC]

Test files for initialization missing? : )

[NoQ]

Looks great, thanks!

CXXDynamicCastExpr: I don't think evalCast would handle this correctly, does it?

Seems so. It is partially supported by StoreManager::attemptDownCast(). I also see relatively little urgency in handling it here because the whole point of dynamic_cast is to be unknown in compile-time; why would anybody need to dynamic_cast a compile-time null pointer?

Also for what-does-what sorts of answers you can consult the code in ExprEngine::Visit* which is essentially a huge switch by statement kinds.

ObjCBridgedCastExpr: I don't know ObjectiveC

Same here, it's quite pointless to have a constant null pointer casted from one language to another.

CastKinds for floating point: Floats cannot yet be handled by the analyzer, right?

Yep, that's right.

In D45774#1071666, @MTC wrote:

Test files for initialization missing? : )

Yep, seems so. This is stuff like

struct S {
  int x = 0;
};

I'm not sure it even needs to be supported explicitly. If the field is static, it most likely acts like a simple global variable (i.e. a VarDecl, not a FieldDecl). If the field is non-static, we should pick-up the in-class initializer in the constructor. Also if it's non-static and const, it is unlikely to have a constant initializer, because why would anybody copy the same constant into every instance of the class.

lib/StaticAnalyzer/Core/RegionStore.cpp

1638

Using const auto * is recommended with dyn_cast (also a plain auto with getAs and such), so that not to write down the type twice. We have a lot of old code around, but the new code should probably follow this recommendation.

1642–1645

Hmm, we might as well want to return an UndefinedVal() when the index is out of bounds of the actual array. The size of the array can be retrieved from VD. Though if you decide to do that, please put it into a separate patch :)

1721

This method crashes when the index is out of range. Not sure - is it possible to skip a few fields in the list? Would the list's AST automatically contain any placeholders in this case?

[george.karpenkov]

lib/StaticAnalyzer/Core/RegionStore.cpp

1718

style note -- by LLVM coding standards {'s are usually omitted when there's only a single statement inside the guard. Would help to avoid giant towers of }'s

[r.stahl]

addressed the comments, thanks!

If it looks good, please commit for me.

[r.stahl]

lib/StaticAnalyzer/Core/RegionStore.cpp

1721

Good catch, but I don't think this is an issue. When testing InitListExpr with missing inits, they get filled with ImplicitValueInitExpr.

[NoQ]

Yup, thanks! I'll commit.

[NoQ]

Two questions for the future:

• Should we skip the initializer binding for local variables (and fields/elements of local variables) entirely? Cause we can load from them anyway. And keeping the Store small might be good for performance.

• Just in case, do you accidentally plan supporting nested initializer lists?

[xazax.hun]

lib/StaticAnalyzer/Core/RegionStore.cpp

1642–1645

+1 for the UndefVal patch :)

[r.stahl]

In D45774#1088453, @NoQ wrote:

Two questions for the future:

• Should we skip the initializer binding for local variables (and fields/elements of local variables) entirely? Cause we can load from them anyway. And keeping the Store small might be good for performance.

I don't think that I can judge this.

• Just in case, do you accidentally plan supporting nested initializer lists?

Not at the moment.

lib/StaticAnalyzer/Core/RegionStore.cpp

SVal RegionStoreManager::getBindingForElement(RegionBindingsConstRef B,
 
   // Check if the region has a binding.
   if (const Optional<SVal> &V = B.getDirectBinding(R))
     return *V;
 
   const MemRegion* superR = R->getSuperRegion();
 
   // Check if the region is an element region of a string literal.
-  if (const StringRegion *StrR=dyn_cast<StringRegion>(superR)) {
+  if (const StringRegion *StrR = dyn_cast<StringRegion>(superR)) {
     // FIXME: Handle loads from strings where the literal is treated as
     // an integer, e.g., *((unsigned int*)"hello")
     QualType T = Ctx.getAsArrayType(StrR->getValueType())->getElementType();
     if (!Ctx.hasSameUnqualifiedType(T, R->getElementType()))
       return UnknownVal();
 
     const StringLiteral *Str = StrR->getStringLiteral();
     SVal Idx = R->getIndex();
     if (Optional<nonloc::ConcreteInt> CI = Idx.getAs<nonloc::ConcreteInt>()) {
       int64_t i = CI->getValue().getSExtValue();
       // Abort on string underrun.  This can be possible by arbitrary
       // clients of getBindingForElement().
       if (i < 0)
         return UndefinedVal();
       int64_t length = Str->getLength();
       // Technically, only i == length is guaranteed to be null.
       // However, such overflows should be caught before reaching this point;
       // the only time such an access would be made is if a string literal was
       // used to initialize a larger array.
       char c = (i >= length) ? '\0' : Str->getCodeUnit(i);
       return svalBuilder.makeIntVal(c, T);
     }
+  } else if (const VarRegion *VR = dyn_cast<VarRegion>(superR)) {
+    // Check if the containing array is const and has an initialized value.
+    const VarDecl *VD = VR->getDecl();
+    // Either the array or the array element has to be const.
+    if (VD->getType().isConstQualified() || R->getElementType().isConstQualified()) {
+      if (const Expr *Init = VD->getInit()) {
+        if (const auto *InitList = dyn_cast<InitListExpr>(Init)) {
+          // The array index has to be known.
+          if (auto CI = R->getIndex().getAs<nonloc::ConcreteInt>()) {
+            int64_t i = CI->getValue().getSExtValue();
+            // Return unknown value if index is out of bounds.
+            if (i < 0 || i >= InitList->getNumInits())
+              return UnknownVal();
+
+            if (const Expr *ElemInit = InitList->getInit(i))
+              if (Optional<SVal> V = svalBuilder.getConstantVal(ElemInit))
+                return *V;
+          }
+        }
+      }
+    }
   }
 
   // Check for loads from a code text region.  For such loads, just give up.
   if (isa<CodeTextRegion>(superR))
     return UnknownVal();
 
   // Handle the case where we are indexing into a larger scalar object.
   // For example, this handles:

 
 SVal RegionStoreManager::getBindingForField(RegionBindingsConstRef B,
                                             const FieldRegion* R) {
 
   // Check if the region has a binding.
   if (const Optional<SVal> &V = B.getDirectBinding(R))
     return *V;
 
-  QualType Ty = R->getValueType();
+  // Is the field declared constant and has an in-class initializer?
+  const FieldDecl *FD = R->getDecl();
+  QualType Ty = FD->getType();
+  if (Ty.isConstQualified())
+    if (const Expr *Init = FD->getInClassInitializer())
+      if (Optional<SVal> V = svalBuilder.getConstantVal(Init))
+        return *V;
+
+  // If the containing record was initialized, try to get its constant value.
+  const MemRegion* superR = R->getSuperRegion();
+  if (const auto *VR = dyn_cast<VarRegion>(superR)) {
+    const VarDecl *VD = VR->getDecl();
+    QualType RecordVarTy = VD->getType();
+    // Either the record variable or the field has to be const qualified.
+    if (RecordVarTy.isConstQualified() || Ty.isConstQualified())
+      if (const Expr *Init = VD->getInit())
+        if (const auto *InitList = dyn_cast<InitListExpr>(Init))
+          if (const Expr *FieldInit = InitList->getInit(FD->getFieldIndex()))
+            if (Optional<SVal> V = svalBuilder.getConstantVal(FieldInit))
+              return *V;
+  }
+
   return getBindingForFieldOrElementCommon(B, R, Ty);
 }
 
 Optional<SVal>
 RegionStoreManager::getBindingForDerivedDefaultValue(RegionBindingsConstRef B,
                                                      const MemRegion *superR,
                                                      const TypedValueRegion *R,
                                                      QualType Ty) {

lib/StaticAnalyzer/Core/SValBuilder.cpp

 }
 
 DefinedOrUnknownSVal
 SValBuilder::getRegionValueSymbolVal(const TypedValueRegion *region) {
   QualType T = region->getValueType();
 
   if (T->isNullPtrType())
     return makeZeroVal(T);
-  
   if (!SymbolManager::canSymbolicate(T))
     return UnknownVal();
 
   SymbolRef sym = SymMgr.getRegionValueSymbol(region);
 
   if (Loc::isLocType(T))
     return loc::MemRegionVal(MemMgr.getSymbolicRegion(sym));
 
case Stmt::IntegerLiteralClass:
     return makeIntVal(cast<IntegerLiteral>(E));
 
   case Stmt::ObjCBoolLiteralExprClass:
     return makeBoolVal(cast<ObjCBoolLiteralExpr>(E));
 
   case Stmt::CXXNullPtrLiteralExprClass:
     return makeNull();
 
+  case Stmt::CStyleCastExprClass:
+  case Stmt::CXXFunctionalCastExprClass:
+  case Stmt::CXXConstCastExprClass:
+  case Stmt::CXXReinterpretCastExprClass:
+  case Stmt::CXXStaticCastExprClass:
   case Stmt::ImplicitCastExprClass: {
     const auto *CE = cast<CastExpr>(E);
     switch (CE->getCastKind()) {
     default:
       break;
     case CK_ArrayToPointerDecay:
+    case CK_IntegralToPointer:
+    case CK_NoOp:
     case CK_BitCast: {
       const Expr *SE = CE->getSubExpr();
       Optional<SVal> Val = getConstantVal(SE);
       if (!Val)
         return None;
       return evalCast(*Val, CE->getType(), SE->getType());
     }
     }

test/Analysis/globals.cpp

+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+
+
+static const unsigned long long scull = 0;
+void static_int()
+{
+    *(int*)scull = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+const unsigned long long cull = 0;
+void const_int()
+{
+    *(int*)cull = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+static int * const spc = 0;
+void static_ptr()
+{
+    *spc = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+int * const pc = 0;
+void const_ptr()
+{
+    *pc = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+const unsigned long long cull_nonnull = 4;
+void nonnull_int()
+{
+    *(int*)(cull_nonnull - 4) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+int * const pc_nonnull = (int*)sizeof(int);
+void nonnull_ptr()
+{
+    *(pc_nonnull - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+int * const constcast = const_cast<int * const>((int*)sizeof(int));
+void cast1()
+{
+    *(constcast - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+int * const recast = reinterpret_cast<int*>(sizeof(int));
+void cast2()
+{
+    *(recast - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+int * const staticcast = static_cast<int * const>((int*)sizeof(int));
+void cast3()
+{
+    *(staticcast - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+struct Foo { int a; };
+Foo * const dyncast = dynamic_cast<Foo * const>((Foo*)sizeof(Foo));
+void cast4()
+{
+    // Do not handle dynamic_cast for now, because it may change the pointer value.
+    (dyncast - 1)->a = 0; // no-warning
+}
+
+typedef int * const intptrconst;
+int * const funccast = intptrconst(sizeof(int));
+void cast5()
+{
+    *(funccast - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+struct S1
+{
+    int * p;
+};
+const S1 s1 = {
+    .p = (int*)sizeof(int)
+};
+void conststruct()
+{
+    *(s1.p - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+struct S2
+{
+    int * const p;
+};
+S2 s2 = {
+    .p = (int*)sizeof(int)
+};
+void constfield()
+{
+    *(s2.p - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+int * const parr[1] = { (int*)sizeof(int) };
+void constarr()
+{
+    *(parr[0] - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+struct S3
+{
+    int * p = (int*)sizeof(int);
+};
+void recordinit()
+{
+    S3 s3;
+    *(s3.p - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}