Skip to content

Commit

Permalink
Fix var hoisting issue when no previous store
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jpbempel committed Dec 20, 2024
1 parent a3e9bda commit 86fd474
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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";
Expand Down Expand Up @@ -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);
Expand Down
Binary file not shown.

0 comments on commit 86fd474

Please sign in to comment.