From f5f6975c2fc123a2c7f9d26cdbcda87d31ac4ea8 Mon Sep 17 00:00:00 2001 From: jhendersonHDF Date: Tue, 5 Mar 2024 08:23:34 -0600 Subject: Fix issue where H5Tset_fields does not account for datatype offsets (#4061) H5Tset_fields did not account for any offset in a floating-point datatype, causing it to fail when a datatype's precision is correctly set such that it doesn't include the offset bits. --- release_docs/RELEASE.txt | 10 ++++++ src/H5Tfloat.c | 6 ++-- test/dtypes.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 97 insertions(+), 3 deletions(-) diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index 9daebcd..12f39c7 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -489,6 +489,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 diff --git a/src/H5Tfloat.c b/src/H5Tfloat.c index 01a5607..9f7b4f7 100644 --- a/src/H5Tfloat.c +++ b/src/H5Tfloat.c @@ -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 */ diff --git a/test/dtypes.c b/test/dtypes.c index ac8697e..7aa3b17 100644 --- a/test/dtypes.c +++ b/test/dtypes.c @@ -4157,6 +4157,89 @@ error: } /*------------------------------------------------------------------------- + * 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 * * Purpose: Tests transient datatypes. @@ -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(); -- cgit v0.12