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: fixes for error union semantics #11699

Merged
merged 18 commits into from
May 25, 2022
Merged

stage2: fixes for error union semantics #11699

merged 18 commits into from
May 25, 2022

Conversation

andrewrk
Copy link
Member

  • Sema: avoid unnecessary safety checks when an error set is empty.
  • Sema: make zirErrorToInt handle comptime errors that are represented
    as integers.
  • Sema: make empty error sets properly integrate with
    typeHasOnePossibleValue.
  • Type: correct the ABI alignment and size of error unions which have
    both zero-bit error set and zero-bit payload. The previous code did
    not account for the fact that we still need to store a bit for
    whether there is an error.
  • LLVM: lower error unions possibly with the payload first or with the
    error code first, depending on alignment. Previously it always put
    the error code first and used a padding array.
  • LLVM: lower functions which have an empty error set as the return
    type the same as anyerror, so that they can be used where
    fn()anyerror function pointers are expected. In such functions, Zig
    will lower ret to returning zero instead of void.

As a result, one more behavior test is passing.

@andrewrk
Copy link
Member Author

Debugging session on the wasm backend failure:

[nix-shell:~/dev/zig/build-release]$ gdb ./stage2/bin/zig -ex 'run test ../test/behavior.zig -I../test 
-target wasm32-wasi -fno-LLVM'
GNU gdb (GDB) 11.1
Copyright (C) 2021 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-unknown-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from ./stage2/bin/zig...
Starting program: /home/andy/dev/zig/build-release/stage2/bin/zig test ../test/behavior.zig -I../test -target wasm32-wasi -fno-LLVM
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/nix/store/g02b1lpbddhymmcjb923kf0l7s9nww58-glibc-2.33-123/lib/libthread_db.so.1".
[New Thread 0x7ffff7c65640 (LWP 3663479)]
[New Thread 0x7ffff6c64640 (LWP 3663480)]
[New Thread 0x7ffff5c63640 (LWP 3663481)]
[New Thread 0x7ffff4c62640 (LWP 3663482)]
[New Thread 0x7ffff3c61640 (LWP 3663483)]
[New Thread 0x7ffff2c60640 (LWP 3663484)]
[New Thread 0x7ffff1c5f640 (LWP 3663485)]
[New Thread 0x7ffff0c5e640 (LWP 3663486)]
[New Thread 0x7fffefc5d640 (LWP 3663487)]
[New Thread 0x7fffeec5c640 (LWP 3663488)]
[New Thread 0x7fffedc5b640 (LWP 3663489)]
[New Thread 0x7fffecc5a640 (LWP 3663490)]
[New Thread 0x7fffebc59640 (LWP 3663491)]
[New Thread 0x7fffeac58640 (LWP 3663492)]
[New Thread 0x7fffe9c57640 (LWP 3663493)]
[New Thread 0x7fffe8c56640 (LWP 3663494)]
thread 3663475 panic: reached unreachable code
/home/andy/dev/zig/src/arch/wasm/CodeGen.zig:230:21: 0x3b897b1 in arch.wasm.CodeGen.buildOpcode (zig)
            else => unreachable,
                    ^
