RFR: 8273152: Store base archive path in CDSFileMapHeaderBase

Ioi Lam iklam at openjdk.java.net
Mon Sep 20 17:25:54 UTC 2021


On Mon, 20 Sep 2021 15:58:54 GMT, Yumin Qi <minqi at openjdk.org> wrote:

> Store base archive path offset in CDSFileMapHeaderBase (which should be invariant in evolving versions) so we can read the base archive name reliably from dynamic archive even with variant header sizes over versions.
> 
> tests: tier1-4
> 
> Thanks
> Yumin

Changes requested by iklam (Reviewer).

src/hotspot/share/include/cds.h line 67:

> 65:   int          _crc;             // header crc checksum
> 66:   int          _version;         // must be CURRENT_CDS_ARCHIVE_VERSION
> 67:   int _base_archive_path_offset; // This field is present with (_version >= 12). It's declaration

`CURRENT_CDS_ARCHIVE_VERSION` in this file should also be changed to 12.

Typo: `It's declaration` -> `Its declaration`

For safety, I think we need an extra field, and also `change _base_archive_path_offse` to unsigned:


  int          _version; 
+ unsigned int _header_size;       // total size of the header, in bytes
! unsigned int _base_archive_path_offset;
 ```

We need new test cases for safety checks:

1. _header_size is larger than the size of the archive file.
2. _base_archive_path_offset points is not zero for static archive.
3. For dynamic archive: _base_archive_path_offset points beyond _header_size
4. For dynamic archive: `(((char*)this) + _base_archive_path_offset)` does not point to a zero-terminated string. I.e., there is no zero byte between `(((char*)this) + _base_archive_path_offset)` and the end of the header.

Also, we should add something like this in the very beginning of the structure. E.g., we don't want someone to add a field in the middle of them.


// The declaration of the following 5 fields must never be changed
// in any future versions of HotSpot

-------------

PR: https://git.openjdk.java.net/jdk/pull/5584


More information about the hotspot-runtime-dev mailing list