Agent should not open java.lang package to unnamed module of the application class loader

org.jacoco.agent.rt/src/org/jacoco/agent/rt/internal/PreMain.java

@@ -13,9 +13,6 @@
 package org.jacoco.agent.rt.internal;
 
 import java.lang.instrument.Instrumentation;
-import java.util.Collections;
-import java.util.Map;
-import java.util.Set;
 
 import org.jacoco.core.runtime.AgentOptions;
 import org.jacoco.core.runtime.IRuntime;
@@ -58,58 +55,23 @@ public static void premain(final String options, final Instrumentation inst)
 	private static IRuntime createRuntime(final Instrumentation inst)
 			throws Exception {
 
-		if (redefineJavaBaseModule(inst)) {
-			return new InjectedClassRuntime(Object.class, "$JaCoCo");
+		final IRuntime injectedClassRuntime = createInjectedClassRuntime(inst);
+		if (injectedClassRuntime != null) {
+			return injectedClassRuntime;
 		}
 
 		return ModifiedSystemClassRuntime.createFor(inst,
 				"java/lang/UnknownError");
 	}
 
-	/**
-	 * Opens {@code java.base} module for {@link InjectedClassRuntime} when
-	 * executed on Java 9 JREs or higher.
-	 *
-	 * @return <code>true</code> when running on Java 9 or higher,
-	 *         <code>false</code> otherwise
-	 * @throws Exception
-	 *             if unable to open
-	 */
-	private static boolean redefineJavaBaseModule(
+	private static IRuntime createInjectedClassRuntime(
 			final Instrumentation instrumentation) throws Exception {
 		try {
 			Class.forName("java.lang.Module");
 		} catch (final ClassNotFoundException e) {
-			return false;
+			return null;
 		}
-
-		Instrumentation.class.getMethod("redefineModule", //
-				Class.forName("java.lang.Module"), //
-				Set.class, //
-				Map.class, //
-				Map.class, //
-				Set.class, //
-				Map.class //
-		).invoke(instrumentation, // instance
-				getModule(Object.class), // module
-				Collections.emptySet(), // extraReads
-				Collections.emptyMap(), // extraExports
-				Collections.singletonMap("java.lang",
-						Collections.singleton(
-								getModule(InjectedClassRuntime.class))), // extraOpens
-				Collections.emptySet(), // extraUses
-				Collections.emptyMap() // extraProvides
-		);
-		return true;
-	}
-
-	/**
-	 * @return {@code cls.getModule()}
-	 */
-	private static Object getModule(final Class<?> cls) throws Exception {
-		return Class.class //
-				.getMethod("getModule") //
-				.invoke(cls);
+		return InjectedClassRuntime.createFor(instrumentation);
 	}
 
 }

org.jacoco.ant.test/src/org/jacoco/ant/CoverageTaskTest.xml

@@ -158,4 +158,12 @@
 		<au:assertLogContains text="java/sql/Timestamp"/>
 	</target>
 
+	<target name="testIllegalReflectiveAccess">
+		<jacoco:coverage destfile="${exec.file}">
+			<java classname="org.jacoco.ant.IllegalReflectiveAccessTarget" fork="true" failonerror="true">
+				<classpath path="${org.jacoco.ant.coverageTaskTest.classes.dir}"/>
+			</java>
+		</jacoco:coverage>
+	</target>
+
 </project>

org.jacoco.ant.test/src/org/jacoco/ant/IllegalReflectiveAccessTarget.java

@@ -0,0 +1,46 @@
+/*******************************************************************************
+ * Copyright (c) 2009, 2022 Mountainminds GmbH & Co. KG and Contributors
+ * This program and the accompanying materials are made available under
+ * the terms of the Eclipse Public License 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *    Evgeny Mandrikov - initial API and implementation
+ *
+ *******************************************************************************/
+
+package org.jacoco.ant;
+
+import java.lang.reflect.Constructor;
+
+public class IllegalReflectiveAccessTarget {
+
+	public static void main(String[] args) throws Exception {
+		try {
+			Class.forName("java.lang.invoke.MethodHandles$Lookup")
+					.getField("ORIGINAL");
+		} catch (ClassNotFoundException e) {
+			// Java < 7
+			return;
+		} catch (NoSuchFieldException e) {
+			// Java < 16
+			return;
+		}
+
+		final Constructor<?> c = Class.forName("java.lang.Module")
+				.getDeclaredConstructors()[0];
+		try {
+			c.setAccessible(true);
+			throw new AssertionError("Exception expected");
+		} catch (RuntimeException e) {
+			if (!e.getClass().getName()
+					.equals("java.lang.reflect.InaccessibleObjectException")) {
+				throw new AssertionError(
+						"InaccessibleObjectException expected");
+			}
+		}
+	}
+
+}

org.jacoco.ant.test/src/org/jacoco/ant/InstrumentTaskTest.xml

@@ -63,7 +63,7 @@
 		<jacoco:instrument destdir="${instr.dir}">
 			<fileset dir="${lib.dir}" includes="*.jar"/>
 		</jacoco:instrument>
-		<au:assertLogContains text="Instrumented 15 classes to ${temp.dir}"/>
+		<au:assertLogContains text="Instrumented 16 classes to ${temp.dir}"/>
 
 		<unzip src="${instr.dir}/test.jar" dest="${instr.dir}"/>
 		<au:assertFileDoesntExist file="${instr.dir}/META-INF/TEST.RSA" />
@@ -84,7 +84,7 @@
 		<jacoco:instrument destdir="${instr.dir}" removesignatures="false">
 			<fileset dir="${lib.dir}" includes="*.jar"/>
 		</jacoco:instrument>
-		<au:assertLogContains text="Instrumented 15 classes to ${temp.dir}"/>
+		<au:assertLogContains text="Instrumented 16 classes to ${temp.dir}"/>
 
 		<unzip src="${instr.dir}/test.jar" dest="${instr.dir}"/>
 		<au:assertFileExists file="${instr.dir}/META-INF/TEST.RSA" />
@@ -95,7 +95,7 @@
 		<jacoco:instrument destdir="${temp.dir}">
 			<fileset dir="${org.jacoco.ant.instrumentTaskTest.classes.dir}" includes="**/*.class"/>
 		</jacoco:instrument>
-		<au:assertLogContains text="Instrumented 15 classes to ${temp.dir}"/>
+		<au:assertLogContains text="Instrumented 16 classes to ${temp.dir}"/>
 		<au:assertFileExists file="${temp.dir}/org/jacoco/ant/InstrumentTaskTest.class" />
 
 		<echo file="${temp.dir}/jacoco-agent.properties">destfile=test.exec</echo>
@@ -112,7 +112,7 @@
 		<jacoco:instrument destdir="${temp.dir}">
 			<fileset dir="${org.jacoco.ant.instrumentTaskTest.classes.dir}" includes="**/*.class"/>
 		</jacoco:instrument>
-		<au:assertLogContains text="Instrumented 15 classes to ${temp.dir}"/>
+		<au:assertLogContains text="Instrumented 16 classes to ${temp.dir}"/>
 		<au:assertFileExists file="${temp.dir}/org/jacoco/ant/InstrumentTaskTest.class" />
 
 		<java classname="org.jacoco.ant.TestTarget" failonerror="true" fork="true">
@@ -129,7 +129,7 @@
 		<jacoco:instrument destdir="${temp.dir}">
 			<fileset dir="${org.jacoco.ant.instrumentTaskTest.classes.dir}" includes="**/*.class"/>
 		</jacoco:instrument>
-		<au:assertLogContains text="Instrumented 15 classes to ${temp.dir}"/>
+		<au:assertLogContains text="Instrumented 16 classes to ${temp.dir}"/>
 		<au:assertFileExists file="${temp.dir}/org/jacoco/ant/InstrumentTaskTest.class" />
 
 		<java classname="org.jacoco.ant.TestTarget" failonerror="false" fork="true">

org.jacoco.core.test/src/org/jacoco/core/runtime/InjectedClassRuntimeTest.java

@@ -54,8 +54,8 @@ public void startup_should_not_create_duplicate_class_definition()
 			createRuntime().startup(null);
 			fail("exception expected");
 		} catch (final InvocationTargetException e) {
-			assertTrue(e.getCause() instanceof LinkageError);
-			assertTrue(e.getCause().getMessage()
+			assertTrue(e.getCause().getCause() instanceof LinkageError);
+			assertTrue(e.getCause().getCause().getMessage()
 					.contains("duplicate class definition"));
 		}
 	}

