[analyzer] Support allocClassWithName in OSObjectCStyleCast checker

[vsavchenko]

Summary

allocClassWithName allocates an object with the given type.
The type is actually provided as a string argument (type's name).
This creates a possibility for not particularly useful warnings
from the analyzer.

In order to combat with those, this patch checks for casts of the
allocClassWithName results to types mentioned directly as its
argument. All other uses of this method should be reasoned about
as before.

rdar://72165694

[NoQ]

LGTM! Thanks for formatting ^.^

WDYT about a heuristic that literally matches the source code of the casted expression as plain text to see if it mentions the type? Like, regardless of allocClassWithName(), if it mentions the type it probably has something to do with that type.

[vsavchenko]

In D99500#2657675, @NoQ wrote:

LGTM! Thanks for formatting ^.^

WDYT about a heuristic that literally matches the source code of the casted expression as plain text to see if it mentions the type? Like, regardless of allocClassWithName(), if it mentions the type it probably has something to do with that type.

This idea sounds good in general, but it feels like it can backfire in some cases. As any regex-like solution, it will have ridiculous corner cases when the name of the type is too generic and can appear as a sub-word.

clang/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp

 static constexpr const char *const WarnAtNode = "WarnAtNode";
 static constexpr const char *const WarnRecordDecl = "WarnRecordDecl";
 
 class OSObjectCStyleCastChecker : public Checker<check::ASTCodeBody> {
 public:
   void checkASTCodeBody(const Decl *D, AnalysisManager &AM,
                         BugReporter &BR) const;
 };
+} // namespace
+
+namespace clang {
+namespace ast_matchers {
+AST_MATCHER_P(StringLiteral, mentionsBoundType, std::string, BindingID) {
+  return Builder->removeBindings([this, &Node](const BoundNodesMap &Nodes) {
+    const auto &BN = Nodes.getNode(this->BindingID);
+    if (const auto *ND = BN.get<NamedDecl>()) {
+      return ND->getName() != Node.getString();
+    }
+    return true;
+  });
 }
+} // end namespace ast_matchers
+} // end namespace clang
 
 static void emitDiagnostics(const BoundNodes &Nodes,
                             BugReporter &BR,
                             AnalysisDeclContext *ADC,
                             const OSObjectCStyleCastChecker *Checker) {
   const auto *CE = Nodes.getNodeAs<CastExpr>(WarnAtNode);
   const CXXRecordDecl *RD = Nodes.getNodeAs<CXXRecordDecl>(WarnRecordDecl);
   assert(CE && RD);
BR.EmitBasicReport(
     PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), ADC),
     CE->getSourceRange());
 }
 
 static decltype(auto) hasTypePointingTo(DeclarationMatcher DeclM) {
   return hasType(pointerType(pointee(hasDeclaration(DeclM))));
 }
 
-void OSObjectCStyleCastChecker::checkASTCodeBody(const Decl *D, AnalysisManager &AM,
+void OSObjectCStyleCastChecker::checkASTCodeBody(const Decl *D,
+                                                 AnalysisManager &AM,
                                                  BugReporter &BR) const {
 
   AnalysisDeclContext *ADC = AM.getAnalysisDeclContext(D);
 
   auto DynamicCastM = callExpr(callee(functionDecl(hasName("safeMetaCast"))));
+  // 'allocClassWithName' allocates an object with the given type.
+  // The type is actually provided as a string argument (type's name).
+  // This makes the following pattern possible:
+  //
+  // Foo *object = (Foo *)allocClassWithName("Foo");
+  //
+  // While OSRequiredCast can be used here, it is still not a useful warning.
+  auto AllocClassWithNameM = callExpr(
+      callee(functionDecl(hasName("allocClassWithName"))),
+      // Here we want to make sure that the string argument matches the
+      // type in the cast expression.
+      hasArgument(0, stringLiteral(mentionsBoundType(WarnRecordDecl))));
 
-  auto OSObjTypeM = hasTypePointingTo(cxxRecordDecl(isDerivedFrom("OSMetaClassBase")));
+  auto OSObjTypeM =
+      hasTypePointingTo(cxxRecordDecl(isDerivedFrom("OSMetaClassBase")));
   auto OSObjSubclassM = hasTypePointingTo(
       cxxRecordDecl(isDerivedFrom("OSObject")).bind(WarnRecordDecl));
 
-  auto CastM = cStyleCastExpr(
-          allOf(hasSourceExpression(allOf(OSObjTypeM, unless(DynamicCastM))),
-          OSObjSubclassM)).bind(WarnAtNode);
+  auto CastM =
+      cStyleCastExpr(
+          allOf(OSObjSubclassM,
+                hasSourceExpression(
+                    allOf(OSObjTypeM,
+                          unless(anyOf(DynamicCastM, AllocClassWithNameM))))))
+          .bind(WarnAtNode);
 
-  auto Matches = match(stmt(forEachDescendant(CastM)), *D->getBody(), AM.getASTContext());
+  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>();
 }
 
 bool ento::shouldRegisterOSObjectCStyleCast(const CheckerManager &mgr) {
   return true;
 }

clang/test/Analysis/os_object_base.h

struct OSObject : public OSMetaClassBase {
 
   static void * operator new(size_t size);
 
   static const OSMetaClass * const metaClass;
 };
 
 struct OSMetaClass : public OSMetaClassBase {
   virtual OSObject * alloc() const;
+  static OSObject * allocClassWithName(const char * name);
   virtual ~OSMetaClass(){}
 };
 
 #endif /* _OS_BASE_H */

clang/test/Analysis/osobjectcstylecastchecker_test.cpp

__SIZE_TYPE__ no_warn_on_primitive_conversion(OSArray *arr) {
   return (__SIZE_TYPE__) arr;
 }
 
 unsigned no_warn_on_other_type_cast(A *a) {
   B *b = (B *) a;
   return b->getCount();
 }
 
+unsigned no_warn_alloc_class_with_name() {
+  OSArray *a = (OSArray *)OSMetaClass::allocClassWithName("OSArray"); // no warning
+  return a->getCount();
+}
+
+unsigned warn_alloc_class_with_name() {
+  OSArray *a = (OSArray *)OSMetaClass::allocClassWithName("OSObject"); // expected-warning{{C-style cast of an OSObject is prone to type confusion attacks; use 'OSRequiredCast' if the object is definitely of type 'OSArray', or 'OSDynamicCast' followed by a null check if unsure}}
+  return a->getCount();
+}