Fix remapping arrays and improve test coverage

.github/workflows/continuous.yml

@@ -31,14 +31,23 @@ jobs:
         uses: gradle/actions/setup-gradle@v4
       - name: Build
         run: ./gradlew build
-      - name: Upload build data
+      - name: Upload dist build data
         if: always()
         uses: actions/upload-artifact@v4
         with:
-          name: build
+          name: dist-build
           retention-days: 1
           path: |
             btrace-dist/build
+            btrace-instr/build/classes/traces
+      - name: Upload test trace data
+        if: always()
+        uses: actions/upload-artifact@v4
+        with:
+          name: test-trace
+          retention-days: 1
+          path: |
+            btrace-instr/build/classes/traces
       - name: Archive test reports
         if: always()
         uses: actions/upload-artifact@v4
@@ -77,8 +86,13 @@ jobs:
       - name: Download build data
         uses: actions/download-artifact@v4
         with:
-          name: build
+          name: dist-build
           path: btrace-dist/build
+      - name: Download test trace data
+        uses: actions/download-artifact@v4
+        with:
+          name: test-trace
+          path: btrace-instr/build/classes/traces
       - name: Run tests
         run: |
           set +x

README.md

@@ -5,7 +5,7 @@
 A safe, dynamic tracing tool for the Java platform
 
 ## Version
-2.2.6
+2.3.0-SNAPSHOT
 
 ## Quick Summary
 BTrace is a safe, dynamic tracing tool for the Java platform.
@@ -38,6 +38,12 @@ cd <btrace>
 The binary dist packages can be found in `<btrace>/btrace-dist/build/distributions` as the *.tar.gz, *.zip, *.rpm and *.deb files.
 The exploded binary folder which can be used right away is located at `<btrace>/btrace-dist/build/resources/main` which serves as the __BTRACE_HOME__ location.
 
+##### Golden Files
+Some of the instrumentor related tests are using golden files. Therefore, it is necessary to update those files
+when the injected code is changed. This can be done with the help of passing in `updateTestData` Gradle property.
+Eg. running the tests like `./gradlew test -PupdateTestData` will regenerate all golden files which then must be
+checked in to the Git repository.
+
 
 ## Using BTrace
 ### Installation

btrace-agent/src/main/java/org/openjdk/btrace/agent/Main.java

@@ -143,46 +143,56 @@ private static synchronized void main(String args, Instrumentation inst) {
       settings.setDebug(isDebug);
       DebugSupport.initLoggers(isDebug, log);
 
-      log.debug("parsed command line arguments");
       parseArgs();
       // settings are all built-up; set the logging system properties accordingly
       DebugSupport.initLoggers(settings.isDebug(), log);
 
-      log.debug("Adding class transformer");
-      inst.addTransformer(transformer, true);
-      int startedScripts = startScripts();
-
       String tmp = argMap.get(NO_SERVER);
       // noServer is defaulting to true if startup scripts are defined
       boolean noServer = tmp != null ? Boolean.parseBoolean(tmp) : hasScripts();
+      Thread agentThread = null;
       if (noServer) {
         log.debug("noServer is true, server not started");
-        return;
+      } else {
+        agentThread =
+            new Thread(
+                () -> {
+                  BTraceRuntime.enter();
+                  try {
+                    startServer();
+                  } finally {
+                    BTraceRuntime.leave();
+                  }
+                });
       }
-      Thread agentThread =
-          new Thread(
-              () -> {
-                BTraceRuntime.enter();
-                try {
-                  startServer();
-                } finally {
-                  BTraceRuntime.leave();
-                }
-              });
       // set the fall-back instrumentation object to BTraceRuntime
       BTraceRuntime.instrumentation = inst;
       // force back-registration of BTraceRuntimeImpl in BTraceRuntime
       BTraceRuntimes.getDefault();
       // init BTraceRuntime
       BTraceRuntime.initUnsafe();
-      BTraceRuntime.enter();
+      if (agentThread != null) {
+        BTraceRuntime.enter();
+        try {
+          agentThread.setDaemon(true);
+          log.debug("starting agent thread");
+
+          agentThread.start();
+        } finally {
+          BTraceRuntime.leave();
+        }
+      }
+
+      log.debug("Adding class transformer");
+      inst.addTransformer(transformer, true);
       try {
-        agentThread.setDaemon(true);
-        log.debug("starting agent thread");
-        agentThread.start();
-      } finally {
-        BTraceRuntime.leave();
+        // the MethodHandleNatives must be instrumented to track start-end of indy linking to avoid deadlocking
+        Class<?> clz = ClassLoader.getSystemClassLoader().loadClass("java.lang.invoke.MethodHandleNatives");
+        inst.retransformClasses(clz);
+      } catch (Throwable t) {
+        log.debug("Failed to instrument MethodHandleNatives", t);
       }
+      int startedScripts = startScripts();
     } finally {
       log.debug("Agent init took: {}", (System.nanoTime() - ts) + "ns");
     }
