Skip to content

Reject synchronization on primitive wrappers #5338

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
liannacasper merged 1 commit into from
Jul 5, 2026
Merged

Reject synchronization on primitive wrappers #5338

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 72 additions & 17 deletions maven/codenameone-maven-plugin/src/main/java/com/codename1/maven/BytecodeComplianceMojo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ public class BytecodeComplianceMojo extends AbstractCN1Mojo {

private static final Map<String, String> SUGGESTED_REPLACEMENTS;
private static final Set<String> SIMD_OWNER_NAMES;
private static final Set<String> PRIMITIVE_WRAPPER_INTERNAL_NAMES;

static {
Map<String, String> m = new HashMap<String, String>();
Expand All @@ -76,6 +77,16 @@ public class BytecodeComplianceMojo extends AbstractCN1Mojo {
simdOwners.add("com/codename1/impl/ios/IOSSimd");
simdOwners.add("com/codename1/impl/javase/JavaSESimd");
SIMD_OWNER_NAMES = Collections.unmodifiableSet(simdOwners);
Set<String> primitiveWrappers = new HashSet<String>();
primitiveWrappers.add("java/lang/Boolean");
primitiveWrappers.add("java/lang/Byte");
primitiveWrappers.add("java/lang/Character");
primitiveWrappers.add("java/lang/Double");
primitiveWrappers.add("java/lang/Float");
primitiveWrappers.add("java/lang/Integer");
primitiveWrappers.add("java/lang/Long");
primitiveWrappers.add("java/lang/Short");
PRIMITIVE_WRAPPER_INTERNAL_NAMES = Collections.unmodifiableSet(primitiveWrappers);
}

private static final int MAX_CLASS_MAJOR_VERSION = Opcodes.V17;
Expand Down Expand Up @@ -503,7 +514,7 @@ private List<Violation> scanProjectClasses(File outputDir, final Map<String, Cla
try {
ClassReader reader = new ClassReader(inputStream);
reader.accept(new ComplianceScanner(classFile, outputDir, allowedIndex, projectAndDependencyIndex, violations), ClassReader.SKIP_FRAMES);
addSimdAllocaViolations(classFile, outputDir, reader, violations);
addSemanticStackViolations(classFile, outputDir, reader, violations);
} finally {
inputStream.close();
}
Expand All @@ -514,7 +525,7 @@ private List<Violation> scanProjectClasses(File outputDir, final Map<String, Cla
return violations;
}

private void addSimdAllocaViolations(File classFile, File outputDir, ClassReader reader, List<Violation> violations) throws IOException, MojoExecutionException {
private void addSemanticStackViolations(File classFile, File outputDir, ClassReader reader, List<Violation> violations) throws IOException, MojoExecutionException {
ClassNode classNode = new ClassNode();
reader.accept(classNode, ClassReader.EXPAND_FRAMES);
for (MethodNode method : classNode.methods) {
Expand All @@ -526,7 +537,7 @@ private void addSimdAllocaViolations(File classFile, File outputDir, ClassReader
Analyzer<BasicValue> analyzer = new Analyzer<BasicValue>(new SimdAllocaInterpreter());
frames = analyzer.analyze(classNode.name, method);
} catch (AnalyzerException ex) {
throw new MojoExecutionException("Failed to analyze SIMD alloca usage for " + classFile + " in " + classNode.name + "#" + method.name + method.desc, ex);
throw new MojoExecutionException("Failed to analyze bytecode semantics for " + classFile + " in " + classNode.name + "#" + method.name + method.desc, ex);
}
for (AbstractInsnNode instruction = method.instructions.getFirst(); instruction != null; instruction = instruction.getNext()) {
int index = method.instructions.indexOf(instruction);
Expand All @@ -535,27 +546,44 @@ private void addSimdAllocaViolations(File classFile, File outputDir, ClassReader
continue;
}
int opcode = instruction.getOpcode();
if (opcode == Opcodes.MONITORENTER) {
BasicValue lockValue = frame.getStack(frame.getStackSize() - 1);
if (isPrimitiveWrapperValue(lockValue)) {
addSemanticViolation(violations, classFile, outputDir, classNode.name, method,
"Synchronization on primitive wrapper " + lockValue.getType().getInternalName(),
"Use a dedicated Object lock instead of synchronizing on primitive wrapper values.");
}
continue;
}
if (opcode == Opcodes.ARETURN) {
if (isAllocaValue(frame.getStack(frame.getStackSize() - 1))) {
addViolation(violations, classFile, outputDir, classNode.name, method, "SIMD alloca value returned from method");
addSemanticViolation(violations, classFile, outputDir, classNode.name, method,
"SIMD alloca value returned from method",
"Keep SIMD alloca scratch arrays method-local and only pass them to Simd methods.");
}
continue;
}
if (opcode == Opcodes.PUTSTATIC) {
if (isAllocaValue(frame.getStack(frame.getStackSize() - 1))) {
addViolation(violations, classFile, outputDir, classNode.name, method, "SIMD alloca value stored into static field");
addSemanticViolation(violations, classFile, outputDir, classNode.name, method,
"SIMD alloca value stored into static field",
"Keep SIMD alloca scratch arrays method-local and only pass them to Simd methods.");
}
continue;
}
if (opcode == Opcodes.PUTFIELD) {
if (isAllocaValue(frame.getStack(frame.getStackSize() - 1))) {
addViolation(violations, classFile, outputDir, classNode.name, method, "SIMD alloca value stored into instance field");
addSemanticViolation(violations, classFile, outputDir, classNode.name, method,
"SIMD alloca value stored into instance field",
"Keep SIMD alloca scratch arrays method-local and only pass them to Simd methods.");
}
continue;
}
if (opcode == Opcodes.AASTORE) {
if (isAllocaValue(frame.getStack(frame.getStackSize() - 1))) {
addViolation(violations, classFile, outputDir, classNode.name, method, "SIMD alloca value stored into object array");
addSemanticViolation(violations, classFile, outputDir, classNode.name, method,
"SIMD alloca value stored into object array",
"Keep SIMD alloca scratch arrays method-local and only pass them to Simd methods.");
}
continue;
}
Expand All @@ -575,16 +603,19 @@ && isAllocaValue(frame.getStack(frame.getStackSize() - 1 - argumentCount))) {
usesAlloca = true;
}
if (usesAlloca && !isSimdOwner(methodInsn.owner)) {
addViolation(violations, classFile, outputDir, classNode.name, method,
"SIMD alloca value passed to non-Simd method " + methodInsn.owner + "#" + methodInsn.name + methodInsn.desc);
addSemanticViolation(violations, classFile, outputDir, classNode.name, method,
"SIMD alloca value passed to non-Simd method " + methodInsn.owner + "#" + methodInsn.name + methodInsn.desc,
"Keep SIMD alloca scratch arrays method-local and only pass them to Simd methods.");
}
continue;
}
if (instruction instanceof InvokeDynamicInsnNode) {
Type[] args = Type.getArgumentTypes(((InvokeDynamicInsnNode) instruction).desc);
for (int i = 0; i < args.length; i++) {
if (isAllocaValue(frame.getStack(frame.getStackSize() - 1 - i))) {
addViolation(violations, classFile, outputDir, classNode.name, method, "SIMD alloca value passed to invokedynamic");
addSemanticViolation(violations, classFile, outputDir, classNode.name, method,
"SIMD alloca value passed to invokedynamic",
"Keep SIMD alloca scratch arrays method-local and only pass them to Simd methods.");
break;
}
}
Expand All @@ -593,19 +624,26 @@ && isAllocaValue(frame.getStack(frame.getStackSize() - 1 - argumentCount))) {
}
}

private void addViolation(List<Violation> violations, File classFile, File outputDir, String sourceClass, MethodNode method, String referencedMember) {
private void addSemanticViolation(List<Violation> violations, File classFile, File outputDir, String sourceClass, MethodNode method, String referencedMember, String suggestion) {
String relativePath = classFile.getAbsolutePath().replace(outputDir.getAbsolutePath(), "");
if (relativePath.startsWith(File.separator)) {
relativePath = relativePath.substring(1);
}
violations.add(new Violation(sourceClass, method.name + method.desc, referencedMember,
"Keep SIMD alloca scratch arrays method-local and only pass them to Simd methods.", relativePath));
suggestion, relativePath));
}

private static boolean isAllocaValue(BasicValue value) {
return value instanceof SimdAllocaValue && ((SimdAllocaValue) value).alloca;
}

private static boolean isPrimitiveWrapperValue(BasicValue value) {
if (value == null || value.getType() == null || value.getType().getSort() != Type.OBJECT) {
return false;
}
return PRIMITIVE_WRAPPER_INTERNAL_NAMES.contains(value.getType().getInternalName());
}

private static final class SimdAllocaValue extends BasicValue {
private final boolean alloca;

Expand Down Expand Up @@ -645,11 +683,13 @@ private SimdAllocaInterpreter() {

@Override
public BasicValue newValue(Type type) {
BasicValue base = super.newValue(type);
if (base == null || base == BasicValue.UNINITIALIZED_VALUE) {
return base;
if (type == null) {
return BasicValue.UNINITIALIZED_VALUE;
}
if (type == Type.VOID_TYPE) {
return null;
}
return new SimdAllocaValue(base.getType(), false);
return new SimdAllocaValue(type, false);
}

@Override
Expand Down Expand Up @@ -708,12 +748,27 @@ public BasicValue naryOperation(AbstractInsnNode insn, List<? extends BasicValue

@Override
public BasicValue merge(BasicValue value1, BasicValue value2) {
boolean alloca = isAllocaValue(value1) || isAllocaValue(value2);
if (value1.equals(value2)) {
return new SimdAllocaValue(value1.getType(), alloca);
}
if (isReferenceValue(value1) && isReferenceValue(value2)) {
return new SimdAllocaValue(Type.getObjectType("java/lang/Object"), alloca);
}
BasicValue base = super.merge(value1, value2);
if (base == null) {
return null;
}
return new SimdAllocaValue(base.getType(), isAllocaValue(value1) || isAllocaValue(value2));
return new SimdAllocaValue(base.getType(), alloca);
}
}

private static boolean isReferenceValue(BasicValue value) {
if (value == null || value.getType() == null) {
return false;
}
int sort = value.getType().getSort();
return sort == Type.OBJECT || sort == Type.ARRAY;
}

private void collectClassFiles(File file, List<File> out) {
Expand Down
111 changes: 111 additions & 0 deletions ...odenameone-maven-plugin/src/test/java/com/codename1/maven/BytecodeComplianceMojoTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,43 @@ void rejectsSimdAllocaValueReturnedFromMethod(@TempDir Path tempDir) throws Exce
"Expected SIMD alloca verifier to reject returning scratch arrays");
}

@Test
void rejectsSynchronizationOnPrimitiveWrapper(@TempDir Path tempDir) throws Exception {
Path outputDir = tempDir.resolve("classes");
Path allowedDir = tempDir.resolve("allowed");
Files.createDirectories(outputDir);
Files.createDirectories(allowedDir);

writeJavaLangObject(allowedDir);
writePrimitiveWrapperApi(allowedDir, "java/lang/Integer", "valueOf", "(I)Ljava/lang/Integer;");
writePrimitiveWrapperSynchronizedClass(outputDir, "app/IntegerLockUser", "java/lang/Integer", "valueOf", "(I)Ljava/lang/Integer;");

BytecodeComplianceMojo mojo = new BytecodeComplianceMojo();
Map<String, ?> allowedIndex = buildClassIndex(mojo, Collections.singletonList(allowedDir.toFile()));
List<?> violations = scanProjectClasses(mojo, outputDir, allowedIndex, Collections.<String, Object>emptyMap());

assertTrue(hasViolationForReferencePrefix(violations, "Synchronization on primitive wrapper java/lang/Integer"),
"Expected primitive wrapper synchronization to be rejected");
}

@Test
void allowsSynchronizationOnDedicatedObjectLock(@TempDir Path tempDir) throws Exception {
Path outputDir = tempDir.resolve("classes");
Path allowedDir = tempDir.resolve("allowed");
Files.createDirectories(outputDir);
Files.createDirectories(allowedDir);

writeJavaLangObject(allowedDir);
writeObjectSynchronizedClass(outputDir, "app/ObjectLockUser");

BytecodeComplianceMojo mojo = new BytecodeComplianceMojo();
Map<String, ?> allowedIndex = buildClassIndex(mojo, Collections.singletonList(allowedDir.toFile()));
List<?> violations = scanProjectClasses(mojo, outputDir, allowedIndex, Collections.<String, Object>emptyMap());

assertFalse(hasViolationForReferencePrefix(violations, "Synchronization on primitive wrapper"),
"Expected synchronization on a normal Object lock to remain allowed");
}

@Test
void recognizesAllSimdAllocaHelperNames() throws Exception {
Method method = BytecodeComplianceMojo.class.getDeclaredMethod("isSimdAllocaMethod", String.class, String.class, String.class);
Expand Down Expand Up @@ -673,6 +710,65 @@ private void writeAllocaCaller(Path root, String className, AllocaUsage usage) t
writeBytes(root, className, writer.toByteArray());
}

private void writePrimitiveWrapperSynchronizedClass(Path root, String className, String owner, String methodName, String descriptor) throws Exception {
ClassWriter writer = new ClassWriter(0);
writer.visit(Opcodes.V1_8, Opcodes.ACC_PUBLIC, className, null, "java/lang/Object", null);

MethodVisitor init = writer.visitMethod(Opcodes.ACC_PUBLIC, "<init>", "()V", null, null);
init.visitCode();
init.visitVarInsn(Opcodes.ALOAD, 0);
init.visitMethodInsn(Opcodes.INVOKESPECIAL, "java/lang/Object", "<init>", "()V", false);
init.visitInsn(Opcodes.RETURN);
init.visitMaxs(1, 1);
init.visitEnd();

MethodVisitor run = writer.visitMethod(Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC, "run", "()V", null, null);
run.visitCode();
run.visitInsn(Opcodes.ICONST_1);
run.visitMethodInsn(Opcodes.INVOKESTATIC, owner, methodName, descriptor, false);
run.visitInsn(Opcodes.DUP);
run.visitVarInsn(Opcodes.ASTORE, 0);
run.visitInsn(Opcodes.MONITORENTER);
run.visitVarInsn(Opcodes.ALOAD, 0);
run.visitInsn(Opcodes.MONITOREXIT);
run.visitInsn(Opcodes.RETURN);
run.visitMaxs(2, 1);
run.visitEnd();

writer.visitEnd();
writeBytes(root, className, writer.toByteArray());
}

private void writeObjectSynchronizedClass(Path root, String className) throws Exception {
ClassWriter writer = new ClassWriter(0);
writer.visit(Opcodes.V1_8, Opcodes.ACC_PUBLIC, className, null, "java/lang/Object", null);

MethodVisitor init = writer.visitMethod(Opcodes.ACC_PUBLIC, "<init>", "()V", null, null);
init.visitCode();
init.visitVarInsn(Opcodes.ALOAD, 0);
init.visitMethodInsn(Opcodes.INVOKESPECIAL, "java/lang/Object", "<init>", "()V", false);
init.visitInsn(Opcodes.RETURN);
init.visitMaxs(1, 1);
init.visitEnd();

MethodVisitor run = writer.visitMethod(Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC, "run", "()V", null, null);
run.visitCode();
run.visitTypeInsn(Opcodes.NEW, "java/lang/Object");
run.visitInsn(Opcodes.DUP);
run.visitMethodInsn(Opcodes.INVOKESPECIAL, "java/lang/Object", "<init>", "()V", false);
run.visitInsn(Opcodes.DUP);
run.visitVarInsn(Opcodes.ASTORE, 0);
run.visitInsn(Opcodes.MONITORENTER);
run.visitVarInsn(Opcodes.ALOAD, 0);
run.visitInsn(Opcodes.MONITOREXIT);
run.visitInsn(Opcodes.RETURN);
run.visitMaxs(2, 1);
run.visitEnd();

writer.visitEnd();
writeBytes(root, className, writer.toByteArray());
}


private void writeSubclass(Path root, String className, String superName) throws Exception {
ClassWriter writer = new ClassWriter(0);
Expand All @@ -690,6 +786,21 @@ private void writeSubclass(Path root, String className, String superName) throws
writeBytes(root, className, writer.toByteArray());
}

private void writePrimitiveWrapperApi(Path root, String className, String methodName, String descriptor) throws Exception {
ClassWriter writer = new ClassWriter(0);
writer.visit(Opcodes.V1_8, Opcodes.ACC_PUBLIC | Opcodes.ACC_FINAL, className, null, "java/lang/Object", null);

MethodVisitor valueOf = writer.visitMethod(Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC, methodName, descriptor, null, null);
valueOf.visitCode();
valueOf.visitInsn(Opcodes.ACONST_NULL);
valueOf.visitInsn(Opcodes.ARETURN);
valueOf.visitMaxs(1, 1);
valueOf.visitEnd();

writer.visitEnd();
writeBytes(root, className, writer.toByteArray());
}

private void writeApiClass(Path root, String className, String methodName, String descriptor) throws Exception {
ClassWriter writer = new ClassWriter(0);
writer.visit(Opcodes.V1_8, Opcodes.ACC_PUBLIC, className, null, "java/lang/Object", null);
Expand Down
Loading