8234539: ArchiveRelocationTest.java failed: Archive mapping should always succeed

Reviewed-by: ccheung

src/hotspot/share/memory/filemap.cpp

@@ -1385,14 +1385,6 @@ MapArchiveResult FileMapInfo::map_regions(int regions[], int num_regions, char*
 
   }
 
-  DEBUG_ONLY(if (addr_delta == 0 && ArchiveRelocationMode == 1) {
-      // This is for simulating mmap failures at the requested address. We do it here (instead
-      // of MetaspaceShared::map_archives) so we can thoroughly test the code for failure handling
-      // (releasing all allocated resource, etc).
-      log_info(cds)("ArchiveRelocationMode == 1: always map archive(s) at an alternative address");
-      return MAP_ARCHIVE_MMAP_FAILURE;
-    });
-
   header()->set_mapped_base_address(header()->requested_base_address() + addr_delta);
   if (addr_delta != 0 && !relocate_pointers(addr_delta)) {
     return MAP_ARCHIVE_OTHER_FAILURE;
@@ -1446,12 +1438,14 @@ MapArchiveResult FileMapInfo::map_region(int i, intx addr_delta, char* mapped_ba
     MemTracker::record_virtual_memory_type((address)requested_addr, mtClassShared);
   }
 
-  if (MetaspaceShared::use_windows_memory_mapping() && addr_delta != 0) {
+  if (MetaspaceShared::use_windows_memory_mapping() && rs.is_reserved()) {
     // This is the second time we try to map the archive(s). We have already created a ReservedSpace
     // that covers all the FileMapRegions to ensure all regions can be mapped. However, Windows
     // can't mmap into a ReservedSpace, so we just os::read() the data. We're going to patch all the
     // regions anyway, so there's no benefit for mmap anyway.
     if (!read_region(i, requested_addr, size)) {
+      log_info(cds)("Failed to read %s shared space into reserved space at " INTPTR_FORMAT,
+                    shared_region_name[i], p2i(requested_addr));
       return MAP_ARCHIVE_OTHER_FAILURE; // oom or I/O error.
     }
   } else {
@@ -1459,7 +1453,8 @@ MapArchiveResult FileMapInfo::map_region(int i, intx addr_delta, char* mapped_ba
                                 requested_addr, size, si->read_only(),
                                 si->allow_exec());
     if (base != requested_addr) {
-      log_info(cds)("Unable to map %s shared space at required address.", shared_region_name[i]);
+      log_info(cds)("Unable to map %s shared space at " INTPTR_FORMAT,
+                    shared_region_name[i], p2i(requested_addr));
       _memory_mapping_failed = true;
       return MAP_ARCHIVE_MMAP_FAILURE;
     }
@@ -1468,7 +1463,7 @@ MapArchiveResult FileMapInfo::map_region(int i, intx addr_delta, char* mapped_ba
   si->set_mapped_base(requested_addr);
 
   if (!rs.is_reserved()) {
-    // When mapping on Windows with (addr_delta == 0), we don't reserve the address space for the regions
+    // When mapping on Windows for the first attempt, we don't reserve the address space for the regions
     // (Windows can't mmap into a ReservedSpace). In this case, NMT requires we call it after
     // os::map_memory has succeeded.
     assert(MetaspaceShared::use_windows_memory_mapping(), "Windows memory mapping only");

src/hotspot/share/memory/metaspaceShared.cpp

@@ -2147,6 +2147,19 @@ MapArchiveResult MetaspaceShared::map_archives(FileMapInfo* static_mapinfo, File
     MapArchiveResult dynamic_result = (static_result == MAP_ARCHIVE_SUCCESS) ?
                                      map_archive(dynamic_mapinfo, mapped_base_address, archive_space_rs) : MAP_ARCHIVE_OTHER_FAILURE;
 
+    DEBUG_ONLY(if (ArchiveRelocationMode == 1 && use_requested_addr) {
+      // This is for simulating mmap failures at the requested address. In debug builds, we do it
+      // here (after all archives have possibly been mapped), so we can thoroughly test the code for
+      // failure handling (releasing all allocated resource, etc).
+      log_info(cds)("ArchiveRelocationMode == 1: always map archive(s) at an alternative address");
+      if (static_result == MAP_ARCHIVE_SUCCESS) {
+        static_result = MAP_ARCHIVE_MMAP_FAILURE;
+      }
+      if (dynamic_result == MAP_ARCHIVE_SUCCESS) {
+        dynamic_result = MAP_ARCHIVE_MMAP_FAILURE;
+      }
+    });
+
     if (static_result == MAP_ARCHIVE_SUCCESS) {
       if (dynamic_result == MAP_ARCHIVE_SUCCESS) {
         result = MAP_ARCHIVE_SUCCESS;
@@ -2298,7 +2311,7 @@ static int dynamic_regions_count = 3;
 MapArchiveResult MetaspaceShared::map_archive(FileMapInfo* mapinfo, char* mapped_base_address, ReservedSpace rs) {
   assert(UseSharedSpaces, "must be runtime");
   if (mapinfo == NULL) {
-    return MAP_ARCHIVE_SUCCESS; // no error has happeed -- trivially succeeded.
+    return MAP_ARCHIVE_SUCCESS; // The dynamic archive has not been specified. No error has happened -- trivially succeeded.
   }
 
   mapinfo->set_is_mapped(false);