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

no_mangle causes compilation errors with async-await on armv7-linux-androideabi and aarch64-linux-android targets #70098

Open
jeswin opened this issue Mar 18, 2020 · 17 comments
Labels
A-async-await A-LLVM AsyncAwait-Triaged C-bug E-needs-bisection E-needs-mcve I-crash ICEBreaker-Cleanup-Crew ICEBreaker-LLVM O-android P-medium regression-from-stable-to-stable T-compiler

Comments

@jeswin
Copy link

@jeswin jeswin commented Mar 18, 2020

Adding the no_mangle attribute to a function which contains an await results in a compilation error for armv7-linux-androideabi and aarch64-linux-android targets. Removing the attribute fixes the issue (though that's not a solution). There are no problems with x86_64-linux-android and i686-linux-android targets.

#![cfg(target_os = "android")]
#![allow(non_snake_case)]

use tokio::{runtime::Runtime };

#[no_mangle]
pub fn Java_blah_blah_func() -> &'static str {
    Runtime::new().unwrap().block_on(async {
        myfunc().await
    })
}

async fn myfunc() -> &'static str {
    "hello"
}

I expected to see this happen: library should get compiled successfully.

Instead, I get the following error:

Function return type does not match operand type of return inst!
  ret i32 undef
 { i8*, i32 }in function _ZN80_$LT$std..future..GenFuture$LT$T$GT$$u20$as$u20$core..future..future..Future$GT$4poll17h2e6af84f5e832d75E
LLVM ERROR: Broken function found, compilation aborted!

Meta

The error exists in the beta and nightly versions. RUST_BACKTRACE=1 doesn't seem to add additional information.

@jeswin jeswin added the C-bug label Mar 18, 2020
@jonas-schievink jonas-schievink added I-crash O-android T-compiler E-needs-bisection E-needs-mcve I-nominated regression-from-stable-to-beta labels Mar 18, 2020
@jeswin
Copy link
Author

@jeswin jeswin commented Mar 18, 2020

Hi @jonas-schievink, Not sure if this is minimal enough, but I've created a repo to demonstrate the issue at https://github.com/jeswin/rust-lang-issue-70098-mcve

Please note that build.sh requires cargo-ndk to build.

@Centril
Copy link
Contributor

@Centril Centril commented Mar 18, 2020

@rustbot ping llvm

@rustbot
Copy link
Collaborator

@rustbot rustbot commented Mar 18, 2020

Hey LLVM ICE-breakers! This bug has been identified as a good
"LLVM ICE-breaking candidate". In case it's useful, here are some
instructions for tackling these sorts of bugs. Maybe take a look?
Thanks! <3

cc @comex @cuviper @DutchGhost @hanna-kruppe @hdhoang @heyrutvik @JOE1994 @jryans @mmilenko @nagisa @nikic @Noah-Kennedy @SiavoshZarrasvand @spastorino @vertexclique @vgxbj

@rustbot rustbot added the ICEBreaker-LLVM label Mar 18, 2020
@Centril Centril added the A-LLVM label Mar 18, 2020
@Centril
Copy link
Contributor

@Centril Centril commented Mar 18, 2020

@rustbot ping cleanup

@rustbot rustbot added the ICEBreaker-Cleanup-Crew label Mar 18, 2020
@rustbot
Copy link
Collaborator

@rustbot rustbot commented Mar 18, 2020

Hey Cleanup Crew ICE-breakers! This bug has been identified as a good
"Cleanup ICE-breaking candidate". In case it's useful, here are some
instructions for tackling these sorts of bugs. Maybe take a look?
Thanks! <3

cc @AminArria @chrissimpkins @contrun @DutchGhost @elshize @ethanboxx @h-michael @HallerPatrick @hdhoang @hellow554 @imtsuki @jakevossen5 @kanru @KarlK90 @LeSeulArtichaut @MAdrianMattocks @matheus-consoli @mental32 @nmccarty @Noah-Kennedy @pard68 @PeytonT @pierreN @Redblueflame @RobbieClarken @RobertoSnap @robjtede @SarthakSingh31 @senden9 @shekohex @sinato @spastorino @turboladen @woshilapin @yerke

