[PATCH] D56146: [Sanitizer] enable arc4random api on Darwin

[David CARLIER via Phabricator via llvm-commits]

devnexen marked an inline comment as done and an inline comment as not done.
devnexen added inline comments.


================
Comment at: lib/sanitizer_common/sanitizer_mac.cc:1122-1123
+  int rverrno = 0;
+  if (internal_iserror(rnd, &rverrno) && rverrno == EINVAL)
+     return false;
   return true;
----------------
cem wrote:
> cem wrote:
> > devnexen wrote:
> > > devnexen wrote:
> > > > devnexen wrote:
> > > > > cem wrote:
> > > > > >  # Why would getentropy produce an `EINVAL` code?  The only documented errors are EFAULT (depends on `buffer`) and EIO (`length` checks above should prevent).  Linux documents an additional ENOSYS code if getrandom is absent, but not EINVAL, AFAIK.
> > > > > > 
> > > > > >   - https://man.openbsd.org/getentropy.2
> > > > > >   - http://man7.org/linux/man-pages/man3/getentropy.3.html
> > > > > >   - https://www.freebsd.org/cgi/man.cgi?query=getentropy&sektion=3&manpath=freebsd-release-ports
> > > > > > 
> > > > > >  (I am unfamiliar with `internal_iserror`; please forgive me if it is expected to squash other errors to `EINVAL` for some reason. :-))
> > > > > > 
> > > > > >  # Why should errors other than `EINVAL` be treated as success conditions?
> > > > > - It is mac os and getentropy could return EINVAL https://www.manpagez.com/man/2/getentropy/
> > > > > - I change to getentropy because arc4random_buf call is intercepted with this PR.
> > > > To add to the discussion EINVAL is not a success here, in contrary.
> > > Sorry I typed too fast ... the EIO error is not happening because of the length check above.
> > I mean, I don't think there's any reason to return `true` if `if (internal_iserror(rnd, &rverrno)` and `rverrno != EINVAL`.  So the check for `EINVAL` seems wrong.
> > 
> > ```
> > if (internal_iserror(rnd, &rverrno))
> >   return false;
> > ```
> > 
> > Seems better to me.  But I do not use Mac so it does not really affect me.
> If errors other than EINVAL are impossible, why check for EINVAL? :-)
```return internal_iserror``` might suffice in fact ... for now unless darwin decides to update their API to be more inline with the rest of the "crew" ... just to be a bit safe :-) why anyway I requested apple guys for the review anyway :-)



Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D56146