diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/ASMHelper.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/ASMHelper.java index 5deabf02d8c..885e0c4dd31 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/ASMHelper.java +++ b/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; diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/CapturedContextInstrumentor.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/CapturedContextInstrumentor.java index 48808cd0b2b..f2c17783763 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/CapturedContextInstrumentor.java +++ b/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,11 +657,13 @@ 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; @@ -668,6 +671,12 @@ private static void rewritePreviousStoreInsn( } } + 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; diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturedSnapshotTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturedSnapshotTest.java index 5b82a207785..006dce057fa 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturedSnapshotTest.java +++ b/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); diff --git a/dd-java-agent/agent-debugger/src/test/resources/classfiles/ByteSourceJsonBootstrapper.classfile b/dd-java-agent/agent-debugger/src/test/resources/classfiles/ByteSourceJsonBootstrapper.classfile new file mode 100644 index 00000000000..e9761336173 Binary files /dev/null and b/dd-java-agent/agent-debugger/src/test/resources/classfiles/ByteSourceJsonBootstrapper.classfile differ