Fix cache image testing issues and re-enable testing (#6311)

Fix issue where chunked datasets could get setup with an incorrect
chunking index type in parallel HDF5

Fix issue where metadata cache images with an undefined address
and size of 0 couldn't be properly decoded

Fix issue where a flag in H5Cimage.c wasn't getting set correctly
for release builds of the library, leading to incorrect error
checking when reconstructing metadata cache entries

release_docs/CHANGELOG.md

@@ -123,6 +123,18 @@ We would like to thank the many HDF5 community members who contributed to this r
 
    The direct I/O VFD attempts to determine data alignment requirements for a file on file open to try and avoid extra work when data alignment isn't required. Depending on the file access flags used when opening a file, the VFD could incorrectly determine these requirements for either writes or reads, eventually leading to a possible EINVAL return value on write or read. This has been fixed by separately determining the requirements for writes and reads and being more conservative about trying to avoid data alignment requirements.
 
+### Fixed an issue with chunked datasets using the wrong index type with parallel HDF5
+
+   Fixed a bug in parallel HDF5 that would cause chunked datasets with fixed dimensions and without filters applied to use the "none" index type instead of the "fixed array" index type.
+
+### Fixed an issue with decoding metadata cache image superblock extension messages
+
+   Fixed a bug where loading of a metadata cache image superblock extension message would fail when the image had an undefined address and size of 0.
+
+### Fixed an issue with an incorrect file format validation check when decoding metadata cache entries
+
+   Fixed a bug where a flag in H5Cimage.c wasn't getting set correctly for release builds of HDF5, leading to incorrect error checking when reconstructing metadata cache entries.
+
 ## Java Library
 
 ## Configuration

src/H5Cimage.c

@@ -2744,11 +2744,11 @@ H5C__reconstruct_cache_entry(const H5F_t *f, H5C_t *cache_ptr, hsize_t *buf_size
     flags = *p++;
     if (flags & H5C__MDCI_ENTRY_DIRTY_FLAG)
         is_dirty = true;
+    if (flags & H5C__MDCI_ENTRY_IS_FD_PARENT_FLAG)
+        is_fd_parent = true;
 #ifndef NDEBUG /* only used in assertions */
     if (flags & H5C__MDCI_ENTRY_IN_LRU_FLAG)
         in_lru = true;
-    if (flags & H5C__MDCI_ENTRY_IS_FD_PARENT_FLAG)
-        is_fd_parent = true;
     if (flags & H5C__MDCI_ENTRY_IS_FD_CHILD_FLAG)
         is_fd_child = true;
 #endif

src/H5Dint.c

@@ -1326,19 +1326,6 @@ H5D__create(H5F_t *file, hid_t type_id, const H5S_t *space, hid_t dcpl_id, hid_t
     if (H5O_fill_set_version(file, &new_dset->shared->dcpl_cache.fill) < 0)
         HGOTO_ERROR(H5E_DATASET, H5E_CANTSET, NULL, "can't set latest version of fill value");
 
-    /* Check if the file driver would like to force early space allocation */
-    if (H5F_HAS_FEATURE(file, H5FD_FEAT_ALLOCATE_EARLY))
-        new_dset->shared->dcpl_cache.fill.alloc_time = H5D_ALLOC_TIME_EARLY;
-
-    /*
-     * Check if this dataset is going into a parallel file and set space allocation time.
-     * If the dataset has filters applied to it, writes to the dataset must be collective,
-     * so we don't need to force early space allocation. Otherwise, we force early space
-     * allocation to facilitate independent raw data operations.
-     */
-    if (H5F_HAS_FEATURE(file, H5FD_FEAT_HAS_MPI) && (new_dset->shared->dcpl_cache.pline.nused == 0))
-        new_dset->shared->dcpl_cache.fill.alloc_time = H5D_ALLOC_TIME_EARLY;
-
     /* Set the dataset's I/O operations */
     if (H5D__layout_set_io_ops(new_dset) < 0)
         HGOTO_ERROR(H5E_DATASET, H5E_CANTINIT, NULL, "unable to initialize I/O operations");
@@ -1352,6 +1339,19 @@ H5D__create(H5F_t *file, hid_t type_id, const H5S_t *space, hid_t dcpl_id, hid_t
     if (new_dset->shared->layout.version > H5O_layout_ver_bounds[H5F_HIGH_BOUND(file)])
         HGOTO_ERROR(H5E_DATASET, H5E_BADRANGE, NULL, "layout version out of bounds");
 
+    /* Check if the file driver would like to force early space allocation */
+    if (H5F_HAS_FEATURE(file, H5FD_FEAT_ALLOCATE_EARLY))
+        new_dset->shared->dcpl_cache.fill.alloc_time = H5D_ALLOC_TIME_EARLY;
+
+    /*
+     * Check if this dataset is going into a parallel file and set space allocation time.
+     * If the dataset has filters applied to it, writes to the dataset must be collective,
+     * so we don't need to force early space allocation. Otherwise, we force early space
+     * allocation to facilitate independent raw data operations.
+     */
+    if (H5F_HAS_FEATURE(file, H5FD_FEAT_HAS_MPI) && (new_dset->shared->dcpl_cache.pline.nused == 0))
+        new_dset->shared->dcpl_cache.fill.alloc_time = H5D_ALLOC_TIME_EARLY;
+
     /* Update the dataset's object header info. */
     if (H5D__update_oh_info(file, new_dset, new_dset->shared->dapl_id) < 0)
         HGOTO_ERROR(H5E_DATASET, H5E_CANTINIT, NULL, "can't update the metadata cache");

src/H5Ocache_image.c

@@ -116,12 +116,12 @@ H5O__mdci_decode(H5F_t *f, H5O_t H5_ATTR_UNUSED *open_oh, unsigned H5_ATTR_UNUSE
         HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding");
     H5F_DECODE_LENGTH(f, p, mesg->size);
 
-    if (mesg->addr >= (HADDR_UNDEF - mesg->size))
+    if (H5_addr_defined(mesg->addr) && mesg->size) {
-        HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, NULL, "address plus size overflows");
+        if (mesg->addr >= (HADDR_UNDEF - mesg->size))
-    if (mesg->addr == HADDR_UNDEF)
+            HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, NULL, "address plus size overflows");
-        HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, NULL, "address is undefined");
+        if ((mesg->addr + mesg->size) > H5F_get_eoa(f, H5FD_MEM_SUPER))
-    if ((mesg->addr + mesg->size) > H5F_get_eoa(f, H5FD_MEM_SUPER))
+            HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, NULL, "address plus size exceeds file eoa");
-        HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, NULL, "address plus size exceeds file eoa");
+    }
 
     /* Set return value */
     ret_value = (void *)mesg;

