You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@openjpa.apache.org by "Heath Thomann (JIRA)" <ji...@apache.org> on 2012/11/16 18:20:12 UTC

[jira] [Created] (OPENJPA-2298) VerifyError/Stack underflow due to extra 'return' statements generated by PCEnhancer.

Heath Thomann created OPENJPA-2298:
--------------------------------------

             Summary: VerifyError/Stack underflow due to extra 'return' statements generated by PCEnhancer.
                 Key: OPENJPA-2298
                 URL: https://issues.apache.org/jira/browse/OPENJPA-2298
             Project: OpenJPA
          Issue Type: Bug
          Components: Enhance, instrumentation
    Affects Versions: 2.2.0, 2.3.0
            Reporter: Heath Thomann
            Assignee: Heath Thomann
            Priority: Critical


I've been assisting Kevin Sutter with a 'VerifyError' and am opening this JIRA on his behalf.  He has discovered a situation where the following VerifyError occurs:

Exception data: java.lang.VerifyError: JVMVRFY036 stack underflow; class=irwwbase/PersistentTimerStatsJPA, method=pcNewObjectIdInstance(Ljava/lang/Object;)Ljava/lang/Object;, pc=26
    at java.lang.J9VMInternals.verifyImpl(Native Method)
    at java.lang.J9VMInternals.verify(J9VMInternals.java:85)
    at java.lang.J9VMInternals.initialize(J9VMInternals.java:162)
    at java.lang.Class.forNameImpl(Native Method)
    at java.lang.Class.forName(Class.java:176)
    at org.apache.openjpa.meta.MetaDataRepository.classForName(MetaDataRepository.java:1552)
    at org.apache.openjpa.meta.MetaDataRepository.loadPersistentTypesInternal(MetaDataRepository.java:1528)
    at org.apache.openjpa.meta.MetaDataRepository.loadPersistentTypes(MetaDataRepository.java:1506)
    at org.apache.openjpa.kernel.AbstractBrokerFactory.loadPersistentTypes(AbstractBrokerFactory.java:282)
    at org.apache.openjpa.kernel.AbstractBrokerFactory.initializeBroker(AbstractBrokerFactory.java:238)
    at org.apache.openjpa.kernel.AbstractBrokerFactory.newBroker(AbstractBrokerFactory.java:212)
    at org.apache.openjpa.kernel.DelegatingBrokerFactory.newBroker(DelegatingBrokerFactory.java:156)
    at org.apache.openjpa.persistence.EntityManagerFactoryImpl.createEntityManager(EntityManagerFactoryImpl.java:227)
    ...........


This issue occurs on, and is unique to, Java 7 and our enhancement code (as well as the entity definition).  That is, in PCEnhancer we are generating extra 'returns' in methods which throw an exception.  This has an effect on ASM post-processing needed on Java 7 which ultimately causes the VerifyError.  This description probably makes no sense without more details and code to go along with it.  So let me now show how this looks in code.  First, I'm attaching an entity, named PersistentTimerStatsJPA.java, and a PK class named 'PersistentTimerStatsPK.java'.  While I won't go into exact details on how to use these classes to recreate the issue, I am providing them for the reader's reference.
Next, lets look at enhanced code before ASM post-processing occurs.  Take this code:

  public java.lang.Object pcNewObjectIdInstance(java.lang.Object);
    flags: ACC_PUBLIC

    Code:
      stack=3, locals=2, args_size=2
         0: new           #207                // class java/lang/IllegalArgumentException
         3: dup           
         4: ldc_w         #367                // String The id type \"class irwwbase.PersistentTimerStatsPK\" specified by persistent type \"class irwwbase.PersistentTimerStatsJPA\" does not have a public class irwwbase.PersistentTimerStatsPK(String) or class irwwbase.PersistentTimerStatsPK(Class, String) constructor.
         7: invokespecial #368                // Method java/lang/IllegalArgumentException."<init>":(Ljava/lang/String;)V
        10: athrow        
        11: return        

As you can see, we have a "10: athrow" and "11: return".  This extraneous "11: return" hasn't caused problems in the past, but as will be explained in more details below, with ASM post-processing necessary in Java 7, this "11: return" causes problems.  To see this, lets look at the code above after going through ASM post-processing:

  public java.lang.Object pcNewObjectIdInstance(java.lang.Object);
    flags: ACC_PUBLIC

    Code:
      stack=3, locals=2, args_size=2
         0: new           #205                // class java/lang/IllegalArgumentException
         3: dup           
         4: ldc_w         #363                // String The id type \"class irwwbase.PersistentTimerStatsPK\" specified by persistent type \"class irwwbase.PersistentTimerStatsJPA\" does not have a public class irwwbase.PersistentTimerStatsPK(String) or class irwwbase.PersistentTimerStatsPK(Class, String) constructor.
         7: invokespecial #364                // Method java/lang/IllegalArgumentException."<init>":(Ljava/lang/String;)V
        10: athrow        
        11: athrow        
      StackMapTable: number_of_entries = 1
           frame_type = 255 /* full_frame */
          offset_delta = 11
          locals = []
          stack = [ class java/lang/Throwable ]

Notice that the "11: return" was changed to a second "11: athrow".  To tie this all together, let me blatantly copy/paste a detailed description sent to me by Kevin: 

"When JPA was attempting to generate the bytecodes for throwing an exception, we were accidentally including a return bytecode as well.  Of course, in a normal Java environment, this would have been flagged since the return would have been unreachable code.  But, since we were generating the bytecodes during enhancement, nothing is going to flag this situation.  But, due to the Java 7 class validation that is now being performed, we had to introduce the ASM post-processing.  Normally, these simple enhanced methods would not have been touched by ASM.  But, since these methods now had multiple exit points (due to the throw and return bytecodes), these methods were being flagged as "complicated" and needed Stack Map table adjustments performed by ASM.  Unfortunately, this additional ASM code now introduced the situation where too many items would have been popped off the stack (if the code could have actually executed as coded).  This is the Stack Underflow exception that was happening during Class load time."

Basically, Kevin's fix was to find all of the areas where we generate a "throw" followed by a "return", and remove the "return".  Since this method is only attempting to throw an exception, we really don't need the second "return" instruction.  I'm providing Kevin's patch, which is named 'VerifyError.patch'.

Thanks,

Heath

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Updated] (OPENJPA-2298) VerifyError/Stack underflow due to extra 'return' statements generated by PCEnhancer.

Posted by "Heath Thomann (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/OPENJPA-2298?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Heath Thomann updated OPENJPA-2298:
-----------------------------------

    Attachment: PersistentTimerStatsPK.java
                PersistentTimerStatsJPA.java
                VerifyError.patch

I'm providing a patch which was created by Kevin Sutter, and am attaching two classes he used when recreating the issue.

Thanks,

Heath
                
