Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue where H5Tset_fields does not account for datatype offsets #4061

Merged
merged 1 commit into from
Mar 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions release_docs/RELEASE.txt
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,16 @@ Bug Fixes since HDF5-1.14.0 release
===================================
Library
-------
- Fixed a bug where H5Tset_fields does not account for any offset
set for a floating-point datatype when determining if values set
for spos, epos, esize, mpos and msize make sense for the datatype

Previously, H5Tset_fields did not take datatype offsets into account
when determining if the values set make sense for the datatype.
This would cause the function to fail when the precision for a
datatype is correctly set such that the offset bits are not included.
This has now been fixed.

- Fixed H5Fget_access_plist so that it returns the file locking
settings for a file

Expand Down
6 changes: 3 additions & 3 deletions src/H5Tfloat.c
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,11 @@ H5Tset_fields(hid_t type_id, size_t spos, size_t epos, size_t esize, size_t mpos
dt = dt->shared->parent; /*defer to parent*/
if (H5T_FLOAT != dt->shared->type)
HGOTO_ERROR(H5E_DATATYPE, H5E_BADTYPE, FAIL, "operation not defined for datatype class");
if (epos + esize > dt->shared->u.atomic.prec)
if (epos + esize - dt->shared->u.atomic.offset > dt->shared->u.atomic.prec)
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "exponent bit field size/location is invalid");
if (mpos + msize > dt->shared->u.atomic.prec)
if (mpos + msize - dt->shared->u.atomic.offset > dt->shared->u.atomic.prec)
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "mantissa bit field size/location is invalid");
if (spos >= dt->shared->u.atomic.prec)
if (spos - dt->shared->u.atomic.offset >= dt->shared->u.atomic.prec)
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "sign location is not valid");

/* Check for overlap */
Expand Down
84 changes: 84 additions & 0 deletions test/dtypes.c
Original file line number Diff line number Diff line change
Expand Up @@ -4156,6 +4156,89 @@ test_query(void)
return 1;
}

/*-------------------------------------------------------------------------
* Function: test_set_fields_offset
*
* Purpose: Tests for a bug in H5Tset_fields in which the function
* didn't account for an offset set for a floating-point
* datatype when checking whether the values set for the
* floating-point fields make sense for the datatype.
*
* Return: Success: 0
* Failure: number of errors
*
*-------------------------------------------------------------------------
*/
static int
test_set_fields_offset(void)
{
hid_t tid = H5I_INVALID_HID;

if ((tid = H5Tcopy(H5T_NATIVE_FLOAT)) < 0) {
H5_FAILED();
printf("Can't copy datatype\n");
goto error;
}

/* Create a custom 128-bit floating-point datatype */
if (H5Tset_size(tid, 16) < 0) {
H5_FAILED();
printf("Can't set datatype size\n");
goto error;
}

/* We will have 7 bytes of MSB padding + 5 bytes of offset padding */
if (H5Tset_precision(tid, 116) < 0) {
H5_FAILED();
printf("Can't set datatype size\n");
goto error;
}

if (H5Tset_offset(tid, 5) < 0) {
H5_FAILED();
printf("Can't set datatype offset\n");
goto error;
}

if (H5Tset_ebias(tid, 16383) < 0) {
H5_FAILED();
printf("Can't set datatype exponent bias\n");
goto error;
}

/*
* Floating-point type with the following:
*
* - 5 bits of LSB padding (bits 0 - 4)
* - 100-bit mantissa starting at bit 5
* - 15-bit exponent starting at bit 105
* - 1 sign bit at bit 120
* - 7 bits of MSB padding
*/
if (H5Tset_fields(tid, 120, 105, 15, 5, 100) < 0) {
H5_FAILED();
printf("Can't set datatype's floating-point fields\n");
goto error;
}

if (H5Tclose(tid) < 0) {
H5_FAILED();
printf("Can't close datatype\n");
goto error;
}

return 0;

error:
H5E_BEGIN_TRY
{
H5Tclose(tid);
}
H5E_END_TRY;

return 1;
}

/*-------------------------------------------------------------------------
* Function: test_transient
*
Expand Down Expand Up @@ -9032,6 +9115,7 @@ main(void)
nerrors += test_detect();
nerrors += test_compound_1();
nerrors += test_query();
nerrors += test_set_fields_offset();
nerrors += test_transient(fapl);
nerrors += test_named(fapl);
nerrors += test_encode();
Expand Down
Loading