Skip to content

Remove static inherited fields collection #8832

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jpbempel merged 1 commit into from
May 19, 2025
Merged

Remove static inherited fields collection #8832

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 4 additions & 43 deletions ...ugger/src/main/java/com/datadog/debugger/instrumentation/CapturedContextInstrumentor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package com.datadog.debugger.instrumentation;

import static com.datadog.debugger.instrumentation.ASMHelper.extractSuperClass;
import static com.datadog.debugger.instrumentation.ASMHelper.getStatic;
import static com.datadog.debugger.instrumentation.ASMHelper.invokeConstructor;
import static com.datadog.debugger.instrumentation.ASMHelper.invokeStatic;
Expand Down Expand Up @@ -1136,51 +1135,13 @@ private static List<FieldNode> extractStaticFields(
}
}
}
if (!Config.get().isDynamicInstrumentationInstrumentTheWorld()) {
// Collects inherited static fields only if the ITW mode is not enabled
// because it can lead to LinkageError: attempted duplicate class definition
// for example, when a probe is located in method overridden in enum element
addInheritedStaticFields(classNode, classLoader, limits, results, fieldCount);
}
// Collecting inherited static fields is problematic because it some cases can lead to
// LinkageError: attempted duplicate class definition
// as we force to load a class to get the static fields in a different order than the JVM
// for example, when a probe is located in method overridden in enum element
return results;
}

private static void addInheritedStaticFields(
ClassNode classNode,
ClassLoader classLoader,
Limits limits,
List<FieldNode> results,
int fieldCount) {
String superClassName = extractSuperClass(classNode);
while (!superClassName.equals(Object.class.getTypeName())) {
Class<?> clazz;
try {
clazz = Class.forName(superClassName, false, classLoader);
} catch (ClassNotFoundException ex) {
break;
}
try {
for (Field field : clazz.getDeclaredFields()) {
if (isStaticField(field) && !isFinalField(field)) {
String desc = Type.getDescriptor(field.getType());
FieldNode fieldNode =
new FieldNode(field.getModifiers(), field.getName(), desc, null, field);
results.add(fieldNode);
LOGGER.debug("Adding static inherited field {}", fieldNode.name);
fieldCount++;
if (fieldCount > limits.maxFieldCount) {
return;
}
}
}
} catch (ClassCircularityError ex) {
break;
}
clazz = clazz.getSuperclass();
superClassName = clazz.getTypeName();
}
}

private int declareContextVar(InsnList insnList) {
int var = newVar(CAPTURED_CONTEXT_TYPE);
getStatic(insnList, CAPTURED_CONTEXT_TYPE, "EMPTY_CAPTURING_CONTEXT");
Expand Down
28 changes: 2 additions & 26 deletions ...a-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturedSnapshotTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -939,27 +939,6 @@ public void fieldExtractorNotAccessible() throws IOException, URISyntaxException
"Field is not accessible: module java.base does not opens/exports to the current module");
}

@Test
@EnabledForJreRange(min = JRE.JAVA_17)
public void staticFieldExtractorNotAccessible() throws IOException, URISyntaxException {
final String CLASS_NAME = "com.datadog.debugger.CapturedSnapshot30";
LogProbe logProbe =
createMethodProbe(PROBE_ID, CLASS_NAME + "$MyHttpURLConnection", "process", "()");
TestSnapshotListener listener = installProbes(logProbe);
Class<?> testClass = compileAndLoadClass(CLASS_NAME);
int result = Reflect.onClass(testClass).call("main", "static").get();
assertEquals(42, result);
Snapshot snapshot = assertOneSnapshot(listener);
assertCaptureStaticFieldsNotCaptured(
snapshot.getCaptures().getReturn(),
"followRedirects",
"Field is not accessible: module java.base does not opens/exports to the current module");
assertCaptureStaticFieldsNotCaptured(
snapshot.getCaptures().getReturn(),
"factory",
"Field is not accessible: module java.base does not opens/exports to the current module");
}

@Test
public void uncaughtException() throws IOException, URISyntaxException {
final String CLASS_NAME = "CapturedSnapshot05";
Expand Down Expand Up @@ -1611,12 +1590,9 @@ public void staticInheritedFields() throws IOException, URISyntaxException {
Snapshot snapshot = assertOneSnapshot(listener);
Map<String, CapturedContext.CapturedValue> staticFields =
snapshot.getCaptures().getReturn().getStaticFields();
assertEquals(7, staticFields.size());
// inherited static fields are not collected
assertEquals(2, staticFields.size());
assertEquals("barfoo", MoshiSnapshotTestHelper.getValue(staticFields.get("strValue")));
assertEquals("48", MoshiSnapshotTestHelper.getValue(staticFields.get("intValue")));
assertEquals("6.28", MoshiSnapshotTestHelper.getValue(staticFields.get("doubleValue")));
assertEquals("[1, 2, 3, 4]", MoshiSnapshotTestHelper.getValue(staticFields.get("longValues")));
assertEquals("[foo, bar]", MoshiSnapshotTestHelper.getValue(staticFields.get("strValues")));
}

@Test
Expand Down
Loading