Skip to content

Commit 821ca84

Browse files
jbachorikjbachorik
committed
improve handling of super classes in instrumentation
1 parent 07f1a7c commit 821ca84

14 files changed

Lines changed: 294 additions & 391 deletions

‎src/share/classes/com/sun/btrace/agent/Client.java‎

Lines changed: 24 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@
4848
import com.sun.btrace.org.objectweb.asm.Opcodes;
4949
import com.sun.btrace.runtime.ClassFilter;
5050
import com.sun.btrace.runtime.ClassRenamer;
51-
import com.sun.btrace.runtime.ClinitInjector;
5251
import com.sun.btrace.runtime.Instrumentor;
5352
import com.sun.btrace.runtime.InstrumentUtils;
5453
import com.sun.btrace.runtime.Location;
@@ -65,9 +64,19 @@
6564
import java.lang.instrument.ClassFileTransformer;
6665
import java.lang.instrument.IllegalClassFormatException;
6766
import java.lang.instrument.Instrumentation;
67+
import java.lang.instrument.UnmodifiableClassException;
68+
import java.lang.reflect.Field;
6869
import java.util.ArrayList;
6970
import java.util.Collection;
71+
import java.util.HashSet;
72+
import java.util.Iterator;
7073
import java.util.Map;
74+
import java.util.Set;
75+
import java.util.Vector;
76+
import java.util.concurrent.Executors;
77+
import java.util.concurrent.ScheduledExecutorService;
78+
import java.util.concurrent.ThreadFactory;
79+
import java.util.concurrent.TimeUnit;
7180
import sun.reflect.annotation.AnnotationParser;
7281
import sun.reflect.annotation.AnnotationType;
7382

