[analyzer] [WIP] Opt-in C Style Cast Checker for OSObject pointers

[george.karpenkov]

Summary

Message and exact matching still remains to be polished

[NoQ]

It sounds to me that for now you're only emitting a warning when a *literally* OSObject is being casted, i.e. not a sub-class to an even more specific sub-class. Is this intended?

clang/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp

20

Was this left around accidentally?

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

 let ParentPackage = Performance in {
 
 def GCDAntipattern : Checker<"GCDAntipattern">,
   HelpText<"Check for performance anti-patterns when using Grand Central "
            "Dispatch">,
   Documentation<NotDocumented>;
 } // end "optin.performance"
 
+let ParentPackage = OSXOptIn in {
+
+def OSObjectCStyleCast : Checker<"OSObjectCStyleCast">,
+  HelpText<"Checker for C-style casts of OSObjects">,
+  Documentation<NotDocumented>;
+
+} // end "optin.osx"
+
 let ParentPackage = CocoaAlpha in {
 
 def InstanceVariableInvalidation : Checker<"InstanceVariableInvalidation">,
   HelpText<"Check that the invalidatable instance variables are invalidated in "
            "the methods annotated with objc_instance_variable_invalidator">,
   Documentation<HasAlphaDocumentation>;
 
 def MissingInvalidationMethod : Checker<"MissingInvalidationMethod">,

clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt

add_clang_library(clangStaticAnalyzerCheckers
   ObjCAutoreleaseWriteChecker.cpp
   ObjCContainersASTChecker.cpp
   ObjCContainersChecker.cpp
   ObjCMissingSuperCallChecker.cpp
   ObjCPropertyChecker.cpp
   ObjCSelfInitChecker.cpp
   ObjCSuperDeallocChecker.cpp
   ObjCUnusedIVarsChecker.cpp
+  OSObjectCStyleCast.cpp
   PaddingChecker.cpp
   PointerArithChecker.cpp
   PointerSubChecker.cpp
   PthreadLockChecker.cpp
   RetainCountChecker/RetainCountChecker.cpp
   RetainCountChecker/RetainCountDiagnostics.cpp
   ReturnPointerRangeChecker.cpp
   ReturnUndefChecker.cpp

clang/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp

+//===- OSObjectCStyleCast.cpp ------------------------------------*- C++ -*-==//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines OSObjectCStyleCast checker, which checks for C-style casts
+// of OSObjects. Such casts almost always indicate a code smell,
+// as an explicit static or dynamic cast should be used instead.
+//===----------------------------------------------------------------------===//
+
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
+#include "llvm/Support/Debug.h"
+
+using namespace clang;
+using namespace ento;
+using namespace ast_matchers;
+
+namespace {
+
+const char *WarnAtNode = "OSObjCast";
+
+class OSObjectCStyleCastChecker : public Checker<check::ASTCodeBody> {
+public:
+  void checkASTCodeBody(const Decl *D,
+                        AnalysisManager &AM,
+                        BugReporter &BR) const;
+};
+
+static void emitDiagnostics(const BoundNodes &Nodes,
+                            BugReporter &BR,
+                            AnalysisDeclContext *ADC,
+                            const OSObjectCStyleCastChecker *Checker) {
+  const auto *CE = Nodes.getNodeAs<CastExpr>(WarnAtNode);
+  assert(CE);
+
+  std::string Diagnostics;
+  llvm::raw_string_ostream OS(Diagnostics);
+  OS << "C-style cast of OSObject. Use OSDynamicCast instead.";
+
+  BR.EmitBasicReport(
+    ADC->getDecl(),
+    Checker,
+    /*Name=*/"OSObject C-Style Cast",
+    /*Category=*/"Security",
+    OS.str(),
+    PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), ADC),
+    CE->getSourceRange());
+}
+
+static auto hasTypePointingTo(DeclarationMatcher DeclM)
+    -> decltype(hasType(pointerType())) {
+  return hasType(pointerType(pointee(hasDeclaration(DeclM))));
+}
+
+void OSObjectCStyleCastChecker::checkASTCodeBody(const Decl *D, AnalysisManager &AM,
+                                                 BugReporter &BR) const {
+
+  AnalysisDeclContext *ADC = AM.getAnalysisDeclContext(D);
+
+  auto DynamicCastM = callExpr(callee(functionDecl(hasName("safeMetaCast"))));
+
+  auto OSObjTypeM = hasTypePointingTo(cxxRecordDecl(isDerivedFrom("OSMetaClassBase")));
+  auto OSObjSubclassM = hasTypePointingTo(
+    cxxRecordDecl(isDerivedFrom("OSObject")));
+
+  auto CastM = cStyleCastExpr(
+          allOf(hasSourceExpression(allOf(OSObjTypeM, unless(DynamicCastM))),
+          OSObjSubclassM)).bind(WarnAtNode);
+
+  auto Matches = match(stmt(forEachDescendant(CastM)), *D->getBody(), AM.getASTContext());
+  for (BoundNodes Match : Matches)
+    emitDiagnostics(Match, BR, ADC, this);
+}
+}
+
+void ento::registerOSObjectCStyleCast(CheckerManager &Mgr) {
+  Mgr.registerChecker<OSObjectCStyleCastChecker>();
+}

clang/test/Analysis/osobjectcstylecastchecker_test.cpp

+// RUN: %clang_analyze_cc1 -analyzer-checker=optin.osx.OSObjectCStyleCast %s -verify
+#include "os_object_base.h"
+
+struct OSArray : public OSObject {
+  unsigned getCount();
+};
+
+struct A {
+  int x;
+};
+struct B : public A {
+  unsigned getCount();
+};
+
+unsigned warn_on_explicit_downcast(OSObject * obj) {
+  OSArray *a = (OSArray *) obj; // expected-warning{{C-style cast of OSObject. Use OSDynamicCast instead}}
+  return a->getCount();
+}
+
+void no_warn_on_upcast(OSArray *arr) {
+  OSObject *obj = (OSObject *) arr;
+  obj->retain();
+  obj->release();
+}
+
+unsigned no_warn_on_dynamic_cast(OSObject *obj) {
+  OSArray *a = OSDynamicCast(OSArray, obj);
+  return a->getCount();
+}
+
+unsigned long no_warn_on_primitive_conversion(OSArray *arr) {
+  return (unsigned long) arr;
+}
+
+unsigned no_warn_on_other_type_cast(A *a) {
+  B *b = (B *) a;
+  return b->getCount();
+}
+