Stage2: simple if expression randomly has wrong result when safety checks are enabled

[Vexu]

test {
    @setRuntimeSafety(false);
    var self: []const u8 = "abcdef";
    var index: usize = 0;
    var left_index = (index << 1) + 1;
    var right_index = left_index + 1;
    var left = if (left_index < self.len) self[left_index] else null;
    var right = if (right_index < self.len) self[right_index] else null;
    std.debug.print("left {d} right {d}\n", .{ left_index < self.len, right_index < self.len });
    std.debug.print("left {d} right {d}\n", .{ left, right });
}

The same binary produced by zig2 test a.zig -femit-bin=a ran multiple times will sometimes print the correct result left 98 right 99 and on other times left null right null. When compiled in a release mode it always produces the correct result, @setRuntimeSafety does not work as the resulting AIR always has calls to panicOutOfBounds.

This is blocking std.PriorityQueue tests.

[andrewrk]

Valgrind reveals:

left ==115594== Conditional jump or move depends on uninitialised value(s)
==115594==    at 0x20DE52: fmt.formatType__anon_3387 (fmt.zig:472)
==115594==    by 0x20DD4A: fmt.format__anon_3374 (fmt.zig:178)
==115594==    by 0x20C66D: io.writer.Writer(fs.file.File,error{AccessDenied,BrokenPipe,ConnectionResetByPeer,DiskQuota,FileTooBig,InputOutput,NoSpaceLeft,NotOpenForWriting,OperationAborted,SystemResources,Unexpected,WouldBlock},(function 'write')).print__anon_2692 (io/writer.zig:28)
==115594==    by 0x20BE39: debug.print__anon_1045 (debug.zig:93)
==115594==    by 0x20BCD7: test3.test_0 (test3.zig:12)
==115594==    by 0x20D06A: test_runner.main (test_runner.zig:79)
==115594==    by 0x20C576: callMain (start.zig:571)
==115594==    by 0x20C576: callMainWithArgs (start.zig:460)
==115594==    by 0x20C576: start.posixCallMainAndExit (start.zig:425)
==115594==    by 0x20C0E2: (below main) (start.zig:338)

Points at

        .Optional => {
            if (value) |payload| {

Suggesting that left or right is not actually being initialized.

Looking at the LLVM IR:

Entry:
  %1 = alloca { i8, i1 }, align 1

; here is the block where `self[left_index]` is stored to `left`:

Block:                                            ; preds = %Then1
  %19 = extractvalue { i8*, i64 } %15, 0, !dbg !454
  %20 = getelementptr inbounds i8, i8* %19, i64 %16, !dbg !454
  %21 = load i8, i8* %20, align 1, !dbg !454
  %22 = bitcast { i8, i1 }* %1 to i8*, !dbg !454
  store i8 %21, i8* %22, align 1, !dbg !454 ; <==== here is the store
  br label %Block3, !dbg !454

So we can see that it is storing the payload of the optional, but not setting the non-null bit.

The corresponding AIR:

  %44 = alloc(*?u8)
  %45!= block(void, {
      ...
      %71 = slice_elem_val(%56!, %57!)
      %72!= dbg_block_end()
      %74 = bitcast(*u8, %44)
      %75!= store(%74!, %71!)

This seems like suspicious AIR code for Sema to emit because Sema is not really supposed to assume the layout of this type. So next I will investigate why this bitcast is being emitted by Sema.

[andrewrk]

With 8bf3e1f reverted, we get the correct behavior:

  %37 = constant(?u8, null)
...
  %12 = alloc(*?u8)
...
    %31!= cond_br(%21!, {
      %37!
      %22!= dbg_block_begin()
      %23 = load([]const u8, %3!)
      %25 = slice_elem_val(%23!, %24!)
      %26!= dbg_block_end()
      %27!= block(noreturn, {
        %34 = wrap_optional(?u8, %25!)
        %35!= br(%13, %34!)
      })
    }, {
      %24! %3!
      %28!= dbg_block_begin()
      %29!= dbg_block_end()
      %30!= br(%13, %37!)
    })

Then:                                             ; preds = %Entry
  %6 = load { i8*, i64 }, { i8*, i64 }* %2, align 8, !dbg !70
  %7 = extractvalue { i8*, i64 } %6, 0, !dbg !70
  %8 = getelementptr inbounds i8, i8* %7, i64 0, !dbg !70
  %9 = load i8, i8* %8, align 1, !dbg !70
  %10 = insertvalue { i8, i1 } undef, i8 %9, 0, !dbg !70
  %11 = insertvalue { i8, i1 } %10, i1 true, 1, !dbg !70
  br label %Block, !dbg !70

Else:                                             ; preds = %Entry
  br label %Block, !dbg !70

Block:                                            ; preds = %Else, %Then
  %12 = phi { i8, i1 } [ %11, %Then ], [ { i8 undef, i1 false }, %Else ], !dbg !70
  store { i8, i1 } %12, { i8, i1 }* %1, align 1, !dbg !70
  call void @llvm.dbg.declare(metadata { i8, i1 }* %1, metadata !63, metadata !DIExpression()), !dbg !70

In this case, each prong gets coerced to the optional type, then stored to the result.

So I think I will investigate reverting this commit while doing something else to keep all the tests passing.