Skip to content

Commit bee273d

Browse files
liachliach
committed
8297271: AccessFlag.maskToAccessFlags should be specific to class file version
Reviewed-by: rriggs
1 parent 34807df commit bee273d

File tree

10 files changed

+136
-60
lines changed

10 files changed

+136
-60
lines changed

‎src/java.base/share/classes/java/lang/Class.java

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1384,10 +1384,8 @@ public Set<AccessFlag> accessFlags() {
13841384
isAnonymousClass() || isArray()) ?
13851385
AccessFlag.Location.INNER_CLASS :
13861386
AccessFlag.Location.CLASS;
1387-
return AccessFlag.maskToAccessFlags((location == AccessFlag.Location.CLASS) ?
1388-
getClassAccessFlagsRaw() :
1389-
getModifiers(),
1390-
location);
1387+
return getReflectionFactory().parseAccessFlags((location == AccessFlag.Location.CLASS) ?
1388+
getClassAccessFlagsRaw() : getModifiers(), location, this);
13911389
}
13921390

13931391
/**
@@ -4125,7 +4123,7 @@ public boolean isSealed() {
41254123
* type is returned. If the class is a primitive type then the latest class
41264124
* file major version is returned and zero is returned for the minor version.
41274125
*/
4128-
private int getClassFileVersion() {
4126+
int getClassFileVersion() {
41294127
Class<?> c = isArray() ? elementType() : this;
41304128
return c.getClassFileVersion0();
41314129
}

‎src/java.base/share/classes/java/lang/System.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2017,6 +2017,9 @@ public byte[] getRawExecutableTypeAnnotations(Executable executable) {
20172017
E[] getEnumConstantsShared(Class<E> klass) {
20182018
return klass.getEnumConstantsShared();
20192019
}
2020+
public int classFileVersion(Class<?> clazz) {
2021+
return clazz.getClassFileVersion();
2022+
}
20202023
public void blockedOn(Interruptible b) {
20212024
Thread.currentThread().blockedOn(b);
20222025
}

‎src/java.base/share/classes/java/lang/reflect/AccessFlag.java

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -372,18 +372,17 @@ public Set<Location> locations(ClassFileFormatVersion cffv) {
372372

373373
/**
374374
* {@return an unmodifiable set of access flags for the given mask value
375-
* appropriate for the location in question}
375+
* appropriate for the location in the current class file format version}
376376
*
377377
* @param mask bit mask of access flags
378378
* @param location context to interpret mask value
379379
* @throws IllegalArgumentException if the mask contains bit
380-
* positions not support for the location in question
380+
* positions not defined for the location in the current class file format
381+
* @throws NullPointerException if {@code location} is {@code null}
381382
*/
382383
public static Set<AccessFlag> maskToAccessFlags(int mask, Location location) {
383-
var definition = findDefinition(location);
384-
int flagsMask = location.flagsMask();
385-
int parsingMask = location == Location.METHOD ? flagsMask | ACC_STRICT : flagsMask; // flagMask lacks strictfp
386-
int unmatchedMask = mask & (~parsingMask);
384+
var definition = findDefinition(location); // null checks location
385+
int unmatchedMask = mask & (~location.flagsMask());
387386
if (unmatchedMask != 0) {
388387
throw new IllegalArgumentException("Unmatched bit position 0x" +
389388
Integer.toHexString(unmatchedMask) +
@@ -392,6 +391,30 @@ public static Set<AccessFlag> maskToAccessFlags(int mask, Location location) {
392391
return new AccessFlagSet(definition, mask);
393392
}
394393

394+
/**
395+
* {@return an unmodifiable set of access flags for the given mask value
396+
* appropriate for the location in the given class file format version}
397+
*
398+
* @param mask bit mask of access flags
399+
* @param location context to interpret mask value
400+
* @param cffv the class file format to interpret mask value
401+
* @throws IllegalArgumentException if the mask contains bit
402+
* positions not defined for the location in the given class file format
403+
* @throws NullPointerException if {@code location} or {@code cffv} is {@code null}
404+
* @since 25
405+
*/
406+
public static Set<AccessFlag> maskToAccessFlags(int mask, Location location, ClassFileFormatVersion cffv) {
407+
var definition = findDefinition(location); // null checks location
408+
int unmatchedMask = mask & (~location.flagsMask(cffv)); // null checks cffv
409+
if (unmatchedMask != 0) {
410+
throw new IllegalArgumentException("Unmatched bit position 0x" +
411+
Integer.toHexString(unmatchedMask) +
412+
" for location " + location +
413+
" for class file format " + cffv);
414+
}
415+
return new AccessFlagSet(definition, mask);
416+
}
417+
395418
/**
396419
* A location within a {@code class} file where flags can be applied.
397420
* <p>

‎src/java.base/share/classes/java/lang/reflect/Executable.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2012, 2024, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2012, 2025, 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
@@ -221,8 +221,9 @@ String sharedToGenericString(int modifierMask, boolean isDefault) {
221221
*/
222222
@Override
223223
public Set<AccessFlag> accessFlags() {
224-
return AccessFlag.maskToAccessFlags(getModifiers(),
225-
AccessFlag.Location.METHOD);
224+
return reflectionFactory.parseAccessFlags(getModifiers(),
225+
AccessFlag.Location.METHOD,
226+
getDeclaringClass());
226227
}
227228

228229
/**

‎src/java.base/share/classes/java/lang/reflect/Field.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 1996, 2024, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 1996, 2025, 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
@@ -218,7 +218,7 @@ public int getModifiers() {
218218
*/
219219
@Override
220220
public Set<AccessFlag> accessFlags() {
221-
return AccessFlag.maskToAccessFlags(getModifiers(), AccessFlag.Location.FIELD);
221+
return reflectionFactory.parseAccessFlags(getModifiers(), AccessFlag.Location.FIELD, getDeclaringClass());
222222
}
223223

224224
/**

‎src/java.base/share/classes/java/lang/reflect/Parameter.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2013, 2022, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2013, 2025, 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
@@ -172,8 +172,8 @@ public int getModifiers() {
172172
* @since 20
173173
*/
174174
public Set<AccessFlag> accessFlags() {
175-
return AccessFlag.maskToAccessFlags(getModifiers(),
176-
AccessFlag.Location.METHOD_PARAMETER);
175+
return AccessibleObject.reflectionFactory.parseAccessFlags(getModifiers(),
176+
AccessFlag.Location.METHOD_PARAMETER, getDeclaringExecutable().getDeclaringClass());
177177
}
178178

179179
/**

‎src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,12 @@ public interface JavaLangAccess {
119119
*/
120120
<E extends Enum<E>> E[] getEnumConstantsShared(Class<E> klass);
121121

122+
/**
123+
* Returns the big-endian packed minor-major version of the class file
124+
* of this class.
125+
*/
126+
int classFileVersion(Class<?> clazz);
127+
122128
/**
123129
* Set current thread's blocker field.
124130
*/

‎src/java.base/share/classes/jdk/internal/reflect/ReflectionFactory.java

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2001, 2024, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2001, 2025, 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
@@ -32,15 +32,10 @@
3232
import java.io.ObjectStreamField;
3333
import java.io.OptionalDataException;
3434
import java.io.Serializable;
35+
import java.lang.classfile.ClassFile;
3536
import java.lang.invoke.MethodHandle;
3637
import java.lang.invoke.MethodHandles;
37-
import java.lang.reflect.Constructor;
38-
import java.lang.reflect.Executable;
39-
import java.lang.reflect.Field;
40-
import java.lang.reflect.InvocationTargetException;
41-
import java.lang.reflect.Method;
42-
import java.lang.reflect.Modifier;
43-
import java.lang.reflect.Proxy;
38+
import java.lang.reflect.*;
4439
import java.util.Set;
4540

4641
import jdk.internal.access.JavaLangReflectAccess;
@@ -518,6 +513,31 @@ public final ObjectStreamField[] serialPersistentFields(Class<?> cl) {
518513
}
519514
}
520515

516+
public final Set<AccessFlag> parseAccessFlags(int mask, AccessFlag.Location location, Class<?> classFile) {
517+
var cffv = classFileFormatVersion(classFile);
518+
return cffv == null ?
519+
AccessFlag.maskToAccessFlags(mask, location) :
520+
AccessFlag.maskToAccessFlags(mask, location, cffv);
521+
}
522+
523+
private final ClassFileFormatVersion classFileFormatVersion(Class<?> cl) {
524+
int raw = SharedSecrets.getJavaLangAccess().classFileVersion(cl);
525+
526+
int major = raw & 0xFFFF;
527+
int minor = raw >>> Character.SIZE;
528+
529+
assert VM.isSupportedClassFileVersion(major, minor) : major + "." + minor;
530+
531+
if (major >= ClassFile.JAVA_12_VERSION) {
532+
if (minor == 0)
533+
return ClassFileFormatVersion.fromMajor(raw);
534+
return null; // preview or old preview, fallback to default handling
535+
} else if (major == ClassFile.JAVA_1_VERSION) {
536+
return minor < 3 ? ClassFileFormatVersion.RELEASE_0 : ClassFileFormatVersion.RELEASE_1;
537+
}
538+
return ClassFileFormatVersion.fromMajor(major);
539+
}
540+
521541
//--------------------------------------------------------------------------
522542
//
523543
// Internals only below this point

‎src/jdk.jdeps/share/classes/com/sun/tools/javap/BasicWriter.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,13 +59,12 @@ protected Set<AccessFlag> flagsReportUnknown(AccessFlags flags, ClassFileFormatV
5959
}
6060

6161
protected Set<AccessFlag> maskToAccessFlagsReportUnknown(int mask, AccessFlag.Location location, ClassFileFormatVersion cffv) {
62-
// TODO pass cffv to maskToAccessFlags
6362
try {
64-
return AccessFlag.maskToAccessFlags(mask, location);
63+
return AccessFlag.maskToAccessFlags(mask, location, cffv);
6564
} catch (IllegalArgumentException ex) {
6665
mask &= location.flagsMask(cffv);
6766
report("Access Flags: " + ex.getMessage());
68-
return AccessFlag.maskToAccessFlags(mask, location);
67+
return AccessFlag.maskToAccessFlags(mask, location, cffv);
6968
}
7069
}
7170

‎test/jdk/java/lang/reflect/AccessFlag/BasicAccessFlagTest.java

Lines changed: 56 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,14 @@
2323

2424
/*
2525
* @test
26-
* @bug 8266670 8293626
26+
* @bug 8266670 8293626 8297271
2727
* @summary Basic tests of AccessFlag
28+
* @run junit BasicAccessFlagTest
2829
*/
2930

31+
import java.lang.classfile.ClassFile;
3032
import java.lang.reflect.AccessFlag;
33+
import java.lang.reflect.ClassFileFormatVersion;
3134
import java.lang.reflect.Field;
3235
import java.lang.reflect.Modifier;
3336
import java.util.EnumSet;
@@ -36,61 +39,57 @@
3639
import java.util.HashSet;
3740
import java.util.Set;
3841

42+
import org.junit.Test;
43+
import org.junit.jupiter.api.Assertions;
44+
45+
import static org.junit.jupiter.api.Assertions.*;
46+
3947
public class BasicAccessFlagTest {
40-
public static void main(String... args) throws Exception {
41-
testSourceModifiers();
42-
testMaskOrdering();
43-
testDisjoint();
44-
testMaskToAccessFlagsPositive();
45-
testLocationsNullHandling();
46-
}
4748

4849
/*
4950
* Verify sourceModifier() == true access flags have a
5051
* corresponding constant in java.lang.reflect.Modifier.
5152
*/
52-
private static void testSourceModifiers() throws Exception {
53+
@Test
54+
public void testSourceModifiers() throws Exception {
5355
Class<?> modifierClass = Modifier.class;
5456

5557
for(AccessFlag accessFlag : AccessFlag.values()) {
5658
if (accessFlag.sourceModifier()) {
5759
// Check for consistency
5860
Field f = modifierClass.getField(accessFlag.name());
59-
if (accessFlag.mask() != f.getInt(null) ) {
60-
throw new RuntimeException("Unexpected mask for " +
61-
accessFlag);
62-
}
61+
assertEquals(f.getInt(null), accessFlag.mask(), accessFlag + " mask");
6362
}
6463
}
6564
}
6665

6766
// The mask values of the enum constants must be non-decreasing;
6867
// in other words stay the same (for colliding mask values) or go
6968
// up.
70-
private static void testMaskOrdering() {
69+
@Test
70+
public void testMaskOrdering() {
7171
AccessFlag[] values = AccessFlag.values();
7272
for (int i = 1; i < values.length; i++) {
7373
AccessFlag left = values[i-1];
7474
AccessFlag right = values[i];
75-
if (left.mask() > right.mask()) {
76-
throw new RuntimeException(left
77-
+ "has a greater mask than "
78-
+ right);
79-
}
75+
assertTrue(left.mask() <= right.mask(), () -> left
76+
+ "has a greater mask than "
77+
+ right);
8078
}
8179
}
8280

8381
// Test that if access flags have a matching mask, their locations
8482
// are disjoint.
85-
private static void testDisjoint() {
83+
@Test
84+
public void testDisjoint() {
8685
// First build the mask -> access flags map...
8786
Map<Integer, Set<AccessFlag>> maskToFlags = new LinkedHashMap<>();
8887

8988
for (var accessFlag : AccessFlag.values()) {
9089
Integer mask = accessFlag.mask();
9190
Set<AccessFlag> flags = maskToFlags.get(mask);
9291

93-
if (flags == null ) {
92+
if (flags == null) {
9493
flags = new HashSet<>();
9594
flags.add(accessFlag);
9695
maskToFlags.put(mask, flags);
@@ -135,7 +134,8 @@ private static void reportError(AccessFlag.Location location,
135134

136135
// For each access flag, make sure it is recognized on every kind
137136
// of location it can apply to
138-
private static void testMaskToAccessFlagsPositive() {
137+
@Test
138+
public void testMaskToAccessFlagsPositive() {
139139
for (var accessFlag : AccessFlag.values()) {
140140
Set<AccessFlag> expectedSet = EnumSet.of(accessFlag);
141141
for (var location : accessFlag.locations()) {
@@ -146,17 +146,43 @@ private static void testMaskToAccessFlagsPositive() {
146146
accessFlag + ", " + location);
147147
}
148148
}
149+
for (var cffv : ClassFileFormatVersion.values()) {
150+
for (var location : accessFlag.locations(cffv)) {
151+
Set<AccessFlag> computedSet =
152+
AccessFlag.maskToAccessFlags(accessFlag.mask(), location, cffv);
153+
if (!expectedSet.equals(computedSet)) {
154+
throw new RuntimeException("Bad set computation on " +
155+
accessFlag + ", " + location);
156+
}
157+
}
158+
}
149159
}
160+
assertEquals(Set.of(AccessFlag.STRICT), AccessFlag.maskToAccessFlags(Modifier.STRICT, AccessFlag.Location.METHOD, ClassFileFormatVersion.RELEASE_8));
150161
}
151162

152-
private static void testLocationsNullHandling() {
153-
for (var flag : AccessFlag.values() ) {
154-
try {
155-
flag.locations(null);
156-
throw new RuntimeException("Did not get NPE on " + flag + ".location(null)");
157-
} catch (NullPointerException npe ) {
158-
; // Expected
159-
}
163+
@Test
164+
public void testMaskToAccessFlagsNegative() {
165+
assertThrows(IllegalArgumentException.class, () -> AccessFlag.maskToAccessFlags(Modifier.STRICT, AccessFlag.Location.METHOD));
166+
assertThrows(IllegalArgumentException.class, () -> AccessFlag.maskToAccessFlags(Modifier.STRICT, AccessFlag.Location.METHOD, ClassFileFormatVersion.RELEASE_17));
167+
assertThrows(IllegalArgumentException.class, () -> AccessFlag.maskToAccessFlags(Modifier.STRICT, AccessFlag.Location.METHOD, ClassFileFormatVersion.RELEASE_1));
168+
assertThrows(IllegalArgumentException.class, () -> AccessFlag.maskToAccessFlags(Modifier.PRIVATE, AccessFlag.Location.CLASS));
169+
assertThrows(IllegalArgumentException.class, () -> AccessFlag.maskToAccessFlags(ClassFile.ACC_MODULE, AccessFlag.Location.CLASS, ClassFileFormatVersion.RELEASE_8));
170+
assertThrows(IllegalArgumentException.class, () -> AccessFlag.maskToAccessFlags(ClassFile.ACC_ANNOTATION, AccessFlag.Location.CLASS, ClassFileFormatVersion.RELEASE_4));
171+
assertThrows(IllegalArgumentException.class, () -> AccessFlag.maskToAccessFlags(ClassFile.ACC_ENUM, AccessFlag.Location.FIELD, ClassFileFormatVersion.RELEASE_4));
172+
assertThrows(IllegalArgumentException.class, () -> AccessFlag.maskToAccessFlags(ClassFile.ACC_SYNTHETIC, AccessFlag.Location.INNER_CLASS, ClassFileFormatVersion.RELEASE_4));
173+
assertThrows(IllegalArgumentException.class, () -> AccessFlag.maskToAccessFlags(ClassFile.ACC_PUBLIC, AccessFlag.Location.INNER_CLASS, ClassFileFormatVersion.RELEASE_0));
174+
assertThrows(IllegalArgumentException.class, () -> AccessFlag.maskToAccessFlags(ClassFile.ACC_MANDATED, AccessFlag.Location.METHOD_PARAMETER, ClassFileFormatVersion.RELEASE_7));
175+
assertThrows(IllegalArgumentException.class, () -> AccessFlag.maskToAccessFlags(ClassFile.ACC_MANDATED, AccessFlag.Location.MODULE, ClassFileFormatVersion.RELEASE_7));
176+
}
177+
178+
@Test
179+
public void testLocationsNullHandling() {
180+
for (var flag : AccessFlag.values()) {
181+
assertThrows(NullPointerException.class, () -> flag.locations(null));
182+
}
183+
184+
for (var location : AccessFlag.Location.values()) {
185+
assertThrows(NullPointerException.class, () -> location.flags(null));
160186
}
161187

162188
for (var location : AccessFlag.Location.values()) {

0 commit comments

Comments
 (0)