Skip to content

segfault running "zig build" when zig is built in debug mode #10719

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

Closed
marler8997 opened this issue Jan 29, 2022 · 3 comments · Fixed by #10729
Closed

segfault running "zig build" when zig is built in debug mode #10719

marler8997 opened this issue Jan 29, 2022 · 3 comments · Fixed by #10729
Labels
bug Observed behavior contradicts documented or intended behavior
Milestone

Comments

@marler8997
Copy link
Contributor

marler8997 commented Jan 29, 2022

Zig Version

0.10.0-dev.5230+4613499fa

Steps to Reproduce

Build a debug version of Zig on Windows. Run it with "zig build".

Expected Behavior

Not to crash

Actual Behavior

Crashes with:

Segmentation fault at address 0xffffffffffffffff

Using a debugger I can see the crash occurs in __muloti4. This is normally provided by compiler_rt, however, in the Zig executable it's shadowed by the one from c++.lib. I can reproduce this by compiling a simple test program and adding -lc++ which will cause it to link to __muloti4 from c++lib and reproduce the crash.

Note that this call to __muloti4 comes from std.os.windows.fromSysTime.

pub fn fromSysTime(hns: i64) i128 {
    const adjusted_epoch: i128 = hns + std.time.epoch.windows * (std.time.ns_per_s / 100);
    return adjusted_epoch * 100;
}

If I remove the adjusted_epoch local variable, the call to _muloti4 goes away and so does the crash. I've included the assembly for both of these functions below.

The segfault occurs early in __muloti4 which makes me guess the segfault probably occurs here:

My guess is that zig isn't able to handle a callconv (.C) function with 2 i128 followed by a pointer, and the pointer ends up being invalid. Note that this is just "integers" and "pointers" which #1481 says it supports, but this might be a regression of it.

Supplemental Info

Not sure if this helps but here are the values of each variable in fromSysTime if that helps.

expr value
hns (from debugger) 132879187288034414
std.time.epoch.windows * (std.time.ns_per_s / 100) (calculated by hand, these are comptime-known) -116444736000000000
adjusted_epoch 16434451288034414
should return 1643445128803441400
__muloti4:
00007FF60FF04180  push        rbp  
00007FF60FF04181  push        r15  
00007FF60FF04183  push        r14  
00007FF60FF04185  push        r13  
00007FF60FF04187  push        r12  
00007FF60FF04189  push        rsi  
00007FF60FF0418A  push        rdi  
00007FF60FF0418B  push        rbx  
00007FF60FF0418C  sub         rsp,68h  
00007FF60FF04190  lea         rbp,[rsp+60h]  
00007FF60FF04195  mov         r14,r8  
00007FF60FF04198  mov         r15,qword ptr [rcx]              *** SEGFAULT HERE ***
00007FF60FF0419B  mov         rbx,qword ptr [rcx+8]  
00007FF60FF0419F  vmovdqa     xmm0,xmmword ptr [rcx]  
00007FF60FF041A3  mov         rdi,qword ptr [rdx]  
00007FF60FF041A6  mov         rsi,qword ptr [rdx+8]  
00007FF60FF041AA  mov         dword ptr [r8],0  
00007FF60FF041B1  vpxor       xmm0,xmm0,xmmword ptr [_ZTISt9bad_alloc+20h (07FF6147BF220h)]  
00007FF60FF041B9  vptest      xmm0,xmm0  
00007FF60FF041BE  jne         __muloti4+49h (07FF60FF041C9h)  
00007FF60FF041C0  cmp         rdi,2  
00007FF60FF041C4  mov         rax,rsi  
00007FF60FF041C7  jmp         __muloti4+65h (07FF60FF041E5h)  
00007FF60FF041C9  mov         r8,8000000000000000h  
00007FF60FF041D3  mov         rax,rsi  
00007FF60FF041D6  xor         rax,r8  
00007FF60FF041D9  or          rax,rdi  
00007FF60FF041DC  jne         __muloti4+0A7h (07FF60FF04227h)  
00007FF60FF041DE  cmp         r15,2  
00007FF60FF041E2  mov         rax,rbx  
00007FF60FF041E5  sbb         rax,0  
00007FF60FF041E9  jb          __muloti4+72h (07FF60FF041F2h)  
00007FF60FF041EB  mov         dword ptr [r14],1  
00007FF60FF041F2  mov         rdx,rdi  
00007FF60FF041F5  mulx        rax,rcx,r15  
00007FF60FF041FA  imul        rbx,rdi  
00007FF60FF041FE  add         rbx,rax  
00007FF60FF04201  imul        rsi,r15  
00007FF60FF04205  add         rsi,rbx  
00007FF60FF04208  vmovq       xmm0,rsi  
00007FF60FF0420D  vmovq       xmm1,rcx  
00007FF60FF04212  vpunpcklqdq xmm0,xmm1,xmm0  
00007FF60FF04216  add         rsp,68h  
00007FF60FF0421A  pop         rbx  
00007FF60FF0421B  pop         rdi  
00007FF60FF0421C  pop         rsi  
00007FF60FF0421D  pop         r12  
00007FF60FF0421F  pop         r13  
00007FF60FF04221  pop         r14  
00007FF60FF04223  pop         r15  
00007FF60FF04225  pop         rbp  
00007FF60FF04226  ret  

