Expected behavior for annotation property with duplicate value definition

[David Holmes]

Hi Rafael,

On 5/05/2021 8:02 pm, Rafael Winterhalter wrote:
> Hi David,
> yes, of course. However, the JVM already yields exceptions upon illegal 
> constructs, most of the time a class would fail to load through 
> verification. 

Yes but verification enforces JVMS rules not JLS rules. There are many 
things allowed in a classfile that cannot be expressed in the Java language.

> For annotations, there is a whole tree of exceptions such 
> as AnnotationTypeMismatchException but this one scenario with two values 
> for a single property does not seem to be handled but is silently 

Reflection can detect some ill-formedness of a classfile and re-apply 
Java language rules.

> ignored. I mainly wonder if this other illegal scenario should not be 
> handled in the same category of throwing an exception. To my knowledge, 
> there is no other annotation inconsistency that is silently suppressed.

Lets work with specifics. Here's an example description of a Deprecated 
attribute on method, as output by javap:

RuntimeVisibleAnnotations:
       0: #14(#15=s#16,#17=Z#18)
         java.lang.Deprecated(
           since="17"
           forRemoval=true
         )

I think the situation you are describing would show up as e.g.:

       0: #14(#15=s#16,#15=s#4,#17=Z#18)

where we "assign" the "since" value (cp entry #15) first with cp entry 
#16 and then with cp entry #4. Is this what you mean?

In terms of the RuntimeVisibleAnnotations attribute as defined by JVMS 
4.7.16 it doesn't state that the number of element_value pairs in an 
annotation structure has to match the number of elements expressed in 
the Java language for that annotation type - indeed no such number 
exists, it is implicitly defined by this array of element_value 
structures in the classfile. Nor does the JVMS require that 
element_value structures be unique within an annotation structure. So 
the VM would not reject such a classfile.

AFAICT the VM just processes things in order as it encounters them so 
the last value "assigned" is what gets recorded. Reflection will then 
return that value and there is nothing at all to tell it that anything 
might have been amiss in the original classfile.

David
-----

> Best regards, Rafael
> 
> Am Mi., 5. Mai 2021 um 11:55 Uhr schrieb David Holmes 
> <david.holmes at oracle.com <mailto:david.holmes at oracle.com>>:
> 
>     Hi Rafael,
> 
>     On 5/05/2021 6:53 pm, Rafael Winterhalter wrote:
>      > Hello,
>      >
>      > I was wondering if the current OpenJDK behavior should yield an
>     exception
>      > or if it is accidental and if so, if it should be altered to avoid
>      > surprising behavior. If an annotation:
>      >
>      > @interface Sample {
>      >    String v();
>      > }
>      >
>      > is added to a member where the property 'v' is assigned a value
>     twice, the
>      > last added value is returned by the reflection API and no error
>     is raised.
>      > I recently observed this for ASM-generated code where a value was
>     added
>      > twice and it led to a longer bug search, but technically this
>     could of
>      > course also happen when javac or other language compilers
>     generate code. I
>      > wonder therefore if this should rather yield an error or if this
>     behavior
>      > should be documented somewhere in case that the change would be
>     possibly
>      > too disruptive for existing code.
> 
>     I think you are describing generated bytecode that violates Java
>     language rules for Annotation types. That is perfectly legal. The VM's
>     notion of what an Annotation type is and how it must behave is much
>     broader than that of the Java language.
> 
>     Cheers,
>     David
> 
>      > I am happy to provide a patch to OpenJDK but wonder what
>     exception should
>      > be thrown when reading the property.
>      >
>      > Best regards, Rafael
>      >
>