Merge pull request #33380 from JuliaLang/cjf/bt-lookup-offset

Clean up return->call address translation for backtraces

base/client.jl

@@ -81,8 +81,7 @@ end
 
 # deprecated function--preserved for DocTests.jl
 function ip_matches_func(ip, func::Symbol)
-    ip isa InterpreterIP || (ip -= 1)
-    for fr in StackTraces.lookupat(ip)
+    for fr in StackTraces.lookup(ip)
         if fr === StackTraces.UNKNOWN || fr.from_c
             return false
         end

base/deprecated.jl

@@ -102,8 +102,7 @@ function firstcaller(bt::Vector, funcsyms)
     # Identify the calling line
     found = false
     for ip in bt
-        ip isa Base.InterpreterIP || (ip -= 1) # convert from return stack to call stack (for inlining info)
-        lkups = StackTraces.lookupat(ip)
+        lkups = StackTraces.lookup(ip)
         for lkup in lkups
             if lkup == StackTraces.UNKNOWN || lkup.from_c
                 continue

base/error.jl

@@ -85,7 +85,19 @@ function _reformat_bt(bt, bt2)
     ret
 end
 
-function backtrace end
+"""
+    backtrace()
+
+Get a backtrace object for the current program point.
+"""
+function backtrace()
+    @_noinline_meta
+    # skip frame for backtrace(). Note that for this to work properly,
+    # backtrace() itself must not be interpreted nor inlined.
+    skip = 1
+    bt1, bt2 = ccall(:jl_backtrace_from_here, Any, (Cint,Cint), false, skip)
+    _reformat_bt(bt1, bt2)
+end
 
 """
     catch_backtrace()

base/errorshow.jl

@@ -615,10 +615,8 @@ function process_backtrace(t::Vector, limit::Int=typemax(Int); skipC = true)
         lkups = t[i]
         if lkups isa StackFrame
             lkups = [lkups]
-        elseif lkups isa Base.InterpreterIP
-            lkups = StackTraces.lookupat(lkups)
         else
-            lkups = StackTraces.lookupat(lkups - 1)
+            lkups = StackTraces.lookup(lkups)
         end
         for lkup in lkups
             if lkup === StackTraces.UNKNOWN
@@ -665,3 +663,14 @@ function show_exception_stack(io::IO, stack::Vector)
         println(io)
     end
 end
+
+# Defined here rather than error.jl for bootstrap ordering
+function show(io::IO, ip::InterpreterIP)
+    print(io, typeof(ip))
+    if ip.code isa Core.CodeInfo
+        print(io, " in top-level CodeInfo at statement $(Int(ip.stmt))")
+    else
+        print(io, " in $(ip.code) at statement $(Int(ip.stmt))")
+    end
+end
+

base/stacktraces.jl

@@ -96,13 +96,13 @@ end
 
 
 """