org.jacoco.core/src/org/jacoco/core/runtime/InjectedClassRuntime.java

@@ -12,10 +12,19 @@
  *******************************************************************************/
 package org.jacoco.core.runtime;
 
+import org.jacoco.core.internal.InputStreams;
 import org.objectweb.asm.ClassWriter;
 import org.objectweb.asm.MethodVisitor;
 import org.objectweb.asm.Opcodes;
 
+import java.io.IOException;
+import java.io.InputStream;
+import java.lang.instrument.Instrumentation;
+import java.lang.reflect.Method;
+import java.util.Collections;
+import java.util.Map;
+import java.util.Set;
+
 /**
  * {@link IRuntime} which defines a new class using
  * {@code java.lang.invoke.MethodHandles.Lookup.defineClass} introduced in Java
@@ -28,6 +37,8 @@ public class InjectedClassRuntime extends AbstractRuntime {
 
 	private static final String FIELD_TYPE = "Ljava/lang/Object;";
 
+	private final ClassLoader classLoader;
+
 	private final Class<?> locator;
 
 	private final String injectedClassName;
@@ -43,19 +54,76 @@ public class InjectedClassRuntime extends AbstractRuntime {
 	 */
 	public InjectedClassRuntime(final Class<?> locator,
 			final String simpleClassName) {
+		this(null, locator, simpleClassName);
+	}
+
+	private InjectedClassRuntime(final ClassLoader classLoader,
+			final Class<?> locator, final String simpleClassName) {
+		this.classLoader = classLoader == null
+				? this.getClass().getClassLoader()
+				: classLoader;
 		this.locator = locator;
 		this.injectedClassName = locator.getPackage().getName().replace('.',
 				'/') + '/' + simpleClassName;
 	}
 
