[PATCH] D106262: [clang][analyzer] Use generic note tag in alpha.unix.Stream .

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

We had a meeting outside phabricator, here is the gist of it:

- It'd be better to have your initially proposed schrödinger-like `NoteTag`s.
- The intent is to emit a warning for each of the error states in `StreamState`. It seems to me that this is not what we're doing now, but, similarly to adding warnings fro FERROR and being able to clear the indeterminate file position indicator state, would truly display how `NoteTag`s might change their message depending on the `BugType`.
- There are numerous discussions to be had on how to handle `fseek`, how much do FERROR and the file position indicator imply one another, but for this patch, adding debug functions such as `void StreamCheckerTest_remove_indeterminate_file_pos_tag(FILE *)` or `void StreamCheckerTest_warn_if_in_ferror(FILE *)` would allow as to test the functionality added by this patch, and land it.

As such, I'd like if you implemented the above mentioned functions with their expected functionality. I'd like to test cases where the very same function call gets a different note message depending on the `BugType`:

  void fseek_caused_feof() {
    FILE *F;
    char Buf[10];
    F = fopen("foo1.c", "r");
    if (!F) // expected-note {{Taking false branch}} expected-note {{'F' is non-null}}
      return;
    fseek(F, 1, SEEK_SET); // expected-note {{Stream reaches end-of-file here}} <============= (*)
    if (ferror(F)) // expected-note {{Assuming the error flag is not set on the stream}}
                   // expected-note at -1 {{Taking false branch}}
      return;
    StreamCheckerTest_remove_indeterminate_file_pos_tag(F);
    fread(Buf, 1, 1, F); // expected-warning{{Read function called when stream is in EOF state. Function has no effect}}
    // expected-note at -1{{Read function called when stream is in EOF state. Function has no effect}}
    fclose(F);
  }
  
  void fseek_caused_ferror() {
    FILE *F;
    char Buf[10];
    F = fopen("foo1.c", "r");
    if (!F) // expected-note {{Taking false branch}} expected-note {{'F' is non-null}}
      return;
    fseek(F, 1, SEEK_SET); // expected-note {{Stream has its error flag set}} <============= (*)
    if (feof(F)) // expected-note {{Assuming the end-of-file flag is not set on the stream}}
                 // expected-note at -1 {{Taking false branch}}
      return;
    StreamCheckerTest_remove_indeterminate_file_pos_tag(F);
    StreamCheckerTest_warn_if_in_ferror(F); // expected-warning{{Read function called when stream is in FERROR state.}}
    // expected-note at -1{Read function called when stream is in FERROR state.}
    fclose(F);
  }
  
  void fseek_caused_indeterminate_file_pos() {
    FILE *F;
    char Buf[10];
    F = fopen("foo1.c", "r");
    if (!F) // expected-note {{Taking false branch}} expected-note {{'F' is non-null}}
      return;
    fseek(F, 1, SEEK_SET); // expected-note {{Stream operation leaves the file position indicator indeterminate}} <============= (*)
    clearerr(F);
    fread(Buf, 1, 1, F); // expected-warning{{File position of the stream might be 'indeterminate' after a failed operation. Can cause undefined behavior}}
    // expected-note at -1{{File position of the stream might be 'indeterminate' after a failed operation. Can cause undefined behavior}}
    fclose(F);
  }

Maybe some others in the spirit of these 3.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106262