Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8273152

Refactor CDS FileMapHeader loading code

XMLWordPrintable

    • Icon: Enhancement Enhancement
    • Resolution: Fixed
    • Icon: P4 P4
    • 18
    • 18
    • hotspot
    • b19

      There are several issues with the CDS FileMapHeader loading code:

      1. FileMapInfo::get_base_archive_name_from_header() does not check CRC. As a
         result, if the header is corrupt, it may return an invalid path.

      2. FileMapHeader::compute_crc() does not cover the base_archive_name. As a result,
         corruption of this part of the header cannot be detected with
         -XX:+VerifySharedSpaces.

      3. Currently, the header has a fixed part (sizeof(FileMapHeader)) plus a variable
         part (the base_archive_name). Loading the variable parts require a separate
         lseek()/read() operation. This will become more complicated if we need to
         add more variable parts into this data structure in the future.

      4. JDK-8261455 (Automatically generate the CDS archive if necessary) needs to read
         the header of an archive generated by a different version of HotSpot. The
         current design does not allow that.

      We should restructure the header and refactor the loading operation to address
      the above issues:

      === proposed data structures ===

      // This portion of the archive file header must remain unchanged for
      // (_version >= 12). This makes it possible to read important information
      // from a CDS archive created by a different version of HotSpot, so that
      // we can automatically regenerate the archive as necessary.
      struct GenericCDSFileMapHeader {
        // identify file type
        unsigned int _magic;

        // header crc checksum
        int _crc;

        // CURRENT_CDS_ARCHIVE_VERSION of the JDK that dumped this archive
        int _version;

        // total size of the header, in bytes
        unsigned int _header_size;

        // If this is a static archive, this field is 0.
        // If this is a dynamic archive:
        // _base_archive_path_offset == 0:
        // The base archive is the default CDS archive.
        // _base_archive_path_offset > 0:
        // (((char*)this) + _base_archive_path_offset) points to
        // a 0-terminated string for the pathname of the base archive.
        unsigned int _base_archive_path_offset;

        // size of the base archive name including NULL terminator, or 0
        // if _base_archive_path_offset == 0.
        unsigned int _base_archive_name_size;
      };

      // This type is used by the Serviceability Agent to access the contents of
      // a memory-mapped CDS archive.
      struct CDSFileMapHeaderBase {
        // We cannot inherit from GenericCDSFileMapHeader as this type may be used
        // by both C and C++ code.
        GenericCDSFileMapHeader _generic_header;
        struct CDSFileMapRegion _space[NUM_CDS_REGIONS];
      };

      === proposed header loading sequence ===

      1. open file
      2. read sizeof(GenericCDSFileMapHeader)
      3. check _magic
      4. Read the full header into a malloc'ed buffer of _header_size bytes
      5. Check CRC
      6. Check the consistency of other fields (this must be done after CRC
         check -- if CRC fas failed, it's meaningless to read other fields as
         they may be corrupt).
      6.1 First check the fields in GenericCDSFileMapHeader
      6.2 if _version == CURRENT_CDS_ARCHIVE_VERSION, check the fields of
         the rest of the data structure.

            minqi Yumin Qi
            iklam Ioi Lam
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Created:
              Updated:
              Resolved: