NoQ created this revision.
NoQ added reviewers: zaks.anna, dcoughlin.
NoQ added a subscriber: cfe-commits.
`CFNumberRef`, much like `NSNumber*`, can also be accidentally mistaken for a
numeric value, so it is worth it to support this type in our new
`NumberObjectConversion` checker.
https://reviews.llvm.org/D25731
Files:
lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp
test/Analysis/number-object-conversion.c
Index: test/Analysis/number-object-conversion.c
===================================================================
--- /dev/null
+++ test/Analysis/number-object-conversion.c
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -w -analyze -analyzer-checker=osx.NumberObjectConversion %s -verify
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -w -analyze -analyzer-checker=osx.NumberObjectConversion -analyzer-config osx.NumberObjectConversion:Pedantic=true -DPEDANTIC %s -verify
+
+#define NULL ((void *)0)
+
+typedef const struct __CFNumber *CFNumberRef;
+
+
+void takes_int(int);
+
+void bad(CFNumberRef p) {
+#ifdef PEDANTIC
+ if (p) {} // expected-warning{{Converting 'CFNumberRef' to a plain boolean value for branching; please compare the pointer to NULL instead to suppress this warning}}
+ if (!p) {} // expected-warning{{Converting 'CFNumberRef' to a plain boolean value for branching; please compare the pointer to NULL instead to suppress this warning}}
+ p ? 1 : 2; // expected-warning{{Converting 'CFNumberRef' to a plain boolean value for branching; please compare the pointer to NULL instead to suppress this warning}}
+#endif
+ int x = p; // expected-warning{{Converting 'CFNumberRef' to a plain integer value; pointer value is being used instead}}
+ takes_int(p); // expected-warning{{Converting 'CFNumberRef' to a plain integer value; pointer value is being used instead}}
+ takes_int(x); // no-warning
+}
Index: lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp
+++ lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp
@@ -69,10 +69,12 @@
const Stmt *Conv = Result.Nodes.getNodeAs<Stmt>("conv");
assert(Conv);
- const Expr *Osboolean = Result.Nodes.getNodeAs<Expr>("osboolean");
- const Expr *Nsnumber = Result.Nodes.getNodeAs<Expr>("nsnumber");
- bool IsObjC = (bool)Nsnumber;
- const Expr *Obj = IsObjC ? Nsnumber : Osboolean;
+ const Expr *CObject = Result.Nodes.getNodeAs<Expr>("c_object");
+ const Expr *CppObject = Result.Nodes.getNodeAs<Expr>("cpp_object");
+ const Expr *ObjCObject = Result.Nodes.getNodeAs<Expr>("objc_object");
+ bool IsCpp = (CppObject != nullptr);
+ bool IsObjC = (ObjCObject != nullptr);
+ const Expr *Obj = IsObjC ? ObjCObject : IsCpp ? CppObject : CObject;
assert(Obj);
ASTContext &ACtx = ADC->getASTContext();
@@ -104,9 +106,11 @@
llvm::SmallString<64> Msg;
llvm::raw_svector_ostream OS(Msg);
- OS << "Converting '"
- << Obj->getType().getCanonicalType().getUnqualifiedType().getAsString()
- << "' to a plain ";
+
+ QualType ObjT = (IsCpp || IsObjC)
+ ? Obj->getType().getCanonicalType().getUnqualifiedType()
+ : Obj->getType();
+ OS << "Converting '" << ObjT.getAsString() << "' to a plain ";
if (Result.Nodes.getNodeAs<QualType>("int_type") != nullptr)
OS << "integer value";
@@ -121,9 +125,12 @@
if (IsObjC) {
OS << "; please compare the pointer to nil instead "
"to suppress this warning";
- } else {
+ } else if (IsCpp) {
OS << "; please compare the pointer to NULL or nullptr instead "
"to suppress this warning";
+ } else {
+ OS << "; please compare the pointer to NULL instead "
+ "to suppress this warning";
}
} else {
OS << "; pointer value is being used instead";
@@ -142,27 +149,41 @@
MatchFinder F;
Callback CB(this, BR, AM.getAnalysisDeclContext(D));
- auto OSBooleanExprM =
+ auto SuspiciousCExprM =
+ expr(ignoringParenImpCasts(
+ expr(hasType(
+ typedefType(hasDeclaration(anyOf(
+ typedefDecl(hasName("CFNumberRef")),
+ typedefDecl(hasName("CFBooleanRef")))))))
+ .bind("c_object")));
+
+ auto SuspiciousCppExprM =
expr(ignoringParenImpCasts(
expr(hasType(hasCanonicalType(
pointerType(pointee(hasCanonicalType(
recordType(hasDeclaration(
- cxxRecordDecl(hasName(
- "OSBoolean")))))))))).bind("osboolean")));
+ anyOf(
+ cxxRecordDecl(hasName("OSBoolean")),
+ cxxRecordDecl(hasName("OSNumber")))))))))))
+ .bind("cpp_object")));
- auto NSNumberExprM =
- expr(ignoringParenImpCasts(expr(hasType(hasCanonicalType(
- objcObjectPointerType(pointee(
- qualType(hasCanonicalType(
- qualType(hasDeclaration(
- objcInterfaceDecl(hasName(
- "NSNumber"))))))))))).bind("nsnumber")));
+ auto SuspiciousObjCExprM =
+ expr(ignoringParenImpCasts(
+ expr(hasType(hasCanonicalType(
+ objcObjectPointerType(pointee(
+ qualType(hasCanonicalType(
+ qualType(hasDeclaration(
+ objcInterfaceDecl(hasName("NSNumber")))))))))))
+ .bind("objc_object")));
auto SuspiciousExprM =
- anyOf(OSBooleanExprM, NSNumberExprM);
+ anyOf(SuspiciousCExprM, SuspiciousCppExprM, SuspiciousObjCExprM);
- auto AnotherNSNumberExprM =
- expr(equalsBoundNode("nsnumber"));
+ auto AnotherSuspiciousExprM =
+ expr(anyOf(
+ equalsBoundNode("c_object"),
+ equalsBoundNode("objc_object"),
+ equalsBoundNode("cpp_object")));
// The .bind here is in order to compose the error message more accurately.
auto ObjCBooleanTypeM =
@@ -219,8 +240,8 @@
auto ConversionThroughConditionalOperatorM =
conditionalOperator(
hasCondition(SuspiciousExprM),
- unless(hasTrueExpression(hasDescendant(AnotherNSNumberExprM))),
- unless(hasFalseExpression(hasDescendant(AnotherNSNumberExprM))))
+ unless(hasTrueExpression(hasDescendant(AnotherSuspiciousExprM))),
+ unless(hasFalseExpression(hasDescendant(AnotherSuspiciousExprM))))
.bind("pedantic");
auto ConversionThroughExclamationMarkM =
@@ -257,11 +278,8 @@
}
void ento::registerNumberObjectConversionChecker(CheckerManager &Mgr) {
- const LangOptions &LO = Mgr.getLangOpts();
- if (LO.CPlusPlus || LO.ObjC2) {
- NumberObjectConversionChecker *Chk =
- Mgr.registerChecker<NumberObjectConversionChecker>();
- Chk->Pedantic =
- Mgr.getAnalyzerOptions().getBooleanOption("Pedantic", false, Chk);
- }
+ NumberObjectConversionChecker *Chk =
+ Mgr.registerChecker<NumberObjectConversionChecker>();
+ Chk->Pedantic =
+ Mgr.getAnalyzerOptions().getBooleanOption("Pedantic", false, Chk);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
