chrisbazley created this revision.
Herald added subscribers: steakhal, martong.
Herald added a reviewer: NoQ.
Herald added a project: All.
chrisbazley requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142739

Files:
  clang/docs/analyzer/checkers.rst
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/OptionalityChecker.cpp
  llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn

Index: llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn
===================================================================
--- llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn
+++ llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn
@@ -97,6 +97,7 @@
     "ObjCSelfInitChecker.cpp",
     "ObjCSuperDeallocChecker.cpp",
     "ObjCUnusedIVarsChecker.cpp",
+    "OptionalityChecker.cpp",
     "PaddingChecker.cpp",
     "PointerArithChecker.cpp",
     "PointerIterationChecker.cpp",
Index: clang/lib/StaticAnalyzer/Checkers/OptionalityChecker.cpp
===================================================================
--- /dev/null
+++ clang/lib/StaticAnalyzer/Checkers/OptionalityChecker.cpp
@@ -0,0 +1,138 @@
+//===-- 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;
+}
Index: clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -88,6 +88,7 @@
   ObjCSelfInitChecker.cpp
   ObjCSuperDeallocChecker.cpp
   ObjCUnusedIVarsChecker.cpp
+  OptionalityChecker.cpp
   OSObjectCStyleCast.cpp
   PaddingChecker.cpp
   PointerArithChecker.cpp
Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -55,6 +55,8 @@
                   Released>
   ]>;
 
+def Optionality : Package<"optionality">;
+
 def Cplusplus : Package<"cplusplus">;
 def CplusplusAlpha : Package<"cplusplus">, ParentPackage<Alpha>;
 def CplusplusOptIn : Package<"cplusplus">, ParentPackage<OptIn>;
@@ -349,6 +351,20 @@
 
 } // 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.
 //===----------------------------------------------------------------------===//
Index: clang/docs/analyzer/checkers.rst
===================================================================
--- clang/docs/analyzer/checkers.rst
+++ clang/docs/analyzer/checkers.rst
@@ -478,6 +478,37 @@
 """"""""""""""""""""""""""""""""""""""""""""""
 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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D142739: Standa... Christopher Bazley via Phabricator via cfe-commits

Reply via email to