> VerifyError/Stack underflow due to extra 'return' statements generated by PCEnhancer.
> -------------------------------------------------------------------------------------
>
>                 Key: OPENJPA-2298
>                 URL: https://issues.apache.org/jira/browse/OPENJPA-2298
>             Project: OpenJPA
>          Issue Type: Bug
>          Components: Enhance, instrumentation
>    Affects Versions: 2.2.0, 2.3.0
>            Reporter: Heath Thomann
>            Assignee: Heath Thomann
>            Priority: Critical
>         Attachments: PersistentTimerStatsJPA.java, PersistentTimerStatsPK.java, VerifyError.patch
>
>
> I've been assisting Kevin Sutter with a 'VerifyError' and am opening this JIRA on his behalf.  He has discovered a situation where the following VerifyError occurs:
> Exception data: java.lang.VerifyError: JVMVRFY036 stack underflow; class=irwwbase/PersistentTimerStatsJPA, method=pcNewObjectIdInstance(Ljava/lang/Object;)Ljava/lang/Object;, pc=26
>     at java.lang.J9VMInternals.verifyImpl(Native Method)
>     at java.lang.J9VMInternals.verify(J9VMInternals.java:85)
>     at java.lang.J9VMInternals.initialize(J9VMInternals.java:162)
>     at java.lang.Class.forNameImpl(Native Method)
>     at java.lang.Class.forName(Class.java:176)
>     at org.apache.openjpa.meta.MetaDataRepository.classForName(MetaDataRepository.java:1552)
>     at org.apache.openjpa.meta.MetaDataRepository.loadPersistentTypesInternal(MetaDataRepository.java:1528)
>     at org.apache.openjpa.meta.MetaDataRepository.loadPersistentTypes(MetaDataRepository.java:1506)
>     at org.apache.openjpa.kernel.AbstractBrokerFactory.loadPersistentTypes(AbstractBrokerFactory.java:282)
>     at org.apache.openjpa.kernel.AbstractBrokerFactory.initializeBroker(AbstractBrokerFactory.java:238)
>     at org.apache.openjpa.kernel.AbstractBrokerFactory.newBroker(AbstractBrokerFactory.java:212)
>     at org.apache.openjpa.kernel.DelegatingBrokerFactory.newBroker(DelegatingBrokerFactory.java:156)
>     at org.apache.openjpa.persistence.EntityManagerFactoryImpl.createEntityManager(EntityManagerFactoryImpl.java:227)
>     ...........
> This issue occurs on, and is unique to, Java 7 and our enhancement code (as well as the entity definition).  That is, in PCEnhancer we are generating extra 'returns' in methods which throw an exception.  This has an effect on ASM post-processing needed on Java 7 which ultimately causes the VerifyError.  This description probably makes no sense without more details and code to go along with it.  So let me now show how this looks in code.  First, I'm attaching an entity, named PersistentTimerStatsJPA.java, and a PK class named 'PersistentTimerStatsPK.java'.  While I won't go into exact details on how to use these classes to recreate the issue, I am providing them for the reader's reference.
> Next, lets look at enhanced code before ASM post-processing occurs.  Take this code:
>   public java.lang.Object pcNewObjectIdInstance(java.lang.Object);
>     flags: ACC_PUBLIC
>     Code:
>       stack=3, locals=2, args_size=2
>          0: new           #207                // class java/lang/IllegalArgumentException
>          3: dup           
>          4: ldc_w         #367                // String The id type \"class irwwbase.PersistentTimerStatsPK\" specified by persistent type \"class irwwbase.PersistentTimerStatsJPA\" does not have a public class irwwbase.PersistentTimerStatsPK(String) or class irwwbase.PersistentTimerStatsPK(Class, String) constructor.
>          7: invokespecial #368                // Method java/lang/IllegalArgumentException."<init>":(Ljava/lang/String;)V
>         10: athrow        
>         11: return        
> As you can see, we have a "10: athrow" and "11: return".  This extraneous "11: return" hasn't caused problems in the past, but as will be explained in more details below, with ASM post-processing necessary in Java 7, this "11: return" causes problems.  To see this, lets look at the code above after going through ASM post-processing:
>   public java.lang.Object pcNewObjectIdInstance(java.lang.Object);
>     flags: ACC_PUBLIC
>     Code:
>       stack=3, locals=2, args_size=2
>          0: new           #205                // class java/lang/IllegalArgumentException
>          3: dup           
>          4: ldc_w         #363                // String The id type \"class irwwbase.PersistentTimerStatsPK\" specified by persistent type \"class irwwbase.PersistentTimerStatsJPA\" does not have a public class irwwbase.PersistentTimerStatsPK(String) or class irwwbase.PersistentTimerStatsPK(Class, String) constructor.
>          7: invokespecial #364                // Method java/lang/IllegalArgumentException."<init>":(Ljava/lang/String;)V
>         10: athrow        
>         11: athrow        
>       StackMapTable: number_of_entries = 1
>            frame_type = 255 /* full_frame */
>           offset_delta = 11
>           locals = []
>           stack = [ class java/lang/Throwable ]
> Notice that the "11: return" was changed to a second "11: athrow".  To tie this all together, let me blatantly copy/paste a detailed description sent to me by Kevin: 
> "When JPA was attempting to generate the bytecodes for throwing an exception, we were accidentally including a return bytecode as well.  Of course, in a normal Java environment, this would have been flagged since the return would have been unreachable code.  But, since we were generating the bytecodes during enhancement, nothing is going to flag this situation.  But, due to the Java 7 class validation that is now being performed, we had to introduce the ASM post-processing.  Normally, these simple enhanced methods would not have been touched by ASM.  But, since these methods now had multiple exit points (due to the throw and return bytecodes), these methods were being flagged as "complicated" and needed Stack Map table adjustments performed by ASM.  Unfortunately, this additional ASM code now introduced the situation where too many items would have been popped off the stack (if the code could have actually executed as coded).  This is the Stack Underflow exception that was happening during Class load time."
> Basically, Kevin's fix was to find all of the areas where we generate a "throw" followed by a "return", and remove the "return".  Since this method is only attempting to throw an exception, we really don't need the second "return" instruction.  I'm providing Kevin's patch, which is named 'VerifyError.patch'.
> Thanks,
> Heath

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (OPENJPA-2298) VerifyError/Stack underflow due to extra 'return' statements generated by PCEnhancer.

Posted by "Kevin Sutter (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/OPENJPA-2298?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13499025#comment-13499025 ] 

Kevin Sutter commented on OPENJPA-2298:
---------------------------------------

Good write-up, Heath.  My patch had commented out the extra "return" bytecode generations.  When you apply the patch, please remove the "return" statements.  Don't comment them out.  Thanks.
                
> VerifyError/Stack underflow due to extra 'return' statements generated by PCEnhancer.
> -------------------------------------------------------------------------------------
>
>                 Key: OPENJPA-2298
>                 URL: https://issues.apache.org/jira/browse/OPENJPA-2298
>             Project: OpenJPA
>          Issue Type: Bug
>          Components: Enhance, instrumentation
>    Affects Versions: 2.2.0, 2.3.0
>            Reporter: Heath Thomann
>            Assignee: Heath Thomann
>            Priority: Critical
>         Attachments: PersistentTimerStatsJPA.java, PersistentTimerStatsPK.java, VerifyError.patch
>
>
> I've been assisting Kevin Sutter with a 'VerifyError' and am opening this JIRA on his behalf.  He has discovered a situation where the following VerifyError occurs:
> Exception data: java.lang.VerifyError: JVMVRFY036 stack underflow; class=irwwbase/PersistentTimerStatsJPA, method=pcNewObjectIdInstance(Ljava/lang/Object;)Ljava/lang/Object;, pc=26
>     at java.lang.J9VMInternals.verifyImpl(Native Method)
>     at java.lang.J9VMInternals.verify(J9VMInternals.java:85)
>     at java.lang.J9VMInternals.initialize(J9VMInternals.java:162)
>     at java.lang.Class.forNameImpl(Native Method)
>     at java.lang.Class.forName(Class.java:176)
>     at org.apache.openjpa.meta.MetaDataRepository.classForName(MetaDataRepository.java:1552)
>     at org.apache.openjpa.meta.MetaDataRepository.loadPersistentTypesInternal(MetaDataRepository.java:1528)
>     at org.apache.openjpa.meta.MetaDataRepository.loadPersistentTypes(MetaDataRepository.java:1506)
>     at org.apache.openjpa.kernel.AbstractBrokerFactory.loadPersistentTypes(AbstractBrokerFactory.java:282)
>     at org.apache.openjpa.kernel.AbstractBrokerFactory.initializeBroker(AbstractBrokerFactory.java:238)
>     at org.apache.openjpa.kernel.AbstractBrokerFactory.newBroker(AbstractBrokerFactory.java:212)
>     at org.apache.openjpa.kernel.DelegatingBrokerFactory.newBroker(DelegatingBrokerFactory.java:156)
>     at org.apache.openjpa.persistence.EntityManagerFactoryImpl.createEntityManager(EntityManagerFactoryImpl.java:227)
>     ...........
> This issue occurs on, and is unique to, Java 7 and our enhancement code (as well as the entity definition).  That is, in PCEnhancer we are generating extra 'returns' in methods which throw an exception.  This has an effect on ASM post-processing needed on Java 7 which ultimately causes the VerifyError.  This description probably makes no sense without more details and code to go along with it.  So let me now show how this looks in code.  First, I'm attaching an entity, named PersistentTimerStatsJPA.java, and a PK class named 'PersistentTimerStatsPK.java'.  While I won't go into exact details on how to use these classes to recreate the issue, I am providing them for the reader's reference.
> Next, lets look at enhanced code before ASM post-processing occurs.  Take this code:
>   public java.lang.Object pcNewObjectIdInstance(java.lang.Object);
>     flags: ACC_PUBLIC
>     Code:
>       stack=3, locals=2, args_size=2
>          0: new           #207                // class java/lang/IllegalArgumentException
>          3: dup           
>          4: ldc_w         #367                // String The id type \"class irwwbase.PersistentTimerStatsPK\" specified by persistent type \"class irwwbase.PersistentTimerStatsJPA\" does not have a public class irwwbase.PersistentTimerStatsPK(String) or class irwwbase.PersistentTimerStatsPK(Class, String) constructor.
>          7: invokespecial #368                // Method java/lang/IllegalArgumentException."<init>":(Ljava/lang/String;)V
>         10: athrow        
>         11: return        
> As you can see, we have a "10: athrow" and "11: return".  This extraneous "11: return" hasn't caused problems in the past, but as will be explained in more details below, with ASM post-processing necessary in Java 7, this "11: return" causes problems.  To see this, lets look at the code above after going through ASM post-processing:
>   public java.lang.Object pcNewObjectIdInstance(java.lang.Object);
>     flags: ACC_PUBLIC
>     Code:
>       stack=3, locals=2, args_size=2
>          0: new           #205                // class java/lang/IllegalArgumentException
>          3: dup           
>          4: ldc_w         #363                // String The id type \"class irwwbase.PersistentTimerStatsPK\" specified by persistent type \"class irwwbase.PersistentTimerStatsJPA\" does not have a public class irwwbase.PersistentTimerStatsPK(String) or class irwwbase.PersistentTimerStatsPK(Class, String) constructor.
>          7: invokespecial #364                // Method java/lang/IllegalArgumentException."<init>":(Ljava/lang/String;)V
>         10: athrow        
>         11: athrow        
>       StackMapTable: number_of_entries = 1
>            frame_type = 255 /* full_frame */
>           offset_delta = 11
>           locals = []
>           stack = [ class java/lang/Throwable ]
> Notice that the "11: return" was changed to a second "11: athrow".  To tie this all together, let me blatantly copy/paste a detailed description sent to me by Kevin: 
> "When JPA was attempting to generate the bytecodes for throwing an exception, we were accidentally including a return bytecode as well.  Of course, in a normal Java environment, this would have been flagged since the return would have been unreachable code.  But, since we were generating the bytecodes during enhancement, nothing is going to flag this situation.  But, due to the Java 7 class validation that is now being performed, we had to introduce the ASM post-processing.  Normally, these simple enhanced methods would not have been touched by ASM.  But, since these methods now had multiple exit points (due to the throw and return bytecodes), these methods were being flagged as "complicated" and needed Stack Map table adjustments performed by ASM.  Unfortunately, this additional ASM code now introduced the situation where too many items would have been popped off the stack (if the code could have actually executed as coded).  This is the Stack Underflow exception that was happening during Class load time."
> Basically, Kevin's fix was to find all of the areas where we generate a "throw" followed by a "return", and remove the "return".  Since this method is only attempting to throw an exception, we really don't need the second "return" instruction.  I'm providing Kevin's patch, which is named 'VerifyError.patch'.
> Thanks,
> Heath

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira