[PATCH] D144003: [clang][analyzer] Improve bug reports of StdLibraryFunctionsChecker.

2023-04-12 Thread Balázs Kéri via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGce1fb03db817: [clang][analyzer] Improve bug reports of 
StdLibraryFunctionsChecker. (authored by balazske).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144003/new/

https://reviews.llvm.org/D144003

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints-notes.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints-tracking-notes.c
  clang/test/Analysis/std-c-library-functions-arg-constraints.c
  clang/test/Analysis/std-c-library-functions-arg-constraints.cpp
  clang/test/Analysis/stream-note.c
  clang/test/Analysis/stream-stdlibraryfunctionargs.c

Index: clang/test/Analysis/stream-stdlibraryfunctionargs.c
===
--- clang/test/Analysis/stream-stdlibraryfunctionargs.c
+++ clang/test/Analysis/stream-stdlibraryfunctionargs.c
@@ -42,7 +42,7 @@
 
 void test_fread(void) {
   FILE *fp = tmpfile();
-  size_t ret = fread(buf, size, n, fp); // stdargs-warning{{The 4th argument to 'fread' should not be NULL}}
+  size_t ret = fread(buf, size, n, fp); // stdargs-warning{{The 4th argument to 'fread' is NULL but should not be NULL}}
   clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
   clang_analyzer_eval(ret <= n); // any-warning{{TRUE}}
   clang_analyzer_eval(ret == n); // any-warning{{TRUE}} any-warning{{FALSE}}
@@ -52,7 +52,7 @@
 
 void test_fwrite(void) {
   FILE *fp = tmpfile();
-  size_t ret = fwrite(buf, size, n, fp); // stdargs-warning{{The 4th argument to 'fwrite' should not be NULL}}
+  size_t ret = fwrite(buf, size, n, fp); // stdargs-warning{{The 4th argument to 'fwrite' is NULL but should not be NULL}}
   clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
   clang_analyzer_eval(ret <= n); // any-warning{{TRUE}}
   clang_analyzer_eval(ret == n); // any-warning{{TRUE}} any-warning{{FALSE}}
Index: clang/test/Analysis/stream-note.c
===
--- clang/test/Analysis/stream-note.c
+++ clang/test/Analysis/stream-note.c
@@ -88,8 +88,8 @@
 fclose(F);
 return;
   }
-  fclose(F); // stdargs-warning {{The 1st argument to 'fclose' should not be NULL}}
- // stdargs-note@-1 {{The 1st argument to 'fclose' should not be NULL}}
+  fclose(F); // stdargs-warning {{The 1st argument to 'fclose' is NULL but should not be NULL}}
+ // stdargs-note@-1 {{The 1st argument to 'fclose' is NULL but should not be NULL}}
 }
 
 void check_eof_notes_feof_after_feof(void) {
Index: clang/test/Analysis/std-c-library-functions-arg-constraints.cpp
===
--- clang/test/Analysis/std-c-library-functions-arg-constraints.cpp
+++ clang/test/Analysis/std-c-library-functions-arg-constraints.cpp
@@ -14,5 +14,5 @@
 
 void test_arg_constraint_on_fun_with_default_param() {
   __defaultparam(nullptr); // \
-  // expected-warning{{The 1st argument to '__defaultparam' should not be NULL}}
+  // expected-warning{{The 1st argument to '__defaultparam' is NULL but should not be NULL}}
 }
Index: clang/test/Analysis/std-c-library-functions-arg-constraints.c
===
--- clang/test/Analysis/std-c-library-functions-arg-constraints.c
+++ clang/test/Analysis/std-c-library-functions-arg-constraints.c
@@ -30,9 +30,9 @@
 
 void test_alnum_concrete(int v) {
   int ret = isalnum(256); // \
-  // report-warning{{The 1st argument to 'isalnum' should be an unsigned char value or EOF}} \
-  // bugpath-warning{{The 1st argument to 'isalnum' should be an unsigned char value or EOF}} \
-  // bugpath-note{{The 1st argument to 'isalnum' should be an unsigned char value or EOF}}
+  // report-warning{{The 1st argument to 'isalnum' is 256 but should be an unsigned char value or EOF}} \
+  // bugpath-warning{{The 1st argument to 'isalnum' is 256 but should be an unsigned char value or EOF}} \
+  // bugpath-note{{The 1st argument to 'isalnum' is 256 but should be an unsigned char value or EOF}}
   (void)ret;
 }
 
@@ -55,9 +55,9 @@
 // bugpath-note{{Taking true branch}}
 
 int ret = isalnum(x); // \
-// report-warning{{The 1st argument to 'isalnum' should be an unsigned char value or EOF}} \
-// bugpath-warning{{The 1st argument to 'isalnum' should be an unsigned char value or EOF}} \
-// bugpath-note{{The 1st argument to 'isalnum' should be an unsigned char value or EOF}}
+// report-warning{{The 1st argument to 'isalnum' is >= 256 but should be an unsigned char value or EOF}} \
+// bugpath-warning{{The 1st argument to 'isalnum' is >= 256 but should be an unsigned char value or EOF}} \
+// bugpath-note{{The 1st argument to 'isalnum' is >= 256 but should be an unsigned char 

[PATCH] D144003: [clang][analyzer] Improve bug reports of StdLibraryFunctionsChecker.

2023-04-11 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision.
Szelethus added a comment.

LGTM! Most of the diagnostic messages are short but precise. I like this very 
much. Well done!

Balázs actually tested the change on open source projects, but accidentally 
uploaded it to our internal server, so I have seen them, and they look good 
from the diagnostic message standpoint.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144003/new/

https://reviews.llvm.org/D144003

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D144003: [clang][analyzer] Improve bug reports of StdLibraryFunctionsChecker.

2023-04-04 Thread Balázs Kéri via Phabricator via cfe-commits
balazske updated this revision to Diff 510816.
balazske marked 3 inline comments as done.
balazske added a comment.

I decided to add back `DescriptionKind` and make possible to use a message like
"should not be NULL". Now all generated strings in functions `describe` and
`describeArgumentValue` start with "should be" or "is" to make this consistent.
The messages are now in form "is ... but should be ..." which sounds at some 
times
too trivial but acceptable as generated message.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144003/new/

https://reviews.llvm.org/D144003

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints-notes.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints-tracking-notes.c
  clang/test/Analysis/std-c-library-functions-arg-constraints.c
  clang/test/Analysis/std-c-library-functions-arg-constraints.cpp
  clang/test/Analysis/stream-note.c
  clang/test/Analysis/stream-stdlibraryfunctionargs.c

Index: clang/test/Analysis/stream-stdlibraryfunctionargs.c
===
--- clang/test/Analysis/stream-stdlibraryfunctionargs.c
+++ clang/test/Analysis/stream-stdlibraryfunctionargs.c
@@ -42,7 +42,7 @@
 
 void test_fread(void) {
   FILE *fp = tmpfile();
-  size_t ret = fread(buf, size, n, fp); // stdargs-warning{{The 4th argument to 'fread' should not be NULL}}
+  size_t ret = fread(buf, size, n, fp); // stdargs-warning{{The 4th argument to 'fread' is NULL but should not be NULL}}
   clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
   clang_analyzer_eval(ret <= n); // any-warning{{TRUE}}
   clang_analyzer_eval(ret == n); // any-warning{{TRUE}} any-warning{{FALSE}}
@@ -52,7 +52,7 @@
 
 void test_fwrite(void) {
   FILE *fp = tmpfile();
-  size_t ret = fwrite(buf, size, n, fp); // stdargs-warning{{The 4th argument to 'fwrite' should not be NULL}}
+  size_t ret = fwrite(buf, size, n, fp); // stdargs-warning{{The 4th argument to 'fwrite' is NULL but should not be NULL}}
   clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
   clang_analyzer_eval(ret <= n); // any-warning{{TRUE}}
   clang_analyzer_eval(ret == n); // any-warning{{TRUE}} any-warning{{FALSE}}
Index: clang/test/Analysis/stream-note.c
===
--- clang/test/Analysis/stream-note.c
+++ clang/test/Analysis/stream-note.c
@@ -88,8 +88,8 @@
 fclose(F);
 return;
   }
-  fclose(F); // stdargs-warning {{The 1st argument to 'fclose' should not be NULL}}
- // stdargs-note@-1 {{The 1st argument to 'fclose' should not be NULL}}
+  fclose(F); // stdargs-warning {{The 1st argument to 'fclose' is NULL but should not be NULL}}
+ // stdargs-note@-1 {{The 1st argument to 'fclose' is NULL but should not be NULL}}
 }
 
 void check_eof_notes_feof_after_feof(void) {
Index: clang/test/Analysis/std-c-library-functions-arg-constraints.cpp
===
--- clang/test/Analysis/std-c-library-functions-arg-constraints.cpp
+++ clang/test/Analysis/std-c-library-functions-arg-constraints.cpp
@@ -14,5 +14,5 @@
 
 void test_arg_constraint_on_fun_with_default_param() {
   __defaultparam(nullptr); // \
-  // expected-warning{{The 1st argument to '__defaultparam' should not be NULL}}
+  // expected-warning{{The 1st argument to '__defaultparam' is NULL but should not be NULL}}
 }
Index: clang/test/Analysis/std-c-library-functions-arg-constraints.c
===
--- clang/test/Analysis/std-c-library-functions-arg-constraints.c
+++ clang/test/Analysis/std-c-library-functions-arg-constraints.c
@@ -30,9 +30,9 @@
 
 void test_alnum_concrete(int v) {
   int ret = isalnum(256); // \
-  // report-warning{{The 1st argument to 'isalnum' should be an unsigned char value or EOF}} \
-  // bugpath-warning{{The 1st argument to 'isalnum' should be an unsigned char value or EOF}} \
-  // bugpath-note{{The 1st argument to 'isalnum' should be an unsigned char value or EOF}}
+  // report-warning{{The 1st argument to 'isalnum' is 256 but should be an unsigned char value or EOF}} \
+  // bugpath-warning{{The 1st argument to 'isalnum' is 256 but should be an unsigned char value or EOF}} \
+  // bugpath-note{{The 1st argument to 'isalnum' is 256 but should be an unsigned char value or EOF}}
   (void)ret;
 }
 
@@ -55,9 +55,9 @@
 // bugpath-note{{Taking true branch}}
 
 int ret = isalnum(x); // \
-// report-warning{{The 1st argument to 'isalnum' should be an unsigned char value or EOF}} \
-// bugpath-warning{{The 1st argument to 'isalnum' should be an unsigned char value or EOF}} \
-// bugpath-note{{The 1st argument to 'isalnum' should be an unsigned char value or EOF}}
+// report-warning{{The 1st 

[PATCH] D144003: [clang][analyzer] Improve bug reports of StdLibraryFunctionsChecker.

2023-03-28 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

Please run this on open source projects and upload the results.




Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:164-166
+/// This function is called when the current constraint represents the
+/// opposite of a constraint that was not satisfied in a given state, but
+/// the opposite is satisfied. In this case the available information in 
the

I know what you mean, but this could use an example.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:168-170
+/// description about the original constraint violation. This can be get by
+/// try to narrow the current constraint while it remains satisfied in the
+/// given program state. If useful information is found it is put into

This sentence makes so sense, unfortunately :( Could you rephrase, and, again, 
support it with an example?



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:803
+if (ValuesPrinted)
+  MsgOs << " that is out of the accepted range; It should be ";
+else

Looking at the tests, this adds very little how about this:
" that is out of the accepted range; It should be " -> ", but should be "

Do you agree? I won't die on this hill, and am willing to accept this is good 
as-is if you really think that it is.

The other case is fine.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:807
+VC->describe(Call, C.getState(), Summary, MsgOs);
+// NegatedVC->describeBug1(Call, N->getState(), Summary, MsgOs);
 Msg[0] = toupper(Msg[0]);

Did you mean to leave this here?



Comment at: 
clang/test/Analysis/std-c-library-functions-arg-constraints-notes.cpp:18
   __not_null(nullptr); // \
-  // expected-warning{{The 1st argument to '__not_null' should not be NULL}}
+  // expected-warning{{The 1st argument to '__not_null' is NULL that is out of 
the accepted range; It should be not NULL [}}
 }

This english is broken, but more importantly, this is the one scenario where 
the original message was just good enough -- in fact, better. How about either
1. Restoring the original message (by somehow excluding `NotNullConstraint` in 
the new `describe()`)
2. Or saying something like "The 1st argument to '__not_null' is NULL, but 
should be non-NULL"


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144003/new/

https://reviews.llvm.org/D144003

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D144003: [clang][analyzer] Improve bug reports of StdLibraryFunctionsChecker.

2023-03-10 Thread Balázs Kéri via Phabricator via cfe-commits
balazske updated this revision to Diff 504185.
balazske added a comment.

Change format of bug reports.
Now the problem is shown first, then the acceptable values.
Sometimes the messages can become too verbose (in case of
"should be NULL") or grammatically not totally correct,
I can not tell if this is acceptable but this was the most
simple implementation.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144003/new/

https://reviews.llvm.org/D144003

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints-notes.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints-tracking-notes.c
  clang/test/Analysis/std-c-library-functions-arg-constraints.c
  clang/test/Analysis/std-c-library-functions-arg-constraints.cpp
  clang/test/Analysis/stream-note.c
  clang/test/Analysis/stream-stdlibraryfunctionargs.c

Index: clang/test/Analysis/stream-stdlibraryfunctionargs.c
===
--- clang/test/Analysis/stream-stdlibraryfunctionargs.c
+++ clang/test/Analysis/stream-stdlibraryfunctionargs.c
@@ -18,31 +18,31 @@
 void test_fopen(void) {
   FILE *fp = fopen("path", "r");
   clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}} any-warning{{FALSE}}
