Broken LLVM module found when mistakenly comparing ?T with T inside for loop

[yettinmoor]

Found a strange bug when I accidentally compared an optional value with an integer:

const std = @import("std");
const print = std.debug.print;

pub fn main() !void {
    var x: ?u8 = 2;
    if (x != null and x == 2) print("found\n", .{});
    //                ^ bug: should unwrap!
    //                  but the compiler catches it.

    const nums = &[_]u8{ 1, 2, 3, 4 };
    for (nums) |num, i|
        if (x != null and x == num) print("found\n", .{});
    //                    ^ bug: should unwrap!
    //                     "broken LLVM module found..."
}

Note that the compiler catches the first error x == 2 and reports it. In the second example however I get the "This is a bug in the Zig compiler" message.

broken LLVM module found: PHI node entries do not match predecessors!
  %16 = phi i1 [ %8, %ForBody ], [ %15, %BoolAndTrue ], !dbg !17256
label %BoolAndTrue
label %CmpOptionalNonOptionalEnd
Instruction does not dominate all uses!
  %15 = phi i1 [ false, %CmpOptionalNonOptionalOptionalNull ], [ %14, %CmpOptionalNonOptionalOpti
onalNotNull ], !dbg !17258
  %16 = phi i1 [ %8, %ForBody ], [ %15, %BoolAndTrue ], !dbg !17256

This is a bug in the Zig compiler.
/home/nico/repos/zig/src-self-hosted/stage2.zig:36:5: 0x55ee16cfbbec in stage2_panic (zigstage2)
    @panic(ptr[0..len]);
    ^
/home/nico/repos/zig/src/util.cpp:20:17: 0x55ee16cac435 in zig_panic (/home/nico/repos/zig/src/ut
il.cpp)
    stage2_panic("", 0);
                ^
/home/nico/repos/zig/src/codegen.cpp:8313:18: 0x55ee16bafb58 in do_code_gen (/home/nico/repos/zig
/src/codegen.cpp)
        zig_panic("broken LLVM module found: %s\nThis is a bug in the Zig compiler.", error);
                 ^
/home/nico/repos/zig/src/codegen.cpp:11067:24: 0x55ee16bbb0ce in codegen_build_and_link (/home/ni
co/repos/zig/src/codegen.cpp)
            do_code_gen(g);
                       ^
/home/nico/repos/zig/src/main.cpp:1677:39: 0x55ee16b882f3 in main0 (/home/nico/repos/zig/src/main
.cpp)
                codegen_build_and_link(g);
                                      ^
/home/nico/repos/zig/src/main.cpp:1864:24: 0x55ee16b88e62 in main (/home/nico/repos/zig/src/main.
cpp)
    auto result = main0(argc, argv);
                       ^
Aborted (core dumped)

shell returned 134

Neither of these variants cause a crash. Zig reports them correctly.

    const nums = &[_]u8{ 1, 2, 3, 4 };

    // Compare to constant
    for (nums) |_|
        if (x != null and x == 5) print("found\n", .{});

    // Compare to index
    for (nums) |_, i|
        if (x != null and x == i) print("found\n", .{});

[LemonBoy]

The boolean chain creates a series of basic blocks (BoolAndFalse and BoolAndTrue here) in the first pass while the ?T==T is "desugared" in a later stage (#5390). This creates a huge mess as the phi node of the boolean chain become out of sync due to the new blocks being forcefully inserted so late in the analysis pass.

I hope the stage2 architecture will be able to support this kind of operations without this messy trick.

[andrewrk]

I hope the stage2 architecture will be able to support this kind of operations without this messy trick.

Interested in taking a peek? Development on this has started in earnest :-)

[LemonBoy]

I'm slo(ooooo)wly catching up with all the new stuff so take the following with a grain of salt, I've just started reading zir/ir.zig.
At first glance the zir->ir pass looks flexible enough to implement this comparison either at the AST level a == b ==> a != null and a.? == b (or blk: { if (a) |tmp| { break :blk tmp == b; } else { break :blk false; } }, letting the sema pass analyze this new synthetic construct, or at the IR level with a new ad-hoc operator.
I believe the former can now be easily done at the zir level, the latter is more complex and the required branching logic needed to implement the comparison may invalidate the already computed control flow.

[Vexu]

Duplicate of #6059.