United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
Bug ID: JDK-6667089 multiple redefinitions of a class break reflection
JDK-6667089 : multiple redefinitions of a class break reflection

Details
Type:
Bug
Submit Date:
2008-02-25
Status:
Closed
Updated Date:
2012-02-01
Project Name:
JDK
Resolved Date:
2011-03-08
Component:
hotspot
OS:
generic
Sub-Component:
jvmti
CPU:
generic
Priority:
P3
Resolution:
Fixed
Affected Versions:
6
Fixed Versions:
hs12 (b02)

Related Reports
Backport:
Backport:
Backport:
Backport:
Relates:

Sub Tasks

Description
FULL PRODUCT VERSION :
java version "1.6.0_03"
Java(TM) SE Runtime Environment (build 1.6.0_03-b05)
Java HotSpot(TM) Client VM (build 1.6.0_03-b05, mixed mode, sharing)

ADDITIONAL OS VERSION INFORMATION :
Ubuntu Gutsy:
Linux giordino 2.6.22-14-generic #1 SMP Sun Oct 14 23:05:12 GMT 2007 i686 GNU/Linux

A DESCRIPTION OF THE PROBLEM :
JDK 6 added capability to add/remove private (static | final) methods.
A method added in a second or subsequent retransformation throws an exception when invoked via reflection, provided that the first retransformation redefined a method in a way that it is not anymore EMCP (equivalent modulo constant pool) with its older version.
I did not try, but I guess constructors have the same problem, though probably more difficult to reproduce.

My evaluation of the problem (though I'm not a hotspot expert) led me to hotspot/src/share/vm/runtime/reflection.cpp, method Reflection::invoke_method(). This is where the exception InternalError is thrown.
This method assumes that the methods of a class have strictly consecutive "slot" numbers.

However, when redefining a class this is not the case, since the redefinition of a method losing EMCP causes the class' method_idnum to increase (the new idnum is assigned to the old version of the method).

A fix that seems to work is to change Reflection::invoke_method() implementation to use:

klass->method_with_idnum(slot)

instead of:

klass->methods()->obj_at(slot)

Thanks !

STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
1) Class A has method m()
2) Redefine A:
     a) change m() code so that m() is not EMCP to its old version
     b) add method m1(); method m1() gets idnum == 2 (this is not strictly necessary, but illustrates better what happens with idnums and why on first redefinition the problem does not show up)
3) Redefine again A:
     a) add method m2()
4) m2() cannot be invoked via reflection

My evaluation is:
At step 1) method m() gets idnum == 1 (where idnum is the method's idnum)
At step 2) method m() is redefined in a non EMCP way and m1() gets idnum == 2. m() is marked as obsolete and its older version gets idnum == 3.
At step 3) m2() is added and gets idnum == 4.
At step 4) some code tries to call m2() via reflection, but there is a check that its "slot" (i.e. its idnum) is within the class' methods array (which is of length 3: m(), m1() and m2()).
The check fails and the reported exception is thrown.

EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
No exception is thrown by java.lang.reflect.Method.invoke()
ACTUAL -
Exception in thread "main" java.lang.InternalError: invoke
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
	at java.lang.reflect.Method.invoke(Method.java:597)


REPRODUCIBILITY :
This bug can be reproduced always.

---------- BEGIN SOURCE ----------
Download and use ASM 3.0 from http://asm.objectweb.org.

Here's the source code to reproduce the problem; use -XX:TraceRedefineClasses=65535 for better logging.
Compiled code must be jarred; the jar must have the relevant attributes for class retransformation.
Agent is the agent class; Main is the main class.


package jdk6.bug;

import java.lang.instrument.Instrumentation;

/**
 * @version $Revision$ $Date$
 */
public class Agent
{
    private static Instrumentation instrumentation;

    public static void premain(String agentArgs, Instrumentation instrumentation) throws Exception
    {
        agentmain(agentArgs, instrumentation);
    }

    public static void agentmain(String agentArgs, Instrumentation instrumentation) throws Exception
    {
        Agent.instrumentation = instrumentation;
        instrumentation.addTransformer(new FooTransformer(), true);
    }

    public static Instrumentation getInstrumentation()
    {
        return instrumentation;
    }
}


package jdk6.bug;

import java.lang.reflect.Method;

/**
 * @version $Revision$ $Date$
 */
public class Foo
{
    public void test(int counter) throws Exception
    {
        Method method = getClass().getDeclaredMethod("transform" + counter);
        method.setAccessible(true);
        method.invoke(this);
    }

    public void transform()
    {
        new Exception().printStackTrace();
    }
}


package jdk6.bug;

import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.lang.instrument.ClassFileTransformer;
import java.lang.instrument.IllegalClassFormatException;
import java.security.ProtectionDomain;

import org.objectweb.asm.ClassAdapter;
import org.objectweb.asm.ClassReader;
import org.objectweb.asm.ClassVisitor;
import org.objectweb.asm.ClassWriter;
import org.objectweb.asm.MethodVisitor;
import org.objectweb.asm.Opcodes;

/**
 * @version $Revision$ $Date$
 */
public class FooTransformer implements ClassFileTransformer
{
    private int count = 0;
    private String method = "transform";

    public byte[] transform(ClassLoader loader, String internalClassName, Class<?> classBeingRedefined, ProtectionDomain protectionDomain, byte[] classBytes) throws IllegalClassFormatException
    {
        String className = internalClassName.replace('/', '.');
        if ("jdk6.bug.Foo".equals(className)) return trasform(internalClassName, classBytes);
        return null;
    }

