Skip to content

Commit

Permalink
when hoisting is disabled, behave like previously
Browse files Browse the repository at this point in the history
  • Loading branch information
@jpbempel
jpbempel committed Jan 7, 2025
1 parent 7c51414 commit 2413b07
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 53 deletions.
5 changes: 3 additions & 2 deletions ...ugger/src/main/java/com/datadog/debugger/instrumentation/CapturedContextInstrumentor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -902,7 +902,8 @@ private void collectLocalVariables(AbstractInsnNode location, InsnList insnList)
return;
}
Collection<LocalVariableNode> localVarNodes;
if (definition.isLineProbe()) {
boolean isLocalVarHoistingEnabled = Config.get().isDebuggerHoistLocalVarsEnabled();
if (definition.isLineProbe() || !isLocalVarHoistingEnabled) {
localVarNodes = methodNode.localVariables;
} else {
localVarNodes = unscopedLocalVars;
Expand All @@ -913,7 +914,7 @@ private void collectLocalVariables(AbstractInsnNode location, InsnList insnList)
int idx = variableNode.index - localVarBaseOffset;
if (idx >= argOffset) {
// var is local not arg
if (isLineProbe) {
if (isLineProbe || !isLocalVarHoistingEnabled) {
if (ASMHelper.isInScope(methodNode, variableNode, location)) {
applicableVars.add(variableNode);
}
Expand Down
126 changes: 75 additions & 51 deletions ...a-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturedSnapshotTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import static com.datadog.debugger.util.MoshiSnapshotHelper.REDACTED_IDENT_REASON;
import static com.datadog.debugger.util.MoshiSnapshotHelper.REDACTED_TYPE_REASON;
import static com.datadog.debugger.util.MoshiSnapshotTestHelper.VALUE_ADAPTER;
import static com.datadog.debugger.util.TestHelper.setFieldInConfig;
import static datadog.trace.bootstrap.debugger.util.Redaction.REDACTED_VALUE;
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
Expand Down Expand Up @@ -238,14 +239,20 @@ public void oldClass1_1() throws Exception {

@Test
public void oldJavacBug() throws Exception {
final String CLASS_NAME = "com.datadog.debugger.classfiles.JavacBug"; // compiled with jdk 1.6
TestSnapshotListener listener = installSingleProbe(CLASS_NAME, "main", null);
Class<?> testClass = Class.forName(CLASS_NAME);
assertNotNull(testClass);
int result = Reflect.onClass(testClass).call("main", "").get();
assertEquals(45, result);
// with local var hoisting and initialization at the beginning of the method, issue is resolved
assertEquals(1, listener.snapshots.size());
setFieldInConfig(Config.get(), "debuggerHoistLocalVarsEnabled", true);
try {
final String CLASS_NAME = "com.datadog.debugger.classfiles.JavacBug"; // compiled with jdk 1.6
TestSnapshotListener listener = installSingleProbe(CLASS_NAME, "main", null);
Class<?> testClass = Class.forName(CLASS_NAME);
assertNotNull(testClass);
int result = Reflect.onClass(testClass).call("main", "").get();
assertEquals(45, result);
// with local var hoisting and initialization at the beginning of the method, issue is
// resolved
assertEquals(1, listener.snapshots.size());
} finally {
setFieldInConfig(Config.get(), "debuggerHoistLocalVarsEnabled", false);
}
}

@Test
Expand Down Expand Up @@ -1801,32 +1808,39 @@ public void evaluateAtExitFalse() throws IOException, URISyntaxException {
value = "datadog.trace.api.Platform#isJ9",
disabledReason = "we cannot get local variable debug info")
public void uncaughtExceptionConditionLocalVar() throws IOException, URISyntaxException {
final String CLASS_NAME = "CapturedSnapshot05";
LogProbe probe =
createProbeBuilder(PROBE_ID, CLASS_NAME, "main", "(String)")
.when(
new ProbeCondition(DSL.when(DSL.ge(DSL.ref("after"), DSL.value(0))), "after >= 0"))
.evaluateAt(MethodLocation.EXIT)
.build();
TestSnapshotListener listener = installProbes(probe);
Class<?> testClass = compileAndLoadClass(CLASS_NAME);
setFieldInConfig(Config.get(), "debuggerHoistLocalVarsEnabled", true);
try {
Reflect.onClass(testClass).call("main", "triggerUncaughtException").get();
Assertions.fail("should not reach this code");
} catch (ReflectException ex) {
assertEquals("oops", ex.getCause().getCause().getMessage());

final String CLASS_NAME = "CapturedSnapshot05";
LogProbe probe =
createProbeBuilder(PROBE_ID, CLASS_NAME, "main", "(String)")
.when(
new ProbeCondition(
DSL.when(DSL.ge(DSL.ref("after"), DSL.value(0))), "after >= 0"))
.evaluateAt(MethodLocation.EXIT)
.build();
TestSnapshotListener listener = installProbes(probe);
Class<?> testClass = compileAndLoadClass(CLASS_NAME);
try {
Reflect.onClass(testClass).call("main", "triggerUncaughtException").get();
Assertions.fail("should not reach this code");
} catch (ReflectException ex) {
assertEquals("oops", ex.getCause().getCause().getMessage());
}
Snapshot snapshot = assertOneSnapshot(listener);
assertCaptureThrowable(
snapshot.getCaptures().getReturn(),
"CapturedSnapshot05$CustomException",
"oops",
"CapturedSnapshot05.triggerUncaughtException",
8);
assertNull(snapshot.getEvaluationErrors());
// after is 0 because the exception is thrown before the assignment and local var initialized
// at the beginning of the method by instrumentation
assertCaptureLocals(snapshot.getCaptures().getReturn(), "after", "long", "0");
} finally {
setFieldInConfig(Config.get(), "debuggerHoistLocalVarsEnabled", false);
}
Snapshot snapshot = assertOneSnapshot(listener);
assertCaptureThrowable(
snapshot.getCaptures().getReturn(),
"CapturedSnapshot05$CustomException",
"oops",
"CapturedSnapshot05.triggerUncaughtException",
8);
assertNull(snapshot.getEvaluationErrors());
// after is 0 because the exception is thrown before the assignment and local var initialized
// at the beginning of the method by instrumentation
assertCaptureLocals(snapshot.getCaptures().getReturn(), "after", "long", "0");
}

@Test
Expand Down Expand Up @@ -1858,15 +1872,20 @@ public void uncaughtExceptionCaptureLocalVars() throws IOException, URISyntaxExc
value = "datadog.trace.api.Platform#isJ9",
disabledReason = "we cannot get local variable debug info")
public void methodProbeLocalVarsLocalScopes() throws IOException, URISyntaxException {
final String CLASS_NAME = "com.datadog.debugger.CapturedSnapshot31";
LogProbe probe = createProbeAtExit(PROBE_ID, CLASS_NAME, "localScopes", "(String)");
TestSnapshotListener listener = installProbes(probe);
Class<?> testClass = compileAndLoadClass(CLASS_NAME);
int result = Reflect.onClass(testClass).call("main", "localScopes").get();
assertEquals(42, result);
Snapshot snapshot = assertOneSnapshot(listener);
assertEquals(1, snapshot.getCaptures().getReturn().getLocals().size());
assertCaptureLocals(snapshot.getCaptures().getReturn(), "@return", "int", "42");
setFieldInConfig(Config.get(), "debuggerHoistLocalVarsEnabled", true);
try {
final String CLASS_NAME = "com.datadog.debugger.CapturedSnapshot31";
LogProbe probe = createProbeAtExit(PROBE_ID, CLASS_NAME, "localScopes", "(String)");
TestSnapshotListener listener = installProbes(probe);
Class<?> testClass = compileAndLoadClass(CLASS_NAME);
int result = Reflect.onClass(testClass).call("main", "localScopes").get();
assertEquals(42, result);
Snapshot snapshot = assertOneSnapshot(listener);
assertEquals(1, snapshot.getCaptures().getReturn().getLocals().size());
assertCaptureLocals(snapshot.getCaptures().getReturn(), "@return", "int", "42");
} finally {
setFieldInConfig(Config.get(), "debuggerHoistLocalVarsEnabled", false);
}
}

@Test
Expand Down Expand Up @@ -1957,16 +1976,21 @@ public void overlappingLocalVarSlot() throws IOException, URISyntaxException {
value = "datadog.trace.api.Platform#isJ9",
disabledReason = "we cannot get local variable debug info")
public void duplicateLocalDifferentScope() throws IOException, URISyntaxException {
final String CLASS_NAME = "com.datadog.debugger.CapturedSnapshot31";
LogProbe probe =
createProbeAtExit(PROBE_ID, CLASS_NAME, "duplicateLocalDifferentScope", "(String)");
TestSnapshotListener listener = installProbes(probe);
Class<?> testClass = compileAndLoadClass(CLASS_NAME);
int result = Reflect.onClass(testClass).call("main", "duplicateLocalDifferentScope").get();
assertEquals(28, result);
Snapshot snapshot = assertOneSnapshot(listener);
assertCaptureLocals(
snapshot.getCaptures().getReturn(), "ch", Character.TYPE.getTypeName(), "e");
setFieldInConfig(Config.get(), "debuggerHoistLocalVarsEnabled", true);
try {
final String CLASS_NAME = "com.datadog.debugger.CapturedSnapshot31";
LogProbe probe =
createProbeAtExit(PROBE_ID, CLASS_NAME, "duplicateLocalDifferentScope", "(String)");
TestSnapshotListener listener = installProbes(probe);
Class<?> testClass = compileAndLoadClass(CLASS_NAME);
int result = Reflect.onClass(testClass).call("main", "duplicateLocalDifferentScope").get();
assertEquals(28, result);
Snapshot snapshot = assertOneSnapshot(listener);
assertCaptureLocals(
snapshot.getCaptures().getReturn(), "ch", Character.TYPE.getTypeName(), "e");
} finally {
setFieldInConfig(Config.get(), "debuggerHoistLocalVarsEnabled", false);
}
}

@Test
Expand Down

0 comments on commit 2413b07

Please sign in to comment.