/home/andy/dev/zig/src/arch/wasm/CodeGen.zig:1886:31: 0x3b88198 in arch.wasm.CodeGen.load (zig)
    const opcode = buildOpcode(.{
                              ^
/home/andy/dev/zig/src/arch/wasm/CodeGen.zig:1878:21: 0x3b6ece5 in arch.wasm.CodeGen.airLoad (zig)
    return self.load(operand, ty, 0);
                    ^
/home/andy/dev/zig/src/arch/wasm/CodeGen.zig:1509:30: 0x39cea40 in arch.wasm.CodeGen.genInst (zig)
        .load => self.airLoad(inst),
                             ^
/home/andy/dev/zig/src/arch/wasm/CodeGen.zig:1619:40: 0x37e2d1d in arch.wasm.CodeGen.genBody (zig)
        const result = try self.genInst(inst);
                                       ^
/home/andy/dev/zig/src/arch/wasm/CodeGen.zig:2446:21: 0x3b695d4 in arch.wasm.CodeGen.airBlock (zig)
    try self.genBody(body);
                    ^
/home/andy/dev/zig/src/arch/wasm/CodeGen.zig:1472:32: 0x39ce495 in arch.wasm.CodeGen.genInst (zig)
        .block => self.airBlock(inst),
                               ^
/home/andy/dev/zig/src/arch/wasm/CodeGen.zig:1619:40: 0x37e2d1d in arch.wasm.CodeGen.genBody (zig)
        const result = try self.genInst(inst);
                                       ^
/home/andy/dev/zig/src/arch/wasm/CodeGen.zig:890:21: 0x35f5a95 in arch.wasm.CodeGen.genFunc (zig)
    try self.genBody(self.air.getMainBody());
                    ^
/home/andy/dev/zig/src/arch/wasm/CodeGen.zig:868:12: 0x3408585 in arch.wasm.CodeGen.generate (zig)
    genFunc(&code_gen) catch |err| switch (err) {
           ^
/home/andy/dev/zig/src/codegen.zig:134:60: 0x33fa458 in codegen.generateFunction (zig)
        => return @import("arch/wasm/CodeGen.zig").generate(bin_file, src_loc, func, air, liveness, code, debug_output),
                                                           ^
/home/andy/dev/zig/src/link/Wasm.zig:570:48: 0x32040cb in link.Wasm.updateFunc (zig)
    const result = try codegen.generateFunction(
                                               ^
/home/andy/dev/zig/src/link.zig:471:77: 0x30484e5 in link.File.updateFunc (zig)
            .wasm  => return @fieldParentPtr(Wasm,  "base", base).updateFunc(module, func, air, liveness),
                                                                            ^
/home/andy/dev/zig/src/Module.zig:3708:41: 0x3027d72 in Module.ensureFuncBodyAnalyzed (zig)
            mod.comp.bin_file.updateFunc(mod, func, air, liveness) catch |err| switch (err) {
                                        ^
/home/andy/dev/zig/src/Compilation.zig:2787:42: 0x2ddd2e7 in Compilation.processOneJob (zig)
            module.ensureFuncBodyAnalyzed(func) catch |err| switch (err) {
                                         ^
/home/andy/dev/zig/src/Compilation.zig:2729:30: 0x2dc996d in Compilation.performAllTheWork (zig)
            try processOneJob(comp, work_item);
                             ^
/home/andy/dev/zig/src/Compilation.zig:2116:31: 0x2dc2632 in Compilation.update (zig)
    try comp.performAllTheWork(main_progress_node);
                              ^
/home/andy/dev/zig/src/main.zig:3101:20: 0x2d2e3af in updateModule (zig)
    try comp.update();
                   ^
/home/andy/dev/zig/src/main.zig:2790:17: 0x2cf6e82 in buildOutputType (zig)
    updateModule(gpa, comp, hook) catch |err| switch (err) {
                ^
/home/andy/dev/zig/src/main.zig:225:31: 0x2cdce76 in mainArgs (zig)
        return buildOutputType(gpa, arena, args, .zig_test);
                              ^
/home/andy/dev/zig/src/main.zig:174:20: 0x2cdbec4 in main (zig)
    return mainArgs(gpa, arena, args);
                   ^
/home/andy/dev/zig/lib/std/start.zig:581:37: 0x31b5ea7 in std.start.callMain (zig)
            const result = root.main() catch |err| {
                                    ^
/home/andy/dev/zig/lib/std/start.zig:515:12: 0x2cde917 in std.start.callMainWithArgs (zig)
    return @call(.{ .modifier = .always_inline }, callMain, .{});
           ^
/home/andy/dev/zig/lib/std/start.zig:480:12: 0x2cde6c2 in std.start.main (zig)
    return @call(.{ .modifier = .always_inline }, callMainWithArgs, .{ @intCast(usize, c_argc), c_argv, envp });
           ^

Thread 1 "zig" received signal SIGABRT, Aborted.
0x00007ffff7cabbda in raise () from /nix/store/g02b1lpbddhymmcjb923kf0l7s9nww58-glibc-2.33-123/lib/libc.so.6
(gdb) up
#1  0x00007ffff7c96533 in abort () from /nix/store/g02b1lpbddhymmcjb923kf0l7s9nww58-glibc-2.33-123/lib/libc.so.6
(gdb) 
#2  0x0000000002cd5559 in std.os.abort () at /home/andy/dev/zig/lib/std/os.zig:472
472	    system.abort();
(gdb) 
#3  0x0000000002fe3589 in crash_report.PanicSwitch.abort () at crash_report.zig:545
545	        os.abort();
(gdb) 
#4  0x00000000031cb4ee in crash_report.PanicSwitch.goTo () at crash_report.zig:553
553	        @call(.{}, func, args);
(gdb) 
#5  crash_report.PanicSwitch.releaseRefCount (state=0x7ffff7c66728) at crash_report.zig:527
527	        goTo(abort, .{});
(gdb) 
#6  0x00000000031b61d0 in crash_report.PanicSwitch.goTo (args=...) at crash_report.zig:553
553	        @call(.{}, func, args);
(gdb) 
#7  crash_report.PanicSwitch.releaseMutex (state=0x7ffff7c66728) at crash_report.zig:499
499	        goTo(releaseRefCount, .{state});
(gdb) 
#8  0x00000000031cb012 in crash_report.PanicSwitch.goTo (args=...) at crash_report.zig:553
553	        @call(.{}, func, args);
(gdb) 
#9  crash_report.PanicSwitch.reportStack (state=0x7ffff7c66728) at crash_report.zig:481
481	        goTo(releaseMutex, .{state});
(gdb) 
#10 0x0000000002fe2661 in crash_report.PanicSwitch.goTo (args=...) at crash_report.zig:553
553	        @call(.{}, func, args);
(gdb) 
#11 crash_report.PanicSwitch.initPanic (state=0x7ffff7c66728, trace=0x0, stack=..., msg="reached unreachable code") at crash_report.zig:456
456	        goTo(reportStack, .{state});
(gdb) 
#12 0x0000000002cdba96 in crash_report.PanicSwitch.goTo (args=...) at crash_report.zig:553
553	        @call(.{}, func, args);
(gdb) 
#13 crash_report.PanicSwitch.dispatch (trace=0x0, stack_ctx=..., msg="reached unreachable code") at crash_report.zig:410
410	            .initialize => goTo(initPanic, .{ panic_state, trace, stack_ctx, msg }),
(gdb) 
#14 0x0000000002cd46de in crash_report.compilerPanic (msg="reached unreachable code", error_return_trace=0x0) at crash_report.zig:163
163	    PanicSwitch.dispatch(error_return_trace, stack_ctx, msg);
(gdb) 
#15 0x0000000003b897b2 in arch.wasm.CodeGen.buildOpcode (args=...) at /home/andy/dev/zig/src/arch/wasm/CodeGen.zig:230
230	            else => unreachable,
(gdb) p args
$1 = {
  valtype1 = {
    payload = i32
  },
  op = load,
  width = {
    payload = 96 '`'
  },
  valtype2 = null,
  signedness = null
}
(gdb) up
#16 0x0000000003b88199 in arch.wasm.CodeGen.load (self=0x7fffffff0180, operand=..., ty={...}, offset=0) at /home/andy/dev/zig/src/arch/wasm/CodeGen.zig:1886
1886	    const opcode = buildOpcode(.{
(gdb) p abi_size
$2 = 12 '\f'
(gdb) p ty
$3 = {
  tag = .error_union,
  payload = {
    error_set = {
      tag = .error_set,
      payload = 0x9dac708
    },
    payload = .const_slice_u8
  }
}
quit) 
A debugging session is active.

	Inferior 1 [process 3663475] will be killed.

Quit anyway? (y or n) y

andrewrk and others added 18 commits May 24, 2022 15:34
 * Sema: avoid unnecessary safety checks when an error set is empty.
 * Sema: make zirErrorToInt handle comptime errors that are represented
   as integers.
 * Sema: make empty error sets properly integrate with
   typeHasOnePossibleValue.
 * Type: correct the ABI alignment and size of error unions which have
   both zero-bit error set and zero-bit payload. The previous code did
   not account for the fact that we still need to store a bit for
   whether there is an error.
 * LLVM: lower error unions possibly with the payload first or with the
   error code first, depending on alignment. Previously it always put
   the error code first and used a padding array.
 * LLVM: lower functions which have an empty error set as the return
   type the same as anyerror, so that they can be used where
   fn()anyerror function pointers are expected. In such functions, Zig
   will lower ret to returning zero instead of void.

As a result, one more behavior test is passing.
Based on the size of the payload the native backends will lower
the error union with its fields (errorset & payload) in the correct order.
e.g. ErrorA!u8 will first lower the error set's value and then the payload.
In the event of ErrorA!u32 will lower the payload first.
I was able to get the backend implementation working on LLVM and the C
backend, but I'm going to ask for some help on the other backends.
This was disabled for macOS but I just tested it on my M1 and it works
fine.
     llvmType -> lowerType
genTypedValue -> lowerValue
 * `?E` where E is an error set with only one field now lowers the same
   as `bool`.
 * Fix implementation of errUnionErrOffset and errUnionPayloadOffset to
   properly compute the offset of each field. Also name them the same
   as the corresponding LLVM functions and have the same function
   signature, to avoid confusion. This fixes a bug where wasm was
   passing the error union type instead of the payload type.
 * Fix C backend handling of optionals with zero-bit payload types.
 * C backend: separate out airOptionalPayload and airOptionalPayloadPtr
   which reduces branching and cleans up control flow.
 * Make Type.isNoReturn return true for error sets with no fields.
 * Make `?error{}` have only one possible value (null).
This is needed because pointers to zero-bit types are not necessarily
comptime known, but when doing a load, only the element type having one
possible value is relevant.
@andrewrk andrewrk merged commit 8373520 into master May 25, 2022
@andrewrk andrewrk deleted the empty-error-sets branch May 25, 2022 07:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants