ziglang / zig

General-purpose programming language and toolchain for maintaining robust, optimal, and reusable software.
https://ziglang.org
MIT License
35.21k stars 2.57k forks source link

ReleaseSafe panic error trace points at line 0 when freeing an undefined pointer #20085

Open IntegratedQuantum opened 6 months ago

IntegratedQuantum commented 6 months ago

Zig Version

0.13.0-dev.75+5c9eb4081

Steps to Reproduce and Observed Behavior

The following code tries to free undefined slices:

const std = @import("std");

pub const Biome = struct {
    subBiomes: std.ArrayListUnmanaged(*const Biome) = undefined,

    pub fn init(self: *Biome) void {
        self.* = Biome {};
    }

    pub fn deinit(self: *Biome) void {
        self.subBiomes.deinit(std.heap.page_allocator);
    }
};

pub fn main() !void {
    var list = std.ArrayList(Biome).init(std.heap.page_allocator);
    defer list.deinit();
    for(0..143) |_| {
        var biome: Biome = undefined;
        biome.init();
        list.append(biome) catch unreachable;
    }
    defer for(list.items) |*biome| {
        biome.deinit();
    };
}

One line of the error message point at array_list.zig:0:0:

$ zig run test.zig -OReleaseSafe
thread 32698 panic: integer overflow
/home/mint/Cubyz/compiler/zig/lib/std/array_list.zig:0:0: 0x100a820 in main (test)
/home/mint/Cubyz/compiler/zig/lib/std/start.zig:511:37: 0x100a4aa in posixCallMainAndExit (test)
            const result = root.main() catch |err| {
                                    ^
/home/mint/Cubyz/compiler/zig/lib/std/start.zig:253:5: 0x100a381 in _start (test)
    asm volatile (switch (native_arch) {
    ^
???:?:?: 0x0 in ??? (???)
Aborted (core dumped)

Expected Behavior

The error should point at the right line instead of 0, which would be array_list.zig line 650

amp-59 commented 6 months ago

What panic do you expect from line 650 of array_list.zig?

When I run this program I get a different source location:

zig run -OReleaseSafe array_list_panic.zig
thread 125935 panic: mul overflowed 'u64': 12297829382473034410 * 8 above 'u64' maximum (18446744073709551615)
~/.local/share/projects/all/zig/lib/std/mem.zig:4121:61: 0x100ba8c in main (array_list_panic)
    return @as(cast_target, @ptrCast(slice))[0 .. slice.len * @sizeOf(std.meta.Elem(Slice))];
                                                            ^
~/.local/share/projects/all/zig/lib/std/start.zig:524:37: 0x100b65c in posixCallMainAndExit (array_list_panic)
            const result = root.main() catch |err| {
                                    ^
~/.local/share/projects/all/zig/lib/std/start.zig:266:5: 0x100b511 in _start (array_list_panic)
    asm volatile (switch (native_arch) {
    ^
???:?:?: 0x0 in ??? (???)
Aborted

This is using a different compiler, but matches the reported panic ID for your program, i.e. integer overflow.

I would note that no panic occurs when this program is compiled and run in Debug. This was true for every compiler I could test (53 in total going back to 0.11.0-dev.3187), and it makes me wonder if I missed something obvious in your code.

Also FYI, that large number being multiplied by 8 is @as(usize, undefined).

IntegratedQuantum commented 6 months ago

What panic do you expect from line 650 of array_list.zig?

Exactly the one I got, integer overflow. It should just tell me the correct line instead of 0.

I would note that no panic occurs when this program is compiled and run in Debug. This was true for every compiler I could test (53 in total going back to 0.11.0-dev.3187), and it makes me wonder if I missed something obvious in your code.

Yeah, the error happens only in release. But that is likely a different issue, I didn't report this because I think I saw a different issue on this already in the past.

This is using a different compiler

Which one? Also what operating system are you on? Maybe this is a linux issue? I just tried the latest release zig-linux-x86_64-0.13.0-dev.267+793f820b3 and I'm still getting the same behavior.

amp-59 commented 6 months ago

It should just tell me the correct line instead of 0.

Right. But the correct source location for the panic encountered by the program you posted is in mem.zig.

If you try compiling with the flags -fno-llvm and -fno-lld, you might find that a release compiler is able to produce the correct stack trace in Debug.

Which one?

This branch https://github.com/amp-59/zig/tree/runtime_safety.

Also what operating system are you on?

I am on Linux.

I have not tested anything, but I think both issues are caused by the same thing, which is the use of LLVM intrinsics for checking overflow when the compiler has LLVM enabled. This produces more efficient machine code but might be causing debug statements to be missing for the call to panic, so the last thing your DWARF parser sees is an address inside array_list.zig because the call to panic is not encoded.

That is just my first idea for debugging this. Honestly, the more concerning issue is that Debug is not panicking at all (or ReleaseSafe is panicking incorrectly).

IntegratedQuantum commented 6 months ago

Right. But the correct source location for the panic encountered by the program you posted is in mem.zig.

From what I've seen some functions are lost from the call stack due to inlining, this is normal behavior from what I've seen so far.

If you try compiling with the flags -fno-llvm and -fno-lld, you might find that a release compiler is able to produce the correct stack trace in Debug.

Yeah you are right, and with the following adaptation it also gives the correct stack trace in debug mode with llvm:

        var biome: Biome = undefined;
---     biome.init();
+++     _ = &biome;
        list.append(biome) catch unreachable;

So this issue is purely a problem of ReleaseSafe vs Debug and has nothing to do with the difference between llvm and the x86 backend.

Honestly, the more concerning issue is that Debug is not panicking at all

I made a new issue for that: #20095 I think in the case here we just get "lucky" and the undefined gets a non-zero value in ReleaseSafe.