8279018: CRC calculation in CDS should not include _version and _head_size

Reviewed-by: iklam, ccheung

Author: yminqi

src/hotspot/share/cds/filemap.cpp

@@ -2404,8 +2404,8 @@ void FileMapHeader::set_as_offset(char* p, size_t *offset) {
 
 int FileMapHeader::compute_crc() {
   char* start = (char*)this;
-  // start computing from the field after _crc to end of base archive name.
-  char* buf = (char*)&(_generic_header._crc) + sizeof(_generic_header._crc);
+  // start computing from the field after _header_size to end of base archive name.
+  char* buf = (char*)&(_generic_header._header_size) + sizeof(_generic_header._header_size);
   size_t sz = header_size() - (buf - start);
   int crc = ClassLoader::crc32(0, buf, (jint)sz);
   return crc;

src/hotspot/share/include/cds.h

@@ -38,8 +38,8 @@
 #define NUM_CDS_REGIONS 7 // this must be the same as MetaspaceShared::n_regions
 #define CDS_ARCHIVE_MAGIC 0xf00baba2
 #define CDS_DYNAMIC_ARCHIVE_MAGIC 0xf00baba8
-#define CDS_GENERIC_HEADER_SUPPORTED_MIN_VERSION 12
-#define CURRENT_CDS_ARCHIVE_VERSION 12
+#define CDS_GENERIC_HEADER_SUPPORTED_MIN_VERSION 13
+#define CURRENT_CDS_ARCHIVE_VERSION 13
 
 typedef struct CDSFileMapRegion {
   int     _crc;               // CRC checksum of this region.
@@ -66,7 +66,7 @@ typedef struct CDSFileMapRegion {
 // a different version of HotSpot, so that we can automatically regenerate the archive as necessary.
 typedef struct GenericCDSFileMapHeader {
   unsigned int _magic;                    // identification of file type
-  int          _crc;                      // header crc checksum
+  int          _crc;                      // header crc checksum, start from _base_archive_name_offset
   int          _version;                  // CURRENT_CDS_ARCHIVE_VERSION of the jdk that dumped the this archive
   unsigned int _header_size;              // total size of the header, in bytes
   unsigned int _base_archive_name_offset; // offset where the base archive name is stored

src/hotspot/share/prims/whitebox.cpp

@@ -24,6 +24,7 @@
 
 #include "precompiled.hpp"
 #include <new>
+#include "cds.h"
 #include "cds/cdsConstants.hpp"
 #include "cds/filemap.hpp"
 #include "cds/heapShared.inline.hpp"
@@ -1962,6 +1963,24 @@ WB_ENTRY(jboolean, WB_IsSharingEnabled(JNIEnv* env, jobject wb))
   return UseSharedSpaces;
 WB_END
 
+WB_ENTRY(jint, WB_GetCDSGenericHeaderMinVersion(JNIEnv* env, jobject wb))
+#if INCLUDE_CDS
+  return (jint)CDS_GENERIC_HEADER_SUPPORTED_MIN_VERSION;
+#else
+  ShouldNotReachHere();
+  return (jint)-1;
+#endif
+WB_END
+
+WB_ENTRY(jint, WB_GetCDSCurrentVersion(JNIEnv* env, jobject wb))
+#if INCLUDE_CDS
+  return (jint)CURRENT_CDS_ARCHIVE_VERSION;
+#else
+  ShouldNotReachHere();
+  return (jint)-1;
+#endif
+WB_END
+
 WB_ENTRY(jboolean, WB_CDSMemoryMappingFailed(JNIEnv* env, jobject wb))
   return FileMapInfo::memory_mapping_failed();
 WB_END
@@ -2684,6 +2703,8 @@ static JNINativeMethod methods[] = {
                                                       (void*)&WB_GetMethodStringOption},
   {CC"getDefaultArchivePath",             CC"()Ljava/lang/String;",
                                                       (void*)&WB_GetDefaultArchivePath},
+  {CC"getCDSGenericHeaderMinVersion",     CC"()I",    (void*)&WB_GetCDSGenericHeaderMinVersion},
+  {CC"getCurrentCDSVersion",              CC"()I",    (void*)&WB_GetCDSCurrentVersion},
   {CC"isSharingEnabled",   CC"()Z",                   (void*)&WB_IsSharingEnabled},
   {CC"isShared",           CC"(Ljava/lang/Object;)Z", (void*)&WB_IsShared },
   {CC"isSharedInternedString", CC"(Ljava/lang/String;)Z", (void*)&WB_IsSharedInternedString },

test/hotspot/jtreg/runtime/cds/appcds/SharedArchiveConsistency.java

@@ -43,6 +43,8 @@ public class SharedArchiveConsistency {
     public static boolean shareAuto;       // true  == -Xshare:auto
                                            // false == -Xshare:on
 
+    private static int genericHeaderMinVersion;  // minimum supported CDS version
+    private static int currentCDSArchiveVersion; // current CDS version in java process
     // The following should be consistent with the enum in the C++ MetaspaceShared class
     public static String[] shared_region_name = {
         "rw",          // ReadWrite
@@ -104,6 +106,9 @@ public static void main(String... args) throws Exception {
             throw new RuntimeException("Arg must be 'on' or 'auto'");
         }
         shareAuto = args[0].equals("auto");
+        genericHeaderMinVersion = CDSArchiveUtils.getGenericHeaderMinVersion();
+        currentCDSArchiveVersion = CDSArchiveUtils.getCurrentCDSArchiveVersion();
+
         String jarFile = JarBuilder.getOrCreateHelloJar();
 
         // dump (appcds.jsa created)
@@ -158,7 +163,7 @@ public static void main(String... args) throws Exception {
         }
 
         // modify _magic, test should fail
-        System.out.println("\n2c. Corrupt _magic, should fail\n");
+        System.out.println("\n2b. Corrupt _magic, should fail\n");
         String modMagic = startNewArchive("modify-magic");
         copiedJsa = CDSArchiveUtils.copyArchiveFile(orgJsaFile, modMagic);
         CDSArchiveUtils.modifyHeaderIntField(copiedJsa, CDSArchiveUtils.offsetMagic(), -1);
@@ -170,23 +175,26 @@ public static void main(String... args) throws Exception {
         }
 
         // modify _version, test should fail
-        System.out.println("\n2d. Corrupt _version, should fail\n");
+        System.out.println("\n2c. Corrupt _version, should fail\n");
         String modVersion = startNewArchive("modify-version");
+        int version = currentCDSArchiveVersion + 1;
         copiedJsa = CDSArchiveUtils.copyArchiveFile(orgJsaFile, modVersion);
-        CDSArchiveUtils.modifyHeaderIntField(copiedJsa, CDSArchiveUtils.offsetVersion(), 0x3FFFFFFF);
+        CDSArchiveUtils.modifyHeaderIntField(copiedJsa, CDSArchiveUtils.offsetVersion(), version);
         output = shareAuto ? TestCommon.execAuto(execArgs) : TestCommon.execCommon(execArgs);
-        output.shouldContain("The shared archive file has the wrong version");
-        output.shouldNotContain("Checksum verification failed");
+        output.shouldContain("The shared archive file has the wrong version")
+              .shouldContain("_version expected: " + currentCDSArchiveVersion)
+              .shouldContain("actual: " + version);
         if (shareAuto) {
             output.shouldContain(HELLO_WORLD);
         }
 
-        System.out.println("\n2e. Corrupt _version, should fail\n");
+        System.out.println("\n2d. Corrupt _version, should fail\n");
         String modVersion2 = startNewArchive("modify-version2");
         copiedJsa = CDSArchiveUtils.copyArchiveFile(orgJsaFile, modVersion2);
-        CDSArchiveUtils.modifyHeaderIntField(copiedJsa, CDSArchiveUtils.offsetVersion(), 0x00000000);
+        version = genericHeaderMinVersion - 1;
+        CDSArchiveUtils.modifyHeaderIntField(copiedJsa, CDSArchiveUtils.offsetVersion(), version);
         output = shareAuto ? TestCommon.execAuto(execArgs) : TestCommon.execCommon(execArgs);
-        output.shouldContain("Cannot handle shared archive file version 0. Must be at least 12");
+        output.shouldContain("Cannot handle shared archive file version " + version + ". Must be at least " + genericHeaderMinVersion);
         output.shouldNotContain("Checksum verification failed");
         if (shareAuto) {
             output.shouldContain(HELLO_WORLD);

test/lib/jdk/test/lib/cds/CDSArchiveUtils.java

@@ -45,6 +45,9 @@
 
 // This class performs operations on shared archive file
 public class CDSArchiveUtils {
+    // Minimum supported CDS file header version
+    private static int genericHeaderMinVersion;    // CDS_GENERIC_HEADER_SUPPORTED_MIN_VERSION
+    private static int currentCDSArchiveVersion;   // CURRENT_CDS_ARCHIVE_VERSION
     // offsets
     private static int offsetMagic;                // offset of GenericCDSFileMapHeader::_magic
     private static int offsetCrc;                  // offset of GenericCDSFileMapHeader::_crc
@@ -82,6 +85,9 @@ public class CDSArchiveUtils {
         WhiteBox wb;
         try {
             wb = WhiteBox.getWhiteBox();
+            // genericHeaderMinVersion
+            genericHeaderMinVersion = wb.getCDSGenericHeaderMinVersion();
+            currentCDSArchiveVersion = wb.getCurrentCDSVersion();
             // offsets
             offsetMagic = wb.getCDSOffsetForName("GenericCDSFileMapHeader::_magic");
             offsetCrc = wb.getCDSOffsetForName("GenericCDSFileMapHeader::_crc");
@@ -116,6 +122,10 @@ public class CDSArchiveUtils {
     }
 
     // accessors
+    // minimum supported file header version
+    public static int getGenericHeaderMinVersion()  { return genericHeaderMinVersion;     }
+    // current CDS version
+    public static int getCurrentCDSArchiveVersion() { return currentCDSArchiveVersion;    }
     // offsets
     public static int offsetMagic()                 { return offsetMagic;                 }
     public static int offsetCrc()                   { return offsetCrc;                   }

test/lib/jdk/test/whitebox/WhiteBox.java

@@ -623,6 +623,8 @@ public Object getMethodOption(Executable method, String name) {
   }
 
   // Sharing & archiving
+  public native int     getCDSGenericHeaderMinVersion();
+  public native int     getCurrentCDSVersion();
   public native String  getDefaultArchivePath();
   public native boolean cdsMemoryMappingFailed();
   public native boolean isSharingEnabled();

test/lib/sun/hotspot/WhiteBox.java

@@ -624,6 +624,8 @@ public Object getMethodOption(Executable method, String name) {
   }
 
   // Sharing & archiving
+  public native int     getCDSGenericHeaderMinVersion();
+  public native int     getCurrentCDSVersion();
   public native String  getDefaultArchivePath();
   public native boolean cdsMemoryMappingFailed();
   public native boolean isSharingEnabled();