Skip to content
This repository was archived by the owner on Aug 27, 2022. It is now read-only.
/ lanai Public archive

Commit 3ab1dfe

Browse files
committedDec 15, 2020
8257828: SafeFetch may crash if invoked in non-JavaThreads
Reviewed-by: mdoerr, kbarrett, coleenp, dholmes
1 parent 381021a commit 3ab1dfe

File tree

12 files changed

+113
-94
lines changed

12 files changed

+113
-94
lines changed
 

‎src/hotspot/os/posix/signals_posix.cpp

+32-2
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,19 @@
3232
#include "runtime/java.hpp"
3333
#include "runtime/os.hpp"
3434
#include "runtime/osThread.hpp"
35+
#include "runtime/stubRoutines.hpp"
3536
#include "runtime/thread.hpp"
3637
#include "signals_posix.hpp"
3738
#include "utilities/events.hpp"
3839
#include "utilities/ostream.hpp"
3940
#include "utilities/vmError.hpp"
4041

42+
#ifdef ZERO
43+
// See stubGenerator_zero.cpp
44+
#include <setjmp.h>
45+
extern sigjmp_buf* get_jmp_buf_for_continuation();
46+
#endif
47+
4148
#include <signal.h>
4249

4350
// Various signal related mechanism are laid out in the following order:
@@ -546,13 +553,36 @@ int JVM_HANDLE_XXX_SIGNAL(int sig, siginfo_t* info,
546553

547554
// Handle assertion poison page accesses.
548555
#ifdef CAN_SHOW_REGISTERS_ON_ASSERT
549-
if ((sig == SIGSEGV || sig == SIGBUS) && info != NULL && info->si_addr == g_assert_poison) {
556+
if (!signal_was_handled &&
557+
((sig == SIGSEGV || sig == SIGBUS) && info != NULL && info->si_addr == g_assert_poison)) {
550558
signal_was_handled = handle_assert_poison_fault(ucVoid, info->si_addr);
551559
}
552560
#endif
553561

562+
if (!signal_was_handled) {
563+
// Handle SafeFetch access.
564+
#ifndef ZERO
565+
if (uc != NULL) {
566+
address pc = os::Posix::ucontext_get_pc(uc);
567+
if (StubRoutines::is_safefetch_fault(pc)) {
568+
os::Posix::ucontext_set_pc(uc, StubRoutines::continuation_for_safefetch_fault(pc));
569+
signal_was_handled = true;
570+
}
571+
}
572+
#else
573+
// See JDK-8076185
574+
if (sig == SIGSEGV || sig == SIGBUS) {
575+
sigjmp_buf* const pjb = get_jmp_buf_for_continuation();
576+
if (pjb) {
577+
siglongjmp(*pjb, 1);
578+
}
579+
}
580+
#endif // ZERO
581+
}
582+
554583
// Ignore SIGPIPE and SIGXFSZ (4229104, 6499219).
555-
if (sig == SIGPIPE || sig == SIGXFSZ) {
584+
if (!signal_was_handled &&
585+
(sig == SIGPIPE || sig == SIGXFSZ)) {
556586
PosixSignals::chained_handler(sig, info, ucVoid);
557587
signal_was_handled = true; // unconditionally.
558588
}

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

+13-18
Original file line numberDiff line numberDiff line change
@@ -2242,29 +2242,24 @@ int os::signal_wait() {
22422242
LONG Handle_Exception(struct _EXCEPTION_POINTERS* exceptionInfo,
22432243
address handler) {
22442244
Thread* thread = Thread::current_or_null();
2245-
// Save pc in thread
2245+
22462246
#if defined(_M_ARM64)
2247-
// Do not blow up if no thread info available.
2248-
if (thread) {
2249-
thread->as_Java_thread()->set_saved_exception_pc((address)(DWORD_PTR)exceptionInfo->ContextRecord->Pc);
2250-
}
2251-
// Set pc to handler
2252-
exceptionInfo->ContextRecord->Pc = (DWORD64)handler;
2247+
#define PC_NAME Pc
22532248
#elif defined(_M_AMD64)
2254-
// Do not blow up if no thread info available.
2255-
if (thread != NULL) {
2256-
thread->as_Java_thread()->set_saved_exception_pc((address)(DWORD_PTR)exceptionInfo->ContextRecord->Rip);
2257-
}
2258-
// Set pc to handler
2259-
exceptionInfo->ContextRecord->Rip = (DWORD64)handler;
2249+
#define PC_NAME Rip
2250+
#elif defined(_M_IX86)
2251+
#define PC_NAME Eip
22602252
#else
2261-
// Do not blow up if no thread info available.
2262-
if (thread != NULL) {
2263-
thread->as_Java_thread()->set_saved_exception_pc((address)(DWORD_PTR)exceptionInfo->ContextRecord->Eip);
2253+
#error unknown architecture
2254+
#endif
2255+
2256+
// Save pc in thread
2257+
if (thread != nullptr && thread->is_Java_thread()) {
2258+
thread->as_Java_thread()->set_saved_exception_pc((address)(DWORD_PTR)exceptionInfo->ContextRecord->PC_NAME);
22642259
}
2260+
22652261
// Set pc to handler
2266-
exceptionInfo->ContextRecord->Eip = (DWORD)(DWORD_PTR)handler;
2267-
#endif
2262+
exceptionInfo->ContextRecord->PC_NAME = (DWORD64)handler;
22682263

22692264
// Continue the execution
22702265
return EXCEPTION_CONTINUE_EXECUTION;

‎src/hotspot/os_cpu/aix_ppc/os_aix_ppc.cpp

-11
Original file line numberDiff line numberDiff line change
@@ -180,17 +180,6 @@ bool PosixSignals::pd_hotspot_signal_handler(int sig, siginfo_t* info,
180180
// retrieve crash address
181181
address const addr = info ? (const address) info->si_addr : NULL;
182182

183-
// SafeFetch 32 handling:
184-
// - make it work if _thread is null
185-
// - make it use the standard os::...::ucontext_get/set_pc APIs
186-
if (uc) {
187-
address const pc = os::Posix::ucontext_get_pc(uc);
188-
if (pc && StubRoutines::is_safefetch_fault(pc)) {
189-
os::Posix::ucontext_set_pc(uc, StubRoutines::continuation_for_safefetch_fault(pc));
190-
return true;
191-
}
192-
}
193-
194183
if (info == NULL || uc == NULL) {
195184
return false; // Fatal error
196185
}

‎src/hotspot/os_cpu/bsd_x86/os_bsd_x86.cpp

-5
Original file line numberDiff line numberDiff line change
@@ -411,11 +411,6 @@ bool PosixSignals::pd_hotspot_signal_handler(int sig, siginfo_t* info,
411411
if (info != NULL && uc != NULL && thread != NULL) {
412412
pc = (address) os::Posix::ucontext_get_pc(uc);
413413

414-
if (StubRoutines::is_safefetch_fault(pc)) {
415-
os::Posix::ucontext_set_pc(uc, StubRoutines::continuation_for_safefetch_fault(pc));
416-
return true;
417-
}
418-
419414
// Handle ALL stack overflow variations here
420415
if (sig == SIGSEGV || sig == SIGBUS) {
421416
address addr = (address) info->si_addr;

‎src/hotspot/os_cpu/bsd_zero/os_bsd_zero.cpp

-12
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,6 @@
5757
#include "utilities/events.hpp"
5858
#include "utilities/vmError.hpp"
5959

60-
// See stubGenerator_zero.cpp
61-
#include <setjmp.h>
62-
extern sigjmp_buf* get_jmp_buf_for_continuation();
63-
6460
address os::current_stack_pointer() {
6561
address dummy = (address) &dummy;
6662
return dummy;
@@ -118,14 +114,6 @@ frame os::fetch_frame_from_context(const void* ucVoid) {
118114
bool PosixSignals::pd_hotspot_signal_handler(int sig, siginfo_t* info,
119115
ucontext_t* uc, JavaThread* thread) {
120116

121-
// handle SafeFetch faults the zero way
122-
if (sig == SIGSEGV || sig == SIGBUS) {
123-
sigjmp_buf* const pjb = get_jmp_buf_for_continuation();
124-
if (pjb) {
125-
siglongjmp(*pjb, 1);
126-
}
127-
}
128-
129117
if (info != NULL && thread != NULL) {
130118
// Handle ALL stack overflow variations here
131119
if (sig == SIGSEGV || sig == SIGBUS) {

‎src/hotspot/os_cpu/linux_aarch64/os_linux_aarch64.cpp

-5
Original file line numberDiff line numberDiff line change
@@ -185,11 +185,6 @@ bool PosixSignals::pd_hotspot_signal_handler(int sig, siginfo_t* info,
185185
if (info != NULL && uc != NULL && thread != NULL) {
186186
pc = (address) os::Posix::ucontext_get_pc(uc);
187187

188-
if (StubRoutines::is_safefetch_fault(pc)) {
189-
os::Posix::ucontext_set_pc(uc, StubRoutines::continuation_for_safefetch_fault(pc));
190-
return true;
191-
}
192-
193188
address addr = (address) info->si_addr;
194189

195190
// Make sure the high order byte is sign extended, as it may be masked away by the hardware.

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

-4
Original file line numberDiff line numberDiff line change
@@ -268,10 +268,6 @@ bool PosixSignals::pd_hotspot_signal_handler(int sig, siginfo_t* info,
268268
if (sig == SIGSEGV) {
269269
address addr = (address) info->si_addr;
270270

271-
if (StubRoutines::is_safefetch_fault(pc)) {
272-
os::Posix::ucontext_set_pc(uc, StubRoutines::continuation_for_safefetch_fault(pc));
273-
return true;
274-
}
275271
// check if fault address is within thread stack
276272
if (thread->is_in_full_stack(addr)) {
277273
// stack overflow

‎src/hotspot/os_cpu/linux_ppc/os_linux_ppc.cpp

-10
Original file line numberDiff line numberDiff line change
@@ -213,16 +213,6 @@ bool PosixSignals::pd_hotspot_signal_handler(int sig, siginfo_t* info,
213213
}
214214
}
215215

216-
// Moved SafeFetch32 handling outside thread!=NULL conditional block to make
217-
// it work if no associated JavaThread object exists.
218-
if (uc) {
219-
address const pc = os::Posix::ucontext_get_pc(uc);
220-
if (pc && StubRoutines::is_safefetch_fault(pc)) {
221-
os::Posix::ucontext_set_pc(uc, StubRoutines::continuation_for_safefetch_fault(pc));
222-
return true;
223-
}
224-
}
225-
226216
// decide if this trap can be handled by a stub
227217
address stub = NULL;
228218
address pc = NULL;

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

-10
Original file line numberDiff line numberDiff line change
@@ -207,16 +207,6 @@ frame os::current_frame() {
207207
bool PosixSignals::pd_hotspot_signal_handler(int sig, siginfo_t* info,
208208
ucontext_t* uc, JavaThread* thread) {
209209

210-
// Moved SafeFetch32 handling outside thread!=NULL conditional block to make
211-
// it work if no associated JavaThread object exists.
212-
if (uc) {
213-
address const pc = os::Posix::ucontext_get_pc(uc);
214-
if (pc && StubRoutines::is_safefetch_fault(pc)) {
215-
os::Posix::ucontext_set_pc(uc, StubRoutines::continuation_for_safefetch_fault(pc));
216-
return true;
217-
}
218-
}
219-
220210
// Decide if this trap can be handled by a stub.
221211
address stub = NULL;
222212
address pc = NULL; // Pc as retrieved from PSW. Usually points past failing instruction.

‎src/hotspot/os_cpu/linux_x86/os_linux_x86.cpp

-5
Original file line numberDiff line numberDiff line change
@@ -221,11 +221,6 @@ bool PosixSignals::pd_hotspot_signal_handler(int sig, siginfo_t* info,
221221
if (info != NULL && uc != NULL && thread != NULL) {
222222
pc = (address) os::Posix::ucontext_get_pc(uc);
223223

224-
if (StubRoutines::is_safefetch_fault(pc)) {
225-
os::Posix::ucontext_set_pc(uc, StubRoutines::continuation_for_safefetch_fault(pc));
226-
return true;
227-
}
228-
229224
#ifndef AMD64
230225
// Halt if SI_KERNEL before more crashes get misdiagnosed as Java bugs
231226
// This can happen in any running code (currently more frequently in

‎src/hotspot/os_cpu/linux_zero/os_linux_zero.cpp

-12
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,6 @@
5353
#include "utilities/events.hpp"
5454
#include "utilities/vmError.hpp"
5555

56-
// See stubGenerator_zero.cpp
57-
#include <setjmp.h>
58-
extern sigjmp_buf* get_jmp_buf_for_continuation();
59-
6056
address os::current_stack_pointer() {
6157
// return the address of the current function
6258
return (address)__builtin_frame_address(0);
@@ -114,14 +110,6 @@ frame os::fetch_frame_from_context(const void* ucVoid) {
114110
bool PosixSignals::pd_hotspot_signal_handler(int sig, siginfo_t* info,
115111
ucontext_t* uc, JavaThread* thread) {
116112

117-
// handle SafeFetch faults
118-
if (sig == SIGSEGV || sig == SIGBUS) {
119-
sigjmp_buf* const pjb = get_jmp_buf_for_continuation();
120-
if (pjb) {
121-
siglongjmp(*pjb, 1);
122-
}
123-
}
124-
125113
if (info != NULL && thread != NULL) {
126114
// Handle ALL stack overflow variations here
127115
if (sig == SIGSEGV) {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
/*
2+
* Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
3+
* Copyright (c) 2020 SAP SE. All rights reserved.
4+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
5+
*
6+
* This code is free software; you can redistribute it and/or modify it
7+
* under the terms of the GNU General Public License version 2 only, as
8+
* published by the Free Software Foundation.
9+
*
10+
* This code is distributed in the hope that it will be useful, but WITHOUT
11+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
12+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
13+
* version 2 for more details (a copy is included in the LICENSE file that
14+
* accompanied this code).
15+
*
16+
* You should have received a copy of the GNU General Public License version
17+
* 2 along with this work; if not, write to the Free Software Foundation,
18+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
19+
*
20+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
21+
* or visit www.oracle.com if you need additional information or have any
22+
* questions.
23+
*/
24+
25+
#include "precompiled.hpp"
26+
#include "runtime/interfaceSupport.inline.hpp"
27+
#include "runtime/stubRoutines.hpp"
28+
#include "runtime/vmOperations.hpp"
29+
#include "runtime/vmThread.hpp"
30+
#include "utilities/globalDefinitions.hpp"
31+
#include "unittest.hpp"
32+
33+
static const intptr_t pattern = LP64_ONLY(0xABCDABCDABCDABCDULL) NOT_LP64(0xABCDABCD);
34+
static intptr_t* invalid_address = (intptr_t*)(intptr_t) NOT_AIX(os::min_page_size()) AIX_ONLY(-1);
35+
36+
TEST_VM(os, safefetch_positive) {
37+
intptr_t v = pattern;
38+
intptr_t a = SafeFetchN(&v, 1);
39+
ASSERT_EQ(v, a);
40+
}
41+
42+
#ifndef _WIN32
43+
// Needs JDK-8185734 to be solved
44+
TEST_VM(os, safefetch_negative) {
45+
intptr_t a = SafeFetchN(invalid_address, pattern);
46+
ASSERT_EQ(pattern, a);
47+
a = SafeFetchN(invalid_address, ~pattern);
48+
ASSERT_EQ(~pattern, a);
49+
}
50+
#endif // _WIN32
51+
52+
class VM_TestSafeFetchAtSafePoint : public VM_GTestExecuteAtSafepoint {
53+
public:
54+
void doit() {
55+
// Regression test for JDK-8257828
56+
// Should not crash.
57+
intptr_t a = SafeFetchN(invalid_address, pattern);
58+
ASSERT_EQ(pattern, a);
59+
a = SafeFetchN(invalid_address, ~pattern);
60+
ASSERT_EQ(~pattern, a);
61+
}
62+
};
63+
64+
TEST_VM(os, safefetch_negative_at_safepoint) {
65+
VM_TestSafeFetchAtSafePoint op;
66+
ThreadInVMfromNative invm(JavaThread::current());
67+
VMThread::execute(&op);
68+
}

0 commit comments

Comments
 (0)
This repository has been archived.