Fix var hoisting issue when no previous store

dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/ASMHelper.java

@@ -340,6 +340,10 @@ public static String extractSuperClass(ClassNode classNode) {
     return Strings.getClassName(classNode.superName);
   }
 
+  public static boolean isStore(int opcode) {
+    return opcode >= Opcodes.ISTORE && opcode <= Opcodes.ASTORE;
+  }
+
   /** Wraps ASM's {@link org.objectweb.asm.Type} with associated generic types */
   public static class Type {
     private final org.objectweb.asm.Type mainType;

dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/CapturedContextInstrumentor.java

@@ -7,6 +7,7 @@
 import static com.datadog.debugger.instrumentation.ASMHelper.invokeVirtual;
 import static com.datadog.debugger.instrumentation.ASMHelper.isFinalField;
 import static com.datadog.debugger.instrumentation.ASMHelper.isStaticField;
+import static com.datadog.debugger.instrumentation.ASMHelper.isStore;
 import static com.datadog.debugger.instrumentation.ASMHelper.ldc;
 import static com.datadog.debugger.instrumentation.ASMHelper.newInstance;
 import static com.datadog.debugger.instrumentation.Types.CAPTURED_CONTEXT_TYPE;
@@ -68,7 +69,7 @@
 import org.slf4j.LoggerFactory;
 
 public class CapturedContextInstrumentor extends Instrumentor {
-  private static final Logger log = LoggerFactory.getLogger(CapturedContextInstrumentor.class);
+  private static final Logger LOGGER = LoggerFactory.getLogger(CapturedContextInstrumentor.class);
   private final boolean captureSnapshot;
   private final Limits limits;
   private final LabelNode contextInitLabel = new LabelNode();
@@ -639,8 +640,8 @@ private void rewriteLocalVarInsn(LocalVariableNode localVar, int oldSlot, int ne
   //   10: astore_1
   //   11: aload_1
   // range for slot 1 starts at 11
-  // javac always starts the range right after the init of the local var, so we can just look for
-  // the previous instruction
+  // javac often starts the range right after the init of the local var, so we can just look for
+  // the previous instruction. But not always, and we put an arbitrary limit to 10 instructions
   // for kotlinc, many instructions can separate the init and the range start
   // ex:
   //    LocalVariableTable:
@@ -656,18 +657,26 @@ private void rewriteLocalVarInsn(LocalVariableNode localVar, int oldSlot, int ne
   private static void rewritePreviousStoreInsn(
       LocalVariableNode localVar, int oldSlot, int newSlot) {
     AbstractInsnNode previous = localVar.start.getPrevious();
-    while (previous != null
-        && (!(previous instanceof VarInsnNode) || ((VarInsnNode) previous).var != oldSlot)) {
+    int processed = 0;
+    // arbitrary fixing limit to 10 previous instructions to look at
+    while (previous != null && !isVarStoreForSlot(previous, oldSlot) && processed < 10) {
       previous = previous.getPrevious();
+      processed++;
     }
-    if (previous != null) {
+    if (isVarStoreForSlot(previous, oldSlot)) {
       VarInsnNode varInsnNode = (VarInsnNode) previous;
       if (varInsnNode.var == oldSlot) {
         varInsnNode.var = newSlot;
       }
     }
   }
 
+  private static boolean isVarStoreForSlot(AbstractInsnNode node, int slotIdx) {
+    return node instanceof VarInsnNode
+        && isStore(node.getOpcode())
+        && ((VarInsnNode) node).var == slotIdx;
+  }
+
   private void createInProbeFinallyHandler(LabelNode inProbeStartLabel, LabelNode inProbeEndLabel) {
     LabelNode handlerLabel = new LabelNode();
     InsnList handler = new InsnList();
@@ -1234,7 +1243,7 @@ private static void addInheritedStaticFields(
             FieldNode fieldNode =
                 new FieldNode(field.getModifiers(), field.getName(), desc, null, field);
             results.add(fieldNode);
-            log.debug("Adding static inherited field {}", fieldNode.name);
+            LOGGER.debug("Adding static inherited field {}", fieldNode.name);
             fieldCount++;
             if (fieldCount > limits.maxFieldCount) {
               return;

dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturedSnapshotTest.java

@@ -66,7 +66,6 @@
 import java.lang.reflect.Modifier;
 import java.net.URISyntaxException;
 import java.net.URL;
-import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -134,6 +133,17 @@ public void methodProbe() throws IOException, URISyntaxException {
     assertEquals("CapturedSnapshot01.main", snapshot.getStack().get(0).getFunction());
   }
 
+  @Test
+  public void localVarHoistingNoPreviousStore() throws IOException, URISyntaxException {
+    final String CLASS_NAME = "com.fasterxml.jackson.core.json.ByteSourceJsonBootstrapper";
+    TestSnapshotListener listener = installSingleProbe(CLASS_NAME, "detectEncoding", null);
+    Class<?> testClass =
+        loadClass(
+            CLASS_NAME,
+            getClass().getResource("/classfiles/ByteSourceJsonBootstrapper.classfile").getFile());
+    assertNotNull(testClass);
+  }
+
   @Test
   public void singleLineProbe() throws IOException, URISyntaxException {
     final String CLASS_NAME = "CapturedSnapshot01";
@@ -219,12 +229,8 @@ public void oldClass1_1() throws Exception {
     when(config.isDebuggerVerifyByteCode()).thenReturn(true);
     Class<?> testClass =
         loadClass(
-            CLASS_NAME,
-            Paths.get(
-                    CapturedSnapshotTest.class
-                        .getResource("/classfiles/BooleanUtils.classfile")
-                        .toURI())
-                .toString());
+            CLASS_NAME, getClass().getResource("/classfiles/BooleanUtils.classfile").getFile());
+
     boolean result = Reflect.onClass(testClass).call("toBoolean", Boolean.TRUE).get();
     assertTrue(result);
     assertOneSnapshot(listener);