RFR(s): Wrong alignment in CDS related Klass* reallocating code

[Calvin Cheung]

Hi Fedor,

Thanks for reporting the problem.
I don't see any bug filed for this problem so I've filed the following bug:

https://bugs.openjdk.java.net/browse/JDK-8224509

On 5/17/19, 6:51 AM, Fedor Burdun wrote:
> Hello all,
>
> I've found an inconsistency in CDS related Klass* reallocating code, in method
>
>          static void ArchiveCompactor::allocate(MetaspaceClosure::Ref* ref, bool read_only) (src/hotspot/share/memory/metaspaceShared.cpp:1122)
>
> the alignment = BytesPerWord has been used, that is fine for 64bit platforms, but may cause problems on 32bit ones.
> For example, in case of ARM gcc compiles accessors for u8/long long types using ldrd/strd instructions that requires a double-word alignment on ARMv5TE and older.
>
>
> Moreover the method (although it used only for compressed oops)
>
>          inline bool check_klass_alignment(Klass* obj) in src/hotspot/share/oops/klass.inline.hpp
Are you working on an older jdk release like 11u?
The above function has been renamed to check_alignment and is located in 
compressedOops.inline.hpp in the same directory in jdk13.
>
> checks classes to be aligned to KlassAlignment const, not to the BytesPerWord one.
>
> So I would like to suggest the fix below:
>
> diff --git a/src/hotspot/share/memory/metaspaceShared.cpp b/src/hotspot/share/memory/metaspaceShared.cpp
> --- a/src/hotspot/share/memory/metaspaceShared.cpp
> +++ b/src/hotspot/share/memory/metaspaceShared.cpp
> @@ -1123,7 +1123,7 @@
>       address obj = ref->obj();
>       int bytes = ref->size() * BytesPerWord;
>       char* p;
> -    size_t alignment = BytesPerWord;
> +    size_t alignment = KlassAlignmentInBytes;
>       char* oldtop;
>       char* newtop;
We think it'd be better if DumpRegion::allocate() always uses 8 as the 
alignment and the 'alignment' parameter could be removed.
Here's my proposed fix for jdk13:
http://cr.openjdk.java.net/~ccheung/8224509/webrev.00/
(Note that the dynamicArchive.cpp is new in jdk13.)

thanks,
Calvin
>
>
> Thanks,
> Fedor
>
>