Skip to content

Commit

Permalink
[clang][analyzer] Display notes in StdLibraryFunctionsChecker only if…
Browse files Browse the repository at this point in the history 
… 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
  • Loading branch information
@balazske
balazske committed Jul 18, 2023
1 parent 39670ae commit f12808a
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 59 deletions.
18 changes: 12 additions & 6 deletions clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down Expand Up @@ -1298,14 +1299,15 @@ 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 =
llvm::formatv(Case.getNote().str().c_str(),
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
Expand All @@ -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
Expand Down
12 changes: 4 additions & 8 deletions clang/test/Analysis/errno-stdlibraryfunctions-notes.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'}}
Expand All @@ -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}}
Expand All @@ -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'}}
Expand Down
4 changes: 2 additions & 2 deletions clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,15 @@ 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}} \
// expected-note{{Division by zero}}
}

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}}
}
6 changes: 2 additions & 4 deletions clang/test/Analysis/std-c-library-functions-arg-constraints.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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); // \
Expand Down Expand Up @@ -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}} \
Expand Down
17 changes: 10 additions & 7 deletions clang/test/Analysis/std-c-library-functions-path-notes.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'}}
}
Expand All @@ -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'}}
Expand All @@ -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;
Expand All @@ -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}}
}
21 changes: 1 addition & 20 deletions clang/test/Analysis/stream-errno-note.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -55,29 +51,25 @@ 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'}}
}

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);
Expand All @@ -86,67 +78,58 @@ 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'}}
}

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'}}
}

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);
}

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);
}

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)
Expand All @@ -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);
Expand Down
12 changes: 0 additions & 12 deletions clang/test/Analysis/stream-note.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand All @@ -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}}
Expand Down Expand Up @@ -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}}
Expand All @@ -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}}
Expand Down Expand Up @@ -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}}
Expand All @@ -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;
Expand All @@ -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}}
Expand All @@ -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}}
Expand Down

0 comments on commit f12808a

Please sign in to comment.