    private byte[] trasform(String internalClassName, byte[] classBytes)
    {
        ClassReader classReader = new ClassReader(classBytes);
        ClassWriter classWriter = new ClassWriter(classReader, ClassWriter.COMPUTE_FRAMES);
        classReader.accept(new FooAdapter(classWriter), ClassReader.SKIP_FRAMES);
        byte[] newBytes = classWriter.toByteArray();
        dump(internalClassName, newBytes);
        return newBytes;
    }

    private void dump(String className, byte[] bytes)
    {
        File tmpDir = new File(System.getProperty("java.io.tmpdir"));
        File baseDir = new File(tmpDir, "jdk6-bug");
        baseDir.mkdir();

        try
        {
            File dir = baseDir;
            int lastSlash = className.lastIndexOf("/");
            if (lastSlash >= 0)
            {
                String packageName = className.substring(0, lastSlash);
                className = className.substring(lastSlash + 1);
                dir = new File(baseDir, packageName);
                dir.mkdirs();
            }

            File file = new File(dir, className + (count - 1) + ".class");
            FileOutputStream fos = new FileOutputStream(file);

            fos.write(bytes);
            fos.close();
        }
        catch (IOException x)
        {
            x.printStackTrace();
        }
    }

    private class FooAdapter extends ClassAdapter
    {
        private String className;

        public FooAdapter(ClassVisitor classVisitor)
        {
            super(classVisitor);
        }

        @Override
        public void visit(int version, int modifiers, String className, String signature, String superClassName, String[] interfaces)
        {
            super.visit(version, modifiers, className, signature, superClassName, interfaces);
            this.className = className;
        }

        @Override
        public MethodVisitor visitMethod(int modifiers, String name, String descriptor, String signature, String[] exceptions)
        {
            if (method.equals(name))
            {
                // Create a new method with the current method's code
                String newMethodName = method + count;
                MethodVisitor newMethod = super.visitMethod(Opcodes.ACC_PRIVATE | Opcodes.ACC_FINAL, newMethodName, descriptor, signature, exceptions);

                // Create the method chain
                for (int i = 0; i < count; ++i)
                {
                    MethodVisitor mv = super.visitMethod(Opcodes.ACC_PRIVATE | Opcodes.ACC_FINAL, method + i, descriptor, signature, exceptions);
                    mv.visitCode();
                    mv.visitVarInsn(Opcodes.ALOAD, 0);
                    mv.visitMethodInsn(Opcodes.INVOKEVIRTUAL, className, method + (i + 1), descriptor);
                    mv.visitInsn(Opcodes.RETURN);
                    mv.visitMaxs(0, 0);
                    mv.visitEnd();
                }

                // Replace current method's code with a call to the first method in the chain
                MethodVisitor mv = super.visitMethod(modifiers, name, descriptor, signature, exceptions);
                mv.visitCode();
                mv.visitVarInsn(Opcodes.ALOAD, 0);
                mv.visitMethodInsn(Opcodes.INVOKEVIRTUAL, className, method + "0", descriptor);
                mv.visitInsn(Opcodes.RETURN);
                mv.visitMaxs(0, 0);
                mv.visitEnd();

                // Prepare for next redefinition
                ++count;

                return newMethod;
            }
            return super.visitMethod(modifiers, name, descriptor, signature, exceptions);
        }
    }
}


package jdk6.bug;

/**
 * @version $Revision$ $Date$
 */
public class Main
{
    public static void main(String[] args) throws Exception
    {
        Foo foo = new Foo();
        foo.test(0);

        // First transformation
        retransform();
        foo.test(1);

        // Second transformation
        retransform();
        foo.test(2);
    }

    private static void retransform() throws Exception
    {
        Agent.getInstrumentation().retransformClasses(Foo.class);
    }
}

---------- END SOURCE ----------
Once this bug is fixed, the following test will fail until the
fix is available in a promoted JDK:

    java/lang/instrument/RedefineMethodAddInvoke.sh

                                    

Comments
EVALUATION

The "slot" values in question are fetched by
java_lang_reflect_Method::slot() and
java_lang_reflect_Constructor::slot(). All other uses of
java_lang_reflect_Method::slot() and
java_lang_reflect_Constructor::slot() pass the slot value
to instanceKlass::method_with_idnum() in order to fetch
the appropriate methodOop. I'm not sure how these two
uses were missed.
                                     
2008-02-27
EVALUATION

This bug was introduced by the fix for the following bug:

    6404550 1/1 Cannot implement late attach in NetBeans Profiler
                due to missing functionality in JVMTI

This bug was fixed in Mustang-B83 and I just verified that the
missed areas in reflection.cpp are present in that workspace
also. This was just a simple miss and since I was one of the
code reviewers, I share the blame.
                                     
2008-02-27
SUGGESTED FIX

Please see the attached 6667089-webrev-hsx-cr0 and
6667089-webrev-sdk-cr0 files for the proposed fix.
The fix is in HotSpot (HSX). However, the test is
part of the INSTRUMENT_REGRESSION test suite so
the test is in the SDK.
                                     
2008-02-27
SUGGESTED FIX

The fix shown in 6667089-webrev-hsx-cr0 is relative to
HSX-10/160_04. This file was changed in both HSX-11 and
HSX-12 so merges will be required as the fix is brought
forward.

The new tests shown in 6667089-webrev-sdk-cr0 were
developed in an SDK workspace relative to Dolphin-B24.
I don't think there will be any issues if the new test
needs to be backported to an earlier SDK.
                                     
2008-02-27



Hardware and Software, Engineered to Work Together