Skip to content

Commit 1f630ff

Browse files
committed
fix: More robust handling of LVT when resolving parameters
A user has reported (#293) a crash in LVT handling. While I couldn't reproduce the crash exactly, I have looked at the relevant code and made some improvements that will hopefully resolve the problem: - Ensure correct handling of local variable table that's not in slot order. While it usually is the case it's not guaranteed. - Ensure graceful fallback in case LVT inspection throws; this can happen eg. when the debug info is corrupted (maybe intentionally by an obfuscator). In this case continue with the next argument instead of crashing out.
1 parent d71b1a1 commit 1f630ff

File tree

1 file changed

+25
-32
lines changed

1 file changed

+25
-32
lines changed

agent/src/main/java/com/appland/appmap/output/v1/Parameters.java

Lines changed: 25 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -79,47 +79,37 @@ public Parameters(CtBehavior behavior) {
7979
logger.debug("No code attribute for {}", fqn);
8080
}
8181

82-
String[] paramNames = null;
8382
int numParams = paramTypes.length;
83+
String[] paramNames = new String[numParams];
8484
if (locals != null && numParams > 0) {
8585
int numLocals = locals.tableLength();
86-
86+
8787
// This is handy when debugging this code, but produces too much
8888
// noise for general use.
8989
if (Properties.DebugLocals) {
90-
Logger.println("local variables for " + fqn);
90+
logger.debug("local variables for {}", fqn);
9191
for (int idx = 0; idx < numLocals; idx++) {
92-
Logger.printf(" %d %s %d\n", idx, locals.variableName(idx), locals.index(idx));
93-
}
94-
}
95-
96-
paramNames = new String[numParams];
97-
Boolean isStatic = (behavior.getModifiers() & Modifier.STATIC) != 0;
98-
int firstParamIdx = isStatic ? 0 : 1; // ignore `this`
99-
int localVarIdx = 0;
100-
101-
// Scan the local variables until we find the one with an index
102-
// that matches the first parameter index.
103-
//
104-
// In some cases, though, there aren't local variables for the
105-
// parameters. For example, the class file for
106-
// org.springframework.samples.petclinic.owner.PetTypeFormatter,
107-
// has the method
108-
// print(Ljava/lang/Object;Ljava/util/Locale;)Ljava/lang/String
109-
// in it, which only has a local variable for `this`. This
110-
// method isn't in the source file, so I'm not sure where it's
111-
// coming from.
112-
for (; localVarIdx < numLocals; localVarIdx++) {
113-
if (locals.index(localVarIdx) == firstParamIdx) {
114-
break;
92+
logger.debug(" {} {} {}", idx, locals.variableName(idx), locals.index(idx));
11593
}
11694
}
11795

118-
if (localVarIdx < numLocals) {
119-
// Assume the rest of the parameters follow the first.
120-
paramNames[0] = locals.variableName(localVarIdx);
121-
for (int idx = 1; idx < numParams; idx++) {
122-
paramNames[idx] = locals.variableName(localVarIdx + idx);
96+
// Iterate through the local variables to find the ones that match the argument slots.
97+
// Arguments are pushed into consecutive slots, starting at 0 (for this or the first argument),
98+
// and then incrementing by 1 for each argument, unless the argument is an unboxed long or double,
99+
// in which case it takes up two slots.
100+
int slot = Modifier.isStatic(behavior.getModifiers()) ? 0 : 1; // ignore `this`
101+
for (int i = 0; i < numParams; i++) {
102+
try {
103+
// note that the slot index is not the same as the
104+
// parameter index or the local variable index
105+
paramNames[i] = locals.variableNameByIndex(slot);
106+
} catch (Exception e) {
107+
// the debug info might be corrupted or partial, let's not crash in this case
108+
logger.debug(e, "Failed to get local variable name for slot {} in {}", slot, fqn);
109+
} finally {
110+
// note these only correspond to unboxed types — boxed double and long will still have width 1
111+
int width = paramTypes[i] == CtClass.doubleType || paramTypes[i] == CtClass.longType ? 2 : 1;
112+
slot += width;
123113
}
124114
}
125115
}
@@ -128,7 +118,10 @@ public Parameters(CtBehavior behavior) {
128118
for (int i = 0; i < paramTypes.length; ++i) {
129119
// Use a real parameter name if we have it, a fake one if we
130120
// don't.
131-
String paramName = paramNames != null ? paramNames[i] : "p" + i;
121+
String paramName = paramNames[i];
122+
if (paramName == null) {
123+
paramName = "p" + i;
124+
}
132125
Value param = new Value()
133126
.setClassType(paramTypes[i].getName())
134127
.setName(paramName)

0 commit comments

Comments
 (0)