Also for reference, here's the assembly for fromSysTime:

/// which is the POSIX one (Jan 01, 1970 AD).
pub fn fromSysTime(hns: i64) i128 {
00007FF645304BE0  push        rbp  
00007FF645304BE1  sub         rsp,70h  
00007FF645304BE5  lea         rbp,[rsp+70h]  
00007FF645304BEA  mov         qword ptr [rbp-28h],rcx  
    const adjusted_epoch: i128 = hns + std.time.epoch.windows * (std.time.ns_per_s / 100);
00007FF645304BEE  mov         rax,0FE624E212AC18000h  
00007FF645304BF8  add         rax,qword ptr [hns]  
00007FF645304BFC  mov         qword ptr [rbp-38h],rax  
00007FF645304C00  seto        al  
00007FF645304C03  jo          std.os.windows.fromSysTime+27h (07FF645304C07h)  
00007FF645304C05  jmp         std.os.windows.fromSysTime+37h (07FF645304C17h)  
00007FF645304C07  lea         rcx,[_ZZ14__mingw_uuidofI18SetupConfigurationERK5_GUIDvE11__uuid_inst+0A0CA8h (07FF64A938450h)]  
00007FF645304C0E  xor         eax,eax  
00007FF645304C10  mov         edx,eax  
00007FF645304C12  call        std.builtin.default_panic (07FF645268100h)  
00007FF645304C17  mov         rcx,qword ptr [rbp-38h]  
00007FF645304C1B  mov         rax,rcx  
00007FF645304C1E  sar         rax,3Fh  
00007FF645304C22  mov         qword ptr [rbp-20h],rcx  
00007FF645304C26  mov         qword ptr [rbp-18h],rax  
    return adjusted_epoch * 100;
00007FF645304C2A  mov         rcx,qword ptr [rbp-20h]  
00007FF645304C2E  mov         rdx,qword ptr [rbp-18h]  
00007FF645304C32  mov         qword ptr [rbp-30h],0  
00007FF645304C3A  mov         rax,rsp  
00007FF645304C3D  lea         r8,[rbp-30h]  
00007FF645304C41  mov         qword ptr [rax+20h],r8  
00007FF645304C45  xor         eax,eax  
00007FF645304C47  mov         r9d,eax  
00007FF645304C4A  mov         r8d,64h  
00007FF645304C50  call        __muloti4 (07FF646048710h)   ***here's where __muloti4 is called when the segfault occurs***
00007FF645304C55  mov         rcx,rax  
00007FF645304C58  mov         rax,qword ptr [rbp-30h]  
00007FF645304C5C  test        rax,rax  
00007FF645304C5F  setne       al  
00007FF645304C62  mov         qword ptr [rbp-48h],rdx  
00007FF645304C66  mov         qword ptr [rbp-40h],rcx  
00007FF645304C6A  test        al,1  
00007FF645304C6C  jne         std.os.windows.fromSysTime+90h (07FF645304C70h)  
00007FF645304C6E  jmp         std.os.windows.fromSysTime+0A0h (07FF645304C80h)  
00007FF645304C70  lea         rcx,[_ZZ14__mingw_uuidofI18SetupConfigurationERK5_GUIDvE11__uuid_inst+0A0CA8h (07FF64A938450h)]  
00007FF645304C77  xor         eax,eax  
00007FF645304C79  mov         edx,eax  
00007FF645304C7B  call        std.builtin.default_panic (07FF645268100h)  
00007FF645304C80  mov         rax,qword ptr [rbp-48h]  
00007FF645304C84  mov         rcx,qword ptr [rbp-40h]  
00007FF645304C88  mov         qword ptr [rbp-10h],rcx  
00007FF645304C8C  mov         qword ptr [rbp-8],rax  
00007FF645304C90  mov         rax,qword ptr [rbp-10h]  
00007FF645304C94  mov         rdx,qword ptr [rbp-8]  
00007FF645304C98  add         rsp,70h  
00007FF645304C9C  pop         rbp  
00007FF645304C9D  ret  
@marler8997 marler8997 added the bug Observed behavior contradicts documented or intended behavior label Jan 29, 2022
marler8997 added a commit to marler8997/zig that referenced this issue Jan 29, 2022
A workaround for issue ziglang#10719.  Moves the expression `std.time.epoch.windows * (std.time.ns_per_s / 100)` into a global const to force compiler to evaluate it a comptime (even in debug builds).  Evaluating it at comptime causes a segfault on my Windows system.  See issue ziglang#10719 for more details.
marler8997 added a commit to marler8997/zig that referenced this issue Jan 29, 2022
A workaround for issue ziglang#10719.  Moves the expression `std.time.epoch.windows * (std.time.ns_per_s / 100)` into a global const to force compiler to evaluate it a comptime (even in debug builds) because evaluating it at runtime causes a segfault on my Windows system.  See issue ziglang#10719 for more details.
marler8997 added a commit to marler8997/zig that referenced this issue Jan 29, 2022
A workaround for issue ziglang#10719.  Moves the expression `std.time.epoch.windows * (std.time.ns_per_s / 100)` into a global const to force compiler to evaluate it a comptime (even in debug builds) because evaluating it at runtime causes a segfault on my Windows system.  See issue ziglang#10719 for more details.
marler8997 added a commit to marler8997/zig that referenced this issue Jan 29, 2022
A workaround for issue ziglang#10719.  Moves the expression `std.time.epoch.windows * (std.time.ns_per_s / 100)` into a global const to force compiler to evaluate it a comptime (even in debug builds) because evaluating it at runtime causes a segfault on my Windows system.  I also needed to remove the temporary `adjusted_epoc` variable. See issue ziglang#10719 for more details.
marler8997 added a commit to marler8997/zig that referenced this issue Jan 29, 2022
A workaround for issue ziglang#10719.  Removes the temporary i128 local variable `adjusted_epoch` which prevents the segfault from occuring.  It's unclear as to why. See issue ziglang#10719 for more details.
marler8997 added a commit to marler8997/zig that referenced this issue Jan 29, 2022
A workaround for issue ziglang#10719.  Removes the temporary i128 local variable `adjusted_epoch` which prevents the segfault from occuring.  It's unclear as to why. See issue ziglang#10719 for more details.
marler8997 added a commit to marler8997/zig that referenced this issue Jan 29, 2022
A workaround for issue ziglang#10719.  Removes the temporary i128 local variable `adjusted_epoch` which prevents the segfault from occuring.  It's unclear as to why. See issue ziglang#10719 for more details.
marler8997 added a commit to marler8997/zig that referenced this issue Jan 29, 2022
A workaround for issue ziglang#10719.  Removes the temporary i128 local variable `adjusted_epoch` which prevents the segfault from occuring.  It's unclear as to why. See issue ziglang#10719 for more details.
@marler8997
Copy link
Contributor Author

For convenience I've attached a diff between the crashing version (with the adjusted_epoch local variable) on the left and the fixed version (removed adjusted_epoch variable) on the right. One big difference is that the version on the right does not call __muloti4 which is where the crash actually occurs, maybe this is a bug with that function?

image

@marler8997

This comment has been minimized.

@marler8997
Copy link
Contributor Author

marler8997 commented Jan 29, 2022

Ok I found a way to reproduce this issue. Here's the program:

bug.zig:

extern fn __muloti4(a: i128, b: i128, overflow: *c_int) callconv(.C) i128;
pub fn main() void {
    var overflow: c_int = -1;
    var result = __muloti4(132879187288034414, -116444736000000000, &overflow);
    @import("std").log.info("result={}, overflow={}", .{result, overflow});
}

Works when you compile/run it like this:

>zig build-exe bug.zig
>bug
info: result=-15473081883649723297144704000000000, overflow=0

Add -lc++ and it will crash:

>zig build-exe bug.zig -lc++
>bug
Segmentation fault at address 0xffffffffffffffff

In the first case, zig links to __muloti4 from compiler_rt which works, in the second case it links to the one from c++.lib which is borked for some reason.

marler8997 added a commit to marler8997/zig that referenced this issue Jan 29, 2022
fixes ziglang#10719

compiler_rt already provides __muloti4 but libc++ is also providing it and when linking libc++ it causes a crash on my windows x86_64 machine.
@andrewrk andrewrk added this to the 0.9.1 milestone Jan 30, 2022
kubkon pushed a commit that referenced this issue Jan 30, 2022
fixes #10719

compiler_rt already provides __muloti4 but libc++ is also providing it and when linking libc++ it causes a crash on my windows x86_64 machine.
andrewrk pushed a commit that referenced this issue Feb 3, 2022
fixes #10719

compiler_rt already provides __muloti4 but libc++ is also providing it and when linking libc++ it causes a crash on my windows x86_64 machine.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants