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.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
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.
llvm/test/Transforms/InstCombine/sprintf-1.ll | ||
---|---|---|
113–118 | 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. |
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. |
@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.
llvm/test/Transforms/InstCombine/sprintf-1.ll | ||
---|---|---|
113–118 | 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.) |
llvm/test/Transforms/InstCombine/sprintf-1.ll | ||
---|---|---|
113–118 | Ah nvm somehow hadn't noticed this already landed. We'll deal with it. |
llvm/test/Transforms/InstCombine/sprintf-1.ll | ||
---|---|---|
113–118 | This was addressed post-commit by https://reviews.llvm.org/D107526. |
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.