-  fclose(fp); // stdargs-warning{{should not be NULL}}
+  fclose(fp); // stdargs-warning{{should be not NULL}}
 }
 
 void test_tmpfile(void) {
   FILE *fp = tmpfile();
   clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}} any-warning{{FALSE}}
-  fclose(fp); // stdargs-warning{{should not be NULL}}
+  fclose(fp); // stdargs-warning{{should be not NULL}}
 }
 
 void test_fclose(void) {
   FILE *fp = tmpfile();
-  fclose(fp); // stdargs-warning{{should not be NULL}}
+  fclose(fp); // stdargs-warning{{should be not NULL}}
   clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
 }
 
 void test_freopen(void) {
   FILE *fp = tmpfile();
-  fp = freopen("file", "w", fp); // stdargs-warning{{should not be NULL}}
-  fclose(fp); // stdargs-warning{{should not be NULL}}
+  fp = freopen("file", "w", fp); // stdargs-warning{{should be not NULL}}
+  fclose(fp); // stdargs-warning{{should be not NULL}}
   clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
 }
 
 void test_fread(void) {
   FILE *fp = tmpfile();
-  size_t ret = fread(buf, size, n, fp); // stdargs-warning{{The 4th argument to 'fread' should not be NULL}}
+  size_t ret = fread(buf, size, n, fp); // stdargs-warning{{The 4th argument to 'fread' is NULL that is out of the accepted range}}
   clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
   clang_analyzer_eval(ret <= n); // any-warning{{TRUE}}
   clang_analyzer_eval(ret == n); // any-warning{{TRUE}} any-warning{{FALSE}}
@@ -52,7 +52,7 @@
 
 void test_fwrite(void) {
   FILE *fp = tmpfile();
-  size_t ret = fwrite(buf, size, n, fp); // stdargs-warning{{The 4th argument to 'fwrite' should not be NULL}}
+  size_t ret = fwrite(buf, size, n, fp); // stdargs-warning{{The 4th argument to 'fwrite' is NULL that is out of the accepted range}}
   clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
   clang_analyzer_eval(ret <= n); // any-warning{{TRUE}}
   clang_analyzer_eval(ret == n); // any-warning{{TRUE}} any-warning{{FALSE}}
@@ -62,21 +62,21 @@
 
 void test_fseek(void) {
   FILE *fp = tmpfile();
-  fseek(fp, 0, 0); // stdargs-warning{{should not be NULL}}
+  fseek(fp, 0, 0); // stdargs-warning{{should be not NULL}}
   clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
   fclose(fp);
 }
 
 void test_ftell(void) {
   FILE *fp = tmpfile();
-  ftell(fp); // stdargs-warning{{should not be NULL}}
+  ftell(fp); // stdargs-warning{{should be not NULL}}
   clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
   fclose(fp);
 }
 
 void test_rewind(void) {
   FILE *fp = tmpfile();
-  rewind(fp); // stdargs-warning{{should not be NULL}}
+  rewind(fp); // stdargs-warning{{should be not NULL}}
   clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
   fclose(fp);
 }
@@ -84,7 +84,7 @@
 void test_fgetpos(void) {
   FILE *fp = tmpfile();
   fpos_t pos;
-  fgetpos(fp, ); // stdargs-warning{{should not be NULL}}
+  fgetpos(fp, ); // stdargs-warning{{should be not NULL}}
   clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
   fclose(fp);
 }
@@ -92,35 +92,35 @@
 void test_fsetpos(void) {
   FILE *fp = tmpfile();
   fpos_t pos;
-  fsetpos(fp, ); // stdargs-warning{{should not be NULL}}
+  fsetpos(fp, ); // stdargs-warning{{should be not NULL}}
   clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
   fclose(fp);
 }
 
 void test_clearerr(void) {
   FILE *fp = tmpfile();
-  clearerr(fp); // stdargs-warning{{should not be NULL}}
+  clearerr(fp); // stdargs-warning{{should be not NULL}}
   clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
   fclose(fp);
 }
 
 void test_feof(void) {
   FILE *fp = 

[PATCH] D144003: [clang][analyzer] Improve bug reports of StdLibraryFunctionsChecker.

2023-02-17 Thread Balázs Kéri via Phabricator via cfe-commits
balazske added a comment.

The plan is to bring the checker out of the alpha package, the bug reports 
should be good enough for that. To modify bug report text generation a new 
patch would be better, after the current is finished. Maybe all single numbers 
in all allowed ranges that are 1, 2 (or 3?) long (like the 1,2,4,5,6 in the 
example) can be collected, this makes report generation more complicated than 
the current way.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144003/new/

https://reviews.llvm.org/D144003

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D144003: [clang][analyzer] Improve bug reports of StdLibraryFunctionsChecker.

2023-02-16 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Looks like a massive improvement. Yes, the warning text traditionally focuses 
on what exactly is wrong. Describing why it's wrong is important as well, but 
usually less important. We're making an extraordinary statement that the 
program is wrong, so warning with path notes should look like a proof of that:

> "Indeed, let's assume 'x' is greater than 0, Then the program takes true 
> branch on this if-statement. Then it assigns value '0' to pointer 'p'. Which 
> brings us to the statement '*p = 1' three lines below. Pointer 'p' is null, 
> so the program will crash. **// Therefore your code is broken, Q.E.D.**

It should not end with a general recommendation:

> "Indeed, let's assume 'x' is greater than 0. Then the program takes true 
> branch on this if-statement. Then it assigns value '0' to pointer 'p'. Which 
> brings us to the statement '*p = 1' three lines below. Null pointers should 
> not be dereferenced." **// I already know that, so what?**

This is especially important because people read reports from bottom to top: 
"Ok, if this happens it's indeed bad, now why do you think this happens?". So 
if they don't understand what happens (in your case, what value *is* there), 
they can't even start asking this question. So this is why I think this patch 
is amazing and it should have been like this from the start.

So if I was to come up with a full warning text myself, I'd probably suggest 
something along the lines of

> `Value 3 passed as 1st parameter of '__range_1_2__4_6', which expects 1 or 2, 
> or 4, 5 or 6`
> `Negative value passed as 1st parameter of '__range_1_2__4_6', which expects 
> 1 or 2, or 4, 5 or 6`

or maybe even

> `Variable 'x' passed as 1st parameter to '__range_1_2__4_6' takes values in 
> range [7, 10] whereas expected values for the parameter are 1 or 2, or 4, 5 
> or 6`

so that to explain what happens first, then explain why this is bad later. But 
even without such further improvements, this patch is great.




Comment at: 
clang/test/Analysis/std-c-library-functions-arg-constraints-notes.cpp:210
+if (x == 3)
+  __range_1_2__4_6(x); // expected-warning{{should be 1 or 2 or 4, 5 or 6 
but is 3 [}}
+return;

I'm definitely not an expert on [[ https://en.wikipedia.org/wiki/Serial_comma | 
Oxford commas ]] but adding one between individual ranges looks valuable as it 
helps the reader understand what makes `5` so special.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144003/new/

