Skip to content

Commit

Permalink
stacktrace: prevent OOB-error in sysimage lookup (#30369)
Browse files Browse the repository at this point in the history
Previously, with a multi-versioned system image, there might be additional entries at the end of the clone list
that do not correspond to an actual method (such as jlplt thunks).

Also some code cleanup for clarity.

fix #28648
  • Loading branch information
vtjnash authored Dec 17, 2018
1 parent 36cc20c commit e51a707
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 3 deletions.
3 changes: 2 additions & 1 deletion src/debuginfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1087,7 +1087,8 @@ static int jl_getDylibFunctionInfo(jl_frame_t **frames, size_t pointer, int skip
for (size_t i = 0; i < sysimg_fptrs.nclones; i++) {
if (diff == sysimg_fptrs.clone_offsets[i]) {
uint32_t idx = sysimg_fptrs.clone_idxs[i] & jl_sysimg_val_mask;
frame0->linfo = sysimg_fvars_linfo[idx];
if (idx < sysimg_fvars_n) // items after this were cloned but not referenced directly by a method (such as our ccall PLT thunks)
frame0->linfo = sysimg_fvars_linfo[idx];
break;
}
}
Expand Down
1 change: 1 addition & 0 deletions src/staticdata.c
Original file line number Diff line number Diff line change
Expand Up @@ -1016,6 +1016,7 @@ static void jl_update_all_fptrs(jl_serializer_state *s)
for (i = 0; i < sysimg_fvars_max; i++) {
uintptr_t val = (uintptr_t)&linfos[i];
uint32_t offset = load_uint32(&val);
linfos[i] = NULL;
if (offset != 0) {
int specfunc = 1;
if (offset & ((uintptr_t)1 << (8 * sizeof(uint32_t) - 1))) {
Expand Down
4 changes: 2 additions & 2 deletions stdlib/Profile/src/Profile.jl
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,7 @@ function tree!(root::StackFrameTree{T}, all::Vector{UInt64}, lidict::Union{LineI
# jump forward to the end of the inlining chain
# avoiding an extra (slow) lookup of `ip` in `lidict`
# and an extra chain of them in `down`
# note that we may even have this === parent (if we're ignoring this frame ip)
this = builder_value[fastkey]
let this = this
while this !== parent
Expand All @@ -532,8 +533,7 @@ function tree!(root::StackFrameTree{T}, all::Vector{UInt64}, lidict::Union{LineI
frame = (frames isa Vector ? frames[i] : frames)
!C && frame.from_c && continue
key = (T === UInt64 ? ip : frame)
down = parent.down
this = get!(down, key) do
this = get!(parent.down, key) do
return StackFrameTree{T}()
end
this.frame = frame
Expand Down

2 comments on commit e51a707

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Executing the daily benchmark build, I will reply here when finished:

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

Please sign in to comment.