[8u] RFR 8180032 Unaligned pointer dereference in ClassFileParser

[Dmitry Cherepanov]

Hi Aleksey,

Thanks for the review.

On 3/16/20 3:25 PM, Aleksey Shipilev wrote:
> Hi Dmitry,
>
> On 2/28/20 7:11 PM, Dmitry Cherepanov wrote:
>> Please review the following backport for 8u.
>>
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8180032
>> Original: http://hg.openjdk.java.net/jdk10/jdk10/hotspot/rev/4b93e1b1d5b7
>> Webrev: http://cr.openjdk.java.net/~dcherepanov/openjdk8u/8180032/webrev.v0/
> *) In classFileParser.hpp, I think the indents are a bit mismatched, should be indented under the
> opening parenthesis?
>
>   259   void* parse_exception_table(u4 code_length, u4 exception_table_length,
>   260                             TRAPS);
>
>   264   void* parse_localvariable_table(u4 code_length, u2 max_locals, u4 code_attribute_length,
>   265                                 u2* localvariable_table_length,
>   266                                 bool isLVTT, TRAPS);
>
>   267   void* parse_checked_exceptions(u2* checked_exceptions_length, u4 method_attribute_length,
>   268                                TRAPS);
Fixed.
>
> *) In bytecodeInterpreter.hpp, you removed the bytes*.hpp include block, but have not added #include
> for bytes.hpp?
In bytecodeInterpreter.hpp, it's a cleanup that removes unused includes. 
It's consistent with what's been done in JDK-8049325 
(http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/22b98ab2a69f#l86.1)
>
>
>> There was a cleanup in the hotspot files. It's done by
>> https://bugs.openjdk.java.net/browse/JDK-8140485 which seems to be too
>> massive to be backported and seems redundant. The patch for JDK-8140485
>> changed the context in 10 (e.g. added const modifier) so I manually
>> re-applied the changes.
> That makes sense to me.
>
>> This umbrella header was added in 9 by
>> https://bugs.openjdk.java.net/browse/JDK-8049325. This patch also seems
>> optional for this backport. So I backport'ed only relevant parts (added
>> bytes.hpp and updated #include directives in affected files) to 8u as a
>> part of this change.
> Ugh. So we need this to get shared Endian utility that is now in bytes.hpp. Seems like the lesser
> evil than backporting JDK-8049325. Maintainers might think differently, though.
>
Agree, complete backport of JDK-8049325 seems like overkill.

New webrev: 
http://cr.openjdk.java.net/~dcherepanov/openjdk8u/8180032/webrev.v1/

Thanks,

Dmitry