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

Unoptimised builds of compiler_builtins on ARM have references to core::panicking::panic #40508

Closed
TimNN opened this issue Mar 14, 2017 · 6 comments

Comments

@TimNN
Copy link
Contributor

TimNN commented Mar 14, 2017

Performing an ARM build with --disable-optimize causes libcompiler_builtins.rlib (for ARM) to have references to core::panicking::panic. Note that the IR does not contain calls to those functions, however readelf / nm shows references to them.

As far as I can tell, this then causes libstd.so to have core::panicking::panic as a non-public symbol, leading to build failures later on.

This does not affect x86_64 linux as far as I can tell.

The IR and readelf output of compiler_builtins (these were generated with LLVM 4.0, but the problem also occurs with LLVM 3.9).

Edit: with LLVM 4.0, the following symbols have references to core::panic::panicking:

_ZN4core3num21_$LT$impl$u20$i64$GT$15overflowing_shr17hbb25084440bea1ffE
_ZN4core3num21_$LT$impl$u20$u32$GT$15overflowing_shl17hebf019cda7aaad4eE
_ZN4core3num21_$LT$impl$u20$u32$GT$15overflowing_shr17h28c64480ffb5b769E
_ZN4core3num21_$LT$impl$u20$u64$GT$15overflowing_shl17h057b937dda71a8daE
_ZN4core3num21_$LT$impl$u20$u64$GT$15overflowing_shr17h77d38bf1169ec68aE
_ZN4core3num22_$LT$impl$u20$i128$GT$15overflowing_shl17h264aab4280fbd05fE
_ZN4core3num22_$LT$impl$u20$i128$GT$15overflowing_shr17ha7a3617a02c8f37bE
_ZN4core3num22_$LT$impl$u20$u128$GT$15overflowing_shl17h7cbe64bca4ffa03cE
_ZN4core3num22_$LT$impl$u20$u128$GT$15overflowing_shr17h363dc51c526c1d4fE
__ashlti3
__ashrti3
__lshrti3
__multi3
__fixunsdfti
__fixunssfti
__fixdfti
__fixsfti

Regarding a fix, see @nagisa's comment #40123 (comment):

The panicking is introduced by overflowing_sh* which has implementation that looks like this:

(self >> (rhs & ($BITS - 1)), (rhs > ($BITS - 1)))

That shift over there is a checked shift and does not get optimised in no-opt mode. wrapping_sh* are implemented in terms of this overflowing_sh*. These could be fixed with some intrinsics, maybe?

Alternatively, is there maybe a way to disable debug-assertions on a per-function basis?

cc @nagisa, @alexcrichton, #40123

@TimNN
Copy link
Contributor Author

TimNN commented Mar 14, 2017

Alternatively, is there maybe a way to disable debug-assertions on a per-function basis?

As far as I can tell, compiler_builtins is already compiled without debug-assertions.

@TimNN TimNN mentioned this issue Mar 14, 2017
5 tasks
@est31
Copy link
Member

est31 commented Mar 14, 2017

I was with the impression that all builds of compiler_builtins were optimized? In this case it seems we have some dead code remnant of a panic call. I think its too hard to avoid all of them, and considering that we want to increase the amount of rust code code in the compiler_builtins implementation (see https://github.com/rust-lang-nursery/compiler-builtins), it will only get worse. So what about always enabling optimisations, at least for the std crates?

@shepmaster shepmaster mentioned this issue Mar 14, 2017
23 tasks
@alexcrichton
Copy link
Member

AFAIK we can't disable debug assertions on a per-function basis, so we'd need to add an intrinsic perhaps here.

@TimNN
Copy link
Contributor Author

TimNN commented Mar 14, 2017

I'll try the "fix with intrinsics" approach and see if that fixes the problem.

@est31
Copy link
Member

est31 commented Mar 16, 2017

I'm not sure IF this kind of panic freedom should be a goal (after all, who wants unoptimized builds?), but if it is a goal, this change should be backported to the upcoming replacement of this crate, the pure rust compiler builtins: https://github.com/rust-lang-nursery/compiler-builtins

@est31
Copy link
Member

est31 commented Mar 16, 2017

(with "this change" I mean #40521)

frewsxcv added a commit to frewsxcv/rust that referenced this issue Mar 17, 2017
Implemente overflowing_sh* with new unchecked_sh* intrinsics

Also update some 128 bit builtins to not rely on the constant evaluator to avoid checked operations.

Fixes rust-lang#40508.

cc @nagisa, @alexcrichton

Note: I still have a build running to see if the 128 bit changes worked (unoptimized builds take *forever* to compile), however at least the overflowing builtins no longer reference `core::panicking::panic`.
frewsxcv added a commit to frewsxcv/rust that referenced this issue Mar 17, 2017
Implemente overflowing_sh* with new unchecked_sh* intrinsics

Also update some 128 bit builtins to not rely on the constant evaluator to avoid checked operations.

Fixes rust-lang#40508.

cc @nagisa, @alexcrichton

Note: I still have a build running to see if the 128 bit changes worked (unoptimized builds take *forever* to compile), however at least the overflowing builtins no longer reference `core::panicking::panic`.
frewsxcv added a commit to frewsxcv/rust that referenced this issue Mar 17, 2017
Implemente overflowing_sh* with new unchecked_sh* intrinsics

Also update some 128 bit builtins to not rely on the constant evaluator to avoid checked operations.

Fixes rust-lang#40508.

cc @nagisa, @alexcrichton

Note: I still have a build running to see if the 128 bit changes worked (unoptimized builds take *forever* to compile), however at least the overflowing builtins no longer reference `core::panicking::panic`.
arielb1 pushed a commit to arielb1/rust that referenced this issue Mar 18, 2017
Implemente overflowing_sh* with new unchecked_sh* intrinsics

Also update some 128 bit builtins to not rely on the constant evaluator to avoid checked operations.

Fixes rust-lang#40508.

cc @nagisa, @alexcrichton

Note: I still have a build running to see if the 128 bit changes worked (unoptimized builds take *forever* to compile), however at least the overflowing builtins no longer reference `core::panicking::panic`.
arielb1 pushed a commit to arielb1/rust that referenced this issue Mar 19, 2017
Implemente overflowing_sh* with new unchecked_sh* intrinsics

Also update some 128 bit builtins to not rely on the constant evaluator to avoid checked operations.

Fixes rust-lang#40508.

cc @nagisa, @alexcrichton

Note: I still have a build running to see if the 128 bit changes worked (unoptimized builds take *forever* to compile), however at least the overflowing builtins no longer reference `core::panicking::panic`.
frewsxcv added a commit to frewsxcv/rust that referenced this issue Mar 20, 2017
Implemente overflowing_sh* with new unchecked_sh* intrinsics

Also update some 128 bit builtins to not rely on the constant evaluator to avoid checked operations.

Fixes rust-lang#40508.

cc @nagisa, @alexcrichton

Note: I still have a build running to see if the 128 bit changes worked (unoptimized builds take *forever* to compile), however at least the overflowing builtins no longer reference `core::panicking::panic`.
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

No branches or pull requests

3 participants