From f986c3d18386e77c974e4272ab27c3b573773c9b Mon Sep 17 00:00:00 2001 From: Alexander Kriegisch Date: Sun, 28 Jan 2024 16:13:22 +0700 Subject: [PATCH] Workaround for defining classes during LTW Overhaul ClassLoaderWeavingAdaptor to use statically initialised Unsafe instances and method handles pointing to their 'defineClass' methods. Those now work universally on JDKs 8-21. In older JDKs, the method used to be in sun.misc.Unsafe, in more recent ones on jdk.internal.misc.Unsafe. It is challenging to fetch instances, especially as reflection protection and module boundaries have been increased in the JDK progressively. But finally, a solution was adapted from Byte Buddy (BB). Kudos to BB author Rafael Winterhalter. The previous solution to use ClassLoader::defineClass and require '--add-opens' is no longer necessary for the first time since it became necessary in AspectJ 1.9.7 with Java 16 support. Add org.ow2.asm:asm-common as a dependency everywhere org.ow2.asm:asm was used before. Maybe that is too many places, but no worse than before. Add missing dependency on loadtime to aspectjweaver. This kept a build like "mvn install -am -pl aspectjweaver" from picking up changed loadtime classes. Fixes #117. Signed-off-by: Alexander Kriegisch --- ajde.core/pom.xml | 4 + ajde/pom.xml | 4 + ajdoc/pom.xml | 4 + aspectjtools/pom.xml | 4 + aspectjweaver/aspectjweaver-assembly.xml | 1 + aspectjweaver/pom.xml | 9 + loadtime/pom.xml | 4 + .../loadtime/ClassLoaderWeavingAdaptor.java | 373 +++++++++++------- org.aspectj.ajdt.core/pom.xml | 4 + .../test/java/org/aspectj/tools/ajc/Ajc.java | 4 +- .../org/aspectj/tools/ajc/AjcTestCase.java | 9 +- pom.xml | 5 + run-all-junit-tests/pom.xml | 5 + taskdefs/pom.xml | 4 + testing-drivers/pom.xml | 4 + testing/pom.xml | 4 + .../java/org/aspectj/testing/AntSpec.java | 12 +- .../java/org/aspectj/testing/RunSpec.java | 15 +- tests/bugs153/pr155033/ant.xml | 4 +- tests/bugs153/pr157474/ant-server.xml | 4 +- tests/bugs153/pr158957/ant.xml | 4 +- tests/java5/ataspectj/ajc-ant.xml | 69 ++-- .../ataspectj/ataspectj/UnweavableTest.java | 4 + tests/ltw/ant-server.xml | 8 +- tests/ltw/ant.xml | 24 +- tests/pom.xml | 4 + tests/profiling/build.xml | 8 +- tests/tracing/ant.xml | 16 +- weaver/pom.xml | 4 + 29 files changed, 395 insertions(+), 223 deletions(-) diff --git a/ajde.core/pom.xml b/ajde.core/pom.xml index 8b147cdcf..474124717 100644 --- a/ajde.core/pom.xml +++ b/ajde.core/pom.xml @@ -45,6 +45,10 @@ org.ow2.asm asm + + org.ow2.asm + asm-commons + org.aspectj testing-util diff --git a/ajde/pom.xml b/ajde/pom.xml index dd552c5af..44428acc4 100644 --- a/ajde/pom.xml +++ b/ajde/pom.xml @@ -36,6 +36,10 @@ org.ow2.asm asm + + org.ow2.asm + asm-commons + org.aspectj org.aspectj.ajdt.core diff --git a/ajdoc/pom.xml b/ajdoc/pom.xml index cb22d2145..f183c37e9 100644 --- a/ajdoc/pom.xml +++ b/ajdoc/pom.xml @@ -58,6 +58,10 @@ org.ow2.asm asm + + org.ow2.asm + asm-commons + org.aspectj testing-util diff --git a/aspectjtools/pom.xml b/aspectjtools/pom.xml index 2280deb3f..12d18fecf 100644 --- a/aspectjtools/pom.xml +++ b/aspectjtools/pom.xml @@ -364,6 +364,10 @@ org.ow2.asm asm + + org.ow2.asm + asm-commons + org.aspectj runtime diff --git a/aspectjweaver/aspectjweaver-assembly.xml b/aspectjweaver/aspectjweaver-assembly.xml index 576bb2700..877ddf6b1 100644 --- a/aspectjweaver/aspectjweaver-assembly.xml +++ b/aspectjweaver/aspectjweaver-assembly.xml @@ -17,6 +17,7 @@ false org.ow2.asm:asm + org.ow2.asm:asm-commons diff --git a/aspectjweaver/pom.xml b/aspectjweaver/pom.xml index 34cefc74c..c5702e2aa 100644 --- a/aspectjweaver/pom.xml +++ b/aspectjweaver/pom.xml @@ -389,10 +389,19 @@ weaver ${project.version} + + org.aspectj + loadtime + ${project.version} + org.ow2.asm asm + + org.ow2.asm + asm-commons + diff --git a/loadtime/pom.xml b/loadtime/pom.xml index e73382812..549ee286d 100644 --- a/loadtime/pom.xml +++ b/loadtime/pom.xml @@ -41,6 +41,10 @@ org.ow2.asm asm + + org.ow2.asm + asm-commons + org.aspectj testing-util diff --git a/loadtime/src/main/java/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java b/loadtime/src/main/java/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java index ad700fcfd..9a1052075 100644 --- a/loadtime/src/main/java/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java +++ b/loadtime/src/main/java/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java @@ -11,23 +11,16 @@ package org.aspectj.weaver.loadtime; import java.io.File; import java.io.IOException; import java.io.InputStream; +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.reflect.AccessibleObject; import java.lang.reflect.Field; import java.lang.reflect.Method; import java.net.MalformedURLException; import java.net.URL; import java.security.ProtectionDomain; -import java.util.ArrayList; -import java.util.Enumeration; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Iterator; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; +import java.util.*; import java.util.Map.Entry; -import java.util.Properties; -import java.util.Set; -import java.util.StringTokenizer; import org.aspectj.bridge.AbortException; import org.aspectj.bridge.Constants; @@ -54,6 +47,9 @@ import org.aspectj.weaver.tools.TraceFactory; import org.aspectj.weaver.tools.WeavingAdaptor; import org.aspectj.weaver.tools.cache.WeavedClassCache; +import org.objectweb.asm.*; +import org.objectweb.asm.commons.ClassRemapper; +import org.objectweb.asm.commons.Remapper; import sun.misc.Unsafe; /** @@ -1029,156 +1025,243 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor { } } - private Unsafe unsafe; + private static final Object lock = new Object(); - private Unsafe getUnsafe() throws NoSuchFieldException, IllegalAccessException { - if (unsafe == null) { - Field theUnsafeField = Unsafe.class.getDeclaredField("theUnsafe"); - theUnsafeField.setAccessible(true); - return (Unsafe) theUnsafeField.get(null); - } - return unsafe; - } + /** + * Instance of either {@link sun.misc.Unsafe} or {@link jdk.internal.misc.Unsafe}. Luckily, both types have + * {@code defineClass} methods with identical signatures. I.e., method handle {@link #defineClassMethodHandle} can be + * invoked with the same set of parameters for both types. + */ + private static Object unsafeInstance = null; - private static Method bindTo_Method, invokeWithArguments_Method = null; - private static Object defineClassMethodHandle = null; + /** + * Method handle for defining new classes in arbitrary class loaders. For invocation, use in connection with + * {@link #unsafeInstance}. + */ + private static MethodHandle defineClassMethodHandle; - private static Boolean initializedForJava11 = false; + static { + try { + createDefineClassMethodHandle(); + } + catch (Exception initializationError) { + new RuntimeException( + "The aspect weaver cannot determine any valid method to define auxiliary classes in arbitrary class loaders. " + + "Aspect weaving will *not* work, and you will see subsequent errors. Please search for corresponding " + + "issues at https://github.com/eclipse-aspectj/aspectj/issues. If there are none, please create a new one.", + initializationError + ).printStackTrace(); + } + } - // In order to let this code compile on earlier versions of Java (8), use reflection to discover the elements - // we need to define classes. - private static synchronized void initializeForJava11() { - if (initializedForJava11) return; + /** + * Scaffolding for defining classes in arbitrary class loaders + *