@@ -791,9 +801,9 @@ private static boolean loadBTraceScript(String filePath, boolean traceToStdOut)
       } else {
         String traceOutput = clientSettings.getOutputFile();
         String outDir = clientSettings.getScriptOutputDir();
-        if (traceOutput == null || traceOutput.length() == 0) {
+        if (traceOutput == null || traceOutput.isEmpty()) {
           clientSettings.setOutputFile("${client}-${agent}.${ts}.btrace[default]");
-          if (outDir == null || outDir.length() == 0) {
+          if (outDir == null || outDir.isEmpty()) {
             clientSettings.setScriptOutputDir(scriptParent);
           }
         }
@@ -838,7 +848,7 @@ private static void startServer() {
       System.setProperty("btrace.wireio", String.valueOf(WireIO.VERSION));
 
       String scriptOutputFile = settings.getOutputFile();
-      if (scriptOutputFile != null && scriptOutputFile.length() > 0) {
+      if (scriptOutputFile != null && !scriptOutputFile.isEmpty()) {
         System.setProperty("btrace.output", scriptOutputFile);
       }
       ss = new ServerSocket(port);
@@ -856,7 +866,7 @@ private static void startServer() {
           log.debug("client accepted {}", sock);
         }
         ClientContext ctx = new ClientContext(inst, transformer, argMap, settings);
-        Client client = RemoteClient.getClient(ctx, sock, value -> handleNewClient(value));
+        Client client = RemoteClient.getClient(ctx, sock, Main::handleNewClient);
       } catch (RuntimeException | IOException re) {
         if (log.isDebugEnabled()) {
           log.debug("BTrace server failed", re);

btrace-compiler/src/main/java/org/openjdk/btrace/compiler/CompilerClassWriter.java

@@ -33,7 +33,9 @@
 import java.util.List;
 import org.objectweb.asm.ClassWriter;
 
-/** @author Jaroslav Bachorik */
+/**
+ * @author Jaroslav Bachorik
+ */
 class CompilerClassWriter extends ClassWriter {
   private final URLClassLoader cl;
 

btrace-compiler/src/main/java/org/openjdk/btrace/compiler/PCPP.java

@@ -545,7 +545,9 @@ private String resolveDefine(String word, boolean returnNullIfNotFound) {
     return lastWord;
   }
 
-  /** @param isIfdef if true, we're processing #ifdef; if false, we're processing #ifndef. */
+  /**
+   * @param isIfdef if true, we're processing #ifdef; if false, we're processing #ifndef.
+   */
   private void handleIfdef(boolean isIfdef) throws IOException {
     // Next token is the name of the #ifdef
     String symbolName = nextWord();
@@ -576,7 +578,9 @@ private void handleEndif() {
     debugPrint(!enabledBeforePopping, "#endif/end-else");
   }
 
-  /** @param isIf if true, we're processing #if; if false, we're processing #elif. */
+  /**
+   * @param isIf if true, we're processing #if; if false, we're processing #elif.
+   */
   private void handleIf(boolean isIf) throws IOException {
     // System.out.println("IN HANDLE_" + (isIf ? "IF" : "ELIF") + " file \"" + filename() + " line "
     // + lineNumber());

btrace-compiler/src/main/java/org/openjdk/btrace/compiler/Postprocessor.java

@@ -42,7 +42,9 @@
 import org.objectweb.asm.Opcodes;
 import org.objectweb.asm.Type;
 
-/** @author Jaroslav Bachorik */
+/**
+ * @author Jaroslav Bachorik
+ */
 public class Postprocessor extends ClassVisitor {
   private final List<FieldDescriptor> fields = new ArrayList<>();
   private boolean shortSyntax = false;

btrace-core/src/main/java/org/openjdk/btrace/core/BTraceRuntime.java

@@ -493,7 +493,9 @@ static long sizeof(Object obj) {
     return getRt().$(key);
   }
 
-  /** @see BTraceUtils#instanceOf(java.lang.Object, java.lang.String) */
+  /**
+   * @see BTraceUtils#instanceOf(java.lang.Object, java.lang.String)
+   */
   static boolean instanceOf(Object obj, String className) {
     if (obj instanceof AnyType) {
       // the only time we can have AnyType on stack
@@ -1155,34 +1157,46 @@ static void printAggregation(String name, Aggregation aggregation, String format
     getRt().send(new GridDataCommand(name, aggregation.getData(), format));
   }
 
-  /** @see BTraceUtils.Profiling#newProfiler() */
+  /**
+   * @see BTraceUtils.Profiling#newProfiler()
+   */
   static Profiler newProfiler() {
     return getRt().newProfiler();
   }
 
-  /** @see BTraceUtils.Profiling#newProfiler(int) */
+  /**
+   * @see BTraceUtils.Profiling#newProfiler(int)
+   */
   static Profiler newProfiler(int expectedMethodCnt) {
     return getRt().newProfiler(expectedMethodCnt);
   }
 
-  /** @see BTraceUtils.Profiling#recordEntry(Profiler, java.lang.String) */
+  /**
+   * @see BTraceUtils.Profiling#recordEntry(Profiler, java.lang.String)
+   */
   static void recordEntry(Profiler profiler, String methodName) {
     profiler.recordEntry(methodName);
   }
 
   // profiling related methods
 
-  /** @see BTraceUtils.Profiling#recordExit(Profiler, java.lang.String, long) */
+  /**
+   * @see BTraceUtils.Profiling#recordExit(Profiler, java.lang.String, long)
+   */
   static void recordExit(Profiler profiler, String methodName, long duration) {
     profiler.recordExit(methodName, duration);
   }
 
-  /** @see BTraceUtils.Profiling#snapshot(Profiler) */
+  /**
+   * @see BTraceUtils.Profiling#snapshot(Profiler)
+   */
   static Profiler.Snapshot snapshot(Profiler profiler) {
     return profiler.snapshot();
   }
 
-  /** @see BTraceUtils.Profiling#snapshotAndReset(Profiler) */
+  /**
+   * @see BTraceUtils.Profiling#snapshotAndReset(Profiler)
+   */
   static Profiler.Snapshot snapshotAndReset(Profiler profiler) {
     return profiler.snapshot(true);
   }
@@ -1291,6 +1305,8 @@ public interface Impl {
     int version();
 
     boolean isBootstrapClass(String className);
+
+    String getClassName();
   }
 
   public interface BTraceRuntimeAccessor {

btrace-core/src/main/java/org/openjdk/btrace/core/DebugSupport.java

@@ -29,6 +29,7 @@
 import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+
 import org.slf4j.Logger;
 import org.slf4j.impl.SimpleLogger;
 
@@ -62,6 +63,7 @@ public static void initLoggers(boolean debug, Logger logger) {
                 : 20); // 10 is the 'debug' level for SLF4J SimpleLogger, 20 is the info level
       } catch (NoSuchFieldException | IllegalAccessException e) {
         System.err.println("[btrace] Unable to set debug log level");
+        e.printStackTrace(System.err);
       }
     }
   }

btrace-core/src/main/java/org/openjdk/btrace/core/SharedSettings.java

@@ -26,7 +26,9 @@
 
 import java.util.Map;
 
-/** @author Jaroslav Bachorik */
+/**
+ * @author Jaroslav Bachorik
+ */
 public final class SharedSettings {
   public static final String DEBUG_KEY = "debug";
   public static final String DUMP_DIR_KEY = "dumpDir";

btrace-core/src/main/java/org/openjdk/btrace/core/annotations/Event.java

@@ -63,21 +63,37 @@ public String getType() {
 
   /** Field kind */
   enum FieldKind {
-    /** @see jdk.jfr.Timestamp */
+    /**
+     * @see jdk.jfr.Timestamp
+     */
     TIMESTAMP,
-    /** @see jdk.jfr.Timespan */
+    /**
+     * @see jdk.jfr.Timespan
+     */
     TIMESPAN,
-    /** @see jdk.jfr.DataAmount */
+    /**
+     * @see jdk.jfr.DataAmount
+     */
     DATAAMOUNT,
-    /** @see jdk.jfr.Frequency */
+    /**
+     * @see jdk.jfr.Frequency
+     */
     FREQUENCY,
-    /** @see jdk.jfr.MemoryAddress */
+    /**
+     * @see jdk.jfr.MemoryAddress
+     */
     MEMORYADDRESS,
-    /** @see jdk.jfr.Percentage */
+    /**
+     * @see jdk.jfr.Percentage
+     */
     PERCENTAGE,
-    /** @see jdk.jfr.BooleanFlag */
+    /**
+     * @see jdk.jfr.BooleanFlag
+     */
     BOOLEANFLAG,
-    /** @see jdk.jfr.Unsigned */
+    /**
+     * @see jdk.jfr.Unsigned
+     */
     UNSIGNED,
     /** No additional field kind specification */
     NONE
@@ -92,19 +108,29 @@ enum FieldKind {
       String value() default "";
     }
 
-    /** @return field type */
+    /**
+     * @return field type
+     */
     FieldType type();
 
-    /** @return field name */
+    /**
+     * @return field name
+     */
     String name();
 
-    /** @return field label */
+    /**
+     * @return field label
+     */
     String label() default "";
 
-    /** @return field description */
+    /**
+     * @return field description
+     */
     String description() default "";
 
-    /** @return additional field kind */
+    /**
+     * @return additional field kind
+     */
     Kind kind() default @Kind(name = FieldKind.NONE);
   }