From c4b81e2dc5a854efde8475d09b33b8f53dde987d Mon Sep 17 00:00:00 2001 From: Richard Reingruber Date: Fri, 14 Feb 2025 16:40:49 +0100 Subject: [PATCH 1/4] Handle attempt to sample stack while handling SIGTRAP --- src/hotspot/os_cpu/aix_ppc/javaThread_aix_ppc.cpp | 10 ++++++++-- src/hotspot/os_cpu/linux_ppc/javaThread_linux_ppc.cpp | 10 ++++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/hotspot/os_cpu/aix_ppc/javaThread_aix_ppc.cpp b/src/hotspot/os_cpu/aix_ppc/javaThread_aix_ppc.cpp index 7cd57b65d32b3..3c500a514c080 100644 --- a/src/hotspot/os_cpu/aix_ppc/javaThread_aix_ppc.cpp +++ b/src/hotspot/os_cpu/aix_ppc/javaThread_aix_ppc.cpp @@ -1,6 +1,6 @@ /* * Copyright (c) 1997, 2025, Oracle and/or its affiliates. All rights reserved. - * Copyright (c) 2012, 2024 SAP SE. All rights reserved. + * Copyright (c) 2012, 2025 SAP SE. All rights reserved. * Copyright (c) 2022, IBM Corp. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * @@ -66,7 +66,13 @@ bool JavaThread::pd_get_top_frame_for_profiling(frame* fr_addr, void* ucontext, return false; } - // pc could refer to a native address outside the code cache even though the thread isInJava. + if (!CodeCache::contains(pc)) { + // The thread `isInJava` but its pc is not in the CodeCache. This is uncommon. It + // can occur, for instance, if we're trying to sample the stack while actually + // handling a SIGTRAP (e.g. because of a range check). It is not safe to do that. + return false; + } + frame ret_frame((intptr_t*)uc->uc_mcontext.jmp_context.gpr[1/*REG_SP*/], pc, frame::kind::unknown); if (ret_frame.fp() == nullptr) { diff --git a/src/hotspot/os_cpu/linux_ppc/javaThread_linux_ppc.cpp b/src/hotspot/os_cpu/linux_ppc/javaThread_linux_ppc.cpp index a1c3d616eea5d..a10e64f2e0c01 100644 --- a/src/hotspot/os_cpu/linux_ppc/javaThread_linux_ppc.cpp +++ b/src/hotspot/os_cpu/linux_ppc/javaThread_linux_ppc.cpp @@ -1,6 +1,6 @@ /* * Copyright (c) 1997, 2025, Oracle and/or its affiliates. All rights reserved. - * Copyright (c) 2012, 2024 SAP SE. All rights reserved. + * Copyright (c) 2012, 2025 SAP SE. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -65,7 +65,13 @@ bool JavaThread::pd_get_top_frame_for_profiling(frame* fr_addr, void* ucontext, return false; } - // pc could refer to a native address outside the code cache even though the thread isInJava. + if (!CodeCache::contains(pc)) { + // The thread `isInJava` but its pc is not in the CodeCache. This is uncommon. It + // can occur, for instance, if we're trying to sample the stack while actually + // handling a SIGTRAP (e.g. because of a range check). It is not safe to do that. + return false; + } + frame ret_frame((intptr_t*)uc->uc_mcontext.regs->gpr[1/*REG_SP*/], pc, frame::kind::unknown); if (ret_frame.fp() == nullptr) { From 17d8032d14676c0ea23a38f312946a152f6cdb63 Mon Sep 17 00:00:00 2001 From: Richard Reingruber Date: Wed, 26 Feb 2025 11:08:35 +0100 Subject: [PATCH 2/4] Revert first fix This reverts commit c4b81e2dc5a854efde8475d09b33b8f53dde987d. --- src/hotspot/os_cpu/aix_ppc/javaThread_aix_ppc.cpp | 10 ++-------- src/hotspot/os_cpu/linux_ppc/javaThread_linux_ppc.cpp | 10 ++-------- 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/src/hotspot/os_cpu/aix_ppc/javaThread_aix_ppc.cpp b/src/hotspot/os_cpu/aix_ppc/javaThread_aix_ppc.cpp index 3c500a514c080..7cd57b65d32b3 100644 --- a/src/hotspot/os_cpu/aix_ppc/javaThread_aix_ppc.cpp +++ b/src/hotspot/os_cpu/aix_ppc/javaThread_aix_ppc.cpp @@ -1,6 +1,6 @@ /* * Copyright (c) 1997, 2025, Oracle and/or its affiliates. All rights reserved. - * Copyright (c) 2012, 2025 SAP SE. All rights reserved. + * Copyright (c) 2012, 2024 SAP SE. All rights reserved. * Copyright (c) 2022, IBM Corp. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * @@ -66,13 +66,7 @@ bool JavaThread::pd_get_top_frame_for_profiling(frame* fr_addr, void* ucontext, return false; } - if (!CodeCache::contains(pc)) { - // The thread `isInJava` but its pc is not in the CodeCache. This is uncommon. It - // can occur, for instance, if we're trying to sample the stack while actually - // handling a SIGTRAP (e.g. because of a range check). It is not safe to do that. - return false; - } - + // pc could refer to a native address outside the code cache even though the thread isInJava. frame ret_frame((intptr_t*)uc->uc_mcontext.jmp_context.gpr[1/*REG_SP*/], pc, frame::kind::unknown); if (ret_frame.fp() == nullptr) { diff --git a/src/hotspot/os_cpu/linux_ppc/javaThread_linux_ppc.cpp b/src/hotspot/os_cpu/linux_ppc/javaThread_linux_ppc.cpp index a10e64f2e0c01..a1c3d616eea5d 100644 --- a/src/hotspot/os_cpu/linux_ppc/javaThread_linux_ppc.cpp +++ b/src/hotspot/os_cpu/linux_ppc/javaThread_linux_ppc.cpp @@ -1,6 +1,6 @@ /* * Copyright (c) 1997, 2025, Oracle and/or its affiliates. All rights reserved. - * Copyright (c) 2012, 2025 SAP SE. All rights reserved. + * Copyright (c) 2012, 2024 SAP SE. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -65,13 +65,7 @@ bool JavaThread::pd_get_top_frame_for_profiling(frame* fr_addr, void* ucontext, return false; } - if (!CodeCache::contains(pc)) { - // The thread `isInJava` but its pc is not in the CodeCache. This is uncommon. It - // can occur, for instance, if we're trying to sample the stack while actually - // handling a SIGTRAP (e.g. because of a range check). It is not safe to do that. - return false; - } - + // pc could refer to a native address outside the code cache even though the thread isInJava. frame ret_frame((intptr_t*)uc->uc_mcontext.regs->gpr[1/*REG_SP*/], pc, frame::kind::unknown); if (ret_frame.fp() == nullptr) { From 9a2eedc538e7802d80d54fd19ce1241ff29799ce Mon Sep 17 00:00:00 2001 From: Richard Reingruber Date: Wed, 26 Feb 2025 11:28:17 +0100 Subject: [PATCH 3/4] A frame isn't safe_for_sender if sender_pc() returns null --- src/hotspot/cpu/ppc/frame_ppc.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/hotspot/cpu/ppc/frame_ppc.cpp b/src/hotspot/cpu/ppc/frame_ppc.cpp index 38c26e5e4970b..c2c5c55336bdf 100644 --- a/src/hotspot/cpu/ppc/frame_ppc.cpp +++ b/src/hotspot/cpu/ppc/frame_ppc.cpp @@ -1,6 +1,6 @@ /* * Copyright (c) 2000, 2025, Oracle and/or its affiliates. All rights reserved. - * Copyright (c) 2012, 2024 SAP SE. All rights reserved. + * Copyright (c) 2012, 2025 SAP SE. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -195,6 +195,15 @@ bool frame::safe_for_sender(JavaThread *thread) { return false; } + if (sender_pc() == nullptr) { + // Likely the return pc was not yet stored to stack. We rather discard this + // sample also because we would hit an assertion in frame::setup(). We can + // find any other random value if the return pc was not yet stored to + // stack. We rely on consistency checks to handle this (see + // e.g. find_initial_Java_frame()) + return false; + } + return true; } From 92b0ef93bf0b40147402e631127799dfe1d5c3b0 Mon Sep 17 00:00:00 2001 From: Richard Reingruber Date: Wed, 26 Feb 2025 15:00:49 +0100 Subject: [PATCH 4/4] Improve whitespace --- src/hotspot/cpu/ppc/frame_ppc.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hotspot/cpu/ppc/frame_ppc.cpp b/src/hotspot/cpu/ppc/frame_ppc.cpp index c2c5c55336bdf..6b6a792117d4e 100644 --- a/src/hotspot/cpu/ppc/frame_ppc.cpp +++ b/src/hotspot/cpu/ppc/frame_ppc.cpp @@ -196,8 +196,8 @@ bool frame::safe_for_sender(JavaThread *thread) { } if (sender_pc() == nullptr) { - // Likely the return pc was not yet stored to stack. We rather discard this - // sample also because we would hit an assertion in frame::setup(). We can + // Likely the return pc was not yet stored to stack. We rather discard this + // sample also because we would hit an assertion in frame::setup(). We can // find any other random value if the return pc was not yet stored to // stack. We rely on consistency checks to handle this (see // e.g. find_initial_Java_frame())