Skip to content

Commit

Permalink
Fix error when overwriting an indirectly nested vlen with a shorter s…
Browse files Browse the repository at this point in the history 
…equence (HDFGroup#4140)
  • Loading branch information
@fortnern
fortnern authored Mar 15, 2024
1 parent 92d1463 commit 40e1c6d
Show file tree
Hide file tree
Showing 3 changed files with 243 additions and 2 deletions.
10 changes: 10 additions & 0 deletions release_docs/RELEASE.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1543,6 +1550,9 @@ Known Problems
in the HDF5 source. Please report any new problems found to
[email protected].

File space may not be released when overwriting or deleting certain nested
variable length or reference types.


CMake vs. Autotools installations
=================================
Expand Down
65 changes: 63 additions & 2 deletions src/H5Tconv.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3148,6 +3148,67 @@ H5T__conv_enum_numeric(H5T_t *src, H5T_t *dst, H5T_cdata_t *cdata,
FUNC_LEAVE_NOAPI(ret_value)
} /* 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
*
Expand Down Expand Up @@ -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 */
Expand Down
170 changes: 170 additions & 0 deletions test/dsets.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15716,6 +15716,174 @@ test_0sized_dset_metadata_alloc(hid_t fapl_id)
return FAIL;
} /* 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
*
Expand Down Expand Up @@ -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 */
Expand Down

0 comments on commit 40e1c6d

Please sign in to comment.