@spastorino spastorino added E-medium P-medium and removed I-nominated E-medium labels Mar 18, 2020
@jeswin
Copy link
Author

@jeswin jeswin commented Mar 20, 2020

If it's of any use, I tested it with rustc 1.39 and got the same error. I'll be happy to help in any way - should you require.

@woshilapin
Copy link
Contributor

@woshilapin woshilapin commented Mar 21, 2020

Hi @jonas-schievink, Not sure if this is minimal enough, but I've created a repo to demonstrate the issue at jeswin/rust-lang-issue-70098-mcve

Please note that build.sh requires cargo-ndk to build.

Using the repository @jonas-schievink provided, I couldn't reproduce the bug. I'm building on a MacOS Catalina 10.15.3 and I tried Rust stable (rustc 1.42.0 (b8cedc004 2020-03-09)) and Rust beta (rustc 1.43.0-beta.1 (4a71daf1c 2020-03-11)).

@jeswin
Copy link
Author

@jeswin jeswin commented Mar 22, 2020

Hi @woshilapin, sorry I forgot to mention - my build environment was Linux 64-bit; Ubuntu 19.10 specifically.

I can try to reproduce the bug on a fresh Linux install.

@jeswin
Copy link
Author

@jeswin jeswin commented Mar 23, 2020

@woshilapin Could you also post the llvm and ndk versions you were able to use successfully?

@woshilapin
Copy link
Contributor

@woshilapin woshilapin commented Mar 23, 2020

@woshilapin Could you also post the llvm and ndk versions you were able to use successfully?

Not sure of what I'll provide but if I remember correctly, version of LLVM would be the same as version of clang, right?

$ clang --version
Apple clang version 11.0.0 (clang-1100.0.33.16)
Target: x86_64-apple-darwin19.3.0
Thread model: posix

For NDK, I downloaded it testing the current issue so it's pretty recent. I have a source.properties file in the NDK folder that seems to give this information.

$ cat source.properties
Pkg.Desc = Android NDK
Pkg.Revision = 21.0.6113669

Please tell me if some other information is needed (and how to get them maybe since I'm not an Android developer so I have no background!).

@jeswin
Copy link
Author

@jeswin jeswin commented Mar 24, 2020

@woshilapin I was able to reproduce the issue on a fresh install of Ubuntu 19.10 64-bit. My ndk version is the same as yours - 21.0.6113669.

I think ndk uses (at least on Linux) clang and llvm bundled and prebuilt with the ndk. For me, it gets installed at ~/Android/Sdk/ndk/21.0.6113669/toolchains/llvm/prebuilt/linux-x86_64/bin.

The clang --version (from the directory above) is

Android (5900059 based on r365631c) clang version 9.0.8 (https://android.googlesource.com/toolchain/llvm-project 207d7abc1a2abf3ef8d4301736d6a7ebc224a290) (based on LLVM 9.0.8svn)
Target: x86_64-unknown-linux-gnu
Thread model: posix

So there seems to be a problem with compilation on Linux. I'll proceed with testing it on my ancient Macbook.

@jeswin
Copy link
Author

@jeswin jeswin commented Mar 25, 2020

@woshilapin I was able to compile it successfully on OSX. So the problem occurs only on Linux.

@lights0123
Copy link

@lights0123 lights0123 commented Mar 28, 2020

I'm experiencing this same issue with a custom, no_std target:

{
    "arch": "arm",
    "cpu": "arm926ej-s",
    "data-layout": "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64",
    "llvm-target": "armv5te-none-eabi",
    "os": "newlib",
    "relocation-model": "pic",
    "target-endian": "little",
    "target-env": "newlib",
    "target-pointer-width": "32",
    "target-c-int-width": "32",
    "linker": "nspire-gcc",
    "linker-flavor": "gcc",
    "dynamic-linking": false,
    "executables": true,
    "features": "+v5te,+soft-float,+strict-align",
    "singlethread": true
}

Running Arch Linux 5.5.10-arch1-1 and rustc 1.44.0-nightly (7520894 2020-03-27).

Here's a sample source:

#[no_mangle]
fn main() {
	doesnt_work();
	does_work();
}

fn doesnt_work() {
	Executor::new().run(Task::new(do_stuff()));
}

fn does_work() {
	Executor::new().run(Task::new(do_more_stuff()));
}

async fn do_stuff() -> &'static str {
	do_more_stuff().await
}

async fn do_more_stuff() -> &'static str {
	"test"
}

