martong added inline comments.
================
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'}}
----------------
balazske wrote:
> 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.
> I must say that I dislike that one cannot disable this warning unless they
> disable the whole stdlibrarymodeling.
Yeah, good point. Perhaps we should have a new checker option that
enables/disables errno related reports. Should this be part of the ErrnoChecker
(or the StdLibraryFunctionsChecker)?
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
