Skip to content

Commit

Permalink
Remove virtual field interfaces from reflection results (#4722)
Browse files Browse the repository at this point in the history
* Remove virtual field interfaces from reflection results

* fix java8 and openj9
  • Loading branch information
laurit authored Nov 29, 2021
1 parent 5c36ed3 commit 92f83f1
Show file tree
Hide file tree
Showing 9 changed files with 164 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,5 @@ plugins {

dependencies {
testInstrumentation(project(":instrumentation:internal:internal-reflection:javaagent"))
testCompileOnly(project(":javaagent-bootstrap"))
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
*/

import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification

import io.opentelemetry.javaagent.bootstrap.VirtualFieldAccessorMarker
import io.opentelemetry.javaagent.bootstrap.VirtualFieldInstalledMarker
import java.lang.reflect.Field
import java.lang.reflect.Method

Expand Down Expand Up @@ -37,16 +38,20 @@ class ReflectionTest extends AgentInstrumentationSpecification {
methodFound == false

and:
def interfaceClass = TestClass.getInterfaces().find {
it.getName().contains("VirtualFieldAccessor\$")
}
interfaceClass != null
def interfaceMethodFound = false
for (Method method : interfaceClass.getDeclaredMethods()) {
if (method.getName().contains("__opentelemetry")) {
interfaceMethodFound = true
}
}
interfaceMethodFound == false
// although marker interfaces are removed from getInterfaces() result class is still assignable
// to them
VirtualFieldInstalledMarker.isAssignableFrom(TestClass)
VirtualFieldAccessorMarker.isAssignableFrom(TestClass)
TestClass.getInterfaces().length == 2
TestClass.getInterfaces() == [Runnable, Serializable]
}

def "test generated serialVersionUID"() {
// expected value is computed with serialver utility that comes with jdk
expect:
ObjectStreamClass.lookup(TestClass).getSerialVersionUID() == -1508684692096503670L

and:
TestClass.getDeclaredFields().length == 0
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
* SPDX-License-Identifier: Apache-2.0
*/

public class TestClass implements Runnable {
import java.io.Serializable;

public class TestClass implements Runnable, Serializable {

@Override
public void run() {}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.instrumentation.internal.reflection;

import static net.bytebuddy.matcher.ElementMatchers.named;

import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
import net.bytebuddy.asm.AsmVisitorWrapper;
import net.bytebuddy.description.field.FieldDescription;
import net.bytebuddy.description.field.FieldList;
import net.bytebuddy.description.method.MethodList;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.implementation.Implementation;
import net.bytebuddy.matcher.ElementMatcher;
import net.bytebuddy.pool.TypePool;
import org.objectweb.asm.ClassVisitor;
import org.objectweb.asm.ClassWriter;
import org.objectweb.asm.MethodVisitor;
import org.objectweb.asm.Opcodes;
import org.objectweb.asm.Type;

public class ClassInstrumentation implements TypeInstrumentation {
@Override
public ElementMatcher<TypeDescription> typeMatcher() {
return named("java.lang.Class");
}

@Override
public void transform(TypeTransformer transformer) {
transformer.applyTransformer(
(builder, typeDescription, classLoader, module) ->
builder.visit(
new AsmVisitorWrapper() {
@Override
public int mergeWriter(int flags) {
return flags | ClassWriter.COMPUTE_MAXS;
}

@Override
public int mergeReader(int flags) {
return flags;
}

@Override
public ClassVisitor wrap(
TypeDescription instrumentedType,
ClassVisitor classVisitor,
Implementation.Context implementationContext,
TypePool typePool,
FieldList<FieldDescription.InDefinedShape> fields,
MethodList<?> methods,
int writerFlags,
int readerFlags) {
return new ClassClassVisitor(classVisitor);
}
}));
}

private static class ClassClassVisitor extends ClassVisitor {

ClassClassVisitor(ClassVisitor cv) {
super(Opcodes.ASM7, cv);
}

@Override
public MethodVisitor visitMethod(
int access, String name, String descriptor, String signature, String[] exceptions) {
MethodVisitor mv = super.visitMethod(access, name, descriptor, signature, exceptions);
if ("getInterfaces".equals(name)
&& ("()[Ljava/lang/Class;".equals(descriptor)
|| "(Z)[Ljava/lang/Class;".equals(descriptor))) {
mv =
new MethodVisitor(api, mv) {
@Override
public void visitMethodInsn(
int opcode, String owner, String name, String descriptor, boolean isInterface) {
super.visitMethodInsn(opcode, owner, name, descriptor, isInterface);
// filter the result of call to getInterfaces0, which is used on hotspot, and
// J9VMInternals.getInterfaces which is used on openj9
if (((opcode == Opcodes.INVOKEVIRTUAL || opcode == Opcodes.INVOKESPECIAL)
&& "getInterfaces0".equals(name)
&& "()[Ljava/lang/Class;".equals(descriptor))
|| (opcode == Opcodes.INVOKESTATIC
&& "getInterfaces".equals(name)
&& "java/lang/J9VMInternals".equals(owner)
&& "(Ljava/lang/Class;)[Ljava/lang/Class;".equals(descriptor))) {
mv.visitVarInsn(Opcodes.ALOAD, 0);
mv.visitMethodInsn(
Opcodes.INVOKESTATIC,
Type.getInternalName(ReflectionHelper.class),
"filterInterfaces",
"([Ljava/lang/Class;Ljava/lang/Class;)[Ljava/lang/Class;",
false);
}
}
};
}
return mv;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

package io.opentelemetry.javaagent.instrumentation.internal.reflection;

import io.opentelemetry.javaagent.bootstrap.VirtualFieldAccessorMarker;
import io.opentelemetry.javaagent.bootstrap.VirtualFieldInstalledMarker;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
Expand All @@ -18,6 +19,7 @@ private ReflectionHelper() {}
public static Field[] filterFields(Class<?> containingClass, Field[] fields) {
if (fields.length == 0
|| !VirtualFieldInstalledMarker.class.isAssignableFrom(containingClass)) {
// nothing to filter when class does not have any added virtual fields
return fields;
}
List<Field> result = new ArrayList<>(fields.length);
Expand All @@ -36,6 +38,7 @@ public static Method[] filterMethods(Class<?> containingClass, Method[] methods)
return methods;
} else if (containingClass.isInterface()
&& containingClass.isSynthetic()
&& VirtualFieldAccessorMarker.class.isAssignableFrom(containingClass)
&& containingClass.getName().contains("VirtualFieldAccessor$")) {
// hide all methods from virtual field accessor interfaces
return new Method[0];
Expand All @@ -55,4 +58,25 @@ public static Method[] filterMethods(Class<?> containingClass, Method[] methods)
}
return result.toArray(new Method[0]);
}

@SuppressWarnings("unused")
public static Class<?>[] filterInterfaces(Class<?>[] interfaces, Class<?> containingClass) {
if (interfaces.length == 0
|| !VirtualFieldInstalledMarker.class.isAssignableFrom(containingClass)) {
// nothing to filter when class does not have any added virtual fields
return interfaces;
}
List<Class<?>> result = new ArrayList<>(interfaces.length);
for (Class<?> interfaceClass : interfaces) {
// filter out virtual field marker and accessor interfaces
if (interfaceClass == VirtualFieldInstalledMarker.class
|| (VirtualFieldAccessorMarker.class.isAssignableFrom(interfaceClass)
&& interfaceClass.isSynthetic()
&& interfaceClass.getName().contains("VirtualFieldAccessor$"))) {
continue;
}
result.add(interfaceClass);
}
return result.toArray(new Class<?>[0]);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,6 @@ public class ReflectionIgnoredTypesConfigurer implements IgnoredTypesConfigurer
public void configure(Config config, IgnoredTypesBuilder builder) {
builder.allowClass("jdk.internal.reflect.Reflection");
builder.allowClass("sun.reflect.Reflection");
builder.allowClass("java.lang.Class");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

package io.opentelemetry.javaagent.instrumentation.internal.reflection;

import static java.util.Collections.singletonList;
import static java.util.Arrays.asList;

import com.google.auto.service.AutoService;
import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
Expand All @@ -26,6 +26,6 @@ public boolean defaultEnabled() {

@Override
public List<TypeInstrumentation> typeInstrumentations() {
return singletonList(new ReflectionInstrumentation());
return asList(new ClassInstrumentation(), new ReflectionInstrumentation());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.bootstrap;

/** A marker interface implemented by virtual field accessor classes. */
public interface VirtualFieldAccessorMarker {}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import static io.opentelemetry.javaagent.tooling.field.GeneratedVirtualFieldNames.getRealGetterName;
import static io.opentelemetry.javaagent.tooling.field.GeneratedVirtualFieldNames.getRealSetterName;

import io.opentelemetry.javaagent.bootstrap.VirtualFieldAccessorMarker;
import io.opentelemetry.javaagent.tooling.muzzle.VirtualFieldMappings;
import java.util.HashMap;
import java.util.Map;
Expand Down Expand Up @@ -54,6 +55,7 @@ private DynamicType.Unloaded<?> makeFieldAccessorInterface(
.makeInterface()
.merge(SyntheticState.SYNTHETIC)
.name(getFieldAccessorInterfaceName(typeName, fieldTypeName))
.implement(VirtualFieldAccessorMarker.class)
.defineMethod(
getRealGetterName(typeName, fieldTypeName),
fieldTypeDesc,
Expand Down

0 comments on commit 92f83f1

Please sign in to comment.