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

TimerUnsupported in release builds #12776

Closed
shish opened this issue Sep 7, 2022 · 12 comments
Closed

TimerUnsupported in release builds #12776

shish opened this issue Sep 7, 2022 · 12 comments
Labels
bug Observed behavior contradicts documented or intended behavior frontend Tokenization, parsing, AstGen, Sema, and Liveness. stage1 The process of building from source via WebAssembly and the C backend.
Milestone

Comments

@shish
Copy link

shish commented Sep 7, 2022

Zig Version

0.10.0-dev.3884+11d14a23a

Steps to Reproduce

(I'm pretty sure that this is some kind of weird memory corruption issue, but I don't know what the heck is happening, so I've used the most obvious symptom as the issue title)

Given this test case (coming from a real app, where I've deleted as much as possible while still reproducing the issue):

const std = @import("std");

pub const RAM = struct {
    data: [0xFFFF + 1]u8,

    pub fn new() !RAM {
        return RAM{ .data = [_]u8{0} ** 0x10000 };
    }

    pub fn get(self: *RAM, addr: u16) u8 {
        return self.data[addr];
    }
};

pub const CPU = packed struct {
    interrupts: bool,
    ram: *RAM,

    pub fn new(ram: *RAM) !CPU {
        return CPU{
            .ram = ram,
            .interrupts = false,
        };
    }

    pub fn tick(self: *CPU) !void {
        var queued_interrupts = self.ram.get(0xFFFF) & self.ram.get(0xFF0F);
        if (self.interrupts and queued_interrupts != 0) {
            self.interrupts = false;
        }
    }
};


pub fn main() anyerror!void {
    var start = try std.time.Timer.start();
    var ram = try RAM.new();
    var cpu = try CPU.new(&ram);
    try cpu.tick();
    std.debug.print("Exit after {d}ns\n", .{start.read()});
}

Debug build works:

$ zig run -O Debug main.zig
Exit after 166375ns

Release build crashes with a really random error:

$ zig run -O ReleaseSafe main.zig
error: TimerUnsupported
/Users/shish2k/homebrew/Cellar/zig/HEAD-11d14a2_1/lib/zig/std/time.zig:270:45: 0x100ed6f93 in main (main)
        const current = Instant.now() catch return error.TimerUnsupported;
                                            ^
./main.zig:36:17: 0x100ed6f9b in main (main)
    var start = try std.time.Timer.start();

And if I delete code after the line that crashes, the error also disappears

In case this is some weird CPU-specific memory corruption issue -- I'm using an M1 MacBook Pro

Expected Behavior

Debug and Release builds should both work

Actual Behavior

Release build appears to crash in mysterious ways

Also, if I update the main function to be:

pub fn main() anyerror!void {
    var start = try std.time.Timer.start();
    std.debug.print("alive after start\n", .{});
    var ram = try RAM.new();
    std.debug.print("alive after RAM.new\n", .{});
    var cpu = try CPU.new(&ram);
    std.debug.print("alive after CPU.new\n", .{});
    try cpu.tick();
    std.debug.print("Exit after {d}ns\n", .{start.read()});
}

Then the addition of print statements appears to trigger a very different error:

$ zig run -O ReleaseSafe main.zig
alive after start
alive after RAM.new
alive after CPU.new
zsh: trace trap  zig run -O ReleaseSafe main.zig

$ echo $?
133

Is it possible that something in CPU.tick() is corrupting the memory (something to do with packed structs?), and somehow this is being mis-attributed to TimerUnsupported??

@shish shish added the bug Observed behavior contradicts documented or intended behavior label Sep 7, 2022
@Vexu Vexu added frontend Tokenization, parsing, AstGen, Sema, and Liveness. stage1 The process of building from source via WebAssembly and the C backend. labels Sep 8, 2022
@Vexu Vexu added this to the 0.10.0 milestone Sep 8, 2022
@Vexu
Copy link
Member

Vexu commented Sep 8, 2022

I'm assuming your issue is with the stage1 compiler since this didn't compile with stage2 without a few fixes but with them it now works as expected. Also note that it doesn't make much sense to packed struct here, at least not with stage2.

@shish
Copy link
Author

shish commented Sep 8, 2022

I don't know what stage1 or stage2 is ^^;;

this didn't compile with stage2

I am curious what didn't work

it doesn't make much sense to packed struct here

The original app (code now deleted) had (in the past) a reason to use it - but now that I check, it doesn't any more (all of the attributes which needed packing got moved to an internal packed struct). While removing "packed" from this minimal example fixes it, if I removed "packed" from the real app's CPU struct (leaving the internal packed struct, which is required), I then get a segfault instead, so that doesn't work as a workaround :(

I don't know if the segfault when using an internal packed struct is the same or a different bug to the TimerUnsupported / trace trap when using an outer packed struct, my gut tells me to fix this one and then see if the other one is fixed at the same time...

@Vexu
Copy link
Member

Vexu commented Sep 8, 2022

I don't know what stage1 or stage2 is ^^;;

stage1 is the bootstrap compiler, stage2 is the self hosted compiler. The version you are using should use the self hosted compiler by default which wasn't able to handle this yet so I'm not quite sure how you managed to run that example without -fstage1.

I am curious what didn't work

Just a few small missed conditions in codegen, you can check the linked commit for more information if you're curious.

The original app (code now deleted) had (in the past) a reason to use it - but now that I check, it doesn't any more (all of the attributes which needed packing got moved to an internal packed struct). While removing "packed" from this minimal example fixes it, if I removed "packed" from the real app's CPU struct (leaving the internal packed struct, which is required), I then get a segfault instead, so that doesn't work as a workaround :(

I don't know if the segfault when using an internal packed struct is the same or a different bug to the TimerUnsupported / trace trap when using an outer packed struct, my gut tells me to fix this one and then see if the other one is fixed at the same time...

I can't say for sure but this case works properly now at least.

@shish
Copy link
Author

shish commented Sep 8, 2022

stage1 is the bootstrap compiler, stage2 is the self hosted compiler. The version you are using should use the self hosted compiler by default which wasn't able to handle this yet so I'm not quite sure how you managed to run that example without -fstage1.

Well that's interesting - for me, stage1 can't compile it, but stage2 compiles (with errors in release mode):

Debug, stage1: compiler error

$ zig run -fstage1 -O Debug main.zig
broken LLVM module found: Invalid bitcast
  %9 = bitcast %RAM* %7 to i64, !dbg !2238
Trunc only produces integer
  %7 = trunc i72 %6 to %RAM*, !dbg !2266
Trunc only produces integer
  %14 = trunc i72 %13 to %RAM*, !dbg !2269

This is a bug in the Zig compiler.thread 4636848 panic: 
Unable to dump stack trace: debug info stripped
zsh: abort      zig run -fstage1 -O Debug main.zig

Debug, stage2: works

$ zig run -fno-stage1 -O Debug main.zig
alive after start
alive after RAM.new
alive after CPU.new
Exit after 158959ns

Release, stage1: compiler error

$ zig run -fstage1 -O ReleaseSafe main.zig
Semantic Analysis [815/980] broken LLVM module found: Invalid bitcast
  %9 = bitcast %RAM* %7 to i64, !dbg !2238
Trunc only produces integer
  %7 = trunc i72 %6 to %RAM*, !dbg !2266
Trunc only produces integer
  %14 = trunc i72 %13 to %RAM*, !dbg !2269

This is a bug in the Zig compiler.thread 4636040 panic: 
Unable to dump stack trace: debug info stripped
zsh: abort      zig run -fstage1 -O ReleaseSafe main.zig

Release, stage2: broken binary

$ zig run -fno-stage1 -O ReleaseSafe main.zig
alive after start
alive after RAM.new
alive after CPU.new
zsh: trace trap  zig run -fno-stage1 -O ReleaseSafe main.zig
zig run -fno-stage1 -O ReleaseSafe main.zig  3.47s user 0.10s system 89% cpu 4.012 total

I can't say for sure but this case works properly now at least.

Awesome, thank you :) I will try that, and then also try removing the now-redundant "packed", and open a new issue if that other crash is still happening

@dee0xeed
Copy link

dee0xeed commented Sep 8, 2022

I don't know what stage1 or stage2 is ^^;;

stage1 is the bootstrap compiler, stage2 is the self hosted compiler.

May I ask too?
Did I get it right that executable zig from binary distribution (for linux amd64) contains both code compiled from C++ source (stage1) and code compiled from zig source (stage2)?

@Vexu Vexu closed this as completed in c7e45ae Sep 8, 2022
@Vexu
Copy link
Member

Vexu commented Sep 8, 2022

Debug, stage1: compiler error

$ zig run -fstage1 -O Debug main.zig
broken LLVM module found: Invalid bitcast
  %9 = bitcast %RAM* %7 to i64, !dbg !2238
Trunc only produces integer
  %7 = trunc i72 %6 to %RAM*, !dbg !2266
Trunc only produces integer
  %14 = trunc i72 %13 to %RAM*, !dbg !2269

This is a bug in the Zig compiler.thread 4636848 panic: 
Unable to dump stack trace: debug info stripped
zsh: abort      zig run -fstage1 -O Debug main.zig

Debug, stage2: works

$ zig run -fno-stage1 -O Debug main.zig
alive after start
alive after RAM.new
alive after CPU.new
Exit after 158959ns

Your stage1/stage2 is reversed somehow, where did you get your Zig binary?

@Vexu
Copy link
Member

Vexu commented Sep 8, 2022

Did I get it right that executable zig from binary distribution (for linux amd64) contains both code compiled from C++ source (stage1) and code compiled from zig source (stage2)?

Yes, by default builds from ziglang.org/download use the self hosted backend stage2 (well actually stage3 which is stage2 built with stage2) but you can also opt for the old C++ bootstrap compiler stage1 with -fstage1 or .use_stage1 = true in your build script.

@dee0xeed
Copy link

dee0xeed commented Sep 8, 2022

Yes, by default builds from ziglang.org/download use the self hosted backend stage2

Thanks, that's good to know

@shish
Copy link
Author

shish commented Sep 8, 2022

Your stage1/stage2 is reversed somehow, where did you get your Zig binary?

using brew's "compile from master branch" script, specifically brew install zig --HEAD (I was originally using the 0.9.X binary installed via brew install zig, but managed to crash that compiler too :D)

@likern
Copy link

likern commented Sep 8, 2022

Did I get it right that executable zig from binary distribution (for linux amd64) contains both code compiled from C++ source (stage1) and code compiled from zig source (stage2)?

Yes, by default builds from ziglang.org/download use the self hosted backend stage2 (well actually stage3 which is stage2 built with stage2) but you can also opt for the old C++ bootstrap compiler stage1 with -fstage1 or .use_stage1 = true in your build script.

Now I suggest that #10873 is stage2?

Since version is 0.10.0-dev.358+36f13f591?

@Vexu
Copy link
Member

Vexu commented Sep 8, 2022

Now I suggest that #10873 is stage2?

Since version is 0.10.0-dev.358+36f13f591?

No, that is stage1. Stage2 doesn't allow arrays in packed structs and has only been the default since around 0.10.0-dev.3800.

@Banacial
Copy link
Contributor

I am on windows, Zig Version 0.10.1 and var timer = try std.time.Timer.start(); is still producing a TimerUnsupported Error

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 frontend Tokenization, parsing, AstGen, Sema, and Liveness. stage1 The process of building from source via WebAssembly and the C backend.
Projects
None yet
Development

No branches or pull requests

5 participants