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

Never returns (only on my library) #15

Closed
rukai opened this issue Apr 2, 2018 · 15 comments
Closed

Never returns (only on my library) #15

rukai opened this issue Apr 2, 2018 · 15 comments
Labels
bug Something isn't working

Comments

@rukai
Copy link

rukai commented Apr 2, 2018

Tested on the latest nightly with CARGO_INCREMENTAL=1
The previous nightly I was using was broken for another reason.

The examples work fine:

git clone git@github.com:gnzlbg/cargo-asm.git
cd cargo-asm/cargo-asm-test/lib_crate
cargo asm lib_crate::baz::Foo::foo_add
cargo asm lib_crate::bar::generic_add2

However I tried it on the library I am working on and it never returns and doesnt output anything.
Its at 0% CPU usage, so not sure what its actually doing...
The crate has nll enabled.

git clone https://github.com/rukai/brawllib_rs.git
cd brawllib_rs
cargo asm brawllib_rs::chr0::Keyframe::get_value_interpolated_n_entry
@gnzlbg
Copy link
Owner

gnzlbg commented Apr 3, 2018

@rukai thank you for the report, I'll look into it :)

@gnzlbg
Copy link
Owner

gnzlbg commented Apr 3, 2018

@rukai I can't reproduce. Could you provide me with more information? (operating system, target, environment variables, do you have a cargo config file?, etc.).

I just tested this on MacOS X:

git clone https://github.com/rukai/brawllib_rs.git
cd brawllib_rs
cargo asm brawllib_rs::chr0::Keyframe::get_value_interpolated_n_entry

and it consistently outputs:

[ERROR]: could not find function at path "brawllib_rs::chr0::Keyframe::get_value_interpolated_n_entry" in the generated assembly.

