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

compiler segfaults with async fn call with tuple after integer parameter #4403

Closed
andrewrk opened this issue Feb 6, 2020 · 4 comments · Fixed by #4404
Closed

compiler segfaults with async fn call with tuple after integer parameter #4403

andrewrk opened this issue Feb 6, 2020 · 4 comments · Fixed by #4404
Labels
bug Observed behavior contradicts documented or intended behavior stage1 The process of building from source via WebAssembly and the C backend.
Milestone

Comments

@andrewrk
Copy link
Member

andrewrk commented Feb 6, 2020

const std = @import("std");

export fn foo() void {
    _ = async bar(1, .{});
}

fn bar(ptr: i32, args: var) anyerror!void {
    suspend;
}

pub fn panic(msg: []const u8, stack_trace: ?*const std.builtin.StackTrace) noreturn {
    @breakpoint();
    unreachable;
}
@andrewrk andrewrk added bug Observed behavior contradicts documented or intended behavior stage1 The process of building from source via WebAssembly and the C backend. labels Feb 6, 2020
@andrewrk andrewrk added this to the 0.6.0 milestone Feb 6, 2020
@andrewrk
Copy link
Member Author

andrewrk commented Feb 6, 2020

fix:

--- a/src/codegen.cpp
+++ b/src/codegen.cpp
@@ -365,7 +365,7 @@ static uint32_t frame_index_arg(CodeGen *g, ZigType *return_type) {
 static uint32_t frame_index_trace_stack(CodeGen *g, FnTypeId *fn_type_id) {
     uint32_t result = frame_index_arg(g, fn_type_id->return_type);
     for (size_t i = 0; i < fn_type_id->param_count; i += 1) {
-        if (type_has_bits(fn_type_id->param_info->type)) {
+        if (type_has_bits(fn_type_id->param_info[i].type)) {
             result += 1;
         }
     }

@andrewrk
Copy link
Member Author

andrewrk commented Feb 6, 2020

Next problem:

const std = @import("std");

export fn foo() void {
    var a: u128 = 1;
    _ = async bar(undefined, .{a});
}

fn bar(ptr: *i32, args: var) anyerror!void {
    suspend;
}

pub fn panic(msg: []const u8, stack_trace: ?*const std.builtin.StackTrace) noreturn {
    @breakpoint();
    unreachable;
}

@andrewrk
Copy link
Member Author

andrewrk commented Feb 6, 2020

analyze.cpp has this logic:

            if (next_offset > llvm_next_offset) {
                size_t pad_bytes = next_offset - (field->offset + LLVMStoreSizeOfType(g->target_data_ref, llvm_type));
                if (pad_bytes != 0) {
                    LLVMTypeRef pad_llvm_type = LLVMArrayType(LLVMInt8Type(), pad_bytes);
                    element_types[gen_field_index] = pad_llvm_type;
                    gen_field_index += 1;
                }
            }

codegen.cpp has this logic:

static uint32_t frame_index_trace_stack(CodeGen *g, FnTypeId *fn_type_id) {
    uint32_t result = frame_index_arg(g, fn_type_id->return_type);
    for (size_t i = 0; i < fn_type_id->param_count; i += 1) {
        if (type_has_bits(fn_type_id->param_info[i].type)) {
            result += 1;
        }
    }
    return result;
}

This is a regression from when structs gained the ability to have padding beyond what LLVM provides.

@andrewrk
Copy link
Member Author

andrewrk commented Feb 6, 2020

fix:

--- a/src/codegen.cpp
+++ b/src/codegen.cpp
@@ -362,14 +362,16 @@ static uint32_t frame_index_arg(CodeGen *g, ZigType *return_type) {
 }
 
 // label (grep this): [fn_frame_struct_layout]
-static uint32_t frame_index_trace_stack(CodeGen *g, FnTypeId *fn_type_id) {
-    uint32_t result = frame_index_arg(g, fn_type_id->return_type);
-    for (size_t i = 0; i < fn_type_id->param_count; i += 1) {
-        if (type_has_bits(fn_type_id->param_info->type)) {
-            result += 1;
-        }
+static uint32_t frame_index_trace_stack(CodeGen *g, ZigFn *fn) {
+    size_t field_index = 6;
+    bool have_stack_trace = codegen_fn_has_err_ret_tracing_arg(g, fn->type_entry->data.fn.fn_type_id.return_type);
+    if (have_stack_trace) {
+        field_index += 2;
     }
-    return result;
+    field_index += fn->type_entry->data.fn.fn_type_id.param_count;
+    ZigType *locals_struct = fn->frame_type->data.frame.locals_struct;
+    TypeStructField *field = locals_struct->data.structure.fields[field_index];
+    return field->gen_index;
 }
 
 
@@ -7742,7 +7744,7 @@ static void do_code_gen(CodeGen *g) {
             }
             uint32_t trace_field_index_stack = UINT32_MAX;
             if (codegen_fn_has_err_ret_tracing_stack(g, fn_table_entry, true)) {
-                trace_field_index_stack = frame_index_trace_stack(g, fn_type_id);
+                trace_field_index_stack = frame_index_trace_stack(g, fn_table_entry);
                 g->cur_err_ret_trace_val_stack = LLVMBuildStructGEP(g->builder, g->cur_frame_ptr,
                         trace_field_index_stack, "");
             }

andrewrk added a commit that referenced this issue Feb 6, 2020
 * `zig test` gainst `--test-evented-io` parameter and gains the ability
   to seamlessly run async tests.
 * `std.ChildProcess` opens its child process pipe with O_NONBLOCK when
   using evented I/O
 * `std.io.getStdErr()` gives a File that is blocking even in evented
   I/O mode.
 * Delete `std.event.fs`. The functionality is now merged into `std.fs`
   and async file system access (using a dedicated thread) is
   automatically handled.
 * `std.fs.File` can be configured to specify whether its handle is
   expected to block, and whether that is OK to block even when in
   async I/O mode. This makes async I/O work correctly for e.g. the
   file system as well as network.
 * `std.fs.File` has some deprecated functions removed.
 * Missing readv,writev,pread,pwrite,preadv,pwritev functions are added
   to `std.os` and `std.fs.File`. They are all integrated with async
   I/O.
 * `std.fs.Watch` is still bit rotted and needs to be audited in light
   of the new async/await syntax.
 * `std.io.OutStream` integrates with async I/O
 * linked list nodes in the std lib have default `null` values for
   `prev` and `next`.
 * Windows async I/O integration is enabled for reading/writing file
   handles.
 * Added `std.os.mode_t`. Integer sizes need to be audited.
 * Fixed #4403 which was causing compiler to crash.

This is working towards:

./zig test ../test/stage1/behavior.zig --test-evented-io

Which does not successfully build yet. I'd like to enable behavioral
tests and std lib tests with --test-evented-io in the test matrix in the
future, to prevent regressions.
andrewrk added a commit that referenced this issue Feb 7, 2020
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 stage1 The process of building from source via WebAssembly and the C backend.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant