groovy
  1. groovy
  2. GROOVY-4922

StackOverflowError when calling super and overriding a package protected java method

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.8.0
    • Fix Version/s: 2.0-beta-3, 1.7.11, 1.8.7
    • Component/s: None
    • Labels:
      None
    • Number of attachments :
      6

      Description

      For reference : http://groovy.329449.n5.nabble.com/StackOverflowError-when-dispatching-to-super-td4572268.html

      If a package protected method written in a Java class is overriden in a Groovy class and that method calls super.method(), Groovy throws a stack overflow :

      Parent.java
      class Parent {
         void someMethod(String param) { ... }
      }
      
      Child.groovy
      class Child {
         void someMethod(String param) { super.someMethod(param) }
      }
      
      java.lang.StackOverflowError
      	at java.lang.Exception.<init>(Exception.java:77)
      	at java.lang.reflect.InvocationTargetException.<init>(InvocationTargetException.java:54)
      	at sun.reflect.GeneratedMethodAccessor1.invoke(Unknown Source)
      	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
      	at org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:90)
      	at groovy.lang.MetaMethod.doMethodInvoke(MetaMethod.java:233)
      	at groovy.lang.MetaClassImpl.invokeMethod(MetaClassImpl.java:1054)
      	at org.codehaus.groovy.runtime.ScriptBytecodeAdapter.invokeMethodOnSuperN(ScriptBytecodeAdapter.java:128)
      	at groovy.bugs.GroovyStackOverflowBug$Child.someMethod(GroovyStackOverflowBug.groovy:39)
      	at sun.reflect.GeneratedMethodAccessor1.invoke(Unknown Source)
      	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
      	at org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:90)
      	at groovy.lang.MetaMethod.doMethodInvoke(MetaMethod.java:233)
      	at groovy.lang.MetaClassImpl.invokeMethod(MetaClassImpl.java:1054)
      	at org.codehaus.groovy.runtime.ScriptBytecodeAdapter.invokeMethodOnSuperN(ScriptBytecodeAdapter.java:128)
      	at groovy.bugs.GroovyStackOverflowBug$Child.someMethod(GroovyStackOverflowBug.groovy:39)
      	at sun.reflect.GeneratedMethodAccessor1.invoke(Unknown Source)
      	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
      	at org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:90)
      	at groovy.lang.MetaMethod.doMethodInvoke(MetaMethod.java:233)
      	at groovy.lang.MetaClassImpl.invokeMethod(MetaClassImpl.java:1054)
      	at org.codehaus.groovy.runtime.ScriptBytecodeAdapter.invokeMethodOnSuperN(ScriptBytecodeAdapter.java:128)
      	at groovy.bugs.GroovyStackOverflowBug$Child.someMethod(GroovyStackOverflowBug.groovy:39)
      	at sun.reflect.GeneratedMethodAccessor1.invoke(Unknown Source)
      ...
      

      Adding a public or protected modifier to the super class solves the problem.

      1. 4922failure.patch
        2 kB
        Hamlet D'Arcy
      2. GROOVY-4922.patch
        5 kB
        CÚdric Champeau
      3. GROOVY-4922-new.patch
        7 kB
        CÚdric Champeau
      4. test-reports.tar.gz
        329 kB
        CÚdric Champeau

        Activity

        Hide
        CÚdric Champeau added a comment -

        Here is what I found. In MetaClassImpl.MethodIndexAction#methodNameAction(),

        1. If you use a protected modifier in the super class method, the mopMethods list contains the $super method. If the modifier is removed, the method disappears from mopMethods.
        1. plus, the following test fails because of the modifier test (line 460) :
          (useThis ^ (method.getModifiers() & (Modifier.PUBLIC | Modifier.PROTECTED)) == 0)

        So it seems there are two problems to solve : the fact that the super method is missing from the MOP methods and the test which forgets about package protected methods. I'm not sure this can be fixed without side effects.

        Show
        CÚdric Champeau added a comment - Here is what I found. In MetaClassImpl.MethodIndexAction#methodNameAction(), If you use a protected modifier in the super class method, the mopMethods list contains the $super method. If the modifier is removed, the method disappears from mopMethods . plus, the following test fails because of the modifier test (line 460) : (useThis ^ (method.getModifiers() & (Modifier.PUBLIC | Modifier.PROTECTED)) == 0) So it seems there are two problems to solve : the fact that the super method is missing from the MOP methods and the test which forgets about package protected methods. I'm not sure this can be fixed without side effects.
        Hide
        Hamlet D'Arcy added a comment -

        I recommend changing the Priority to Blocker.

        Show
        Hamlet D'Arcy added a comment - I recommend changing the Priority to Blocker.
        Hide
        CÚdric Champeau added a comment -

        Hamlet,

        Here is my test case for this bug. In the thread, you say that you have the error even with a non package protected method. Could you update the test class with an example of such a behaviour ? Thank you !

        StackOverflowBugSupport.java
        package groovy.bugs;
        
        /**
         * Support class for GROOVY-4922 bug. Must be written in Java.
         */
        class StackOverflowBugSupport {
            protected String support;
        
            void someMethod(String value) {
                support = value;
            }
        }
        
        
        GroovyStackOverflowBug
        
        package groovy.bugs
        
        
        class GroovyStackOverflowBug extends GroovyTestCase {
            void testShouldNotThrowStackOverflow() {
                def child = new Child()
                child.someMethod("value")
                assert child.support == "value"
            }
        
        
            class Child extends StackOverflowBugSupport {
                protected void someMethod(String parameter) {
                    super.someMethod(parameter)
                }
            }
        }
        

        I'll update this ticket if I find anything relevant meanwhile.

        Show
        CÚdric Champeau added a comment - Hamlet, Here is my test case for this bug. In the thread, you say that you have the error even with a non package protected method. Could you update the test class with an example of such a behaviour ? Thank you ! StackOverflowBugSupport.java package groovy.bugs; /** * Support class for GROOVY-4922 bug. Must be written in Java. */ class StackOverflowBugSupport { protected String support; void someMethod( String value) { support = value; } } GroovyStackOverflowBug package groovy.bugs class GroovyStackOverflowBug extends GroovyTestCase { void testShouldNotThrowStackOverflow() { def child = new Child() child.someMethod( "value" ) assert child.support == "value" } class Child extends StackOverflowBugSupport { protected void someMethod( String parameter) { super .someMethod(parameter) } } } I'll update this ticket if I find anything relevant meanwhile.
        Hide
        blackdrag blackdrag added a comment -

        Cedric be careful and keep inner classes out of this, they may have differing semantics here and there. not that its necessarily intentional, but that would be a different bug then

        Show
        blackdrag blackdrag added a comment - Cedric be careful and keep inner classes out of this, they may have differing semantics here and there. not that its necessarily intentional, but that would be a different bug then
        Hide
        CÚdric Champeau added a comment -

        Ok, I'll be testing both. I have a patch which solves the problem by adding package protected methods to the list of generated super$ methods. Doing so (and updating the modifiers test too), I can work around the stack overflow, but right now it seems to break the build (3 tests fail). I don't know if it's related to my changes or not, since I didn't do a clean build before patching, but this bug is the kind of ones which could have serious side effects.

        I'll tell you as I advance, I'll have 2 hours of train where I'll be able to play around

        Show
        CÚdric Champeau added a comment - Ok, I'll be testing both. I have a patch which solves the problem by adding package protected methods to the list of generated super$ methods. Doing so (and updating the modifiers test too), I can work around the stack overflow, but right now it seems to break the build (3 tests fail). I don't know if it's related to my changes or not, since I didn't do a clean build before patching, but this bug is the kind of ones which could have serious side effects. I'll tell you as I advance, I'll have 2 hours of train where I'll be able to play around
        Hide
        blackdrag blackdrag added a comment -

        most probably this is the right way to solve the issue. it would be good to (a) attach the not yet complete patch here and (b) to show what 3 tests fail. Normally I wouldn't be pressing, but we intend to release tomorrow and I would like this issue here to be solved till then

        Show
        blackdrag blackdrag added a comment - most probably this is the right way to solve the issue. it would be good to (a) attach the not yet complete patch here and (b) to show what 3 tests fail. Normally I wouldn't be pressing, but we intend to release tomorrow and I would like this issue here to be solved till then
        Hide
        CÚdric Champeau added a comment -

        Here is the patch and the test case results. I won't be able to deliver a full fix until tomorrow because I won't be home, so feel free to complete.

        Sorry I can't give you more information, I have to take my train now

        Show
        CÚdric Champeau added a comment - Here is the patch and the test case results. I won't be able to deliver a full fix until tomorrow because I won't be home, so feel free to complete. Sorry I can't give you more information, I have to take my train now
        Hide
        CÚdric Champeau added a comment -

        The following patch (new) passes all tests on trunk. Basically, I moved my "Child" test class to its own file and made "shouldNotCompile" in GenericsTestBase protected. However, at first glance, it should have worked even without the latter modification. I would be great if Hamlet could test it.

        Show
        CÚdric Champeau added a comment - The following patch (new) passes all tests on trunk. Basically, I moved my "Child" test class to its own file and made "shouldNotCompile" in GenericsTestBase protected. However, at first glance, it should have worked even without the latter modification. I would be great if Hamlet could test it.
        Hide
        blackdrag blackdrag added a comment -

        I modified your test Cedric to test for private directly instead of excluding others. That did let the tests pass. So I am closing the issue for now

        Show
        blackdrag blackdrag added a comment - I modified your test Cedric to test for private directly instead of excluding others. That did let the tests pass. So I am closing the issue for now
        Hide
        Hamlet D'Arcy added a comment -

        I still have this error when using 1.8.x branch and the trunk.

        Show
        Hamlet D'Arcy added a comment - I still have this error when using 1.8.x branch and the trunk.
        Hide
        CÚdric Champeau added a comment -

        The fix requires that you recompile the child class. Did you do so ? If yes then it is probably a separate bug and it would help if you could update the unit test to highlight your use case.

        Show
        CÚdric Champeau added a comment - The fix requires that you recompile the child class. Did you do so ? If yes then it is probably a separate bug and it would help if you could update the unit test to highlight your use case.
        Hide
        Hamlet D'Arcy added a comment -

        here is a patch showing that this still fails. You probably fixed it for protected methods but not for package-private methods.

        Show
        Hamlet D'Arcy added a comment - here is a patch showing that this still fails. You probably fixed it for protected methods but not for package-private methods.
        Hide
        blackdrag blackdrag added a comment -

        I think I am missing information here. The test I added uses a package private method in the java super class. In the test the child class uses a protected method to overwrite this. In the patch from Hamlet it is a public method. Oh I see the test case is not ending with Test, so maybe it is not picked up.

        Show
        blackdrag blackdrag added a comment - I think I am missing information here. The test I added uses a package private method in the java super class. In the test the child class uses a protected method to overwrite this. In the patch from Hamlet it is a public method. Oh I see the test case is not ending with Test, so maybe it is not picked up.
        Hide
        Guillaume Laforge added a comment -

        oops (closed wrong issue), so reopen

        Show
        Guillaume Laforge added a comment - oops (closed wrong issue), so reopen
        Hide
        blackdrag blackdrag added a comment -

        ok, seems not to be the issue.

        Show
        blackdrag blackdrag added a comment - ok, seems not to be the issue.
        Hide
        blackdrag blackdrag added a comment -

        the example from the list is causing an illegal access error in 1.7.11, since the class is package private. Same for trunk

        Show
        blackdrag blackdrag added a comment - the example from the list is causing an illegal access error in 1.7.11, since the class is package private. Same for trunk
        Hide
        blackdrag blackdrag added a comment -

        and any version since 1.5.1 I tested has the same problem

        Show
        blackdrag blackdrag added a comment - and any version since 1.5.1 I tested has the same problem
        Hide
        CÚdric Champeau added a comment -

        Yes there are two separate problems in the latest patch from Hamlet :

        1. the field "value" in the parent class is package protected and not recognized in the subclass
        2. it throws a StackOverflowError while my example does not

        While I'm still scratching my head wondering why his example throws the error and not mine, I think the 1st one should be considered as a separate bug.

        Show
        CÚdric Champeau added a comment - Yes there are two separate problems in the latest patch from Hamlet : the field "value" in the parent class is package protected and not recognized in the subclass it throws a StackOverflowError while my example does not While I'm still scratching my head wondering why his example throws the error and not mine, I think the 1st one should be considered as a separate bug.
        Hide
        blackdrag blackdrag added a comment -

        I just tried it with the precompilation as suggested and this does indeed reproduce the issue... only 1.7.0 has exactly the same problem. And even 1.5.1 has this problem. So we are far away from a regression

        Show
        blackdrag blackdrag added a comment - I just tried it with the precompilation as suggested and this does indeed reproduce the issue... only 1.7.0 has exactly the same problem. And even 1.5.1 has this problem. So we are far away from a regression
        Hide
        Hamlet D'Arcy added a comment -

        When CodeNarc users run CodeNarc with 1.7, they never receive a stack overflow exception. When they run it with CodeNarc 1.8 they do. While this specific scenario in the use case may not be a regression, there is something in Groovy 1.8 that causes this. For instance, if Groovy provides a Java class called "ClassCodeVisitorSupport" and the visibility modifier of a method was changed from protected to package-visibile (which should be a non-breaking change), then there is a chance that this is introduced, correct? There is definitely a regression somewhere. If we don't fix this ticket, then I will start looking for changed visibility modifiers that need to be reverted... because from a user perspective there are stackoverflow errors in 1.8 that are not in 1.7.

        Show
        Hamlet D'Arcy added a comment - When CodeNarc users run CodeNarc with 1.7, they never receive a stack overflow exception. When they run it with CodeNarc 1.8 they do. While this specific scenario in the use case may not be a regression, there is something in Groovy 1.8 that causes this. For instance, if Groovy provides a Java class called "ClassCodeVisitorSupport" and the visibility modifier of a method was changed from protected to package-visibile (which should be a non-breaking change), then there is a chance that this is introduced, correct? There is definitely a regression somewhere. If we don't fix this ticket, then I will start looking for changed visibility modifiers that need to be reverted... because from a user perspective there are stackoverflow errors in 1.8 that are not in 1.7.
        Hide
        blackdrag blackdrag added a comment -

        changing from protected to package-private is a breaking change in Java. A subclass in a different package can see protected methods, but not package-private methods. The problem is that I don't have a test case. The one from here is not exhibiting the special problem, the zip from the list also not. Without test case I cannot reproduce the issue. Since it seems that you can reproduce the issue, I guess bisecting is the one way to identify the commit that introduced that. You can of course also look for changed modifiers. In any case we will then have to discuss why this is a problem.

        It is too bad that the stackoverflow example you gave is not so easy to debug here

        Show
        blackdrag blackdrag added a comment - changing from protected to package-private is a breaking change in Java. A subclass in a different package can see protected methods, but not package-private methods. The problem is that I don't have a test case. The one from here is not exhibiting the special problem, the zip from the list also not. Without test case I cannot reproduce the issue. Since it seems that you can reproduce the issue, I guess bisecting is the one way to identify the commit that introduced that. You can of course also look for changed modifiers. In any case we will then have to discuss why this is a problem. It is too bad that the stackoverflow example you gave is not so easy to debug here
        Hide
        Hamlet D'Arcy added a comment -

        what is wrong with the failing test case I attached? that reproduces the error every time for me?

        Show
        Hamlet D'Arcy added a comment - what is wrong with the failing test case I attached? that reproduces the error every time for me?
        Hide
        CÚdric Champeau added a comment -

        Hamlet,

        Yes, I can reproduce the error with your test case. I think what Jochen says is that this test does not correspond to the error you initially reported which is related to the ClassCodeVisitorSupport. There are two things mixed up here (well, I could identify more) : the regression which was introduced somewhere in ClassCodeVisitorSupport or any other class, and the bug which reproduces the StackOverflowError here, but which is not a regression as it could be identified in every version of Groovy tested so far.

        Here's the result of my late investigations :

        • the initial patch works if the superclass is public and a method is package private and the child class is defined in its own file (bug #1, fixed)
        • if you remove the "public" modifier from the superclass definition, it fails with "class groovy.bugs.Groovy4922BugChild cannot access its superclass groovy.bugs.Groovy4922BugSupport" (bug #2, not fixed)
        • if you remove the "protected" modifier on the "value" property of the superclass, it fails with "groovy.lang.MissingPropertyException: No such property: support for class: groovy.bugs.Groovy4922BugChild" (bug #3, not fixed but less critical)
        • if you define the child class directly in the test case, like you did Hamlet, instead of in the "assertScript" section, it fails with a StackOverflowError (bug #4)

        So, I would say your initial error is bug #5, which could be a modifier changed somewhere in the ClassCodeVisitorSupport class or its hierarchy between 1.7 and 1.8.

        There's still some work to fully understand what's going on there

        Show
        CÚdric Champeau added a comment - Hamlet, Yes, I can reproduce the error with your test case. I think what Jochen says is that this test does not correspond to the error you initially reported which is related to the ClassCodeVisitorSupport. There are two things mixed up here (well, I could identify more) : the regression which was introduced somewhere in ClassCodeVisitorSupport or any other class, and the bug which reproduces the StackOverflowError here, but which is not a regression as it could be identified in every version of Groovy tested so far. Here's the result of my late investigations : the initial patch works if the superclass is public and a method is package private and the child class is defined in its own file (bug #1, fixed) if you remove the "public" modifier from the superclass definition, it fails with "class groovy.bugs.Groovy4922BugChild cannot access its superclass groovy.bugs.Groovy4922BugSupport" (bug #2, not fixed) if you remove the "protected" modifier on the "value" property of the superclass, it fails with "groovy.lang.MissingPropertyException: No such property: support for class: groovy.bugs.Groovy4922BugChild" (bug #3, not fixed but less critical) if you define the child class directly in the test case, like you did Hamlet, instead of in the "assertScript" section, it fails with a StackOverflowError (bug #4) So, I would say your initial error is bug #5, which could be a modifier changed somewhere in the ClassCodeVisitorSupport class or its hierarchy between 1.7 and 1.8. There's still some work to fully understand what's going on there
        Hide
        Hamlet D'Arcy added a comment -

        Bug #4 - This is the most direct representation of the CodeNarc code that is seeing the regression. i suggest focusing on this use case.

        Bug #5 - "CodeNarc use case" - CodeNarc subclasses ClassCodeVisitorSupport, and the overflow comes out of the overridden "void visitDeclarationExpression(DeclarationExpression expression)". This method is overridden in CCVS in 1.7 but not in 1.8. The method isn't on the class in 1.7, it is higher up in the hierarchy, but it is on the class in 1.8. Perhaps this is causing the issue, in which case the method should be added back on.

        Regarding whether this is a regression or not... No CodeNarc users see a StackOverflowError using 1.7 and almost all users see it with 1.8. If we're finding defects that are present in 1.7 and earlier then those are nice to fix, but not the root cause of the CodeNarc exceptions.

        Show
        Hamlet D'Arcy added a comment - Bug #4 - This is the most direct representation of the CodeNarc code that is seeing the regression. i suggest focusing on this use case. Bug #5 - "CodeNarc use case" - CodeNarc subclasses ClassCodeVisitorSupport, and the overflow comes out of the overridden "void visitDeclarationExpression(DeclarationExpression expression)". This method is overridden in CCVS in 1.7 but not in 1.8. The method isn't on the class in 1.7, it is higher up in the hierarchy, but it is on the class in 1.8. Perhaps this is causing the issue, in which case the method should be added back on. Regarding whether this is a regression or not... No CodeNarc users see a StackOverflowError using 1.7 and almost all users see it with 1.8. If we're finding defects that are present in 1.7 and earlier then those are nice to fix, but not the root cause of the CodeNarc exceptions.
        Hide
        blackdrag blackdrag added a comment -

        to clarify things... the method is not overriden in ClassCodeVisitorSupport in 1.7, but is in 1.8 and 1.9. The method is also overridden in CodeVisitorSupport, the first place in 1.7 and the second case in 1.8/1.9, the class is higher up in the hierarchy.

        The CodeNarc problem is then fully unrelated to modifiers, since the method is public only.

        The next step would be to reproduce the issue using overridden public methods only. And that is a setup that does not exist as of yet.

        Show
        blackdrag blackdrag added a comment - to clarify things... the method is not overriden in ClassCodeVisitorSupport in 1.7, but is in 1.8 and 1.9. The method is also overridden in CodeVisitorSupport, the first place in 1.7 and the second case in 1.8/1.9, the class is higher up in the hierarchy. The CodeNarc problem is then fully unrelated to modifiers, since the method is public only. The next step would be to reproduce the issue using overridden public methods only. And that is a setup that does not exist as of yet.
        Hide
        blackdrag blackdrag added a comment -

        since the CodeNarc problem seems to be a different one than the one here I created GROOVY-4936. If GROOVY-4936 is fixed we can test if this issue is resolved as well - by chance.

        Show
        blackdrag blackdrag added a comment - since the CodeNarc problem seems to be a different one than the one here I created GROOVY-4936 . If GROOVY-4936 is fixed we can test if this issue is resolved as well - by chance.
        Hide
        blackdrag blackdrag added a comment - - edited

        GROOVY-4936 is now fixed, it would be good to know if that fixed the original issue here as well.

        Show
        blackdrag blackdrag added a comment - - edited GROOVY-4936 is now fixed, it would be good to know if that fixed the original issue here as well.
        Hide
        Hamlet D'Arcy added a comment -

        I retested the scenario in the .zip file that I uploaded, and that is fixed.
        However, I retested against CodeNarc and that is not fixed.
        I will try to make a new small, self-executing example that shows the problem.

        Show
        Hamlet D'Arcy added a comment - I retested the scenario in the .zip file that I uploaded, and that is fixed. However, I retested against CodeNarc and that is not fixed. I will try to make a new small, self-executing example that shows the problem.
        Hide
        blackdrag blackdrag added a comment -

        I was afraid that may be the case, that's why I made the other issue

        Show
        blackdrag blackdrag added a comment - I was afraid that may be the case, that's why I made the other issue
        Hide
        Hamlet D'Arcy added a comment - - edited

        I attached a new self executing stack overflow example.
        This shows it broken when using the ast visitors.

        Show
        Hamlet D'Arcy added a comment - - edited I attached a new self executing stack overflow example. This shows it broken when using the ast visitors.
        Hide
        blackdrag blackdrag added a comment -

        Isn't example the same as before?

        Show
        blackdrag blackdrag added a comment - Isn't example the same as before?
        Hide
        Hamlet D'Arcy added a comment -

        It is slightly different and the example extends ClassCodeVisitorSupport the way our production code does.

        Show
        Hamlet D'Arcy added a comment - It is slightly different and the example extends ClassCodeVisitorSupport the way our production code does.
        Hide
        blackdrag blackdrag added a comment -

        still, when I execute your "run.sh" it works here with trunk and 1.8

        Show
        blackdrag blackdrag added a comment - still, when I execute your "run.sh" it works here with trunk and 1.8
        Hide
        Hamlet D'Arcy added a comment -

        Does your command line environment call the Groovy 1.7 groovyc?
        If you have Groovy 1.8 in your path then the sample runs fine.
        You need to make sure that groovyc dispatches to the 1.7 version.

        Show
        Hamlet D'Arcy added a comment - Does your command line environment call the Groovy 1.7 groovyc? If you have Groovy 1.8 in your path then the sample runs fine. You need to make sure that groovyc dispatches to the 1.7 version.
        Hide
        blackdrag blackdrag added a comment -

        compiled with 1.7.11 and run with 1.8.1 works fine

        Show
        blackdrag blackdrag added a comment - compiled with 1.7.11 and run with 1.8.1 works fine
        Hide
        blackdrag blackdrag added a comment -

        compiled with 1.7.8 and run with 1.8.1 does not work, but run with 1.7.8 does also not

        Show
        blackdrag blackdrag added a comment - compiled with 1.7.8 and run with 1.8.1 does not work, but run with 1.7.8 does also not
        Hide
        blackdrag blackdrag added a comment -

        comparing the bytecode generated by 1.8.1 and 1.7.8 I can see that it simply cannot work. there is a method

          public super$2$someMethod(Ljava/lang/String;)V
            ALOAD 0
            ALOAD 1
            INVOKESPECIAL Parent.someMethod (Ljava/lang/String;)V
            RETURN
            MAXSTACK = 2
            MAXLOCALS = 2
        

        needed to call the super method, which in 1.7.8 does simply not exist. That is also the reason why compiled with 1.7.8 and run with 1.7.8 cannot work as well. 1.7.11 fixes the problem as does 1.8.1. But there is no chance that code compiled with 1.7.8 will ever run correctly like this. And there is nothing we can fix, since the support method needed for this is missing.

        Show
        blackdrag blackdrag added a comment - comparing the bytecode generated by 1.8.1 and 1.7.8 I can see that it simply cannot work. there is a method public super $2$someMethod(Ljava/lang/ String ;)V ALOAD 0 ALOAD 1 INVOKESPECIAL Parent.someMethod (Ljava/lang/ String ;)V RETURN MAXSTACK = 2 MAXLOCALS = 2 needed to call the super method, which in 1.7.8 does simply not exist. That is also the reason why compiled with 1.7.8 and run with 1.7.8 cannot work as well. 1.7.11 fixes the problem as does 1.8.1. But there is no chance that code compiled with 1.7.8 will ever run correctly like this. And there is nothing we can fix, since the support method needed for this is missing.
        Hide
        blackdrag blackdrag added a comment -

        for CodeNarc you could look at the bytecode of the class containing the super method call. Does it have a method like above?

        Show
        blackdrag blackdrag added a comment - for CodeNarc you could look at the bytecode of the class containing the super method call. Does it have a method like above?
        Hide
        Hamlet D'Arcy added a comment -

        Here is the "new" stack overflow example. You and I are clearly working from different examples.

        Show
        Hamlet D'Arcy added a comment - Here is the "new" stack overflow example. You and I are clearly working from different examples.
        Hide
        Hamlet D'Arcy added a comment -

        Jochen, concerning the super method in CodeNarc.

        In CodeNarc, the stack overflow comes from:

        public void visitDeclarationExpression(DeclarationExpression expression)
        

        The super implementation is in CodeVisitorSupport. This method was present in 1.7 and 1.8.

        Show
        Hamlet D'Arcy added a comment - Jochen, concerning the super method in CodeNarc. In CodeNarc, the stack overflow comes from: public void visitDeclarationExpression(DeclarationExpression expression) The super implementation is in CodeVisitorSupport. This method was present in 1.7 and 1.8.
        Hide
        blackdrag blackdrag added a comment -

        ok, the important difference between 1.7 and 1.8/1.9 is in 1.7 we have

          public super$2$visitDeclarationExpression(Lorg/codehaus/groovy/ast/expr/DeclarationExpression;)V
            ALOAD 0
            ALOAD 1
            INVOKESPECIAL org/codehaus/groovy/ast/CodeVisitorSupport.visitDeclarationExpression (Lorg/codehaus/groovy/ast/expr/DeclarationExpression;)V
            RETURN
            MAXSTACK = 2
            MAXLOCALS = 2

        and in 1.8 we have

          public super$3$visitDeclarationExpression(Lorg/codehaus/groovy/ast/expr/DeclarationExpression;)V
            ALOAD 0
            ALOAD 1
            INVOKESPECIAL org/codehaus/groovy/ast/ClassCodeVisitorSupport.visitDeclarationExpression (Lorg/codehaus/groovy/ast/expr/DeclarationExpression;)V
            RETURN
            MAXSTACK = 2
            MAXLOCALS = 2

        As can be clearly see there is a difference in what class is called as well as what number is used

        Show
        blackdrag blackdrag added a comment - ok, the important difference between 1.7 and 1.8/1.9 is in 1.7 we have public super $2$visitDeclarationExpression(Lorg/codehaus/groovy/ast/expr/DeclarationExpression;)V ALOAD 0 ALOAD 1 INVOKESPECIAL org/codehaus/groovy/ast/CodeVisitorSupport.visitDeclarationExpression (Lorg/codehaus/groovy/ast/expr/DeclarationExpression;)V RETURN MAXSTACK = 2 MAXLOCALS = 2 and in 1.8 we have public super $3$visitDeclarationExpression(Lorg/codehaus/groovy/ast/expr/DeclarationExpression;)V ALOAD 0 ALOAD 1 INVOKESPECIAL org/codehaus/groovy/ast/ClassCodeVisitorSupport.visitDeclarationExpression (Lorg/codehaus/groovy/ast/expr/DeclarationExpression;)V RETURN MAXSTACK = 2 MAXLOCALS = 2 As can be clearly see there is a difference in what class is called as well as what number is used
        Hide
        blackdrag blackdrag added a comment -

        From the trace I can tell the 1.7 method is not called. I assume the runtime looks only for the exact match, which for 1.8 is super$3 and not super$2, thus not finding the method and then placing the normal method for the call to super, when it should have caused an exception instead

        Show
        blackdrag blackdrag added a comment - From the trace I can tell the 1.7 method is not called. I assume the runtime looks only for the exact match, which for 1.8 is super$3 and not super$2, thus not finding the method and then placing the normal method for the call to super, when it should have caused an exception instead
        Hide
        blackdrag blackdrag added a comment -

        If my assumption with the fall back is right, then GROOVY-5285 is caused by the exact same logic

        Show
        blackdrag blackdrag added a comment - If my assumption with the fall back is right, then GROOVY-5285 is caused by the exact same logic
        Hide
        CÚdric Champeau added a comment -

        GROOVY-5285 is unrelated. Another stack overflow

        Show
        CÚdric Champeau added a comment - GROOVY-5285 is unrelated. Another stack overflow
        Hide
        CÚdric Champeau added a comment -

        Fixed by testing if other MOP methods exist with the same name but with a lower index.

        Show
        CÚdric Champeau added a comment - Fixed by testing if other MOP methods exist with the same name but with a lower index.

          People

          • Assignee:
            CÚdric Champeau
            Reporter:
            CÚdric Champeau
          • Votes:
            10 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: