Skip to content

Commit 4f9b661

Browse files
committed
Fix async stack walking and cleanup
1 parent 2d12796 commit 4f9b661

File tree

9 files changed

+56
-58
lines changed

9 files changed

+56
-58
lines changed

src/hotspot/cpu/aarch64/continuation_aarch64.inline.hpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,18 +54,16 @@ static void patch_callee_link_relative(const frame& f, intptr_t* fp) {
5454
}
5555

5656
template<typename FKind, typename RegisterMapT>
57-
inline void ContinuationHelper::update_register_map(RegisterMapT* map, const frame& f) {
57+
inline void ContinuationHelper::update_register_map(const frame& f, RegisterMapT* map) {
5858
frame::update_map_with_saved_link(map, link_address<FKind>(f));
5959
}
6060

6161
template<typename RegisterMapT>
62-
inline void ContinuationHelper::update_register_map_with_callee(RegisterMapT* map, const frame& f) {
62+
inline void ContinuationHelper::update_register_map_with_callee(const frame& f, RegisterMapT* map) {
6363
frame::update_map_with_saved_link(map, Frame::callee_link_address(f));
6464
}
6565

6666
inline void ContinuationHelper::push_pd(const frame& f) {
67-
log_develop_trace(jvmcont)("ContinuationHelper::push_pd: " INTPTR_FORMAT, p2i(f.fp()));
68-
// os::print_location(tty, (intptr_t)f.fp());
6967
*(intptr_t**)(f.sp() - frame::sender_sp_offset) = f.fp();
7068
}
7169

src/hotspot/cpu/x86/continuation_x86.inline.hpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,18 +53,16 @@ static void patch_callee_link_relative(const frame& f, intptr_t* fp) {
5353
}
5454

5555
template<typename FKind, typename RegisterMapT>
56-
inline void ContinuationHelper::update_register_map(RegisterMapT* map, const frame& f) {
56+
inline void ContinuationHelper::update_register_map(const frame& f, RegisterMapT* map) {
5757
frame::update_map_with_saved_link(map, link_address<FKind>(f));
5858
}
5959

6060
template<typename RegisterMapT>
61-
inline void ContinuationHelper::update_register_map_with_callee(RegisterMapT* map, const frame& f) {
61+
inline void ContinuationHelper::update_register_map_with_callee(const frame& f, RegisterMapT* map) {
6262
frame::update_map_with_saved_link(map, Frame::callee_link_address(f));
6363
}
6464

6565
inline void ContinuationHelper::push_pd(const frame& f) {
66-
log_develop_trace(jvmcont)("ContinuationHelper::push_pd: " INTPTR_FORMAT, p2i(f.fp()));
67-
// os::print_location(tty, (intptr_t)f.fp());
6866
*(intptr_t**)(f.sp() - frame::sender_sp_offset) = f.fp();
6967
}
7068

src/hotspot/share/code/codeCache.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -656,11 +656,15 @@ CodeBlob* CodeCache::find_blob_unsafe(void* start) {
656656

657657
CodeBlob* CodeCache::patch_nop(NativePostCallNop* nop, void* pc, int& slot) {
658658
CodeBlob* cb = CodeCache::find_blob(pc);
659-
int oopmap_slot = cb->oop_maps()->find_slot_for_offset((intptr_t) pc - (intptr_t) cb->code_begin());
660659
intptr_t cbaddr = (intptr_t) cb;
661660
intptr_t offset = ((intptr_t) pc) - cbaddr;
662661

663-
if (((oopmap_slot & 0xff) == oopmap_slot) && ((offset & 0xffffff) == offset)) {
662+
int oopmap_slot = cb->oop_maps()->find_slot_for_offset((intptr_t) pc - (intptr_t) cb->code_begin());
663+
if (oopmap_slot < 0) { // this can happen at asynchronous (non-safepoint) stackwalks
664+
slot = -1;
665+
log_debug(codecache)("failed to find oopmap for cb: " INTPTR_FORMAT " offset: %d", cbaddr, (int) offset);
666+
// tty->print_cr("deopt: %d", cb->as_compiled_method()->is_deopt_pc((address)pc)); os::print_location(tty, (intptr_t)pc);
667+
} else if (((oopmap_slot & 0xff) == oopmap_slot) && ((offset & 0xffffff) == offset)) {
664668
jint value = (oopmap_slot << 24) | (jint) offset;
665669
nop->patch(value);
666670
slot = oopmap_slot;

src/hotspot/share/compiler/oopMap.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -746,8 +746,8 @@ int ImmutableOopMapSet::find_slot_for_offset(int pc_offset) const {
746746
for (int i = 0; i < _count; ++i) {
747747
if (pairs[i].pc_offset() >= pc_offset) {
748748
ImmutableOopMapPair* last = &pairs[i];
749-
assert(last->pc_offset() == pc_offset, "oopmap not found");
750-
return i;
749+
return last->pc_offset() == pc_offset ? i
750+
: -1; // this can happen at asynchronous (non-safepoint) stackwalks
751751
}
752752
}
753753

src/hotspot/share/jfr/recorder/stacktrace/jfrStackTrace.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,7 @@ inline void JfrVframeStream::seek_stable_frame() {
166166

167167
JfrVframeStream::JfrVframeStream(JavaThread* jt, const frame& fr, bool async_mode) : vframeStreamCommon(RegisterMap(jt, false, false, true)),
168168
_continuation(jt->last_continuation()->cont_oop()), _continuation_scope(NULL), _continuation_scope_end_condition(false), _async_mode(async_mode) {
169+
_reg_map.set_async(async_mode);
169170
_stop_at_java_call_stub = false;
170171
_frame = fr;
171172
seek_stable_frame();

src/hotspot/share/prims/forte.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ vframeStreamForte::vframeStreamForte(JavaThread *jt,
9494
frame fr,
9595
bool stop_at_java_call_stub)
9696
: vframeStreamCommon(RegisterMap(jt, false, false, false)) {
97-
97+
_reg_map.set_async(true);
9898
_stop_at_java_call_stub = stop_at_java_call_stub;
9999
_frame = fr;
100100

src/hotspot/share/runtime/continuation.cpp

Lines changed: 24 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -321,19 +321,18 @@ class ContinuationHelper {
321321
static const int align_wiggle; // size, in words, of maximum shift in frame position due to alignment
322322

323323
static oop get_continuation(JavaThread* thread);
324-
324+
static inline frame last_frame(JavaThread* thread);
325325
static bool stack_overflow_check(JavaThread* thread, int size, address sp);
326+
327+
static inline void clear_anchor(JavaThread* thread);
326328
static void set_anchor(JavaThread* thread, intptr_t* sp);
327329
static void set_anchor_pd(JavaFrameAnchor* anchor, intptr_t* sp);
328-
static inline void clear_anchor(JavaThread* thread);
329330
static void set_anchor_to_entry(JavaThread* thread, ContinuationEntry* cont);
330331
static void set_anchor_to_entry_pd(JavaFrameAnchor* anchor, ContinuationEntry* cont);
331332

332-
static void update_map_for_chunk_frame(RegisterMap* map);
333-
template<typename FKind, typename RegisterMapT> static void update_register_map(RegisterMapT* map, const frame& f); // TODO invert parameter order
334-
template<typename RegisterMapT> static void update_register_map_with_callee(RegisterMapT* map, const frame& f); // TODO invert parameter order
333+
template<typename FKind, typename RegisterMapT> static void update_register_map(const frame& f, RegisterMapT* map);
334+
template<typename RegisterMapT> static void update_register_map_with_callee(const frame& f, RegisterMapT* map);
335335

336-
static inline frame last_frame(JavaThread* thread);
337336
static inline void push_pd(const frame& f);
338337

339338
static inline void maybe_flush_stack_processing(JavaThread* thread, ContinuationEntry* entry);
@@ -362,6 +361,10 @@ bool ContinuationHelper::stack_overflow_check(JavaThread* thread, int size, addr
362361
return true;
363362
}
364363

364+
inline void ContinuationHelper::clear_anchor(JavaThread* thread) {
365+
thread->frame_anchor()->clear();
366+
}
367+
365368
void ContinuationHelper::set_anchor(JavaThread* thread, intptr_t* sp) {
366369
address pc = *(address*)(sp - frame::sender_sp_ret_address_offset());
367370
assert (pc != nullptr, "");
@@ -372,15 +375,9 @@ void ContinuationHelper::set_anchor(JavaThread* thread, intptr_t* sp) {
372375
set_anchor_pd(anchor, sp);
373376

374377
assert (thread->has_last_Java_frame(), "");
375-
log_develop_trace(jvmcont)("set_anchor: [" JLONG_FORMAT "] [%d]", java_tid(thread), thread->osthread()->thread_id());
376-
print_vframe(thread->last_frame());
377378
assert(thread->last_frame().cb() != nullptr, "");
378379
}
379380

380-
inline void ContinuationHelper::clear_anchor(JavaThread* thread) {
381-
thread->frame_anchor()->clear();
382-
}
383-
384381
void ContinuationHelper::set_anchor_to_entry(JavaThread* thread, ContinuationEntry* cont) {
385382
JavaFrameAnchor* anchor = thread->frame_anchor();
386383
anchor->set_last_Java_sp(cont->entry_sp());
@@ -389,8 +386,6 @@ void ContinuationHelper::set_anchor_to_entry(JavaThread* thread, ContinuationEnt
389386

390387
assert (thread->has_last_Java_frame(), "");
391388
assert(thread->last_frame().cb() != nullptr, "");
392-
log_develop_trace(jvmcont)("set_anchor: [" JLONG_FORMAT "] [%d]", java_tid(thread), thread->osthread()->thread_id());
393-
print_vframe(thread->last_frame());
394389
}
395390

396391
inline void ContinuationHelper::maybe_flush_stack_processing(JavaThread* thread, ContinuationEntry* entry) {
@@ -446,7 +441,7 @@ class ContMirror {
446441
JavaThread* thread() const { return _thread; }
447442
oop mirror() { return _cont; }
448443
stackChunkOop tail() const { return _tail; }
449-
void set_tail(stackChunkOop chunk) { _tail = chunk; }
444+
void set_tail(stackChunkOop chunk) { _tail = chunk; }
450445
inline void set_empty() { _tail = nullptr; }
451446

452447
oop parent() { return jdk_internal_vm_Continuation::parent(_cont); }
@@ -1145,9 +1140,9 @@ class Freeze {
11451140
intptr_t *_top_address;
11461141

11471142
int _size; // total size of all frames plus metadata in words. keeps track of offset where a frame should be written and how many bytes we need to allocate.
1148-
int _frames;
11491143
int _align_size;
11501144

1145+
NOT_PRODUCT(int _frames;)
11511146
DEBUG_ONLY(intptr_t* _last_write;)
11521147

11531148
inline void set_top_frame_metadata_pd(const frame& hf);
@@ -1186,13 +1181,10 @@ class Freeze {
11861181

11871182
void init_rest() { // we want to postpone some initialization after chunk handling
11881183
_size = 0;
1189-
_frames = 0;
11901184
_align_size = 0;
1185+
NOT_PRODUCT(_frames = 0;)
11911186
}
11921187

1193-
int nr_bytes() const { return _size << LogBytesPerWord; }
1194-
int nr_frames() const { return _frames; }
1195-
11961188
template <bool aligned = true>
11971189
void copy_to_chunk(intptr_t* from, intptr_t* to, int size) {
11981190
stackChunkOop chunk = _cont.tail();
@@ -1201,8 +1193,10 @@ class Freeze {
12011193

12021194
#ifdef ASSERT
12031195
if (_last_write != nullptr) {
1204-
assert (_last_write == to + size, "Missed a spot: _last_write: " INTPTR_FORMAT " to+size: " INTPTR_FORMAT " stack_size: %d _last_write offset: " PTR_FORMAT " to+size: " PTR_FORMAT,
1205-
p2i(_last_write), p2i(to + size), chunk->stack_size(), _last_write - chunk->start_address(), to + size - chunk->start_address());
1196+
assert (_last_write == to + size, "Missed a spot: _last_write: " INTPTR_FORMAT " to+size: " INTPTR_FORMAT
1197+
" stack_size: %d _last_write offset: " PTR_FORMAT " to+size: " PTR_FORMAT,
1198+
p2i(_last_write), p2i(to + size),
1199+
chunk->stack_size(), _last_write - chunk->start_address(), to + size - chunk->start_address());
12061200
_last_write = to;
12071201
}
12081202
#endif
@@ -1316,7 +1310,6 @@ class Freeze {
13161310
chunk->set_max_size(chunk->max_size() + size - argsize);
13171311

13181312
intptr_t* const bottom_sp = bottom - argsize;
1319-
log_develop_trace(jvmcont)("patching bottom sp: " INTPTR_FORMAT, p2i(bottom_sp));
13201313
assert (bottom_sp == _bottom_address, "");
13211314
assert (*(address*)(bottom_sp - frame::sender_sp_ret_address_offset()) == StubRoutines::cont_returnBarrier(), "");
13221315
patch_chunk_pd(bottom_sp, chunk->sp_address());
@@ -1375,7 +1368,6 @@ class Freeze {
13751368
assert (chunk->is_stackChunk(), "");
13761369
assert (!chunk->requires_barriers(), "");
13771370
assert (chunk == _cont.tail(), "");
1378-
// assert (chunk == jdk_internal_vm_Continuation::tail(_cont.mirror()), "");
13791371
// assert (!chunk->is_gc_mode(), "allocated: %d empty: %d", allocated, empty);
13801372
assert (sp <= chunk->stack_size(), "sp: %d chunk size: %d size: %d argsize: %d allocated: %d", sp, chunk->stack_size(), size, argsize, allocated);
13811373

@@ -1394,7 +1386,6 @@ class Freeze {
13941386

13951387
// patch pc
13961388
intptr_t* chunk_bottom_sp = chunk_top + size - argsize;
1397-
log_develop_trace(jvmcont)("freeze_fast patching return address at: " INTPTR_FORMAT " to: " INTPTR_FORMAT, p2i(chunk_bottom_sp - frame::sender_sp_ret_address_offset()), p2i(chunk->pc()));
13981389
assert (empty || *(address*)(chunk_bottom_sp - frame::sender_sp_ret_address_offset()) == StubRoutines::cont_returnBarrier(), "");
13991390
*(address*)(chunk_bottom_sp - frame::sender_sp_ret_address_offset()) = chunk->pc();
14001391

@@ -1432,7 +1423,6 @@ class Freeze {
14321423
#endif
14331424

14341425
log_develop_trace(jvmcont)("freeze_slow #" INTPTR_FORMAT, _cont.hash());
1435-
14361426
assert (_thread->thread_state() == _thread_in_vm || _thread->thread_state() == _thread_blocked, "");
14371427

14381428
init_rest();
@@ -1535,7 +1525,7 @@ class Freeze {
15351525
// log_develop_trace(jvmcont)("recurse_freeze_java_frame fsize: %d frame_bottom: " INTPTR_FORMAT " _bottom_address: " INTPTR_FORMAT, fsize, p2i(FKind::frame_bottom(f)), p2i(_bottom_address));
15361526

15371527
assert (fsize > 0 && argsize >= 0, "");
1538-
_frames++;
1528+
NOT_PRODUCT(_frames++;)
15391529
_size += fsize;
15401530

15411531
if (FKind::frame_bottom(f) >= _bottom_address - 1) { // sometimes there's a space between enterSpecial and the next frame
@@ -1576,7 +1566,7 @@ class Freeze {
15761566
#endif
15771567

15781568
assert (FKind::interpreted || argsize == _cont.argsize(), "argsize: %d _cont.argsize(): %d", argsize, _cont.argsize());
1579-
log_develop_trace(jvmcont)("bottom: " INTPTR_FORMAT " count %d size: %d argsize: %d", p2i(_bottom_address), nr_frames(), nr_bytes(), argsize);
1569+
log_develop_trace(jvmcont)("bottom: " INTPTR_FORMAT " count %d size: %d argsize: %d", p2i(_bottom_address), _frames, _size << LogBytesPerWord, argsize);
15801570

15811571
#ifdef ASSERT
15821572
bool empty = _cont.is_empty();
@@ -1848,12 +1838,12 @@ class Freeze {
18481838
f.cb()->name(), _size, fsize, p2i(vsp), p2i(vsp+fsize));
18491839

18501840
// we're inlining recurse_freeze_java_frame and freeze here because we need to use a full RegisterMap to test lock ownership
1851-
_frames++;
1841+
NOT_PRODUCT(_frames++;)
18521842
_size += fsize;
18531843

18541844
RegisterMap map(_cont.thread(), true, false, false);
18551845
map.set_include_argument_oops(false);
1856-
ContinuationHelper::update_register_map<StubF>(&map, f);
1846+
ContinuationHelper::update_register_map<StubF>(f, &map);
18571847
f.oop_map()->update_register_map(&f, &map); // we have callee-save registers in this case
18581848
frame senderf = sender<StubF>(f);
18591849
assert (senderf.unextended_sp() < _bottom_address - 1, "");
@@ -2793,7 +2783,7 @@ class Thaw {
27932783

27942784
if (f.is_deoptimized_frame()) {
27952785
maybe_set_fastpath(f.sp());
2796-
} else if (_thread->is_interp_only_mode()
2786+
} else if (_thread->is_interp_only_mode()
27972787
|| (_cont.is_preempted() && f.cb()->as_compiled_method()->is_marked_for_deoptimization())) {
27982788
// The caller of the safepoint stub when the continuation is preempted is not at a call instruction, and so
27992789
// cannot rely on nmethod patching for deopt.
@@ -2860,7 +2850,7 @@ class Thaw {
28602850
RegisterMap map(nullptr, true, false, false); // map.clear();
28612851
map.set_include_argument_oops(false);
28622852
f.oop_map()->update_register_map(&f, &map);
2863-
ContinuationHelper::update_register_map_with_callee(&map, caller);
2853+
ContinuationHelper::update_register_map_with_callee(caller, &map);
28642854
InstanceStackChunkKlass::fix_thawed_frame(_cont.tail(), caller, &map);
28652855
}
28662856

@@ -3024,7 +3014,7 @@ void do_deopt_after_thaw(JavaThread* thread) {
30243014
int i = 0;
30253015
StackFrameStream fst(thread, true, false);
30263016
fst.register_map()->set_include_argument_oops(false);
3027-
ContinuationHelper::update_register_map_with_callee(fst.register_map(), *fst.current());
3017+
ContinuationHelper::update_register_map_with_callee(*fst.current(), fst.register_map());
30283018
for (; !fst.is_done(); fst.next()) {
30293019
if (fst.current()->cb()->is_compiled()) {
30303020
CompiledMethod* cm = fst.current()->cb()->as_compiled_method();
@@ -3045,7 +3035,7 @@ bool do_verify_after_thaw(JavaThread* thread, int mode, bool barriers, stackChun
30453035
int i = 0;
30463036
StackFrameStream fst(thread, true, false);
30473037
fst.register_map()->set_include_argument_oops(false);
3048-
ContinuationHelper::update_register_map_with_callee(fst.register_map(), *fst.current());
3038+
ContinuationHelper::update_register_map_with_callee(*fst.current(), fst.register_map());
30493039
for (; !fst.is_done() && !Continuation::is_continuation_enterSpecial(*fst.current()); fst.next()) {
30503040
if (fst.current()->cb()->is_compiled() && fst.current()->cb()->as_compiled_method()->is_marked_for_deoptimization()) {
30513041
tty->print_cr(">>> do_verify_after_thaw deopt");

src/hotspot/share/runtime/frame.cpp

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,12 @@
6262
RegisterMap::RegisterMap(JavaThread *thread, bool update_map, bool process_frames, bool walk_cont) {
6363
_thread = thread;
6464
_update_map = update_map;
65-
_walk_cont = walk_cont;
66-
DEBUG_ONLY(_skip_missing = false;)
6765
_process_frames = process_frames;
66+
_walk_cont = walk_cont;
6867
clear();
69-
debug_only(_update_for_id = NULL;)
68+
DEBUG_ONLY (_update_for_id = NULL;)
69+
NOT_PRODUCT(_skip_missing = false;)
70+
NOT_PRODUCT(_async = false;)
7071

7172
if (walk_cont && thread != NULL && thread->last_continuation() != NULL) {
7273
_chunk = stackChunkHandle(Thread::current(), NULL, true);
@@ -80,11 +81,12 @@ RegisterMap::RegisterMap(JavaThread *thread, bool update_map, bool process_frame
8081
RegisterMap::RegisterMap(oop continuation, bool update_map) {
8182
_thread = NULL;
8283
_update_map = update_map;
83-
_walk_cont = true;
84-
DEBUG_ONLY(_skip_missing = false;)
8584
_process_frames = false;
85+
_walk_cont = true;
8686
clear();
87-
debug_only(_update_for_id = NULL;)
87+
DEBUG_ONLY (_update_for_id = NULL;)
88+
NOT_PRODUCT(_skip_missing = false;)
89+
NOT_PRODUCT(_async = false;)
8890

8991
_chunk = stackChunkHandle(Thread::current(), NULL, true);
9092

@@ -99,10 +101,11 @@ RegisterMap::RegisterMap(const RegisterMap* map) {
99101
_thread = map->thread();
100102
_update_map = map->update_map();
101103
_process_frames = map->process_frames();
104+
_walk_cont = map->_walk_cont;
102105
_include_argument_oops = map->include_argument_oops();
103-
debug_only(_update_for_id = map->_update_for_id;)
104-
_walk_cont = map->_walk_cont;
105-
DEBUG_ONLY(_skip_missing = map->_skip_missing;)
106+
DEBUG_ONLY (_update_for_id = map->_update_for_id;)
107+
NOT_PRODUCT(_skip_missing = map->_skip_missing;)
108+
NOT_PRODUCT(_async = map->_async;)
106109

107110
// only the original RegisterMap's handle lives long enough for StackWalker; this is bound to cause trouble with nested continuations.
108111
_chunk = map->_chunk; // stackChunkHandle(Thread::current(), map->_chunk(), map->_chunk.not_null()); //

src/hotspot/share/runtime/registerMap.hpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,8 @@ class RegisterMap : public StackObj {
8282
bool _process_frames; // Should frames be processed by stack watermark barriers?
8383
bool _walk_cont; // whether to walk frames on a continuation stack
8484

85-
NOT_PRODUCT(bool _skip_missing;)
85+
NOT_PRODUCT(bool _skip_missing;) // ignore missing registers
86+
NOT_PRODUCT(bool _async;) // walking frames asynchronously, at arbitrary points
8687

8788
#ifdef ASSERT
8889
void check_location_valid();
@@ -157,8 +158,11 @@ class RegisterMap : public StackObj {
157158
void print_on(outputStream* st) const;
158159
void print() const;
159160

161+
void set_async(bool value) { NOT_PRODUCT(_async = value;) }
162+
void set_skip_missing(bool value) { NOT_PRODUCT(_skip_missing = value;) }
163+
160164
#ifndef PRODUCT
161-
void set_skip_missing(bool value) { _skip_missing = value; }
165+
bool is_async() const { return _async; }
162166
bool should_skip_missing() const { return _skip_missing; }
163167

164168
VMReg find_register_spilled_here(void* p, intptr_t* sp);

0 commit comments

Comments
 (0)