Skip to content

Commit 4e4d1f2

Browse files
author
David Holmes
committed
8234372: Investigate use of Thread::stack_base() and queries for "in stack"
Reviewed-by: dcubed, stuefe
1 parent 25c5a23 commit 4e4d1f2

File tree

12 files changed

+68
-109
lines changed

12 files changed

+68
-109
lines changed

‎src/hotspot/cpu/aarch64/frame_aarch64.cpp‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 1997, 2019, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 1997, 2020, Oracle and/or its affiliates. All rights reserved.
33
* Copyright (c) 2014, Red Hat Inc. All rights reserved.
44
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
55
*
@@ -59,16 +59,8 @@ bool frame::safe_for_sender(JavaThread *thread) {
5959
address unextended_sp = (address)_unextended_sp;
6060

6161
// consider stack guards when trying to determine "safe" stack pointers
62-
static size_t stack_guard_size = os::uses_stack_guard_pages() ?
63-
(JavaThread::stack_red_zone_size() + JavaThread::stack_yellow_zone_size()) : 0;
64-
size_t usable_stack_size = thread->stack_size() - stack_guard_size;
65-
6662
// sp must be within the usable part of the stack (not in guards)
67-
bool sp_safe = (sp < thread->stack_base()) &&
68-
(sp >= thread->stack_base() - usable_stack_size);
69-
70-
71-
if (!sp_safe) {
63+
if (!thread->is_in_usable_stack(sp)) {
7264
return false;
7365
}
7466

@@ -566,7 +558,7 @@ bool frame::is_interpreted_frame_valid(JavaThread* thread) const {
566558

567559
address locals = (address) *interpreter_frame_locals_addr();
568560

569-
if (locals > thread->stack_base() || locals < (address) fp()) return false;
561+
if (locals >= thread->stack_base() || locals < (address) fp()) return false;
570562

571563
// We'd have to be pretty unlucky to be mislead at this point
572564
return true;

‎src/hotspot/cpu/arm/frame_arm.cpp‎

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2008, 2018, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2008, 2020, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -57,21 +57,14 @@ bool frame::safe_for_sender(JavaThread *thread) {
5757
address fp = (address)_fp;
5858
address unextended_sp = (address)_unextended_sp;
5959

60-
static size_t stack_guard_size = os::uses_stack_guard_pages() ?
61-
(JavaThread::stack_red_zone_size() + JavaThread::stack_yellow_zone_size()) : 0;
62-
size_t usable_stack_size = thread->stack_size() - stack_guard_size;
63-
60+
// consider stack guards when trying to determine "safe" stack pointers
6461
// sp must be within the usable part of the stack (not in guards)
65-
bool sp_safe = (sp != NULL &&
66-
(sp <= thread->stack_base()) &&
67-
(sp >= thread->stack_base() - usable_stack_size));
68-
69-
if (!sp_safe) {
62+
if (!thread->is_in_usable_stack(sp)) {
7063
return false;
7164
}
7265

7366
bool unextended_sp_safe = (unextended_sp != NULL &&
74-
(unextended_sp <= thread->stack_base()) &&
67+
(unextended_sp < thread->stack_base()) &&
7568
(unextended_sp >= sp));
7669
if (!unextended_sp_safe) {
7770
return false;
@@ -80,7 +73,7 @@ bool frame::safe_for_sender(JavaThread *thread) {
8073
// We know sp/unextended_sp are safe. Only fp is questionable here.
8174

8275
bool fp_safe = (fp != NULL &&
83-
(fp <= thread->stack_base()) &&
76+
(fp < thread->stack_base()) &&
8477
fp >= sp);
8578

8679
if (_cb != NULL ) {
@@ -148,7 +141,7 @@ bool frame::safe_for_sender(JavaThread *thread) {
148141
// is really a frame pointer.
149142

150143
intptr_t *saved_fp = (intptr_t*)*(sender_sp - frame::sender_sp_offset + link_offset);
151-
bool saved_fp_safe = ((address)saved_fp <= thread->stack_base()) && (saved_fp > sender_sp);
144+
bool saved_fp_safe = ((address)saved_fp < thread->stack_base()) && (saved_fp > sender_sp);
152145

153146
if (!saved_fp_safe) {
154147
return false;
@@ -178,7 +171,7 @@ bool frame::safe_for_sender(JavaThread *thread) {
178171
// Could be the call_stub
179172
if (StubRoutines::returns_to_call_stub(sender_pc)) {
180173
intptr_t *saved_fp = (intptr_t*)*(sender_sp - frame::sender_sp_offset + link_offset);
181-
bool saved_fp_safe = ((address)saved_fp <= thread->stack_base()) && (saved_fp >= sender_sp);
174+
bool saved_fp_safe = ((address)saved_fp < thread->stack_base()) && (saved_fp > sender_sp);
182175

183176
if (!saved_fp_safe) {
184177
return false;
@@ -191,7 +184,7 @@ bool frame::safe_for_sender(JavaThread *thread) {
191184
// Validate the JavaCallWrapper an entry frame must have
192185
address jcw = (address)sender.entry_frame_call_wrapper();
193186

194-
bool jcw_safe = (jcw <= thread->stack_base()) && (jcw > (address)sender.fp());
187+
bool jcw_safe = (jcw < thread->stack_base()) && (jcw > (address)sender.fp());
195188

196189
return jcw_safe;
197190
}
@@ -501,7 +494,7 @@ bool frame::is_interpreted_frame_valid(JavaThread* thread) const {
501494

502495
address locals = (address) *interpreter_frame_locals_addr();
503496

504-
if (locals > thread->stack_base() || locals < (address) fp()) return false;
497+
if (locals >= thread->stack_base() || locals < (address) fp()) return false;
505498

506499
// We'd have to be pretty unlucky to be mislead at this point
507500

‎src/hotspot/cpu/ppc/frame_ppc.cpp‎

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2000, 2018, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2000, 2020, Oracle and/or its affiliates. All rights reserved.
33
* Copyright (c) 2012, 2017 SAP SE. All rights reserved.
44
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
55
*
@@ -55,17 +55,9 @@ bool frame::safe_for_sender(JavaThread *thread) {
5555
address fp = (address)_fp;
5656
address unextended_sp = (address)_unextended_sp;
5757

58-
// Consider stack guards when trying to determine "safe" stack pointers
59-
static size_t stack_guard_size = os::uses_stack_guard_pages() ?
60-
JavaThread::stack_red_zone_size() + JavaThread::stack_yellow_reserved_zone_size() : 0;
61-
size_t usable_stack_size = thread->stack_size() - stack_guard_size;
62-
58+
// consider stack guards when trying to determine "safe" stack pointers
6359
// sp must be within the usable part of the stack (not in guards)
64-
bool sp_safe = (sp < thread->stack_base()) &&
65-
(sp >= thread->stack_base() - usable_stack_size);
66-
67-
68-
if (!sp_safe) {
60+
if (!thread->is_in_usable_stack(sp)) {
6961
return false;
7062
}
7163

@@ -77,10 +69,10 @@ bool frame::safe_for_sender(JavaThread *thread) {
7769
}
7870

7971
// An fp must be within the stack and above (but not equal) sp.
80-
bool fp_safe = (fp <= thread->stack_base()) && (fp > sp);
72+
bool fp_safe = (fp < thread->stack_base()) && (fp > sp);
8173
// An interpreter fp must be within the stack and above (but not equal) sp.
8274
// Moreover, it must be at least the size of the ijava_state structure.
83-
bool fp_interp_safe = (fp <= thread->stack_base()) && (fp > sp) &&
75+
bool fp_interp_safe = (fp < thread->stack_base()) && (fp > sp) &&
8476
((fp - sp) >= ijava_state_size);
8577

8678
// We know sp/unextended_sp are safe, only fp is questionable here
@@ -140,7 +132,7 @@ bool frame::safe_for_sender(JavaThread *thread) {
140132

141133
// sender_fp must be within the stack and above (but not
142134
// equal) current frame's fp.
143-
if (sender_fp > thread->stack_base() || sender_fp <= fp) {
135+
if (sender_fp >= thread->stack_base() || sender_fp <= fp) {
144136
return false;
145137
}
146138

‎src/hotspot/cpu/s390/frame_s390.cpp‎

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2016, 2019, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2016, 2020, Oracle and/or its affiliates. All rights reserved.
33
* Copyright (c) 2016, 2019, SAP SE. All rights reserved.
44
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
55
*
@@ -59,17 +59,9 @@ bool frame::safe_for_sender(JavaThread *thread) {
5959
address fp = (address)_fp;
6060
address unextended_sp = (address)_unextended_sp;
6161

62-
// Consider stack guards when trying to determine "safe" stack pointers
63-
static size_t stack_guard_size = os::uses_stack_guard_pages() ?
64-
JavaThread::stack_red_zone_size() + JavaThread::stack_yellow_reserved_zone_size() : 0;
65-
size_t usable_stack_size = thread->stack_size() - stack_guard_size;
66-
62+
// consider stack guards when trying to determine "safe" stack pointers
6763
// sp must be within the usable part of the stack (not in guards)
68-
bool sp_safe = (sp < thread->stack_base()) &&
69-
(sp >= thread->stack_base() - usable_stack_size);
70-
71-
72-
if (!sp_safe) {
64+
if (!thread->is_in_usable_stack(sp)) {
7365
return false;
7466
}
7567

@@ -81,10 +73,10 @@ bool frame::safe_for_sender(JavaThread *thread) {
8173
}
8274

8375
// An fp must be within the stack and above (but not equal) sp.
84-
bool fp_safe = (fp <= thread->stack_base()) && (fp > sp);
76+
bool fp_safe = (fp < thread->stack_base()) && (fp > sp);
8577
// An interpreter fp must be within the stack and above (but not equal) sp.
8678
// Moreover, it must be at least the size of the z_ijava_state structure.
87-
bool fp_interp_safe = (fp <= thread->stack_base()) && (fp > sp) &&
79+
bool fp_interp_safe = (fp < thread->stack_base()) && (fp > sp) &&
8880
((fp - sp) >= z_ijava_state_size);
8981

9082
// We know sp/unextended_sp are safe, only fp is questionable here
@@ -144,7 +136,7 @@ bool frame::safe_for_sender(JavaThread *thread) {
144136

145137
// sender_fp must be within the stack and above (but not
146138
// equal) current frame's fp.
147-
if (sender_fp > thread->stack_base() || sender_fp <= fp) {
139+
if (sender_fp >= thread->stack_base() || sender_fp <= fp) {
148140
return false;
149141
}
150142

‎src/hotspot/cpu/sparc/frame_sparc.cpp‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 1997, 2020, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -177,22 +177,21 @@ bool frame::safe_for_sender(JavaThread *thread) {
177177
address _SP = (address) sp();
178178
address _FP = (address) fp();
179179
address _UNEXTENDED_SP = (address) unextended_sp();
180-
// sp must be within the stack
181-
bool sp_safe = (_SP <= thread->stack_base()) &&
182-
(_SP >= thread->stack_base() - thread->stack_size());
183180

184-
if (!sp_safe) {
181+
// consider stack guards when trying to determine "safe" stack pointers
182+
// sp must be within the usable part of the stack (not in guards)
183+
if (!thread->is_in_usable_stack(_SP)) {
185184
return false;
186185
}
187186

188187
// unextended sp must be within the stack and above or equal sp
189-
bool unextended_sp_safe = (_UNEXTENDED_SP <= thread->stack_base()) &&
188+
bool unextended_sp_safe = (_UNEXTENDED_SP < thread->stack_base()) &&
190189
(_UNEXTENDED_SP >= _SP);
191190

192191
if (!unextended_sp_safe) return false;
193192

194193
// an fp must be within the stack and above (but not equal) sp
195-
bool fp_safe = (_FP <= thread->stack_base()) &&
194+
bool fp_safe = (_FP < thread->stack_base()) &&
196195
(_FP > _SP);
197196

198197
// We know sp/unextended_sp are safe only fp is questionable here
@@ -252,7 +251,7 @@ bool frame::safe_for_sender(JavaThread *thread) {
252251

253252
// an fp must be within the stack and above (but not equal) current frame's _FP
254253

255-
bool sender_fp_safe = (sender_fp <= thread->stack_base()) &&
254+
bool sender_fp_safe = (sender_fp < thread->stack_base()) &&
256255
(sender_fp > _FP);
257256

258257
if (!sender_fp_safe) {
@@ -280,7 +279,7 @@ bool frame::safe_for_sender(JavaThread *thread) {
280279

281280
address jcw = (address)sender.entry_frame_call_wrapper();
282281

283-
bool jcw_safe = (jcw <= thread->stack_base()) && (jcw > sender_fp);
282+
bool jcw_safe = (jcw < thread->stack_base()) && (jcw > sender_fp);
284283

285284
return jcw_safe;
286285
}
@@ -672,7 +671,7 @@ bool frame::is_interpreted_frame_valid(JavaThread* thread) const {
672671

673672
address locals = (address) *interpreter_frame_locals_addr();
674673

675-
if (locals > thread->stack_base() || locals < (address) fp()) return false;
674+
if (locals >= thread->stack_base() || locals < (address) fp()) return false;
676675

677676
// We'd have to be pretty unlucky to be mislead at this point
678677
return true;

‎src/hotspot/cpu/x86/frame_x86.cpp‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 1997, 2020, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -57,16 +57,8 @@ bool frame::safe_for_sender(JavaThread *thread) {
5757
address unextended_sp = (address)_unextended_sp;
5858

5959
// consider stack guards when trying to determine "safe" stack pointers
60-
static size_t stack_guard_size = os::uses_stack_guard_pages() ?
61-
JavaThread::stack_red_zone_size() + JavaThread::stack_yellow_zone_size() : 0;
62-
size_t usable_stack_size = thread->stack_size() - stack_guard_size;
63-
6460
// sp must be within the usable part of the stack (not in guards)
65-
bool sp_safe = (sp < thread->stack_base()) &&
66-
(sp >= thread->stack_base() - usable_stack_size);
67-
68-
69-
if (!sp_safe) {
61+
if (!thread->is_in_usable_stack(sp)) {
7062
return false;
7163
}
7264

@@ -553,7 +545,7 @@ bool frame::is_interpreted_frame_valid(JavaThread* thread) const {
553545

554546
address locals = (address) *interpreter_frame_locals_addr();
555547

556-
if (locals > thread->stack_base() || locals < (address) fp()) return false;
548+
if (locals >= thread->stack_base() || locals < (address) fp()) return false;
557549

558550
// We'd have to be pretty unlucky to be mislead at this point
559551
return true;

‎src/hotspot/os/linux/os_linux.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 1999, 2019, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 1999, 2020, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -719,7 +719,7 @@ bool os::Linux::manually_expand_stack(JavaThread * t, address addr) {
719719
assert(t->osthread()->expanding_stack(), "expand should be set");
720720
assert(t->stack_base() != NULL, "stack_base was not initialized");
721721

722-
if (addr < t->stack_base() && addr >= t->stack_reserved_zone_base()) {
722+
if (t->is_in_usable_stack(addr)) {
723723
sigset_t mask_all, old_sigset;
724724
sigfillset(&mask_all);
725725
pthread_sigmask(SIG_SETMASK, &mask_all, &old_sigset);

‎src/hotspot/os/windows/os_windows.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 1997, 2019, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 1997, 2020, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -2542,7 +2542,7 @@ LONG WINAPI topLevelExceptionFilter(struct _EXCEPTION_POINTERS* exceptionInfo) {
25422542
//
25432543
PEXCEPTION_RECORD exceptionRecord = exceptionInfo->ExceptionRecord;
25442544
address addr = (address) exceptionRecord->ExceptionInformation[1];
2545-
if (addr > thread->stack_reserved_zone_base() && addr < thread->stack_base()) {
2545+
if (thread->is_in_usable_stack(addr)) {
25462546
addr = (address)((uintptr_t)addr &
25472547
(~((uintptr_t)os::vm_page_size() - (uintptr_t)1)));
25482548
os::commit_memory((char *)addr, thread->stack_base() - addr,

‎src/hotspot/os_cpu/linux_arm/os_linux_arm.cpp‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2008, 2019, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2008, 2020, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -336,8 +336,7 @@ extern "C" int JVM_handle_linux_signal(int sig, siginfo_t* info,
336336
return 1;
337337
}
338338
// check if fault address is within thread stack
339-
if (addr < thread->stack_base() &&
340-
addr >= thread->stack_base() - thread->stack_size()) {
339+
if (thread->on_local_stack(addr)) {
341340
// stack overflow
342341
if (thread->in_stack_yellow_reserved_zone(addr)) {
343342
thread->disable_stack_yellow_reserved_zone();

‎src/hotspot/os_cpu/linux_s390/thread_linux_s390.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2016, 2019, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2016, 2020, Oracle and/or its affiliates. All rights reserved.
33
* Copyright (c) 2016, 2019 SAP SE. All rights reserved.
44
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
55
*
@@ -63,7 +63,7 @@ bool JavaThread::pd_get_top_frame_for_profiling(frame* fr_addr, void* ucontext,
6363

6464
if (ret_frame.is_interpreted_frame()) {
6565
frame::z_ijava_state* istate = ret_frame.ijava_state_unchecked();
66-
if (stack_base() >= (address)istate && (address)istate > stack_end()) {
66+
if (on_local_stack((address)istate)) {
6767
return false;
6868
}
6969
const Method *m = (const Method*)(istate->method);

0 commit comments

Comments
 (0)