Improved jl_unw_stepn: skip internal frames, don't adjust ip for sign…

…al frames

Add an option to jl_unw_stepn to avoid adjusting the instruction
pointer when we know the cursor derives from a signal frame. (We could
alternatively try to do this with unw_is_signal_frame, but that
wouldn't work for windows and would be an extra function call for each
jl_unw_step which seems a bit unnecessary when we already know the top
frame is the signal frame.)

Also generalize skipping of the first few backtrace frames as needed to
hide the internal backtrace machinery itself: Add a `skip` option to
jl_unw_stepn/jl_backtrace_from_here and use this from backtrace() as
well as rec_backtrace, etc. As part of this, move the workaround for
32-bit windows missing the first frame into the backtrace internals.

base/stacktraces.jl

@@ -155,27 +155,12 @@ end
 Get a backtrace object for the current program point.
 """
 function Base.backtrace()
-    bt, bt2 = ccall(:jl_backtrace_from_here, Any, (Int32,), false)
-    if length(bt) > 2
-        # remove frames for jl_backtrace_from_here and backtrace()
-        if bt[2] == Ptr{Cvoid}(-1%UInt)
-            # backtrace() is interpreted
-            # Note: win32 is missing the top frame (see https://bugs.chromium.org/p/crashpad/issues/detail?id=53)
-            @static if Base.Sys.iswindows() && Int === Int32
-                deleteat!(bt, 1:2)
-            else
-                deleteat!(bt, 1:3)
-            end
-            pushfirst!(bt2)
-        else
-            @static if Base.Sys.iswindows() && Int === Int32
-                deleteat!(bt, 1)
-            else
-                deleteat!(bt, 1:2)
-            end
-        end
-    end
-    return Base._reformat_bt(bt, bt2)
+    # skip frame for backtrace(). Note that for this to work properly,
+    # backtrace() itself must not be interpreted or some additional interpreter
+    # frames will interpose.
+    skip = 1
+    bt1, bt2 = ccall(:jl_backtrace_from_here, Any, (Cint,Cint), false, skip)
+    Base._reformat_bt(bt1, bt2)
 end
 
 """

src/gf.c

@@ -1714,7 +1714,7 @@ static void JL_NORETURN jl_method_error_bare(jl_function_t *f, jl_value_t *args,
         jl_static_show((JL_STREAM*)STDERR_FILENO,(jl_value_t*)f); jl_printf((JL_STREAM*)STDERR_FILENO," world %u\n", (unsigned)world);
         jl_static_show((JL_STREAM*)STDERR_FILENO,args); jl_printf((JL_STREAM*)STDERR_FILENO,"\n");
         jl_ptls_t ptls = jl_get_ptls_states();
-        ptls->bt_size = rec_backtrace(ptls->bt_data, JL_MAX_BT_SIZE);
+        ptls->bt_size = rec_backtrace(ptls->bt_data, JL_MAX_BT_SIZE, 0);
         jl_critical_error(0, NULL, ptls->bt_data, &ptls->bt_size);
         abort();
     }

src/julia_internal.h

@@ -638,8 +638,11 @@ typedef int bt_cursor_t;
 #define JL_BT_INTERP_FRAME (((uintptr_t)0)-1)
 // Maximum number of elements of bt_data taken up by interpreter frame
 #define JL_BT_MAX_ENTRY_SIZE 3
-size_t rec_backtrace(uintptr_t *bt_data, size_t maxsize) JL_NOTSAFEPOINT;
-size_t rec_backtrace_ctx(uintptr_t *bt_data, size_t maxsize, bt_context_t *ctx, int add_interp_frames) JL_NOTSAFEPOINT;
+size_t rec_backtrace(uintptr_t *bt_data, size_t maxsize, int skip) JL_NOTSAFEPOINT;
+// Record backtrace from a signal handler. `ctx` is the context of the code
+// which was asynchronously interrupted.
+size_t rec_backtrace_ctx(uintptr_t *bt_data, size_t maxsize, bt_context_t *ctx,
+                         int add_interp_frames) JL_NOTSAFEPOINT;
 #ifdef LIBOSXUNWIND
 size_t rec_backtrace_ctx_dwarf(uintptr_t *bt_data, size_t maxsize, bt_context_t *ctx, int add_interp_frames) JL_NOTSAFEPOINT;
 #endif

