diff --git a/nullability/BUILD b/nullability/BUILD index 167961abf..694184f08 100644 --- a/nullability/BUILD +++ b/nullability/BUILD @@ -5,6 +5,60 @@ load("@rules_cc//cc:cc_test.bzl", "cc_test") package(default_applicable_licenses = ["//:license"]) +cc_library( + name = "googlesql_value_nullability_lattice", + hdrs = ["googlesql_value_nullability_lattice.h"], + visibility = ["//visibility:public"], + deps = [ + "@llvm-project//clang:analysis", + ], +) + +cc_library( + name = "googlesql_value_nullability_analysis", + srcs = ["googlesql_value_nullability_analysis.cc"], + hdrs = ["googlesql_value_nullability_analysis.h"], + visibility = ["//visibility:public"], + deps = [ + ":googlesql_value_nullability", + ":googlesql_value_nullability_lattice", + "@llvm-project//clang:analysis", + "@llvm-project//clang:ast", + "@llvm-project//clang:ast_matchers", + "@llvm-project//clang:basic", + "@llvm-project//llvm:Support", + ], +) + +cc_library( + name = "googlesql_value_nullability", + srcs = ["googlesql_value_nullability.cc"], + hdrs = ["googlesql_value_nullability.h"], + deps = [ + "@abseil-cpp//absl/base:nullability", + "@llvm-project//clang:analysis", + "@llvm-project//clang:ast", + ], +) + +cc_test( + name = "googlesql_value_nullability_analysis_test", + srcs = ["googlesql_value_nullability_analysis_test.cc"], + deps = [ + ":googlesql_value_nullability_analysis", + ":googlesql_value_nullability_lattice", + "@llvm-project//clang:analysis", + "@llvm-project//clang:ast", + "@llvm-project//clang:ast_matchers", + "@llvm-project//clang:testing", + "@llvm-project//llvm:Support", + "@llvm-project//llvm:TestingAnnotations", + "@llvm-project//third-party/unittest:gmock", + "@llvm-project//third-party/unittest:gtest", + "@llvm-project//third-party/unittest:gtest_main", + ], +) + cc_library( name = "ast_helpers", hdrs = ["ast_helpers.h"], diff --git a/nullability/googlesql_value_nullability.cc b/nullability/googlesql_value_nullability.cc new file mode 100644 index 000000000..35a4a2f53 --- /dev/null +++ b/nullability/googlesql_value_nullability.cc @@ -0,0 +1,95 @@ +// Part of the Crubit project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include "nullability/googlesql_value_nullability.h" + +#include + +#include "absl/base/nullability.h" +#include "clang/AST/DeclCXX.h" +#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h" +#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h" +#include "clang/Analysis/FlowSensitive/DataflowLattice.h" +#include "clang/Analysis/FlowSensitive/Formula.h" +#include "clang/Analysis/FlowSensitive/StorageLocation.h" +#include "clang/Analysis/FlowSensitive/Value.h" + +namespace clang { +namespace tidy { +namespace nullability { +namespace { + +static constexpr char kGoogleSqlIsNull[] = "googlesql_is_null"; + +} // namespace + +dataflow::LatticeJoinEffect GoogleSqlValueNullState::join( + const GoogleSqlValueNullState& Other) { + if (*this == Other) return dataflow::LatticeJoinEffect::Unchanged; + if (IsNull == Other.IsNull) return dataflow::LatticeJoinEffect::Unchanged; + IsNull = nullptr; + return dataflow::LatticeJoinEffect::Changed; +} + +static dataflow::StorageLocation& getOrAddGoogleSqlNullField( + dataflow::RecordStorageLocation& Loc, dataflow::Environment& Env) { + for (const auto& Entry : Loc.synthetic_fields()) { + if (Entry.getKey() == kGoogleSqlIsNull) return *Entry.getValue(); + } + auto& Ctx = Loc.getType()->getAsCXXRecordDecl()->getASTContext(); + auto& FieldLoc = Env.createStorageLocation(Ctx.BoolTy); + Loc.addSyntheticField(kGoogleSqlIsNull, FieldLoc); + return FieldLoc; +} + +bool hasGoogleSqlValueNullState(const dataflow::RecordStorageLocation& Loc, + const dataflow::Environment& Env) { + for (const auto& Entry : Loc.synthetic_fields()) { + if (Entry.getKey() == kGoogleSqlIsNull) { + return Env.getValue(*Entry.getValue()) != nullptr; + } + } + return false; +} + +GoogleSqlValueNullState getGoogleSqlValueNullState( + const dataflow::RecordStorageLocation& Loc, + const dataflow::Environment& Env) { + const dataflow::StorageLocation* FieldLoc = nullptr; + for (const auto& Entry : Loc.synthetic_fields()) { + if (Entry.getKey() == kGoogleSqlIsNull) { + FieldLoc = Entry.getValue(); + break; + } + } + if (!FieldLoc) return GoogleSqlValueNullState::getTop(); + + auto* Val = Env.get(*FieldLoc); + if (!Val) return GoogleSqlValueNullState::getTop(); + + return {&Val->formula()}; +} + +void initGoogleSqlValueNullState( + dataflow::RecordStorageLocation& Loc, dataflow::Environment& Env, + const dataflow::Formula* absl_nullable IsNull) { + dataflow::StorageLocation& FieldLoc = getOrAddGoogleSqlNullField(Loc, Env); + auto& A = Env.arena(); + assert(Env.getValue(FieldLoc) == nullptr); + Env.setValue(FieldLoc, + IsNull != nullptr ? A.makeBoolValue(*IsNull) : A.makeTopValue()); +} + +void setGoogleSqlValueNullState(dataflow::RecordStorageLocation& Loc, + dataflow::Environment& Env, + const dataflow::Formula* absl_nullable IsNull) { + dataflow::StorageLocation& FieldLoc = getOrAddGoogleSqlNullField(Loc, Env); + auto& A = Env.arena(); + Env.setValue(FieldLoc, + IsNull != nullptr ? A.makeBoolValue(*IsNull) : A.makeTopValue()); +} + +} // namespace nullability +} // namespace tidy +} // namespace clang diff --git a/nullability/googlesql_value_nullability.h b/nullability/googlesql_value_nullability.h new file mode 100644 index 000000000..7fc88467f --- /dev/null +++ b/nullability/googlesql_value_nullability.h @@ -0,0 +1,75 @@ +// Part of the Crubit project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#ifndef CRUBIT_NULLABILITY_GOOGLESQL_VALUE_NULLABILITY_H_ +#define CRUBIT_NULLABILITY_GOOGLESQL_VALUE_NULLABILITY_H_ + +#include "absl/base/nullability.h" +#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h" +#include "clang/Analysis/FlowSensitive/DataflowLattice.h" +#include "clang/Analysis/FlowSensitive/Formula.h" + +namespace clang { +namespace dataflow { +class Arena; +class RecordStorageLocation; +} // namespace dataflow + +namespace tidy { +namespace nullability { + +// Represents the nullability state of a `googlesql::Value` object. +// +// This state is tracked via a boolean formula stored as a synthetic field on +// the `RecordStorageLocation` representing the `googlesql::Value` instance. +struct GoogleSqlValueNullState { + // A boolean formula representing whether the value is null. + // If null, the state is unknown (equivalent to Top). + const dataflow::Formula* absl_nullable IsNull = nullptr; + + bool operator==(const GoogleSqlValueNullState& Other) const { + return IsNull == Other.IsNull; + } + bool operator!=(const GoogleSqlValueNullState& Other) const { + return !(*this == Other); + } + + // Returns an unknown state (Top). + static GoogleSqlValueNullState getTop() { return {}; } + + // Joins this state with another state. + // If the states differ, the result is unknown (Top). + dataflow::LatticeJoinEffect join(const GoogleSqlValueNullState& Other); +}; + +// Returns true if the given location has a mapped GoogleSQL value null state +// in the environment. +bool hasGoogleSqlValueNullState(const dataflow::RecordStorageLocation& Loc, + const dataflow::Environment& Env); + +// Retrieves the GoogleSQL value null state for the given location. +// Returns an unknown state (Top) if not found. +GoogleSqlValueNullState getGoogleSqlValueNullState( + const dataflow::RecordStorageLocation& Loc, + const dataflow::Environment& Env); + +// Initializes the GoogleSQL value null state for the given location. +// This should be called when a `googlesql::Value` object is newly created +// and we need to establish its initial nullability state. +void initGoogleSqlValueNullState(dataflow::RecordStorageLocation& Loc, + dataflow::Environment& Env, + const dataflow::Formula* absl_nullable IsNull); + +// Sets the GoogleSQL value null state for the given location. +// This can be used to update the state of an existing object, e.g., after +// an assignment. +void setGoogleSqlValueNullState(dataflow::RecordStorageLocation& Loc, + dataflow::Environment& Env, + const dataflow::Formula* absl_nullable IsNull); + +} // namespace nullability +} // namespace tidy +} // namespace clang + +#endif // CRUBIT_NULLABILITY_GOOGLESQL_VALUE_NULLABILITY_H_ diff --git a/nullability/googlesql_value_nullability_analysis.cc b/nullability/googlesql_value_nullability_analysis.cc new file mode 100644 index 000000000..954fe040b --- /dev/null +++ b/nullability/googlesql_value_nullability_analysis.cc @@ -0,0 +1,294 @@ +// Part of the Crubit project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include "nullability/googlesql_value_nullability_analysis.h" + +#include + +#include "nullability/googlesql_value_nullability.h" +#include "nullability/googlesql_value_nullability_lattice.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Expr.h" +#include "clang/AST/ExprCXX.h" +#include "clang/AST/Stmt.h" +#include "clang/AST/Type.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Analysis/CFG.h" +#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h" +#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h" +#include "clang/Analysis/FlowSensitive/MatchSwitch.h" +#include "clang/Analysis/FlowSensitive/StorageLocation.h" +#include "clang/Basic/LLVM.h" +#include "llvm/ADT/SmallVector.h" + +namespace clang { +namespace tidy { +namespace nullability { + +using ::clang::ast_matchers::anyOf; +using ::clang::ast_matchers::callee; +using ::clang::ast_matchers::callExpr; +using ::clang::ast_matchers::cxxConstructExpr; +using ::clang::ast_matchers::cxxConstructorDecl; +using ::clang::ast_matchers::cxxMemberCallExpr; +using ::clang::ast_matchers::cxxMethodDecl; +using ::clang::ast_matchers::cxxOperatorCallExpr; +using ::clang::ast_matchers::cxxRecordDecl; +using ::clang::ast_matchers::hasAnyName; +using ::clang::ast_matchers::hasArgument; +using ::clang::ast_matchers::hasCanonicalType; +using ::clang::ast_matchers::hasDeclaration; +using ::clang::ast_matchers::hasName; +using ::clang::ast_matchers::hasOverloadedOperatorName; +using ::clang::ast_matchers::hasType; +using ::clang::ast_matchers::isCopyConstructor; +using ::clang::ast_matchers::isDefaultConstructor; +using ::clang::ast_matchers::isMoveConstructor; +using ::clang::ast_matchers::isStaticStorageClass; +using ::clang::ast_matchers::matchesName; +using ::clang::ast_matchers::MatchFinder; +using ::clang::ast_matchers::ofClass; +using ::clang::ast_matchers::recordType; +using ::clang::ast_matchers::unless; +using dataflow::CFGMatchSwitchBuilder; +using dataflow::RecordStorageLocation; +using dataflow::TransferState; + +// Helper to ensure a googlesql::Value has a mapped null state. +static void ensureGoogleSqlValueInitialized(RecordStorageLocation& Loc, + dataflow::Environment& Env) { + if (!hasGoogleSqlValueNullState(Loc, Env)) { + auto& A = Env.arena(); + initGoogleSqlValueNullState(Loc, Env, &A.makeAtomRef(A.makeAtom())); + } +} + +// Transfer function for default constructor: Value() -> Null. +static void transferDefaultConstructor( + const CXXConstructExpr* Ctor, const MatchFinder::MatchResult& Result, + TransferState& State) { + auto& A = State.Env.arena(); + RecordStorageLocation& Loc = State.Env.getResultObjectLocation(*Ctor); + + // Overwrite null state to definitely null (default ctor is null). + setGoogleSqlValueNullState(Loc, State.Env, &A.makeLiteral(true)); +} + +// Transfer function for copy/move constructors: inherit state. +static void transferCopyOrMoveConstructor( + const CXXConstructExpr* Ctor, const MatchFinder::MatchResult& Result, + TransferState& State) { + RecordStorageLocation& Loc = State.Env.getResultObjectLocation(*Ctor); + + const Expr* Arg = Ctor->getArg(0); + RecordStorageLocation* SrcLoc = nullptr; + if (Arg->isGLValue()) { + SrcLoc = State.Env.get(*Arg); + } else { + SrcLoc = &State.Env.getResultObjectLocation(*Arg); + } + + if (SrcLoc) { + ensureGoogleSqlValueInitialized(*SrcLoc, State.Env); + auto SrcState = getGoogleSqlValueNullState(*SrcLoc, State.Env); + setGoogleSqlValueNullState(Loc, State.Env, SrcState.IsNull); + } +} + +// Transfer function for factory methods that create non-null values +// (e.g. Value::Int64(...)) -> Non-Null. +static void transferFactoryMethod( + const CallExpr* CE, const MatchFinder::MatchResult& Result, + TransferState& State) { + auto& A = State.Env.arena(); + RecordStorageLocation& Loc = State.Env.getResultObjectLocation(*CE); + + // Overwrite null state to definitely non-null. + setGoogleSqlValueNullState(Loc, State.Env, &A.makeLiteral(false)); +} + +// Transfer function for factory methods that create null values +// (e.g. Value::NullInt64()) -> Null. +// These methods explicitly create a null value, so we mark the state as null. +static void transferNullFactoryMethod( + const CallExpr* CE, const MatchFinder::MatchResult& Result, + TransferState& State) { + auto& A = State.Env.arena(); + RecordStorageLocation& Loc = State.Env.getResultObjectLocation(*CE); + + // Overwrite null state to definitely null. + setGoogleSqlValueNullState(Loc, State.Env, &A.makeLiteral(true)); +} + +// Transfer function for .is_null() call: tie result to internal IsNull formula. +static void transferIsNullCall( + const CXXMemberCallExpr* MCE, const MatchFinder::MatchResult& Result, + TransferState& State) { + auto& A = State.Env.arena(); + + RecordStorageLocation* RecordLoc = + dataflow::getImplicitObjectLocation(*MCE, State.Env); + if (!RecordLoc) return; + + ensureGoogleSqlValueInitialized(*RecordLoc, State.Env); + auto NullState = getGoogleSqlValueNullState(*RecordLoc, State.Env); + + if (NullState.IsNull != nullptr) { + State.Env.setValue(*MCE, A.makeBoolValue(*NullState.IsNull)); + } else { + State.Env.setValue(*MCE, A.makeTopValue()); + } +} + +// Transfer function for assignment operator: v1 = v2. +static void transferAssignment( + const CXXOperatorCallExpr* OCE, const MatchFinder::MatchResult& Result, + TransferState& State) { + auto* DestLoc = cast_or_null( + State.Env.getStorageLocation(*OCE->getArg(0))); + if (!DestLoc) return; + + const Expr* SrcExpr = OCE->getArg(1); + RecordStorageLocation* SrcLoc = nullptr; + if (SrcExpr->isGLValue()) { + SrcLoc = State.Env.get(*SrcExpr); + } else { + SrcLoc = &State.Env.getResultObjectLocation(*SrcExpr); + } + + if (SrcLoc) { + ensureGoogleSqlValueInitialized(*SrcLoc, State.Env); + auto SrcState = getGoogleSqlValueNullState(*SrcLoc, State.Env); + setGoogleSqlValueNullState(*DestLoc, State.Env, SrcState.IsNull); + } +} + +// Matcher for googlesql::Value type. +static auto isGoogleSqlValue() { + return hasType(hasCanonicalType( + recordType(hasDeclaration(cxxRecordDecl(hasName("googlesql::Value")))))); +} + +// Build the transferer CFGMatchSwitch. +static dataflow::CFGMatchSwitch> +buildTransferer() { + return CFGMatchSwitchBuilder< + TransferState>() + // Default constructor: Value(). + .CaseOfCFGStmt( + cxxConstructExpr( + hasDeclaration(cxxConstructorDecl(isDefaultConstructor())), + isGoogleSqlValue()), + transferDefaultConstructor) + // Copy/Move constructor. + .CaseOfCFGStmt( + cxxConstructExpr(hasDeclaration(cxxConstructorDecl(anyOf( + isCopyConstructor(), isMoveConstructor()))), + isGoogleSqlValue()), + transferCopyOrMoveConstructor) + // Null factory methods: e.g. Value::NullInt64(). + // Explicitly creates a null value. + .CaseOfCFGStmt( + callExpr( + callee(cxxMethodDecl(isStaticStorageClass(), + ofClass(hasName("googlesql::Value")), + matchesName("::googlesql::Value::Null"))), + isGoogleSqlValue()), + transferNullFactoryMethod) + // Factory methods: e.g. Value::Int64(...). Creates a non-null value. + // We exclude the Null* methods handled above. + .CaseOfCFGStmt( + callExpr( + callee(cxxMethodDecl( + isStaticStorageClass(), ofClass(hasName("googlesql::Value")), + unless(matchesName("::googlesql::Value::Null")))), + isGoogleSqlValue()), + transferFactoryMethod) + // .is_null() call. + .CaseOfCFGStmt( + cxxMemberCallExpr(callee(cxxMethodDecl( + hasName("is_null"), ofClass(hasName("googlesql::Value"))))), + transferIsNullCall) + // operator=. + .CaseOfCFGStmt( + cxxOperatorCallExpr(hasOverloadedOperatorName("="), + hasArgument(0, isGoogleSqlValue())), + transferAssignment) + .Build(); +} + +// Diagnosis lambda for accessor calls. +static llvm::SmallVector diagnoseAccessorCall( + const CXXMemberCallExpr* MCE, const MatchFinder::MatchResult& Result, + const dataflow::Environment& Env) { + auto& A = Env.arena(); + + RecordStorageLocation* RecordLoc = + dataflow::getImplicitObjectLocation(*MCE, Env); + if (!RecordLoc) return {}; + + if (!hasGoogleSqlValueNullState(*RecordLoc, Env)) { + // Unmodeled value, conservatively assume unsafe. + return {MCE}; + } + + auto NullState = getGoogleSqlValueNullState(*RecordLoc, Env); + if (NullState.IsNull == nullptr) { + // Top (unknown), conservatively assume unsafe. + return {MCE}; + } + + // Verify that we can prove the value is NOT null. + if (!Env.proves(A.makeNot(*NullState.IsNull))) { + return {MCE}; // Unsafe access! + } + + return {}; // Safe access. +} + +// Build the diagnoser CFGMatchSwitch. +static dataflow::CFGMatchSwitch> +buildDiagnoser() { + return CFGMatchSwitchBuilder>() + .CaseOfCFGStmt( + cxxMemberCallExpr(callee(cxxMethodDecl( + ofClass(hasName("googlesql::Value")), + // Catch-all for the entire googlesql::Value class, except for the + // two methods we know are safe to call on a null object! + unless(hasAnyName("is_null", "type"))))), + diagnoseAccessorCall) + .Build(); +} + +// Dispatches to specific transfer functions based on the CFG element type. +// This method is called by the Clang Dataflow framework for each element in the +// CFG. +void GoogleSqlValueNullabilityAnalysis::transfer( + const CFGElement& Elt, GoogleSqlValueNullabilityLattice& Lattice, + dataflow::Environment& Env) { + static auto* Switch = new dataflow::CFGMatchSwitch< + TransferState>(buildTransferer()); + TransferState State{Lattice, Env}; + (*Switch)(Elt, getASTContext(), State); +} + +// The diagnosis function for the analysis. +// Checks for unsafe accesses to googlesql::Value objects and returns the +// statements that violate null safety. +llvm::SmallVector diagnoseGoogleSqlValueNullability( + const CFGElement& Elt, ASTContext& ASTCtx, + const dataflow::Environment& Env) { + static auto* Switch = + new dataflow::CFGMatchSwitch>( + buildDiagnoser()); + return (*Switch)(Elt, ASTCtx, Env); +} + +} // namespace nullability +} // namespace tidy +} // namespace clang diff --git a/nullability/googlesql_value_nullability_analysis.h b/nullability/googlesql_value_nullability_analysis.h new file mode 100644 index 000000000..1a5d4733f --- /dev/null +++ b/nullability/googlesql_value_nullability_analysis.h @@ -0,0 +1,59 @@ +#include "clang/AST/Stmt.h" +#include "clang/Analysis/CFG.h" +// Part of the Crubit project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#ifndef CRUBIT_NULLABILITY_GOOGLESQL_VALUE_NULLABILITY_ANALYSIS_H_ +#define CRUBIT_NULLABILITY_GOOGLESQL_VALUE_NULLABILITY_ANALYSIS_H_ + +#include "nullability/googlesql_value_nullability_lattice.h" +#include "clang/AST/ASTContext.h" +#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h" +#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h" +#include "llvm/ADT/SmallVector.h" + +namespace clang { +namespace tidy { +namespace nullability { + +// Custom dataflow analysis to track the nullability of `googlesql::Value` +// objects. +// +// This analysis ensures that accessor methods on `googlesql::Value` are only +// called after a successful `is_null()` check, preventing runtime crashes. +// It uses synthetic fields on `RecordStorageLocation` to track the nullability +// state across control flow merges. +class GoogleSqlValueNullabilityAnalysis + : public dataflow::DataflowAnalysis { + public: + explicit GoogleSqlValueNullabilityAnalysis(ASTContext& Context) + : DataflowAnalysis(Context) {} + + // Returns the initial lattice element for the analysis. + GoogleSqlValueNullabilityLattice initialElement() { return {}; } + + // Dispatches to specific transfer functions based on the CFG element type. + // This method is called by the Clang Dataflow framework for each element in + // the CFG. + void transfer(const CFGElement& Elt, + GoogleSqlValueNullabilityLattice& Lattice, + dataflow::Environment& Env); +}; + +// Diagnoses unsafe accesses to `googlesql::Value` objects. +// +// Returns a list of statements where an accessor method is called on a +// `googlesql::Value` object that is not proven to be non-null. These statements +// represent potential runtime crashes. +llvm::SmallVector diagnoseGoogleSqlValueNullability( + const CFGElement& Elt, ASTContext& ASTCtx, + const dataflow::Environment& Env); + +} // namespace nullability +} // namespace tidy +} // namespace clang + +#endif // CRUBIT_NULLABILITY_GOOGLESQL_VALUE_NULLABILITY_ANALYSIS_H_ diff --git a/nullability/googlesql_value_nullability_analysis_test.cc b/nullability/googlesql_value_nullability_analysis_test.cc new file mode 100644 index 000000000..568a251dd --- /dev/null +++ b/nullability/googlesql_value_nullability_analysis_test.cc @@ -0,0 +1,284 @@ +// Part of the Crubit project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include "nullability/googlesql_value_nullability_analysis.h" + +#include +#include +#include + +#include "nullability/googlesql_value_nullability_lattice.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Analysis/CFG.h" +#include "clang/Analysis/FlowSensitive/AdornedCFG.h" +#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h" +#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h" +#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h" +#include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h" +#include "clang/Testing/CommandLineArgs.h" +#include "clang/Testing/TestAST.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Testing/Annotations/Annotations.h" +#include "external/llvm-project/third-party/unittest/googlemock/include/gmock/gmock.h" +#include "external/llvm-project/third-party/unittest/googletest/include/gtest/gtest.h" + +namespace clang::tidy::nullability { + +// Preamble providing a stub for googlesql::Value. +static const char* kPreamble = R"cpp( + namespace googlesql { + class Value { + public: + Value(); + static Value Int64(int v); + static Value NullInt64(); + bool is_null() const; + int value() const; + int Int64Value() const; + Value field(int i) const; + }; + } // namespace googlesql +)cpp"; + +static void runTest(llvm::StringRef TestCode) { + std::string FullCode = std::string(kPreamble) + "\n" + TestCode.str(); + llvm::Annotations AnnotatedCode(FullCode); + + TestInputs Inputs(AnnotatedCode.code()); + Inputs.Language = TestLanguage::Lang_CXX17; + + TestAST AST(Inputs); + ASTContext& Ctx = AST.context(); + + // Find the target function. + auto MatchResult = ast_matchers::match( + ast_matchers::functionDecl(ast_matchers::hasName("target")) + .bind("target"), + Ctx); + ASSERT_FALSE(MatchResult.empty()) << "Could not find function 'target'"; + const auto* Target = MatchResult.front().getNodeAs("target"); + + auto CFG = dataflow::AdornedCFG::build(*Target); + ASSERT_TRUE(!!CFG); + + dataflow::DataflowAnalysisContext::Options Opts; + dataflow::DataflowAnalysisContext DACtx( + std::make_unique(), Opts); + dataflow::Environment Env(DACtx, *Target); + + GoogleSqlValueNullabilityAnalysis Analysis(Ctx); + + std::set ActualLines; + dataflow::CFGEltCallbacks Callbacks; + Callbacks.Before = [&](const CFGElement& Elt, + const dataflow::DataflowAnalysisState< + GoogleSqlValueNullabilityLattice>& State) { + auto Diags = diagnoseGoogleSqlValueNullability(Elt, Ctx, State.Env); + for (const Stmt* S : Diags) { + ActualLines.insert( + Ctx.getSourceManager().getPresumedLineNumber(S->getBeginLoc())); + } + }; + + auto Result = dataflow::runDataflowAnalysis(*CFG, Analysis, Env, Callbacks); + ASSERT_TRUE(!!Result); + + // Get expected lines from annotations (points). + std::set ExpectedLines; + for (const auto& Point : AnnotatedCode.points()) { + ExpectedLines.insert(Ctx.getSourceManager().getPresumedLineNumber( + Ctx.getSourceManager() + .getLocForStartOfFile(Ctx.getSourceManager().getMainFileID()) + .getLocWithOffset(Point))); + } + + EXPECT_THAT(ActualLines, testing::ContainerEq(ExpectedLines)) + << "Diagnostics mismatch. Expected at lines: " + << testing::PrintToString(ExpectedLines) + << ", but found at lines: " << testing::PrintToString(ActualLines); +} + +namespace { + +TEST(GoogleSqlValueNullabilityTest, DefaultConstructorIsUnsafe) { + runTest(R"cpp( + void target() { + googlesql::Value v; + v.value(); // ^ + } + )cpp"); +} + +TEST(GoogleSqlValueNullabilityTest, FactoryMethodIsSafe) { + runTest(R"cpp( + void target() { + googlesql::Value v = googlesql::Value::Int64(1); + v.value(); // Safe! No annotation. + } + )cpp"); +} + +TEST(GoogleSqlValueNullabilityTest, ExplicitCheckIsSafe) { + runTest(R"cpp( + void target(googlesql::Value v) { + if (!v.is_null()) { + v.value(); // Safe! + } + } + )cpp"); +} + +TEST(GoogleSqlValueNullabilityTest, UncheckedAccessIsUnsafe) { + runTest(R"cpp( + void target(googlesql::Value v) { + v.value(); // ^ Unsafe! + } + )cpp"); +} + +TEST(GoogleSqlValueNullabilityTest, EarlyReturnIsSafe) { + runTest(R"cpp( + void target(googlesql::Value v) { + if (v.is_null()) return; + v.value(); // Safe! + } + )cpp"); +} + +TEST(GoogleSqlValueNullabilityTest, CheckedInBranchAccessIsSafe) { + runTest(R"cpp( + void target(googlesql::Value v) { + if (v.is_null()) { + // do nothing + } else { + v.value(); // Safe! + } + } + )cpp"); +} + +TEST(GoogleSqlValueNullabilityTest, LogicalAndCheckIsSafe) { + runTest(R"cpp( + void target(googlesql::Value v) { + if (!v.is_null() && v.value() > 0) { // Safe! + // ... + } + } + )cpp"); +} + +TEST(GoogleSqlValueNullabilityTest, LogicalOrCheckIsUnsafe) { + runTest(R"cpp( + void target(googlesql::Value v) { + if (!v.is_null() || v.value() > 0) { // ^ + // ... + } + } + )cpp"); +} + +TEST(GoogleSqlValueNullabilityTest, TernaryOperatorIsSafe) { + runTest(R"cpp( + int target(googlesql::Value v) { + return v.is_null() ? 0 : v.value(); // Safe! + } + )cpp"); +} + +TEST(GoogleSqlValueNullabilityTest, ReassignmentClearsNullability) { + runTest(R"cpp( + void target() { + googlesql::Value v; + v.value(); // ^ Unsafe! + + v = googlesql::Value::Int64(1); + v.value(); // Safe! + + v = googlesql::Value(); + v.value(); // ^ Unsafe again! + } + )cpp"); +} + +TEST(GoogleSqlValueNullabilityTest, ChainedAccessIsUnsafe) { + runTest(R"cpp( + void target(googlesql::Value v) { + if (!v.is_null()) { + googlesql::Value f = v.field(0); + f.value(); // ^ + } + } + )cpp"); +} + +TEST(GoogleSqlValueNullabilityTest, ReferenceAliasingIsSafe) { + runTest(R"cpp( + void target(googlesql::Value v) { + googlesql::Value& r = v; + if (!r.is_null()) { + v.value(); // Safe! + } + } + )cpp"); +} + +TEST(GoogleSqlValueNullabilityTest, CopyConstructorPropagatesSafety) { + runTest(R"cpp( + void target(googlesql::Value v1) { + if (!v1.is_null()) { + googlesql::Value v2 = v1; + v2.value(); // Safe! + } + } + )cpp"); +} + +TEST(GoogleSqlValueNullabilityTest, LoopConditionCheckIsSafe) { + runTest(R"cpp( + void target(googlesql::Value v) { + while (v.is_null()) { + v = googlesql::Value::Int64(1); + } + v.value(); // Safe! + } + )cpp"); +} + +TEST(GoogleSqlValueNullabilityTest, DoubleNegationIsHandled) { + runTest(R"cpp( + void target(googlesql::Value v) { + if (!!v.is_null()) { + v.value(); // ^ + } else { + v.value(); // Safe! + } + } + )cpp"); +} + +TEST(GoogleSqlValueNullabilityTest, NullFactoryMethodIsUnsafe) { + runTest(R"cpp( + void target() { + googlesql::Value v = googlesql::Value::NullInt64(); + v.value(); // ^ + } + )cpp"); +} + +TEST(GoogleSqlValueNullabilityTest, CopyConstructorSharesState) { + runTest(R"cpp( + void target(googlesql::Value v1) { + googlesql::Value v2 = v1; + if (!v1.is_null()) { + v2.value(); // Safe! + } + } + )cpp"); +} + +} // namespace +} // namespace clang::tidy::nullability diff --git a/nullability/googlesql_value_nullability_lattice.h b/nullability/googlesql_value_nullability_lattice.h new file mode 100644 index 000000000..52164b730 --- /dev/null +++ b/nullability/googlesql_value_nullability_lattice.h @@ -0,0 +1,37 @@ +// Part of the Crubit project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#ifndef CRUBIT_NULLABILITY_GOOGLESQL_VALUE_NULLABILITY_LATTICE_H_ +#define CRUBIT_NULLABILITY_GOOGLESQL_VALUE_NULLABILITY_LATTICE_H_ + +#include "clang/Analysis/FlowSensitive/DataflowLattice.h" + +namespace clang { +namespace tidy { +namespace nullability { + +// The lattice for GoogleSQL value nullability analysis is empty in terms of +// tracked analysis state fields. We track all specific states (nullability +// logical formulas) in the `dataflow::Environment` using synthetic fields on +// `RecordStorageLocation`. +// +// However, the Clang Dataflow Analysis framework requires a lattice type to be +// provided as a template parameter to manage the state that joins at control +// flow branches. Therefore, this struct acts as a trivial placeholder with +// no-op `==` and `join` hooks to satisfy the API. +struct GoogleSqlValueNullabilityLattice { + bool operator==(const GoogleSqlValueNullabilityLattice& Other) const { + return true; + } + dataflow::LatticeJoinEffect join( + const GoogleSqlValueNullabilityLattice& Other) { + return dataflow::LatticeJoinEffect::Unchanged; + } +}; + +} // namespace nullability +} // namespace tidy +} // namespace clang + +#endif // THIRD_PARTY_CRUBIT_NULLABILITY_GOOGLESQL_VALUE_NULLABILITY_LATTICE_H_