-    lookupat(pointer::Union{Ptr{Cvoid}, UInt}) -> Vector{StackFrame}
+    lookup(pointer::Ptr{Cvoid}) -> Vector{StackFrame}
 
 Given a pointer to an execution context (usually generated by a call to `backtrace`), looks
 up stack frame context information. Returns an array of frame information for all functions
 inlined at that point, innermost function first.
 """
-function lookupat(pointer::Ptr{Cvoid})
+function lookup(pointer::Ptr{Cvoid})
     infos = ccall(:jl_lookup_code_address, Any, (Ptr{Cvoid}, Cint), pointer, false)
     pointer = convert(UInt64, pointer)
     isempty(infos) && return [StackFrame(empty_sym, empty_sym, -1, nothing, true, false, pointer)] # this is equal to UNKNOWN
@@ -117,7 +117,7 @@ end
 
 const top_level_scope_sym = Symbol("top-level scope")
 
-function lookupat(ip::Base.InterpreterIP)
+function lookup(ip::Base.InterpreterIP)
     if ip.code isa Core.MethodInstance && ip.code.def isa Method
         codeinfo = ip.code.uninferred
         func = ip.code.def.name
@@ -149,35 +149,6 @@ function lookupat(ip::Base.InterpreterIP)
     return scopes
 end
 
-"""
-    backtrace()
-
-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)
-end
-
 """
     stacktrace([trace::Vector{Ptr{Cvoid}},] [c_funcs::Bool=false]) -> StackTrace
 
@@ -188,8 +159,7 @@ trace, `stacktrace` first calls `backtrace`.
 function stacktrace(trace::Vector{<:Union{Base.InterpreterIP,Ptr{Cvoid}}}, c_funcs::Bool=false)
     stack = StackTrace()
     for ip in trace
-        ip isa Base.InterpreterIP || (ip -= 1) # convert from return stack to call stack (for inlining info)
-        for frame in lookupat(ip)
+        for frame in lookup(ip)
             # Skip frames that come from C calls.
             if c_funcs || !frame.from_c
                 push!(stack, frame)

doc/src/base/stacktraces.md

@@ -7,9 +7,9 @@ Base.StackTraces.stacktrace
 ```
 
 The following methods and types in `Base.StackTraces` are not exported and need to be called e.g.
-as `StackTraces.lookupat(ptr)`.
+as `StackTraces.lookup(ptr)`.
 
 ```@docs
-Base.StackTraces.lookupat
+Base.StackTraces.lookup
 Base.StackTraces.remove_frames!
 ```

doc/src/manual/stacktraces.md

@@ -300,12 +300,12 @@ julia> stacktrace(trace, true)
 ```
 
 Individual pointers returned by [`backtrace`](@ref) can be translated into [`StackTraces.StackFrame`](@ref)
-s by passing them into [`StackTraces.lookupat`](@ref):
+s by passing them into [`StackTraces.lookup`](@ref):
 
 ```julia-repl
 julia> pointer = backtrace()[1];
 
-julia> frame = StackTraces.lookupat(pointer - 1)
+julia> frame = StackTraces.lookup(pointer)
 1-element Array{Base.StackTraces.StackFrame,1}:
  jl_apply_generic at gf.c:2167
 

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/interpreter-stacktrace.c

@@ -397,13 +397,14 @@ JL_DLLEXPORT size_t jl_capture_interp_frame(uintptr_t *data, uintptr_t sp, uintp
 #else
     interpreter_state *s = (interpreter_state *)(sp+TOTAL_STACK_PADDING);
 #endif
-    if (space_remaining <= 1)
+    int required_space = 3;
+    if (space_remaining < required_space)
         return 0;
     // Sentinel value to indicate an interpreter frame
     data[0] = JL_BT_INTERP_FRAME;
     data[1] = s->mi ? (uintptr_t)s->mi : s->src ? (uintptr_t)s->src : (uintptr_t)jl_nothing;
     data[2] = (uintptr_t)s->ip;
-    return 2;
+    return required_space;
 }
 
 extern void * CALLBACK_ABI enter_interpreter_frame(void * CALLBACK_ABI (*callback)(interpreter_state *, void *), void *arg);
@@ -420,6 +421,7 @@ JL_DLLEXPORT int jl_is_enter_interpreter_frame(uintptr_t ip)
 
 JL_DLLEXPORT size_t jl_capture_interp_frame(uintptr_t *data, uintptr_t sp, uintptr_t fp, size_t space_remaining)
 {
+    // Leave bt_entry[0] as the native instruction ptr
     return 0;
 }
 #define CALLBACK_ABI

src/julia_internal.h

@@ -636,10 +636,15 @@ typedef int bt_cursor_t;
 #endif
 // Special marker in backtrace data for encoding interpreter frames
 #define JL_BT_INTERP_FRAME (((uintptr_t)0)-1)
-size_t rec_backtrace(uintptr_t *data, size_t maxsize) JL_NOTSAFEPOINT;
-size_t rec_backtrace_ctx(uintptr_t *data, size_t maxsize, bt_context_t *ctx, int add_interp_frames) JL_NOTSAFEPOINT;
+// 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, 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 *data, size_t maxsize, bt_context_t *ctx, int add_interp_frames) JL_NOTSAFEPOINT;
+size_t rec_backtrace_ctx_dwarf(uintptr_t *bt_data, size_t maxsize, bt_context_t *ctx, int add_interp_frames) JL_NOTSAFEPOINT;
 #endif
 JL_DLLEXPORT void jl_get_backtrace(jl_array_t **bt, jl_array_t **bt2);
 void jl_critical_error(int sig, bt_context_t *context, uintptr_t *bt_data, size_t *bt_size);