(I'll post the code of the custom types if needed, mostly stolen from this recent blog post) Interestingly enough, the function does_work does work and doesn't produce a compiler error when doesnt_work is commented out. I get the following error with doesnt_work:

Function return type does not match operand type of return inst!
  ret i32 undef
 { i8*, i32 }in function _ZN97_$LT$core..future..from_generator..GenFuture$LT$T$GT$$u20$as$u20$core..future..future..Future$GT$4poll17h8f0e6fe9387a4fc1E
LLVM ERROR: Broken function found, compilation aborted!

Edit: this error does not occur with this test case when changing to an i32:

async fn do_more_stuff() -> i32 {
	5
}

@pnkfelix pnkfelix added the A-async-await label Apr 2, 2020
@nikomatsakis
Copy link
Contributor

@nikomatsakis nikomatsakis commented Apr 2, 2020

@lights0123 the problem in that case might be applying #[no_mangle] to main. Can you try this example?

fn main() {
    main1();
}

#[no_mangle]
fn main1() {
	doesnt_work();
	does_work();
}

fn doesnt_work() {
	Executor::new().run(Task::new(do_stuff()));
}

fn does_work() {
	Executor::new().run(Task::new(do_more_stuff()));
}

async fn do_stuff() -> &'static str {
	do_more_stuff().await
}

async fn do_more_stuff() -> &'static str {
	"test"
}

@lights0123
Copy link

@lights0123 lights0123 commented Apr 2, 2020

@nikomatsakis Same error:

Function return type does not match operand type of return inst!
  ret i32 undef
 { i8*, i32 }in function _ZN97_$LT$core..future..from_generator..GenFuture$LT$T$GT$$u20$as$u20$core..future..future..Future$GT$4poll17h26160800281fdd7cE
LLVM ERROR: Broken function found, compilation aborted!

Just as a note, I won't be able to test running the code like that as I need to export the symbol main, as that's what my target is expecting to execute. I also recently changed the API of my executor to execute a Future directly instead of one wrapped in a Task. I'm going to create a repo that includes both my new and old executor so you can figure out problems.

@lights0123
Copy link

@lights0123 lights0123 commented Apr 2, 2020

@nikomatsakis @jeswin Minimal example with only 1 dependency: https://github.com/lights0123/ice-70098

This doesn't require installing any toolchains other than cargo-xbuild, which is just one cargo install away, but it does require nightly for no_std stuff.

What I learned:

  • Doesn't matter what the name of the function is, as long as one is #[no_mangle] or using #[export_name = "whatever"]
  • Occurs when storing a Pin<Box<dyn Future<Output = T>>> in a struct, and then panicing. Before, I was able to get it to return the same error even when it didn't unconditionally panic, but I was unable to replicate it with this.
  • Occurs when returning a &'static str, but not when returning a &'static i32 referencing a static i32
  • Occurs with an async function calling another async function
  • Only occurs in release builds

@nikomatsakis
Copy link
Contributor

@nikomatsakis nikomatsakis commented Apr 3, 2020

Huh! Fascinating. Thanks @lights0123, that's good work.

@tmandry tmandry added the AsyncAwait-Triaged label Apr 7, 2020
@pietroalbini pietroalbini added regression-from-stable-to-stable and removed regression-from-stable-to-beta labels Aug 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-async-await A-LLVM AsyncAwait-Triaged C-bug E-needs-bisection E-needs-mcve I-crash ICEBreaker-Cleanup-Crew ICEBreaker-LLVM O-android P-medium regression-from-stable-to-stable T-compiler
Projects
None yet
Development

No branches or pull requests