test/CMakeTests.cmake

@@ -473,29 +473,27 @@ if (NOT CYGWIN)
   endif ()
 endif ()
 
-if (TEST_CACHE_IMAGE)
+#-- Adding test for cache_image
-  #-- Adding test for cache_image
+add_test (
-  add_test (
+    NAME H5TEST-cache_image-clear-objects
-      NAME H5TEST-cache_image-clear-objects
+    COMMAND ${CMAKE_COMMAND} -E remove cache_image_test.h5
-      COMMAND ${CMAKE_COMMAND} -E remove cache_image_test.h5
+    WORKING_DIRECTORY ${HDF5_TEST_BINARY_DIR}/H5TEST
-      WORKING_DIRECTORY ${HDF5_TEST_BINARY_DIR}/H5TEST
+)
-  )
+set_tests_properties (H5TEST-cache_image-clear-objects PROPERTIES FIXTURES_SETUP clear_cache_image)
-  set_tests_properties (H5TEST-cache_image-clear-objects PROPERTIES FIXTURES_SETUP clear_cache_image)
+add_test (
-  add_test (
+    NAME H5TEST-cache_image-clean-objects
-      NAME H5TEST-cache_image-clean-objects
+    COMMAND ${CMAKE_COMMAND} -E remove cache_image_test.h5
-      COMMAND ${CMAKE_COMMAND} -E remove cache_image_test.h5
+    WORKING_DIRECTORY ${HDF5_TEST_BINARY_DIR}/H5TEST
-      WORKING_DIRECTORY ${HDF5_TEST_BINARY_DIR}/H5TEST
+)
-  )
+set_tests_properties (H5TEST-cache_image-clean-objects PROPERTIES FIXTURES_CLEANUP clear_cache_image)
-  set_tests_properties (H5TEST-cache_image-clean-objects PROPERTIES FIXTURES_CLEANUP clear_cache_image)
+add_test (NAME H5TEST-cache_image COMMAND $<TARGET_FILE:cache_image>)
-  add_test (NAME H5TEST-cache_image COMMAND $<TARGET_FILE:cache_image>)
+set_tests_properties (H5TEST-cache_image PROPERTIES
-  set_tests_properties (H5TEST-cache_image PROPERTIES
+    FIXTURES_REQUIRED clear_cache_image
-      FIXTURES_REQUIRED clear_cache_image
+    ENVIRONMENT "srcdir=${HDF5_TEST_BINARY_DIR}/H5TEST"
-      ENVIRONMENT "srcdir=${HDF5_TEST_BINARY_DIR}/H5TEST"
+    WORKING_DIRECTORY ${HDF5_TEST_BINARY_DIR}/H5TEST
-      WORKING_DIRECTORY ${HDF5_TEST_BINARY_DIR}/H5TEST
+)
-  )
+if ("H5TEST-cache_image" MATCHES "${HDF5_DISABLE_TESTS_REGEX}")
-  if ("H5TEST-cache_image" MATCHES "${HDF5_DISABLE_TESTS_REGEX}")
+  set_tests_properties (H5TEST-cache_image PROPERTIES DISABLED true)
-    set_tests_properties (H5TEST-cache_image PROPERTIES DISABLED true)
-  endif ()
 endif ()
 
 #-- Adding test for external_env

