Skip to content

Fix stack pointer retrieval in jl_backtrace_from_here #42585

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 29 commits into from
Nov 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
2635252
Take pointer after grow
tkf Oct 10, 2021
2990b77
Align stack and instruction pointers
tkf Oct 10, 2021
6ca4eb5
Can we just always (up)cast pointers to UInt64?
tkf Oct 10, 2021
ac3468d
Alternative/proper fix; use WORD_SIZE
tkf Oct 10, 2021
458cb89
Actually allocate nbytes
tkf Oct 11, 2021
7a338ae
Skip closure-based tests on ARM etc.
tkf Oct 11, 2021
4eb674d
Don't use closure
tkf Oct 11, 2021
e4a1edd
Pass a function as Any
tkf Oct 11, 2021
fcae973
Further simplify llvmcall
tkf Oct 11, 2021
a935c7a
Revert "Further simplify llvmcall"
tkf Oct 11, 2021
d284a34
Revert "Pass a function as Any"
tkf Oct 11, 2021
b51e198
Move _reformat_sp to test suite and fix the typos
tkf Oct 13, 2021
24c2aa1
Use llvm.frameaddress
tkf Oct 13, 2021
03e5129
Merge branch 'master' into fix-sp
tkf Oct 14, 2021
4fcda37
Use inferencebarrier to be extra sure
tkf Oct 14, 2021
9270661
Revert "Use inferencebarrier to be extra sure"
tkf Oct 14, 2021
778def1
Revert "Use llvm.frameaddress"
tkf Oct 14, 2021
d0cedca
Merge branch 'master' into fix-sp
tkf Oct 31, 2021
6194475
Use llvm.frameaddress
tkf Oct 13, 2021
0a07e32
Use Int32 as LangRef mentions
tkf Oct 31, 2021
9e80b08
Update test/backtrace.jl
tkf Nov 13, 2021
715c673
Call llvm.frameaddress using Intrinsics.llvmcall
tkf Nov 13, 2021
de860b2
Directly invoke llvmcall in withframeaddress
tkf Nov 13, 2021
e37c0d5
Put back missing `@eval`
tkf Nov 14, 2021
3f8868a
Add dummy frames
tkf Nov 14, 2021
4b6a27f
Revert "Add dummy frames"
tkf Nov 15, 2021
66ff7e9
Check `sp[1] < ptr1`, not `sp[2] < ptr1`
tkf Nov 15, 2021
4c62333
Merge branch 'master' into fix-sp
tkf Nov 16, 2021
df9d96d
Merge branch 'master' into fix-sp
tkf Nov 17, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/stackwalk.c
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,6 @@ static int jl_unw_stepn(bt_cursor_t *cursor, jl_bt_element_t *bt_data, size_t *b
from_signal_handler = 0;
continue;
}
if (sp)
sp[n] = thesp;
// 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
Expand Down Expand Up @@ -168,6 +166,8 @@ static int jl_unw_stepn(bt_cursor_t *cursor, jl_bt_element_t *bt_data, size_t *b
}
}
bt_entry->uintptr = call_ip;
if (sp)
sp[n] = thesp;
n++;
Comment on lines 168 to 171
Copy link
Member Author

@tkf tkf Oct 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix 2: set sp[n] where bt_data[n] contains the call IP. It makes filtering out invalid stack pointers easy (see Base._reformat_sp). Previously, it was stored at the beginning of the extended entry.

All calls to jl_unw_stepn pass sp = NULL expect the one from jl_backtrace_from_here. So, I think this is a NFC.

$ git grep 'jl_unw_stepn('
src/stackwalk.c:static int jl_unw_stepn(bt_cursor_t *cursor, jl_bt_element_t *bt_data, size_t *bt_size,
src/stackwalk.c:    jl_unw_stepn(&cursor, bt_data, &bt_size, NULL, maxsize, 0, &pgcstack, 1);
src/stackwalk.c:    jl_unw_stepn(&cursor, bt_data, &bt_size, NULL, maxsize, skip + 1, &pgcstack, 0);
src/stackwalk.c:            have_more_frames = jl_unw_stepn(&cursor, (jl_bt_element_t*)jl_array_data(ip) + offset,
src/stackwalk.c:    jl_unw_stepn(&cursor, bt_data, &bt_size, NULL, maxsize, 0, &pgcstack, 1);

}
// NOTE: if we have some pgcstack entries remaining (because the
Expand Down Expand Up @@ -259,8 +259,8 @@ JL_DLLEXPORT jl_value_t *jl_backtrace_from_here(int returnsp, int skip)
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);
sp_ptr = (uintptr_t*)jl_array_data(sp) + offset;
Comment on lines 261 to +263
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix 1: Take the pointer after grow, to avoid invalidation.

}
size_t size_incr = 0;
have_more_frames = jl_unw_stepn(&cursor, (jl_bt_element_t*)jl_array_data(ip) + offset,
Expand Down
85 changes: 85 additions & 0 deletions test/backtrace.jl
Original file line number Diff line number Diff line change
Expand Up @@ -258,3 +258,88 @@ let code = """
@test occursin("InterpreterIP in top-level CodeInfo for Main.A", bt_str)
end

"""
_reformat_sp(bt_data...) -> sp::Vector{Ptr{Cvoid}}

Convert the output `bt_data` of `jl_backtrace_from_here` with `returnsp` flag set to a
vector of valid stack pointers `sp`; i.e., `sp` is a subset of `bt_data[3]`.

See also `Base._reformat_bt`.
"""
function _reformat_sp(
bt_raw::Array{Ptr{Cvoid},1},
bt2::Array{Any,1},
sp_raw::Array{Ptr{Cvoid},1},
)
bt = Base._reformat_bt(bt_raw, bt2)
sp = empty!(similar(sp_raw))
i = j = 0
while true
# Advance `i` such that `bt[i] isa Ptr{Cvoid}` (native pointer).
local ip
while true
if i == lastindex(bt)
return sp
end
i += 1
x = bt[i]
if x isa Ptr{Cvoid}
ip = x
break
end
end
# Advance `j` such that `bt_raw[j] == bt[i]` to find a valid stack pointer.
while true
if j == lastindex(bt_raw)
return sp
end
j += 1
if bt_raw[j] == ip
push!(sp, sp_raw[j])
break
end
end
end
end

"""
withframeaddress(f)

Call function `f` with an address `ptr::Ptr{Cvoid}` of an independent frame
immediately outer to `f`.
"""
withframeaddress
@eval @noinline function withframeaddress(f)
sp = Core.Intrinsics.llvmcall(
($"""
declare i8* @llvm.frameaddress(i32)
define private i$(Sys.WORD_SIZE) @frameaddr() {
%1 = call i8* @llvm.frameaddress(i32 0)
%2 = ptrtoint i8* %1 to i$(Sys.WORD_SIZE)
ret i$(Sys.WORD_SIZE) %2
}""", "frameaddr"),
UInt,
Tuple{},
)
@noinline f(Ptr{Cvoid}(sp))
end

function sandwiched_backtrace()
local ptr1, ptr2, bt
withframeaddress() do p1
ptr1 = p1
bt = ccall(:jl_backtrace_from_here, Ref{Base.SimpleVector}, (Cint, Cint), true, 0)
withframeaddress() do p2
ptr2 = p2
end
end
return ptr1, ptr2, bt
end

@testset "stack pointers" begin
ptr1, ptr2, bt_data = sandwiched_backtrace()
sp = _reformat_sp(bt_data...)
@test ptr2 < sp[2]
@test sp[1] < ptr1
@test all(diff(Int128.(UInt.(sp))) .> 0)
end