Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import datadog.trace.api.git.GitInfoProvider;
import datadog.trace.bootstrap.ContextStore;
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
import datadog.trace.civisibility.compiler.CompilerModuleExporter;
import datadog.trace.civisibility.config.ExecutionSettings;
import datadog.trace.civisibility.config.JvmInfo;
import datadog.trace.civisibility.coverage.file.instrumentation.CoverageClassTransformer;
Expand Down Expand Up @@ -75,6 +76,10 @@ public static void start(Instrumentation inst, SharedCommunicationObjects sco) {

sco.createRemaining(config);

if (config.isCiVisibilityCompilerPluginAutoConfigurationEnabled()) {
inst.addTransformer(new CompilerModuleExporter(inst));
}

CiVisibilityMetricCollector metricCollector =
config.isCiVisibilityTelemetryEnabled()
? new CiVisibilityMetricCollectorImpl()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
package datadog.trace.civisibility.compiler;

import datadog.trace.util.JDK9ModuleAccess;
import java.lang.instrument.ClassFileTransformer;
import java.lang.instrument.Instrumentation;
import java.security.ProtectionDomain;
import java.util.Collections;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* Exports jdk.compiler internal packages to the classloader that loads dd-javac-plugin.
*
* <p>On JDK 16+ (strong encapsulation), dd-javac-plugin's CompilerModuleOpener uses burningwave to
* export these packages. On JDK 26+, burningwave fails due to Unsafe restrictions (JEP 471/498).
* This transformer intercepts dd-javac-plugin class loading and does the export using
* Instrumentation.redefineModule() instead.
*
* <p>Each Maven compilation step (compile, testCompile) may use a different classloader, so we
* track which classloaders have already been exported to and re-export for new ones.
*/
public class CompilerModuleExporter implements ClassFileTransformer {

private static final Logger LOGGER = LoggerFactory.getLogger(CompilerModuleExporter.class);

private static final String COMPILER_PLUGIN_CLASS_PREFIX = "datadog/compiler/";
private static final String[] COMPILER_PACKAGES = {
"com.sun.tools.javac.api",
"com.sun.tools.javac.code",
"com.sun.tools.javac.comp",
"com.sun.tools.javac.tree",
"com.sun.tools.javac.util"
};

private final Instrumentation inst;
private final Set<ClassLoader> exportedClassLoaders =
Collections.newSetFromMap(new ConcurrentHashMap<>());

public CompilerModuleExporter(Instrumentation inst) {
this.inst = inst;
}

@Override
public byte[] transform(
ClassLoader loader,
String className,
Class<?> classBeingRedefined,
ProtectionDomain protectionDomain,
byte[] classfileBuffer) {
if (loader != null
&& className != null
&& className.startsWith(COMPILER_PLUGIN_CLASS_PREFIX)
&& exportedClassLoaders.add(loader)) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could probably use something like exportedClassLoaders.computeIfAbsent(loader, this::exportJdkCompilerModule) to make sure this is truly thread safe (otherwise a thread that lost the race on add just exits, without waiting for the module to be exported in the other thread

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense! Updated the approach in adb3df5

try {
JDK9ModuleAccess.exportModuleToUnnamedModule(
inst, "jdk.compiler", COMPILER_PACKAGES, loader);
LOGGER.debug("Exported jdk.compiler to classloader {}", loader);
} catch (Throwable e) {
LOGGER.debug("Could not export jdk.compiler packages for compiler plugin", e);
}
}
return null; // no bytecode modification
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,6 @@ import spock.lang.IgnoreIf
import spock.lang.Shared
import spock.lang.TempDir

@IgnoreIf(reason = "TODO: Fix for Java 26. Javac plugin fails to populate source tags correctly.", value = {
JavaVirtualMachine.isJavaVersionAtLeast(26)
})
class GradleDaemonSmokeTest extends AbstractGradleTest {

private static final String TEST_SERVICE_NAME = "test-gradle-service"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@ import java.util.concurrent.TimeoutException

import static org.junit.jupiter.api.Assumptions.assumeTrue

@IgnoreIf(reason = "TODO: Fix for Java 26. Maven compiler fails to compile the tests for Java 26-ea.", value = {
JavaVirtualMachine.isJavaVersionAtLeast(26)
})
@IgnoreIf(reason = "IBM8 has flaky AES-GCM TLS failures when downloading Maven artifacts", value = {
JavaVirtualMachine.isIbm8()
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
<dependency>
<groupId>org.apache.groovy</groupId>
<artifactId>groovy-bom</artifactId>
<version>4.0.28</version>
<version>4.0.30</version>
<type>pom</type>
<scope>import</scope>
</dependency>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@

import java.lang.instrument.Instrumentation;
import java.lang.reflect.AnnotatedElement;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;

/** Use standard API to work with JPMS modules on Java9+. */
Expand Down Expand Up @@ -33,4 +36,28 @@ public static void addModuleReads(
emptySet(),
emptyMap());
}

/** Exports specific packages of a named module to a classloader's unnamed module. */
@SuppressWarnings({"rawtypes", "unchecked"})
public static void exportModuleToUnnamedModule(
Instrumentation inst,
String moduleName,
String[] packageNames,
ClassLoader targetClassLoader) {
java.util.Optional<java.lang.Module> optModule =
java.lang.ModuleLayer.boot().findModule(moduleName);
if (!optModule.isPresent()) {
return;
}
java.lang.Module module = optModule.get();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can java.lang.Module be imported so that we don't have to specify its full name every time?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I followed the approach that was already in use in the file out of caution, although adding the imports doesn't seem to cause any failures/side effects. @mcculls might have better context here as he did the original implementation for the class

java.lang.Module unnamedModule = targetClassLoader.getUnnamedModule();

Set<java.lang.Module> target = Collections.singleton(unnamedModule);
Map<String, Set<java.lang.Module>> extraExports = new HashMap<>();
for (String packageName : packageNames) {
extraExports.put(packageName, target);
}

inst.redefineModule(module, emptySet(), (Map) extraExports, emptyMap(), emptySet(), emptyMap());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the (Map) cast?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, missed it during cleanup, addressed in adb3df5

}
}
Loading