Skip to content

Commit

Permalink
Consider offset, not index, when adjusting local variable table durin…
Browse files Browse the repository at this point in the history
…g advice.
  • Loading branch information
raphw committed Mar 12, 2023
1 parent b19eaba commit 6963886
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 41 deletions.
45 changes: 8 additions & 37 deletions byte-buddy-dep/src/main/java/net/bytebuddy/asm/Advice.java
Original file line number Diff line number Diff line change
Expand Up @@ -4924,14 +4924,6 @@ public interface ArgumentHandler {
*/
interface ForInstrumentedMethod extends ArgumentHandler {

/**
* Resolves a local variable index.
*
* @param index The index to resolve.
* @return The resolved local variable index.
*/
int variable(int index);

/**
* Prepares this argument handler for future offset access.
*
Expand Down Expand Up @@ -5120,15 +5112,6 @@ public int argument(int offset) {
: offset + exitType.getStackSize().getSize() + StackSize.of(namedTypes.values()) + enterType.getStackSize().getSize();
}

/**
* {@inheritDoc}
*/
public int variable(int index) {
return index < (instrumentedMethod.isStatic() ? 0 : 1) + instrumentedMethod.getParameters().size()
? index
: index + (exitType.represents(void.class) ? 0 : 1) + namedTypes.size() + (enterType.represents(void.class) ? 0 : 1);
}

/**
* {@inheritDoc}
*/
Expand Down Expand Up @@ -5176,18 +5159,6 @@ public int argument(int offset) {
+ offset;
}

/**
* {@inheritDoc}
*/
public int variable(int index) {
return (instrumentedMethod.isStatic() ? 0 : 1)
+ instrumentedMethod.getParameters().size()
+ (exitType.represents(void.class) ? 0 : 1)
+ namedTypes.size()
+ (enterType.represents(void.class) ? 0 : 1)
+ index;
}

/**
* {@inheritDoc}
*/
Expand Down Expand Up @@ -11415,24 +11386,24 @@ public void visitMaxs(int stackSize, int localVariableLength) {
}

@Override
public void visitLocalVariable(String name, String descriptor, String signature, Label start, Label end, int index) {
public void visitLocalVariable(String name, String descriptor, String signature, Label start, Label end, int offset) {
// The 'this' variable is exempt from remapping as it is assumed immutable and remapping it confuses debuggers to not display the variable.
mv.visitLocalVariable(name, descriptor, signature, start, end, index == THIS_VARIABLE_INDEX && THIS_VARIABLE_NAME.equals(name)
? index
: argumentHandler.variable(index));
mv.visitLocalVariable(name, descriptor, signature, start, end, offset == THIS_VARIABLE_INDEX && THIS_VARIABLE_NAME.equals(name)
? offset
: argumentHandler.argument(offset));
}

@Override
public AnnotationVisitor visitLocalVariableAnnotation(int typeReference,
TypePath typePath,
Label[] start,
Label[] end,
int[] index,
int[] offset,
String descriptor,
boolean visible) {
int[] translated = new int[index.length];
for (int anIndex = 0; anIndex < index.length; anIndex++) {
translated[anIndex] = argumentHandler.variable(index[anIndex]);
int[] translated = new int[offset.length];
for (int index = 0; index < offset.length; index++) {
translated[index] = argumentHandler.argument(offset[index]);
}
return mv.visitLocalVariableAnnotation(typeReference, typePath, start, end, translated, descriptor, visible);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -559,11 +559,11 @@ public AnnotationVisitor visitLocalVariableAnnotation(int typeReference,
@MaybeNull TypePath typePath,
Label[] start,
Label[] end,
int[] index,
int[] offset,
String descriptor,
boolean visible) {
observedTypes.add(Type.getType(descriptor).getInternalName());
AnnotationVisitor annotationVisitor = super.visitLocalVariableAnnotation(typeReference, typePath, start, end, index, descriptor, visible);
AnnotationVisitor annotationVisitor = super.visitLocalVariableAnnotation(typeReference, typePath, start, end, offset, descriptor, visible);
if (annotationVisitor != null) {
return new TypeReferenceAnnotationVisitor(annotationVisitor);
} else {
Expand Down
4 changes: 2 additions & 2 deletions byte-buddy-dep/src/main/java/net/bytebuddy/pool/TypePool.java
Original file line number Diff line number Diff line change
Expand Up @@ -8769,9 +8769,9 @@ public void visitLabel(Label label) {
}

@Override
public void visitLocalVariable(String name, String descriptor, String signature, Label start, Label end, int index) {
public void visitLocalVariable(String name, String descriptor, String signature, Label start, Label end, int offset) {
if (readerMode.isExtended() && start == firstLabel) {
legacyParameterBag.register(index, name);
legacyParameterBag.register(offset, name);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
package net.bytebuddy.asm;

import net.bytebuddy.ByteBuddy;
import net.bytebuddy.dynamic.loading.ClassLoadingStrategy;
import org.hamcrest.CoreMatchers;
import org.junit.Test;

import java.lang.reflect.InvocationTargetException;

import static net.bytebuddy.matcher.ElementMatchers.named;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.fail;

public class AdviceLocalVariableArrayTest {

@Test
public void testDebuggingSymbols() throws Exception {
Class<?> type = new ByteBuddy()
.redefine(Foo.class)
.visit(Advice.to(ShiftVariablesAdvice.class).on(named("method")))
.make()
.load(ClassLoadingStrategy.BOOTSTRAP_LOADER, ClassLoadingStrategy.Default.WRAPPER_PERSISTENT)
.getLoaded();
try {
type.getMethod("method", long.class, Void.class, Void.class).invoke(type.getConstructor().newInstance(),
0L,
null,
null);
fail();
} catch (InvocationTargetException exception) {
assertThat(exception.getTargetException(), instanceOf(NullPointerException.class));
String message = exception.getTargetException().getMessage();
if (message != null) {
assertThat(message, containsString("\"b\""));
}
}
}

public static class Foo {

@SuppressWarnings("unused")
public void method(long a, Void b, Void c) {
b.hashCode();
}
}

public static class ShiftVariablesAdvice {

static void enter(@Advice.Argument(1) Void ignored) {
/* empty */
}

static void exit(@Advice.Argument(1) Void ignored) {
/* empty */
}
}
}

0 comments on commit 6963886

Please sign in to comment.