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

rustc: Handle #[no_mangle] anywhere in a crate #45189

Merged
merged 1 commit into from
Oct 13, 2017

Conversation

alexcrichton
Copy link
Member

This commit updates the reachability pass of the compiler to seed the local
worklist with #[no_mangle]-like items anywhere in a crate, not just those
reachable from public items.

Closes #45165

@alexcrichton
Copy link
Member Author

r? @michaelwoerister

@rust-highfive
Copy link
Collaborator

r? @pnkfelix

(rust_highfive has picked a reviewer for you, use r? to override)

@michaelwoerister
Copy link
Member

Looks good to me. Some tests are failing though.

@alexcrichton
Copy link
Member Author

Ok I changed this to just handling #[linkage] instead of #[no_mangle] which I believe should fix the tests.

@bors: r=michaelwoerister

@bors
Copy link
Contributor

bors commented Oct 11, 2017

📌 Commit c7aa741 has been approved by michaelwoerister

kennytm added a commit to kennytm/rust that referenced this pull request Oct 12, 2017
…michaelwoerister

rustc: Handle `#[no_mangle]` anywhere in a crate

This commit updates the reachability pass of the compiler to seed the local
worklist with `#[no_mangle]`-like items anywhere in a crate, not just those
reachable from public items.

Closes rust-lang#45165
@kennytm
Copy link
Member

kennytm commented Oct 12, 2017

@bors r-

asm.js's run-pass/smallest-hello-world.rs test failed to link due to "undefined symbol: rust_begin_unwind" in rollup #45207 (Travis log: https://travis-ci.org/rust-lang/rust/jobs/286875799).

The test involves #[global_allocator] and only this PR touches the linkage step, so I guess this should be the cause. The rollup uses c7aa741 so it should not be due to the #[no_mangle].

[01:37:33] ---- [run-pass] run-pass/smallest-hello-world.rs stdout ----
[01:37:33] 	
[01:37:33] error: compilation failed!
[01:37:33] status: exit code: 101
[01:37:33] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/run-pass/smallest-hello-world.rs" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass" "--target=asmjs-unknown-emscripten" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/smallest-hello-world.stage2-asmjs-unknown-emscripten.js" "-Crpath" "-O" "-Lnative=/checkout/obj/build/asmjs-unknown-emscripten/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/smallest-hello-world.stage2-asmjs-unknown-emscripten.run-pass.libaux"
[01:37:33] stdout:
[01:37:33] ------------------------------------------
[01:37:33] 
[01:37:33] ------------------------------------------
[01:37:33] stderr:
[01:37:33] ------------------------------------------
[01:37:33] error: linking with `emcc` failed: exit code: 1
[01:37:33]   |
[01:37:33]   = note: "emcc" "-s" "DISABLE_EXCEPTION_CATCHING=0" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/asmjs-unknown-emscripten/lib" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/smallest-hello-world.smallest_hello_world0.rust-cgu.o" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/smallest-hello-world.stage2-asmjs-unknown-emscripten.js" "-s" "EXPORTED_FUNCTIONS=[\"___rg_shrink_in_place\",\"___rg_alloc\",\"___rg_oom\",\"___rg_realloc\",\"___rg_alloc_zeroed\",\"_rust_eh_personality\",\"___rg_usable_size\",\"___rg_alloc_excess\",\"___rg_grow_in_place\",\"___rg_dealloc\",\"___rg_realloc_excess\",\"_main\"]" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/smallest-hello-world.crate.allocator.rust-cgu.o" "-O2" "--memory-init-file" "0" "-g0" "-s" "DEFAULT_LIBRARY_FUNCS_TO_INCLUDE=[]" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass" "-L" "/checkout/obj/build/asmjs-unknown-emscripten/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/smallest-hello-world.stage2-asmjs-unknown-emscripten.run-pass.libaux" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/asmjs-unknown-emscripten/lib" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/asmjs-unknown-emscripten/lib/liballoc_system-67992b04c9027e70.rlib" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/asmjs-unknown-emscripten/lib/liballoc-c15d9e20191e711b.rlib" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/asmjs-unknown-emscripten/lib/libstd_unicode-b3230a4723442795.rlib" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/asmjs-unknown-emscripten/lib/liblibc-bdd38e4620ffab1c.rlib" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/asmjs-unknown-emscripten/lib/libcore-3181dd9e46400ebd.rlib" "-l" "c" "-s" "ERROR_ON_UNDEFINED_SYMBOLS=1"
[01:37:33]   = note: error: unresolved symbol: rust_begin_unwind
[01:37:33]           Aborting compilation due to previous errors | undefined
[01:37:33]           Traceback (most recent call last):
[01:37:33]             File "/emsdk-portable/emscripten/1.37.13//emcc", line 13, in <module>
[01:37:33]               emcc.run()
[01:37:33]             File "/emsdk-portable/emscripten/1.37.13/emcc.py", line 1526, in run
[01:37:33]               final = shared.Building.emscripten(final, append_ext=False, extra_args=extra_args)
[01:37:33]             File "/emsdk-portable/emscripten/1.37.13/tools/shared.py", line 1963, in emscripten
[01:37:33]               call_emscripten(cmdline)
[01:37:33]             File "/emsdk-portable/emscripten/1.37.13/emscripten.py", line 2190, in _main
[01:37:33]               temp_files.run_and_clean(lambda: main(
[01:37:33]             File "/emsdk-portable/emscripten/1.37.13/tools/tempfiles.py", line 78, in run_and_clean
[01:37:33]               return func()
[01:37:33]             File "/emsdk-portable/emscripten/1.37.13/emscripten.py", line 2195, in <lambda>
[01:37:33]               DEBUG=DEBUG,
[01:37:33]             File "/emsdk-portable/emscripten/1.37.13/emscripten.py", line 2095, in main
[01:37:33]               temp_files=temp_files, DEBUG=DEBUG)
[01:37:33]             File "/emsdk-portable/emscripten/1.37.13/emscripten.py", line 98, in emscript
[01:37:33]               glue, forwarded_data = compiler_glue(metadata, settings, libraries, compiler_engine, temp_files, DEBUG)
[01:37:33]             File "/emsdk-portable/emscripten/1.37.13/emscripten.py", line 219, in compiler_glue
[01:37:33]               glue, forwarded_data = compile_settings(compiler_engine, settings, libraries, temp_files)
[01:37:33]             File "/emsdk-portable/emscripten/1.37.13/emscripten.py", line 531, in compile_settings
[01:37:33]               cwd=path_from_root('src'), error_limit=300)
[01:37:33]             File "/emsdk-portable/emscripten/1.37.13/tools/jsrun.py", line 128, in run_js
[01:37:33]               raise Exception('Expected the command ' + str(command) + ' to finish with return code ' + str(assert_returncode) + ', but it returned with code ' + str(proc.returncode) + ' instead! Output: ' + str(ret)[:error_limit])
[01:37:33]           Exception: Expected the command ['/emsdk-portable/node/4.1.1_64bit/bin/node', '/emsdk-portable/emscripten/1.37.13/src/compiler.js', '/tmp/tmp1WwN3A.txt', '/emsdk-portable/emscripten/1.37.13/src/library_pthread_stub.js'] to finish with return code 0, but it returned with code 1 instead! Output: // The Module object: Our interface to the outside world. We import
[01:37:33]           // and export values on it, and do the work to get that through
[01:37:33]           // closure compiler if necessary. There are various ways Module can be used:
[01:37:33]           // 1. Not defined. We create it here
[01:37:33]           // 2. A function parameter, function(Module) { ..gener
[01:37:33]           
[01:37:33] 
[01:37:33] error: aborting due to previous error
[01:37:33] 
[01:37:33] 
[01:37:33] ------------------------------------------
[01:37:33] 
[01:37:33] thread '[run-pass] run-pass/smallest-hello-world.rs' panicked at 'explicit panic', /checkout/src/tools/compiletest/src/runtest.rs:2425:8
[01:37:33] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[01:37:33] 
[01:37:33] 
[01:37:33] failures:
[01:37:33]     [run-pass] run-pass/smallest-hello-world.rs
[01:37:33] 
[01:37:33] test result: FAILED. 2643 passed; 1 failed; 145 ignored; 0 measured; 0 filtered out

@michaelwoerister
Copy link
Member

@alexcrichton So what exactly is going on here? I.e. what is the root cause of the problem this PR tries to fix?

@aidanhs aidanhs added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Oct 12, 2017
@alexcrichton
Copy link
Member Author

@michaelwoerister the problem is that ThinLTO is internalizing these symbols when they otherwise need to be exposed. That in turns happens because locally defined allocator symbols aren't in the list of "rust exported symbols" (the exported_symbols) query. That happens because they're not in the reachable set. That then finally happens because reachability doesn't take #[linkage] (an unstable attribute) into account.

@michaelwoerister
Copy link
Member

@alexcrichton But why is that a problem with ThinLTO but not the regular multi-CGU case?

@alexcrichton
Copy link
Member Author

I think that happens is that in normal trans we will translate everything to LLVM and at that time we'll recognize #[linkage] and apply it. With ThinLTO enabled, however, the items in each CGU will get its linkage/visibility frobbed by the various passes. We instruct ThinLTO to only retain a particular set of symbols, and the allocator functions aren't in this set (hence this bug)

Does that make sense?

This commit updates the reachability pass of the compiler to seed the local
worklist with `#[linkage]`-like items anywhere in a crate, not just those
reachable from public items.

Closes rust-lang#45165
@alexcrichton
Copy link
Member Author

Also I'm removing that test once and for all, it has caused dozens and dozens of failures and has literally never caught a regression.

@bors: r=michaelwoerister

@bors
Copy link
Contributor

bors commented Oct 12, 2017

📌 Commit 6cae080 has been approved by michaelwoerister

@michaelwoerister
Copy link
Member

Also I'm removing that test once and for all, it has caused dozens and dozens of failures and has literally never caught a regression.

:D

@michaelwoerister
Copy link
Member

Thanks for the clarification.

kennytm added a commit to kennytm/rust that referenced this pull request Oct 13, 2017
…michaelwoerister

rustc: Handle `#[no_mangle]` anywhere in a crate

This commit updates the reachability pass of the compiler to seed the local
worklist with `#[no_mangle]`-like items anywhere in a crate, not just those
reachable from public items.

Closes rust-lang#45165
bors added a commit that referenced this pull request Oct 13, 2017
Rollup of 14 pull requests

- Successful merges: #44855, #45110, #45122, #45133, #45173, #45178, #45189, #45203, #45209, #45221, #45236, #45240, #45245, #45253
- Failed merges:
@bors bors merged commit 6cae080 into rust-lang:master Oct 13, 2017
@alexcrichton alexcrichton deleted the thinlto-allocators branch October 14, 2017 02:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants