[analyzer] CastValueChecker: Model isa(), isa_and_nonnull()

[Charusso]

Summary

-

[Szelethus]

Whoa, the test cases look AMAZING! I admit that I haven't read much of the other code just yet, but I like the results in any case.

clang/test/Analysis/cast-value-notes.cpp

5–27 ↗

(On Diff #215908)

Hmm, we may want to move these eventually to clang/test/Inputs/, since this isn't the only LLVM checker. Thought that said, I want to see the other LLVM checker gone, like, purged from the codebase for being the abomination that it is.

Feel free to leave these here for know, just thinking aloud :)

[NoQ]

clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp

183 ↗

(On Diff #215908)

A body of a function is always either a CompoundStmt or (shockingly) a CXXTryStmt. This cast will always fail.

188 ↗

(On Diff #215908)

I suspect that DRE may still be null (eg., when calling isa through function pointer).

I think you should just lookup Call.getDecl()'s template arguments instead. It's going to be the declaration of the specific instantiation, so it'll have all the arguments.

216 ↗

(On Diff #215908)

Can you use the overload without the bug report parameter? Cause you don't seem to be using it anyway.

clang/test/Analysis/cast-value-notes.cpp

5–27 ↗

(On Diff #215908)

I agree, extracting common definitions into a system header simulator usually ends up being worth it pretty quickly.

89 ↗

(On Diff #215908)

I suggest: 'C' is a 'Circle', not a 'Triangle'.

95 ↗

(On Diff #215908)

Just 'C' is a 'Circle'.

111 ↗

(On Diff #215908)

This test looks incorrect. We shouldn't be assuming this, we already know that it's not a triangle because we know that it's a circle.

[Charusso]

clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp

216 ↗

(On Diff #215908)

I think you have forgotten to implement that scenario, but now we have that as well since D66325.

clang/test/Analysis/cast-value-notes.cpp

5–27 ↗

(On Diff #215908)

Thanks for the idea! Moved in D66325.

111 ↗

(On Diff #215908)

Let us discuss in D66325.

[Charusso]

• Simplify the template argument obtaining.
• Added a tiny new test case.

[Charusso]

Thanks for the reviews so far! I had a contradiction in my mind about regions, but now everything is okay and the notes are not misleading.

clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp

183 ↗

(On Diff #215908)

I do not remember what I did here, but that was a working solution.

188 ↗

(On Diff #215908)

That was the plan, just I did not realized I am a FunctionDecl away to achieve that. Thanks!

[NoQ]

LGTM with one minor nit!

clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp

233–235 ↗

(On Diff #216322)

Mmm, this will need to be de-duplicated out.

[Charusso]

• Fix.

[Charusso]

Thanks for the review!

cfe/trunk/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp

 // TODO list:
 // - It only allows one succesful cast between two types however in the wild
 //   the object could be casted to multiple types.
 // - It needs to check the most likely type information from the dynamic type
 //   map to increase precision of dynamic casting.
 //
 //===----------------------------------------------------------------------===//
 
+#include "clang/AST/DeclTemplate.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicType.h"
 #include "llvm/ADT/Optional.h"
 #include <utility>
 
 using namespace clang;
 using namespace ento;
 
 namespace {
 class CastValueChecker : public Checker<eval::Call> {
-  enum class CallKind { Function, Method };
+  enum class CallKind { Function, Method, InstanceOf };
 
   using CastCheck =
       std::function<void(const CastValueChecker *, const CallEvent &Call,
                          DefinedOrUnknownSVal, CheckerContext &)>;
 
 public:
   // We have five cases to evaluate a cast:
   // 1) The parameter is non-null, the return value is non-null.
   // 2) The parameter is non-null, the return value is null.
   // 3) The parameter is null, the return value is null.
   // cast: 1;  dyn_cast: 1, 2;  cast_or_null: 1, 3;  dyn_cast_or_null: 1, 2, 3.
   //
   // 4) castAs: Has no parameter, the return value is non-null.
   // 5) getAs:  Has no parameter, the return value is null or non-null.
+  //
+  // We have two cases to check the parameter is an instance of the given type.
+  // 1) isa:             The parameter is non-null, returns boolean.
+  // 2) isa_and_nonnull: The parameter is null or non-null, returns boolean.
   bool evalCall(const CallEvent &Call, CheckerContext &C) const;
   void checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const;
 
 private:
   // These are known in the LLVM project. The pairs are in the following form:
   // {{{namespace, call}, argument-count}, {callback, kind}}
   const CallDescriptionMap<std::pair<CastCheck, CallKind>> CDM = {
       {{{"llvm", "cast"}, 1},
        {&CastValueChecker::evalCast, CallKind::Function}},
       {{{"llvm", "dyn_cast"}, 1},
        {&CastValueChecker::evalDynCast, CallKind::Function}},
       {{{"llvm", "cast_or_null"}, 1},
        {&CastValueChecker::evalCastOrNull, CallKind::Function}},
       {{{"llvm", "dyn_cast_or_null"}, 1},
        {&CastValueChecker::evalDynCastOrNull, CallKind::Function}},
       {{{"clang", "castAs"}, 0},
        {&CastValueChecker::evalCastAs, CallKind::Method}},
       {{{"clang", "getAs"}, 0},
-       {&CastValueChecker::evalGetAs, CallKind::Method}}};
+       {&CastValueChecker::evalGetAs, CallKind::Method}},
+      {{{"llvm", "isa"}, 1},
+       {&CastValueChecker::evalIsa, CallKind::InstanceOf}},
+      {{{"llvm", "isa_and_nonnull"}, 1},
+       {&CastValueChecker::evalIsaAndNonNull, CallKind::InstanceOf}}};
 
   void evalCast(const CallEvent &Call, DefinedOrUnknownSVal DV,
                 CheckerContext &C) const;
   void evalDynCast(const CallEvent &Call, DefinedOrUnknownSVal DV,
                    CheckerContext &C) const;
   void evalCastOrNull(const CallEvent &Call, DefinedOrUnknownSVal DV,
                       CheckerContext &C) const;
   void evalDynCastOrNull(const CallEvent &Call, DefinedOrUnknownSVal DV,
                          CheckerContext &C) const;
   void evalCastAs(const CallEvent &Call, DefinedOrUnknownSVal DV,
                   CheckerContext &C) const;
   void evalGetAs(const CallEvent &Call, DefinedOrUnknownSVal DV,
                  CheckerContext &C) const;
+  void evalIsa(const CallEvent &Call, DefinedOrUnknownSVal DV,
+               CheckerContext &C) const;
+  void evalIsaAndNonNull(const CallEvent &Call, DefinedOrUnknownSVal DV,
+                         CheckerContext &C) const;
 };
 } // namespace
 
 static QualType getRecordType(QualType Ty) {
   Ty = Ty.getCanonicalType();
 
   if (Ty->isPointerType())
     Ty = Ty->getPointeeType();
State = setDynamicTypeAndCastInfo(State, MR, CastFromTy, CastToTy,
                                       Call.getResultType(), CastSucceeds);
 
   SVal V = CastSucceeds ? DV : C.getSValBuilder().makeNull();
   C.addTransition(
       State->BindExpr(Call.getOriginExpr(), C.getLocationContext(), V, false),
       getNoteTag(C, CastInfo, CastToTy, Object, CastSucceeds, IsKnownCast));
 }
 
+static void addInstanceOfTransition(const CallEvent &Call,
+                                    DefinedOrUnknownSVal DV,
+                                    ProgramStateRef State, CheckerContext &C,
+                                    bool IsInstanceOf) {
+  const FunctionDecl *FD = Call.getDecl()->getAsFunction();
+  QualType CastToTy = FD->getTemplateSpecializationArgs()->get(0).getAsType();
+  QualType CastFromTy = getRecordType(Call.parameters()[0]->getType());
+
+  const MemRegion *MR = DV.getAsRegion();
+  const DynamicCastInfo *CastInfo =
+      getDynamicCastInfo(State, MR, CastFromTy, CastToTy);
+
+  bool CastSucceeds;
+  if (CastInfo)
+    CastSucceeds = IsInstanceOf && CastInfo->succeeds();
+  else
+    CastSucceeds = IsInstanceOf || CastFromTy == CastToTy;
+
+  if (isInfeasibleCast(CastInfo, CastSucceeds)) {
+    C.generateSink(State, C.getPredecessor());
+    return;
+  }
+
+  // Store the type and the cast information.
+  bool IsKnownCast = CastInfo || CastFromTy == CastToTy;
+  if (!IsKnownCast)
+    State = setDynamicTypeAndCastInfo(State, MR, CastFromTy, CastToTy,
+                                      Call.getResultType(), IsInstanceOf);
+
+  C.addTransition(
+      State->BindExpr(Call.getOriginExpr(), C.getLocationContext(),
+                      C.getSValBuilder().makeTruthVal(CastSucceeds)),
+      getNoteTag(C, CastInfo, CastToTy, Call.getArgExpr(0), CastSucceeds,
+                 IsKnownCast));
+}
+
 //===----------------------------------------------------------------------===//
 // Evaluating cast, dyn_cast, cast_or_null, dyn_cast_or_null.
 //===----------------------------------------------------------------------===//
 
 static void evalNonNullParamNonNullReturn(const CallEvent &Call,
                                           DefinedOrUnknownSVal DV,
                                           CheckerContext &C,
                                           bool IsCheckedCast = false) {

 
 void CastValueChecker::evalGetAs(const CallEvent &Call, DefinedOrUnknownSVal DV,
                                  CheckerContext &C) const {
   evalZeroParamNonNullReturn(Call, DV, C);
   evalZeroParamNullReturn(Call, DV, C);
 }
 
 //===----------------------------------------------------------------------===//
+// Evaluating isa, isa_and_nonnull.
+//===----------------------------------------------------------------------===//
+
+void CastValueChecker::evalIsa(const CallEvent &Call, DefinedOrUnknownSVal DV,
+                               CheckerContext &C) const {
+  ProgramStateRef NonNullState, NullState;
+  std::tie(NonNullState, NullState) = C.getState()->assume(DV);
+
+  if (NonNullState) {
+    addInstanceOfTransition(Call, DV, NonNullState, C, /*IsInstanceOf=*/true);
+    addInstanceOfTransition(Call, DV, NonNullState, C, /*IsInstanceOf=*/false);
+  }
+
+  if (NullState) {
+    C.generateSink(NullState, C.getPredecessor());
+  }
+}
+
+void CastValueChecker::evalIsaAndNonNull(const CallEvent &Call,
+                                         DefinedOrUnknownSVal DV,
+                                         CheckerContext &C) const {
+  ProgramStateRef NonNullState, NullState;
+  std::tie(NonNullState, NullState) = C.getState()->assume(DV);
+
+  if (NonNullState) {
+    addInstanceOfTransition(Call, DV, NonNullState, C, /*IsInstanceOf=*/true);
+    addInstanceOfTransition(Call, DV, NonNullState, C, /*IsInstanceOf=*/false);
+  }
+
+  if (NullState) {
+    addInstanceOfTransition(Call, DV, NullState, C, /*IsInstanceOf=*/false);
+  }
+}
+
+//===----------------------------------------------------------------------===//
 // Main logic to evaluate a call.
 //===----------------------------------------------------------------------===//
 
 bool CastValueChecker::evalCall(const CallEvent &Call,
                                 CheckerContext &C) const {
   const auto *Lookup = CDM.lookup(Call);
   if (!Lookup)
     return false;
 
+  const CastCheck &Check = Lookup->first;
+  CallKind Kind = Lookup->second;
+
   // We need to obtain the record type of the call's result to model it.
-  if (!getRecordType(Call.getResultType())->isRecordType())
+  if (Kind != CallKind::InstanceOf &&
+      !getRecordType(Call.getResultType())->isRecordType())
     return false;
 
-  const CastCheck &Check = Lookup->first;
-  CallKind Kind = Lookup->second;
   Optional<DefinedOrUnknownSVal> DV;
 
   switch (Kind) {
   case CallKind::Function: {
     // We need to obtain the record type of the call's parameter to model it.
     if (!getRecordType(Call.parameters()[0]->getType())->isRecordType())
       return false;
 
     DV = Call.getArgSVal(0).getAs<DefinedOrUnknownSVal>();
     break;
   }
+  case CallKind::InstanceOf: {
+    // We need to obtain the only template argument to determinte the type.
+    const FunctionDecl *FD = Call.getDecl()->getAsFunction();
+    if (!FD || !FD->getTemplateSpecializationArgs())
+      return false;
+
+    DV = Call.getArgSVal(0).getAs<DefinedOrUnknownSVal>();
+    break;
+  }
   case CallKind::Method:
     const auto *InstanceCall = dyn_cast<CXXInstanceCall>(&Call);
     if (!InstanceCall)
       return false;
 
     DV = InstanceCall->getCXXThisVal().getAs<DefinedOrUnknownSVal>();
     break;
   }

cfe/trunk/test/Analysis/Inputs/llvm.h

 
 template <class X, class Y>
 const X *cast_or_null(Y Value);
 
 template <class X, class Y>
 const X *dyn_cast_or_null(Y *Value);
 template <class X, class Y>
 const X *dyn_cast_or_null(Y &Value);
+
+template <class X, class Y>
+bool isa(Y Value);
+
+template <class X, class Y>
+bool isa_and_nonnull(Y Value);
 } // namespace llvm

cfe/trunk/test/Analysis/cast-value-logic.cpp

 };
 class Triangle : public Shape {};
 class Circle : public Shape {};
 } // namespace clang
 
 using namespace llvm;
 using namespace clang;
 
-void test_regions(const Shape *A, const Shape *B) {
+void test_regions_dyn_cast(const Shape *A, const Shape *B) {
   if (dyn_cast<Circle>(A) && !dyn_cast<Circle>(B))
     clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
 }
 
+void test_regions_isa(const Shape *A, const Shape *B) {
+  if (isa<Circle>(A) && !isa<Circle>(B))
+    clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+}
+
 namespace test_cast {
 void evalLogic(const Shape *S) {
   const Circle *C = cast<Circle>(S);
   clang_analyzer_numTimesReached(); // expected-warning {{1}}
 
   if (S && C)
     clang_analyzer_eval(C == S); // expected-warning {{TRUE}}
 

cfe/trunk/test/Analysis/cast-value-notes.cpp

 // RUN: %clang_analyze_cc1 \
-// RUN:  -analyzer-checker=core,apiModeling.llvm.CastValue \
+// RUN:  -analyzer-checker=core,apiModeling.llvm.CastValue,debug.ExprInspection\
 // RUN:  -analyzer-output=text -verify %s
 
 #include "Inputs/llvm.h"
 
 namespace clang {
 struct Shape {
   template <typename T>
   const T *castAs() const;
void evalNonNullParamNonNullReturnReference(const Shape &S) {
   }
 
   if (dyn_cast_or_null<Triangle>(C)) {
     // expected-note@-1 {{Assuming 'C' is not a 'Triangle'}}
     // expected-note@-2 {{Taking false branch}}
     return;
   }
 
-  if (dyn_cast_or_null<Triangle>(C)) {
+  if (isa<Triangle>(C)) {
     // expected-note@-1 {{'C' is not a 'Triangle'}}
     // expected-note@-2 {{Taking false branch}}
     return;
   }
 
+  if (isa<Circle>(C)) {
+    // expected-note@-1 {{'C' is a 'Circle'}}
+    // expected-note@-2 {{Taking true branch}}
+
     (void)(1 / !C);
     // expected-note@-1 {{'C' is non-null}}
     // expected-note@-2 {{Division by zero}}
     // expected-warning@-3 {{Division by zero}}
   }
+}
 
 void evalNonNullParamNonNullReturn(const Shape *S) {
   const auto *C = cast<Circle>(S);
   // expected-note@-1 {{'S' is a 'Circle'}}
   // expected-note@-2 {{'C' initialized here}}
 
-  if (!cast<Triangle>(C)) {
+  if (!isa<Triangle>(C)) {
+    // expected-note@-1 {{Assuming 'C' is a 'Triangle'}}
+    // expected-note@-2 {{Taking false branch}}
+    return;
+  }
+
+  if (!isa<Triangle>(C)) {
     // expected-note@-1 {{'C' is a 'Triangle'}}
     // expected-note@-2 {{Taking false branch}}
     return;
   }
 
   (void)(1 / !C);
   // expected-note@-1 {{'C' is non-null}}
   // expected-note@-2 {{Division by zero}}