Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Android linking fails with NDK r11b: undefined reference to 'bsd_signal' #236

Closed
skligys opened this issue Mar 18, 2016 · 7 comments · Fixed by rust-lang/rust#32415
Closed

Comments

@skligys
Copy link

skligys commented Mar 18, 2016

Cross-compiling on Linux x64 to Android, rustc compiled from nightly sources as of 2016-03-17.

$ rustc --version -v
rustc 1.9.0-dev
binary: rustc
commit-hash: unknown
commit-date: unknown
host: x86_64-unknown-linux-gnu
release: 1.9.0-dev

$ cat - >hello.rs
fn main() {
println!("Hello, rusty Android!");
}
^D

$ rustc --target=arm-linux-androideabi -C linker=$HOME/Work/Android/NdkStandalone/bin/arm-linux-androideabi-gcc -C link-args="-pie -fPIE" hello.rs
error: linking with /home/skirmantas/Work/Android/NdkStandalone/bin/arm-linux-androideabi-gcc failed: exit code: 1
note: "/home/skirmantas/Work/Android/NdkStandalone/bin/arm-linux-androideabi-gcc" "-Wl,--as-needed" "-Wl,-z,noexecstack" "-Wl,--allow-multiple-definition" "-L" "/usr/local/stow/rustc-20160317/lib/rustlib/arm-linux-androideabi/lib" "hello.0.o" "-o" "hello" "-Wl,--gc-sections" "-pie" "-nodefaultlibs" "-L" "/usr/local/stow/rustc-20160317/lib/rustlib/arm-linux-androideabi/lib" "-Wl,-Bstatic" "-Wl,-Bdynamic" "/usr/local/stow/rustc-20160317/lib/rustlib/arm-linux-androideabi/lib/libstd-9026086f.rlib" "/usr/local/stow/rustc-20160317/lib/rustlib/arm-linux-androideabi/lib/libcollections-9026086f.rlib" "/usr/local/stow/rustc-20160317/lib/rustlib/arm-linux-androideabi/lib/librustc_unicode-9026086f.rlib" "/usr/local/stow/rustc-20160317/lib/rustlib/arm-linux-androideabi/lib/librand-9026086f.rlib" "/usr/local/stow/rustc-20160317/lib/rustlib/arm-linux-androideabi/lib/liballoc-9026086f.rlib" "/usr/local/stow/rustc-20160317/lib/rustlib/arm-linux-androideabi/lib/liballoc_jemalloc-9026086f.rlib" "/usr/local/stow/rustc-20160317/lib/rustlib/arm-linux-androideabi/lib/liblibc-9026086f.rlib" "/usr/local/stow/rustc-20160317/lib/rustlib/arm-linux-androideabi/lib/libcore-9026086f.rlib" "-l" "dl" "-l" "log" "-l" "gcc" "-l" "gcc" "-l" "c" "-l" "m" "-pie" "-fPIE" "-l" "compiler-rt"
note: /usr/local/stow/rustc-20160317/lib/rustlib/arm-linux-androideabi/lib/libstd-9026086f.rlib(std-9026086f.0.o):std.0.rs:function rt::lang_start::h1deaaba938bd63a9Xqz: error: undefined reference to 'bsd_signal'
collect2: error: ld returned 1 exit status

error: aborting due to previous error

Nercury added a commit to Nercury/libc that referenced this issue Mar 19, 2016
@alexcrichton
Copy link
Member

Hm this may be a backwards incompatible change made in the NDK? I believe super old versions had the function called bsd_signal and then it changed recently to signal and now the bsd_signal function has been removed?

@skligys
Copy link
Author

skligys commented Mar 19, 2016

Thanks for taking a look.

Indeed, the arch-arm/usr/include/signal.h header up to "android-19" contained:

/* differentiater between sysv and bsd behaviour 8*/
extern __sighandler_t sysv_signal(int, __sighandler_t);
extern __sighandler_t bsd_signal(int, __sighandler_t);

/* the default is bsd */
static __inline__ __sighandler_t signal(int s, __sighandler_t f)
{
    return bsd_signal(s,f);
}

but starting at "android-21" (there is no android-20) has just:

extern sighandler_t signal(int, sighandler_t);

So it looks that stdlib would have to use different symbols depending on which version of Android SDK it was targeting.

@Nercury
Copy link
Contributor

Nercury commented Mar 19, 2016

How are we going to solve BC compatibility? Can we link to different names depending on android ndk version?

I have tried rebuilding rust std library with this change in libc and I can link my little android lib successfully.

@alexcrichton
Copy link
Member

