Disable LibFuncs for stpcpy and stpncpy for Android < 21

[lanza]

Summary

These functions don't exist in android API levels < 21. A change in
llvm-12 (rG6dbf0cfcf789) caused Oz builds to emit this symbol
assuming it's available and thus is causing link errors. Simply
disable the LibFunc here.

[smeenai]

Might be helpful to mention rG6dbf0cfcf789 in the commit message.

Is there any way to add a test to this (especially for the triple version conditional), similar to what rGc5638a71d805 did? I imagine it might be complicated because I think our original test case triggered two transforms (sprintf -> strcpy and then strcpy -> stpcpy), but hopefully there's some test for the stpcpy transform that we can piggy-back off.

EDIT: looks like the test modified by rGc5638a71d805 also has segments for the stpcpy transform, so we should just be able to use it.

Adding a bunch of Android folks as reviewers.

[lanza]

Add test

[smeenai]

llvm/test/Transforms/InstCombine/sprintf-1.ll

113–118 ↗

(On Diff #364297)

This is identical to the Windows case, right? Can we just combine the cases and use a more generic label (e.g. NOSTPCPY or something)?

You should also add a test using an Android 21 triple and verify that the stpcpy transform triggers for it.

[rprichard]

Yes, Android did add stpcpy and stpncpy in API 21:
https://android.googlesource.com/platform/bionic/+/refs/heads/android11-dev/libc/libc.map.txt

llvm/lib/Analysis/TargetLibraryInfo.cpp

595

I assume the problem is that LibFunc_stpcpy is available by default, and we need to override the default, marking it unavailable.

I wondered why the *_unlocked functions (D45736, D75933) seemed to assume the opposite default, but that's because there's code at the start of this function marking them unavailable. It took me a while to find it because LibFunc_getc_unlocked isn't initialized. (LibFunc_getchar_unlocked and various others are, though.) I'll upload a patch if it still looks broken after digging some more.

[lanza]

@tstellar This could get picked to llvm-13. It's a bug that was first found with us rolling out llvm-12 internally and is still present in 13.

[tstellar]

@lanza Can you file a bug for this and but release-13.0.0 in the blocks field.

[probinson]

llvm/test/Transforms/InstCombine/sprintf-1.ll

113–118 ↗

(On Diff #364297)

Yes, please make this a generic label; PS4 is going to have to do the same thing, we just had somebody trip over it. (Not asking you to do the PS4 change, just make the test easier for us to add our case later.)

[probinson]

llvm/test/Transforms/InstCombine/sprintf-1.ll

113–118 ↗

(On Diff #364297)

Ah nvm somehow hadn't noticed this already landed. We'll deal with it.

[smeenai]

llvm/test/Transforms/InstCombine/sprintf-1.ll

113–118 ↗

(On Diff #364297)

This was addressed post-commit by https://reviews.llvm.org/D107526.

llvm/lib/Analysis/TargetLibraryInfo.cpp

if ((T.isOSLinux() && T.isGNUEnvironment()) ||
     TLI.setAvailable(LibFunc_fputc_unlocked);
     TLI.setAvailable(LibFunc_fgetc_unlocked);
     TLI.setAvailable(LibFunc_fread_unlocked);
     TLI.setAvailable(LibFunc_fwrite_unlocked);
     TLI.setAvailable(LibFunc_fputs_unlocked);
     TLI.setAvailable(LibFunc_fgets_unlocked);
   }
 
+  if (T.isAndroid() && T.isAndroidVersionLT(21)) {
+    TLI.setUnavailable(LibFunc_stpcpy);
+    TLI.setUnavailable(LibFunc_stpncpy);
+  }
+
   // As currently implemented in clang, NVPTX code has no standard library to
   // speak of.  Headers provide a standard-ish library implementation, but many
   // of the signatures are wrong -- for example, many libm functions are not
   // extern "C".
   //
   // libdevice, an IR library provided by nvidia, is linked in by the front-end,
   // but only used functions are provided to llvm.  Moreover, most of the
   // functions in libdevice don't map precisely to standard library functions.