Invalid reordering in ClassReader

[Werner Dietl]

Ah, there is no test case for this. I'm not sure how to test for this
- you want to make sure that ClassReader/ClassWriter are consistent.
This particular re-ordering mistake might get unnoticed if the bytes
happen to align in both orderings.

Why did the changeset that introduced the bug go in without test case?

Could you at least add the issue that this problem doesn't get lost?

cu, WMD.


On Tue, Feb 25, 2014 at 10:54 AM, Eric McCorkle
<eric.mccorkle at oracle.com> wrote:
> I've been working on other patches.  The only reason I haven't put this
> in already is that I'd need to also develop a test for it.  Is there a
> test for this in the type annotations repo?  If so, I could commit it to
> the main repository.
>
> On 02/25/14 10:38, Werner Dietl wrote:
>> Eric,
>>
>> On Thu, Feb 13, 2014 at 11:46 AM, Eric McCorkle
>> <eric.mccorkle at oracle.com> wrote:
>>> I'll open one.
>>>
>>> Sent from my iPhone
>>
>> What is the status of this?
>> The fix is already in the type-annotations repository.
>>
>> cu, WMD.
>>
>>
>>>> On Feb 13, 2014, at 11:43 AM, Werner Dietl <wdietl at gmail.com> wrote:
>>>>
>>>> Any thoughts on this?
>>>> Should I open an issue?
>>>>
>>>> cu, WMD.
>>>>
>>>>> On Fri, Feb 7, 2014 at 7:30 PM, Werner Dietl <wdietl at gmail.com> wrote:
>>>>> This changeset:
>>>>>
>>>>> http://hg.openjdk.java.net/jdk9/dev/langtools/rev/d607ae60772d
>>>>>
>>>>> contains a re-ordering of how the bytecode is read.
>>>>>
>>>>> --- a/src/share/classes/com/sun/tools/javac/jvm/ClassReader.java Sun
>>>>> Feb 02 12:12:01 2014 +0100
>>>>> +++ b/src/share/classes/com/sun/tools/javac/jvm/ClassReader.java Mon
>>>>> Feb 03 17:19:15 2014 -0500
>>>>>
>>>>>         // local variable
>>>>> -        case LOCAL_VARIABLE:
>>>>> -        // resource variable
>>>>> -        case RESOURCE_VARIABLE:
>>>>> -            int table_length = nextChar();
>>>>> +        case LOCAL_VARIABLE: {
>>>>> +            final int table_length = nextChar();
>>>>> +            final TypeAnnotationPosition position =
>>>>> +                TypeAnnotationPosition.localVariable(readTypePath());
>>>>> +
>>>>>             position.lvarOffset = new int[table_length];
>>>>>             position.lvarLength = new int[table_length];
>>>>>             position.lvarIndex = new int[table_length];
>>>>> @@ -1498,67 +1518,142 @@ public class ClassReader {
>>>>>                 position.lvarLength[i] = nextChar();
>>>>>                 position.lvarIndex[i] = nextChar();
>>>>>             }
>>>>> -            break;
>>>>> +            return position;
>>>>> +        }
>>>>>
>>>>> The code now reads the TypePath before the local variable information.
>>>>> This breaks the order in which the ClassWriter writes this information.
>>>>>
>>>>> This results in a ClassCastException in unrelated code, because the
>>>>> ClassReader gets confused about the class file.
>>>>>
>>>>> In type-annotations, I changed the code back:
>>>>>
>>>>> http://hg.openjdk.java.net/type-annotations/type-annotations/langtools/rev/7e9c983565ac
>>>>>
>>>>> and this resolves these exceptions.
>>>>>
>>>>> Is there a reason for this change? I think the previous implementation
>>>>> did the right ordering according to the spec.
>>>>>
>>>>> cu, WMD.
>>>>>
>>>>> --
>>>>> http://www.google.com/profiles/wdietl
>>>>
>>>>
>>>>
>>>> --
>>>> http://www.google.com/profiles/wdietl
>>
>>
>>



-- 
http://www.google.com/profiles/wdietl