+	/**
+	 * Creates a new {@link InjectedClassRuntime}.
+	 *
+	 * @param instrumentation
+	 *            instrumentation interface
+	 * @return new runtime instance
+	 */
+	public static IRuntime createFor(final Instrumentation instrumentation)
+			throws Exception {
+		final ClassLoader classLoader = new ClassLoader() {
+			@Override
+			protected Class<?> loadClass(String name, boolean resolve)
+					throws ClassNotFoundException {
+				if (!name.equals(Lookup.class.getName())) {
+					return super.loadClass(name, resolve);
+				}
+				final InputStream resourceAsStream = getResourceAsStream(
+						name.replace('.', '/') + ".class");
+				final byte[] bytes;
+				try {
+					bytes = InputStreams.readFully(resourceAsStream);
+				} catch (IOException e) {
+					throw new RuntimeException(e);
+				}
+				return defineClass(name, bytes, 0, bytes.length);
+			}
+		};
+		Instrumentation.class.getMethod("redefineModule", //
+				Class.forName("java.lang.Module"), //
+				Set.class, //
+				Map.class, //
+				Map.class, //
+				Set.class, //
+				Map.class //
+		).invoke(instrumentation, // instance
+				Class.class.getMethod("getModule").invoke(Object.class), // module
+				Collections.emptySet(), // extraReads
+				Collections.emptyMap(), // extraExports
+				Collections.singletonMap("java.lang",
+						Collections.singleton(
+								ClassLoader.class.getMethod("getUnnamedModule")
+										.invoke(classLoader))), // extraOpens
+				Collections.emptySet(), // extraUses
+				Collections.emptyMap() // extraProvides
+		);
+		return new InjectedClassRuntime(classLoader, Object.class, "$JaCoCo");
+	}
+
 	@Override
 	public void startup(final RuntimeData data) throws Exception {
 		super.startup(data);
-		Lookup //
-				.privateLookupIn(locator, Lookup.lookup()) //
-				.defineClass(createClass(injectedClassName)) //
-				.getField(FIELD_NAME) //
-				.set(null, data);
+		final Method injector = classLoader.loadClass(Lookup.class.getName())
+				.getDeclaredMethod("defineClass", Class.class, byte[].class);
+		injector.setAccessible(true);
+		final Class<?> injectedClass = (Class<?>) injector.invoke(null, locator,
+				createClass(injectedClassName));
+		injectedClass.getField(FIELD_NAME).set(null, data);
 	}
 
 	public void shutdown() {
@@ -97,7 +165,7 @@ private Lookup(final Object instance) {
 		/**
 		 * @return a lookup object for the caller of this method
 		 */
-		static Lookup lookup() throws Exception {
+		private static Lookup lookup() throws Exception {
 			return new Lookup(Class //
 					.forName("java.lang.invoke.MethodHandles") //
 					.getMethod("lookup") //
@@ -113,7 +181,7 @@ static Lookup lookup() throws Exception {
 		 *            the caller lookup object
 		 * @return a lookup object for the target class, with private access
 		 */
-		static Lookup privateLookupIn(final Class<?> targetClass,
+		private static Lookup privateLookupIn(final Class<?> targetClass,
 				final Lookup lookup) throws Exception {
 			return new Lookup(Class //
 					.forName("java.lang.invoke.MethodHandles") //
@@ -130,13 +198,30 @@ static Lookup privateLookupIn(final Class<?> targetClass,
 		 *            the class bytes
 		 * @return class
 		 */
-		Class<?> defineClass(final byte[] bytes) throws Exception {
+		private Class<?> defineClass(final byte[] bytes) throws Exception {
 			return (Class<?>) Class //
 					.forName("java.lang.invoke.MethodHandles$Lookup")
 					.getMethod("defineClass", byte[].class)
 					.invoke(this.instance, new Object[] { bytes });
 		}
 
+		/**
+		 * Defines a class to the same class loader and in the same package and
+		 * protection domain as given class.
+		 *
+		 * @param locator
+		 *            the target class
+		 * @param bytes
+		 *            the class bytes
+		 * @return class
+		 */
+		static Class<?> defineClass(final Class<?> locator, final byte[] bytes)
+				throws Exception {
+			return Lookup //
+					.privateLookupIn(locator, Lookup.lookup()) //
+					.defineClass(bytes); //
+		}
+
 	}
 
 }