Skip to content

Commit

Permalink
[analyzer] Report violations of the "returns_nonnull" attribute (llvm…
Browse files Browse the repository at this point in the history
…#106048)

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.

This commit also reverts an old hack introduced by
49bd58f
because it is no longer needed

CPP-4741
  • Loading branch information
necto authored Aug 27, 2024
1 parent 6f62757 commit 4f33e7c
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 8 deletions.
22 changes: 19 additions & 3 deletions clang/docs/analyzer/checkers.rst
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@ if ((y = make_int())) {
nullability
^^^^^^^^^^^
Objective C checkers that warn for null pointer passing and dereferencing errors.
Checkers (mostly Objective C) that warn for null pointer passing and dereferencing errors.
.. _nullability-NullPassedToNonnull:
Expand All @@ -588,8 +588,8 @@ Warns when a null pointer is passed to a pointer which has a _Nonnull type.
.. _nullability-NullReturnedFromNonnull:
nullability.NullReturnedFromNonnull (ObjC)
""""""""""""""""""""""""""""""""""""""""""
nullability.NullReturnedFromNonnull (C, C++, ObjC)
""""""""""""""""""""""""""""""""""""""""""""""""""
Warns when a null pointer is returned from a function that has _Nonnull return type.
.. code-block:: objc
Expand All @@ -604,6 +604,22 @@ Warns when a null pointer is returned from a function that has _Nonnull return t
return result;
}
Warns when a null pointer is returned from a function annotated with ``__attribute__((returns_nonnull))``
.. code-block:: cpp
int global;
__attribute__((returns_nonnull)) void* getPtr(void* p);
void* getPtr(void* p) {
if (p) { // forgot to negate the condition
return &global;
}
// Warning: nullptr returned from a function that is expected
// to return a non-null value
return p;
}
.. _nullability-NullableDereferenced:
nullability.NullableDereferenced (ObjC)
Expand Down
10 changes: 9 additions & 1 deletion clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -705,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)
Expand Down
59 changes: 56 additions & 3 deletions clang/test/Analysis/nullability.c
Original file line number Diff line number Diff line change
@@ -1,12 +1,65 @@
// 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 -verify %s

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(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(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(int* ptr) __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(void) __attribute__((returns_nonnull));

__attribute__((returns_nonnull))
int *cannot_return_null(void) {
int *x = produce_nonnull_ptr();
if (!x) {
clang_analyzer_warnIfReached();
// 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.
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
}

__attribute__((returns_nonnull)) int *passthrough2(int *p);
int *passthrough2(int *p) {
return p; // expected-warning{{Null returned from a function that is expected to return a non-null value}}
}

void call_with_null(void) {
passthrough2(0);
}
2 changes: 1 addition & 1 deletion clang/test/Analysis/nullability.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 4f33e7c

Please sign in to comment.