From 40e1c6d078ddd6d4a3e149c242bb3290dd4ee2c1 Mon Sep 17 00:00:00 2001 From: Neil Fortner Date: Fri, 15 Mar 2024 11:26:56 -0500 Subject: Fix error when overwriting an indirectly nested vlen with a shorter sequence (#4140) --- release_docs/RELEASE.txt | 10 +++ src/H5Tconv.c | 65 +++++++++++++++++- test/dsets.c | 170 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 243 insertions(+), 2 deletions(-) diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index 688762e..2d8eb32 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -591,6 +591,13 @@ Bug Fixes since HDF5-1.14.0 release Library ------- + - Fixed error when overwriting certain nested variable length types + + Previously, when using a datatype that included a variable length type + within a compound or array within another variable length type, and + overwriting data with a shorter (top level) variable length sequence, an + error could occur. This has been fixed. + - Fixed asserts raised by large values of H5Pset_est_link_info() parameters If large values for est_num_entries and/or est_name_len were passed @@ -1543,6 +1550,9 @@ Known Problems in the HDF5 source. Please report any new problems found to help@hdfgroup.org. + File space may not be released when overwriting or deleting certain nested + variable length or reference types. + CMake vs. Autotools installations ================================= diff --git a/src/H5Tconv.c b/src/H5Tconv.c index 9a9d656..a37800b 100644 --- a/src/H5Tconv.c +++ b/src/H5Tconv.c @@ -3149,6 +3149,67 @@ done: } /* end H5T__conv_enum_numeric() */ /*------------------------------------------------------------------------- + * Function: H5T__conv_vlen_nested_free + * + * Purpose: Recursively locates and frees any nested VLEN components of + * complex data types (including COMPOUND). + * + * Return: Non-negative on success/Negative on failure. + * + *------------------------------------------------------------------------- + */ +static herr_t +H5T__conv_vlen_nested_free(uint8_t *buf, H5T_t *dt) +{ + herr_t ret_value = SUCCEED; /* Return value */ + + FUNC_ENTER_PACKAGE + + switch (dt->shared->type) { + case H5T_VLEN: + /* Pointer buf refers to VLEN data; free it (always reset tmp) */ + if ((*(dt->shared->u.vlen.cls->del))(dt->shared->u.vlen.file, buf) < 0) + HGOTO_ERROR(H5E_DATATYPE, H5E_CANTFREE, FAIL, "can't free nested vlen"); + break; + + case H5T_COMPOUND: + /* Pointer buf refers to COMPOUND data; recurse for each member. */ + for (unsigned i = 0; i < dt->shared->u.compnd.nmembs; ++i) + if (H5T__conv_vlen_nested_free(buf + dt->shared->u.compnd.memb[i].offset, + dt->shared->u.compnd.memb[i].type) < 0) + HGOTO_ERROR(H5E_DATATYPE, H5E_CANTFREE, FAIL, "can't free compound member"); + break; + + case H5T_ARRAY: + /* Pointer buf refers to ARRAY data; recurse for each element. */ + for (unsigned i = 0; i < dt->shared->u.array.nelem; ++i) + if (H5T__conv_vlen_nested_free(buf + i * dt->shared->parent->shared->size, + dt->shared->parent) < 0) + HGOTO_ERROR(H5E_DATATYPE, H5E_CANTFREE, FAIL, "can't free array data"); + break; + + case H5T_INTEGER: + case H5T_FLOAT: + case H5T_TIME: + case H5T_STRING: + case H5T_BITFIELD: + case H5T_OPAQUE: + case H5T_REFERENCE: + case H5T_ENUM: + /* These types cannot contain vl data */ + break; + + case H5T_NO_CLASS: + case H5T_NCLASSES: + default: + HGOTO_ERROR(H5E_DATATYPE, H5E_BADTYPE, FAIL, "invalid datatype class"); + } + +done: + FUNC_LEAVE_NOAPI(ret_value) +} /* H5T__conv_vlen_nested_free() */ + +/*------------------------------------------------------------------------- * Function: H5T__conv_vlen * * Purpose: Converts between VL datatypes in memory and on disk. @@ -3506,8 +3567,8 @@ H5T__conv_vlen(H5T_t *src, H5T_t *dst, H5T_cdata_t *cdata, const H5T_conv_ctx_t tmp = (uint8_t *)tmp_buf + seq_len * dst_base_size; for (u = seq_len; u < bg_seq_len; u++, tmp += dst_base_size) { - /* Delete sequence in destination location */ - if ((*(dst->shared->u.vlen.cls->del))(dst->shared->u.vlen.file, tmp) < 0) + /* Recursively free destination data */ + if (H5T__conv_vlen_nested_free(tmp, dst->shared->parent) < 0) HGOTO_ERROR(H5E_DATATYPE, H5E_CANTREMOVE, FAIL, "unable to remove heap object"); } /* end for */ diff --git a/test/dsets.c b/test/dsets.c index 5fa74f9..3203ed0 100644 --- a/test/dsets.c +++ b/test/dsets.c @@ -15717,6 +15717,174 @@ error: } /* end test_0sized_dset_metadata_alloc() */ /*------------------------------------------------------------------------- + * Function: test_downsize_vlen_scalar_dataset + * + * Purpose: Tests H5Dwrite on a scalar dataset containing a VLEN array + * of { double, C-string }. This causes special code to free + * the nested VLEN (in this case, C-string) allocations. + * + * Return: Success: 0 + * Failure: -1 + * + *------------------------------------------------------------------------- + */ +#define VLEN_DS_NAME "test_downsize_vlen_scalar_dataset" +#define VLEN_DS_DIM 100 +#define VLEN_DS_STRLEN 20 +#define VLEN_DS_STRING "vlen test string" +#define VLEN_DS_VALUE 0.12345678901234567890 +#define VLEN_DS_ARRAY_DIM1 3 +#define VLEN_DS_ARRAY_DIM2 5 + +typedef struct { + double value; + char *string[VLEN_DS_ARRAY_DIM1][VLEN_DS_ARRAY_DIM2]; +} vlen_ds_compound_file_t; + +typedef struct { + int padding1; + double value; + int padding2; + char *string[VLEN_DS_ARRAY_DIM1][VLEN_DS_ARRAY_DIM2]; + int padding3; +} vlen_ds_compound_memory_t; + +static herr_t +test_downsize_vlen_scalar_dataset(hid_t file) +{ + hid_t scalar_sid = -1; /* Scalar dataspace ID */ + hid_t string_tid = -1; /* VARIABLE string datatype ID */ + hid_t string_array_tid = -1; /* VARIABLE string array datatype ID */ + hid_t compound_file_tid = -1; /* COMPOUND datatype ID */ + hid_t compound_memory_tid = -1; /* COMPOUND datatype ID */ + hid_t vlen_compound_file_tid = -1; /* VARIABLE COMPOUND datatype ID */ + hid_t vlen_compound_memory_tid = -1; /* VARIABLE COMPOUND datatype ID */ + hid_t scalar_did = -1; /* SCALAR dataset ID */ + hvl_t vlen_compound_data; /* Top-level VLEN data */ + vlen_ds_compound_memory_t *compound_data = NULL; /* Contents of VLEN data */ + char common_string[VLEN_DS_STRLEN]; /* Common string contents */ + hsize_t array_dims[2] = {VLEN_DS_ARRAY_DIM1, VLEN_DS_ARRAY_DIM2}; + int i, dim1, dim2; /* Local index variables */ + + TESTING("H5Dwrite() on down-sized VLEN contents"); + + /* Allocate space for compound data */ + if (NULL == (compound_data = + (vlen_ds_compound_memory_t *)malloc(VLEN_DS_DIM * sizeof(vlen_ds_compound_memory_t)))) + TEST_ERROR; + + /* Create scalar dataspace */ + if ((scalar_sid = H5Screate(H5S_SCALAR)) < 0) + TEST_ERROR; + + /* Create datatype VLEN{COMPOUND{"value":H5T_NATIVE_DOUBLE, "string":H5T_C_S1|H5T_VARIABLE}} */ + /* Note: the memory and file structures must be different to invoke the bug @ H5Tconv.c:3323 */ + if ((string_tid = H5Tcopy(H5T_C_S1)) < 0) + TEST_ERROR; + if (H5Tset_size(string_tid, H5T_VARIABLE) < 0) + TEST_ERROR; + + if ((string_array_tid = H5Tarray_create2(string_tid, 2, array_dims)) < 0) + TEST_ERROR; + + if ((compound_file_tid = H5Tcreate(H5T_COMPOUND, sizeof(vlen_ds_compound_file_t))) < 0) + TEST_ERROR; + if (H5Tinsert(compound_file_tid, "value", HOFFSET(vlen_ds_compound_file_t, value), H5T_NATIVE_DOUBLE) < 0) + TEST_ERROR; + if (H5Tinsert(compound_file_tid, "string", HOFFSET(vlen_ds_compound_file_t, string), string_array_tid) < + 0) + TEST_ERROR; + if ((vlen_compound_file_tid = H5Tvlen_create(compound_file_tid)) < 0) + TEST_ERROR; + + if ((compound_memory_tid = H5Tcreate(H5T_COMPOUND, sizeof(vlen_ds_compound_memory_t))) < 0) + TEST_ERROR; + if (H5Tinsert(compound_memory_tid, "value", HOFFSET(vlen_ds_compound_memory_t, value), + H5T_NATIVE_DOUBLE) < 0) + TEST_ERROR; + if (H5Tinsert(compound_memory_tid, "string", HOFFSET(vlen_ds_compound_memory_t, string), + string_array_tid) < 0) + TEST_ERROR; + if ((vlen_compound_memory_tid = H5Tvlen_create(compound_memory_tid)) < 0) + TEST_ERROR; + + /* Create the scalar dataset of this data type */ + if ((scalar_did = H5Dcreate2(file, VLEN_DS_NAME, vlen_compound_file_tid, scalar_sid, H5P_DEFAULT, + H5P_DEFAULT, H5P_DEFAULT)) < 0) + TEST_ERROR; + + /* Setup the variable-length data. Note that if the double "value" field is set to 0.0, the bug will NOT + */ + /* occur because this is the data at offset zero of the element, and it then looks like a NULL VLEN data + */ + strcpy(common_string, VLEN_DS_STRING); + + for (i = 0; i < VLEN_DS_DIM; ++i) { + compound_data[i].value = VLEN_DS_VALUE; + for (dim1 = 0; dim1 < VLEN_DS_ARRAY_DIM1; ++dim1) { + for (dim2 = 0; dim2 < VLEN_DS_ARRAY_DIM2; ++dim2) { + compound_data[i].string[dim1][dim2] = common_string; + } + } + compound_data[i].padding1 = 0; + compound_data[i].padding2 = 0; + compound_data[i].padding3 = 0; + } + + /* Starting with the maximum size, progressively over-write the content of the dataset with smaller + * arrays. */ + /* Note: the bug in v1.8.14 is tripped on the second iteration, when 100 elements are over-written + * with 99. */ + for (i = VLEN_DS_DIM; i > 0; --i) { + vlen_compound_data.len = (size_t)i; + vlen_compound_data.p = compound_data; + if (H5Dwrite(scalar_did, vlen_compound_memory_tid, scalar_sid, scalar_sid, H5P_DEFAULT, + &vlen_compound_data) < 0) + TEST_ERROR; + } + + /* Close everything */ + if (H5Sclose(scalar_sid) < 0) + TEST_ERROR; + if (H5Tclose(string_tid) < 0) + TEST_ERROR; + if (H5Tclose(string_array_tid) < 0) + TEST_ERROR; + if (H5Tclose(compound_file_tid) < 0) + TEST_ERROR; + if (H5Tclose(vlen_compound_file_tid) < 0) + TEST_ERROR; + if (H5Tclose(compound_memory_tid) < 0) + TEST_ERROR; + if (H5Tclose(vlen_compound_memory_tid) < 0) + TEST_ERROR; + if (H5Dclose(scalar_did) < 0) + TEST_ERROR; + free(compound_data); + compound_data = NULL; + + PASSED(); + return 0; + +error: + H5E_BEGIN_TRY + { + H5Sclose(scalar_sid); + H5Tclose(string_tid); + H5Tclose(string_array_tid); + H5Tclose(compound_file_tid); + H5Tclose(vlen_compound_file_tid); + H5Tclose(compound_memory_tid); + H5Tclose(vlen_compound_memory_tid); + H5Dclose(scalar_did); + free(compound_data); + compound_data = NULL; + } + H5E_END_TRY; + return -1; +} /* end test_downsize_vlen_scalar_dataset() */ + +/*------------------------------------------------------------------------- * Function: main * * Purpose: Tests the dataset interface (H5D) @@ -15958,6 +16126,8 @@ main(void) nerrors += (test_farray_hdr_fd(envval, my_fapl) < 0 ? 1 : 0); nerrors += (test_bt2_hdr_fd(envval, my_fapl) < 0 ? 1 : 0); + nerrors += (test_downsize_vlen_scalar_dataset(file) < 0 ? 1 : 0); + if (H5Fclose(file) < 0) goto error; } /* end for new_format */ -- cgit v0.12