balazske added a comment.
The documentation of `ErrnoChecker` is added in the previous change D122150
<https://reviews.llvm.org/D122150>. Really this documentation should be updated
when any checker is changed related to errno modeling (errno modeling can be
added later to any checker that evaluates standard functions, for example
`StreamChecker`). Is it required to mention the `ErrnoModeling` dependency?
(The modeling checker is turned on always?) Or add documentation for
`ErrnoModeling` too?
================
Comment at: clang/test/Analysis/errno-stdlibraryfunctions-notes.c:15-23
+void test1() {
+ access("path", 0); // no note here
+ access("path", 0);
+ // expected-note@-1{{Assuming that function 'access' is successful, in this
case the value 'errno' may be undefined after the call and should not be used}}
+ 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'}}
----------------
steakhal wrote:
> I must say that I dislike that one cannot disable this warning unless they
> disable the whole stdlibrarymodeling. It's getting bigger and bigger, thus
> increasing the value it provides. Yet, not everybody wants to follow SEI CERT
> guidelines - and I would expect plenty of users in that category, who might
> be surprised that reading `errno` is a bug.
>
> Just imagine in the example that between the two `access()` calls, you set
> `errno` to zero.
> The code would look reasonable.
> 1) reset errno
> 2) do some syscall
> 3) check errno
>
> I would expect this to be a good practice. We should at least have an option
> for suppressing reports for such scenarios. IMO it should be rather opt-in,
> than opt-out, but it might be my personal preference.
> WDYT @martong @xazax.hun
The CERT rule says that there is no guarantee that `errno` is not changed if
the function is successful, regardless if it was 0 or not. There are some
functions (that have //in-band return value//) where it is required to set
`errno` to 0, there can be another checker (or extend `ErrnoChecker`) to check
this.
The new warning is turned on when `alpha.unix.Errno` is turned on, otherwise it
should not appear.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125400/new/
https://reviews.llvm.org/D125400
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
