Skip to content

Commit 721ef76

Browse files
author
Markus Grönlund
committed
8352696: JFR: assert(false): EA: missing memory path
Reviewed-by: thartmann, shade, kvn
1 parent c002b97 commit 721ef76

File tree

2 files changed

+99
-7
lines changed

2 files changed

+99
-7
lines changed

src/hotspot/share/opto/library_call.cpp

+6-7
Original file line numberDiff line numberDiff line change
@@ -3154,7 +3154,8 @@ bool LibraryCallKit::inline_native_jvm_commit() {
31543154
set_control(is_notified);
31553155

31563156
// Reset notified state.
3157-
Node* notified_reset_memory = store_to_memory(control(), notified_offset, _gvn.intcon(0), T_BOOLEAN, MemNode::unordered);
3157+
store_to_memory(control(), notified_offset, _gvn.intcon(0), T_BOOLEAN, MemNode::unordered);
3158+
Node* notified_reset_memory = reset_memory();
31583159

31593160
// Iff notified, the return address of the commit method is the current position of the backing java buffer. This is used to reset the event writer.
31603161
Node* current_pos_X = _gvn.transform(new LoadXNode(control(), input_memory_state, java_buffer_pos_offset, TypeRawPtr::NOTNULL, TypeX_X, MemNode::unordered));
@@ -3172,12 +3173,10 @@ bool LibraryCallKit::inline_native_jvm_commit() {
31723173
Node* next_pos_X = _gvn.transform(ConvL2X(arg));
31733174

31743175
// Store the next_position to the underlying jfr java buffer.
3175-
Node* commit_memory;
3176-
#ifdef _LP64
3177-
commit_memory = store_to_memory(control(), java_buffer_pos_offset, next_pos_X, T_LONG, MemNode::release);
3178-
#else
3179-
commit_memory = store_to_memory(control(), java_buffer_pos_offset, next_pos_X, T_INT, MemNode::release);
3180-
#endif
3176+
store_to_memory(control(), java_buffer_pos_offset, next_pos_X, LP64_ONLY(T_LONG) NOT_LP64(T_INT), MemNode::release);
3177+
3178+
Node* commit_memory = reset_memory();
3179+
set_all_memory(commit_memory);
31813180

31823181
// Now load the flags from off the java buffer and decide if the buffer is a lease. If so, it needs to be returned post-commit.
31833182
Node* java_buffer_flags_offset = _gvn.transform(new AddPNode(top(), java_buffer, _gvn.transform(MakeConX(in_bytes(JFR_BUFFER_FLAGS_OFFSET)))));
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
/*
2+
* Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
24+
package jdk.jfr.jvm;
25+
26+
import java.util.concurrent.Executor;
27+
import java.util.concurrent.ExecutorService;
28+
import java.util.concurrent.Executors;
29+
import java.util.concurrent.ThreadFactory;
30+
import java.util.concurrent.CountDownLatch;
31+
import jdk.jfr.consumer.RecordingStream;
32+
import jdk.test.lib.jfr.EventNames;
33+
34+
/**
35+
*
36+
* A successful execution is when the JTREG test executes successfully,
37+
* i.e., with no exceptions or JVM asserts.
38+
*
39+
* In JVM debug builds, C2 will assert as part of EscapeAnalysis and dump an hs_err<pid>.log:
40+
*
41+
* #
42+
* # A fatal error has been detected by the Java Runtime Environment:
43+
* #
44+
* # Internal Error (..\open\src\hotspot\share\opto\escape.cpp:4788)
45+
* # assert(false) failed: EA: missing memory path
46+
*
47+
* Current thread JavaThread "C2 CompilerThread2"
48+
* Current CompileTask:
49+
* C2:27036 1966 ! 4 java.lang.VirtualThread::run (173 bytes)
50+
*
51+
* ConnectionGraph::split_unique_types+0x2409 (escape.cpp:4788)
52+
* ConnectionGraph::compute_escape+0x11e6 (escape.cpp:397)
53+
* ConnectionGraph::do_analysis+0xf2 (escape.cpp:118)
54+
* Compile::Optimize+0x85d (compile.cpp:2381)
55+
* ...
56+
*
57+
* The error is because C2's Escape Analysis does not recognize a pattern
58+
* where one input of memory Phi node is MergeMem node, and another is RAW store.
59+
* This pattern is created by the jdk.jfr.internal.JVM.commit() intrinsic,
60+
* which is inlined because of inlining the JFR event jdk.VirtualThreadStart.
61+
*
62+
* As a result, EA complains about a strange memory graph.
63+
*/
64+
65+
/**
66+
* @test
67+
* @bug 8352696
68+
* @requires vm.flagless & vm.hasJFR & vm.debug
69+
* @library /test/lib /test/jdk
70+
* @run main/othervm -Xbatch jdk.jfr.jvm.TestJvmCommitIntrinsicAndEA
71+
*/
72+
public final class TestJvmCommitIntrinsicAndEA {
73+
private static final int NUM_TASKS = 10_000;
74+
75+
public static void main(String[] args) throws Throwable {
76+
CountDownLatch latch = new CountDownLatch(NUM_TASKS);
77+
78+
try (RecordingStream rs = new RecordingStream()) {
79+
rs.enable(EventNames.VirtualThreadStart).withoutStackTrace();
80+
rs.enable(EventNames.VirtualThreadEnd).withoutStackTrace();
81+
rs.onEvent(EventNames.VirtualThreadEnd, e -> latch.countDown());
82+
rs.startAsync();
83+
// Execute NUM_TASKS, each in their own virtual thread.
84+
ThreadFactory factory = Thread.ofVirtual().factory();
85+
try (var executor = Executors.newThreadPerTaskExecutor(factory)) {
86+
for (int i = 0; i < NUM_TASKS; i++) {
87+
executor.submit(() -> { });
88+
}
89+
}
90+
latch.await();
91+
}
92+
}
93+
}

0 commit comments

Comments
 (0)