[analyzer] NFCi: Refactor CStringChecker: use strongly typed internal API

Summary:
I wanted to extend the diagnostics of the CStringChecker with taintedness.
This requires the CStringChecker to be refactored to support a more flexible
reporting mechanism.

This patch does only refactorings, such:
 - eliminates always false parameters (like WarnAboutSize)
 - reduces the number of parameters
 - makes strong types differentiating *source* and *destination* buffers
   (same with size expressions)
 - binds the argument expression and the index, making diagnostics accurate
   and easy to emit
 - removes a bunch of default parameters to make it more readable
 - remove random const char* warning message parameters, making clear where
   and what is going to be emitted

Note that:
 - CheckBufferAccess now checks *only* one buffer, this removed about 100 LOC
   code duplication
 - not every function was refactored to use the /new/ strongly typed API, since
   the CString related functions are really closely coupled monolithic beasts,
   I will refactor them separately
 - all tests are preserved and passing; only the message changed at some places.
   In my opinion, these messages are holding the same information.

I would also highlight that this refactoring caught a bug in
clang/test/Analysis/string.c:454 where the diagnostic did not reflect reality.
This catch backs my effort on simplifying this monolithic CStringChecker.

Reviewers: NoQ, baloghadamsoftware, Szelethus, rengolin, Charusso

Reviewed By: NoQ

Subscribers: whisperity, xazax.hun, szepet, rnkovacs, a.sidorin,
mikhail.ramalho, donat.nagy, dkrupp, Charusso, martong, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D74806
This commit is contained in:
Balazs Benics 2020-04-09 16:06:32 +02:00
parent ec2752730d
commit 30e5c7e82f
5 changed files with 339 additions and 318 deletions

File diff suppressed because it is too large Load Diff

View File

