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,14 @@ 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 = InjectedClassRuntime
+				.createFor(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(
-			final Instrumentation instrumentation) throws Exception {
-		try {
-			Class.forName("java.lang.Module");
-		} catch (final ClassNotFoundException e) {
-			return false;
-		}
-
-		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);
-	}
-
 }

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/src/org/jacoco/core/runtime/InjectedClassRuntime.java

@@ -12,10 +12,18 @@
  *******************************************************************************/
 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.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
@@ -48,6 +56,71 @@ public InjectedClassRuntime(final Class<?> locator,
 				'/') + '/' + simpleClassName;
 	}
 
+	/**
+	 * Creates a new {@link InjectedClassRuntime} that will define class in the
+	 * {@code java.lang} package. To do so this method opens package
+	 * {@code java.lang} to the unnamed module of a new ClassLoader that is used
+	 * to create an instance of {@link InjectedClassRuntime}, so this <a href=
+	 * "https://docs.oracle.com/javase/specs/jvms/se11/html/jvms-5.html#jvms-5.3.6">
+	 * unnamed module is distinct from all run-time modules (including unnamed
+	 * modules) bound to other class loaders</a>.
+	 *
+	 * @param instrumentation
+	 *            instrumentation interface
+	 * @return new runtime instance, null if JVM version is lower than 9
+	 * @throws Exception
+	 *             if unable to create
+	 */
+	public static IRuntime createFor(final Instrumentation instrumentation)
+			throws Exception {
+		try {
+			Class.forName("java.lang.Module");
+		} catch (final ClassNotFoundException e) {
+			return null;
+		}
+		final ClassLoader classLoader = new ClassLoader() {
+			@Override
+			protected Class<?> loadClass(String name, boolean resolve)
+					throws ClassNotFoundException {
+				if (!name.equals(InjectedClassRuntime.class.getName())
+						&& !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 (IRuntime) classLoader
+				.loadClass(InjectedClassRuntime.class.getName())
+				.getConstructor(Class.class, String.class)
+				.newInstance(Object.class, "$JaCoCo");
+	}
+
 	@Override
 	public void startup(final RuntimeData data) throws Exception {
 		super.startup(data);

org.jacoco.doc/docroot/doc/changes.html

@@ -35,6 +35,13 @@ <h3>New Features</h3>
       (GitHub <a href="https://github.com/jacoco/jacoco/issues/1393">#1393</a>).</li>
 </ul>
 
+<h3>Fixed bugs</h3>
+<ul>
+  <li>Agent should not open <code>java.lang</code> package to unnamed module of the
+      application class loader
+      (GitHub <a href="https://github.com/jacoco/jacoco/issues/1334">#1334</a>).</li>
+</ul>
+
 <h3>Non-functional Changes</h3>
 <ul>
   <li>JaCoCo now depends on ASM 9.4