+ * Inspired by and shamelessly adapted from Byte Buddy's {@code ClassInjector}. + * Special thanks to Byte Buddy (BB) author Rafael Winterhalter, who briefly mentioned this approach in a + * GitHub comment related to JDK issue + * JDK-8200559. + *

+ * Background: Instead of BB, we use ASM and reflection as follows: + *

    + *
  • + * Create a mirror class for {@link AccessibleObject} with a different package name in a separate, throw-away + * class loader. + *
    • + *
  • + * Use the mirror class to calculate the {@link Unsafe#objectFieldOffset(Field)} for boolean field + * {@link AccessibleObject#override}, which is expected to be identical to the offset of the same field in the + * original class. + *
    • + *
  • + * After we have the offset, we can use it to override the field value in the original class, deactivating access + * checks for {@link jdk.internal.misc.Unsafe#defineClass(String, byte[], int, int, ClassLoader, ProtectionDomain)}, + * the method we need to execute using a method handle. + *
    • + *
+ * All these serve the sole purpose enable LTW without {@code --add-opens java.base/java.lang=ALL-UNNAMED} on the + * JVM command line on JDK 16+, which was necessary for AspectJ 1.9.7 (Java 16) to 1.9.21 (Java 21). + * + * @throws Exception if anything goes wrong, trying to determine a usable {@code defineClass} method handle from any + * of the inspected classes + */ + private static synchronized void createDefineClassMethodHandle() throws Exception { + Unsafe publicUnsafeInstance = null; try { - // MethodType defineClassMethodType = MethodType.methodType(Class.class, new Class[]{String.class, byte[].class, int.class, int.class, ProtectionDomain.class}); - Class methodType_Class = Class.forName("java.lang.invoke.MethodType"); - Method methodTypeMethodOnMethodTypeClass = methodType_Class.getDeclaredMethod("methodType", Class.class,Class[].class); - methodTypeMethodOnMethodTypeClass.setAccessible(true); - Object defineClassMethodType = methodTypeMethodOnMethodTypeClass.invoke(null, Class.class, new Class[] {String.class,byte[].class,int.class,int.class,ProtectionDomain.class}); - - // MethodHandles.Lookup methodHandlesLookup = MethodHandles.lookup(); - Class methodHandles_Class = Class.forName("java.lang.invoke.MethodHandles"); - Method lookupMethodOnMethodHandlesClass = methodHandles_Class.getDeclaredMethod("lookup"); - lookupMethodOnMethodHandlesClass.setAccessible(true); - Object methodHandlesLookup = lookupMethodOnMethodHandlesClass.invoke(null); - - // MethodHandles.Lookup lookup = MethodHandles.privateLookupIn(ClassLoader.class, methodHandlesLookup); - Class methodHandlesLookup_Class = Class.forName("java.lang.invoke.MethodHandles$Lookup"); - Method privateLookupMethodOnMethodHandlesClass = methodHandles_Class.getDeclaredMethod("privateLookupIn",Class.class,methodHandlesLookup_Class); - privateLookupMethodOnMethodHandlesClass.setAccessible(true); - Object lookup = privateLookupMethodOnMethodHandlesClass.invoke(null, ClassLoader.class, methodHandlesLookup); - - // MethodHandle defineClassMethodHandle = lookup.findVirtual(ClassLoader.class, "defineClass", defineClassMethodType); - Method findVirtual_Method = methodHandlesLookup_Class.getDeclaredMethod("findVirtual", Class.class,String.class,methodType_Class); - findVirtual_Method.setAccessible(true); - defineClassMethodHandle = findVirtual_Method.invoke(lookup, ClassLoader.class, "defineClass",defineClassMethodType); - - // clazz = defineClassMethodHandle.bindTo(loader).invokeWithArguments(name, bytes, 0, bytes.length); - Class methodHandle_Class = Class.forName("java.lang.invoke.MethodHandle"); - bindTo_Method = methodHandle_Class.getDeclaredMethod("bindTo", Object.class); - invokeWithArguments_Method = methodHandle_Class.getDeclaredMethod("invokeWithArguments",Object[].class); - - initializedForJava11 = true; - } catch (Exception e) { - e.printStackTrace(); + Field publicUnsafeField = Unsafe.class.getDeclaredField("theUnsafe"); + publicUnsafeField.setAccessible(true); + publicUnsafeInstance = (Unsafe) publicUnsafeField.get(null); + synchronized (lock) { + unsafeInstance = publicUnsafeInstance; + defineClassMethodHandle = createMethodHandle( + "sun.misc.Unsafe", "defineClass", + String.class, byte[].class, int.class, int.class, ClassLoader.class, ProtectionDomain.class + ); + } + } + catch (Exception publicUnsafeException) { + if (publicUnsafeInstance == null) + throw publicUnsafeException; + long overrideOffset = getAccessibleObjectOverrideOffset(publicUnsafeInstance); + Class internalUnsafeType = Class.forName("jdk.internal.misc.Unsafe"); + Field internalUnsafeField = internalUnsafeType.getDeclaredField("theUnsafe"); + publicUnsafeInstance.putBoolean(internalUnsafeField, overrideOffset, true); + Method internalUnsafeDefineClassMethod = internalUnsafeType.getMethod( + "defineClass", + String.class, byte[].class, int.class, int.class, ClassLoader.class, ProtectionDomain.class + ); + publicUnsafeInstance.putBoolean(internalUnsafeDefineClassMethod, overrideOffset, true); + synchronized (lock) { + unsafeInstance = internalUnsafeField.get(null); + defineClassMethodHandle = createMethodHandle(internalUnsafeDefineClassMethod); + } } } - private void defineClass(ClassLoader loader, String name, byte[] bytes, ProtectionDomain protectionDomain) { - if (trace.isTraceEnabled()) { - trace.enter("defineClass", this, new Object[] { loader, name, bytes }); + private static long getAccessibleObjectOverrideOffset(Unsafe unsafe) + throws IOException, ClassNotFoundException, NoSuchFieldException + { + Objects.requireNonNull(unsafe); + Field overrideField; + try { + overrideField = AccessibleObject.class.getDeclaredField("override"); + } + catch (NoSuchFieldException ignored) { + // On JDK 12+, field AccessibleObject.override is protected from reflection. The work-around is to create a + // mirror class with the same field layout by transforming the original class, so we can calculate the field + // offset of 'override' and set a value in the original class using the now known offset. + Class mirrorClass = getMirrorClass( + "java.lang.reflect.AccessibleObject", "org.aspectj.mirror.AccessibleObject", true + ); + overrideField = mirrorClass.getDeclaredField("override"); + } + return unsafe.objectFieldOffset(overrideField); + } + + public static MethodHandle createMethodHandle(String className, String methodName, Class... argumentTypes) + throws ClassNotFoundException, NoSuchMethodException, IllegalAccessException + { + Class clazz = Class.forName(className); + Method method = clazz.getDeclaredMethod(methodName, argumentTypes); + return createMethodHandle(method, false); + } + + public static MethodHandle createMethodHandle(Method method) throws IllegalAccessException { + return createMethodHandle(method, false); + } + + public static MethodHandle createMethodHandle(Method method, boolean setAccessible) + throws IllegalAccessException + { + // Use Method::setAccessible to access private methods. Caveat: This does not work for classes in packages not + // exported to the calling module (for LTW usually the unnamed module). + if (setAccessible) + method.setAccessible(true); + return MethodHandles.lookup().unreflect(method); + } + + @SuppressWarnings("SameParameterValue") + private static Class getMirrorClass(String originalClass, String mirrorClass, boolean removeMethods) + throws IOException, ClassNotFoundException + { + Objects.requireNonNull(originalClass); + Objects.requireNonNull(mirrorClass); + if (mirrorClass.equals(originalClass)) + throw new IllegalArgumentException("Mirror class name must be different from original " + originalClass); + byte[] mirrorClassBytes = getMirrorClassBytes(originalClass, mirrorClass, removeMethods); + ClassLoader mirrorClassLoader = new SingleClassLoader(mirrorClass, mirrorClassBytes); + return mirrorClassLoader.loadClass(mirrorClass); + } + + private static byte[] getMirrorClassBytes(String originalClass, String mirrorClass, boolean removeMethods) + throws IOException, ClassNotFoundException + { + Class aClass = Class.forName(originalClass); + try (InputStream input = aClass.getResourceAsStream(aClass.getSimpleName() + ".class")) { + Objects.requireNonNull(input); + ClassWriter classWriter = new ClassWriter(ClassWriter.COMPUTE_MAXS); + ClassRemapper classRemapper = new ClassRemapper(classWriter, new ClassNameRemapper(originalClass, mirrorClass)); + ClassVisitor classVisitor = removeMethods ? new MethodAndConstructorRemover(classRemapper) : classRemapper; + new ClassReader(input).accept(classVisitor, 0); + return classWriter.toByteArray(); } - Object clazz = null; - debug("generating class '" + name + "'"); - if (LangUtil.is11VMOrGreater()) { - try { - if (!initializedForJava11) { - initializeForJava11(); - } - // Do this: clazz = defineClassMethodHandle.bindTo(loader).invokeWithArguments(name, bytes, 0, bytes.length, protectionDomain); - Object o = bindTo_Method.invoke(defineClassMethodHandle,loader); - clazz = invokeWithArguments_Method.invoke(o, new Object[] {new Object[] {name, bytes, 0, bytes.length, protectionDomain}}); + } - } catch (Throwable t) { - t.printStackTrace(System.err); - warn("define generated class failed", t); - } - } else { - try { - if (defineClassMethod == null) { - synchronized (lock) { - getUnsafe(); - defineClassMethod = - Unsafe.class.getDeclaredMethod("defineClass", String.class,byte[].class,Integer.TYPE,Integer.TYPE, ClassLoader.class,ProtectionDomain.class); - } - } - defineClassMethod.setAccessible(true); - clazz = defineClassMethod.invoke(getUnsafe(), name,bytes,0,bytes.length,loader,protectionDomain); - } catch (LinkageError le) { - le.printStackTrace(); - // likely thrown due to defining something that already exists? - // Old comments from before moving to Unsafe.defineClass(): - // is already defined (happens for X$ajcMightHaveAspect interfaces since aspects are reweaved) - // TODO maw I don't think this is OK and - } catch (Exception e) { - e.printStackTrace(System.err); - warn("define generated class failed", e); - } + private static class ClassNameRemapper extends Remapper { + private final String originalClass; + private final String mirrorClass; + + public ClassNameRemapper(String originalClass, String mirrorClass) { + this.originalClass = originalClass.replace('.', '/'); + this.mirrorClass = mirrorClass.replace('.', '/'); } - if (trace.isTraceEnabled()) { - trace.exit("defineClass", clazz); + @Override + public String map(String internalName) { + return internalName.equals(originalClass) ? mirrorClass : internalName; } } - static Method defineClassMethod; - private static final Object lock = new Object(); + /** + * ASM class visitor removing all methods and constructors from the given class, leaving only the original fields + */ + private static class MethodAndConstructorRemover extends ClassVisitor { + public MethodAndConstructorRemover(ClassRemapper classRemapper) { + super(Opcodes.ASM9, classRemapper); + } -// /* -// This method is equivalent to the following code but use reflection to compile on Java 7: -// MethodHandles.Lookup baseLookup = MethodHandles.lookup(); -// MethodHandles.Lookup lookup = MethodHandles.privateLookupIn(ClassLoader.class, baseLookup); -// MethodHandle defineClassMethodHandle = lookup.findVirtual(ClassLoader.class, "defineClass", defineClassMethodType); -// handle.bindTo(classLoader).invokeWithArguments(className, classBytes, 0, classBytes.length)); -// */ -//@Override -//@SuppressWarnings("unchecked") -//public Class defineClass(ClassLoader classLoader, String className, byte[] classBytes) { -// Object baseLookup = methodHandlesLookup.invoke(null); -// Object lookup = methodHandlesPrivateLookupIn.invoke(null, ClassLoader.class, baseLookup); -// MethodHandle defineClassMethodHandle = (MethodHandle) lookupFindVirtual.invoke(lookup, ClassLoader.class, "defineClass", defineClassMethodType); -// try { -// return Cast.uncheckedCast(defineClassMethodHandle.bindTo(classLoader).invokeWithArguments(className, classBytes, 0, classBytes.length)); -// } catch (Throwable throwable) { -// throw new RuntimeException(throwable); -// return (Class) defineClassMethodHandle.bindTo(classLoader).invokeWithArguments(className, classBytes, 0, classBytes.length); -// } catch (Throwable e) { -// throw new RuntimeException(e); -// } -//} - - private void defineClass(ClassLoader loader, String name, byte[] bytes){ - defineClass(loader,name,bytes,null);//, ProtectionDomain protectionDomain) { + @Override + public MethodVisitor visitMethod(int access, String name, String descriptor, String signature, String[] exceptions) { + // Do not visit any methods or constructors, effectively removing them + return null; + } + } + + /** + * Throw-away child classloader with the sole purpose to define a single {@link Class} from the given bytecode + */ + private static class SingleClassLoader extends ClassLoader { + private final String mirrorClass; + private final byte[] mirrorClassBytes; + + private SingleClassLoader(String mirrorClass, byte[] mirrorClassBytes) { + super(SingleClassLoader.class.getClassLoader()); + this.mirrorClass = mirrorClass; + this.mirrorClassBytes = mirrorClassBytes; + } + + @Override + public Class loadClass(String name) throws ClassNotFoundException { + return name.equals(mirrorClass) + ? super.defineClass(null, mirrorClassBytes, 0, mirrorClassBytes.length) + : super.loadClass(name); + } + } + + private void defineClass(ClassLoader loader, String name, byte[] bytes) { + defineClass(loader, name, bytes, null); + } + + private void defineClass(ClassLoader loader, String name, byte[] bytes, ProtectionDomain protectionDomain) { + if (trace.isTraceEnabled()) + trace.enter("defineClass", this, new Object[] { loader, name, bytes }); + debug("generating class '" + name + "'"); + Class definedClass = null; + try { + if (defineClassMethodHandle == null) + throw new RuntimeException("no valid method to define auxiliary classes -> weaver not working"); + definedClass = (Class) defineClassMethodHandle + .bindTo(unsafeInstance) + .invokeWithArguments(name, bytes, 0, bytes.length, loader, protectionDomain); + } + catch (Throwable t) { + t.printStackTrace(System.err); + warn("define generated class failed", t); + } + finally { + if (trace.isTraceEnabled()) + trace.exit("defineClass", definedClass); + } } -// if (trace.isTraceEnabled()) { -// trace.enter("defineClass", this, new Object[] { loader, name, bytes, protectionDomain }); -// } -// Object clazz = null; -// debug("generating class '" + name + "'"); -// try { -// getUnsafe().defineClass(name, bytes, 0, bytes.length, loader, protectionDomain); -// } catch (LinkageError le) { -// // likely thrown due to defining something that already exists? -// // Old comments from before moving to Unsafe.defineClass(): -// // is already defined (happens for X$ajcMightHaveAspect interfaces since aspects are reweaved) -// // TODO maw I don't think this is OK and -// } catch (Exception e) { -// warn("define generated class failed", e); -// } -// -// if (trace.isTraceEnabled()) { -// trace.exit("defineClass", clazz); -// } -// } } diff --git a/org.aspectj.ajdt.core/pom.xml b/org.aspectj.ajdt.core/pom.xml index 6ef954270..6bb09b6eb 100644 --- a/org.aspectj.ajdt.core/pom.xml +++ b/org.aspectj.ajdt.core/pom.xml @@ -67,6 +67,10 @@ org.ow2.asm asm + + org.ow2.asm + asm-commons + diff --git a/org.aspectj.ajdt.core/src/test/java/org/aspectj/tools/ajc/Ajc.java b/org.aspectj.ajdt.core/src/test/java/org/aspectj/tools/ajc/Ajc.java index 7f4ae0109..c6d8c99a8 100644 --- a/org.aspectj.ajdt.core/src/test/java/org/aspectj/tools/ajc/Ajc.java +++ b/org.aspectj.ajdt.core/src/test/java/org/aspectj/tools/ajc/Ajc.java @@ -38,8 +38,7 @@ import org.aspectj.util.FileUtil; import static java.io.File.pathSeparator; import static java.io.File.separator; -import static org.aspectj.tools.ajc.AjcTestCase.CLASSPATH_ASM; -import static org.aspectj.tools.ajc.AjcTestCase.CLASSPATH_JUNIT; +import static org.aspectj.tools.ajc.AjcTestCase.*; /** * The Ajc class is intended for use as part of a unit-test suite, it drives the AspectJ compiler and lets you check the compilation @@ -74,6 +73,7 @@ public class Ajc { + outputFolder("bcel-builder") + pathSeparator + CLASSPATH_JUNIT + pathSeparator + CLASSPATH_ASM + + pathSeparator + CLASSPATH_ASM_COMMONS + outputFolder("bridge") + outputFolder("loadtime") + outputFolder("weaver") diff --git a/org.aspectj.ajdt.core/src/test/java/org/aspectj/tools/ajc/AjcTestCase.java b/org.aspectj.ajdt.core/src/test/java/org/aspectj/tools/ajc/AjcTestCase.java index 9aec7d947..6fb2d2602 100644 --- a/org.aspectj.ajdt.core/src/test/java/org/aspectj/tools/ajc/AjcTestCase.java +++ b/org.aspectj.ajdt.core/src/test/java/org/aspectj/tools/ajc/AjcTestCase.java @@ -70,9 +70,15 @@ public abstract class AjcTestCase extends TestCase { public static final String CLASSPATH_ASM = Arrays.stream(System.getProperty("java.class.path") .split(pathSeparator)) - .filter(path -> path.replace('\\', '/').contains("org/ow2/asm/")) + .filter(path -> path.replace('\\', '/').contains("org/ow2/asm/asm/")) .findFirst() .orElseThrow(() -> new RuntimeException("ASM library not found on classpath")); + public static final String CLASSPATH_ASM_COMMONS = + Arrays.stream(System.getProperty("java.class.path") + .split(pathSeparator)) + .filter(path -> path.replace('\\', '/').contains("org/ow2/asm/asm-commons/")) + .findFirst() + .orElseThrow(() -> new RuntimeException("ASM Commons library not found on classpath")); public static final String CLASSPATH_JDT_CORE = Arrays.stream(System.getProperty("java.class.path") .split(pathSeparator)) @@ -102,6 +108,7 @@ public abstract class AjcTestCase extends TestCase { + pathSeparator + ".." + separator + "lib" + separator + "bcel" + separator + "bcel-verifier.jar" + pathSeparator + CLASSPATH_JDT_CORE + pathSeparator + CLASSPATH_ASM + + pathSeparator + CLASSPATH_ASM_COMMONS // hmmm, this next one should perhaps point to an aj-build jar... + pathSeparator + ".." + separator + "lib" + separator + "test" + separator + "aspectjrt.jar" ; diff --git a/pom.xml b/pom.xml index 823f65ccd..5a62b005e 100644 --- a/pom.xml +++ b/pom.xml @@ -555,6 +555,11 @@ asm ${asm.version} + + org.ow2.asm + asm-commons + ${asm.version} + org.aspectj diff --git a/run-all-junit-tests/pom.xml b/run-all-junit-tests/pom.xml index 537d2db41..f2370f312 100644 --- a/run-all-junit-tests/pom.xml +++ b/run-all-junit-tests/pom.xml @@ -161,6 +161,11 @@ asm test + + org.ow2.asm + asm-commons + test + org.aspectj ajde diff --git a/taskdefs/pom.xml b/taskdefs/pom.xml index 9e6831a2d..8bc3c35bb 100644 --- a/taskdefs/pom.xml +++ b/taskdefs/pom.xml @@ -42,6 +42,10 @@ org.ow2.asm asm + + org.ow2.asm + asm-commons + ant diff --git a/testing-drivers/pom.xml b/testing-drivers/pom.xml index a15c3a534..d6975d22b 100644 --- a/testing-drivers/pom.xml +++ b/testing-drivers/pom.xml @@ -38,6 +38,10 @@ org.ow2.asm asm + + org.ow2.asm + asm-commons + diff --git a/testing/pom.xml b/testing/pom.xml index 38784790a..c5e082255 100644 --- a/testing/pom.xml +++ b/testing/pom.xml @@ -52,6 +52,10 @@ org.ow2.asm asm + + org.ow2.asm + asm-commons + org.aspectj testing-client diff --git a/testing/src/test/java/org/aspectj/testing/AntSpec.java b/testing/src/test/java/org/aspectj/testing/AntSpec.java index 7a6bd3cd9..33668d509 100644 --- a/testing/src/test/java/org/aspectj/testing/AntSpec.java +++ b/testing/src/test/java/org/aspectj/testing/AntSpec.java @@ -98,9 +98,15 @@ public class AntSpec implements ITestStep { // setup aj.dir "modules" folder p.setUserProperty("aj.root", new File("..").getAbsolutePath()); - // On Java 16+, LTW no longer works without this parameter. Add the argument here and not in AjcTestCase::run, - // because even if 'useLTW' and 'useFullLTW' are not set, we might in the future have tests for weaver attachment - // during runtime. See also docs/release/README-1.8.7.html. + // On Java 16+, LTW did not work on AspectJ 1.9.7 to 1.9.21 without this parameter. So, we added the argument here + // and not in AjcTestCase::run, because without 'useLTW' or 'useFullLTW', there might have been a need for weaver + // attachment during runtime. See also docs/release/README-1.8.7.adoc. + // + // Since AspectJ 1.9.21.1, '--add-opens' is no longer necessary, because we found a workaround for defining + // classes in arbitrary class loaders. But some tests, e.g. AtAjLTWTests.testLTWUnweavable, still use + // ClassLoader::defineClass to inject dynamically generated classes into the current class loader. Therefore, we + // still set the parameters, so they can be used on demand - not for LTW as such, but for class injection. See + // also tests/java5/ataspectj/ataspectj/UnweavableTest.java. // // Attention: Ant 1.6.3 under Linux neither likes "" (empty string) nor " " (space), on Windows it would not be // a problem. So we use "_dummy" Java system properties, even though they pollute the command line. diff --git a/testing/src/test/java/org/aspectj/testing/RunSpec.java b/testing/src/test/java/org/aspectj/testing/RunSpec.java index 593c5b957..4d7526f44 100644 --- a/testing/src/test/java/org/aspectj/testing/RunSpec.java +++ b/testing/src/test/java/org/aspectj/testing/RunSpec.java @@ -68,14 +68,21 @@ public class RunSpec implements ITestStep { if (vmargs == null) vmargs = ""; - // On Java 16+, LTW no longer works without this parameter. Add the argument here and not in AjcTestCase::run, - // because even if 'useLTW' and 'useFullLTW' are not set, we might in the future have tests for weaver attachment - // during runtime. See also docs/release/README-1.8.7.html. + // On Java 16+, LTW did not work on AspectJ 1.9.7 to 1.9.21 without this parameter. So, we added the argument here + // and not in AjcTestCase::run, because without 'useLTW' or 'useFullLTW', there might have been a need for weaver + // attachment during runtime. See also docs/release/README-1.8.7.adoc. + // + // Since AspectJ 1.9.21.1, '--add-opens' is no longer necessary, because we found a workaround for defining + // classes in arbitrary class loaders. But some tests, e.g. AtAjLTWTests.testLTWUnweavable, still use + // ClassLoader::defineClass to inject dynamically generated classes into the current class loader. Therefore, we + // still set the parameters, so they can be used on demand - not for LTW as such, but for class injection. See + // also tests/java5/ataspectj/ataspectj/UnweavableTest.java. // // The reason for setting this parameter for Java 9+ instead of 16+ is that it helps to avoid the JVM printing // unwanted illegal access warnings during weaving in 'useFullLTW' mode, either making existing tests fail or // having to assert on the warning messages. - vmargs += is16VMOrGreater() ? " --add-opens java.base/java.lang=ALL-UNNAMED" : ""; + // + // vmargs += is16VMOrGreater() ? " --add-opens java.base/java.lang=ALL-UNNAMED" : ""; AjcTestCase.RunResult rr = inTestCase.run(getClassToRun(), getModuleToRun(), args, vmargs, getClasspath(), getModulepath(), useLtw, "true".equalsIgnoreCase(usefullltw)); diff --git a/tests/bugs153/pr155033/ant.xml b/tests/bugs153/pr155033/ant.xml index 4f2dfb885..6ef88c87b 100644 --- a/tests/bugs153/pr155033/ant.xml +++ b/tests/bugs153/pr155033/ant.xml @@ -15,8 +15,8 @@ - - + + + diff --git a/tests/bugs153/pr158957/ant.xml b/tests/bugs153/pr158957/ant.xml index ca5dbc254..74dac1409 100644 --- a/tests/bugs153/pr158957/ant.xml +++ b/tests/bugs153/pr158957/ant.xml @@ -20,8 +20,8 @@ - - + + diff --git a/tests/java5/ataspectj/ajc-ant.xml b/tests/java5/ataspectj/ajc-ant.xml index 9fa620572..285082612 100644 --- a/tests/java5/ataspectj/ajc-ant.xml +++ b/tests/java5/ataspectj/ajc-ant.xml @@ -24,8 +24,8 @@ - - + + @@ -36,8 +36,8 @@ - - + + @@ -48,8 +48,8 @@ - - + + @@ -60,8 +60,8 @@ - - + + @@ -69,8 +69,8 @@ - - + + @@ -80,8 +80,8 @@ - - + + @@ -101,8 +101,8 @@ - - + + @@ -119,8 +119,8 @@ - - + + - - + + - - + + @@ -160,6 +160,7 @@ + @@ -175,8 +176,8 @@ - - + + - - + + @@ -211,8 +212,8 @@ - - + + @@ -230,8 +231,8 @@ - - + + - - + + @@ -273,8 +274,8 @@ - - + + @@ -291,8 +292,8 @@ - - + + diff --git a/tests/java5/ataspectj/ataspectj/UnweavableTest.java b/tests/java5/ataspectj/ataspectj/UnweavableTest.java index 1ef970612..50c877930 100644 --- a/tests/java5/ataspectj/ataspectj/UnweavableTest.java +++ b/tests/java5/ataspectj/ataspectj/UnweavableTest.java @@ -104,6 +104,8 @@ public class UnweavableTest extends TestCase { try { ClassLoader loader = this.getClass().getClassLoader(); + // Needs "--add-opens java.base/java.lang=ALL-UNNAMED" on the JVM command line, injected in Ant build via + // aj.addOpensKey and aj.addOpensValue variables. See also AntSpec::execute. Method def = ClassLoader.class.getDeclaredMethod("defineClass", String.class, byte[].class, int.class, int.class); def.setAccessible(true); Class gen = (Class) def.invoke(loader, "ataspectj.ISomeGen", cw.toByteArray(), 0, cw.toByteArray().length); @@ -127,6 +129,8 @@ public class UnweavableTest extends TestCase { try { ClassLoader loader = this.getClass().getClassLoader(); + // Needs "--add-opens java.base/java.lang=ALL-UNNAMED" on the JVM command line, injected in Ant build via + // aj.addOpensKey and aj.addOpensValue variables. See also AntSpec::execute. Method def = ClassLoader.class.getDeclaredMethod("defineClass", String.class, byte[].class, int.class, int.class); def.setAccessible(true); Class gen = (Class) def.invoke(loader, "ataspectj.unmatched.Gen", cw.toByteArray(), 0, cw.toByteArray().length); diff --git a/tests/ltw/ant-server.xml b/tests/ltw/ant-server.xml index c5f143fea..d4958e3e3 100644 --- a/tests/ltw/ant-server.xml +++ b/tests/ltw/ant-server.xml @@ -14,8 +14,8 @@ - - + + @@ -30,8 +30,8 @@ - - + + diff --git a/tests/ltw/ant.xml b/tests/ltw/ant.xml index 0cae97a36..abaf349d0 100644 --- a/tests/ltw/ant.xml +++ b/tests/ltw/ant.xml @@ -11,8 +11,8 @@ - - + + + - - + + @@ -80,8 +80,8 @@ to bootclasspath --> - - + + @@ -109,8 +109,8 @@ - - + + @@ -124,8 +124,8 @@ - - + + diff --git a/tests/pom.xml b/tests/pom.xml index 65d6ddc7a..1489268b1 100644 --- a/tests/pom.xml +++ b/tests/pom.xml @@ -43,6 +43,10 @@ org.ow2.asm asm + + org.ow2.asm + asm-commons + org.aspectj weaver diff --git a/tests/profiling/build.xml b/tests/profiling/build.xml index aa7db9d0e..23a98c2ba 100644 --- a/tests/profiling/build.xml +++ b/tests/profiling/build.xml @@ -290,8 +290,8 @@ - - + + @@ -323,8 +323,8 @@ - - + + diff --git a/tests/tracing/ant.xml b/tests/tracing/ant.xml index 6e0ddd78e..687d41b10 100644 --- a/tests/tracing/ant.xml +++ b/tests/tracing/ant.xml @@ -33,8 +33,8 @@ - - + + @@ -48,8 +48,8 @@ - - + + @@ -63,8 +63,8 @@ - - + + @@ -76,8 +76,8 @@ - - + + diff --git a/weaver/pom.xml b/weaver/pom.xml index 657813b8d..f51e590e1 100644 --- a/weaver/pom.xml +++ b/weaver/pom.xml @@ -65,5 +65,9 @@ org.ow2.asm asm + + org.ow2.asm + asm-commons + -- 2.39.5