Skip to content

Commit 8e903ee

Browse files
committed
8331896: JFR: Improve check for JDK classes
Reviewed-by: mgronlun
1 parent 3cbdf8d commit 8e903ee

File tree

6 files changed

+26
-13
lines changed

6 files changed

+26
-13
lines changed

src/jdk.jfr/share/classes/jdk/jfr/internal/JVMUpcalls.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828

2929
import jdk.jfr.internal.event.EventConfiguration;
3030
import jdk.jfr.internal.util.Bytecode;
31+
import jdk.jfr.internal.util.Utils;
3132
/**
3233
* All upcalls from the JVM should go through this class.
3334
*
@@ -59,7 +60,7 @@ final class JVMUpcalls {
5960
static byte[] onRetransform(long traceId, boolean dummy1, boolean dummy2, Class<?> clazz, byte[] oldBytes) throws Throwable {
6061
try {
6162
if (jdk.internal.event.Event.class.isAssignableFrom(clazz) && !Modifier.isAbstract(clazz.getModifiers())) {
62-
if (!JVMSupport.shouldInstrument(clazz.getClassLoader() == null, clazz.getName())) {
63+
if (!JVMSupport.shouldInstrument(Utils.isJDKClass(clazz), clazz.getName())) {
6364
Logger.log(LogTag.JFR_SYSTEM, LogLevel.INFO, "Skipping instrumentation for " + clazz.getName() + " since container support is missing");
6465
return oldBytes;
6566
}
@@ -70,9 +71,9 @@ static byte[] onRetransform(long traceId, boolean dummy1, boolean dummy2, Class<
7071
// Probably triggered by some other agent
7172
return oldBytes;
7273
}
73-
boolean bootClassLoader = clazz.getClassLoader() == null;
74+
boolean jdkClass = Utils.isJDKClass(clazz);
7475
Logger.log(LogTag.JFR_SYSTEM, LogLevel.INFO, "Adding instrumentation to event class " + clazz.getName() + " using retransform");
75-
EventInstrumentation ei = new EventInstrumentation(clazz.getSuperclass(), oldBytes, traceId, bootClassLoader, false);
76+
EventInstrumentation ei = new EventInstrumentation(clazz.getSuperclass(), oldBytes, traceId, jdkClass, false);
7677
byte[] bytes = ei.buildInstrumented();
7778
Bytecode.log(clazz.getName(), bytes);
7879
return bytes;

src/jdk.jfr/share/classes/jdk/jfr/internal/MetadataRepository.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ private EventConfiguration makeConfiguration(Class<? extends jdk.internal.event.
204204
// and assign the result to a long field is not enough to always get a proper
205205
// stack trace. Purpose of the mechanism is to transfer metadata, such as
206206
// native type IDs, without specialized Java logic for each type.
207-
if (eventClass.getClassLoader() == null) {
207+
if (Utils.isJDKClass(eventClass)) {
208208
Name name = eventClass.getAnnotation(Name.class);
209209
if (name != null) {
210210
String n = name.value();

src/jdk.jfr/share/classes/jdk/jfr/internal/MirrorEvents.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
import jdk.jfr.events.VirtualThreadSubmitFailedEvent;
4949
import jdk.jfr.events.X509CertificateEvent;
5050
import jdk.jfr.events.X509ValidationEvent;
51+
import jdk.jfr.internal.util.Utils;
5152

5253
/**
5354
* This class registers all mirror events.
@@ -85,7 +86,7 @@ private static void register(String eventClassName, Class<? extends MirrorEvent>
8586
}
8687

8788
static Class<? extends MirrorEvent> find(Class<? extends jdk.internal.event.Event> eventClass) {
88-
return find(eventClass.getClassLoader() == null, eventClass.getName());
89+
return find(Utils.isJDKClass(eventClass), eventClass.getName());
8990
}
9091

9192
static Class<? extends MirrorEvent> find(boolean bootClassLoader, String name) {

src/jdk.jfr/share/classes/jdk/jfr/internal/TypeLibrary.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,9 @@ public static synchronized AnnotationElement createAnnotation(Annotation annotat
166166
for (ValueDescriptor v : type.getFields()) {
167167
values.add(invokeAnnotation(annotation, v.getName()));
168168
}
169-
return PrivateAccess.getInstance().newAnnotation(type, values, annotation.annotationType().getClassLoader() == null);
169+
// Only annotation classes in the boot class loader can always be resolved.
170+
boolean bootClassLoader = annotationType.getClassLoader() == null;
171+
return PrivateAccess.getInstance().newAnnotation(type, values, bootClassLoader);
170172
}
171173
return null;
172174
}
@@ -220,7 +222,7 @@ private static Type defineType(Class<?> clazz, String superType, boolean eventTy
220222
long id = Type.getTypeId(clazz);
221223
Type t;
222224
if (eventType) {
223-
t = new PlatformEventType(typeName, id, clazz.getClassLoader() == null, true);
225+
t = new PlatformEventType(typeName, id, Utils.isJDKClass(clazz), true);
224226
} else {
225227
t = new Type(typeName, superType, id);
226228
}
@@ -283,7 +285,7 @@ public static synchronized Type createType(Class<?> clazz, List<AnnotationElemen
283285
}
284286
addAnnotations(clazz, type, dynamicAnnotations);
285287

286-
if (clazz.getClassLoader() == null) {
288+
if (Utils.isJDKClass(clazz)) {
287289
type.log("Added", LogTag.JFR_SYSTEM_METADATA, LogLevel.INFO);
288290
} else {
289291
type.log("Added", LogTag.JFR_METADATA, LogLevel.INFO);

src/jdk.jfr/share/classes/jdk/jfr/internal/periodic/JDKEventTask.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2023, 2024, 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
@@ -25,6 +25,7 @@
2525
package jdk.jfr.internal.periodic;
2626

2727
import jdk.internal.event.Event;
28+
import jdk.jfr.internal.util.Utils;
2829

2930
/**
3031
* Periodic task that runs trusted code that doesn't require an access control
@@ -39,11 +40,11 @@ public JDKEventTask(Class<? extends Event> eventClass, Runnable runnable) {
3940
if (!getEventType().isJDK()) {
4041
throw new InternalError("Must be a JDK event");
4142
}
42-
if (eventClass.getClassLoader() != null) {
43-
throw new SecurityException("Periodic task can only be registered for event classes that are loaded by the bootstrap class loader");
43+
if (!Utils.isJDKClass(eventClass)) {
44+
throw new SecurityException("Periodic task can only be registered for event classes that belongs to the JDK");
4445
}
45-
if (runnable.getClass().getClassLoader() != null) {
46-
throw new SecurityException("Runnable class must be loaded by the bootstrap class loader");
46+
if (!Utils.isJDKClass(runnable.getClass())) {
47+
throw new SecurityException("Runnable class must belong to the JDK");
4748
}
4849
}
4950

src/jdk.jfr/share/classes/jdk/jfr/internal/util/Utils.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,4 +435,12 @@ public static String format(String template, Map<String, String> parameters) {
435435
}
436436
return sb.toString();
437437
}
438+
439+
public static boolean isJDKClass(Class<?> type) {
440+
return type.getClassLoader() == null;
441+
// In the future we might want to also do:
442+
// type.getClassLoader() == ClassLoader.getPlatformClassLoader();
443+
// but only if it is safe and there is a mechanism to register event
444+
// classes in other modules besides jdk.jfr and java.base.
445+
}
438446
}

0 commit comments

Comments
 (0)