Standalone checker for use of _Optional qualifier

[chrisbazley]

Summary

This checker tries to find violations in use of
pointers to _Optional objects which cannot be
detected by the type system alone. This requires
detection of null pointer dereferences at the
expression level, rather than at the level of
simulated memory accesses (which is already
implemented by other checkers).

Such expressions include those which implicitly
remove the _Optional qualifier from pointer
targets without actually accessing the pointed-to
object.

[chrisbazley]

Please refer to https://discourse.llvm.org/t/rfc-optional-a-type-qualifier-to-indicate-pointer-nullability/ and https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3089.pdf for the wider context of this commit.

clang/docs/analyzer/checkers.rst

  }
 
 .. _nullability-NullableReturnedFromNonnull:
 
 nullability.NullableReturnedFromNonnull (ObjC)
 """"""""""""""""""""""""""""""""""""""""""""""
 Warns when a nullable pointer is returned from a function that has _Nonnull return type.
 
+.. _optionality-checkers:
+
+optionality
+^^^^^^^^^^^
+
+Warn about misuse of the ``_Optional`` type qualifier.
+
+.. _optionality-OptionalityChecker:
+
+_optionality.OptionalityChecker
+""""""""""""""""""""""""""""
+Warns when a pointer to an object of ``_Optional`` type is dereferenced (even if only syntactically) without a preceding check for null.
+This is stricter than the nullability checker in order to validate expressions such as ``&*x`` or ``&x[0]``, which implicitly remove the qualifier from a pointee's type without actually accessing its stored value.
+Such expressions are not strictly-conformant when applied to a null pointer anyway: they have undefined behaviour.
+
+.. code-block:: c
+
+ struct LinkedList {
+   int data;
+   _Optional struct LinkedList *next;
+ };
+
+ struct LinkedList updateNextDataAndGetNext(struct LinkedList *list, int newData) {
+   _Optional struct LinkedList *next = list->next;
+   next->data = newData; // warn (unguarded dereference of pointer to optional)
+   if (next) {
+     next->data = newData; // ok
+   }
+   return &*next; // warn (unguarded implicit removal of optional qualifier)
+ }
+
 .. _optin-checkers:
 
 optin
 ^^^^^
 
 Checkers for portability, performance or coding style specific rules.
 
 .. _optin-cplusplus-UninitializedObject:

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

CmdLineOption<Boolean,
                   "of system header functions. This is useful if you are "
                   "concerned with your custom nullability annotations more "
                   "than with following nullability specifications of system "
                   "header functions.",
                   "false",
                   Released>
   ]>;
 
+def Optionality : Package<"optionality">;
+
 def Cplusplus : Package<"cplusplus">;
 def CplusplusAlpha : Package<"cplusplus">, ParentPackage<Alpha>;
 def CplusplusOptIn : Package<"cplusplus">, ParentPackage<OptIn>;
 
 def Valist : Package<"valist">;
 
 def DeadCode : Package<"deadcode">;
 def DeadCodeAlpha : Package<"deadcode">, ParentPackage<Alpha>;
def NullableReturnedFromNonnullChecker : Checker<"NullableReturnedFromNonnull">,
   HelpText<"Warns when a nullable pointer is returned from a function that has "
            "_Nonnull return type.">,
   Dependencies<[NullabilityBase]>,
   Documentation<NotDocumented>;
 
 } // end "nullability"
 
 //===----------------------------------------------------------------------===//