Tips:
  * make sure that the function is present in the final binary (e.g. if it's a generic function, make sure that it is actually monomorphized)
  * try to do a --clean build (sometimes changes are not picked up)

Then I tried with

CARGO_INCREMENTAL=1 cargo asm brawllib_rs::chr0::Keyframe::get_value_interpolated_n_entry

and I get:

brawllib_rs::chr0::Keyframe::get_value_interpolated_n_entry (src/chr0.rs:319):
 push    rbp
 mov     rbp, rsp
 push    r15
 push    r14
 push    r13
 push    r12
 push    rbx
 sub     rsp, 120
 mov     ebx, ecx
 call    <I as core::iter::traits::IntoIterator>::into_iter
...

which hints that without incremental compilation the function is not present in the final binary (maybe it's getting inlined or something). But even if it is not present in the binary, you should just get an error message instead of cargo asm hanging forever :/

@rukai
Copy link
Author

rukai commented Apr 3, 2018

OS: Arch linux

Only rust environment variables I have are:

export RUST_BACKTRACE=1
export CARGO_INCREMENTAL=1

.cargo/config only contains my crates.io key

I have also tried without nll and without incremental compilation and they dont seem to affect it.
So the only variable here seems to be linux

I was also able to reduce the problem down from my entire library to just any function that uses a macro.
e.g. all of the following prevent cargo asm from returning.

pub fn foo_print() {
    println!("hello world");
}

or

pub fn foo_format() {
    format!("hello world");
}

or

pub fn foo_vec() {
    let foo: Vec<i32> = vec!();
}

@gnzlbg
Copy link
Owner

gnzlbg commented Apr 3, 2018

Could you maybe run cargo asm with --debug-mode and dump the output in a gist (until it hangs?)?

@rukai
Copy link
Author

rukai commented Apr 3, 2018

out.txt

@gnzlbg
Copy link
Owner

gnzlbg commented Apr 3, 2018

Thanks, that actually looks fine :/

I'll try to get hold of a Linux machine and see if I can reproduce it there.

@rukai
Copy link
Author

rukai commented Apr 3, 2018

So I did the rustup component add rust-src that the last line of the log recommends. And now it works!
Maybe on your machine your not hitting the code path without rust-src?

@gnzlbg
Copy link
Owner

gnzlbg commented Apr 3, 2018

And now it works!

Good to hear that!

Maybe on your machine your not hitting the code path without rust-src?

This is possible. I just set up a Linux VM so that I can try it there. Will try it without installing rust-src first.

If rust-src is not installed, what should happen is that you get a warning if you are passing the --rust flag, but otherwise it should just work. I am tagging this as a bug, since on CI cargo-asm is only tested with rust-src installed.

@gnzlbg gnzlbg added the bug Something isn't working label Apr 3, 2018
@gnzlbg
Copy link
Owner

gnzlbg commented Apr 3, 2018

So this is very weird.

On Linux, if I do a cargo asm brawllib_rs::chr0::Keyframe::get_value_interpolated_n_entry without rust-src I get the proper error message. But if I do CARGO_INCREMENTAL=1 cargo asm brawllib_rs::chr0::Keyframe::get_value_interpolated_n_entry then it takes forever / never finishes.

Could you try if not using CARGO_INCREMENTAL=1 fixes the problem for you as well? If so, then CARGO_INCREMENTAL would be the culprit, and maybe cargo-asm is doing something that interacts badly with CARGO_INCREMENTAL.

@rukai
Copy link
Author

rukai commented Apr 3, 2018

CARGO_INCREMENTAL=0 does not resolve the problem for me.

I believe I've found the issue although it doesnt explain why you arent running into the issue on mac os.
Perhaps rust on mac os or mac os itself handles locks differently o.0
It looks like the opts RwLock has a read and write lock at the same time.

the stacktrace looks like:

  1. main (takes read lock on opts)
  2. mod::run
  3. rust::parse
  4. OptionsExt::set_rust (takes write lock on opts)

Havnt been able to test it though, as I've got to run.

@gnzlbg
Copy link
Owner

gnzlbg commented Apr 4, 2018

Nice find. So I've switched to parking_lot::RwLock that has an optional dead-lock detection feature. I've enabled it by default, and pushed a new commit to master that has that enabled.

Could you try using the master branch with cargo-asm ? If there is a deadlock you should get an error message saying which threads holds a lock where .

@rukai
Copy link
Author

rukai commented Apr 4, 2018

Yep, running: cargo asm brawllib_rs::chr0::Keyframe::get_value_interpolated_n_entry

gives:

1 deadlocks detected
Deadlock #0
Thread Id 140625588098240
stack backtrace:
   0:     0x55d9da20e58c - backtrace::backtrace::trace::h92323bce52b8fd95
   1:     0x55d9da20cd72 - backtrace::capture::Backtrace::new::hebfa6cbf290849c0
   2:     0x55d9da1f561a - parking_lot_core::parking_lot::deadlock_impl::on_unpark::h9a6543bebefcd8a3
   3:     0x55d9da1fc591 - parking_lot_core::parking_lot::park_internal::hd9d9d692e4a2565e
   4:     0x55d9da1f4654 - parking_lot::raw_rwlock::RawRwLock::lock_exclusive_slow::h9158aa8a976380e3
   5:     0x55d9da1ca50b - cargo_asm::rust::parse::hc801734a99554376
   6:     0x55d9da1bf580 - cargo_asm::asm::run::hf0d903153af1bcfd
   7:     0x55d9da1d9d33 - cargo_asm::main::he648138659a422fe
   8:     0x55d9da1ce252 - std::rt::lang_start::{{closure}}::hdeeec6e3fcb0bb07
   9:     0x55d9da296307 - std::rt::lang_start_internal::{{closure}}::hf1bc4b51ec610edd
                        at libstd/rt.rs:59
                         - std::panicking::try::do_call::hedf7b06e6b8c6819
                        at libstd/panicking.rs:306
  10:     0x55d9da2a9c1e - __rust_maybe_catch_panic
                        at libpanic_unwind/lib.rs:102
  11:     0x55d9da2935d5 - std::panicking::try::h4839266d9e75b2e7
                        at libstd/panicking.rs:285
                         - std::panic::catch_unwind::h87626006d5536401
                        at libstd/panic.rs:361
                         - std::rt::lang_start_internal::hd045a9bbfffc82a4
                        at libstd/rt.rs:58
  12:     0x55d9da1dab33 - main
  13:     0x7fe5f15e5f49 - __libc_start_main
  14:     0x55d9da1b8579 - _start
  15:                0x0 - <unknown>
Aborted (core dumped)

@gnzlbg
Copy link
Owner

gnzlbg commented Apr 4, 2018

wow parking_lot is great! I'll let you know when I push a fix.

@gnzlbg
Copy link
Owner

gnzlbg commented Apr 10, 2018

@rukai I've fixed this on master, would be cool if you could check that it does indeed work :)

@gnzlbg gnzlbg closed this as completed Apr 10, 2018
@gnzlbg
Copy link
Owner

gnzlbg commented Apr 10, 2018

I've released 0.1.13 with the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants