From 0c734cbe80010459509f122af507a969e80b00da Mon Sep 17 00:00:00 2001 From: jhendersonHDF Date: Fri, 27 Mar 2026 11:29:18 -0500 Subject: [PATCH] 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 | 12 +++++++++++ src/H5Cimage.c | 4 ++-- src/H5Dint.c | 26 +++++++++++------------ src/H5Ocache_image.c | 12 +++++------ test/CMakeTests.cmake | 44 +++++++++++++++++++-------------------- test/cache_image.c | 34 +++++++++++++++++------------- testpar/CMakeTests.cmake | 3 --- testpar/t_cache_image.c | 2 +- 8 files changed, 75 insertions(+), 62 deletions(-) diff --git a/release_docs/CHANGELOG.md b/release_docs/CHANGELOG.md index 85cba87957e..b88c4ec1e1b 100644 --- a/release_docs/CHANGELOG.md +++ b/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 diff --git a/src/H5Cimage.c b/src/H5Cimage.c index e82d76aedd6..b0d5ae8a456 100644 --- a/src/H5Cimage.c +++ b/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 diff --git a/src/H5Dint.c b/src/H5Dint.c index f277677bc48..63af3a844a7 100644 --- a/src/H5Dint.c +++ b/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"); diff --git a/src/H5Ocache_image.c b/src/H5Ocache_image.c index e7c6765d32e..bab1e3728d8 100644 --- a/src/H5Ocache_image.c +++ b/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)) - HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, NULL, "address plus size overflows"); - if (mesg->addr == HADDR_UNDEF) - HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, NULL, "address is undefined"); - 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"); + if (H5_addr_defined(mesg->addr) && mesg->size) { + if (mesg->addr >= (HADDR_UNDEF - mesg->size)) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, NULL, "address plus size overflows"); + 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"); + } /* Set return value */ ret_value = (void *)mesg; diff --git a/test/CMakeTests.cmake b/test/CMakeTests.cmake index e954e137b0c..999bc8dc994 100644 --- a/test/CMakeTests.cmake +++ b/test/CMakeTests.cmake @@ -473,29 +473,27 @@ if (NOT CYGWIN) endif () endif () -if (TEST_CACHE_IMAGE) - #-- Adding test for cache_image - add_test ( - NAME H5TEST-cache_image-clear-objects - COMMAND ${CMAKE_COMMAND} -E remove cache_image_test.h5 - WORKING_DIRECTORY ${HDF5_TEST_BINARY_DIR}/H5TEST - ) - set_tests_properties (H5TEST-cache_image-clear-objects PROPERTIES FIXTURES_SETUP clear_cache_image) - add_test ( - NAME H5TEST-cache_image-clean-objects - COMMAND ${CMAKE_COMMAND} -E remove cache_image_test.h5 - WORKING_DIRECTORY ${HDF5_TEST_BINARY_DIR}/H5TEST - ) - set_tests_properties (H5TEST-cache_image-clean-objects PROPERTIES FIXTURES_CLEANUP clear_cache_image) - add_test (NAME H5TEST-cache_image COMMAND $) - set_tests_properties (H5TEST-cache_image PROPERTIES - FIXTURES_REQUIRED clear_cache_image - ENVIRONMENT "srcdir=${HDF5_TEST_BINARY_DIR}/H5TEST" - WORKING_DIRECTORY ${HDF5_TEST_BINARY_DIR}/H5TEST - ) - if ("H5TEST-cache_image" MATCHES "${HDF5_DISABLE_TESTS_REGEX}") - set_tests_properties (H5TEST-cache_image PROPERTIES DISABLED true) - endif () +#-- Adding test for cache_image +add_test ( + NAME H5TEST-cache_image-clear-objects + COMMAND ${CMAKE_COMMAND} -E remove cache_image_test.h5 + WORKING_DIRECTORY ${HDF5_TEST_BINARY_DIR}/H5TEST +) +set_tests_properties (H5TEST-cache_image-clear-objects PROPERTIES FIXTURES_SETUP clear_cache_image) +add_test ( + NAME H5TEST-cache_image-clean-objects + COMMAND ${CMAKE_COMMAND} -E remove cache_image_test.h5 + WORKING_DIRECTORY ${HDF5_TEST_BINARY_DIR}/H5TEST +) +set_tests_properties (H5TEST-cache_image-clean-objects PROPERTIES FIXTURES_CLEANUP clear_cache_image) +add_test (NAME H5TEST-cache_image COMMAND $) +set_tests_properties (H5TEST-cache_image PROPERTIES + FIXTURES_REQUIRED clear_cache_image + ENVIRONMENT "srcdir=${HDF5_TEST_BINARY_DIR}/H5TEST" + WORKING_DIRECTORY ${HDF5_TEST_BINARY_DIR}/H5TEST +) +if ("H5TEST-cache_image" MATCHES "${HDF5_DISABLE_TESTS_REGEX}") + set_tests_properties (H5TEST-cache_image PROPERTIES DISABLED true) endif () #-- Adding test for external_env diff --git a/test/cache_image.c b/test/cache_image.c index bf44492bd23..bf8def20b52 100644 --- a/test/cache_image.c +++ b/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) diff --git a/testpar/CMakeTests.cmake b/testpar/CMakeTests.cmake index 1d003e84c62..a7d806b24a7 100644 --- a/testpar/CMakeTests.cmake +++ b/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 diff --git a/testpar/t_cache_image.c b/testpar/t_cache_image.c index fc6a02f9506..d0bdb7decf0 100644 --- a/testpar/t_cache_image.c +++ b/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. *