RFR(S) 8244536 cds/DeterministicDump.java failed: File content different

[Thomas Stüfe]

Hi Ioi,

thanks for explaining. I understand now, and your patch looks good to me.

Cheers, Thomas


On Thu, May 14, 2020 at 1:29 AM Ioi Lam <ioi.lam at oracle.com> wrote:

>
>
> On 5/13/20 11:03 AM, Thomas Stüfe wrote:
>
> Hi Ioi,
>
> Could is_heap_object_archiving_allowed() be different between runtime and
> dumptime?
> If yes, would it make sense to explicitly reserve values for "header field
> is invalid"? Since zero can be ambiguous _narrow_oop_mode=0 is Unscaled).
>
>
> HI Thomas,
>
> is_heap_object_archiving_allowed()  can be different between run time and
> runtime:
>
>   static bool is_heap_object_archiving_allowed() {
>     CDS_JAVA_HEAP_ONLY(return (UseG1GC && UseCompressedOops &&
> UseCompressedClassPointers);)
>     NOT_CDS_JAVA_HEAP(return false;)
>   }
>
> However, if it was false during dump time, the CDS archive will have no
> heap regions so those 4 fields will never be accessed (they are accessed
> only when you have an archived heap region to map).
>
> Before my change, 3 of the 4 fields are already zeros. This patch only
> makes the 4th field (_heap_end) into a zero, so the header has
> deterministic contents.
>
> Thanks
> - Ioi
>
> ..Thomas
>
> On Wed, May 13, 2020 at 7:52 PM Ioi Lam <ioi.lam at oracle.com> wrote:
>
>>
>>
>> On 5/13/20 10:42 AM, Yumin Qi wrote:
>> > HI, Ioi
>> >
>> >   I have a question here:
>> >
>> >    _version = CURRENT_CDS_ARCHIVE_VERSION;
>> >    _alignment = alignment;
>> >    _obj_alignment = ObjectAlignmentInBytes;
>> >    _compact_strings = CompactStrings;
>> > + if (HeapShared::is_heap_object_archiving_allowed()) {
>> >    _narrow_oop_mode = CompressedOops::mode();
>> >    _narrow_oop_base = CompressedOops::base();
>> >    _narrow_oop_shift = CompressedOops::shift();
>> > + _heap_end = CompressedOops::end();
>> > + }
>> >    _compressed_oops = UseCompressedOops;
>> >    _compressed_class_ptrs = UseCompressedClassPointers;
>> >    _max_heap_size = MaxHeapSize;
>> >    _narrow_klass_shift = CompressedKlassPointers::shift();
>> > - if (HeapShared::is_heap_object_archiving_allowed()) {
>> > - _heap_end = CompressedOops::end();
>> > - }
>> >
>> > _heap_end seems not calculated if heap object not archived.
>> >
>>
>> Hi Yumin,
>>
>> Thanks for your comment.
>>
>> The header is cleared to zeros in here:
>>
>> FileMapInfo::FileMapInfo(bool is_static) {
>>    ...
>>    _header = (FileMapHeader*)os::malloc(header_size, mtInternal);
>>    memset((void*)_header, 0, header_size);
>>
>> So if heap objects are not archived, _narrow_oop_mode, _narrow_oop_base,
>> _narrow_oop_shift and _heap_end will all be zeros. These fields will not
>> be used during runtime.
>>
>> Thanks
>> - Ioi
>>
>>
>> >
>> >
>> > Thanks
>> > Yumin
>> >
>> > On 5/11/20 2:00 PM, Ioi Lam wrote:
>> >> https://bugs.openjdk.java.net/browse/JDK-8244536
>> >>
>> http://cr.openjdk.java.net/~iklam/jdk15/8244536-DeterministicDump-fails-on-windows.v01/
>> >>
>> >>
>> >> I fixed 2 issues that are likely to happen on Windows due to
>> >> it aggressive ASLR policy (address space layout randomization).
>> >> These would cause the CDS archive to have non-deterministic
>> >> contents:
>> >>
>> >> [1] Don't save the _narrow_oop_{mode,shift,base} header fields
>> >>     for platforms (including Windows) that don't support heap
>> >>     archiving, as these fields would be useless anyway.
>> >>
>> >>     This was the cause of the failure cases in the bug report.
>> >>
>> >> [2] Fixed a hashtable that would lay out differently if
>> >>     the archive was relocated during -Xshare:dump. This is also
>> >>     likely to happen on Windows but is not as frequent as #1.
>> >>
>> >>     Added test case for [2] in DeterministicDump.java (using
>> >>     -XX:ArchiveRelocationMode=1).
>> >>
>> >>
>> >> Test on mach5 with tiers 1/2/3.
>> >>
>> >> Thanks
>> >> - Ioi
>> >
>>
>>
>