@ -29,7 +29,7 @@ void f2() {
void f3() {
char dst[2];
const char *src = "abdef";
strlcpy(dst, src, 5); // expected-warning{{Size argument is greater than the length of the destination buffer}}
strlcpy(dst, src, 5); // expected-warning{{String copy function overflows the destination buffer}}
}
void f4() {
@ -112,7 +112,8 @@ void f9(int unknown_size, char* unknown_src, char* unknown_dst){
clang_analyzer_eval(strlen(unknown_dst));// expected-warning{{UNKNOWN}}
//size is unknown
len = strlcat(buf+2,unknown_src+1, sizeof(buf));// expected-warning{{Size argument is greater than the length of the destination buffer}};
len = strlcat(buf + 2, unknown_src + 1, sizeof(buf));
// expected-warning@-1 {{String concatenation function overflows the destination buffer}}
}
void f10(){
@ -121,7 +122,8 @@ void f10(){
len = strlcpy(buf,"abba",sizeof(buf));
clang_analyzer_eval(len==4);// expected-warning{{TRUE}}
strlcat(buf, "efghi",9);// expected-warning{{Size argument is greater than the length of the destination buffer}}
strlcat(buf, "efghi", 9);
// expected-warning@-1 {{String concatenation function overflows the destination buffer}}
}
void f11() {

View File

@ -95,9 +95,9 @@ void memcpy2 () {
char src[] = {1, 2, 3, 4};
char dst[1];
memcpy(dst, src, 4); // expected-warning{{Memory copy function overflows destination buffer}}
memcpy(dst, src, 4); // expected-warning {{Memory copy function overflows the destination buffer}}
#ifndef VARIANT
// expected-warning@-2{{memcpy' will always overflow; destination buffer has size 1, but size argument is 4}}
// expected-warning@-2 {{memcpy' will always overflow; destination buffer has size 1, but size argument is 4}}
#endif
}
@ -119,7 +119,7 @@ void memcpy5() {
char src[] = {1, 2, 3, 4};
char dst[3];
memcpy(dst+2, src+2, 2); // expected-warning{{Memory copy function overflows destination buffer}}
memcpy(dst + 2, src + 2, 2); // expected-warning{{Memory copy function overflows the destination buffer}}
#ifndef VARIANT
// expected-warning@-2{{memcpy' will always overflow; destination buffer has size 1, but size argument is 2}}
#endif
@ -221,7 +221,7 @@ void mempcpy2 () {
char src[] = {1, 2, 3, 4};
char dst[1];
mempcpy(dst, src, 4); // expected-warning{{Memory copy function overflows destination buffer}}
mempcpy(dst, src, 4); // expected-warning{{Memory copy function overflows the destination buffer}}
#ifndef VARIANT
// expected-warning@-2{{'mempcpy' will always overflow; destination buffer has size 1, but size argument is 4}}
#endif
@ -245,7 +245,7 @@ void mempcpy5() {
char src[] = {1, 2, 3, 4};
char dst[3];
mempcpy(dst+2, src+2, 2); // expected-warning{{Memory copy function overflows destination buffer}}
mempcpy(dst + 2, src + 2, 2); // expected-warning{{Memory copy function overflows the destination buffer}}
#ifndef VARIANT
// expected-warning@-2{{'mempcpy' will always overflow; destination buffer has size 1, but size argument is 2}}
#endif
@ -386,7 +386,7 @@ void memmove2 () {
char src[] = {1, 2, 3, 4};
char dst[1];
memmove(dst, src, 4); // expected-warning{{Memory copy function overflows destination buffer}}
memmove(dst, src, 4); // expected-warning{{Memory copy function overflows the destination buffer}}
#ifndef VARIANT
// expected-warning@-2{{memmove' will always overflow; destination buffer has size 1, but size argument is 4}}
#endif

View File

@ -55,12 +55,15 @@ void testHeapSymbol() {
void testStackArrayOutOfBound() {
char buf[1];
memset(buf, 0, 1024); // expected-warning {{Memory set function accesses out-of-bound array element}} expected-warning {{'memset' will always overflow; destination buffer has size 1, but size argument is 1024}}
memset(buf, 0, 1024);
// expected-warning@-1 {{Memory set function overflows the destination buffer}}
// expected-warning@-2 {{'memset' will always overflow; destination buffer has size 1, but size argument is 1024}}
}
void testHeapSymbolOutOfBound() {
char *buf = (char *)malloc(1);
memset(buf, 0, 1024); // expected-warning {{Memory set function accesses out-of-bound array element}}
memset(buf, 0, 1024);
// expected-warning@-1 {{Memory set function overflows the destination buffer}}
free(buf);
}

View File

@ -353,7 +353,7 @@ void strcpy_effects(char *x, char *y) {
void strcpy_overflow(char *y) {
char x[4];
if (strlen(y) == 4)
strcpy(x, y); // expected-warning{{String copy function overflows destination buffer}}
strcpy(x, y); // expected-warning{{String copy function overflows the destination buffer}}
}
#endif
@ -394,7 +394,7 @@ void stpcpy_effect(char *x, char *y) {
void stpcpy_overflow(char *y) {
char x[4];
if (strlen(y) == 4)
stpcpy(x, y); // expected-warning{{String copy function overflows destination buffer}}
stpcpy(x, y); // expected-warning{{String copy function overflows the destination buffer}}
}
#endif
@ -451,19 +451,19 @@ void strcat_effects(char *y) {
void strcat_overflow_0(char *y) {
char x[4] = "12";
if (strlen(y) == 4)
strcat(x, y); // expected-warning{{String copy function overflows destination buffer}}
strcat(x, y); // expected-warning{{String concatenation function overflows the destination buffer}}
}
void strcat_overflow_1(char *y) {
char x[4] = "12";
if (strlen(y) == 3)
strcat(x, y); // expected-warning{{String copy function overflows destination buffer}}
strcat(x, y); // expected-warning{{String concatenation function overflows the destination buffer}}
}
void strcat_overflow_2(char *y) {
char x[4] = "12";
if (strlen(y) == 2)
strcat(x, y); // expected-warning{{String copy function overflows destination buffer}}
strcat(x, y); // expected-warning{{String concatenation function overflows the destination buffer}}
}
#endif
@ -547,25 +547,28 @@ void strncpy_effects(char *x, char *y) {
// of the C-string checker.
void cstringchecker_bounds_nocrash() {
char *p = malloc(2);
strncpy(p, "AAA", sizeof("AAA")); // expected-warning {{Size argument is greater than the length of the destination buffer}}
strncpy(p, "AAA", sizeof("AAA"));
// expected-warning@-1 {{String copy function overflows the destination buffer}}
free(p);
}
void strncpy_overflow(char *y) {
char x[4];
if (strlen(y) == 4)
strncpy(x, y, 5); // expected-warning{{Size argument is greater than the length of the destination buffer}}
strncpy(x, y, 5);
// expected-warning@-1 {{String copy function overflows the destination buffer}}
#ifndef VARIANT
// expected-warning@-2{{size argument is too large; destination buffer has size 4, but size argument is 5}}
// expected-warning@-3 {{size argument is too large; destination buffer has size 4, but size argument is 5}}
#endif
}
void strncpy_no_overflow(char *y) {
char x[4];
if (strlen(y) == 3)
strncpy(x, y, 5); // expected-warning{{Size argument is greater than the length of the destination buffer}}
strncpy(x, y, 5);
// expected-warning@-1 {{String copy function overflows the destination buffer}}
#ifndef VARIANT
// expected-warning@-2{{size argument is too large; destination buffer has size 4, but size argument is 5}}
// expected-warning@-3 {{size argument is too large; destination buffer has size 4, but size argument is 5}}
#endif
}
@ -575,7 +578,8 @@ void strncpy_no_overflow2(char *y, int n) {
char x[4];
if (strlen(y) == 3)
strncpy(x, y, n); // expected-warning{{Size argument is greater than the length of the destination buffer}}
strncpy(x, y, n);
// expected-warning@-1 {{String copy function overflows the destination buffer}}
}
#endif
@ -658,25 +662,29 @@ void strncat_effects(char *y) {
void strncat_overflow_0(char *y) {
char x[4] = "12";
if (strlen(y) == 4)
strncat(x, y, strlen(y)); // expected-warning{{Size argument is greater than the free space in the destination buffer}}
strncat(x, y, strlen(y));
// expected-warning@-1 {{String concatenation function overflows the destination buffer}}
}
void strncat_overflow_1(char *y) {
char x[4] = "12";
if (strlen(y) == 3)
strncat(x, y, strlen(y)); // expected-warning{{Size argument is greater than the free space in the destination buffer}}
strncat(x, y, strlen(y));
// expected-warning@-1 {{String concatenation function overflows the destination buffer}}
}
void strncat_overflow_2(char *y) {
char x[4] = "12";
if (strlen(y) == 2)
strncat(x, y, strlen(y)); // expected-warning{{Size argument is greater than the free space in the destination buffer}}
strncat(x, y, strlen(y));
// expected-warning@-1 {{String concatenation function overflows the destination buffer}}
}
void strncat_overflow_3(char *y) {
char x[4] = "12";
if (strlen(y) == 4)
strncat(x, y, 2); // expected-warning{{Size argument is greater than the free space in the destination buffer}}
strncat(x, y, 2);
// expected-warning@-1 {{String concatenation function overflows the destination buffer}}
}
#endif
@ -704,7 +712,8 @@ void strncat_symbolic_src_length(char *src) {
clang_analyzer_eval(strlen(dst) >= 4); // expected-warning{{TRUE}}
char dst2[8] = "1234";
strncat(dst2, src, 4); // expected-warning{{Size argument is greater than the free space in the destination buffer}}
strncat(dst2, src, 4);
// expected-warning@-1 {{String concatenation function overflows the destination buffer}}
}
void strncat_unknown_src_length(char *src, int offset) {
@ -713,7 +722,8 @@ void strncat_unknown_src_length(char *src, int offset) {
clang_analyzer_eval(strlen(dst) >= 4); // expected-warning{{TRUE}}
char dst2[8] = "1234";
strncat(dst2, &src[offset], 4); // expected-warning{{Size argument is greater than the free space in the destination buffer}}
strncat(dst2, &src[offset], 4);
// expected-warning@-1 {{String concatenation function overflows the destination buffer}}
}
#endif
@ -1496,7 +1506,7 @@ void explicit_bzero3_out_ofbound() {
strcpy(privkey, "random");
explicit_bzero(privkey, sizeof(newprivkey));
#ifndef SUPPRESS_OUT_OF_BOUND
// expected-warning@-2 {{Memory clearance function accesses out-of-bound array element}}
// expected-warning@-2 {{Memory clearance function overflows the destination buffer}}
#endif
clang_analyzer_eval(privkey[0] == '\0');
#ifdef SUPPRESS_OUT_OF_BOUND