[PATCH] D63117: [analyzer] RetainCount: Add support for OSRequiredCast().

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added a reviewer: dcoughlin.
Herald added subscribers: cfe-commits, Charusso, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: clang.

It's a new API for custom RTTI in Apple IOKit/DriverKit framework that is similar to `OSDynamicCast()` that's already supported, but crashes with assertion instead of returning null (and therefore causing UB when the cast fails unexpectedly). Kinda like `cast_or_null<>` as opposed to `dyn_cast_or_null<>` in LLVM's RTTI.

Historically, `RetainCountChecker` is responsible for modeling `OSDynamicCast`, so i simply extend this functionality.


Repository:
  rC Clang

https://reviews.llvm.org/D63117

Files:
  clang/lib/Analysis/RetainSummaryManager.cpp
  clang/test/Analysis/os_object_base.h
  clang/test/Analysis/osobject-retain-release.cpp


Index: clang/test/Analysis/osobject-retain-release.cpp
===================================================================
--- clang/test/Analysis/osobject-retain-release.cpp
+++ clang/test/Analysis/osobject-retain-release.cpp
@@ -1,9 +1,11 @@
 // RUN: %clang_analyze_cc1 -fblocks -analyze -analyzer-output=text\
-// RUN:                    -analyzer-checker=core,osx -verify %s
+// RUN:   -analyzer-checker=core,osx,debug.ExprInspection -verify %s
 
 #include "os_object_base.h"
 #include "os_smart_ptr.h"
 
+void clang_analyzer_eval(bool);
+
 struct OSIterator : public OSObject {
   static const OSMetaClass * const metaClass;
 };
@@ -483,6 +485,23 @@
   arr->release();
 }
 
+void check_required_cast() {
+  OSArray *arr = OSRequiredCast(OSArray, OSObject::generateObject(1));
+  arr->release(); // no-warning
+}
+
+void check_cast_behavior(OSObject *obj) {
+  OSArray *arr1 = OSDynamicCast(OSArray, obj);
+  clang_analyzer_eval(arr1 == obj); // expected-warning{{TRUE}}
+                                    // expected-note at -1{{TRUE}}
+                                    // expected-note at -2{{Assuming 'arr1' is not equal to 'obj'}}
+                                    // expected-warning at -3{{FALSE}}
+                                    // expected-note at -4   {{FALSE}}
+  OSArray *arr2 = OSRequiredCast(OSArray, obj);
+  clang_analyzer_eval(arr2 == obj); // expected-warning{{TRUE}}
+                                    // expected-note at -1{{TRUE}}
+}
+
 unsigned int check_dynamic_cast_no_null_on_orig(OSObject *obj) {
   OSArray *arr = OSDynamicCast(OSArray, obj);
   if (arr) {
Index: clang/test/Analysis/os_object_base.h
===================================================================
--- clang/test/Analysis/os_object_base.h
+++ clang/test/Analysis/os_object_base.h
@@ -12,6 +12,8 @@
 
 #define OSDynamicCast(type, inst)   \
     ((type *) OSMetaClassBase::safeMetaCast((inst), OSTypeID(type)))
+#define OSRequiredCast(type, inst)   \
+    ((type *) OSMetaClassBase::requiredMetaCast((inst), OSTypeID(type)))
 
 #define OSTypeAlloc(type)   ((type *) ((type::metaClass)->alloc()))
 
@@ -22,6 +24,8 @@
 struct OSMetaClassBase {
   static OSMetaClassBase *safeMetaCast(const OSMetaClassBase *inst,
                                        const OSMetaClass *meta);
+  static OSMetaClassBase *requiredMetaCast(const OSMetaClassBase *inst,
+                                           const OSMetaClass *meta);
 
   OSMetaClassBase *metaCast(const char *toMeta);
 
Index: clang/lib/Analysis/RetainSummaryManager.cpp
===================================================================
--- clang/lib/Analysis/RetainSummaryManager.cpp
+++ clang/lib/Analysis/RetainSummaryManager.cpp
@@ -152,6 +152,10 @@
   return S == "safeMetaCast";
 }
 
+static bool isOSObjectRequiredCast(StringRef S) {
+  return S == "requiredMetaCast";
+}
+
 static bool isOSObjectThisCast(StringRef S) {
   return S == "metaCast";
 }
@@ -234,7 +238,8 @@
   if (RetTy->isPointerType()) {
     const CXXRecordDecl *PD = RetTy->getPointeeType()->getAsCXXRecordDecl();
     if (PD && isOSObjectSubclass(PD)) {
-      if (isOSObjectDynamicCast(FName) || isOSObjectThisCast(FName))
+      if (isOSObjectDynamicCast(FName) || isOSObjectRequiredCast(FName) ||
+          isOSObjectThisCast(FName))
         return getDefaultSummary();
 
       // TODO: Add support for the slightly common *Matching(table) idiom.
@@ -745,6 +750,8 @@
     if (TrackOSObjects) {
       if (isOSObjectDynamicCast(FName) && FD->param_size() >= 1) {
         return BehaviorSummary::IdentityOrZero;
+      } else if (isOSObjectRequiredCast(FName) && FD->param_size() >= 1) {
+        return BehaviorSummary::Identity;
       } else if (isOSObjectThisCast(FName) && isa<CXXMethodDecl>(FD) &&
                  !cast<CXXMethodDecl>(FD)->isStatic()) {
         return BehaviorSummary::IdentityThis;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D63117.203959.patch
Type: text/x-patch
Size: 3845 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190611/88f7ce5a/attachment.bin>