From ed17c55ea34b3b6009dab11d64f21e0b7af3d701 Mon Sep 17 00:00:00 2001 From: Coleen Phillimore Date: Wed, 12 Feb 2025 12:04:40 +0000 Subject: [PATCH] 8349145: Make Class.getProtectionDomain() non-native Reviewed-by: liach, dholmes, yzheng --- src/hotspot/share/classfile/javaClasses.cpp | 3 +- src/hotspot/share/classfile/javaClasses.hpp | 1 - src/hotspot/share/classfile/vmSymbols.hpp | 1 + src/hotspot/share/prims/jvm.cpp | 16 --------- .../share/classes/java/lang/Class.java | 36 ++++++++----------- .../share/classes/java/lang/System.java | 4 +-- .../jdk/internal/reflect/Reflection.java | 2 +- src/java.base/share/native/libjava/Class.c | 2 -- .../hotspot/gtest/oops/test_instanceKlass.cpp | 2 +- .../ci/runtime/test/TestResolvedJavaType.java | 5 ++- .../ModuleSetAccessibleTest.java | 3 +- .../TrySetAccessibleTest.java | 3 +- .../reflect/Reflection/Filtering.java | 3 +- 13 files changed, 31 insertions(+), 50 deletions(-) diff --git a/src/hotspot/share/classfile/javaClasses.cpp b/src/hotspot/share/classfile/javaClasses.cpp index c286a33c17a..d6d1f799253 100644 --- a/src/hotspot/share/classfile/javaClasses.cpp +++ b/src/hotspot/share/classfile/javaClasses.cpp @@ -1500,7 +1500,8 @@ oop java_lang_Class::primitive_mirror(BasicType t) { macro(_classData_offset, k, "classData", object_signature, false); \ macro(_reflectionData_offset, k, "reflectionData", java_lang_ref_SoftReference_signature, false); \ macro(_signers_offset, k, "signers", object_array_signature, false); \ - macro(_modifiers_offset, k, vmSymbols::modifiers_name(), int_signature, false); + macro(_modifiers_offset, k, vmSymbols::modifiers_name(), int_signature, false); \ + macro(_protection_domain_offset, k, "protectionDomain", java_security_ProtectionDomain_signature, false); void java_lang_Class::compute_offsets() { if (_offsets_computed) { diff --git a/src/hotspot/share/classfile/javaClasses.hpp b/src/hotspot/share/classfile/javaClasses.hpp index 317c4752f56..43b358f6c46 100644 --- a/src/hotspot/share/classfile/javaClasses.hpp +++ b/src/hotspot/share/classfile/javaClasses.hpp @@ -228,7 +228,6 @@ class java_lang_String : AllStatic { macro(java_lang_Class, array_klass, intptr_signature, false) \ macro(java_lang_Class, oop_size, int_signature, false) \ macro(java_lang_Class, static_oop_field_count, int_signature, false) \ - macro(java_lang_Class, protection_domain, object_signature, false) \ macro(java_lang_Class, source_file, object_signature, false) \ macro(java_lang_Class, init_lock, object_signature, false) diff --git a/src/hotspot/share/classfile/vmSymbols.hpp b/src/hotspot/share/classfile/vmSymbols.hpp index 867a992839e..622fa8640a6 100644 --- a/src/hotspot/share/classfile/vmSymbols.hpp +++ b/src/hotspot/share/classfile/vmSymbols.hpp @@ -306,6 +306,7 @@ class SerializeClosure; template(jdk_internal_vm_annotation_JvmtiMountTransition_signature, "Ljdk/internal/vm/annotation/JvmtiMountTransition;") \ \ template(java_lang_ref_SoftReference_signature, "Ljava/lang/ref/SoftReference;") \ + template(java_security_ProtectionDomain_signature, "Ljava/security/ProtectionDomain;") \ \ /* Support for JSR 292 & invokedynamic (JDK 1.7 and above) */ \ template(java_lang_invoke_CallSite, "java/lang/invoke/CallSite") \ diff --git a/src/hotspot/share/prims/jvm.cpp b/src/hotspot/share/prims/jvm.cpp index f3015c12972..33d82045b6e 100644 --- a/src/hotspot/share/prims/jvm.cpp +++ b/src/hotspot/share/prims/jvm.cpp @@ -1211,22 +1211,6 @@ JVM_ENTRY(jboolean, JVM_IsHiddenClass(JNIEnv *env, jclass cls)) JVM_END -JVM_ENTRY(jobject, JVM_GetProtectionDomain(JNIEnv *env, jclass cls)) - oop mirror = JNIHandles::resolve_non_null(cls); - if (mirror == nullptr) { - THROW_(vmSymbols::java_lang_NullPointerException(), nullptr); - } - - if (java_lang_Class::is_primitive(mirror)) { - // Primitive types does not have a protection domain. - return nullptr; - } - - oop pd = java_lang_Class::protection_domain(mirror); - return (jobject) JNIHandles::make_local(THREAD, pd); -JVM_END - - class ScopedValueBindingsResolver { public: InstanceKlass* Carrier_klass; diff --git a/src/java.base/share/classes/java/lang/Class.java b/src/java.base/share/classes/java/lang/Class.java index 68921601eca..4607858faa8 100644 --- a/src/java.base/share/classes/java/lang/Class.java +++ b/src/java.base/share/classes/java/lang/Class.java @@ -236,12 +236,13 @@ public final class Class implements java.io.Serializable, * This constructor is not used and prevents the default constructor being * generated. */ - private Class(ClassLoader loader, Class arrayComponentType, int mods) { + private Class(ClassLoader loader, Class arrayComponentType, int mods, ProtectionDomain pd) { // Initialize final field for classLoader. The initialization value of non-null // prevents future JIT optimizations from assuming this final field is null. classLoader = loader; componentType = arrayComponentType; modifiers = mods; + protectionDomain = pd; } /** @@ -2697,17 +2698,7 @@ public final class Class implements java.io.Serializable, return true; } - /** - * Returns the {@code ProtectionDomain} of this class. - * - * @return the ProtectionDomain of this class - * - * @see java.security.ProtectionDomain - * @since 1.2 - */ - public ProtectionDomain getProtectionDomain() { - return protectionDomain(); - } + private transient final ProtectionDomain protectionDomain; /** Holder for the protection domain returned when the internal domain is null */ private static class Holder { @@ -2719,21 +2710,22 @@ public final class Class implements java.io.Serializable, } } - // package-private - ProtectionDomain protectionDomain() { - ProtectionDomain pd = getProtectionDomain0(); - if (pd == null) { + /** + * Returns the {@code ProtectionDomain} of this class. + * + * @return the ProtectionDomain of this class + * + * @see java.security.ProtectionDomain + * @since 1.2 + */ + public ProtectionDomain getProtectionDomain() { + if (protectionDomain == null) { return Holder.allPermDomain; } else { - return pd; + return protectionDomain; } } - /** - * Returns the ProtectionDomain of this class. - */ - private native ProtectionDomain getProtectionDomain0(); - /* * Returns the Class object for the named primitive type. Type parameter T * avoids redundant casts for trusted code. diff --git a/src/java.base/share/classes/java/lang/System.java b/src/java.base/share/classes/java/lang/System.java index d159bc8262a..67a03efa10b 100644 --- a/src/java.base/share/classes/java/lang/System.java +++ b/src/java.base/share/classes/java/lang/System.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1994, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1994, 2025, 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 @@ -2148,7 +2148,7 @@ public final class System { } public ProtectionDomain protectionDomain(Class c) { - return c.protectionDomain(); + return c.getProtectionDomain(); } public MethodHandle stringConcatHelper(String name, MethodType methodType) { diff --git a/src/java.base/share/classes/jdk/internal/reflect/Reflection.java b/src/java.base/share/classes/jdk/internal/reflect/Reflection.java index 95156dd19c2..e1fe26684c8 100644 --- a/src/java.base/share/classes/jdk/internal/reflect/Reflection.java +++ b/src/java.base/share/classes/jdk/internal/reflect/Reflection.java @@ -56,7 +56,7 @@ public class Reflection { fieldFilterMap = Map.of( Reflection.class, ALL_MEMBERS, AccessibleObject.class, ALL_MEMBERS, - Class.class, Set.of("classLoader", "classData", "modifiers"), + Class.class, Set.of("classLoader", "classData", "modifiers", "protectionDomain"), ClassLoader.class, ALL_MEMBERS, Constructor.class, ALL_MEMBERS, Field.class, ALL_MEMBERS, diff --git a/src/java.base/share/native/libjava/Class.c b/src/java.base/share/native/libjava/Class.c index 6e0ac7d2302..47eedaa59d9 100644 --- a/src/java.base/share/native/libjava/Class.c +++ b/src/java.base/share/native/libjava/Class.c @@ -49,7 +49,6 @@ extern jboolean VerifyFixClassname(char *utf_name); #define FLD "Ljava/lang/reflect/Field;" #define MHD "Ljava/lang/reflect/Method;" #define CTR "Ljava/lang/reflect/Constructor;" -#define PD "Ljava/security/ProtectionDomain;" #define BA "[B" #define RC "Ljava/lang/reflect/RecordComponent;" @@ -64,7 +63,6 @@ static JNINativeMethod methods[] = { {"getDeclaredFields0","(Z)[" FLD, (void *)&JVM_GetClassDeclaredFields}, {"getDeclaredMethods0","(Z)[" MHD, (void *)&JVM_GetClassDeclaredMethods}, {"getDeclaredConstructors0","(Z)[" CTR, (void *)&JVM_GetClassDeclaredConstructors}, - {"getProtectionDomain0", "()" PD, (void *)&JVM_GetProtectionDomain}, {"getDeclaredClasses0", "()[" CLS, (void *)&JVM_GetDeclaredClasses}, {"getDeclaringClass0", "()" CLS, (void *)&JVM_GetDeclaringClass}, {"getSimpleBinaryName0", "()" STR, (void *)&JVM_GetSimpleBinaryName}, diff --git a/test/hotspot/gtest/oops/test_instanceKlass.cpp b/test/hotspot/gtest/oops/test_instanceKlass.cpp index 7aa44236f50..14fbd7ed53c 100644 --- a/test/hotspot/gtest/oops/test_instanceKlass.cpp +++ b/test/hotspot/gtest/oops/test_instanceKlass.cpp @@ -57,7 +57,7 @@ TEST_VM(InstanceKlass, class_loader_printer) { // See if mirror injected fields are printed. oop mirror = vmClasses::ClassLoader_klass()->java_mirror(); mirror->print_on(&st); - ASSERT_THAT(st.base(), HasSubstr("injected 'protection_domain'")) << "Must contain injected fields"; + ASSERT_THAT(st.base(), HasSubstr("injected 'array_klass'")) << "Must contain injected fields"; // We should test other printing functions too. #ifndef PRODUCT st.reset(); diff --git a/test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/TestResolvedJavaType.java b/test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/TestResolvedJavaType.java index 836378ae1f8..6450f9e1e29 100644 --- a/test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/TestResolvedJavaType.java +++ b/test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/TestResolvedJavaType.java @@ -929,7 +929,10 @@ public class TestResolvedJavaType extends TypeUniverse { return true; } if (f.getDeclaringClass().equals(metaAccess.lookupJavaType(Class.class))) { - return f.getName().equals("classLoader") || f.getName().equals("classData") || f.getName().equals("modifiers"); + return f.getName().equals("classLoader") || + f.getName().equals("classData") || + f.getName().equals("modifiers") || + f.getName().equals("protectionDomain"); } if (f.getDeclaringClass().equals(metaAccess.lookupJavaType(Lookup.class))) { return f.getName().equals("allowedModes") || f.getName().equals("lookupClass"); diff --git a/test/jdk/java/lang/reflect/AccessibleObject/ModuleSetAccessibleTest.java b/test/jdk/java/lang/reflect/AccessibleObject/ModuleSetAccessibleTest.java index f101c87229a..08db3265c94 100644 --- a/test/jdk/java/lang/reflect/AccessibleObject/ModuleSetAccessibleTest.java +++ b/test/jdk/java/lang/reflect/AccessibleObject/ModuleSetAccessibleTest.java @@ -36,6 +36,7 @@ import java.lang.reflect.Field; import java.lang.reflect.InaccessibleObjectException; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.security.ProtectionDomain; import jdk.internal.misc.Unsafe; @@ -147,7 +148,7 @@ public class ModuleSetAccessibleTest { // non-public constructor Constructor ctor - = Class.class.getDeclaredConstructor(ClassLoader.class, Class.class, int.class); + = Class.class.getDeclaredConstructor(ClassLoader.class, Class.class, int.class, ProtectionDomain.class); AccessibleObject[] ctors = { ctor }; try { diff --git a/test/jdk/java/lang/reflect/AccessibleObject/TrySetAccessibleTest.java b/test/jdk/java/lang/reflect/AccessibleObject/TrySetAccessibleTest.java index 5fb7bb97bfc..d08798c8757 100644 --- a/test/jdk/java/lang/reflect/AccessibleObject/TrySetAccessibleTest.java +++ b/test/jdk/java/lang/reflect/AccessibleObject/TrySetAccessibleTest.java @@ -41,6 +41,7 @@ import java.lang.reflect.Method; import jdk.internal.misc.Unsafe; import jdk.internal.module.ModulePath; import jdk.internal.perf.Perf; +import java.security.ProtectionDomain; import org.testng.annotations.Test; import static org.testng.Assert.*; @@ -193,7 +194,7 @@ public class TrySetAccessibleTest { // non-public constructor Constructor ctor - = Class.class.getDeclaredConstructor(ClassLoader.class, Class.class, int.class); + = Class.class.getDeclaredConstructor(ClassLoader.class, Class.class, int.class, ProtectionDomain.class); AccessibleObject[] ctors = { ctor }; assertFalse(ctor.trySetAccessible()); diff --git a/test/jdk/jdk/internal/reflect/Reflection/Filtering.java b/test/jdk/jdk/internal/reflect/Reflection/Filtering.java index d714ff9d334..c399b8ff42d 100644 --- a/test/jdk/jdk/internal/reflect/Reflection/Filtering.java +++ b/test/jdk/jdk/internal/reflect/Reflection/Filtering.java @@ -23,7 +23,7 @@ /** * @test - * @bug 8210496 8346567 + * @bug 8210496 8349145 8346567 * @modules java.base/jdk.internal.reflect * @run testng Filtering * @summary Test that security sensitive fields that filtered by core reflection @@ -56,6 +56,7 @@ public class Filtering { { Class.class, "classLoader" }, { Class.class, "classData" }, { Class.class, "modifiers" }, + { Class.class, "protectionDomain" }, { ClassLoader.class, "parent" }, { Field.class, "clazz" }, { Field.class, "modifiers" },