From 8e903eeb1fa04130fa7f154870ffcb1eae36c070 Mon Sep 17 00:00:00 2001 From: Erik Gahlin Date: Wed, 5 Jun 2024 12:35:24 +0000 Subject: [PATCH] 8331896: JFR: Improve check for JDK classes Reviewed-by: mgronlun --- .../share/classes/jdk/jfr/internal/JVMUpcalls.java | 7 ++++--- .../classes/jdk/jfr/internal/MetadataRepository.java | 2 +- .../share/classes/jdk/jfr/internal/MirrorEvents.java | 3 ++- .../share/classes/jdk/jfr/internal/TypeLibrary.java | 8 +++++--- .../jdk/jfr/internal/periodic/JDKEventTask.java | 11 ++++++----- .../share/classes/jdk/jfr/internal/util/Utils.java | 8 ++++++++ 6 files changed, 26 insertions(+), 13 deletions(-) diff --git a/src/jdk.jfr/share/classes/jdk/jfr/internal/JVMUpcalls.java b/src/jdk.jfr/share/classes/jdk/jfr/internal/JVMUpcalls.java index 2eed7b918fd..6d1d5cf7938 100644 --- a/src/jdk.jfr/share/classes/jdk/jfr/internal/JVMUpcalls.java +++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/JVMUpcalls.java @@ -28,6 +28,7 @@ import java.lang.reflect.Modifier; import jdk.jfr.internal.event.EventConfiguration; import jdk.jfr.internal.util.Bytecode; +import jdk.jfr.internal.util.Utils; /** * 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 { try { 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"); return oldBytes; } @@ -70,9 +71,9 @@ final class JVMUpcalls { // Probably triggered by some other agent 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"); - EventInstrumentation ei = new EventInstrumentation(clazz.getSuperclass(), oldBytes, traceId, bootClassLoader, false); + EventInstrumentation ei = new EventInstrumentation(clazz.getSuperclass(), oldBytes, traceId, jdkClass, false); byte[] bytes = ei.buildInstrumented(); Bytecode.log(clazz.getName(), bytes); return bytes; diff --git a/src/jdk.jfr/share/classes/jdk/jfr/internal/MetadataRepository.java b/src/jdk.jfr/share/classes/jdk/jfr/internal/MetadataRepository.java index 9a2bbfe4c63..7031718cb28 100644 --- a/src/jdk.jfr/share/classes/jdk/jfr/internal/MetadataRepository.java +++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/MetadataRepository.java @@ -204,7 +204,7 @@ public final class MetadataRepository { // 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 // native type IDs, without specialized Java logic for each type. - if (eventClass.getClassLoader() == null) { + if (Utils.isJDKClass(eventClass)) { Name name = eventClass.getAnnotation(Name.class); if (name != null) { String n = name.value(); diff --git a/src/jdk.jfr/share/classes/jdk/jfr/internal/MirrorEvents.java b/src/jdk.jfr/share/classes/jdk/jfr/internal/MirrorEvents.java index 35eadff40d1..e8a26fe425d 100644 --- a/src/jdk.jfr/share/classes/jdk/jfr/internal/MirrorEvents.java +++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/MirrorEvents.java @@ -48,6 +48,7 @@ import jdk.jfr.events.VirtualThreadStartEvent; import jdk.jfr.events.VirtualThreadSubmitFailedEvent; import jdk.jfr.events.X509CertificateEvent; import jdk.jfr.events.X509ValidationEvent; +import jdk.jfr.internal.util.Utils; /** * This class registers all mirror events. @@ -85,7 +86,7 @@ final class MirrorEvents { } static Class find(Class eventClass) { - return find(eventClass.getClassLoader() == null, eventClass.getName()); + return find(Utils.isJDKClass(eventClass), eventClass.getName()); } static Class find(boolean bootClassLoader, String name) { diff --git a/src/jdk.jfr/share/classes/jdk/jfr/internal/TypeLibrary.java b/src/jdk.jfr/share/classes/jdk/jfr/internal/TypeLibrary.java index cab6e7befe1..350bf55ccd4 100644 --- a/src/jdk.jfr/share/classes/jdk/jfr/internal/TypeLibrary.java +++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/TypeLibrary.java @@ -166,7 +166,9 @@ public final class TypeLibrary { for (ValueDescriptor v : type.getFields()) { 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; } @@ -220,7 +222,7 @@ public final class TypeLibrary { long id = Type.getTypeId(clazz); Type t; if (eventType) { - t = new PlatformEventType(typeName, id, clazz.getClassLoader() == null, true); + t = new PlatformEventType(typeName, id, Utils.isJDKClass(clazz), true); } else { t = new Type(typeName, superType, id); } @@ -283,7 +285,7 @@ public final class TypeLibrary { } addAnnotations(clazz, type, dynamicAnnotations); - if (clazz.getClassLoader() == null) { + if (Utils.isJDKClass(clazz)) { type.log("Added", LogTag.JFR_SYSTEM_METADATA, LogLevel.INFO); } else { type.log("Added", LogTag.JFR_METADATA, LogLevel.INFO); diff --git a/src/jdk.jfr/share/classes/jdk/jfr/internal/periodic/JDKEventTask.java b/src/jdk.jfr/share/classes/jdk/jfr/internal/periodic/JDKEventTask.java index 73d4f5479eb..a891b1c3775 100644 --- a/src/jdk.jfr/share/classes/jdk/jfr/internal/periodic/JDKEventTask.java +++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/periodic/JDKEventTask.java @@ -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. * * This code is free software; you can redistribute it and/or modify it @@ -25,6 +25,7 @@ package jdk.jfr.internal.periodic; import jdk.internal.event.Event; +import jdk.jfr.internal.util.Utils; /** * 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()) { throw new InternalError("Must be a JDK event"); } - if (eventClass.getClassLoader() != null) { - throw new SecurityException("Periodic task can only be registered for event classes that are loaded by the bootstrap class loader"); + if (!Utils.isJDKClass(eventClass)) { + throw new SecurityException("Periodic task can only be registered for event classes that belongs to the JDK"); } - if (runnable.getClass().getClassLoader() != null) { - throw new SecurityException("Runnable class must be loaded by the bootstrap class loader"); + if (!Utils.isJDKClass(runnable.getClass())) { + throw new SecurityException("Runnable class must belong to the JDK"); } } diff --git a/src/jdk.jfr/share/classes/jdk/jfr/internal/util/Utils.java b/src/jdk.jfr/share/classes/jdk/jfr/internal/util/Utils.java index 53b3599f9d9..0049662e9a1 100644 --- a/src/jdk.jfr/share/classes/jdk/jfr/internal/util/Utils.java +++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/util/Utils.java @@ -435,4 +435,12 @@ public final class Utils { } 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. + } }