https://reviews.llvm.org/D144003

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D144003: [clang][analyzer] Improve bug reports of StdLibraryFunctionsChecker.

2023-02-14 Thread Balázs Kéri via Phabricator via cfe-commits
balazske created this revision.
Herald added subscribers: steakhal, manas, ASDenysPetrov, martong, gamesh411, 
dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, 
baloghadamsoftware, xazax.hun.
Herald added a reviewer: Szelethus.
Herald added a reviewer: NoQ.
Herald added a project: All.
balazske requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Add an additional explanation of what is wrong if a constraint is
not satisfied, in some cases.
Additionally the bug report generation is changed to use raw_ostream.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144003

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints-notes.cpp

Index: clang/test/Analysis/std-c-library-functions-arg-constraints-notes.cpp
===
--- clang/test/Analysis/std-c-library-functions-arg-constraints-notes.cpp
+++ clang/test/Analysis/std-c-library-functions-arg-constraints-notes.cpp
@@ -78,34 +78,36 @@
 int __range_out_1_2__4_6(int); // [1, 2], [4, 6]
 int __range_out_1_2__4_inf(int); // [1, 2], [4, inf]
 
+int __test_case_range_1_2__4_6(int);
+
 void test_range_values(int x) {
   switch (x) {
   case 0:
-__single_val_0(1); // expected-warning{{should be zero}}
+__single_val_0(1); // expected-warning{{should be zero but is 1}}
 break;
   case 1:
-__single_val_1(2); // expected-warning{{should be 1}}
+__single_val_1(2); // expected-warning{{should be 1 but is 2}}
 break;
   case 2:
-__range_1_2(3); // expected-warning{{should be 1 or 2}}
+__range_1_2(3); // expected-warning{{should be 1 or 2 but is 3}}
 break;
   case 3:
-__range_m1_1(3); // expected-warning{{should be between -1 and 1}}
+__range_m1_1(3); // expected-warning{{should be between -1 and 1 but is 3}}
 break;
   case 4:
-__range_m2_m1(1); // expected-warning{{should be -2 or -1}}
+__range_m2_m1(1); // expected-warning{{should be -2 or -1 but is 1}}
 break;
   case 5:
-__range_m10_10(11); // expected-warning{{should be between -10 and 10}}
+__range_m10_10(11); // expected-warning{{should be between -10 and 10 but is 11}}
 break;
   case 6:
-__range_m10_10(-11); // expected-warning{{should be between -10 and 10}}
+__range_m10_10(-11); // expected-warning{{should be between -10 and 10 but is -11}}
 break;
   case 7:
-__range_1_2__4_6(3); // expected-warning{{should be 1 or 2 or 4, 5 or 6}}
+__range_1_2__4_6(3); // expected-warning{{should be 1 or 2 or 4, 5 or 6 but is 3}}
 break;
   case 8:
-__range_1_2__4_inf(3); // expected-warning{{should be 1 or 2 or >= 4}}
+__range_1_2__4_inf(3); // expected-warning{{should be 1 or 2 or >= 4 but is 3}}
 break;
   }
 }
