From 60b2031831320186f3920d63cfa35bda40930450 Mon Sep 17 00:00:00 2001
From: Andrew Kelley <superjoe30@gmail.com>
Date: Fri, 9 Mar 2018 22:06:24 -0500
Subject: [PATCH] improvements to stack traces

 * @panic generates an error return trace
 * printing an error return trace no longer interferes with
   normal stack traces.
 * instead of ignore_frame_count, we look at the return address
   when you call panic, and that's the first stack trace function
   makes stack traces much cleaner - the error return trace
   flows gracefully into the stack trace
---
 src/codegen.cpp       |  4 ++--
 std/debug/index.zig   | 39 +++++++++++++++++++++++++++++----------
 std/special/panic.zig |  5 +++--
 3 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/src/codegen.cpp b/src/codegen.cpp
index 4a7b8f628f..f4275243c3 100644
--- a/src/codegen.cpp
+++ b/src/codegen.cpp
@@ -3299,8 +3299,8 @@ static LLVMValueRef ir_render_align_cast(CodeGen *g, IrExecutable *executable, I
 static LLVMValueRef ir_render_error_return_trace(CodeGen *g, IrExecutable *executable,
         IrInstructionErrorReturnTrace *instruction)
 {
-    TypeTableEntry *ptr_to_stack_trace_type = get_ptr_to_stack_trace_type(g);
     if (g->cur_err_ret_trace_val == nullptr) {
+        TypeTableEntry *ptr_to_stack_trace_type = get_ptr_to_stack_trace_type(g);
         return LLVMConstNull(ptr_to_stack_trace_type->type_ref);
     }
     return g->cur_err_ret_trace_val;
@@ -3925,7 +3925,7 @@ static LLVMValueRef ir_render_container_init_list(CodeGen *g, IrExecutable *exec
 }
 
 static LLVMValueRef ir_render_panic(CodeGen *g, IrExecutable *executable, IrInstructionPanic *instruction) {
-    gen_panic(g, ir_llvm_value(g, instruction->msg), nullptr);
+    gen_panic(g, ir_llvm_value(g, instruction->msg), g->cur_err_ret_trace_val);
     return nullptr;
 }
 
diff --git a/std/debug/index.zig b/std/debug/index.zig
index 147f9b7713..b20b8f6e5e 100644
--- a/std/debug/index.zig
+++ b/std/debug/index.zig
@@ -9,6 +9,7 @@ const macho = std.macho;
 const ArrayList = std.ArrayList;
 const builtin = @import("builtin");
 
+pub var stack_trace_start_address: ?usize = null;
 pub const FailingAllocator = @import("failing_allocator.zig").FailingAllocator;
 
 /// Tries to write to stderr, unbuffered, and ignores any error returned.
@@ -51,8 +52,7 @@ pub fn dumpCurrentStackTrace() void {
         stderr.print("Unable to dump stack trace: Unable to open debug info: {}\n", @errorName(err)) catch return;
         return;
     };
-    defer debug_info.close();
-    writeCurrentStackTrace(stderr, global_allocator, debug_info, stderr_file.isTty(), 1) catch |err| {
+    writeCurrentStackTrace(stderr, global_allocator, debug_info, stderr_file.isTty()) catch |err| {
         stderr.print("Unable to dump stack trace: {}\n", @errorName(err)) catch return;
         return;
     };
@@ -65,7 +65,6 @@ pub fn dumpStackTrace(stack_trace: &const builtin.StackTrace) void {
         stderr.print("Unable to dump stack trace: Unable to open debug info: {}\n", @errorName(err)) catch return;
         return;
     };
-    defer debug_info.close();
     writeStackTrace(stack_trace, stderr, global_allocator, debug_info, stderr_file.isTty()) catch |err| {
         stderr.print("Unable to dump stack trace: {}\n", @errorName(err)) catch return;
         return;
@@ -162,18 +161,38 @@ pub fn writeStackTrace(stack_trace: &const builtin.StackTrace, out_stream: var,
 }
 
 pub fn writeCurrentStackTrace(out_stream: var, allocator: &mem.Allocator,
-    debug_info: &ElfStackTrace, tty_color: bool, ignore_frame_count: usize) !void
+    debug_info: &ElfStackTrace, tty_color: bool) !void
 {
-    var ignored_count: usize = 0;
+    const AddressState = union(enum) {
+        NotLookingForStartAddress,
+        LookingForStartAddress: usize,
+        FoundStartAddress,
+    };
+    // TODO: I want to express like this:
+    //var addr_state = if (stack_trace_start_address) |addr| AddressState { .LookingForStartAddress = addr }
+    //    else AddressState.NotLookingForStartAddress;
+    var addr_state: AddressState = undefined;
+    if (stack_trace_start_address) |addr| {
+        addr_state = AddressState { .LookingForStartAddress = addr };
+    } else {
+        addr_state = AddressState.NotLookingForStartAddress;
+    }
 
     var fp = @ptrToInt(@frameAddress());
     while (fp != 0) : (fp = *@intToPtr(&const usize, fp)) {
-        if (ignored_count < ignore_frame_count) {
-            ignored_count += 1;
-            continue;
-        }
-
         const return_address = *@intToPtr(&const usize, fp + @sizeOf(usize));
+
+        switch (addr_state) {
+            AddressState.NotLookingForStartAddress => continue,
+            AddressState.LookingForStartAddress => |addr| {
+                if (return_address == addr) {
+                    addr_state = AddressState.FoundStartAddress;
+                } else {
+                    continue;
+                }
+            },
+            AddressState.FoundStartAddress => {},
+        }
         try printSourceAtAddress(debug_info, out_stream, return_address);
     }
 }
diff --git a/std/special/panic.zig b/std/special/panic.zig
index cfefa8e839..4459382f9b 100644
--- a/std/special/panic.zig
+++ b/std/special/panic.zig
@@ -14,10 +14,11 @@ pub fn panic(msg: []const u8, error_return_trace: ?&builtin.StackTrace) noreturn
             while (true) {}
         },
         else => {
+            std.debug.stack_trace_start_address = @ptrToInt(@returnAddress());
             if (error_return_trace) |trace| {
-                @import("std").debug.panicWithTrace(trace, "{}", msg);
+                std.debug.panicWithTrace(trace, "{}", msg);
             }
-            @import("std").debug.panic("{}", msg);
+            std.debug.panic("{}", msg);
         },
     }
 }