+// Optionality checkers.
+//===----------------------------------------------------------------------===//
+
+let ParentPackage = Optionality in {
+
+def OptionalityChecker : Checker<"OptionalityChecker">,
+  HelpText<"Warns when a pointer to an object of _Optional type is "
+           "dereferenced (even if only syntactically) without a preceding "
+           "check for null.">,
+  Documentation<HasDocumentation>;
+
+} // end "optionality"
+
+//===----------------------------------------------------------------------===//
 // APIModeling.
 //===----------------------------------------------------------------------===//
 
 let ParentPackage = APIModeling in {
 
 def ErrnoModeling : Checker<"Errno">,
   HelpText<"Make the special value 'errno' available to other checkers.">,
   Documentation<NotDocumented>;

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
+  OptionalityChecker.cpp
   OSObjectCStyleCast.cpp
   PaddingChecker.cpp
   PointerArithChecker.cpp
   PointerIterationChecker.cpp
   PointerSortingChecker.cpp
   PointerSubChecker.cpp
   PthreadLockChecker.cpp
   cert/PutenvWithAutoChecker.cpp

clang/lib/StaticAnalyzer/Checkers/OptionalityChecker.cpp

+//===-- OptionalityChecker.cpp - Optionality checker ----------------------===//
+//
+// 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 checker tries to find violations in use of pointers to _Optional
+// objects which cannot be detected by the type system alone. This requires
+// detection of null pointer dereferences at the expression level, rather than
+// at the level of simulated memory accesses (which is already implemented by
+// other checkers).
+//
+// Such expressions include those which implicitly remove the _Optional
+// qualifier from pointer targets without actually accessing the pointed-to
+// object.
+//
+//===----------------------------------------------------------------------===//
+
+#include "Iterator.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
+
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/Path.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+
+class OptionalityChecker
+    : public Checker<
+          check::PreStmt<UnaryOperator>, check::PreStmt<BinaryOperator>,
+          check::PreStmt<ArraySubscriptExpr>, check::PreStmt<MemberExpr>> {
+
+  void verifyAccess(CheckerContext &C, const Expr *E) const;
+
+public:
+  void checkPreStmt(const UnaryOperator *UO, CheckerContext &C) const;
+  void checkPreStmt(const BinaryOperator *BO, CheckerContext &C) const;
+  void checkPreStmt(const ArraySubscriptExpr *ASE, CheckerContext &C) const;
+  void checkPreStmt(const MemberExpr *ME, CheckerContext &C) const;
+
+  CheckerNameRef CheckName;
+  mutable std::unique_ptr<BugType> BT;
+
+  const std::unique_ptr<BugType> &getBugType() const {
+    if (!BT)
+      BT.reset(new BugType(CheckName, "Optionality", categories::MemoryError));
+    return BT;
+  }
+
+private:
+  void reportBug(StringRef Msg, ExplodedNode *N, BugReporter &BR) const {
+    const std::unique_ptr<BugType> &BT = getBugType();
+    auto R = std::make_unique<PathSensitiveBugReport>(*BT, Msg, N);
+    BR.emitReport(std::move(R));
+  }
+};
+
+} // end anonymous namespace
+
+void OptionalityChecker::checkPreStmt(const UnaryOperator *UO,
+                                      CheckerContext &C) const {
+  if (isa<CXXThisExpr>(UO->getSubExpr()))
+    return;
+
+  UnaryOperatorKind OK = UO->getOpcode();
+
+  if (clang::ento::iterator::isAccessOperator(OK)) {
+    verifyAccess(C, UO->getSubExpr());
+  }
+}
+
+void OptionalityChecker::checkPreStmt(const BinaryOperator *BO,
+                                      CheckerContext &C) const {
+  BinaryOperatorKind OK = BO->getOpcode();
+
+  if (clang::ento::iterator::isAccessOperator(OK)) {
+    verifyAccess(C, BO->getLHS());
+  }
+}
+
+void OptionalityChecker::checkPreStmt(const ArraySubscriptExpr *ASE,
+                                      CheckerContext &C) const {
+  verifyAccess(C, ASE->getLHS());
+}
+
+void OptionalityChecker::checkPreStmt(const MemberExpr *ME,
+                                      CheckerContext &C) const {
+  if (!ME->isArrow() || ME->isImplicitAccess())
+    return;
+
+  verifyAccess(C, ME->getBase());
+}
+
+void OptionalityChecker::verifyAccess(CheckerContext &C, const Expr *E) const {
+  if (!pointeeIsOptional(E->getType()))
+    return;
+
+  ProgramStateRef State = C.getState();
+  SVal Val = State->getSVal(E, C.getLocationContext());
+  auto DefOrUnknown = Val.getAs<DefinedOrUnknownSVal>();
+  if (!DefOrUnknown)
+    return;
+
+  if (State->isNonNull(*DefOrUnknown).isConstrainedTrue())
+    return;
+
+  static CheckerProgramPointTag Tag(this, "OptionalityChecker");
+  ExplodedNode *N = C.generateErrorNode(State, &Tag);
+  if (!N)
+    return;
+
+  BugReporter &BR = C.getBugReporter();
+  // Do not suppress errors on defensive code paths, because dereferencing
+  // a nullable pointer is always an error.
+  reportBug("Pointer to _Optional object is dereferenced without a preceding "
+            "check for null",
+            N, BR);
+}
+
+void ento::registerOptionalityChecker(CheckerManager &mgr) {
+  mgr.registerChecker<OptionalityChecker>();
+  OptionalityChecker *checker = mgr.getChecker<OptionalityChecker>();
+  checker->CheckName = mgr.getCurrentCheckerName();
+}
+
+bool ento::shouldRegisterOptionalityChecker(const CheckerManager &mgr) {
+  return true;
+}

llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn

sources = [
     "ObjCAutoreleaseWriteChecker.cpp",
     "ObjCContainersASTChecker.cpp",
     "ObjCContainersChecker.cpp",
     "ObjCMissingSuperCallChecker.cpp",
     "ObjCPropertyChecker.cpp",
     "ObjCSelfInitChecker.cpp",
     "ObjCSuperDeallocChecker.cpp",
     "ObjCUnusedIVarsChecker.cpp",
+    "OptionalityChecker.cpp",
     "PaddingChecker.cpp",
     "PointerArithChecker.cpp",
     "PointerIterationChecker.cpp",
     "PointerSortingChecker.cpp",
     "PointerSubChecker.cpp",
     "PthreadLockChecker.cpp",
     "RetainCountChecker/RetainCountChecker.cpp",
     "RetainCountChecker/RetainCountDiagnostics.cpp",