RFR(XS) 8210523 runtime/appcds/cacheObject/DifferentHeapSizes.java crash

[Jiangli Zhou]

Hi Ioi,

The fix looks correct. Please rename 
start_address_with_archived_oop_encoding_mode() to 
start_address_with_relocated_oop_encoding_mode(). The original name 
might be a confusing factor that caused the bug (adding the delta to the 
result of start_address_with_archived_oop_encoding_mode()).

Some more comments will be beneficial for readers who are not familiar 
with the region relocation code. We should explain that the first 
HeapShared::init_narrow_oop_decoding() call sets the relocation decoding 
mode and all addresses decoded using it afterwards are relocated. The 
second HeapShared::init_narrow_oop_decoding() resets the relocation 
decoding mode and adjusts alignment.

Thanks,

Jiangi

On 9/9/18 4:21 PM, Ioi Lam wrote:
> https://bugs.openjdk.java.net/browse/JDK-8210523
> http://cr.openjdk.java.net/~iklam/jdk12/8210523-DifferentHeapSizes-crash.v01/ 
>
>
> The bug is here:
>
>    address relocated_strings_bottom = 
> start_address_with_archived_oop_encoding_mode(si);
> -  if (!is_aligned(relocated_strings_bottom + delta, 
> HeapRegion::GrainBytes)) {
> +  if (!is_aligned(relocated_strings_bottom, HeapRegion::GrainBytes)) {
>
> relocated_strings_bottom was already relocated, so you don't need the 
> "+ delta"
> here. The bug was hidden because in all of our test runs, delta had been
> a multiple of HeapRegion::GrainBytes, so that didn't affect the result 
> of the
> is_align() call.
>
> Then, just by chance (probably due to Address Space Layout Randomization
> on MacOS), we have a delta / GrainBytes == (29509025792 / 4194304.0) = 
> 7035.5,
> so the bug is uncovered.
>
> Thanks
> - Ioi
>
>