Yeah this is an interesting question for us. It seems like right now we can't span multiple android versions. Currently the oldest version supported is API level 18, but we could mitigate this in a few ways:

  1. Bump the minimum API level. Note that we have requests to move this back as well.
  2. Avoid usage of signal in the standard library (may produce inconsistent results).
  3. Look up the signal symbol at runtime rather than link to it.

I'd probably lean towards option 3 for now and modifying the standard library as it seems the most flexible?

@alexcrichton
Copy link
Member

Oh another options is for consumers of libc to have a choice, we could do NDK version detection like @Nercury mentioned via something like:

  • A build script could detect the version of the Android compiler and tweak the API appropriately
  • Features could be used on libc to indicate what level of android compatibility is desired
  • We could bake the API level of the Android NDK into the target triple, for example arm-linux-androideabi18 or something like that.

I'd personally rather avoid these options for now, but may as well consider everything!

Also cc @rust-lang/tools, interesting API compatibility story here.

@Nercury
Copy link
Contributor

Nercury commented Mar 21, 2016

Some thoughts:

The least painful option for users would be to do the symbol lookup at runtime, however I fear that moving such solutions to runtime would require us to debug any issues at runtime too.

Baking android tools version to arm-linux-androideabi18 might be an issue, because of the number of triplets that are going to appear (if we wish to add more build targets for android, like arm7-hf, arm64, x64, etc).

If we use features like android-18, android-19, we may keep the latest one as the "default". The only issue I see here that other platforms do not need to care about android platform and these flags may get a bit messy.

If we don't want to have a separate std lib for different android versions, it looks like the smoothest solution is indeed to do the symbol lookup at runtime. And do that only on Android platform.

@alexcrichton
Copy link
Member

cc rust-lang/rust#32415, a patch for the standard library

alexcrichton added a commit to alexcrichton/rust that referenced this issue Apr 5, 2016
Currently the minimum supported Android version of the standard library is
API level 18 (android-18). Back in those days [1] the `signal` function was
just an inline wrapper around `bsd_signal`, but starting in API level
android-20 the `signal` symbols was introduced [2]. Finally, in android-21
the API `bsd_signal` was removed [3].

Basically this means that if we want to be binary compatible with multiple
Android releases (oldest being 18 and newest being 21) then we need to check
for both symbols and not actually link against either.

This was first discovered in rust-lang/libc#236 with a fix proposed in
rust-lang/libc#237. I suspect that we'll want to accept rust-lang/libc#237 so
Rust crates at large continue to be compatible with newer releases of Android
and crates, like the standard library, that want to opt into older support can
continue to do so via similar means.

Closes rust-lang/libc#236

[1]: https://chromium.googlesource.com/android_tools/+/20ee6d20/ndk/platforms/android-18/arch-arm/usr/include/signal.h
[2]: https://chromium.googlesource.com/android_tools/+/fbd420/ndk_experimental/platforms/android-20/arch-arm/usr/include/signal.h
[3]: https://chromium.googlesource.com/android_tools/+/20ee6d/ndk/platforms/android-21/arch-arm/usr/include/signal.h
bors added a commit to rust-lang/rust that referenced this issue Apr 5, 2016
std: Fix linking against `signal` on Android

Currently the minimum supported Android version of the standard library is
API level 18 (android-18). Back in those days [1] the `signal` function was
just an inline wrapper around `bsd_signal`, but starting in API level
android-20 the `signal` symbols was introduced [2]. Finally, in android-21
the API `bsd_signal` was removed [3].

Basically this means that if we want to be binary compatible with multiple
Android releases (oldest being 18 and newest being 21) then we need to check
for both symbols and not actually link against either.

This was first discovered in rust-lang/libc#236 with a fix proposed in
rust-lang/libc#237. I suspect that we'll want to accept rust-lang/libc#237 so
Rust crates at large continue to be compatible with newer releases of Android
and crates, like the standard library, that want to opt into older support can
continue to do so via similar means.

Closes rust-lang/libc#236

[1]: https://chromium.googlesource.com/android_tools/+/20ee6d20/ndk/platforms/android-18/arch-arm/usr/include/signal.h
[2]: https://chromium.googlesource.com/android_tools/+/fbd420/ndk_experimental/platforms/android-20/arch-arm/usr/include/signal.h
[3]: https://chromium.googlesource.com/android_tools/+/20ee6d/ndk/platforms/android-21/arch-arm/usr/include/signal.h
bors added a commit that referenced this issue Apr 6, 2016
…crichton

Use now available link name "signal" instead of "bsd_signal"

On android, the `bsd_signal` is gone, the `signal` is available.

While this is the most obvious solution, I am not sure of a few things:
- How are we going to keep compatibility with older NDKs where `signal` does not exist;
- Was something dependent on this being different on android and thus would break (for example, the rust compiler uses this function, so it may break somewhere).

Fixes #236.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants