Fixes #140 - Native method tracing breaks instrumentation for many Ja…

…va core classes
btraceio · Jul 10, 2015 · f82489b · f82489b
1 parent 2231a62
commit f82489b
Show file tree

Hide file tree

Showing 7 changed files with 4 additions and 135 deletions.
diff --git a/src/share/classes/META-INF/agent-manifest.mf b/src/share/classes/META-INF/agent-manifest.mf
@@ -3,4 +3,3 @@ Agent-Class: com.sun.btrace.agent.Main
 Boot-Class-Path: btrace-boot.jar
 Can-Redefine-Classes: true
 Can-Retransform-Classes: true
-Can-Set-Native-Method-Prefix: true
diff --git a/src/share/classes/com/sun/btrace/agent/Client.java b/src/share/classes/com/sun/btrace/agent/Client.java
@@ -196,7 +196,6 @@ public byte[] transform(
     void registerTransformer() {
         inst.addTransformer(clInitTransformer, false);
         inst.addTransformer(this, true);
-        inst.setNativeMethodPrefix(this, Constants.BTRACE_NATIVE_PREFIX);
     }
 
     void unregisterTransformer() {

diff --git a/src/share/classes/com/sun/btrace/client/Client.java b/src/share/classes/com/sun/btrace/client/Client.java
@@ -473,12 +473,14 @@ private void commandLoop(CommandListener listener)
                 }
                 listener.onCommand(cmd);
                 if (cmd.getType() == Command.EXIT) {
+                    debugPrint("received EXIT cmd");
                     return;
                 }
             } catch (IOException e) {
                 if (exited.compareAndSet(false, true)) listener.onCommand(new ExitCommand(-1));
                 throw e;
             } catch (NullPointerException e) {
+                e.printStackTrace();
                 if (exited.compareAndSet(false, true))listener.onCommand(new ExitCommand(-1));
             }
         }

diff --git a/src/share/classes/com/sun/btrace/client/Main.java b/src/share/classes/com/sun/btrace/client/Main.java
@@ -202,6 +202,7 @@ public void onCommand(Command cmd) throws IOException {
                     out.flush();
                 } else if (type == Command.EXIT) {
                     exiting = true;
+                    out.flush();
                     ExitCommand ecmd = (ExitCommand)cmd;
                     System.exit(ecmd.getExitCode());
                 } else if (type == Command.ERROR) {

diff --git a/src/share/classes/com/sun/btrace/runtime/Constants.java b/src/share/classes/com/sun/btrace/runtime/Constants.java
@@ -48,9 +48,6 @@ public abstract class Constants {
     public static final String BTRACE_METHOD_PREFIX =
         "$btrace$";
 
-    public static final String BTRACE_NATIVE_PREFIX =
-        "$btrace$native$";
-
     public static final String JAVA_LANG_OBJECT =
         Type.getInternalName(Object.class);
     public static final String JAVA_LANG_THROWABLE =

diff --git a/src/share/classes/com/sun/btrace/runtime/Instrumentor.java b/src/share/classes/com/sun/btrace/runtime/Instrumentor.java
@@ -169,11 +169,6 @@ public AnnotationVisitor visitAnnotation(String desc, boolean visible) {
     public MethodVisitor visitMethod(int access, final String name,
         final String desc, String signature, String[] exceptions) {
 
-        if (name.startsWith(Constants.BTRACE_NATIVE_PREFIX)) {
-            // don't process the already prefixed native methods
-            return super.visitMethod(access, name, desc, signature, exceptions);
-        }
-
         List<OnMethod> appliedOnMethods = new LinkedList<>();
 
         if (applicableOnMethods.isEmpty() ||
@@ -227,41 +222,7 @@ public MethodVisitor visitMethod(int access, final String name,
 
         MethodVisitor methodVisitor;
 
-        if ((access & ACC_NATIVE) != 0) {
-            // the instrumented method will become non-native and delegate to the original native one
-            // the native method will be generated with an appropriate prefix
-            String nativeMethod = Constants.BTRACE_NATIVE_PREFIX + name;
-            Type retType = Type.getReturnType(desc);
-            // generate the non-native wrapper method
-            MethodVisitor newMv = visitMethod(access & (~ACC_NATIVE), name, desc, signature, exceptions);
-            newMv.visitCode();
-            int argIdx = 0;
-            boolean isStatic = (access & ACC_STATIC) != 0;
-            // forward to the prefixed native method
-            if (!isStatic) {
-                newMv.visitIntInsn(Opcodes.ALOAD, 0); // load 'this'
-                argIdx++;
-            }
-            Type[] args = Type.getArgumentTypes(desc);
-            for(Type aType : args) {
-                newMv.visitVarInsn(aType.getOpcode(Opcodes.ILOAD), argIdx);
-                argIdx += aType.getSize();
-            }
-            newMv.visitMethodInsn(
-                isStatic ? Opcodes.INVOKESTATIC : Opcodes.INVOKESPECIAL,
-                className,
-                nativeMethod,
-                desc, false
-            );
-
-            newMv.visitInsn(retType.getOpcode(Opcodes.IRETURN));
-            newMv.visitMaxs(0, 0);
-            newMv.visitEnd();
-            // and now proceed to generate the prefixed native method
-            return super.visitMethod(access, nativeMethod, desc, signature, exceptions);
-        } else {
-            methodVisitor = super.visitMethod(access, name, desc, signature, exceptions);
-        }
+        methodVisitor = super.visitMethod(access, name, desc, signature, exceptions);
 
         LocalVariableHelperImpl lvs = new LocalVariableHelperImpl(methodVisitor, access, desc);
 

diff --git a/src/test/com/sun/btrace/runtime/InstrumentorTest.java b/src/test/com/sun/btrace/runtime/InstrumentorTest.java
@@ -1692,94 +1692,4 @@ public void onTimerTest() throws Exception {
             "RETURN"
         );
     }
-
-    @Test
-    public void nativeWithReturnTracingTest() throws Exception {
-        loadTargetClass("OnMethodTest");
-        transform("onmethod/NativeWithReturn", false);
-
-        checkTransformation(
-            "// access flags 0x1\n" +
-            "public nativeWithReturn(ILjava/lang/String;[J[Ljava/lang/Object;)J\n" +
-            "ALOAD 0\n" +
-            "INVOKESTATIC resources/OnMethodTest.$btrace$traces$onmethod$NativeWithReturn$nMethod (Ljava/lang/Object;)V\n" +
-            "ALOAD 0\n" +
-            "ILOAD 1\n" +
-            "ALOAD 2\n" +
-            "ALOAD 3\n" +
-            "ALOAD 4\n" +
-            "INVOKESPECIAL resources/OnMethodTest.$btrace$native$nativeWithReturn (ILjava/lang/String;[J[Ljava/lang/Object;)J\n" +
-            "LRETURN\n" +
-            "MAXSTACK = 5\n" +
-            "MAXLOCALS = 5\n" +
-            "\n" +
-            "public native $btrace$native$nativeWithReturn(ILjava/lang/String;[J[Ljava/lang/Object;)J\n" +
-            "\n" +
-            "// access flags 0xA\n" +
-            "private static $btrace$traces$onmethod$NativeWithReturn$nMethod(Ljava/lang/Object;)V\n" +
-            "@Lcom/sun/btrace/annotations/OnMethod;(clazz=\"/.*\\\\.OnMethodTest/\", method=\"nativeWithReturn\")\n" +
-            "@Lcom/sun/btrace/annotations/Self;() // parameter 0\n" +
-            "TRYCATCHBLOCK L0 L1 L1 java/lang/Throwable\n" +
-            "GETSTATIC traces/onmethod/NativeWithReturn.runtime : Lcom/sun/btrace/BTraceRuntime;\n" +
-            "INVOKESTATIC com/sun/btrace/BTraceRuntime.enter (Lcom/sun/btrace/BTraceRuntime;)Z\n" +
-            "IFNE L0\n" +
-            "RETURN\n" +
-            "L0\n" +
-            "LDC \"args\"\n" +
-            "INVOKESTATIC com/sun/btrace/BTraceUtils.println (Ljava/lang/Object;)V\n" +
-            "INVOKESTATIC com/sun/btrace/BTraceRuntime.leave ()V\n" +
-            "RETURN\n" +
-            "L1\n" +
-            "INVOKESTATIC com/sun/btrace/BTraceRuntime.handleException (Ljava/lang/Throwable;)V\n" +
-            "INVOKESTATIC com/sun/btrace/BTraceRuntime.leave ()V\n" +
-            "RETURN\n" +
-            "MAXSTACK = 1\n" +
-            "MAXLOCALS = 1"
-        );
-    }
-
-    @Test
-    public void nativeWithoutReturnTracingTest() throws Exception {
-        loadTargetClass("OnMethodTest");
-        transform("onmethod/NativeWithoutReturn", false);
-
-        checkTransformation(
-            "// access flags 0x1\n" +
-            "public nativeWithoutReturn(ILjava/lang/String;[J[Ljava/lang/Object;)V\n" +
-            "ALOAD 0\n" +
-            "INVOKESTATIC resources/OnMethodTest.$btrace$traces$onmethod$NativeWithoutReturn$nMethod (Ljava/lang/Object;)V\n" +
-            "ALOAD 0\n" +
-            "ILOAD 1\n" +
-            "ALOAD 2\n" +
-            "ALOAD 3\n" +
-            "ALOAD 4\n" +
-            "INVOKESPECIAL resources/OnMethodTest.$btrace$native$nativeWithoutReturn (ILjava/lang/String;[J[Ljava/lang/Object;)V\n" +
-            "RETURN\n" +
-            "MAXSTACK = 5\n" +
-            "MAXLOCALS = 5\n" +
-            "\n" +
-            "public native $btrace$native$nativeWithoutReturn(ILjava/lang/String;[J[Ljava/lang/Object;)V\n" +
-            "\n" +
-            "// access flags 0xA\n" +
-            "private static $btrace$traces$onmethod$NativeWithoutReturn$nMethod(Ljava/lang/Object;)V\n" +
-            "@Lcom/sun/btrace/annotations/OnMethod;(clazz=\"/.*\\\\.OnMethodTest/\", method=\"nativeWithoutReturn\")\n" +
-            "@Lcom/sun/btrace/annotations/Self;() // parameter 0\n" +
-            "TRYCATCHBLOCK L0 L1 L1 java/lang/Throwable\n" +
-            "GETSTATIC traces/onmethod/NativeWithoutReturn.runtime : Lcom/sun/btrace/BTraceRuntime;\n" +
-            "INVOKESTATIC com/sun/btrace/BTraceRuntime.enter (Lcom/sun/btrace/BTraceRuntime;)Z\n" +
-            "IFNE L0\n" +
-            "RETURN\n" +
-            "L0\n" +
-            "LDC \"args\"\n" +
-            "INVOKESTATIC com/sun/btrace/BTraceUtils.println (Ljava/lang/Object;)V\n" +
-            "INVOKESTATIC com/sun/btrace/BTraceRuntime.leave ()V\n" +
-            "RETURN\n" +
-            "L1\n" +
-            "INVOKESTATIC com/sun/btrace/BTraceRuntime.handleException (Ljava/lang/Throwable;)V\n" +
-            "INVOKESTATIC com/sun/btrace/BTraceRuntime.leave ()V\n" +
-            "RETURN\n" +
-            "MAXSTACK = 1\n" +
-            "MAXLOCALS = 1"
-        );
-    }
 }