Annotations on return-overridden methods also end up on bridge methods

[Vicente-Arturo Romero-Zaldivar]

Hi Remi,

On 21/11/13 11:27, Remi Forax wrote:
> On 11/21/2013 12:20 PM, Vicente-Arturo Romero-Zaldivar wrote:
>> Hi Charles,
>>
>> Comments below.
>>
>> On 21/11/13 08:16, Charles Oliver Nutter wrote:
>>> Hey there!
>>>
>>> Sorry if this has been asked before, but I've been trying to get JRuby
>>> to compile properly on JDK8 javac and noticed a change.
>>>
>>> JDK8 generates bridge methods in cases where a child class overrides a
>>> superclass method with a more specific return type. This may have been
>>> present in JDK7 and lower, I'm not sure.
>>>
>>> What does seem to be different is that annotations attached to the
>>> hand-written override are also being attached to the bridge method.
>>>
>>> An example from JRuby...
>>>
>>> RubyBasicObject defines op_equal:
>>>
>>> public class RubyBasicObject ... {
>>> ...
>>>      @Override
>>>      public IRubyObject op_equal(ThreadContext context, IRubyObject 
>>> obj) {
>>>          return op_equal_19(context, obj);
>>>      }
>>> ...
>>> }
>>>
>>> RubyMethod extends RubyObject which extends RubyBasicObject, and it
>>> also overrides op_equal with a more specific return type:
>>>
>>> public class RubyMethod extends RubyObject {
>>> ...
>>>      @JRubyMethod(name = "==", required = 1)
>>>      @Override
>>>      public RubyBoolean op_equal(ThreadContext context, IRubyObject 
>>> other) {
>>> ...
>>> }
>>>
>>> We use the JRubyMethod annotation to generate stubs for binding the
>>> Ruby class's methods at boot time. These stubs are generated by
>>> walking declared methods looking for the JRubyMethod annotation.
>>>
>>> However, on JDK8, we get an error like this:
>>>
>>> Exception in thread "main" java.lang.ClassFormatError: Duplicate
>>> method name&signature in class file
>>> org/jruby/RubyMethod$INVOKER$i$op_equal
>>>
>>> This is because JDK8's generated IRubyObject-returning bridge method
>>> also has the JRubyMethod annotation.
>>>
>>> Here's the relevant entry in the constant pool:
>>>
>>> system ~/projects/jruby $ javap -v -cp core/target/classes/ 
>>> org.jruby.RubyMethod
>>> Classfile 
>>> /Users/headius/projects/jruby/core/target/classes/org/jruby/RubyMethod.class
>>> ...
>>>    #122 = Utf8               RuntimeVisibleAnnotations
>>>    #123 = Utf8               Lorg/jruby/anno/JRubyMethod;
>>>    #124 = Utf8               name
>>>
>>> Here's the hand-written method plus RuntimeVisibleAnnotations:
>>>
>>>    public org.jruby.RubyBoolean
>>> op_equal(org.jruby.runtime.ThreadContext,
>>> org.jruby.runtime.builtin.IRubyObject);
>>>      descriptor:
>>> (Lorg/jruby/runtime/ThreadContext;Lorg/jruby/runtime/builtin/IRubyObject;)Lorg/jruby/RubyBoolean; 
>>>
>>>      ...
>>>      RuntimeVisibleAnnotations:
>>>        0: #123(#124=[s#149],#150=I#137)
>>>
>>> And here's the bridge method, which also acquires this annotation:
>>>
>>>            public org.jruby.runtime.builtin.IRubyObject
>>> op_equal(org.jruby.runtime.ThreadContext,
>>> org.jruby.runtime.builtin.IRubyObject);
>>>              descriptor:
>>> (Lorg/jruby/runtime/ThreadContext;Lorg/jruby/runtime/builtin/IRubyObject;)Lorg/jruby/runtime/builtin/IRubyObject; 
>>>
>>>              flags: ACC_PUBLIC, ACC_BRIDGE, ACC_SYNTHETIC
>>>              ...
>>>              RuntimeVisibleAnnotations:
>>>                0: #123(#124=[s#149],#150=I#137)
>>>
>>> I'm not arguing that this is correct or incorrect behavior...merely
>>> that it is a change, and it may bite others the same way it has bitten
>>> us. I have patched our stub generator to skip bridge methods, but it
>>> feels kinda wrong.
>>
>> This is not a bug but intended behavior, I implemented this as a 
>> feature request, see [1]. Some users asked this as a nice to have 
>> feature :)
>>
>> [1] https://bugs.openjdk.java.net/browse/JDK-6695379
>
> while I agree that it's a feature not a bug, or even a bugfix if you 
> consider that without that fix you have a way to bypass
> JavaEE security frameworks that use annotations.
> Anyway, it has to be documented in the compatibility notes because 
> this change in the behavior
> may impact some annotations processor or runtime that use the 
> annotations.

Thanks for your suggestion, I have modified the bug entry accordingly.

Vicente

>
>>
>>>
>>> - Charlie
>>
>
> Rémi
>