llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-static-analyzer-1 Author: Balázs Kéri (balazske) <details> <summary>Changes</summary> --- Full diff: https://github.com/llvm/llvm-project/pull/157129.diff 6 Files Affected: - (modified) clang/docs/analyzer/checkers.rst (+41) - (modified) clang/include/clang/StaticAnalyzer/Checkers/Checkers.td (+5) - (modified) clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp (+132-12) - (modified) clang/test/Analysis/analyzer-enabled-checkers.c (+1) - (added) clang/test/Analysis/null-pointer-arithm.c (+76) - (modified) clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c (+1) ``````````diff diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index b2effadacf9f1..b3982391358c6 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -205,6 +205,47 @@ pointers with a specified address space. If the option is set to false, then reports from the specific x86 address spaces 256, 257 and 258 are still suppressed, but null dereferences from other address spaces are reported. +.. _core-NullPointerArithm: + +core.NullPointerArithm (C, C++) +""""""""""""""""""""""""""""""" +Check for undefined arithmetic operations with null pointers. + +The checker can detect the following cases: + + - `p + x` and `x + p` where `p` is a null pointer and `x` is a nonzero integer + value. + - `p - x` where `p` is a null pointer and `x` is a nonzero integer + value. + - `p1` - `p2` where one of `p1` and `p2` is null and the other a non-null + pointer. + +Result of these operations is undefined according to the standard. + +.. code-block:: c + + void test1(int *p, int offset) { + if (p) + return; + + int *p1 = p + offset; // warn: 'p' is null, 'offset' can be likely non-zero + } + + void test2(int *p, int offset) { + if (p) { ... } + if (offset == 0) + return; + + int *p1 = p - offset; // warn: 'p' can be null, 'offset' is non-zero + } + + void test3(char *p1, char *p2) { + if (p1) + return; + + int a = p1 - p2; // warn: 'p1' is null, 'p2' can be likely non-null + } + .. _core-StackAddressEscape: core.StackAddressEscape (C) diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index 73f702de581d9..a6f504c7edcb0 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -216,6 +216,11 @@ def NullDereferenceChecker HelpText<"Check for dereferences of null pointers">, Documentation<HasDocumentation>; +def NullPointerArithmChecker + : Checker<"NullPointerArithm">, + HelpText<"Check for undefined arithmetic operations on null pointers">, + Documentation<HasDocumentation>; + def NonNullParamChecker : Checker<"NonNullParamChecker">, HelpText<"Check for null pointers passed as arguments to a function whose " "arguments are references or marked with the 'nonnull' attribute">, diff --git a/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp index 395d724cdfd11..ee37af9ed0c3d 100644 --- a/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp @@ -39,9 +39,10 @@ class DerefBugType : public BugType { class DereferenceChecker : public CheckerFamily<check::Location, check::Bind, + check::PreStmt<BinaryOperator>, EventDispatcher<ImplicitNullDerefEvent>> { - void reportBug(const DerefBugType &BT, ProgramStateRef State, const Stmt *S, - CheckerContext &C) const; + void reportDerefBug(const DerefBugType &BT, ProgramStateRef State, + const Stmt *S, CheckerContext &C) const; bool suppressReport(CheckerContext &C, const Expr *E) const; @@ -50,6 +51,7 @@ class DereferenceChecker CheckerContext &C) const; void checkBind(SVal L, SVal V, const Stmt *S, bool AtDeclInit, CheckerContext &C) const; + void checkPreStmt(const BinaryOperator *Op, CheckerContext &C) const; static void AddDerefSource(raw_ostream &os, SmallVectorImpl<SourceRange> &Ranges, @@ -57,7 +59,7 @@ class DereferenceChecker const LocationContext *LCtx, bool loadedFrom = false); - CheckerFrontend NullDerefChecker, FixedDerefChecker; + CheckerFrontend NullDerefChecker, FixedDerefChecker, NullPointerArithmChecker; const DerefBugType NullBug{&NullDerefChecker, "Dereference of null pointer", "a null pointer dereference", "a dereference of a null pointer"}; @@ -72,6 +74,9 @@ class DereferenceChecker const DerefBugType FixedAddressBug{&FixedDerefChecker, "Dereference of a fixed address", "a dereference of a fixed address"}; + const BugType NullPointerArithmBug{ + &NullPointerArithmChecker, + "Possible undefined arithmetic operation involving a null pointer"}; StringRef getDebugTag() const override { return "DereferenceChecker"; } }; @@ -173,9 +178,11 @@ static bool isDeclRefExprToReference(const Expr *E) { return false; } -void DereferenceChecker::reportBug(const DerefBugType &BT, - ProgramStateRef State, const Stmt *S, - CheckerContext &C) const { +void DereferenceChecker::reportDerefBug(const DerefBugType &BT, + ProgramStateRef State, const Stmt *S, + CheckerContext &C) const { + assert(&BT != &NullPointerArithmBug && "Invalid use of function"); + if (&BT == &FixedAddressBug) { if (!FixedDerefChecker.isEnabled()) // Deliberately don't add a sink node if check is disabled. @@ -262,7 +269,7 @@ void DereferenceChecker::checkLocation(SVal l, bool isLoad, const Stmt* S, if (l.isUndef()) { const Expr *DerefExpr = getDereferenceExpr(S); if (!suppressReport(C, DerefExpr)) - reportBug(UndefBug, C.getState(), DerefExpr, C); + reportDerefBug(UndefBug, C.getState(), DerefExpr, C); return; } @@ -283,7 +290,7 @@ void DereferenceChecker::checkLocation(SVal l, bool isLoad, const Stmt* S, // we call an "explicit" null dereference. const Expr *expr = getDereferenceExpr(S); if (!suppressReport(C, expr)) { - reportBug(NullBug, nullState, expr, C); + reportDerefBug(NullBug, nullState, expr, C); return; } } @@ -301,7 +308,7 @@ void DereferenceChecker::checkLocation(SVal l, bool isLoad, const Stmt* S, if (location.isConstant()) { const Expr *DerefExpr = getDereferenceExpr(S, isLoad); if (!suppressReport(C, DerefExpr)) - reportBug(FixedAddressBug, notNullState, DerefExpr, C); + reportDerefBug(FixedAddressBug, notNullState, DerefExpr, C); return; } @@ -317,7 +324,7 @@ void DereferenceChecker::checkBind(SVal L, SVal V, const Stmt *S, // One should never write to label addresses. if (auto Label = L.getAs<loc::GotoLabel>()) { - reportBug(LabelBug, C.getState(), S, C); + reportDerefBug(LabelBug, C.getState(), S, C); return; } @@ -338,7 +345,7 @@ void DereferenceChecker::checkBind(SVal L, SVal V, const Stmt *S, if (!StNonNull) { const Expr *expr = getDereferenceExpr(S, /*IsBind=*/true); if (!suppressReport(C, expr)) { - reportBug(NullBug, StNull, expr, C); + reportDerefBug(NullBug, StNull, expr, C); return; } } @@ -356,7 +363,7 @@ void DereferenceChecker::checkBind(SVal L, SVal V, const Stmt *S, if (V.isConstant()) { const Expr *DerefExpr = getDereferenceExpr(S, true); if (!suppressReport(C, DerefExpr)) - reportBug(FixedAddressBug, State, DerefExpr, C); + reportDerefBug(FixedAddressBug, State, DerefExpr, C); return; } @@ -379,6 +386,111 @@ void DereferenceChecker::checkBind(SVal L, SVal V, const Stmt *S, C.addTransition(State, this); } +void DereferenceChecker::checkPreStmt(const BinaryOperator *Op, + CheckerContext &C) const { + if (!Op->isAdditiveOp()) + return; + const Expr *E1 = Op->getLHS(); + const Expr *E2 = Op->getRHS(); + QualType T1 = E1->getType().getCanonicalType(); + QualType T2 = E2->getType().getCanonicalType(); + if (T1->isIntegerType() && T2->isIntegerType()) + return; + if (!T1->isPointerType() && !T1->isIntegerType() && !T2->isPointerType() && + !T2->isIntegerType()) + return; + + ProgramStateRef State = C.getState(); + SVal V1 = State->getSVal(E1, C.getLocationContext()); + SVal V2 = State->getSVal(E2, C.getLocationContext()); + if (V1.isUndef() || V2.isUndef()) + return; + + ConditionTruthVal V1IsNull = State->isNull(V1); + ConditionTruthVal V2IsNull = State->isNull(V2); + bool IsConstrained = true; + + // Check cases 'NULL + x' and 'NULL - x' + if (T1->isPointerType() && T2->isIntegerType()) { + if (!V1IsNull.isConstrainedTrue() || V2IsNull.isConstrainedTrue()) + return; + IsConstrained = V2IsNull.isConstrainedFalse(); + } + + // Check case 'x + NULL' + if (T1->isIntegerType() && T2->isPointerType()) { + if (V1IsNull.isConstrainedTrue() || !V2IsNull.isConstrainedTrue()) + return; + IsConstrained = V1IsNull.isConstrainedFalse(); + } + + // Check case 'NULL - p' or 'p - NULL' + if (T1->isPointerType() && T2->isPointerType()) { + if (!V1IsNull.isConstrainedTrue() && !V2IsNull.isConstrainedTrue()) + return; + if (V1IsNull.isConstrainedTrue() && V2IsNull.isConstrainedTrue()) + return; + IsConstrained = + V1IsNull.isConstrainedFalse() || V2IsNull.isConstrainedFalse(); + } + + SmallString<100> Buf; + llvm::raw_svector_ostream Out(Buf); + SmallVector<SourceRange, 2> Ranges; + + auto AddSubExprStr = [&](const Expr *E, bool IsPointer, + ConditionTruthVal IsNull) { + if (IsNull.isConstrainedTrue()) { + if (IsPointer) + Out << "null pointer"; + else + Out << "zero"; + } else { + if (!IsNull.isConstrainedFalse()) + Out << "probably "; + if (IsPointer) + Out << "non-null pointer"; + else + Out << "nonzero integer value"; + } + if (IsPointer) + AddDerefSource(Out, Ranges, E, State.get(), C.getLocationContext(), + false); + }; + + if (Op->getOpcode() == BO_Add) + Out << "Addition of a "; + else + Out << "Subtraction of a "; + AddSubExprStr(E1, T1->isPointerType(), V1IsNull); + Out << " and a "; + AddSubExprStr(E2, T2->isPointerType(), V2IsNull); + + if (IsConstrained) + Out << " results "; + else + Out << " may result "; + Out << "in undefined behavior"; + + ExplodedNode *N = C.generateErrorNode(State); + if (!N) + return; + auto BR = std::make_unique<PathSensitiveBugReport>(NullPointerArithmBug, + Buf.str(), N); + + if (T1->isPointerType()) + bugreporter::trackExpressionValue(N, E1, *BR); + if (T2->isPointerType()) + bugreporter::trackExpressionValue(N, E2, *BR); + + for (SmallVectorImpl<SourceRange>::iterator I = Ranges.begin(), + E = Ranges.end(); + I != E; ++I) + BR->addRange(*I); + + C.emitReport(std::move(BR)); +} + void ento::registerNullDereferenceChecker(CheckerManager &Mgr) { Mgr.getChecker<DereferenceChecker>()->NullDerefChecker.enable(Mgr); } @@ -395,3 +507,11 @@ bool ento::shouldRegisterFixedAddressDereferenceChecker( const CheckerManager &) { return true; } + +void ento::registerNullPointerArithmChecker(CheckerManager &Mgr) { + Mgr.getChecker<DereferenceChecker>()->NullPointerArithmChecker.enable(Mgr); +} + +bool ento::shouldRegisterNullPointerArithmChecker(const CheckerManager &) { + return true; +} diff --git a/clang/test/Analysis/analyzer-enabled-checkers.c b/clang/test/Analysis/analyzer-enabled-checkers.c index 32afcf30646bf..e213a3d4417ff 100644 --- a/clang/test/Analysis/analyzer-enabled-checkers.c +++ b/clang/test/Analysis/analyzer-enabled-checkers.c @@ -20,6 +20,7 @@ // CHECK-NEXT: core.NonNullParamChecker // CHECK-NEXT: core.NonnilStringConstants // CHECK-NEXT: core.NullDereference +// CHECK-NEXT: core.NullPointerArithm // CHECK-NEXT: core.StackAddressEscape // CHECK-NEXT: core.UndefinedBinaryOperatorResult // CHECK-NEXT: core.VLASize diff --git a/clang/test/Analysis/null-pointer-arithm.c b/clang/test/Analysis/null-pointer-arithm.c new file mode 100644 index 0000000000000..6395abd7e4db9 --- /dev/null +++ b/clang/test/Analysis/null-pointer-arithm.c @@ -0,0 +1,76 @@ +// RUN: %clang_analyze_cc1 -verify %s \ +// RUN: -analyzer-checker=core + +extern int *get_pointer(); + +int *test_add1(int offset) { + int *p = get_pointer(); + if (p) {} + return p + offset; // expected-warning{{Addition of a null pointer (from variable 'p') and a probably nonzero integer value may result in undefined behavior}} +} + +int *test_add2(int offset) { + int *p = get_pointer(); + if (p) {} + if (offset) {} + return p + offset; // expected-warning{{Addition of a null pointer (from variable 'p') and a nonzero integer value results in undefined behavior}} +} + +int *test_add3(int offset) { + int *p = get_pointer(); + if (p) {} + if (offset != 0) return 0; + return p + offset; +} + +int *test_add4(int offset) { + int *p = get_pointer(); + if (p) {} + if (offset == 0) return 0; + return p + offset; // expected-warning{{Addition of a null pointer (from variable 'p') and a nonzero integer value results in undefined behavior}} +} + +int *test_add5(int offset) { + int *p = get_pointer(); + if (p) {} + return offset + p; // expected-warning{{Addition of a probably nonzero integer value and a null pointer (from variable 'p') may result in undefined behavior}} +} + +int *test_sub1(int offset) { + int *p = get_pointer(); + if (p) {} + return p - offset; // expected-warning{{Subtraction of a null pointer (from variable 'p') and a probably nonzero integer value may result in undefined behavior}} +} + +int test_sub_p1() { + int *p = get_pointer(); + if (p) {} + return p - p; +} + +int test_sub_p2() { + int *p1 = get_pointer(); + int *p2 = get_pointer(); + if (p1) {} + if (p2) {} + return p1 - p2; + // expected-warning@-1{{Subtraction of a non-null pointer (from variable 'p1') and a null pointer (from variable 'p2') results in undefined behavior}} + // expected-warning@-2{{Subtraction of a null pointer (from variable 'p1') and a non-null pointer (from variable 'p2') results in undefined behavior}} +} + +int test_sub_p3() { + int *p1 = get_pointer(); + int *p2 = get_pointer(); + if (p1) {} + return p1 - p2; // expected-warning{{Subtraction of a null pointer (from variable 'p1') and a probably non-null pointer (from variable 'p2') may result in undefined behavior}} +} + +struct S { + char *p; + int offset; +}; + +char *test_struct(struct S s) { + if (s.p) {} + return s.p + s.offset; // expected-warning{{Addition of a null pointer (via field 'p') and a probably nonzero integer value may result in undefined behavior}} +} diff --git a/clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c b/clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c index 77fa037259440..f03b5c229059a 100644 --- a/clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c +++ b/clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c @@ -28,6 +28,7 @@ // CHECK-NEXT: core.NonNullParamChecker // CHECK-NEXT: core.NonnilStringConstants // CHECK-NEXT: core.NullDereference +// CHECK-NEXT: core.NullPointerArithm // CHECK-NEXT: core.StackAddressEscape // CHECK-NEXT: core.UndefinedBinaryOperatorResult // CHECK-NEXT: core.VLASize `````````` </details> https://github.com/llvm/llvm-project/pull/157129 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
