NDK's version markers (e.g. LIBC) cause warnings on L MR1 (API 22)

[rprichard]

Split off from #602. That issue complained that even a Hello World executable prints linker errors about an unrecognized PIE flag. One of the comments instead complained about DT entry errors:

#602 (comment)

I debugged it here: https://android-review.googlesource.com/c/toolchain/binutils/+/571550#message-07efb48b6e8c26830153ec7ae05384dfb2acf14a. It only happens on L MR1 (API 22).

Test program:

#include <stdio.h>
int main() {
  printf("hi\n");
  return 0;
}

$ /ssd/stand-arm-14-r16b/bin/clang -pie -fPIE hello.c

$ export ANDROID_SERIAL=emulator-5554
$ adb shell getprop | grep '\(fingerprint\|version.sdk\)'
[ro.build.fingerprint]: [Android/sdk_google_phone_armv7/generic:5.1.1/LMY48X/4409265:userdebug/test-keys]
[ro.build.version.sdk]: [22]

$ adb push a.out /data/local/tmp/a.out
a.out: 1 file pushed. 0.0 MB/s (6620 bytes in 0.292s)

$ adb shell /data/local/tmp/a.out
WARNING: linker: /data/local/tmp/a.out: unused DT entry: type 0x6ffffffe arg 0x300
WARNING: linker: /data/local/tmp/a.out: unused DT entry: type 0x6fffffff arg 0x1
hi

The unrecognized 0x6ffffffe and 0x6fffffff DT entries are the VERNEED and VERNEEDNUM entries in a.out:

$ readelf -d a.out | grep VERNEED
 0x6ffffffe (VERNEED)                    0x300
 0x6fffffff (VERNEEDNUM)                 1

$ readelf -V a.out

Version symbols section '.gnu.version' contains 7 entries:
 Addr: 00000000000002f0  Offset: 0x0002f0  Link: 3 (.dynsym)
  000:   0 (*local*)       2 (LIBC)          2 (LIBC)          2 (LIBC)       
  004:   1 (*global*)      1 (*global*)      1 (*global*)   

Version needs section '.gnu.version_r' contains 1 entries:
 Addr: 0x0000000000000300  Offset: 0x000300  Link: 4 (.dynstr)
  000000: Version: 1  File: libc.so  Cnt: 1
  0x0010:   Name: LIBC  Flags: none  Version: 2

a.out has these symbols because the libc.so stub library has marked all its symbols with a LIBC version.

$ readelf --dyn-syms -W ~/android-ndk-r16b/platforms/android-14/arch-arm/usr/lib/libc.so | grep '\bprintf'
   675: 00007a85     2 FUNC    GLOBAL DEFAULT    7 printf@@LIBC

$ readelf --dyn-syms -W a.out | grep '\bprintf'
     1: 00000000     0 FUNC    GLOBAL DEFAULT  UND printf@LIBC (2)

I'm not sure which NDK release was the first to use these version markers. r13b has them. r10e doesn't:

$ readelf --dyn-syms -W ~/android-ndk-r10e/platforms/android-14/arch-arm/usr/lib/libc.so | grep '\bprintf'
   564: 0000ab4c    20 FUNC    GLOBAL DEFAULT    5 printf

$ /ssd/stand-arm-14-r10e/bin/arm-linux-androideabi-gcc -pie -fPIE hello.c
$ readelf -d a.out | grep VERNEED
$ readelf -V a.out

No version information found in this file.

$ adb push a.out /data/local/tmp/a.out
a.out: 1 file pushed. 1.0 MB/s (6200 bytes in 0.006s)
$ adb shell /data/local/tmp/a.out
hi

Maybe we can simply leave the version markers off of libc.so stub libraries for 22 and below?

I'm not quite sure why we have these version markers. Maybe we want binaries on newer OSs to have them, and they don't break typical apps even on L MR1. There's also a LIBC_DEPRECATED version marker, and starting with N, each new version's APIs have a different marker (LIBC_N, LIBC_O, LIBC_P).

