george.burgess.iv created this revision.
george.burgess.iv added a reviewer: rtrieu.
george.burgess.iv added a subscriber: cfe-commits.
Addresses a problem brought up by Xavier here:
http://permalink.gmane.org/gmane.comp.compilers.clang.devel/46163
tl;dr: We get a warning on the following code
```
void foo(void *p) __attribute__((nonnull(1))) {
if (p == NULL) {} // warning: always equal to false
}
```
…But not this code:
```
void foo() __attribute__((returns_nonnull));
int main() {
if (foo() == NULL) {} // no warning
}
```
This patch makes us give a warning in the second case.
http://reviews.llvm.org/D15324
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaChecking.cpp
test/Sema/nonnull.c
Index: test/Sema/nonnull.c
===================================================================
--- test/Sema/nonnull.c
+++ test/Sema/nonnull.c
@@ -153,3 +153,17 @@
if (p) // No warning
;
}
+
+__attribute__((returns_nonnull)) void *returns_nonnull_whee();
+
+void returns_nonnull_tests() {
+ if (returns_nonnull_whee() == NULL) {} // expected-warning {{comparison of nonnull function call 'returns_nonnull_whee()' equal to a null pointer is false on first encounter}}
+
+ if (returns_nonnull_whee() != NULL) {} // expected-warning {{comparison of nonnull function call 'returns_nonnull_whee()' not equal to a null pointer is true on first encounter}}
+
+ if (returns_nonnull_whee()) {} // expected-warning {{nonnull function call 'returns_nonnull_whee()' will evaluate to 'true' on first encounter}}
+ if (!returns_nonnull_whee()) {} // expected-warning {{nonnull function call 'returns_nonnull_whee()' will evaluate to 'true' on first encounter}}
+
+ int and_again = !returns_nonnull_whee(); // expected-warning {{nonnull function call 'returns_nonnull_whee()' will evaluate to 'true' on first encounter}}
+ and_again = !returns_nonnull_whee(); // expected-warning {{nonnull function call 'returns_nonnull_whee()' will evaluate to 'true' on first encounter}}
+}
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -1180,8 +1180,7 @@
/// Checks if a the given expression evaluates to null.
///
/// \brief Returns true if the value evaluates to null.
-static bool CheckNonNullExpr(Sema &S,
- const Expr *Expr) {
+static bool CheckNonNullExpr(Sema &S, const Expr *Expr) {
// If the expression has non-null type, it doesn't evaluate to null.
if (auto nullability
= Expr->IgnoreImplicit()->getType()->getNullability(S.Context)) {
@@ -7666,6 +7665,22 @@
}
}
+ auto ComplainAboutNonnullParamOrCall = [&](bool IsParam) {
+ std::string Str;
+ llvm::raw_string_ostream S(Str);
+ E->printPretty(S, nullptr, getPrintingPolicy());
+ unsigned DiagID = IsCompare ? diag::warn_nonnull_parameter_compare
+ : diag::warn_cast_nonnull_to_bool;
+ Diag(E->getExprLoc(), DiagID) << int(IsParam) << S.str()
+ << E->getSourceRange() << Range << IsEqual;
+ };
+
+ // If we have a CallExpr that is tagged with returns_nonnull, we can complain.
+ if (auto *Call = dyn_cast<CallExpr>(E->IgnoreParenImpCasts()))
+ if (auto *Callee = Call->getDirectCallee())
+ if (Callee->hasAttr<ReturnsNonNullAttr>())
+ return ComplainAboutNonnullParamOrCall(false);
+
// Expect to find a single Decl. Skip anything more complicated.
ValueDecl *D = nullptr;
if (DeclRefExpr *R = dyn_cast<DeclRefExpr>(E)) {
@@ -7679,9 +7694,9 @@
return;
// Check for parameter decl with nonnull attribute
- if (const ParmVarDecl* PV = dyn_cast<ParmVarDecl>(D)) {
+ if (const auto *PV = dyn_cast<ParmVarDecl>(D)) {
if (getCurFunction() && !getCurFunction()->ModifiedNonNullParams.count(PV))
- if (const FunctionDecl* FD = dyn_cast<FunctionDecl>(PV->getDeclContext())) {
+ if (const auto *FD = dyn_cast<FunctionDecl>(PV->getDeclContext())) {
unsigned NumArgs = FD->getNumParams();
llvm::SmallBitVector AttrNonNull(NumArgs);
for (const auto *NonNull : FD->specific_attrs<NonNullAttr>()) {
@@ -7695,21 +7710,12 @@
AttrNonNull.set(Val);
}
}
- if (!AttrNonNull.empty())
- for (unsigned i = 0; i < NumArgs; ++i)
- if (FD->getParamDecl(i) == PV &&
- (AttrNonNull[i] || PV->hasAttr<NonNullAttr>())) {
- std::string Str;
- llvm::raw_string_ostream S(Str);
- E->printPretty(S, nullptr, getPrintingPolicy());
- unsigned DiagID = IsCompare ? diag::warn_nonnull_parameter_compare
- : diag::warn_cast_nonnull_to_bool;
- Diag(E->getExprLoc(), DiagID) << S.str() << E->getSourceRange()
- << Range << IsEqual;
- return;
- }
+ for (unsigned i = 0; i < NumArgs; ++i)
+ if (FD->getParamDecl(i) == PV &&
+ (AttrNonNull[i] || PV->hasAttr<NonNullAttr>()))
+ return ComplainAboutNonnullParamOrCall(true);
}
- }
+ }
QualType T = D->getType();
const bool IsArray = T->isArrayType();
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -2727,7 +2727,7 @@
"'true'">,
InGroup<PointerBoolConversion>;
def warn_cast_nonnull_to_bool : Warning<
- "nonnull parameter '%0' will evaluate to "
+ "nonnull %select{function call|parameter}0 '%1' will evaluate to "
"'true' on first encounter">,
InGroup<PointerBoolConversion>;
def warn_this_bool_conversion : Warning<
@@ -2743,8 +2743,9 @@
"equal to a null pointer is always %select{true|false}2">,
InGroup<TautologicalPointerCompare>;
def warn_nonnull_parameter_compare : Warning<
- "comparison of nonnull parameter '%0' %select{not |}1"
- "equal to a null pointer is %select{true|false}1 on first encounter">,
+ "comparison of nonnull %select{function call|parameter}0 '%1' "
+ "%select{not |}2equal to a null pointer is %select{true|false}2 on first "
+ "encounter">,
InGroup<TautologicalPointerCompare>;
def warn_this_null_compare : Warning<
"'this' pointer cannot be null in well-defined C++ code; comparison may be "
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
