fix coroutine accessing freed memory

andrewrk · andrewrk · commit 2fa588e81d60 · 2018-06-27T18:45:21.000-04:00
closes #1164
diff --git a/src/analyze.cpp b/src/analyze.cpp
@@ -5583,7 +5583,7 @@ void render_const_val_ptr(CodeGen *g, Buf *buf, ConstExprValue *const_val, TypeT
                 return;
             }
         case ConstPtrSpecialHardCodedAddr:
-            buf_appendf(buf, "(*%s)(%" ZIG_PRI_x64 ")", buf_ptr(&type_entry->data.pointer.child_type->name),
+            buf_appendf(buf, "(%s)(%" ZIG_PRI_x64 ")", buf_ptr(&type_entry->name),
                     const_val->data.x_ptr.data.hard_coded_addr.addr);
             return;
         case ConstPtrSpecialDiscard:
diff --git a/src/ir.cpp b/src/ir.cpp
@@ -7112,6 +7112,12 @@ bool ir_gen(CodeGen *codegen, AstNode *node, Scope *scope, IrExecutable *ir_exec
             IrInstruction *dest_err_ret_trace_ptr = ir_build_load_ptr(irb, scope, node, err_ret_trace_ptr_field_ptr);
             ir_build_merge_err_ret_traces(irb, scope, node, coro_promise_ptr, err_ret_trace_ptr, dest_err_ret_trace_ptr);
         }
+        // Before we destroy the coroutine frame, we need to load the target promise into
+        // a register or local variable which does not get spilled into the frame,
+        // otherwise llvm tries to access memory inside the destroyed frame.
+        IrInstruction *unwrapped_await_handle_ptr = ir_build_unwrap_maybe(irb, scope, node,
+                irb->exec->await_handle_var_ptr, false);
+        IrInstruction *await_handle_in_block = ir_build_load_ptr(irb, scope, node, unwrapped_await_handle_ptr);
         ir_build_br(irb, scope, node, check_free_block, const_bool_false);
 
         ir_set_cursor_at_end_and_append_block(irb, irb->exec->coro_final_cleanup_block);
@@ -7126,6 +7132,14 @@ bool ir_gen(CodeGen *codegen, AstNode *node, Scope *scope, IrExecutable *ir_exec
         incoming_values[1] = const_bool_true;
         IrInstruction *resume_awaiter = ir_build_phi(irb, scope, node, 2, incoming_blocks, incoming_values);
 
+        IrBasicBlock **merge_incoming_blocks = allocate<IrBasicBlock *>(2);
+        IrInstruction **merge_incoming_values = allocate<IrInstruction *>(2);
+        merge_incoming_blocks[0] = irb->exec->coro_final_cleanup_block;
+        merge_incoming_values[0] = ir_build_const_undefined(irb, scope, node);
+        merge_incoming_blocks[1] = irb->exec->coro_normal_final;
+        merge_incoming_values[1] = await_handle_in_block;
+        IrInstruction *awaiter_handle = ir_build_phi(irb, scope, node, 2, merge_incoming_blocks, merge_incoming_values);
+
         Buf *free_field_name = buf_create_from_str(ASYNC_FREE_FIELD_NAME);
         IrInstruction *implicit_allocator_ptr = ir_build_get_implicit_allocator(irb, scope, node,
                 ImplicitAllocatorIdLocalVar);
@@ -7152,9 +7166,6 @@ bool ir_gen(CodeGen *codegen, AstNode *node, Scope *scope, IrExecutable *ir_exec
         ir_build_cond_br(irb, scope, node, resume_awaiter, resume_block, irb->exec->coro_suspend_block, const_bool_false);
 
         ir_set_cursor_at_end_and_append_block(irb, resume_block);
-        IrInstruction *unwrapped_await_handle_ptr = ir_build_unwrap_maybe(irb, scope, node,
-                irb->exec->await_handle_var_ptr, false);
-        IrInstruction *awaiter_handle = ir_build_load_ptr(irb, scope, node, unwrapped_await_handle_ptr);
         ir_build_coro_resume(irb, scope, node, awaiter_handle);
         ir_build_br(irb, scope, node, irb->exec->coro_suspend_block, const_bool_false);
     }
diff --git a/test/cases/coroutines.zig b/test/cases/coroutines.zig
@@ -5,7 +5,10 @@ const assert = std.debug.assert;
 var x: i32 = 1;
 
 test "create a coroutine and cancel it" {
-    const p = try async<std.debug.global_allocator> simpleAsyncFn();
+    var da = std.heap.DirectAllocator.init();
+    defer da.deinit();
+
+    const p = try async<&da.allocator> simpleAsyncFn();
     comptime assert(@typeOf(p) == promise->void);
     cancel p;
     assert(x == 2);
@@ -17,8 +20,11 @@ async fn simpleAsyncFn() void {
 }
 
 test "coroutine suspend, resume, cancel" {
+    var da = std.heap.DirectAllocator.init();
+    defer da.deinit();
+
     seq('a');
-    const p = try async<std.debug.global_allocator> testAsyncSeq();
+    const p = try async<&da.allocator> testAsyncSeq();
     seq('c');
     resume p;
     seq('f');
@@ -43,7 +49,10 @@ fn seq(c: u8) void {
 }
 
 test "coroutine suspend with block" {
-    const p = try async<std.debug.global_allocator> testSuspendBlock();
+    var da = std.heap.DirectAllocator.init();
+    defer da.deinit();
+
+    const p = try async<&da.allocator> testSuspendBlock();
     std.debug.assert(!result);
     resume a_promise;
     std.debug.assert(result);
@@ -64,8 +73,11 @@ var await_a_promise: promise = undefined;
 var await_final_result: i32 = 0;
 
 test "coroutine await" {
+    var da = std.heap.DirectAllocator.init();
+    defer da.deinit();
+
     await_seq('a');
-    const p = async<std.debug.global_allocator> await_amain() catch unreachable;
+    const p = async<&da.allocator> await_amain() catch unreachable;
     await_seq('f');
     resume await_a_promise;
     await_seq('i');
@@ -100,8 +112,11 @@ fn await_seq(c: u8) void {
 var early_final_result: i32 = 0;
 
 test "coroutine await early return" {
+    var da = std.heap.DirectAllocator.init();
+    defer da.deinit();
+
     early_seq('a');
-    const p = async<std.debug.global_allocator> early_amain() catch unreachable;
+    const p = async<&da.allocator> early_amain() catch unreachable;
     early_seq('f');
     assert(early_final_result == 1234);
     assert(std.mem.eql(u8, early_points, "abcdef"));
@@ -146,7 +161,9 @@ test "async function with dot syntax" {
             suspend;
         }
     };
-    const p = try async<std.debug.global_allocator> S.foo();
+    var da = std.heap.DirectAllocator.init();
+    defer da.deinit();
+    const p = try async<&da.allocator> S.foo();
     cancel p;
     assert(S.y == 2);
 }
@@ -157,7 +174,9 @@ test "async fn pointer in a struct field" {
         bar: async<*std.mem.Allocator> fn (*i32) void,
     };
     var foo = Foo{ .bar = simpleAsyncFn2 };
-    const p = (async<std.debug.global_allocator> foo.bar(&data)) catch unreachable;
+    var da = std.heap.DirectAllocator.init();
+    defer da.deinit();
+    const p = (async<&da.allocator> foo.bar(&data)) catch unreachable;
     assert(data == 2);
     cancel p;
     assert(data == 4);
@@ -169,7 +188,9 @@ async<*std.mem.Allocator> fn simpleAsyncFn2(y: *i32) void {
 }
 
 test "async fn with inferred error set" {
-    const p = (async<std.debug.global_allocator> failing()) catch unreachable;
+    var da = std.heap.DirectAllocator.init();
+    defer da.deinit();
+    const p = (async<&da.allocator> failing()) catch unreachable;
     resume p;
     cancel p;
 }
@@ -181,7 +202,9 @@ async fn failing() !void {
 test "error return trace across suspend points - early return" {
     const p = nonFailing();
     resume p;
-    const p2 = try async<std.debug.global_allocator> printTrace(p);
+    var da = std.heap.DirectAllocator.init();
+    defer da.deinit();
+    const p2 = try async<&da.allocator> printTrace(p);
     cancel p2;
 }
 

Original file line number	Diff line number	Diff line change
`@@ -5583,7 +5583,7 @@ void render_const_val_ptr(CodeGen g, Buf buf, ConstExprValue *const_val, TypeT`
`5583`	`5583`	`return;`
`5584`	`5584`	`}`
`5585`	`5585`	`case ConstPtrSpecialHardCodedAddr:`
`5586`		`- buf_appendf(buf, "(*%s)(%" ZIG_PRI_x64 ")", buf_ptr(&type_entry->data.pointer.child_type->name),`
	`5586`	`+ buf_appendf(buf, "(%s)(%" ZIG_PRI_x64 ")", buf_ptr(&type_entry->name),`
`5587`	`5587`	`const_val->data.x_ptr.data.hard_coded_addr.addr);`
`5588`	`5588`	`return;`
`5589`	`5589`	`case ConstPtrSpecialDiscard:`