8331896: JFR: Improve check for JDK classes

Reviewed-by: mgronlun
This commit is contained in:
Erik Gahlin 2024-06-05 12:35:24 +00:00
parent 3cbdf8d4d4
commit 8e903eeb1f
6 changed files with 26 additions and 13 deletions

View File

@ -28,6 +28,7 @@ import java.lang.reflect.Modifier;
import jdk.jfr.internal.event.EventConfiguration; import jdk.jfr.internal.event.EventConfiguration;
import jdk.jfr.internal.util.Bytecode; import jdk.jfr.internal.util.Bytecode;
import jdk.jfr.internal.util.Utils;
/** /**
* All upcalls from the JVM should go through this class. * All upcalls from the JVM should go through this class.
* *
@ -59,7 +60,7 @@ final class JVMUpcalls {
static byte[] onRetransform(long traceId, boolean dummy1, boolean dummy2, Class<?> clazz, byte[] oldBytes) throws Throwable { static byte[] onRetransform(long traceId, boolean dummy1, boolean dummy2, Class<?> clazz, byte[] oldBytes) throws Throwable {
try { try {
if (jdk.internal.event.Event.class.isAssignableFrom(clazz) && !Modifier.isAbstract(clazz.getModifiers())) { if (jdk.internal.event.Event.class.isAssignableFrom(clazz) && !Modifier.isAbstract(clazz.getModifiers())) {
if (!JVMSupport.shouldInstrument(clazz.getClassLoader() == null, clazz.getName())) { if (!JVMSupport.shouldInstrument(Utils.isJDKClass(clazz), clazz.getName())) {
Logger.log(LogTag.JFR_SYSTEM, LogLevel.INFO, "Skipping instrumentation for " + clazz.getName() + " since container support is missing"); Logger.log(LogTag.JFR_SYSTEM, LogLevel.INFO, "Skipping instrumentation for " + clazz.getName() + " since container support is missing");
return oldBytes; return oldBytes;
} }
@ -70,9 +71,9 @@ final class JVMUpcalls {
// Probably triggered by some other agent // Probably triggered by some other agent
return oldBytes; return oldBytes;
} }
boolean bootClassLoader = clazz.getClassLoader() == null; boolean jdkClass = Utils.isJDKClass(clazz);
Logger.log(LogTag.JFR_SYSTEM, LogLevel.INFO, "Adding instrumentation to event class " + clazz.getName() + " using retransform"); Logger.log(LogTag.JFR_SYSTEM, LogLevel.INFO, "Adding instrumentation to event class " + clazz.getName() + " using retransform");
EventInstrumentation ei = new EventInstrumentation(clazz.getSuperclass(), oldBytes, traceId, bootClassLoader, false); EventInstrumentation ei = new EventInstrumentation(clazz.getSuperclass(), oldBytes, traceId, jdkClass, false);
byte[] bytes = ei.buildInstrumented(); byte[] bytes = ei.buildInstrumented();
Bytecode.log(clazz.getName(), bytes); Bytecode.log(clazz.getName(), bytes);
return bytes; return bytes;

View File

@ -204,7 +204,7 @@ public final class MetadataRepository {
// and assign the result to a long field is not enough to always get a proper // and assign the result to a long field is not enough to always get a proper
// stack trace. Purpose of the mechanism is to transfer metadata, such as // stack trace. Purpose of the mechanism is to transfer metadata, such as
// native type IDs, without specialized Java logic for each type. // native type IDs, without specialized Java logic for each type.
if (eventClass.getClassLoader() == null) { if (Utils.isJDKClass(eventClass)) {
Name name = eventClass.getAnnotation(Name.class); Name name = eventClass.getAnnotation(Name.class);
if (name != null) { if (name != null) {
String n = name.value(); String n = name.value();

View File

@ -48,6 +48,7 @@ import jdk.jfr.events.VirtualThreadStartEvent;
import jdk.jfr.events.VirtualThreadSubmitFailedEvent; import jdk.jfr.events.VirtualThreadSubmitFailedEvent;
import jdk.jfr.events.X509CertificateEvent; import jdk.jfr.events.X509CertificateEvent;
import jdk.jfr.events.X509ValidationEvent; import jdk.jfr.events.X509ValidationEvent;
import jdk.jfr.internal.util.Utils;
/** /**
* This class registers all mirror events. * This class registers all mirror events.
@ -85,7 +86,7 @@ final class MirrorEvents {
} }
static Class<? extends MirrorEvent> find(Class<? extends jdk.internal.event.Event> eventClass) { static Class<? extends MirrorEvent> find(Class<? extends jdk.internal.event.Event> eventClass) {
return find(eventClass.getClassLoader() == null, eventClass.getName()); return find(Utils.isJDKClass(eventClass), eventClass.getName());
} }
static Class<? extends MirrorEvent> find(boolean bootClassLoader, String name) { static Class<? extends MirrorEvent> find(boolean bootClassLoader, String name) {

View File

@ -166,7 +166,9 @@ public final class TypeLibrary {
for (ValueDescriptor v : type.getFields()) { for (ValueDescriptor v : type.getFields()) {
values.add(invokeAnnotation(annotation, v.getName())); values.add(invokeAnnotation(annotation, v.getName()));
} }
return PrivateAccess.getInstance().newAnnotation(type, values, annotation.annotationType().getClassLoader() == null); // Only annotation classes in the boot class loader can always be resolved.
boolean bootClassLoader = annotationType.getClassLoader() == null;
return PrivateAccess.getInstance().newAnnotation(type, values, bootClassLoader);
} }
return null; return null;
} }
@ -220,7 +222,7 @@ public final class TypeLibrary {
long id = Type.getTypeId(clazz); long id = Type.getTypeId(clazz);
Type t; Type t;
if (eventType) { if (eventType) {
t = new PlatformEventType(typeName, id, clazz.getClassLoader() == null, true); t = new PlatformEventType(typeName, id, Utils.isJDKClass(clazz), true);
} else { } else {
t = new Type(typeName, superType, id); t = new Type(typeName, superType, id);
} }
@ -283,7 +285,7 @@ public final class TypeLibrary {
} }
addAnnotations(clazz, type, dynamicAnnotations); addAnnotations(clazz, type, dynamicAnnotations);
if (clazz.getClassLoader() == null) { if (Utils.isJDKClass(clazz)) {
type.log("Added", LogTag.JFR_SYSTEM_METADATA, LogLevel.INFO); type.log("Added", LogTag.JFR_SYSTEM_METADATA, LogLevel.INFO);
} else { } else {
type.log("Added", LogTag.JFR_METADATA, LogLevel.INFO); type.log("Added", LogTag.JFR_METADATA, LogLevel.INFO);

View File

@ -1,5 +1,5 @@
/* /*
* Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved. * Copyright (c) 2023, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
* *
* This code is free software; you can redistribute it and/or modify it * This code is free software; you can redistribute it and/or modify it
@ -25,6 +25,7 @@
package jdk.jfr.internal.periodic; package jdk.jfr.internal.periodic;
import jdk.internal.event.Event; import jdk.internal.event.Event;
import jdk.jfr.internal.util.Utils;
/** /**
* Periodic task that runs trusted code that doesn't require an access control * Periodic task that runs trusted code that doesn't require an access control
@ -39,11 +40,11 @@ final class JDKEventTask extends JavaEventTask {
if (!getEventType().isJDK()) { if (!getEventType().isJDK()) {
throw new InternalError("Must be a JDK event"); throw new InternalError("Must be a JDK event");
} }
if (eventClass.getClassLoader() != null) { if (!Utils.isJDKClass(eventClass)) {
throw new SecurityException("Periodic task can only be registered for event classes that are loaded by the bootstrap class loader"); throw new SecurityException("Periodic task can only be registered for event classes that belongs to the JDK");
} }
if (runnable.getClass().getClassLoader() != null) { if (!Utils.isJDKClass(runnable.getClass())) {
throw new SecurityException("Runnable class must be loaded by the bootstrap class loader"); throw new SecurityException("Runnable class must belong to the JDK");
} }
} }

View File

@ -435,4 +435,12 @@ public final class Utils {
} }
return sb.toString(); return sb.toString();
} }
public static boolean isJDKClass(Class<?> type) {
return type.getClassLoader() == null;
// In the future we might want to also do:
// type.getClassLoader() == ClassLoader.getPlatformClassLoader();
// but only if it is safe and there is a mechanism to register event
// classes in other modules besides jdk.jfr and java.base.
}
} }