https://github.com/necto updated https://github.com/llvm/llvm-project/pull/106048
>From 7d5ae515f7727de98e7e8ce2f259e579a1f24463 Mon Sep 17 00:00:00 2001 From: Arseniy Zaostrovnykh <necto...@gmail.com> Date: Tue, 20 Aug 2024 17:31:11 +0200 Subject: [PATCH 1/5] [analyzer] Report violations of the "returns_nonnull" attribute Make sure code respects the GNU-extension __attribute__((returns_nonnull)). Extend the NullabilityChecker to check that a function returns_nonnull does not return a nullptr. CPP-4741 --- .../Checkers/NullabilityChecker.cpp | 8 ++++ clang/test/Analysis/nullability.c | 43 ++++++++++++++++++- 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp index 60934e51febe84..2035d50eea4c2d 100644 --- a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp @@ -692,6 +692,14 @@ void NullabilityChecker::checkPreStmt(const ReturnStmt *S, NullConstraint Nullness = getNullConstraint(*RetSVal, State); Nullability RequiredNullability = getNullabilityAnnotation(RequiredRetType); + if (const auto *FunDecl = C.getLocationContext()->getDecl(); + FunDecl && FunDecl->getAttr<ReturnsNonNullAttr>() && + (RequiredNullability == Nullability::Unspecified || + RequiredNullability == Nullability::Nullable)) { + // If a function is marked with the returns_nonnull attribute, + // the return value must be non-null. + RequiredNullability = Nullability::Nonnull; + } // If the returned value is null but the type of the expression // generating it is nonnull then we will suppress the diagnostic. diff --git a/clang/test/Analysis/nullability.c b/clang/test/Analysis/nullability.c index fbc03c864ad83f..9ddb9c8d2dc34f 100644 --- a/clang/test/Analysis/nullability.c +++ b/clang/test/Analysis/nullability.c @@ -1,4 +1,6 @@ -// RUN: %clang_analyze_cc1 -fblocks -analyzer-checker=core,nullability -Wno-deprecated-non-prototype -verify %s +// RUN: %clang_analyze_cc1 -fblocks -analyzer-checker=core,nullability,debug.ExprInspection -Wno-deprecated-non-prototype -verify %s + +void clang_analyzer_warnIfReached(); void it_takes_two(int a, int b); void function_pointer_arity_mismatch() { @@ -10,3 +12,42 @@ void block_arity_mismatch() { void(^b)() = ^(int a, int b) { }; b(1); // no-crash expected-warning {{Block taking 2 arguments is called with fewer (1)}} } + +int *nonnull_return_annotation_indirect() __attribute__((returns_nonnull)); +int *nonnull_return_annotation_indirect() { + int *x = 0; + return x; // expected-warning {{Null returned from a function that is expected to return a non-null value}} +} + +int *nonnull_return_annotation_direct() __attribute__((returns_nonnull)); +int *nonnull_return_annotation_direct() { + return 0; // expected-warning {{Null returned from a function that is expected to return a non-null value}} +} // expected-warning@-1 {{null returned from function that requires a non-null return value}} + +int *nonnull_return_annotation_assumed() __attribute__((returns_nonnull)); +int *nonnull_return_annotation_assumed(int* ptr) { + if (ptr) { + return ptr; + } + return ptr; // expected-warning {{Null returned from a function that is expected to return a non-null value}} +} + +int *produce_nonnull_ptr() __attribute__((returns_nonnull)); + +__attribute__((returns_nonnull)) +int *cannot_return_null() { + int *x = produce_nonnull_ptr(); + if (!x) { + clang_analyzer_warnIfReached(); + // Incorrect: expected-warning@-1 {{REACHABLE}} + // According to produce_nonnull_ptr contract, x cannot be null. + } + // Regardless of the potential state split above, x cannot be nullptr + // according to the produce_nonnull_ptr annotation. + return x; + // False positive: expected-warning@-1 {{Null returned from a function that is expected to return a non-null value}} +} + +__attribute__((returns_nonnull)) int *passthrough(int *p) { + return p; // no-warning: we have no evidence that `p` is null, i.e., violating the contract +} >From d3b30f3f31cc3a4cc71ae967ea6fc554a4764e37 Mon Sep 17 00:00:00 2001 From: Arseniy Zaostrovnykh <arseniy.zaostrovn...@sonarsource.com> Date: Mon, 26 Aug 2024 15:59:59 +0200 Subject: [PATCH 2/5] Update clang/test/Analysis/nullability.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Donát Nagy <donat.n...@ericsson.com> --- clang/test/Analysis/nullability.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/clang/test/Analysis/nullability.c b/clang/test/Analysis/nullability.c index 9ddb9c8d2dc34f..341d29c6e99d1e 100644 --- a/clang/test/Analysis/nullability.c +++ b/clang/test/Analysis/nullability.c @@ -39,8 +39,9 @@ int *cannot_return_null() { int *x = produce_nonnull_ptr(); if (!x) { clang_analyzer_warnIfReached(); - // Incorrect: expected-warning@-1 {{REACHABLE}} - // According to produce_nonnull_ptr contract, x cannot be null. + // expected-warning@-1 {{REACHABLE}} + // TODO: This warning is a false positive, according to the contract of + // produce_nonnull_ptr, x cannot be null. } // Regardless of the potential state split above, x cannot be nullptr // according to the produce_nonnull_ptr annotation. >From 3d5e750aec0d35b20d5c479db74cd0a5d3ea7853 Mon Sep 17 00:00:00 2001 From: Arseniy Zaostrovnykh <necto...@gmail.com> Date: Mon, 26 Aug 2024 16:47:57 +0200 Subject: [PATCH 3/5] Add test for inlined nullability violation --- clang/test/Analysis/nullability.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/clang/test/Analysis/nullability.c b/clang/test/Analysis/nullability.c index 341d29c6e99d1e..92fd077f01cd96 100644 --- a/clang/test/Analysis/nullability.c +++ b/clang/test/Analysis/nullability.c @@ -52,3 +52,15 @@ int *cannot_return_null() { __attribute__((returns_nonnull)) int *passthrough(int *p) { return p; // no-warning: we have no evidence that `p` is null, i.e., violating the contract } + +__attribute__((noreturn)) +void exit(int); + +__attribute__((returns_nonnull)) int *passthrough2(int *p); +int *passthrough2(int *p) { + return p; // FIXME: no-warning Explicitly disabled to avoid FPs +} + +void call_with_null(void) { + passthrough2(0); +} >From 598c574e0703d316c369f5796fd496af725da6f5 Mon Sep 17 00:00:00 2001 From: Arseniy Zaostrovnykh <necto...@gmail.com> Date: Mon, 26 Aug 2024 16:52:35 +0200 Subject: [PATCH 4/5] Undo report suppression for null returned from non-null functions This removes the additional constraint introduced in 49bd58f1ebe28d97e4949e9c757bc5dfd8b2d72f because its premise (producing FP if a parameter reclaimed early) seems to no longer hold, and because it interferes with C coverage. --- clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp | 2 +- clang/test/Analysis/nullability.c | 2 +- clang/test/Analysis/nullability.mm | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp index 2035d50eea4c2d..04472bb3895a78 100644 --- a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp @@ -713,7 +713,7 @@ void NullabilityChecker::checkPreStmt(const ReturnStmt *S, Nullness == NullConstraint::IsNull); if (ChecksEnabled[CK_NullReturnedFromNonnull] && NullReturnedFromNonNull && RetExprTypeLevelNullability != Nullability::Nonnull && - !InSuppressedMethodFamily && C.getLocationContext()->inTopFrame()) { + !InSuppressedMethodFamily) { static CheckerProgramPointTag Tag(this, "NullReturnedFromNonnull"); ExplodedNode *N = C.generateErrorNode(State, &Tag); if (!N) diff --git a/clang/test/Analysis/nullability.c b/clang/test/Analysis/nullability.c index 92fd077f01cd96..ed70336c412016 100644 --- a/clang/test/Analysis/nullability.c +++ b/clang/test/Analysis/nullability.c @@ -58,7 +58,7 @@ void exit(int); __attribute__((returns_nonnull)) int *passthrough2(int *p); int *passthrough2(int *p) { - return p; // FIXME: no-warning Explicitly disabled to avoid FPs + return p; // expected-warning{{Null returned from a function that is expected to return a non-null value}} } void call_with_null(void) { diff --git a/clang/test/Analysis/nullability.mm b/clang/test/Analysis/nullability.mm index d69116d03df746..64222d939bdd06 100644 --- a/clang/test/Analysis/nullability.mm +++ b/clang/test/Analysis/nullability.mm @@ -438,7 +438,7 @@ -(Dummy *)callerWithParam:(Dummy * _Nonnull) p1 { int * _Nonnull InlinedReturnNullOverSuppressionCallee(int * _Nonnull p2) { int *result = 0; - return result; // no-warning; but this is an over suppression + return result; // expected-warning{{Null returned from a function that is expected to return a non-null value}} } int *InlinedReturnNullOverSuppressionCaller(int * _Nonnull p1) { >From a9c489f683fc7d18a2a26933b65bca7b668b708f Mon Sep 17 00:00:00 2001 From: Arseniy Zaostrovnykh <necto...@gmail.com> Date: Mon, 26 Aug 2024 16:57:27 +0200 Subject: [PATCH 5/5] Remove -Wno-deprecated-non-prototype --- clang/test/Analysis/nullability.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/clang/test/Analysis/nullability.c b/clang/test/Analysis/nullability.c index ed70336c412016..4e801b355bd9d5 100644 --- a/clang/test/Analysis/nullability.c +++ b/clang/test/Analysis/nullability.c @@ -1,30 +1,32 @@ -// RUN: %clang_analyze_cc1 -fblocks -analyzer-checker=core,nullability,debug.ExprInspection -Wno-deprecated-non-prototype -verify %s +// RUN: %clang_analyze_cc1 -fblocks -analyzer-checker=core,nullability,debug.ExprInspection -verify %s -void clang_analyzer_warnIfReached(); +void clang_analyzer_warnIfReached(void); void it_takes_two(int a, int b); -void function_pointer_arity_mismatch() { +void function_pointer_arity_mismatch(void) { void(*fptr)() = it_takes_two; fptr(1); // no-crash expected-warning {{Function taking 2 arguments is called with fewer (1)}} + // expected-warning@-1 {{passing arguments to a function without a prototype is deprecated in all versions of C and is not supported in C23}} } -void block_arity_mismatch() { +void block_arity_mismatch(void) { void(^b)() = ^(int a, int b) { }; b(1); // no-crash expected-warning {{Block taking 2 arguments is called with fewer (1)}} + // expected-warning@-1 {{passing arguments to a function without a prototype is deprecated in all versions of C and is not supported in C23}} } -int *nonnull_return_annotation_indirect() __attribute__((returns_nonnull)); -int *nonnull_return_annotation_indirect() { +int *nonnull_return_annotation_indirect(void) __attribute__((returns_nonnull)); +int *nonnull_return_annotation_indirect(void) { int *x = 0; return x; // expected-warning {{Null returned from a function that is expected to return a non-null value}} } -int *nonnull_return_annotation_direct() __attribute__((returns_nonnull)); -int *nonnull_return_annotation_direct() { +int *nonnull_return_annotation_direct(void) __attribute__((returns_nonnull)); +int *nonnull_return_annotation_direct(void) { return 0; // expected-warning {{Null returned from a function that is expected to return a non-null value}} } // expected-warning@-1 {{null returned from function that requires a non-null return value}} -int *nonnull_return_annotation_assumed() __attribute__((returns_nonnull)); +int *nonnull_return_annotation_assumed(int* ptr) __attribute__((returns_nonnull)); int *nonnull_return_annotation_assumed(int* ptr) { if (ptr) { return ptr; @@ -32,10 +34,10 @@ int *nonnull_return_annotation_assumed(int* ptr) { return ptr; // expected-warning {{Null returned from a function that is expected to return a non-null value}} } -int *produce_nonnull_ptr() __attribute__((returns_nonnull)); +int *produce_nonnull_ptr(void) __attribute__((returns_nonnull)); __attribute__((returns_nonnull)) -int *cannot_return_null() { +int *cannot_return_null(void) { int *x = produce_nonnull_ptr(); if (!x) { clang_analyzer_warnIfReached(); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits