RFR (XS) JDK-8051002: Incorrectly merged share/vm/classfile/classFileParser.cpp was pushed to 8u20

[Lois Foltan]

On 8/28/2014 6:56 AM, David Holmes wrote:
> On 28/08/2014 8:50 PM, Lois Foltan wrote:
>>
>> On 8/28/2014 6:42 AM, David Holmes wrote:
>>> Hi Lois,
>>>
>>> On 23/08/2014 4:14 AM, Lois Foltan wrote:
>>>> Hi Everyone,
>>>>
>>>> Please review the following fix targeted specifically for 8u.
>>>>
>>>> Webrev:
>>>>      http://cr.openjdk.java.net/~lfoltan/bug_jdk8051002/
>>>>
>>>> Bug: Incorrectly merged share/vm/classfile/classFileParser.cpp was
>>>> pushed to 8u20
>>>>      https://bugs.openjdk.java.net/browse/JDK-8051002
>>>>
>>>> Summary of fix:
>>>> Incorrect JDK 8u merge of classfile/classFileParser.cpp resulted in a
>>>> duplicate call to check the attribute_byte_length within
>>>> ClassFileParser::parse_classfile_bootstrap_methods_attribute(). The
>>>> first call did not reflect the fixed condition check and resulted 
>>>> in the
>>>> failure of
>>>> test/runtime/classFileParserBug/TestEmptyBootstrapMethodsAttr.java. 
>>>> This
>>>> change fixes the condition check and removes the duplicate call to
>>>> guarantee_property.
>>>
>>> From what I recall of this it was the second call site that was the
>>> correct one, so the first (with the wrong check) is what should have
>>> been removed.
>>
>> Hi David,
>>
>> Thanks for pointing that out.  Actually, there is a lot of history here,
>> but why I think the 8u merges went awry is because Harold and I both
>> changed the same area of code.  I changed the second call site in my
>> backport of 8u to fix the condition.  Harold, in a different fix, made a
>> change to move the guarantee_property call to the first call site.  So,
>> we do want the location of the call to be the first call site but of
>> course with the correct condition.  This is what this fix does and this
>> matches the code in JDK 9.  Hope that makes sense.
>
> Sorry - yes - I misread the diff from Harold's changeset in the bug 
> report.

No problem, thanks for reviewing it!
Lois

>
> Thanks,
> David
>
>> Thanks,
>> Lois
>>
>>>
>>> David
>>> -----
>>>
>>>> Testing:
>>>>      vm.quick.testlist, Hotspot's full jtreg tests
>>>>
>>>> Thanks,
>>>> Lois
>>>>
>>>>
>>>>
>>