test/cache_image.c

@@ -529,7 +529,7 @@ open_hdf5_file(bool create_file, bool mdci_sbem_expected, bool read_only, bool s
     }
 
     if (show_progress)
-        fprintf(stdout, "%s: cp = %d.\n", fcn_name, cp++);
+        fprintf(stdout, "%s: cp = %d, pass = %d.\n", fcn_name, cp++, pass);
 
     /* create a file access property list. */
     if (pass) {
@@ -544,7 +544,7 @@ open_hdf5_file(bool create_file, bool mdci_sbem_expected, bool read_only, bool s
     }
 
     if (show_progress)
-        fprintf(stdout, "%s: cp = %d.\n", fcn_name, cp++);
+        fprintf(stdout, "%s: cp = %d, pass = %d.\n", fcn_name, cp++, pass);
 
     /* call H5Pset_libver_bounds() on the fapl_id */
     if (pass) {
@@ -557,7 +557,7 @@ open_hdf5_file(bool create_file, bool mdci_sbem_expected, bool read_only, bool s
     }
 
     if (show_progress)
-        fprintf(stdout, "%s: cp = %d.\n", fcn_name, cp++);
+        fprintf(stdout, "%s: cp = %d, pass = %d.\n", fcn_name, cp++, pass);
 
     /* get metadata cache image config -- verify that it is the default */
     if (pass) {
@@ -581,7 +581,7 @@ open_hdf5_file(bool create_file, bool mdci_sbem_expected, bool read_only, bool s
     }
 
     if (show_progress)
-        fprintf(stdout, "%s: cp = %d.\n", fcn_name, cp++);
+        fprintf(stdout, "%s: cp = %d, pass = %d.\n", fcn_name, cp++, pass);
 
     /* set metadata cache image fapl entry if indicated */
     if ((pass) && (set_mdci_fapl)) {
@@ -601,7 +601,7 @@ open_hdf5_file(bool create_file, bool mdci_sbem_expected, bool read_only, bool s
     }
 
     if (show_progress)
-        fprintf(stdout, "%s: cp = %d.\n", fcn_name, cp++);
+        fprintf(stdout, "%s: cp = %d, pass = %d.\n", fcn_name, cp++, pass);
 
     /* setup the persistent free space manager if indicated */
     if ((pass) && (config_fsm)) {
@@ -623,7 +623,7 @@ open_hdf5_file(bool create_file, bool mdci_sbem_expected, bool read_only, bool s
     }
 
     if (show_progress)
-        fprintf(stdout, "%s: cp = %d.\n", fcn_name, cp++);
+        fprintf(stdout, "%s: cp = %d, pass = %d.\n", fcn_name, cp++, pass);
 
     /* set evict on close if indicated */
     if ((pass) && (set_eoc)) {
@@ -636,7 +636,7 @@ open_hdf5_file(bool create_file, bool mdci_sbem_expected, bool read_only, bool s
     }
 
     if (show_progress)
-        fprintf(stdout, "%s: cp = %d.\n", fcn_name, cp++);
+        fprintf(stdout, "%s: cp = %d, pass = %d.\n", fcn_name, cp++, pass);
 
     /* open the file */
     if (pass) {
@@ -686,7 +686,7 @@ open_hdf5_file(bool create_file, bool mdci_sbem_expected, bool read_only, bool s
     }
 
     if (show_progress)
-        fprintf(stdout, "%s: cp = %d.\n", fcn_name, cp++);
+        fprintf(stdout, "%s: cp = %d, pass = %d.\n", fcn_name, cp++, pass);
 
     /* get a pointer to the files internal data structure and then
      * to the cache structure
@@ -705,7 +705,7 @@ open_hdf5_file(bool create_file, bool mdci_sbem_expected, bool read_only, bool s
     }
 
     if (show_progress)
-        fprintf(stdout, "%s: cp = %d.\n", fcn_name, cp++);
+        fprintf(stdout, "%s: cp = %d, pass = %d.\n", fcn_name, cp++, pass);
 
     /* verify expected metadata cache status */
 
@@ -724,7 +724,7 @@ open_hdf5_file(bool create_file, bool mdci_sbem_expected, bool read_only, bool s
     }
 
     if (show_progress)
-        fprintf(stdout, "%s: cp = %d.\n", fcn_name, cp++);
+        fprintf(stdout, "%s: cp = %d, pass = %d.\n", fcn_name, cp++, pass);
 
     if (pass) {
 
@@ -767,7 +767,7 @@ open_hdf5_file(bool create_file, bool mdci_sbem_expected, bool read_only, bool s
     }
 
     if (show_progress)
-        fprintf(stdout, "%s: cp = %d.\n", fcn_name, cp++);
+        fprintf(stdout, "%s: cp = %d, pass = %d.\n", fcn_name, cp++, pass);
 
     if ((pass) && (set_mdci_fapl)) {
 
@@ -781,7 +781,7 @@ open_hdf5_file(bool create_file, bool mdci_sbem_expected, bool read_only, bool s
     }
 
     if (show_progress)
-        fprintf(stdout, "%s: cp = %d.\n", fcn_name, cp++);
+        fprintf(stdout, "%s: cp = %d, pass = %d.\n", fcn_name, cp++, pass);
 
     if (pass) {
 
@@ -821,7 +821,7 @@ open_hdf5_file(bool create_file, bool mdci_sbem_expected, bool read_only, bool s
     }
 
     if (show_progress)
-        fprintf(stdout, "%s: cp = %d.\n", fcn_name, cp++);
+        fprintf(stdout, "%s: cp = %d, pass = %d.\n", fcn_name, cp++, pass);
 
     if (pass) {
 
@@ -831,7 +831,7 @@ open_hdf5_file(bool create_file, bool mdci_sbem_expected, bool read_only, bool s
     }
 
     if (show_progress)
-        fprintf(stdout, "%s: cp = %d -- exiting.\n", fcn_name, cp++);
+        fprintf(stdout, "%s: cp = %d, pass = %d -- exiting.\n", fcn_name, cp++, pass);
 
 } /* open_hdf5_file() */
 
@@ -6477,6 +6477,12 @@ cache_image_api_error_check_4(bool single_file_vfd)
             pass         = false;
             failure_mssg = "h5_fileaccess() failed.\n";
         }
+
+        if (H5Pset_libver_bounds(fapl_id, H5F_LIBVER_EARLIEST, H5F_LIBVER_V18) < 0) {
+
+            pass         = false;
+            failure_mssg = "H5Pset_libver_bounds() failed.\n";
+        }
     }
 
     if (show_progress)

testpar/CMakeTests.cmake

@@ -84,9 +84,6 @@ endforeach ()
 #  list (REMOVE_ITEM H5P_TESTS t_shapesame)
 #endif ()
 
-# do not test until new version is added
-list (REMOVE_ITEM H5P_TESTS t_cache_image)
-
 set (test_par_CLEANFILES
     t_cache_image_00.h5
     t_cache_image_01.h5

testpar/t_cache_image.c

@@ -3425,7 +3425,7 @@ smoke_check_1(MPI_Comm mpi_comm, MPI_Info mpi_info, int mpi_rank, int mpi_size)
 
     /* 13) Get the size of the file.  Verify that it is less
      *     than 20 KB.  Without deletions and persistent free
-     *     space managers, size size is about 30 MB, so this
+     *     space managers, file size is about 30 MB, so this
      *     is sufficient to verify that the persistent free
      *     space managers are more or less doing their job.
      *