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

Fix the weird-ass crash on AArch64 #4832

Merged
merged 2 commits into from
Mar 28, 2020
Merged

Conversation

LemonBoy
Copy link
Contributor

What

It turns out that adding our own implementation of __addtf3 uncovered a (painfully obvious in hindsight) bug in another unrelated builtin.

How

The main clue leading to this discovery was this piece of ASM:

mov  x19, x1                                
bl   0xffffa3b8e320 <__extendsftf2@plt>     
str  q0, [sp, #48]                          
mov  x0, x21                                
bl   0xffffa3b865b0 <__floatunditf@plt>     
ldr  q2, [sp, #48]                          
mov  v1.16b, v2.16b                         
str  q2, [sp, #80]                          
bl   0xffffa3b8e0b0 <__divtf3@plt>          
str  q0, [sp, #48]                          
mov  x0, x19                                
bl   0xffffa3b865b0 <__floatunditf@plt>     
str  q0, [sp, #64]                          
ldr  q4, [sp, #48]                          
mov  v1.16b, v4.16b                         
bl   0xffffa3b87fc0 <__letf2@plt>           
cmp  w0, #0x0                               
ldr  q6, [sp, #64]                          
ldr  q2, [sp, #80]                          

Can you spot the error? No? Because there's no error!
This is the code from libstdc++'s Prime_rehash_policy::need_rehash that's completely correct and indeed works just fine when linked against libgcc or LLVM's compiler-rt.

What prompted me to look at __floatundidf was this piece of code:

str  q0, [sp, #48]                          
mov  x0, x19                                
bl   0xffffa3b865b0 <__floatunditf@plt>    

As you can see it only setups x0, the first register used for passing arguments, and, being the xN registers 64-bit wide, that meant x1 was actually filled with hot garbage! (Remember that x0-x7 are scratch registers and are not saved across calls).

The fix is easy peasy, the big question looming over us is: are there any other mistakes like this?

Proudly closes #4822

This builtin converts a u64 into a f128, not a u128 into a f128.

Fixes some weird-ass crashes that happened only on AArch64 systems.
@andrewrk
Copy link
Member

👏 👏 👏 👏 👏 👏

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 this pull request may close these issues.

Attempting to link against zig library with --bundle-compiler-rt throwing bad_alloc() on alpine linux aarch64
2 participants