I think the following would be the most consistent with the rest of the debug checkers: - BugReport *R = new BugReport(*BT, "REACHABLE", N); + BugReport *R = new BugReport(*BT, "Analyzer reached this line (WARN-REACHED)", N);
Cheers, Anna. On Sep 30, 2013, at 8:40 PM, Jared Grubb <[email protected]> wrote: > Sorry for the delay. My free time has been limited lately. > > From your comments, the only change you suggested was the capitalization: > - clang_analyzer_warnIfReached(); // expected-warning{{reached}} > + clang_analyzer_warnIfReached(); // expected-warning{{REACHED}} > > Attached are the minor changes required to make that work. Unit tests > continue to all pass. > > <patch-warnIfReached.diff> > > > Jared > > On Sep 16, 2013, at 18:25, Jordan Rose <[email protected]> wrote: > >> [+Anna for opinions] >> >> Two comments: >> >> - I think we should keep the ExprInspection output in all caps, to make it >> clear that it's not a usual analyzer warning >> - The null-pointer deref tests also halt analysis along that path. I don't >> think that's something we usually need to do, but there might be a few cases >> where it's important. We could probably just add a return in most cases, >> though. >> >> I think you're right that it's better to be explicit, even though it's >> functionally equivalent to constructs we have already. Anna? >> >> Jordan >> >> >> On Sep 16, 2013, at 9:07 , Jared Grubb <[email protected]> wrote: >> >>> While working on unit tests for a checker, I really wanted a facility to >>> detect that the analyzer has (or has not) made certain branch choices. >>> >>> Looking through the unit test, this kind of check is done in various ways: >>> * Intentional deref of null pointer (the most popular, apparently) >>> * Intentional divide by zero (tests in "cast.c") >>> * "clang_analyzer_eval(true); // expected-warning{{TRUE}}" >>> >>> Adding a new directive to explicitly serve this purpose helps the >>> readability of unit tests. As proof of concept, I've modified an existing >>> test to remove the fake null-pointer derefs. >>> >>> Unit tests continue to pass. The direct patch to the analyzer is very >>> minimal. >>> >>> Jared >>> >>> >>> diff --git a/docs/analyzer/DebugChecks.rst b/docs/analyzer/DebugChecks.rst >>> index a0f2a07..b0983e8 100644 >>> --- a/docs/analyzer/DebugChecks.rst >>> +++ b/docs/analyzer/DebugChecks.rst >>> @@ -125,6 +125,19 @@ ExprInspection checks >>> clang_analyzer_eval(value == 42); // expected-warning{{TRUE}} >>> } >>> >>> +- void clang_analyzer_warnIfReached(); >>> + >>> + Generate a warning if this line of code gets reached by the analyzer. >>> + >>> + Example usage:: >>> + >>> + if (true) { >>> + clang_analyzer_warnIfReached(); // expected-warning{{reached}} >>> + } >>> + else { >>> + clang_analyzer_warnIfReached(); // no-warning >>> + } >>> + >>> >>> Statistics >>> ========== >>> diff --git a/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp >>> b/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp >>> index 9522d1d..53f7c3d 100644 >>> --- a/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp >>> +++ b/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp >>> @@ -22,6 +22,7 @@ class ExprInspectionChecker : public Checker< eval::Call >>> > { >>> >>> void analyzerEval(const CallExpr *CE, CheckerContext &C) const; >>> void analyzerCheckInlined(const CallExpr *CE, CheckerContext &C) const; >>> + void analyzerWarnIfReached(const CallExpr *CE, CheckerContext &C) const; >>> void analyzerCrash(const CallExpr *CE, CheckerContext &C) const; >>> >>> typedef void (ExprInspectionChecker::*FnCheck)(const CallExpr *, >>> @@ -41,6 +42,7 @@ bool ExprInspectionChecker::evalCall(const CallExpr *CE, >>> .Case("clang_analyzer_checkInlined", >>> &ExprInspectionChecker::analyzerCheckInlined) >>> .Case("clang_analyzer_crash", &ExprInspectionChecker::analyzerCrash) >>> + .Case("clang_analyzer_warnIfReached", >>> &ExprInspectionChecker::analyzerWarnIfReached) >>> .Default(0); >>> >>> if (!Handler) >>> @@ -99,6 +101,17 @@ void ExprInspectionChecker::analyzerEval(const CallExpr >>> *CE, >>> C.emitReport(R); >>> } >>> >>> +void ExprInspectionChecker::analyzerWarnIfReached(const CallExpr *CE, >>> + CheckerContext &C) const >>> { >>> + ExplodedNode *N = C.getPredecessor(); >>> + >>> + if (!BT) >>> + BT.reset(new BugType("Checking analyzer assumptions", "debug")); >>> + >>> + BugReport *R = new BugReport(*BT, "Analyzer reached this line", N); >>> + C.emitReport(R); >>> +} >>> + >>> void ExprInspectionChecker::analyzerCheckInlined(const CallExpr *CE, >>> CheckerContext &C) const { >>> ExplodedNode *N = C.getPredecessor(); >>> diff --git a/test/Analysis/misc-ps-region-store.cpp >>> b/test/Analysis/misc-ps-region-store.cpp >>> index 902a5e5..c00d357 100644 >>> --- a/test/Analysis/misc-ps-region-store.cpp >>> +++ b/test/Analysis/misc-ps-region-store.cpp >>> @@ -1,5 +1,7 @@ >>> -// RUN: %clang_cc1 -triple i386-apple-darwin9 -analyze >>> -analyzer-checker=core,alpha.core -analyzer-store=region -verify -fblocks >>> -analyzer-opt-analyze-nested-blocks %s -fexceptions -fcxx-exceptions >>> -// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -analyze >>> -analyzer-checker=core,alpha.core -analyzer-store=region -verify -fblocks >>> -analyzer-opt-analyze-nested-blocks %s -fexceptions -fcxx-exceptions >>> +// RUN: %clang_cc1 -triple i386-apple-darwin9 -analyze >>> -analyzer-checker=core,alpha.core,debug.ExprInspection >>> -analyzer-store=region -verify -fblocks -analyzer-opt-analyze-nested-blocks >>> %s -fexceptions -fcxx-exceptions >>> +// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -analyze >>> -analyzer-checker=core,alpha.core,debug.ExprInspection >>> -analyzer-store=region -verify -fblocks -analyzer-opt-analyze-nested-blocks >>> %s -fexceptions -fcxx-exceptions >>> + >>> +void clang_analyzer_warnIfReached(); >>> >>> // Test basic handling of references. >>> char &test1_aux(); >>> @@ -54,9 +56,7 @@ int test_init_in_condition_switch() { >>> if (x == 2) >>> return 0; >>> else { >>> - // Unreachable. >>> - int *p = 0; >>> - *p = 0xDEADBEEF; // no-warning >>> + clang_analyzer_warnIfReached(); // unreachable >>> } >>> default: >>> break; >>> @@ -74,8 +74,7 @@ int test_init_in_condition_while() { >>> if (z == 2) >>> return 0; >>> >>> - int *p = 0; >>> - *p = 0xDEADBEEF; // no-warning >>> + clang_analyzer_warnIfReached(); // unreachable >>> return 0; >>> } >>> >>> @@ -89,8 +88,7 @@ int test_init_in_condition_for() { >>> if (z == 1) >>> return 0; >>> >>> - int *p = 0; >>> - *p = 0xDEADBEEF; // no-warning >>> + clang_analyzer_warnIfReached(); // unreachable >>> return 0; >>> } >>> >>> @@ -117,8 +115,7 @@ int TestHandleThis::null_deref_negative() { >>> if (x == 10) { >>> return 1; >>> } >>> - int *p = 0; >>> - *p = 0xDEADBEEF; // no-warning >>> + clang_analyzer_warnIfReached(); // unreachable >>> return 0; >>> } >>> >>> @@ -127,8 +124,7 @@ int TestHandleThis::null_deref_positive() { >>> if (x == 9) { >>> return 1; >>> } >>> - int *p = 0; >>> - *p = 0xDEADBEEF; // expected-warning{{null pointer}} >>> + clang_analyzer_warnIfReached(); // expected-warning{{reached}} >>> return 0; >>> } >>> >>> @@ -143,9 +139,9 @@ void pr7675_test() { >>> pr7675(10); >>> pr7675('c'); >>> pr7675_i(4.0i); >>> - // Add null deref to ensure we are analyzing the code up to this point. >>> - int *p = 0; >>> - *p = 0xDEADBEEF; // expected-warning{{null pointer}} >>> + >>> + // Add check to ensure we are analyzing the code up to this point. >>> + clang_analyzer_warnIfReached(); // expected-warning{{reached}} >>> } >>> >>> // <rdar://problem/8375510> - CFGBuilder should handle temporaries. >>> @@ -328,26 +324,23 @@ class RDar9267815 { >>> }; >>> >>> void RDar9267815::test_pos() { >>> - int *p = 0; >>> if (x == 42) >>> return; >>> - *p = 0xDEADBEEF; // expected-warning {{null}} >>> + clang_analyzer_warnIfReached(); // expected-warning{{reached}} >>> } >>> void RDar9267815::test() { >>> - int *p = 0; >>> if (x == 42) >>> return; >>> if (x == 42) >>> - *p = 0xDEADBEEF; // no-warning >>> + clang_analyzer_warnIfReached(); // no-warning >>> } >>> >>> void RDar9267815::test2() { >>> - int *p = 0; >>> if (x == 42) >>> return; >>> invalidate(); >>> if (x == 42) >>> - *p = 0xDEADBEEF; // expected-warning {{null}} >>> + clang_analyzer_warnIfReached(); // expected-warning{{reached}} >>> } >>> >>> // Test reference parameters. >>> @@ -440,8 +433,7 @@ int rdar9948787_negative() { >>> unsigned value = classWithStatic.value; >>> if (value == 1) >>> return 1; >>> - int *p = 0; >>> - *p = 0xDEADBEEF; // no-warning >>> + clang_analyzer_warnIfReached(); // no-warning >>> return 0; >>> } >>> >>> @@ -450,8 +442,7 @@ int rdar9948787_positive() { >>> unsigned value = classWithStatic.value; >>> if (value == 0) >>> return 1; >>> - int *p = 0; >>> - *p = 0xDEADBEEF; // expected-warning {{null}} >>> + clang_analyzer_warnIfReached(); // expected-warning{{reached}} >>> return 0; >>> } >>> >>> @@ -467,8 +458,7 @@ void rdar10202899_test1() { >>> void rdar10202899_test2() { >>> if (val == rdar10202899_ValTA) >>> return; >>> - int *p = 0; >>> - *p = 0xDEADBEEF; >>> + clang_analyzer_warnIfReached(); // no-warning >>> } >>> >>> void rdar10202899_test3() { >>> @@ -476,8 +466,7 @@ void rdar10202899_test3() { >>> case rdar10202899_ValTA: return; >>> default: ; >>> }; >>> - int *p = 0; >>> - *p = 0xDEADBEEF; >>> + clang_analyzer_warnIfReached(); // no-warning >>> } >>> >>> // This used to crash the analyzer because of the unnamed bitfield. >>> @@ -489,13 +478,12 @@ void PR11249() >>> char f2[1]; >>> char f3; >>> } V = { 1, {2}, 3 }; >>> - int *p = 0; >>> if (V.f1 != 1) >>> - *p = 0xDEADBEEF; // no-warning >>> + clang_analyzer_warnIfReached(); // no-warning >>> if (V.f2[0] != 2) >>> - *p = 0xDEADBEEF; // no-warning >>> + clang_analyzer_warnIfReached(); // no-warning >>> if (V.f3 != 3) >>> - *p = 0xDEADBEEF; // no-warning >>> + clang_analyzer_warnIfReached(); // no-warning >>> } >>> >>> // Handle doing a load from the memory associated with the code for >>> @@ -599,12 +587,10 @@ void rdar10924675(unsigned short x[], int index, int >>> index2) { >>> void rdar11401827() { >>> int x = int(); >>> if (!x) { >>> - int *p = 0; >>> - *p = 0xDEADBEEF; // expected-warning {{null pointer}} >>> + clang_analyzer_warnIfReached(); // expected-warning{{reached}} >>> } >>> else { >>> - int *p = 0; >>> - *p = 0xDEADBEEF; >>> + clang_analyzer_warnIfReached(); // no-warning >>> } >>> } >>> >>> @@ -701,8 +687,7 @@ const Rdar12755044_foo *radar12755044() { >>> void rdar12759044() { >>> int flag = 512; >>> if (!(flag & 512)) { >>> - int *p = 0; >>> - *p = 0xDEADBEEF; // no-warning >>> + clang_analyzer_warnIfReached(); // no-warning >>> } >>> } >>> >>> >>> _______________________________________________ >>> cfe-commits mailing list >>> [email protected] >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
