Fix var hoisting issue when no previous store

In some cases the range of a local var may not have previous init
in the immediate neighborhood because of basic block arrangement.
Limit the rewirite of previous instruction for store only and looking
 back only 10 instructions

Author: jpbempel

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();
@@ -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 (!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);