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

stage2: array-like tuple initialization gets LLVM verification panic #11159

Closed
Tracked by #89
mitchellh opened this issue Mar 14, 2022 · 3 comments · Fixed by #11185
Closed
Tracked by #89

stage2: array-like tuple initialization gets LLVM verification panic #11159

mitchellh opened this issue Mar 14, 2022 · 3 comments · Fixed by #11185
Labels
bug Observed behavior contradicts documented or intended behavior frontend Tokenization, parsing, AstGen, Sema, and Liveness.
Milestone

Comments

@mitchellh
Copy link
Contributor

test {
    const T = @TypeOf(.{ @as(i32, 0), @as(u32, 0) });
    var a: T = .{ -1234, 5678 };
    _ = a;
}

result:

Stored value type does not match pointer operand type!
  store i32 -1234, i8* getelementptr inbounds (i8, i8* inttoptr (i64 -6148914691236517206 to i8*), i64 1), align 4, !dbg !647
 i32Stored value type does not match pointer operand type!
  store i32 5678, i8* getelementptr inbounds (i8, i8* inttoptr (i64 -6148914691236517206 to i8*), i64 1), align 4, !dbg !647
 i32
thread 711227 panic: LLVM module verification failed

Works on stage1.

@andrewrk andrewrk added bug Observed behavior contradicts documented or intended behavior frontend Tokenization, parsing, AstGen, Sema, and Liveness. labels Mar 14, 2022
@andrewrk andrewrk added this to the 0.10.0 milestone Mar 14, 2022
@topolarity
Copy link
Contributor

Based on inspecting the AIR, looks like the bug here is that either Sema needs to avoid generating a store to a comptime field, or that the LLVM backend needs to not generate a store when the target has no runtime bits.

@mitchellh
Copy link
Contributor Author

Note, as @topolarity pointed out, this doesn't actually work on stage1, but it has no error.

In stage1, both a[0] and a[1] are set to 0.

However, an LLVM module verification panic is probably not the desired outcome in stage2 regardless of correct behavior. :)

@topolarity
Copy link
Contributor

That's me :-)

This does actually work on stage1, but the test case above just happens to overlap with #11162. Here's one that doesn't:

test {
    const T = @TypeOf(.{ @as(i32, -1234), @as(u32, 5678) });
    var a: T = .{ -1234, 5678 };
    _ = a;
}

This works correctly on stage1 but panics on stage2, because of an AIR store to a comptime field:

  %16 = constant(i32, -1234)
...
  %10 = alloc(*tuple{comptime i32 = -1234, comptime u32 = 5678})
  %13 = struct_field_ptr_index_0(*i32, %10)
  %17!= store(%13!, %16!)

I think the bug is that Sema should not be generating that store of an i32 to a comptime field.

topolarity added a commit to topolarity/zig that referenced this issue Mar 15, 2022
This resolves ziglang#11159

The problem was that:
  1. We were not correctly deleting the field stores after recognizing
     that an array initializer was a comptime-known value.
  2. LLVM was not checking that the final type had no runtime bits, and
     so would generate an invalid store.

This also adds several test cases for related bugs, just to check these
in for later work.
topolarity added a commit to topolarity/zig that referenced this issue Mar 15, 2022
This resolves ziglang#11159

The problem was that:
  1. We were not correctly deleting the field stores after recognizing
     that an array initializer was a comptime-known value.
  2. LLVM was not checking that the final type had no runtime bits, and
     so would generate an invalid store.

This also adds several test cases for related bugs, just to check these
in for later work.
andrewrk pushed a commit to topolarity/zig that referenced this issue Mar 16, 2022
This resolves ziglang#11159

The problem was that:
  1. We were not correctly deleting the field stores after recognizing
     that an array initializer was a comptime-known value.
  2. LLVM was not checking that the final type had no runtime bits, and
     so would generate an invalid store.

This also adds several test cases for related bugs, just to check these
in for later work.
tiehuis pushed a commit to tiehuis/zig that referenced this issue Mar 19, 2022
This resolves ziglang#11159

The problem was that:
  1. We were not correctly deleting the field stores after recognizing
     that an array initializer was a comptime-known value.
  2. LLVM was not checking that the final type had no runtime bits, and
     so would generate an invalid store.

This also adds several test cases for related bugs, just to check these
in for later work.
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.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants