Skip to content

Commit 74e42f1

Browse files
jpbempelsethsamuel
authored andcommitted
Fix mixed local vars for suspend funs in Kotlin (#7748)
Recent Kotlin compiler generates multiple range for same local vars and mess up our algorithm for adjustment. Fix this by made a list of unique local vars
1 parent 62b5838 commit 74e42f1

File tree

4 files changed

+92
-4
lines changed

4 files changed

+92
-4
lines changed

dd-java-agent/agent-debugger/build.gradle

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@ dependencies {
5252
testImplementation group: 'org.springframework.boot', name: 'spring-boot-starter-web', version: '2.3.5.RELEASE'
5353
testImplementation group: 'org.freemarker', name: 'freemarker', version: '2.3.30'
5454
testImplementation group: 'org.jooq', name: 'joor-java-8', version: '0.9.13'
55-
testImplementation group: 'org.jetbrains.kotlin', name: 'kotlin-compiler-embeddable', version: "1.5.21"
55+
testImplementation group: 'org.jetbrains.kotlin', name: 'kotlin-compiler-embeddable', version: "1.9.25"
56+
testImplementation group: 'org.jetbrains.kotlinx', name: 'kotlinx-coroutines-core', version: "1.0.0"
5657
testImplementation project(':dd-trace-core')
5758
testImplementation project(':dd-java-agent:agent-builder')
5859
testImplementation project(':remote-config:remote-config-core')

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

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -244,21 +244,35 @@ public static void adjustLocalVarsBasedOnArgs(
244244
// https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-2.html#jvms-2.6.1
245245
// so we reassigned local var in arg slots if they are empty
246246
if (argTypes.length < localVars.length) {
247+
List<LocalVariableNode> uniqueSortedLocalVars = dedupLocalVars(sortedLocalVars);
247248
int slot = isStatic ? 0 : 1;
248249
int localVarTableIdx = slot;
249250
for (org.objectweb.asm.Type t : argTypes) {
250251
if (slot >= localVars.length) {
251252
break;
252253
}
253-
if (localVars[slot] == null && localVarTableIdx < sortedLocalVars.size()) {
254-
localVars[slot] = sortedLocalVars.get(localVarTableIdx);
254+
if (localVars[slot] == null && localVarTableIdx < uniqueSortedLocalVars.size()) {
255+
localVars[slot] = uniqueSortedLocalVars.get(localVarTableIdx);
255256
}
256257
slot += t.getSize();
257258
localVarTableIdx++;
258259
}
259260
}
260261
}
261262

263+
private static List<LocalVariableNode> dedupLocalVars(List<LocalVariableNode> sortedLocalVars) {
264+
List<LocalVariableNode> uniqueSortedLocalVars = new ArrayList<>();
265+
int maxIndex = sortedLocalVars.get(sortedLocalVars.size() - 1).index;
266+
boolean[] usedIndexes = new boolean[maxIndex + 1];
267+
for (LocalVariableNode localVariableNode : sortedLocalVars) {
268+
if (!usedIndexes[localVariableNode.index]) {
269+
uniqueSortedLocalVars.add(localVariableNode);
270+
usedIndexes[localVariableNode.index] = true;
271+
}
272+
}
273+
return uniqueSortedLocalVars;
274+
}
275+
262276
public static void newInstance(InsnList insnList, org.objectweb.asm.Type type) {
263277
insnList.add(new TypeInsnNode(Opcodes.NEW, type.getInternalName()));
264278
}

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

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -590,8 +590,9 @@ public void sourceFileProbeKotlin() {
590590
URL resource = CapturedSnapshotTest.class.getResource("/" + CLASS_NAME + ".kt");
591591
assertNotNull(resource);
592592
List<File> filesToDelete = new ArrayList<>();
593-
Class<?> testClass = KotlinHelper.compileAndLoad(CLASS_NAME, resource.getFile(), filesToDelete);
594593
try {
594+
Class<?> testClass =
595+
KotlinHelper.compileAndLoad(CLASS_NAME, resource.getFile(), filesToDelete);
595596
Object companion = Reflect.onClass(testClass).get("Companion");
596597
int result = Reflect.on(companion).call("main", "").get();
597598
assertEquals(48, result);
@@ -607,6 +608,29 @@ public void sourceFileProbeKotlin() {
607608
}
608609
}
609610

611+
@Test
612+
public void suspendKotlin() {
613+
final String CLASS_NAME = "CapturedSnapshot302";
614+
TestSnapshotListener listener =
615+
installProbes(CLASS_NAME, createSourceFileProbe(PROBE_ID, CLASS_NAME + ".kt", 9));
616+
URL resource = CapturedSnapshotTest.class.getResource("/" + CLASS_NAME + ".kt");
617+
assertNotNull(resource);
618+
List<File> filesToDelete = new ArrayList<>();
619+
try {
620+
Class<?> testClass =
621+
KotlinHelper.compileAndLoad(CLASS_NAME, resource.getFile(), filesToDelete);
622+
Object companion = Reflect.onClass(testClass).get("Companion");
623+
int result = Reflect.on(companion).call("main", "").get();
624+
assertEquals(0, result);
625+
Snapshot snapshot = assertOneSnapshot(listener);
626+
assertCaptureFields(snapshot.getCaptures().getLines().get(9), "intField", "int", "42");
627+
assertCaptureFields(
628+
snapshot.getCaptures().getLines().get(9), "strField", String.class.getTypeName(), "foo");
629+
} finally {
630+
filesToDelete.forEach(File::delete);
631+
}
632+
}
633+
610634
@Test
611635
public void fieldExtractor() throws IOException, URISyntaxException {
612636
final String CLASS_NAME = "CapturedSnapshot04";
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import kotlinx.coroutines.*
2+
3+
class CapturedSnapshot302 {
4+
var intField = 42
5+
var strField = "foo"
6+
val list: MutableList<Int> = mutableListOf()
7+
8+
suspend fun process(arg: String): Int {
9+
println(intField)
10+
try {
11+
list.add(1)
12+
val content = download(arg)
13+
list.add(2)
14+
subprocess() {
15+
println(arg)
16+
println(list)
17+
delay(1L)
18+
intField = 43
19+
strField = "bar"
20+
}
21+
println(strField)
22+
println(content)
23+
} catch (e: Exception) {
24+
println(list)
25+
println(e)
26+
}
27+
return arg.length
28+
}
29+
30+
suspend fun download(arg: String): String {
31+
delay(10L)
32+
return arg
33+
}
34+
35+
suspend fun subprocess(f: suspend () -> Unit): Unit {
36+
println(intField)
37+
f()
38+
println(strField)
39+
}
40+
41+
42+
43+
companion object {
44+
fun main(arg: String): Int = runBlocking {
45+
val c = CapturedSnapshot302()
46+
c.process(arg)
47+
}
48+
}
49+
}

0 commit comments

Comments
 (0)