[PATCH] D132017: [clang][analyzer] Add errno modeling to StreamChecker

[Balázs Kéri via Phabricator via cfe-commits]

balazske abandoned this revision.
balazske added a comment.

A new solution is in D135247 . The new 
approach is that only `StdLibraryFunctionsChecker` adds the errno modeling 
part, together with other checks (that are applicable if StreamChecker is 
turned off) for pre and post conditions at the stream functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132017

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

[PATCH] D132017: [clang][analyzer] Add errno modeling to StreamChecker

[Gabor Marton via Phabricator via cfe-commits]

martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!




Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:465
+  /// post-call event.
+  class NoErrnoConstraint : public ErrnoConstraintBase {
+  public:

Makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132017

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

[PATCH] D132017: [clang][analyzer] Add errno modeling to StreamChecker

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a reviewer: martong.
balazske added inline comments.
Herald added a subscriber: rnkovacs.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:537
+  errno_modeling::getNoteTagForStdSuccess(
+  C, cast(Call.getDecl())->getNameAsString()));
   C.addTransition(StateNull);

steakhal wrote:
> I believe, `getDecl()` might return null, e.g. for calling a function pointer.
In this case the function should be known (we know that `fopen` is the called 
function, if the name is known the `Decl` should be known too), I think it can 
not be null here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132017

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

[PATCH] D132017: [clang][analyzer] Add errno modeling to StreamChecker

[Balázs Kéri via Phabricator via cfe-commits]

balazske updated this revision to Diff 454026.
balazske marked 2 inline comments as done.
balazske added a comment.

- Use `Optional` for `EofInitialized`
- Split tests into two files with and without note check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132017

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
  clang/test/Analysis/stream-errno-note.c
  clang/test/Analysis/stream-errno.c

