• Bug
  • Status: Closed
  • 2 Major
  • Resolution: Fixed
  • DSO:L1
  • hhuynh
  • Reporter: ekulesho
  • June 04, 2007
  • 0
  • Watchers: 0
  • June 19, 2007
  • June 19, 2007

Attachments

Description

We saw some failure when instrumenting bytecode compiled with -target 1.6:

java.lang.IllegalStateException: ClassReader.accept() should be called with EXPAND_FRAMES flag at com.tc.asm.commons.LocalVariablesSorter.visitFrame(LocalVariablesSorter.java:169)
at com.tc.asm.ClassReader.accept(ClassReader.java:1159) at com.tc.asm.ClassReader.accept(ClassReader.java:394) at com.tc.object.bytecode.hook.impl.DefaultWeavingStrategy.transform(DefaultWeavingStrategy.java:269)
at com.tc.object.bytecode.hook.impl.DSOContextImpl.preProcess(DSOContextImpl.java:137) at com.tc.object.bytecode.hook.impl.ClassProcessorHelper.defineClass0Pre(ClassProcessorHelper.java:416) at java.lang.ClassLoader.defineClass(ClassLoader.java:620) at java.security.SecureClassLoader.defineClass(SecureClassLoader.java:124) at java.net.URLClassLoader.defineClass(URLClassLoader.java:260) at java.net.URLClassLoader.access$100(URLClassLoader.java:56)

Comments

Eugene Kuleshov 2007-06-04

The exception says it. There is a loaded feature in “LocalVariablesSorter” that allows to incrementally update stackmap frames, to allow to avoid full recalculation. So, ClassReader.accept() calls that receive any subclass of LocalVariable sorter need EXPAND_FRAMES flag.

However to make it work, all the chained visitors should also support incremental stackmap updates (which is not always simple task and it would require special attention).

If we can’t support incremental updates, then we’ll need to turn on COMPUTE_FRAMES on ClassWriters. But then we’ll need special class resolver (ClassWriter.getCommonSuperClass() need to be overwritten) that would resolve classes without classloading. That all will slowdown transformation quite a bit.

There is an alternative way. Since JVM would fall back to old bytecode verifier and there is no other bytecode additions in Java 6, we can downgrade Java 6 bytecode to Java 5 and strip all frame information from transformed classes using ClassReader.SKIP_FRAMES flag. This option will buy us time until Java 7.

Eugene Kuleshov 2007-06-04

For the future reference see ClassInfo in http://tinyurl.com/2ueuk2 for an example of the naive non-caching class resolver. We should be able to use and extend ClasInfo implementation from AW.

Eugene Kuleshov 2007-06-13

Fix committed to trunk and 2.4 branch

Sreenivasan Iyer 2007-06-15

Using this nightly build results in an ASM assertion elsewhere (jdk1.6) plz advise.

Steve Harris 2007-06-15

I don’t see the new exception attached to this bug entry. Am I missing it? Tim felt it was not related?

Nathaniel Harward 2007-06-15

From Tim Eck on email: ‘Any idea what the “tc.server” system property is? It looks like it might be getting set, and the code expects it to be of the form “server:port,server2:port,etc”. The actual value in the property is missing the “:port” part…’

I agree with Tim here, the attached terracotta-client_asmAssert.log file looks like it has nothing to do with ASM at all, just a configuration error. You should be able to simply omit this system property in which case the tc-config.xml file will be consulted, and appears to have this:

    <server host="localhost" name="localhost:9510">
        <data>%(tc.home)/terracotta/server-data</data>
        <logs>%(tc.home)/terracotta/server-logs</logs>
    </server>

Or if you have to set tc.server to something, set it to “localhost:9510”. Please try that, it should either work or get to the next problem in the chain :)

Nathaniel Harward 2007-06-15

Assigning back to Eugene, there may be something else to finish up or test before this is closed out.

Steve Harris 2007-06-19

Do we think this works now? Can you and tim figure this out so the customer doesn’t bolt. Eu is out till thursday.

Eugene Kuleshov 2007-06-19

Apparently I missed few places where ClassReader.accept() method is called without SKIP_FRAMES (stupid method call references search in Eclipse). Here is the list:

\code\base\aspectwerkz\src\com\tc\aspectwerkz\proxy\ProxyDelegationCompiler.java \code\base\aspectwerkz\src\com\tc\aspectwerkz\transform\inlining\ProxyWeavingStrategy.java \code\base\aspectwerkz\src\com\tc\aspectwerkz\transform\inlining\weaver\SerialVersionUidVisitor.java \code\base\dso-l1\src\com\tc\object\bytecode\hook\impl\DefaultWeavingStrategy.java

So, you need to find all calls to ClassReader.accept() methods and add ClassReader.SKIP_FRAMES bitmask into the second parameter. This is a trivial fix, I

Tim Eck 2007-06-19

The last comment seems to be cut short – I’m going to make the suggested changes now in 2.4 branch

Nathaniel Harward 2007-06-19

Tim is working on this for Iyer, so I’ll assign it to him.

Tim Eck 2007-06-19

should have all accept() calls modified now