src/stackwalk.c

@@ -29,19 +29,25 @@ static int jl_unw_step(bt_cursor_t *cursor, uintptr_t *ip, uintptr_t *sp, uintpt
 
 // Record backtrace entries into bt_data by stepping cursor with jl_unw_step
 // until the outermost frame is encountered or the buffer bt_data is (close to)
-// full. Native instruction pointers are adjusted to point to the address of
-// the call instruction.
+// full. Returned instruction pointers are adjusted to point to the address of
+// the call instruction. The first `skip` frames are not included in `bt_data`.
 //
 // `maxsize` is the size of the buffer `bt_data` (and `sp` if non-NULL). It
 // must be at least JL_BT_MAX_ENTRY_SIZE to accommodate extended backtrace
 // entries.  If `sp != NULL`, the stack pointer corresponding `bt_data[i]` is
 // stored in `sp[i]`.
 //
+// Flag `add_interp_frames==1` should be set to record an extended backtrace
+// entries in `bt_data` for each julia interpreter frame.
+//
+// Flag `from_signal_handler==1` should be set if the cursor was obtained by
+// asynchronously interrupting the code.
+//
 // jl_unw_stepn will return 1 if there are more frames to come. The number of
-// elements of bt_data (and sp if non-NULL) which were used are returned in
-// bt_size.
+// elements written to bt_data (and sp if non-NULL) are returned in bt_size.
 int jl_unw_stepn(bt_cursor_t *cursor, uintptr_t *bt_data, size_t *bt_size,
-                 uintptr_t *sp, size_t maxsize, int add_interp_frames) JL_NOTSAFEPOINT
+                 uintptr_t *sp, size_t maxsize, int skip, int add_interp_frames,
+                 int from_signal_handler) JL_NOTSAFEPOINT
 {
     jl_ptls_t ptls = jl_get_ptls_states();
     volatile size_t n = 0;
@@ -52,50 +58,65 @@ int jl_unw_stepn(bt_cursor_t *cursor, uintptr_t *bt_data, size_t *bt_size,
 #if defined(_OS_WINDOWS_) && !defined(_CPU_X86_64_)
     assert(!jl_in_stackwalk);
     jl_in_stackwalk = 1;
+    if (!from_signal_handler) {
+        // Workaround 32-bit windows bug missing top frame
+        // See for example https://bugs.chromium.org/p/crashpad/issues/detail?id=53
+        skip--;
+    }
 #endif
 #if !defined(_OS_WINDOWS_)
     jl_jmp_buf *old_buf = ptls->safe_restore;
     jl_jmp_buf buf;
     if (!jl_setjmp(buf, 0)) {
         ptls->safe_restore = &buf;
 #endif
-        while (1) {
+        int have_more_frames = 1;
+        while (have_more_frames) {
             if (n + JL_BT_MAX_ENTRY_SIZE > maxsize) {
                 // Postpone advancing the cursor: may need more space
                 need_more_space = 1;
                 break;
             }
-            int have_more_frames = jl_unw_step(cursor, &return_ip, &thesp, &thefp);
+            have_more_frames = jl_unw_step(cursor, &return_ip, &thesp, &thefp);
+            if (skip > 0) {
+                skip--;
+                continue;
+            }
             if (sp)
                 sp[n] = thesp;
-            // ARM instruction pointer encoding uses the low bit as a flag for
-            // thumb mode, which must be cleared before further use. (Note not
-            // needed for ARM AArch64.) See
-            // https://github.com/libunwind/libunwind/pull/131
-            #ifdef _CPU_ARM_
-            return_ip &= ~(uintptr_t)0x1;
-            #endif
             // For the purposes of looking up debug info for functions, we want
             // to harvest addresses for the *call* instruction `call_ip` during
             // stack walking.  However, this information isn't directly
             // available. Instead, the stack walk discovers the address
             // `return_ip` which would be *returned to* as the stack is
             // unwound.
             //
-            // To infer `call_ip` in full generality we would need to
-            // understand each platform ABI instruction pointer encoding and
-            // calling conventions, noting that these may vary per stack frame.
-            // (For example signal frames on linux x86_64 have `call_ip ==
-            // return_ip`.)
-            //
-            // However for our current purposes it seems sufficient to assume
-            // that `call_ip = return_ip-1`. See also:
+            // To infer `call_ip` in full generality we need to understand each
+            // platform ABI instruction pointer encoding and calling
+            // conventions, noting that the latter may vary per stack frame.
             //
+            // See also:
             // * The LLVM unwinder functions step() and setInfoBasedOnIPRegister()
             //   https://github.com/llvm/llvm-project/blob/master/libunwind/src/UnwindCursor.hpp
             // * The way that libunwind handles it in `unw_get_proc_name`:
             //   https://lists.nongnu.org/archive/html/libunwind-devel/2014-06/msg00025.html
-            uintptr_t call_ip = return_ip - 1;
+            uintptr_t call_ip = return_ip;
+            // ARM instruction pointer encoding uses the low bit as a flag for
+            // thumb mode, which must be cleared before further use. (Note not
+            // needed for ARM AArch64.) See
+            // https://github.com/libunwind/libunwind/pull/131
+            #ifdef _CPU_ARM_
+            call_ip &= ~(uintptr_t)0x1;
+            #endif
+            // Now there's two main cases to adjust for:
+            // * Normal stack frames where compilers emit a `call` instruction
+            //   which we can get from the return address via `call_ip = return_ip - 1`.
+            // * Code which was interrupted asynchronously (eg, via a signal)
+            //   is expected to have `call_ip == return_ip`.
+            if (n != 0 || !from_signal_handler) {
+                // normal frame
+                call_ip -= 1;
+            }
             if (call_ip == JL_BT_INTERP_FRAME) {
                 // Never leave special marker in the bt data as it can corrupt the GC.
                 call_ip = 0;
@@ -109,8 +130,6 @@ int jl_unw_stepn(bt_cursor_t *cursor, uintptr_t *bt_data, size_t *bt_size,
                 *bt_entry = call_ip;
                 n++;
             }
-            if (!have_more_frames)
-                break;
         }
 #if !defined(_OS_WINDOWS_)
     }
@@ -131,26 +150,44 @@ int jl_unw_stepn(bt_cursor_t *cursor, uintptr_t *bt_data, size_t *bt_size,
 }
 
 size_t rec_backtrace_ctx(uintptr_t *bt_data, size_t maxsize,
-                         bt_context_t *context, int add_interp_frames)
+                         bt_context_t *context, int add_interp_frames) JL_NOTSAFEPOINT
 {
-    size_t bt_size = 0;
     bt_cursor_t cursor;
     if (!jl_unw_init(&cursor, context))
         return 0;
-    jl_unw_stepn(&cursor, bt_data, &bt_size, NULL, maxsize, add_interp_frames);
+    size_t bt_size = 0;
+    jl_unw_stepn(&cursor, bt_data, &bt_size, NULL, maxsize, 0, add_interp_frames, 1);
     return bt_size;
 }
 
-size_t rec_backtrace(uintptr_t *bt_data, size_t maxsize)
+// Record backtrace into buffer `bt_data`, using a maximum of `maxsize`
+// elements, and returning the number of elements written.
+//
+// The first `skip` frames are omitted, in addition to omitting the frame from
+// `rec_backtrace` itself.
+size_t rec_backtrace(uintptr_t *bt_data, size_t maxsize, int skip)
 {
     bt_context_t context;
     memset(&context, 0, sizeof(context));
     jl_unw_get(&context);
-    return rec_backtrace_ctx(bt_data, maxsize, &context, 1);
+    bt_cursor_t cursor;
+    if (!jl_unw_init(&cursor, &context))
+        return 0;
+    size_t bt_size = 0;
+    jl_unw_stepn(&cursor, bt_data, &bt_size, NULL, maxsize, skip + 1, 1, 0);
+    return bt_size;
 }
 
 static jl_value_t *array_ptr_void_type JL_ALWAYS_LEAFTYPE = NULL;
-JL_DLLEXPORT jl_value_t *jl_backtrace_from_here(int returnsp)
+// Return backtrace information as an svec of (bt1, bt2, [sp])
+//
+// The stack pointers `sp` are returned only when `returnsp` evaluates to true.
+// bt1 contains raw backtrace entries, while bt2 exists to root any julia
+// objects associated with the entries in bt1.
+//
+// The frame from jl_backtrace_from_here will be skipped; set `skip > 0` to
+// skip additional native frames from the start of the backtrace.
+JL_DLLEXPORT jl_value_t *jl_backtrace_from_here(int returnsp, int skip)
 {
     jl_array_t *ip = NULL;
     jl_array_t *sp = NULL;
@@ -168,25 +205,26 @@ JL_DLLEXPORT jl_value_t *jl_backtrace_from_here(int returnsp)
     memset(&context, 0, sizeof(context));
     jl_unw_get(&context);
     if (jl_unw_init(&cursor, &context)) {
+        // Skip frame for jl_backtrace_from_here itself
+        skip += 1;
         size_t offset = 0;
-        while (1) {
+        int have_more_frames = 1;
+        while (have_more_frames) {
             jl_array_grow_end(ip, maxincr);
             uintptr_t *sp_ptr = NULL;
             if (returnsp) {
                 sp_ptr = (uintptr_t*)jl_array_data(sp) + offset;
                 jl_array_grow_end(sp, maxincr);
             }
             size_t size_incr = 0;
-            int need_more_space = jl_unw_stepn(&cursor, (uintptr_t*)jl_array_data(ip) + offset,
-                                               &size_incr, sp_ptr, maxincr, 1);
+            have_more_frames = jl_unw_stepn(&cursor, (uintptr_t*)jl_array_data(ip) + offset,
+                                            &size_incr, sp_ptr, maxincr, skip, 1, 0);
+            skip = 0;
             offset += size_incr;
-            if (!need_more_space) {
-                jl_array_del_end(ip, jl_array_len(ip) - offset);
-                if (returnsp)
-                    jl_array_del_end(sp, jl_array_len(sp) - offset);
-                break;
-            }
         }
+        jl_array_del_end(ip, jl_array_len(ip) - offset);
+        if (returnsp)
+            jl_array_del_end(sp, jl_array_len(sp) - offset);
 
         size_t n = 0;
         while (n < jl_array_len(ip)) {
@@ -491,7 +529,7 @@ size_t rec_backtrace_ctx_dwarf(uintptr_t *bt_data, size_t maxsize,
     bt_cursor_t cursor;
     if (!jl_unw_init_dwarf(&cursor, context))
         return 0;
-    jl_unw_stepn(&cursor, bt_data, &bt_size, NULL, maxsize, add_interp_frames);
+    jl_unw_stepn(&cursor, bt_data, &bt_size, NULL, maxsize, 0, add_interp_frames, 1);
     return bt_size;
 }
 #endif

src/task.c

@@ -237,7 +237,7 @@ JL_DLLEXPORT void *jl_task_stack_buffer(jl_task_t *task, size_t *size, int *tid)
 static void record_backtrace(jl_ptls_t ptls) JL_NOTSAFEPOINT
 {
     // storing bt_size in ptls ensures roots in bt_data will be found
-    ptls->bt_size = rec_backtrace(ptls->bt_data, JL_MAX_BT_SIZE);
+    ptls->bt_size = rec_backtrace(ptls->bt_data, JL_MAX_BT_SIZE, 1);
 }
 
 JL_DLLEXPORT void julia_init(JL_IMAGE_SEARCH rel)