Author: Balázs Kéri Date: 2023-07-18T09:29:15+02:00 New Revision: f12808ab20369c85ddb602e5a78bab40d16bb83f
URL: https://github.com/llvm/llvm-project/commit/f12808ab20369c85ddb602e5a78bab40d16bb83f DIFF: https://github.com/llvm/llvm-project/commit/f12808ab20369c85ddb602e5a78bab40d16bb83f.diff LOG: [clang][analyzer] Display notes in StdLibraryFunctionsChecker only if interesting The note tag that was previously added in all cases when a standard function call is found is displayed now only if the function call (return value) is "interesting". This results in less unneeded notes but some of the previously good notes disappear too. This is because interestingness is not always set as it should be. Reviewed By: donat.nagy Differential Revision: https://reviews.llvm.org/D153776 Added: Modified: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp clang/test/Analysis/errno-stdlibraryfunctions-notes.c clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp clang/test/Analysis/std-c-library-functions-arg-constraints.c clang/test/Analysis/std-c-library-functions-path-notes.c clang/test/Analysis/stream-errno-note.c clang/test/Analysis/stream-note.c Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp index 8d4fb12061c3d6..1fb248b8ed0e97 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp @@ -835,6 +835,7 @@ class StdLibraryFunctionsChecker for (ArgNo ArgN : VC->getArgsToTrack()) { bugreporter::trackExpressionValue(N, Call.getArgExpr(ArgN), *R); + R->markInteresting(Call.getArgSVal(ArgN)); // All tracked arguments are important, highlight them. R->addRange(Call.getArgSourceRange(ArgN)); } @@ -1298,6 +1299,7 @@ void StdLibraryFunctionsChecker::checkPostCall(const CallEvent &Call, // StdLibraryFunctionsChecker. ExplodedNode *Pred = Node; if (!Case.getNote().empty()) { + const SVal RV = Call.getReturnValue(); // If there is a description for this execution branch (summary case), // use it as a note tag. std::string Note = @@ -1305,7 +1307,7 @@ void StdLibraryFunctionsChecker::checkPostCall(const CallEvent &Call, cast<NamedDecl>(Call.getDecl())->getDeclName()); if (Summary.getInvalidationKd() == EvalCallAsPure) { const NoteTag *Tag = C.getNoteTag( - [Node, Note](PathSensitiveBugReport &BR) -> std::string { + [Node, Note, RV](PathSensitiveBugReport &BR) -> std::string { // Try to omit the note if we know in advance which branch is // taken (this means, only one branch exists). // This check is performed inside the lambda, after other @@ -1316,18 +1318,22 @@ void StdLibraryFunctionsChecker::checkPostCall(const CallEvent &Call, // split that was performed by another checker (and can not find // the successors). This is why this check is only used in the // EvalCallAsPure case. - if (Node->succ_size() > 1) + if (BR.isInteresting(RV) && Node->succ_size() > 1) return Note; return ""; - }, - /*IsPrunable=*/true); + }); Pred = C.addTransition(NewState, Pred, Tag); } else { - const NoteTag *Tag = C.getNoteTag(Note, /*IsPrunable=*/true); + const NoteTag *Tag = + C.getNoteTag([Note, RV](PathSensitiveBugReport &BR) -> std::string { + if (BR.isInteresting(RV)) + return Note; + return ""; + }); Pred = C.addTransition(NewState, Pred, Tag); } if (!Pred) - break; + continue; } // If we can get a note tag for the errno change, add this additionally to diff --git a/clang/test/Analysis/errno-stdlibraryfunctions-notes.c b/clang/test/Analysis/errno-stdlibraryfunctions-notes.c index e520e4fa76497f..991384cc373ef3 100644 --- a/clang/test/Analysis/errno-stdlibraryfunctions-notes.c +++ b/clang/test/Analysis/errno-stdlibraryfunctions-notes.c @@ -14,10 +14,8 @@ void clang_analyzer_warnIfReached(); void test1() { access("path", 0); - // expected-note@-1{{Assuming that 'access' fails}} access("path", 0); - // expected-note@-1{{Assuming that 'access' is successful}} - // expected-note@-2{{'errno' may be undefined after successful call to 'access'}} + // expected-note@-1{{'errno' may be undefined after successful call to 'access'}} if (errno != 0) { // expected-warning@-1{{An undefined value may be read from 'errno'}} // expected-note@-2{{An undefined value may be read from 'errno'}} @@ -26,8 +24,7 @@ void test1() { void test2() { if (access("path", 0) == -1) { - // expected-note@-1{{Assuming that 'access' fails}} - // expected-note@-2{{Taking true branch}} + // expected-note@-1{{Taking true branch}} // Failure path. if (errno != 0) { // expected-note@-1{{'errno' is not equal to 0}} @@ -42,9 +39,8 @@ void test2() { void test3() { if (access("path", 0) != -1) { // Success path. - // expected-note@-2{{Assuming that 'access' is successful}} - // expected-note@-3{{'errno' may be undefined after successful call to 'access'}} - // expected-note@-4{{Taking true branch}} + // expected-note@-2{{'errno' may be undefined after successful call to 'access'}} + // expected-note@-3{{Taking true branch}} if (errno != 0) { // expected-warning@-1{{An undefined value may be read from 'errno'}} // expected-note@-2{{An undefined value may be read from 'errno'}} diff --git a/clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp b/clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp index fc7116d3e669ce..573b0076a0e731 100644 --- a/clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp +++ b/clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp @@ -52,7 +52,7 @@ void test_buffer_size_note(char *buf, int y) { int __test_case_note(); int test_case_note_1(int y) { - int x0 = __test_case_note(); // expected-note{{Function returns 1}} + int x0 = __test_case_note(); int x = __test_case_note(); // expected-note{{Function returns 0}} \ // expected-note{{'x' initialized here}} return y / x; // expected-warning{{Division by zero}} \ @@ -60,7 +60,7 @@ int test_case_note_1(int y) { } int test_case_note_2(int y) { - int x = __test_case_note(); // expected-note{{Function returns 1}} + int x = __test_case_note(); return y / (x - 1); // expected-warning{{Division by zero}} \ // expected-note{{Division by zero}} } diff --git a/clang/test/Analysis/std-c-library-functions-arg-constraints.c b/clang/test/Analysis/std-c-library-functions-arg-constraints.c index 2fdea1fecd831e..21f1d9f8ad8dd5 100644 --- a/clang/test/Analysis/std-c-library-functions-arg-constraints.c +++ b/clang/test/Analysis/std-c-library-functions-arg-constraints.c @@ -35,8 +35,7 @@ void test_alnum_concrete(int v) { } void test_alnum_symbolic(int x) { - int ret = isalnum(x); // \ - // bugpath-note{{Assuming the character is non-alphanumeric}} + int ret = isalnum(x); (void)ret; clang_analyzer_eval(EOF <= x && x <= 255); // \ @@ -170,8 +169,7 @@ void test_notnull_concrete(FILE *fp) { // bugpath-note{{The 1st argument to 'fread' is NULL but should not be NULL}} } void test_notnull_symbolic(FILE *fp, int *buf) { - fread(buf, sizeof(int), 10, fp); // \ - // bugpath-note{{'fread' fails}} + fread(buf, sizeof(int), 10, fp); clang_analyzer_eval(buf != 0); // \ // report-warning{{TRUE}} \ // bugpath-warning{{TRUE}} \ diff --git a/clang/test/Analysis/std-c-library-functions-path-notes.c b/clang/test/Analysis/std-c-library-functions-path-notes.c index 33d6152376c57c..6b5d1d7bd4eb97 100644 --- a/clang/test/Analysis/std-c-library-functions-path-notes.c +++ b/clang/test/Analysis/std-c-library-functions-path-notes.c @@ -22,11 +22,9 @@ char test_getenv() { } int test_isalpha(int *x, char c, char d) { - int iad = isalpha(d);// \ - // expected-note{{Assuming the character is non-alphabetical}} + int iad = isalpha(d); if (isalpha(c)) {// \ - // expected-note{{Taking true branch}} \ - // expected-note{{Assuming the character is alphabetical}} + // expected-note{{Taking true branch}} x = NULL; // \ // expected-note{{Null pointer value stored to 'x'}} } @@ -38,7 +36,6 @@ int test_isalpha(int *x, char c, char d) { int test_isdigit(int *x, char c) { if (!isdigit(c)) {// \ - // expected-note{{Assuming the character is not a digit}} \ // expected-note{{Taking true branch}} x = NULL; // \ // expected-note{{Null pointer value stored to 'x'}} @@ -64,8 +61,7 @@ int test_islower(int *x) { } int test_bugpath_notes(FILE *f1, char c, FILE *f2) { - int f = fileno(f2); // \ - // expected-note{{Assuming that 'fileno' is successful}} + int f = fileno(f2); if (f == -1) // \ // expected-note{{Taking false branch}} return 0; @@ -77,3 +73,10 @@ int test_bugpath_notes(FILE *f1, char c, FILE *f2) { // expected-warning{{The 1st argument to 'dup' is -1 but should be >= 0}} \ // expected-note{{The 1st argument to 'dup' is -1 but should be >= 0}} } + +int test_fileno_arg_note(FILE *f1) { + return dup(fileno(f1)); // \ + // expected-warning{{The 1st argument to 'dup' is < 0 but should be >= 0}} \ + // expected-note{{The 1st argument to 'dup' is < 0 but should be >= 0}} \ + // expected-note{{Assuming that 'fileno' fails}} +} diff --git a/clang/test/Analysis/stream-errno-note.c b/clang/test/Analysis/stream-errno-note.c index d8aefbabd0b81c..4ab215a64539de 100644 --- a/clang/test/Analysis/stream-errno-note.c +++ b/clang/test/Analysis/stream-errno-note.c @@ -11,7 +11,6 @@ void check_fopen(void) { FILE *F = fopen("xxx", "r"); // expected-note@-1{{'errno' may be undefined after successful call to 'fopen'}} - // expected-note@-2{{'fopen' is successful}} // expected-note@+2{{'F' is non-null}} // expected-note@+1{{Taking false branch}} if (!F) @@ -24,7 +23,6 @@ void check_fopen(void) { void check_tmpfile(void) { FILE *F = tmpfile(); // expected-note@-1{{'errno' may be undefined after successful call to 'tmpfile'}} - // expected-note@-2{{'tmpfile' is successful}} // expected-note@+2{{'F' is non-null}} // expected-note@+1{{Taking false branch}} if (!F) @@ -36,14 +34,12 @@ void check_tmpfile(void) { void check_freopen(void) { FILE *F = tmpfile(); - // expected-note@-1{{'tmpfile' is successful}} // expected-note@+2{{'F' is non-null}} // expected-note@+1{{Taking false branch}} if (!F) return; F = freopen("xxx", "w", F); // expected-note@-1{{'errno' may be undefined after successful call to 'freopen'}} - // expected-note@-2{{'freopen' is successful}} // expected-note@+2{{'F' is non-null}} // expected-note@+1{{Taking false branch}} if (!F) @@ -55,14 +51,12 @@ void check_freopen(void) { void check_fclose(void) { FILE *F = tmpfile(); - // expected-note@-1{{'tmpfile' is successful}} // expected-note@+2{{'F' is non-null}} // expected-note@+1{{Taking false branch}} if (!F) return; (void)fclose(F); // expected-note@-1{{'errno' may be undefined after successful call to 'fclose'}} - // expected-note@-2{{'fclose' is successful}} if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}} // expected-note@-1{{An undefined value may be read from 'errno'}} } @@ -70,14 +64,12 @@ void check_fclose(void) { void check_fread(void) { char Buf[10]; FILE *F = tmpfile(); - // expected-note@-1{{'tmpfile' is successful}} // expected-note@+2{{'F' is non-null}} // expected-note@+1{{Taking false branch}} if (!F) return; (void)fread(Buf, 1, 10, F); // expected-note@-1{{'errno' may be undefined after successful call to 'fread'}} - // expected-note@-2{{'fread' is successful}} if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}} // expected-note@-1{{An undefined value may be read from 'errno'}} (void)fclose(F); @@ -86,14 +78,12 @@ void check_fread(void) { void check_fread_size0(void) { char Buf[10]; FILE *F = tmpfile(); - // expected-note@-1{{'tmpfile' is successful}} // expected-note@+2{{'F' is non-null}} // expected-note@+1{{Taking false branch}} if (!F) return; fread(Buf, 0, 1, F); // expected-note@-1{{'errno' may be undefined after successful call to 'fread'}} - // expected-note@-2{{'fread' is successful}} if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}} // expected-note@-1{{An undefined value may be read from 'errno'}} } @@ -101,14 +91,12 @@ void check_fread_size0(void) { void check_fread_nmemb0(void) { char Buf[10]; FILE *F = tmpfile(); - // expected-note@-1{{'tmpfile' is successful}} // expected-note@+2{{'F' is non-null}} // expected-note@+1{{Taking false branch}} if (!F) return; fread(Buf, 1, 0, F); // expected-note@-1{{'errno' may be undefined after successful call to 'fread'}} - // expected-note@-2{{'fread' is successful}} if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}} // expected-note@-1{{An undefined value may be read from 'errno'}} } @@ -116,14 +104,12 @@ void check_fread_nmemb0(void) { void check_fwrite(void) { char Buf[] = "0123456789"; FILE *F = tmpfile(); - // expected-note@-1{{'tmpfile' is successful}} // expected-note@+2{{'F' is non-null}} // expected-note@+1{{Taking false branch}} if (!F) return; int R = fwrite(Buf, 1, 10, F); // expected-note@-1{{'errno' may be undefined after successful call to 'fwrite'}} - // expected-note@-2{{'fwrite' is successful}} if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}} // expected-note@-1{{An undefined value may be read from 'errno'}} (void)fclose(F); @@ -131,14 +117,12 @@ void check_fwrite(void) { void check_fseek(void) { FILE *F = tmpfile(); - // expected-note@-1{{'tmpfile' is successful}} // expected-note@+2{{'F' is non-null}} // expected-note@+1{{Taking false branch}} if (!F) return; (void)fseek(F, 11, SEEK_SET); // expected-note@-1{{'errno' may be undefined after successful call to 'fseek'}} - // expected-note@-2{{'fseek' is successful}} if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}} // expected-note@-1{{An undefined value may be read from 'errno'}} (void)fclose(F); @@ -146,7 +130,6 @@ void check_fseek(void) { void check_rewind_errnocheck(void) { FILE *F = tmpfile(); - // expected-note@-1{{'tmpfile' is successful}} // expected-note@+2{{'F' is non-null}} // expected-note@+1{{Taking false branch}} if (!F) @@ -159,14 +142,12 @@ void check_rewind_errnocheck(void) { void check_fileno(void) { FILE *F = tmpfile(); - // expected-note@-1{{'tmpfile' is successful}} // expected-note@+2{{'F' is non-null}} // expected-note@+1{{Taking false branch}} if (!F) return; fileno(F); - // expected-note@-1{{Assuming that 'fileno' is successful}} - // expected-note@-2{{'errno' may be undefined after successful call to 'fileno'}} + // expected-note@-1{{'errno' may be undefined after successful call to 'fileno'}} if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}} // expected-note@-1{{An undefined value may be read from 'errno'}} (void)fclose(F); diff --git a/clang/test/Analysis/stream-note.c b/clang/test/Analysis/stream-note.c index 8d297715b03a3a..257245754daddc 100644 --- a/clang/test/Analysis/stream-note.c +++ b/clang/test/Analysis/stream-note.c @@ -13,7 +13,6 @@ void check_note_at_correct_open(void) { // expected-note@-2 {{Taking false branch}} return; FILE *F2 = tmpfile(); - // stdargs-note@-1 {{'tmpfile' is successful}} if (!F2) { // expected-note@-1 {{'F2' is non-null}} // expected-note@-2 {{Taking false branch}} @@ -22,7 +21,6 @@ void check_note_at_correct_open(void) { } rewind(F2); fclose(F2); - // stdargs-note@-1 {{'fclose' fails}} rewind(F1); } // expected-warning@-1 {{Opened stream never closed. Potential resource leak}} @@ -59,7 +57,6 @@ void check_note_freopen(void) { void check_note_leak_2(int c) { FILE *F1 = fopen("foo1.c", "r"); // expected-note {{Stream opened here}} // stdargs-note@-1 {{'fopen' is successful}} - // stdargs-note@-2 {{'fopen' is successful}} if (!F1) // expected-note@-1 {{'F1' is non-null}} // expected-note@-2 {{Taking false branch}} @@ -68,7 +65,6 @@ void check_note_leak_2(int c) { return; FILE *F2 = fopen("foo2.c", "r"); // expected-note {{Stream opened here}} // stdargs-note@-1 {{'fopen' is successful}} - // stdargs-note@-2 {{'fopen' is successful}} if (!F2) { // expected-note@-1 {{'F2' is non-null}} // expected-note@-2 {{Taking false branch}} @@ -107,16 +103,13 @@ void check_eof_notes_feof_after_feof(void) { FILE *F; char Buf[10]; F = fopen("foo1.c", "r"); - // stdargs-note@-1 {{'fopen' is successful}} if (F == NULL) { // expected-note {{Taking false branch}} expected-note {{'F' is not equal to NULL}} return; } fread(Buf, 1, 1, F); - // stdargs-note@-1 {{'fread' fails}} if (feof(F)) { // expected-note {{Taking true branch}} clearerr(F); fread(Buf, 1, 1, F); // expected-note {{Assuming stream reaches end-of-file here}} - // stdargs-note@-1 {{'fread' fails}} if (feof(F)) { // expected-note {{Taking true branch}} fread(Buf, 1, 1, F); // expected-warning {{Read function called when stream is in EOF state. Function has no effect}} // expected-note@-1 {{Read function called when stream is in EOF state. Function has no effect}} @@ -129,12 +122,10 @@ void check_eof_notes_feof_after_no_feof(void) { FILE *F; char Buf[10]; F = fopen("foo1.c", "r"); - // stdargs-note@-1 {{'fopen' is successful}} if (F == NULL) { // expected-note {{Taking false branch}} expected-note {{'F' is not equal to NULL}} return; } fread(Buf, 1, 1, F); - // stdargs-note@-1 {{'fread' is successful}} if (feof(F)) { // expected-note {{Taking false branch}} fclose(F); return; @@ -143,7 +134,6 @@ void check_eof_notes_feof_after_no_feof(void) { return; } fread(Buf, 1, 1, F); // expected-note {{Assuming stream reaches end-of-file here}} - // stdargs-note@-1 {{'fread' fails}} if (feof(F)) { // expected-note {{Taking true branch}} fread(Buf, 1, 1, F); // expected-warning {{Read function called when stream is in EOF state. Function has no effect}} // expected-note@-1 {{Read function called when stream is in EOF state. Function has no effect}} @@ -155,11 +145,9 @@ void check_eof_notes_feof_or_no_error(void) { FILE *F; char Buf[10]; F = fopen("foo1.c", "r"); - // stdargs-note@-1 {{'fopen' is successful}} if (F == NULL) // expected-note {{Taking false branch}} expected-note {{'F' is not equal to NULL}} return; int RRet = fread(Buf, 1, 1, F); // expected-note {{Assuming stream reaches end-of-file here}} - // stdargs-note@-1 {{'fread' fails}} if (ferror(F)) { // expected-note {{Taking false branch}} } else { fread(Buf, 1, 1, F); // expected-warning {{Read function called when stream is in EOF state. Function has no effect}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