@@ -80,6 +89,7 @@
8089
abstract class Client implements ClassFileTransformer, CommandListener {
8190
protected final Instrumentation inst;
8291
private volatile BTraceRuntime runtime;
92+
private volatile boolean isClassRenamed = false;
8393
private volatile String className;
8494
private volatile Class btraceClazz;
8595
private volatile byte[] btraceCode;
@@ -106,36 +116,6 @@ abstract class Client implements ClassFileTransformer, CommandListener {
106116
BTraceRuntime.init(createPerfReaderImpl(), new RunnableGeneratorImpl());
107117
}
108118

109-
final private ClassFileTransformer clInitTransformer = new ClassFileTransformer() {
110-
111-
@Override
112-
public byte[] transform(ClassLoader loader, final String cname, Class<?> classBeingRedefined, ProtectionDomain protectionDomain, byte[] classfileBuffer) throws IllegalClassFormatException {
113-
if (!hasSubclassChecks || classBeingRedefined != null || isBTraceClass(cname) || isSensitiveClass(cname)) return null;
114-
115-
if (!skipRetransforms) {
116-
if (isDebug()) {
117-
Client.this.debugPrint("injecting <clinit> for " + cname); // NOI18N
118-
}
119-
ClassReader cr = new ClassReader(classfileBuffer);
120-
ClassWriter cw = new ClassWriter(cr, ClassWriter.COMPUTE_MAXS);
121-
ClinitInjector injector = new ClinitInjector(cw, className, cname);
122-
InstrumentUtils.accept(cr, injector);
123-
if (injector.isTransformed()) {
124-
byte[] instrumentedCode = cw.toByteArray();
125-
if (settings.isDumpClasses()) {
126-
debug.dumpClass(className, cname + "_clinit", instrumentedCode); // NOI18N
127-
}
128-
return instrumentedCode;
129-
}
130-
} else {
131-
if (isDebug()) {
132-
Client.this.debugPrint("client " + className + ": skipping transform for " + cname); // NOI18N
133-
}
134-
}
135-
return null;
136-
}
137-
};
138-
139119
private static PerfReader createPerfReaderImpl() {
140120
// see if we can access any jvmstat class
141121
try {
@@ -178,21 +158,19 @@ public byte[] transform(
178158
if (classBeingRedefined != null) {
179159
// class already defined; retransforming
180160
if (!skipRetransforms && filter.isCandidate(classBeingRedefined)) {
181-
return doTransform(classBeingRedefined, cname, classfileBuffer);
161+
return doTransform(loader, classBeingRedefined, cname, classfileBuffer);
182162
} else {
183163
if (isDebug()) {
184-
debugPrint("client " + className + ": skipping transform for " + cname); // NOi18N
164+
debugPrint("client " + className + "[" + skipRetransforms + "]: skipping transform for " + cname); // NOi18N
185165
}
186166
}
187167
} else {
188168
// class not yet defined
189-
if (!hasSubclassChecks) {
190-
if (filter.isCandidate(classfileBuffer)) {
191-
return doTransform(classBeingRedefined, cname, classfileBuffer);
192-
} else {
193-
if (isDebug()) {
194-
debugPrint("client " + className + ": skipping transform for " + cname); // NOI18N
195-
}
169+
if (filter.isCandidate(loader, classfileBuffer, hasSubclassChecks)) {
170+
return doTransform(loader, classBeingRedefined, cname, classfileBuffer);
171+
} else {
172+
if (isDebug()) {
173+
debugPrint("client " + className + "[" + skipRetransforms + "]: skipping transform for " + cname); // NOI18N
196174
}
197175
}
198176
}
@@ -220,16 +198,14 @@ protected final void setSettings(Map<String, Object> params) {
220198
}
221199

222200
void registerTransformer() {
223-
inst.addTransformer(clInitTransformer, false);
224201
inst.addTransformer(this, true);
225202
}
226203

227204
void unregisterTransformer() {
228205
inst.removeTransformer(this);
229-
inst.removeTransformer(clInitTransformer);
230206
}
231207

232-
private byte[] doTransform(Class<?> classBeingRedefined, String cname, byte[] classfileBuffer) {
208+
private byte[] doTransform(ClassLoader loader, Class<?> classBeingRedefined, String cname, byte[] classfileBuffer) {
233209
if (isDebug()) {
234210
debugPrint("client " + className + ": instrumenting " + cname);
235211
}
@@ -240,7 +216,7 @@ private byte[] doTransform(Class<?> classBeingRedefined, String cname, byte[] cl
240216
debugPrint(e);
241217
}
242218
}
243-
return instrument(classBeingRedefined, cname, classfileBuffer);
219+
return instrument(loader, classBeingRedefined, cname, classfileBuffer);
244220
}
245221

246222
protected synchronized void onExit(int exitCode) {
@@ -277,8 +253,7 @@ protected Class loadClass(InstrumentCommand instr) throws IOException {
277253
ClassWriter writer = InstrumentUtils.newClassWriter(btraceCode);
278254
ClassReader reader = new ClassReader(btraceCode);
279255
ClassVisitor visitor = new Preprocessor(writer);
280-
if (BTraceRuntime.classNameExists(className)) {
281-
className += "$" + getCount();
256+
if (isClassRenamed) {
282257
if (isDebug()) {
283258
debugPrint("class renamed to " + className);
284259
}
@@ -465,10 +440,10 @@ private static boolean isSensitiveClass(String name) {
465440
name.equals("java/lang/VerifyError"); // NOI18N
466441
}
467442

468-
private byte[] instrument(Class clazz, String cname, byte[] target) {
443+
private byte[] instrument(ClassLoader loader, Class clazz, String cname, byte[] target) {
469444
byte[] instrumentedCode;
470445
try {
471-
ClassWriter writer = InstrumentUtils.newClassWriter(target);
446+
ClassWriter writer = InstrumentUtils.newClassWriter(loader, target);
472447
ClassReader reader = new ClassReader(target);
473448
Instrumentor i = new Instrumentor(clazz, className, btraceCode, onMethods, writer);
474449
InstrumentUtils.accept(reader, i);
@@ -492,6 +467,7 @@ private void verify(byte[] buf) {
492467
debugPrint("verifying BTrace class");
493468
InstrumentUtils.accept(reader, verifier);
494469
className = verifier.getClassName().replace('/', '.');
470+
isClassRenamed = verifier.isClassRenamed();
495471
if (isDebug()) {
496472
debugPrint("verified '" + className + "' successfully");
497473
}
@@ -505,7 +481,6 @@ private void verify(byte[] buf) {
505481
verifySpecialParameters(om);
506482
if (om.getClazz().startsWith("+")) {
507483
hasSubclassChecks = true;
508-
break;
509484
}
510485
}
511486
}
@@ -609,9 +584,4 @@ private List<OnMethod> mapOnProbes(List<OnProbe> onProbes) {
609584
}
610585
return res;
611586
}
612-
613-
private static long count = 0L;
614-
private static long getCount() {
615-
return count++;
616-
}
617587
}

‎src/share/classes/com/sun/btrace/runtime/BTraceConfigurator.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,15 +54,17 @@ final public class BTraceConfigurator extends MethodVisitor {
5454
protected boolean asBTrace = false;
5555
protected Location loc;
5656

57+
private final String className;
5758
private final String methodName;
5859
private final String methodDesc;
5960
private final String methodId;
6061
private final CycleDetector graph;
6162
private final List<OnMethod> onMethods;
6263
private final List<OnProbe> onProbes;
6364

64-
public BTraceConfigurator(MethodVisitor mv, CycleDetector graph, List<OnMethod> onMethods, List<OnProbe> onProbes, String methodName, String desc) {
65+
public BTraceConfigurator(MethodVisitor mv, CycleDetector graph, List<OnMethod> onMethods, List<OnProbe> onProbes, String className, String methodName, String desc) {
6566
super(Opcodes.ASM5, mv);
67+
this.className = className;
6668
this.methodName = methodName;
6769
this.methodDesc = desc;
6870
this.methodId = methodName + desc;

‎src/share/classes/com/sun/btrace/runtime/ClassFilter.java‎

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
*/
2525
package com.sun.btrace.runtime;
2626

27+
import com.sun.btrace.DebugSupport;
2728
import java.lang.annotation.Annotation;
2829
import java.util.ArrayList;
2930
import java.util.List;
@@ -40,6 +41,8 @@
4041
import com.sun.btrace.annotations.BTrace;
4142
import com.sun.btrace.org.objectweb.asm.Opcodes;
4243
import java.lang.ref.Reference;
44+
import java.util.Arrays;
45+
import java.util.LinkedList;
4346
import java.util.regex.PatternSyntaxException;
4447

4548
/**
@@ -149,12 +152,12 @@ public boolean isCandidate(Class target) {
149152
return false;
150153
}
151154

152-
public boolean isCandidate(byte[] classBytes) {
153-
return isCandidate(new ClassReader(classBytes));
155+
public boolean isCandidate(ClassLoader loader, byte[] classBytes, boolean subClassChecks) {
156+
return isCandidate(loader, new ClassReader(classBytes), subClassChecks);
154157
}
155158

156-
public boolean isCandidate(ClassReader reader) {
157-
CheckingVisitor cv = new CheckingVisitor();
159+
public boolean isCandidate(ClassLoader loader, ClassReader reader, boolean subClassChecks) {
160+
CheckingVisitor cv = new CheckingVisitor(loader, subClassChecks);
158161
InstrumentUtils.accept(reader, cv);
159162
return cv.isCandidate();
160163
}
@@ -183,10 +186,14 @@ private class CheckingVisitor extends ClassVisitor {
183186

184187
private boolean isInterface;
185188
private boolean isCandidate;
189+
private final ClassLoader loader;
190+
private final boolean subClassChecks;
186191
private final AnnotationVisitor nullAnnotationVisitor = new AnnotationVisitor(Opcodes.ASM5) {};
187192

188-
public CheckingVisitor() {
193+
public CheckingVisitor(ClassLoader loader, boolean subClassChecks) {
189194
super(Opcodes.ASM5);
195+
this.loader = loader != null ? loader : ClassLoader.getSystemClassLoader();
196+
this.subClassChecks = subClassChecks;
190197
}
191198

192199
boolean isCandidate() {
@@ -201,6 +208,27 @@ public void visit(int version, int access, String name,
201208
isCandidate = false;
202209
return;
203210
}
211+
212+
if (subClassChecks) {
213+
try {
214+
List<String> toCheck = new java.util.LinkedList<>(java.util.Arrays.asList(interfaces));
215+
toCheck.add(superName);
216+
for(String cName : toCheck) {
217+
if (!cName.equals("java/lang/Object")) {
218+
// can safely use the associated classloader to access super-classes
219+
// all of them already have been loaded and potentially instrumented
220+
Class<?> checkingClass = loader.loadClass(cName.replace("/", "."));
221+
if (ClassFilter.this.isCandidate(checkingClass)) {
222+
isCandidate = true;
223+
return;
224+
}
225+
}
226+
}
227+
} catch (ClassNotFoundException e) {
228+
DebugSupport.warning(e);
229+
}
230+
}
231+
204232
name = name.replace('/', '.');
205233

206234
if (referenceClz.getName().equals(name)) {

0 commit comments

Comments
 (0)