Merge pull request JuliaLang#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);