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