Skip to content

Commit 44a723e

Browse files
rakachanrakachan
committed
[GR-65064] Improve handling of the magic accessor across Espresso.
1 parent de0c31c commit 44a723e

File tree

7 files changed

+73
-51
lines changed

7 files changed

+73
-51
lines changed

‎espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/constantpool/RuntimeConstantPool.java‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,7 @@ public ResolvedClassConstant resolveClassConstant(int classIndex, ObjectKlass ac
361361
Symbol<Type> type = context.getTypes().fromClassNameEntry(klassName);
362362
Klass klass = context.getMeta().resolveSymbolOrFail(type, accessingKlass.getDefiningClassLoader(), accessingKlass.protectionDomain());
363363
Klass checkedKlass = klass.getElementalType();
364-
if (!Klass.checkAccess(checkedKlass, accessingKlass, false)) {
364+
if (!Klass.checkAccess(checkedKlass, accessingKlass)) {
365365
Meta meta = context.getMeta();
366366
context.getLogger().log(Level.FINE,
367367
"Access check of: " + checkedKlass.getType() + " from " + accessingKlass.getType() + " throws IllegalAccessError");
@@ -372,13 +372,13 @@ public ResolvedClassConstant resolveClassConstant(int classIndex, ObjectKlass ac
372372
if (accessingKlass.module() == checkedKlass.module()) {
373373
errorMessage.append(checkedKlass.getExternalName());
374374
errorMessage.append(" and ");
375-
ClassRegistry.classInModuleOfLoader(accessingKlass, true, errorMessage, meta);
375+
ClassRegistry.classInModuleOfLoader(context.getClassLoadingEnv(), accessingKlass, true, errorMessage, meta);
376376
} else {
377377
// checkedKlass is not an array type (getElementalType) nor a
378378
// primitive type (it would have passed the access checks)
379-
ClassRegistry.classInModuleOfLoader((ObjectKlass) checkedKlass, false, errorMessage, meta);
379+
ClassRegistry.classInModuleOfLoader(context.getClassLoadingEnv(), (ObjectKlass) checkedKlass, false, errorMessage, meta);
380380
errorMessage.append("; ");
381-
ClassRegistry.classInModuleOfLoader(accessingKlass, false, errorMessage, meta);
381+
ClassRegistry.classInModuleOfLoader(context.getClassLoadingEnv(), accessingKlass, false, errorMessage, meta);
382382
}
383383
errorMessage.append(")");
384384
}
@@ -479,8 +479,7 @@ public static boolean memberCheckAccess(Klass accessingKlass, Klass resolvedKlas
479479
return true;
480480
}
481481
// MagicAccessorImpl marks internal reflection classes that have access to everything.
482-
if (accessingKlass.getJavaVersion().java23OrEarlier() &&
483-
accessingKlass.getMeta().sun_reflect_MagicAccessorImpl.isAssignableFrom(accessingKlass)) {
482+
if (accessingKlass.isMagicAccessor()) {
484483
return true;
485484
}
486485

‎espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/descriptors/EspressoSymbols.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,7 @@ public static void ensureInitialized() {
279279
// MagicAccessorImpl is not public.
280280
public static final Symbol<Type> sun_reflect_MagicAccessorImpl = SYMBOLS.putType("Lsun/reflect/MagicAccessorImpl;");
281281
public static final Symbol<Type> jdk_internal_reflect_MagicAccessorImpl = SYMBOLS.putType("Ljdk/internal/reflect/MagicAccessorImpl;");
282+
public static final Symbol<Type> jdk_internal_reflect_SerializationConstructorAccessorImpl = SYMBOLS.putType("Ljdk/internal/reflect/SerializationConstructorAccessorImpl;");
282283
// DelegatingClassLoader is not public.
283284
public static final Symbol<Type> sun_reflect_DelegatingClassLoader = SYMBOLS.putType("Lsun/reflect/DelegatingClassLoader;");
284285
public static final Symbol<Type> jdk_internal_reflect_DelegatingClassLoader = SYMBOLS.putType("Ljdk/internal/reflect/DelegatingClassLoader;");
@@ -574,6 +575,8 @@ public static class Names {
574575

575576
public static final Symbol<Name> main = SYMBOLS.putName("main");
576577
// Reflection
578+
public static final Symbol<Name> jdk_internal_reflect = SYMBOLS.putName("jdk/internal/reflect");
579+
public static final Symbol<Name> sun_reflect = SYMBOLS.putName("sun/reflect");
577580
public static final Symbol<Name> clazz = SYMBOLS.putName("clazz");
578581
public static final Symbol<Name> getParameterTypes = SYMBOLS.putName("getParameterTypes");
579582
public static final Symbol<Name> override = SYMBOLS.putName("override");

‎espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/impl/ClassLoadingEnv.java‎

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import com.oracle.truffle.espresso.classfile.descriptors.Type;
4040
import com.oracle.truffle.espresso.classfile.descriptors.TypeSymbols;
4141
import com.oracle.truffle.espresso.classfile.perf.TimerCollection;
42+
import com.oracle.truffle.espresso.descriptors.EspressoSymbols;
4243
import com.oracle.truffle.espresso.meta.EspressoError;
4344
import com.oracle.truffle.espresso.meta.Meta;
4445
import com.oracle.truffle.espresso.runtime.staticobject.StaticObject;
@@ -94,16 +95,38 @@ public boolean shouldCacheClass(ClassRegistry.ClassDefinitionInfo info, StaticOb
9495
(loaderIsBootOrPlatform(loader) || loaderIsAppLoader(loader));
9596
}
9697

98+
public boolean isReflectPackage(Symbol<Name> pkg) {
99+
/*
100+
* Note: This class is created too early in the init process to make this variable a final
101+
* field.
102+
*/
103+
Symbol<Name> reflectPackage = getLanguage().getJavaVersion().java8OrEarlier()
104+
? EspressoSymbols.Names.sun_reflect
105+
: EspressoSymbols.Names.jdk_internal_reflect;
106+
return pkg == reflectPackage;
107+
}
108+
109+
@SuppressWarnings("static-method")
110+
public boolean loaderIsBoot(StaticObject loader) {
111+
return StaticObject.isNull(loader);
112+
}
113+
97114
public boolean loaderIsBootOrPlatform(StaticObject loader) {
98-
return StaticObject.isNull(loader) ||
115+
return loaderIsBoot(loader) ||
99116
(language.getJavaVersion().java9OrLater() && meta.jdk_internal_loader_ClassLoaders$PlatformClassLoader.isAssignableFrom(loader.getKlass()));
100117
}
101118

102119
public boolean loaderIsAppLoader(StaticObject loader) {
103-
return !StaticObject.isNull(loader) &&
120+
return !loaderIsBoot(loader) &&
104121
(meta.jdk_internal_loader_ClassLoaders$AppClassLoader.isAssignableFrom(loader.getKlass()));
105122
}
106123

124+
public boolean loaderIsReflection(StaticObject loader) {
125+
return meta.sun_reflect_DelegatingClassLoader != null &&
126+
!loaderIsBoot(loader) &&
127+
meta.sun_reflect_DelegatingClassLoader.isAssignableFrom(loader.getKlass());
128+
}
129+
107130
public long getNewKlassId() {
108131
long id = klassIdProvider.getAndIncrement();
109132
if (id < 0) {

‎espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/impl/ClassRegistry.java‎

Lines changed: 35 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -576,9 +576,24 @@ private ObjectKlass createKlass(EspressoContext context, ParserKlass parserKlass
576576
}
577577

578578
if (superKlass != null) {
579-
if (!Klass.checkAccess(superKlass, klass, true)) {
579+
/*
580+
* These checks ensure that only classes defined in the boot or reflection class loader
581+
* can declare a superclass in the 'jdk.internal.reflect' package.
582+
*
583+
* In turn, this ensures that only these classes may be magic accessors.
584+
*/
585+
if (!env.loaderIsBoot(getClassLoader()) &&
586+
env.isReflectPackage(superKlass.getRuntimePackage()) &&
587+
!env.loaderIsReflection(getClassLoader())) {
588+
throw EspressoClassLoadingException.illegalAccessError(
589+
String.format("class %s loaded by %s cannot access reflection superclass %s",
590+
klass.getExternalName(),
591+
loaderDesc(env, context.getMeta(), getClassLoader()),
592+
superKlass.getExternalName()));
593+
}
594+
if (!Klass.checkAccess(superKlass, klass)) {
580595
StringBuilder sb = new StringBuilder().append("class ").append(klass.getExternalName()).append(" cannot access its superclass ").append(superKlass.getExternalName());
581-
superTypeAccessMessage(klass, superKlass, sb, context);
596+
superTypeAccessMessage(env, klass, superKlass, sb, context);
582597
throw EspressoClassLoadingException.illegalAccessError(sb.toString());
583598
}
584599
if (!superKlass.permittedSubclassCheck(klass)) {
@@ -588,9 +603,9 @@ private ObjectKlass createKlass(EspressoContext context, ParserKlass parserKlass
588603

589604
for (ObjectKlass interf : superInterfaces) {
590605
if (interf != null) {
591-
if (!Klass.checkAccess(interf, klass, true)) {
606+
if (!Klass.checkAccess(interf, klass)) {
592607
StringBuilder sb = new StringBuilder().append("class ").append(klass.getExternalName()).append(" cannot access its superinterface ").append(interf.getExternalName());
593-
superTypeAccessMessage(klass, interf, sb, context);
608+
superTypeAccessMessage(env, klass, interf, sb, context);
594609
throw EspressoClassLoadingException.illegalAccessError(sb.toString());
595610
}
596611
if (!interf.permittedSubclassCheck(klass)) {
@@ -602,24 +617,24 @@ private ObjectKlass createKlass(EspressoContext context, ParserKlass parserKlass
602617
return klass;
603618
}
604619

605-
private static void superTypeAccessMessage(ObjectKlass sub, ObjectKlass sup, StringBuilder sb, EspressoContext context) {
620+
private static void superTypeAccessMessage(ClassLoadingEnv env, ObjectKlass sub, ObjectKlass sup, StringBuilder sb, EspressoContext context) {
606621
if (context.getJavaVersion().modulesEnabled()) {
607622
sb.append(" (");
608623
Meta meta = context.getMeta();
609624
if (sup.module() == sub.module()) {
610625
sb.append(sub.getExternalName());
611626
sb.append(" and ");
612-
classInModuleOfLoader(sup, true, sb, meta);
627+
classInModuleOfLoader(env, sup, true, sb, meta);
613628
} else {
614-
classInModuleOfLoader(sub, false, sb, meta);
629+
classInModuleOfLoader(env, sub, false, sb, meta);
615630
sb.append("; ");
616-
classInModuleOfLoader(sup, false, sb, meta);
631+
classInModuleOfLoader(env, sup, false, sb, meta);
617632
}
618633
sb.append(")");
619634
}
620635
}
621636

622-
public static void classInModuleOfLoader(ObjectKlass klass, boolean plural, StringBuilder sb, Meta meta) {
637+
public static void classInModuleOfLoader(ClassLoadingEnv env, ObjectKlass klass, boolean plural, StringBuilder sb, Meta meta) {
623638
assert meta.getJavaVersion().modulesEnabled() && meta.java_lang_ClassLoader_nameAndId != null;
624639
sb.append(klass.getExternalName());
625640
if (plural) {
@@ -635,16 +650,18 @@ public static void classInModuleOfLoader(ObjectKlass klass, boolean plural, Stri
635650
sb.append("unnamed module");
636651
}
637652
sb.append(" of loader ");
638-
StaticObject loader = klass.getDefiningClassLoader();
639-
if (StaticObject.isNull(loader)) {
640-
sb.append("bootstrap");
653+
sb.append(loaderDesc(env, meta, klass.getDefiningClassLoader()));
654+
}
655+
656+
private static String loaderDesc(ClassLoadingEnv env, Meta meta, StaticObject loader) {
657+
if (env.loaderIsBoot(loader)) {
658+
return "bootstrap";
659+
}
660+
StaticObject nameAndId = meta.java_lang_ClassLoader_nameAndId.getObject(loader);
661+
if (StaticObject.isNull(nameAndId)) {
662+
return loader.getKlass().getExternalName();
641663
} else {
642-
StaticObject nameAndId = meta.java_lang_ClassLoader_nameAndId.getObject(loader);
643-
if (StaticObject.isNull(nameAndId)) {
644-
sb.append(loader.getKlass().getExternalName());
645-
} else {
646-
sb.append(meta.toHostString(nameAndId));
647-
}
664+
return meta.toHostString(nameAndId);
648665
}
649666
}
650667

‎espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/impl/Klass.java‎

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -769,7 +769,7 @@ protected static boolean hasFinalInstanceField(Class<?> clazz) {
769769
* <li>C is not public, and C and D are members of the same run-time package.
770770
* </ul>
771771
*/
772-
public static boolean checkAccess(Klass klass, ObjectKlass accessingKlass, boolean ignoreMagicAccessor) {
772+
public static boolean checkAccess(Klass klass, ObjectKlass accessingKlass) {
773773
if (accessingKlass == null) {
774774
return true;
775775
}
@@ -795,26 +795,7 @@ public static boolean checkAccess(Klass klass, ObjectKlass accessingKlass, boole
795795
}
796796
}
797797

798-
if (context.getJavaVersion().java21OrEarlier()) {
799-
if (ignoreMagicAccessor) {
800-
/*
801-
* Prevents any class inheriting from MagicAccessorImpl to have access to
802-
* MagicAccessorImpl just because it implements MagicAccessorImpl.
803-
*
804-
* Only generated accessors in the {sun|jdk.internal}.reflect package, defined by
805-
* {sun|jdk.internal}.reflect.DelegatingClassLoader(s) have access to
806-
* MagicAccessorImpl.
807-
*/
808-
ObjectKlass magicAccessorImpl = context.getMeta().sun_reflect_MagicAccessorImpl;
809-
return !StaticObject.isNull(accessingKlass.getDefiningClassLoader()) &&
810-
context.getMeta().sun_reflect_DelegatingClassLoader.equals(accessingKlass.getDefiningClassLoader().getKlass()) &&
811-
magicAccessorImpl.getRuntimePackage().equals(accessingKlass.getRuntimePackage()) &&
812-
magicAccessorImpl.isAssignableFrom(accessingKlass);
813-
}
814-
815-
return (context.getMeta().sun_reflect_MagicAccessorImpl.isAssignableFrom(accessingKlass));
816-
}
817-
return false;
798+
return accessingKlass.isMagicAccessor();
818799
}
819800

820801
public static boolean doModuleAccessChecks(Klass klass, ObjectKlass accessingKlass, EspressoContext context) {
@@ -1917,7 +1898,7 @@ public final Symbol<Name> getSymbolicRuntimePackage() {
19171898

19181899
@Override
19191900
public final boolean isMagicAccessor() {
1920-
if (getJavaVersion().java21OrEarlier()) {
1901+
if (getMeta().sun_reflect_MagicAccessorImpl != null) {
19211902
return getMeta().sun_reflect_MagicAccessorImpl.isAssignableFrom(this);
19221903
}
19231904
return false;

‎espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/impl/ObjectKlass.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -698,8 +698,7 @@ private void verifyImpl() {
698698
for (ObjectKlass interf : getSuperInterfaces()) {
699699
interf.verify();
700700
}
701-
if (getJavaVersion().java21OrEarlier() &&
702-
meta.sun_reflect_MagicAccessorImpl.isAssignableFrom(this)) {
701+
if (isMagicAccessor()) {
703702
/*
704703
* Hotspot comment:
705704
*

‎espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/meta/Meta.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2981,7 +2981,7 @@ public Klass resolveSymbolOrFail(Symbol<Type> type, @JavaType(ClassLoader.class)
29812981
public Klass resolveSymbolAndAccessCheck(Symbol<Type> type, ObjectKlass accessingKlass) {
29822982
assert accessingKlass != null;
29832983
Klass klass = resolveSymbolOrFail(type, accessingKlass.getDefiningClassLoader(), accessingKlass.protectionDomain());
2984-
if (!Klass.checkAccess(klass.getElementalType(), accessingKlass, false)) {
2984+
if (!Klass.checkAccess(klass.getElementalType(), accessingKlass)) {
29852985
throw throwException(java_lang_IllegalAccessError);
29862986
}
29872987
return klass;

0 commit comments

Comments
 (0)