[PATCH] D47002: [Analysis] Only use _unlocked stdio functions on linux

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added inline comments.


================
Comment at: lib/Analysis/TargetLibraryInfo.cpp:485
 
-  if (T.isGNUEnvironment() || (T.isAndroid() && !T.isAndroidVersionLT(28))) {
+  if (T.isOSLinux() || (T.isAndroid() && !T.isAndroidVersionLT(28))) {
     // available IO unlocked variants on GNU/Linux and Android P or later
----------------
xbolva00 wrote:
> xbolva00 wrote:
> > srhines wrote:
> > > mstorsjo wrote:
> > > > xbolva00 wrote:
> > > > > I would leave isLinux && isGNU...
> > > > Yeah, I thought about that as well - but the rest of this function just plainly does isOSLinux everywhere, even if comments talk about glibc specifically. But I can make it Linux && GNU here for extra clarity.
> > > isOSLinux is true for Android, isn't it? Doesn't that break the rest of the case for < API 28 on Android?
> > Probably yes.
> > 
> > E.g. musl implements them as aliases to locked variants..
> > https://github.com/esmil/musl/blob/194f9cf93da8ae62491b7386edf481ea8565ae4e/src/stdio/fwrite.c#L37
> I think this is okay:
> 
> if ((T.isOSLinux() && T.isGNUEnvironment()) || (T.isAndroid() && !T.isAndroidVersionLT(28))) {
> isOSLinux is true for Android, isn't it? Doesn't that break the rest of the case for < API 28 on Android?

Oh right, yes, we clearly should make it linux&&GNU then.


Repository:
  rL LLVM

https://reviews.llvm.org/D47002