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

[David Holmes]

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.

Thanks,
David

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