[enh]

Maybe we want binaries on newer OSs to have them...

yeah, the idea is that once NDK apps refer to the specific version they want, we can offer modified behaviors via new versions rather than api-level checks. (though obviously the semantics are quite different: an api-level check applies based on your AndroidManifest targetSdkVersion regardless of the NDK target you used, whereas symbol versioning applies based on the NDK target you used regardless of the AndroidManifest targetSdkVersion. so probably not very useful for apps, but maybe useful for vendor code...)

[alexcohn]

api-level check applies based on your AndroidManifest targetSdkVersion

What api-level checks do you mean? I know either static checks for the platform API (usually, based on minSdkVersion), or dynamic checks (based on the platform we are running on).

True, the system dynamic linker uses some shims if the targetSdkVersion is less than the platform version, but this has little to do with version markers on libc and her kin.

[enh]

we're not using this yet but the idea is that if have function foo and it has a bug, but fixing the bug would break too many existing callers, we can offer foo@LIBC_Q with the "correct" behavior without taking the old behavior away for existing code. in practice we currently (a) don't find out about such things until it's far too late anyway or (b) use if in the implementation to decide what to do based on API level.

[alexcohn]

Yes, adding if in implementation shim is much safer than relying on mysterious version markers.

[DanAlbert]

So where did we land on this? Do we want to drop the versioning info for minSdkVersion < 23 or just mark with WAI?

@dimitry- was the one that was pushing for getting versioning information on everything, so maybe he remembers better why we did it this way (iirc it's so that you get the correctly versioned symbol on M+ if we end up altering it).

I think I agree with @enh that it's probably better to do this sort of thing based on your targetSdkVersion (do we have that info in libc?), so triaging for r18 unless Dimitry reminds us of something we're forgetting.

[dimitry-]

The main reason was to get as much apps as possible to use symbols so we could take advantage of being able to provide to different implementation for different version sooner.

Checking targetSdkVersion and having bug-compatible implementation for the symbol are similar but I can thing of one case where symbol versions can provide better compatibility management. That is if app has a third party library which relies on bug-compatible behavior of a libc function but app itself wants to target different sdk version.

Another less likely scenario is app using 2 different third party libraries and only one of them relies on bug-compatible implementation of a libc function. the other one has already been fixed and uses the later version.

[alexcohn]

@dimitry- the latter scenario falls in the category of "nightmare".

The most important challenge is to keep the native libs upwards compatible. The minSdkVersion (reflected by ANDROID_PLATFORM for Cmake) should be the only thing that defines this.

[DanAlbert]

Yeah, I think we should go ahead and unversion pre-M. We're not punting versions indefinitely, it's just a few more years down the road.

[DanAlbert]

https://android-review.googlesource.com/#/c/platform/build/soong/+/678046

[DanAlbert]

@rprichard: You suggested the following:

I'm wondering if we can fix the problem with a libc.so stub that has a default unversioned symbol as well as a LIBC versioned symbol. I've done a little testing, and it seems plausible.

Want to give that a try for r19?

[rprichard]

@DanAlbert Yeah, that sounds good to me.

[DanAlbert]

@rprichard: if you haven't had a chance to look at this by beta 2, punt to r20.

[frroliveira]

@rprichard I'm facing this issue when running a cocos2d-x app with API 22. App won't start anymore. Other APIs (23 and 28) are ok.
Can you advise a solution before fix is released? I'm using latest ndk.

Thanks

[DanAlbert]

This won't prevent an app from starting. It's just a warning.

[tesoy-xx]

https://github.com/tesoy-xx/FrankeNDK.git

[DanAlbert]

@rprichard do you want to look at this for r21?

[rprichard]

I can take a closer look at this before r21 ships, but my current impression is that we can't fix it until the NDK switches to lld, because of differences in the way bfd/gold and lld handle version markers.

[DanAlbert]

Ah, that's right. Okay, I'm going to move this into unplanned then and we'll pick it up later.