Index: clang/test/Analysis/stream-errno.c
===
--- /dev/null
+++ clang/test/Analysis/stream-errno.c
@@ -0,0 +1,123 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,alpha.unix.Errno,apiModeling.StdCLibraryFunctions,debug.ExprInspection \
+// RUN:   -verify %s
+
+#include "Inputs/system-header-simulator.h"
+#include "Inputs/errno_func.h"
+
+extern void clang_analyzer_eval(int);
+extern void clang_analyzer_dump(int);
+
+void check_fopen(void) {
+  FILE *F = fopen("xxx", "r");
+  if (!F) {
+clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+if (errno) {} // no-warning
+return;
+  }
+  if (errno) {} // expected-warning{{An undefined value may be read from 'errno' [alpha.unix.Errno]}}
+}
+
+void check_tmpfile(void) {
+  FILE *F = tmpfile();
+  if (!F) {
+clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+if (errno) {} // no-warning
+return;
+  }
+  if (errno) {} // expected-warning{{An undefined value may be read from 'errno' [alpha.unix.Errno]}}
+}
+
+void check_freopen(void) {
+  FILE *F = tmpfile();
+  if (!F)
+return;
+  F = freopen("xxx", "w", F);
+  if (!F) {
+clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+if (errno) {} // no-warning
+return;
+  }
+  if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
+}
+
+void check_fclose(void) {
+  FILE *F = tmpfile();
+  if (!F)
+return;
+  int Ret = fclose(F);
+  if (Ret == EOF) {
+clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+if (errno) {} // no-warning
+return;
+  }
+  if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
+}
+
+void check_fread(void) {
+  char Buf[10];
+  FILE *F = tmpfile();
+  if (!F)
+return;
+  fread(Buf, 0, 1, F);
+  if (errno) {} // no-warning
+  fread(Buf, 1, 0, F);
+  if (errno) {} // no-warning
+
+  int R = fread(Buf, 1, 10, F);
+  if (R < 10) {
+clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+if (errno) {} // no-warning
+fclose(F);
+return;
+  }
+  if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
+}
+
+void check_fwrite(void) {
+  char Buf[] = "0123456789";
+  FILE *F = tmpfile();
+  if (!F)
+return;
+  fwrite(Buf, 0, 1, F);
+  if (errno) {} // no-warning
+  fwrite(Buf, 1, 0, F);
+  if (errno) {} // no-warning
+
+  int R = fwrite(Buf, 1, 10, F);
+  if (R < 10) {
+clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+if (errno) {} // no-warning
+fclose(F);
+return;
+  }
+  if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
+}
+
+void check_fseek(void) {
+  FILE *F = tmpfile();
+  if (!F)
+return;
+  int S = fseek(F, 11, SEEK_SET);
+  if (S != 0) {
+clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+if (errno) {} // no-warning
+fclose(F);
+return;
+  }
+  if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
+}
+
+void check_no_errno_change(void) {
+  FILE *F = tmpfile();
+  if (!F)
+return;
+  errno = 1;
+  clearerr(F);
+  if (errno) {} // no-warning
+  feof(F);
+  if (errno) {} // no-warning
+  ferror(F);
+  if (errno) {} // no-warning
+  clang_analyzer_eval(errno == 1); // expected-warning{{TRUE}}
+  fclose(F);
+}
Index: clang/test/Analysis/stream-errno-note.c
===
--- /dev/null
+++ clang/test/Analysis/stream-errno-note.c
@@ -0,0 +1,99 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,alpha.unix.Errno,apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-output text -verify %s
+
+#include "Inputs/system-header-simulator.h"
+#include "Inputs/errno_func.h"
+
+void check_fopen(void) {
+  FILE *F = fopen("xxx", "r");
+  // expected-note@-1{{Assuming that function 'fopen' is successful, in this case the value 'errno' may be undefined after the call and should not be used}}
+  // expected-note@+2{{'F' is non-null}}
+  // expected-note@+1{{Taking false branch}}
+  if (!F)
+return;
+  if (errno) {} // expected-warning{{An undefined value may be read from 'errno' [alpha.unix.Errno]}}
+  // expected-note@-1{{An undefined value may be read from 'errno'}}
+  fclose(F);
+}
+
+void check_tmpfile(void) {
+  FILE *F = tmpfile();
+  // expected-note@-1{{A

[PATCH] D132017: [clang][analyzer] Add errno modeling to StreamChecker

[Balázs Kéri via Phabricator via cfe-commits]

balazske updated this revision to Diff 453297.
balazske added a comment.

StdLibraryFunctionsChecker should not overwrite errno constraints
that are set by StreamChecker.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132017

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
  clang/test/Analysis/stream-errno.c

Index: clang/test/Analysis/stream-errno.c
===
--- /dev/null
+++ clang/test/Analysis/stream-errno.c
@@ -0,0 +1,196 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,alpha.unix.Errno,debug.ExprInspection -analyzer-output text -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,alpha.unix.Errno,apiModeling.StdCLibraryFunctions,debug.ExprInspection -analyzer-output text -verify %s
+
+#include "Inputs/system-header-simulator.h"
+#include "Inputs/errno_func.h"
+
+extern void clang_analyzer_eval(int);
+extern void clang_analyzer_dump(int);
+
+void check_fopen(void) {
+  FILE *F = fopen("xxx", "r");
+  // expected-note@-1{{Assuming that function 'fopen' is successful, in this case the value 'errno' may be undefined after the call and should not be used}}
+  // expected-note@+4{{'F' is null}}
+  // expected-note@+3{{Taking true branch}}
+  // expected-note@+2{{'F' is non-null}}
+  // expected-note@+1{{Taking false branch}}
+  if (!F) {
+clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}} expected-note{{TRUE}}
+if (errno) {}
+return;
+  }
+  if (errno) {} // expected-warning{{An undefined value may be read from 'errno' [alpha.unix.Errno]}}
+  // expected-note@-1{{An undefined value may be read from 'errno'}}
+}
+
+void check_tmpfile(void) {
+  FILE *F = tmpfile();
+  // expected-note@-1{{Assuming that function 'tmpfile' is successful, in this case the value 'errno' may be undefined after the call and should not be used}}
+  // expected-note@+4{{'F' is null}}
+  // expected-note@+3{{Taking true branch}}
+  // expected-note@+2{{'F' is non-null}}
+  // expected-note@+1{{Taking false branch}}
+  if (!F) {
+clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}} expected-note{{TRUE}}
+if (errno) {}
+return;
+  }
+  if (errno) {} // expected-warning{{An undefined value may be read from 'errno' [alpha.unix.Errno]}}
+  // expected-note@-1{{An undefined value may be read from 'errno'}}
+}
+
+void check_freopen(void) {
+  FILE *F = tmpfile();
+  // expected-note@+4{{'F' is non-null}}
+  // expected-note@+3{{Taking false branch}}
+  // expected-note@+2{{'F' is non-null}}
+  // expected-note@+1{{Taking false branch}}
+  if (!F)
+return;
+  F = freopen("xxx", "w", F);
+  // expected-note@-1{{Assuming that function 'freopen' is successful, in this case the value 'errno' may be undefined after the call and should not be used}}
+  // expected-note@+4{{'F' is null}}
+  // expected-note@+3{{Taking true branch}}
+  // expected-note@+2{{'F' is non-null}}
+  // expected-note@+1{{Taking false branch}}
+  if (!F) {
+clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}} expected-note{{TRUE}}
+if (errno) {}
+return;
+  }
+  if (errno) {} // expected-warning{{An undefined value may be read from 'errno' [alpha.unix.Errno]}}
+  // expected-note@-1{{An undefined value may be read from 'errno'}}
+}
+
+void check_fclose(void) {
+  FILE *F = tmpfile();
+  // expected-note@+4{{'F' is non-null}}
+  // expected-note@+3{{Taking false branch}}
+  // expected-note@+2{{'F' is non-null}}
+  // expected-note@+1{{Taking false branch}}
+  if (!F)
+return;
+  int Ret = fclose(F);
+  // expected-note@-1{{Assuming that function 'fclose' is successful, in this case the value 'errno' may be undefined after the call and should not be used}}
+  // expected-note@+2{{Taking true branch}}
+  // expected-note@+1{{Taking false branch}}
+  if (Ret == EOF) {
+clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}} expected-note{{TRUE}}
+if (errno) {}
+return;
+  }
+  if (errno) {} // expected-warning{{An undefined value may be read from 'errno' [alpha.unix.Errno]}}
+  // expected-note@-1{{An undefined value may be read from 'errno'}}
+}
+
+void check_fread(void) {
+  char Buf[10];
+  FILE *F = tmpfile();
+  // expected-note@+4{{'F' is non-null}}
+  // expected-note@+3{{Taking false branch}}
+  // expected-note@+2{{'F' is non-null}}
+  // expected-note@+1{{Taking false branch}}
+  if (!F)
+return;
+  fread(Buf, 0, 1, F);
+  // expected-note@+2{{Taking false branch}}
+  // expected-note@+1{{Taking false branch}}
+  if (errno) {} // no-warning
+  fread(Buf, 1, 0, F);
+  // expected-note@+2{{Taking false branch}}
+  // expected-note@+1{{Taking false branch}}
+  if (errno) {} // no-warning
+
+  int R = fread(Buf, 1, 10, F);
+  // expected-note@-1{{function 'fread' is successful}}
+

[PATCH] D132017: [clang][analyzer] Add errno modeling to StreamChecker

[Balázs Benics via Phabricator via cfe-commits]

steakhal added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:283-284
 
+  mutable bool EofInitialized = false;
+  mutable int EofVal = -1;
+

Have you considered using `llvm::Optional` for this?



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:537
+  errno_modeling::getNoteTagForStdSuccess(
+  C, cast(Call.getDecl())->getNameAsString()));
   C.addTransition(StateNull);