@@ -113,22 +115,22 @@
 void test_range_values_inf(int x) {
   switch (x) {
   case 1:
-__range_m1_inf(-2); // expected-warning{{should be >= -1}}
+__range_m1_inf(-2); // expected-warning{{should be >= -1 but is -2}}
 break;
   case 2:
-__range_0_inf(-1); // expected-warning{{should be >= 0}}
+__range_0_inf(-1); // expected-warning{{should be >= 0 but is -1}}
 break;
   case 3:
-__range_1_inf(0); // expected-warning{{should be > 0}}
+__range_1_inf(0); // expected-warning{{should be > 0 but is 0}}
 break;
   case 4:
-__range_minf_m1(0); // expected-warning{{should be < 0}}
+__range_minf_m1(0); // expected-warning{{should be < 0 but is 0}}
 break;
   case 5:
-__range_minf_0(1); // expected-warning{{should be <= 0}}
+__range_minf_0(1); // expected-warning{{should be <= 0 but is 1}}
 break;
   case 6:
-__range_minf_1(2); // expected-warning{{should be <= 1}}
+__range_minf_1(2); // expected-warning{{should be <= 1 but is 2}}
 break;
   }
 }
@@ -136,28 +138,28 @@
 void test_range_values_out(int x) {
   switch (x) {
   case 0:
-__single_val_out_0(0); // expected-warning{{should be nonzero}}
+__single_val_out_0(0); // expected-warning{{should be nonzero but is 0}}
 break;
   case 1:
-__single_val_out_1(1); // expected-warning{{should be not equal to 1}}
+__single_val_out_1(1); // expected-warning{{should be not equal to 1 but is 1}}
 break;
   case 2:
-__range_out_1_2(2); // expected-warning{{should be not 1 and not 2}}
+__range_out_1_2(2); // expected-warning{{should be not 1 and not 2 but is 2}}
 break;
   case 3:
-__range_out_m1_1(-1); // expected-warning{{should be not between -1 and 1}}
+__range_out_m1_1(-1); // expected-warning{{should be not between -1 and 1 but is -1}}
 break;
   case 4:
-__range_out_m2_m1(-2); // expected-warning{{should be not -2 and not -1}}
+__range_out_m2_m1(-2); // expected-warning{{should be not -2 and not -1 but is -2}}
 break;
   case 5:
-__range_out_m10_10(0); // expected-warning{{should be not between -10 and 10}}
+