I believe, `getDecl()` might return null, e.g. for calling a function pointer.



Comment at: clang/test/Analysis/stream-errno.c:12-13
+  // expected-note@-1{{Assuming that function 'fopen' is successful, in this 
case the value 'errno' may be undefined after the call and should not be used}}
+  // expected-note@+4{{'F' is null}}
+  // expected-note@+3{{Taking true branch}}
+  // expected-note@+2{{'F' is non-null}}

So these two lines correspond to the following `clang_analyzer_eval()`.
It took me a while to make sense of the two independent note streams at once.
Please consider splitting this and similar cases to have a single stream of 
notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132017

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

[PATCH] D132017: [clang][analyzer] Add errno modeling to StreamChecker

[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.

Update the existing modeling of stream functions in StreamChecker
with 'errno' modeling. The not (yet) evaluated stream functions
are not updated.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132017

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
  clang/test/Analysis/stream-errno.c

Index: clang/test/Analysis/stream-errno.c
===
--- /dev/null
+++ clang/test/Analysis/stream-errno.c
@@ -0,0 +1,196 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,alpha.unix.Errno,debug.ExprInspection -analyzer-output text -verify %s
+
+#include "Inputs/system-header-simulator.h"
+#include "Inputs/errno_func.h"
+
+extern void clang_analyzer_eval(int);
+extern void clang_analyzer_dump(int);
+
+void check_fopen(void) {
+  FILE *F = fopen("xxx", "r");
+  // expected-note@-1{{Assuming that function 'fopen' is successful, in this case the value 'errno' may be undefined after the call and should not be used}}
+  // expected-note@+4{{'F' is null}}
+  // expected-note@+3{{Taking true branch}}
+  // expected-note@+2{{'F' is non-null}}
+  // expected-note@+1{{Taking false branch}}
+  if (!F) {
+clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}} expected-note{{TRUE}}
+if (errno) {}
+return;
+  }
+  if (errno) {} // expected-warning{{An undefined value may be read from 'errno' [alpha.unix.Errno]}}
+  // expected-note@-1{{An undefined value may be read from 'errno'}}
+}
+
+void check_tmpfile(void) {
+  FILE *F = tmpfile();
+  // expected-note@-1{{Assuming that function 'tmpfile' is successful, in this case the value 'errno' may be undefined after the call and should not be used}}
+  // expected-note@+4{{'F' is null}}
+  // expected-note@+3{{Taking true branch}}
+  // expected-note@+2{{'F' is non-null}}
+  // expected-note@+1{{Taking false branch}}
+  if (!F) {
+clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}} expected-note{{TRUE}}
+if (errno) {}
+return;
+  }
+  if (errno) {} // expected-warning{{An undefined value may be read from 'errno' [alpha.unix.Errno]}}
+  // expected-note@-1{{An undefined value may be read from 'errno'}}
+}
+
+void check_freopen(void) {
+  FILE *F = tmpfile();
+  // expected-note@+4{{'F' is non-null}}
+  // expected-note@+3{{Taking false branch}}
+  // expected-note@+2{{'F' is non-null}}
+  // expected-note@+1{{Taking false branch}}
+  if (!F)
+return;
+  F = freopen("xxx", "w", F);
+  // expected-note@-1{{Assuming that function 'freopen' is successful, in this case the value 'errno' may be undefined after the call and should not be used}}
+  // expected-note@+4{{'F' is null}}
+  // expected-note@+3{{Taking true branch}}
+  // expected-note@+2{{'F' is non-null}}
+  // expected-note@+1{{Taking false branch}}
+  if (!F) {
+clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}} expected-note{{TRUE}}
+if (errno) {}
+return;
+  }
+  if (errno) {} // expected-warning{{An undefined value may be read from 'errno' [alpha.unix.Errno]}}
+  // expected-note@-1{{An undefined value may be read from 'errno'}}
+}
+
+void check_fclose(void) {
+  FILE *F = tmpfile();
+  // expected-note@+4{{'F' is non-null}}
+  // expected-note@+3{{Taking false branch}}
+  // expected-note@+2{{'F' is non-null}}
+  // expected-note@+1{{Taking false branch}}
+  if (!F)
+return;
+  int Ret = fclose(F);
+  // expected-note@-1{{Assuming that function 'fclose' is successful, in this case the value 'errno' may be undefined after the call and should not be used}}
+  // expected-note@+2{{Taking true branch}}
+  // expected-note@+1{{Taking false branch}}
+  if (Ret == EOF) {
+clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}} expected-note{{TRUE}}
+if (errno) {}
+return;
+  }
+  if (errno) {} // expected-warning{{An undefined value may be read from 'errno' [alpha.unix.Errno]}}
+  // expected-note@-1{{An undefined value may be read from 'errno'}}
+}
+
+void check_fread(void) {
+  char Buf[10];
+  FILE *F = tmpfile();
+  // expected-note@+4{{'F' is non-null}}
+  // expected-note@+3{{Taking false branch}}
+  // expected-note@+2{{'F' is non-null}}
+  // expected-note@+1{{Taking false branch}}
+  if (!F)
+return;
+  fread(Buf, 0, 1, F);
+  // expected-note@+2{{Taking false branch}}
+  // expected-note@+1{{Taking false branch}}
+  if (errno) {} // no-warning
+  fread(Buf, 1, 0, F);
+  // expected-note@+2{{Taking false branch}}
+  // expected-note@+1{{Taking false branch}}
+  if (errno) {} // no-warning
+
+