From 0134f60fe1431887e7b318d60b38ae7cbd1ff485 Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Mon, 6 Nov 2023 14:25:45 -0800 Subject: [PATCH 1/7] Tidy DETECT_F macro in H5Tinit_float.c * unsigned char --> uint8_t * Accurate error messages * Remove unused parameter --- src/H5Tinit_float.c | 39 +++++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/src/H5Tinit_float.c b/src/H5Tinit_float.c index 3b9e127fe4e..30f6c0544ea 100644 --- a/src/H5Tinit_float.c +++ b/src/H5Tinit_float.c @@ -51,19 +51,23 @@ * Function: DETECT_F * * Purpose: This macro takes a floating point type like `double' and - * a base name like `natd' and detects byte order, mantissa - * location, exponent location, sign bit location, presence or - * absence of implicit mantissa bit, and exponent bias and - * initializes a detected_t structure with those properties. + * and detects byte order, mantissa location, exponent location, + * sign bit location, presence or absence of implicit mantissa + * bit, and exponent bias and initializes a detected_t structure + * with those properties. + * + * Note that these operations can raise floating-point + * exceptions and building with some compiler options + * (especially Fortran) can cause problems. *------------------------------------------------------------------------- */ -#define DETECT_F(TYPE, VAR, INFO) \ +#define DETECT_F(TYPE, INFO) \ do { \ - TYPE _v1, _v2, _v3; \ - unsigned char _buf1[sizeof(TYPE)], _buf3[sizeof(TYPE)]; \ - unsigned char _pad_mask[sizeof(TYPE)]; \ - unsigned char _byte_mask; \ - int _i, _j, _last = (-1); \ + TYPE _v1, _v2, _v3; \ + uint8_t _buf1[sizeof(TYPE)], _buf3[sizeof(TYPE)]; \ + uint8_t _pad_mask[sizeof(TYPE)]; \ + uint8_t _byte_mask; \ + int _i, _j, _last = -1; \ \ memset(&INFO, 0, sizeof(INFO)); \ INFO.size = sizeof(TYPE); \ @@ -81,7 +85,7 @@ _v1 = (TYPE)4.0L; \ H5MM_memcpy(_buf1, (const void *)&_v1, sizeof(TYPE)); \ for (_i = 0; _i < (int)sizeof(TYPE); _i++) \ - for (_byte_mask = (unsigned char)1; _byte_mask; _byte_mask = (unsigned char)(_byte_mask << 1)) { \ + for (_byte_mask = (uint8_t)1; _byte_mask; _byte_mask = (uint8_t)(_byte_mask << 1)) { \ _buf1[_i] ^= _byte_mask; \ H5MM_memcpy((void *)&_v2, (const void *)_buf1, sizeof(TYPE)); \ H5_GCC_CLANG_DIAG_OFF("float-equal") \ @@ -118,7 +122,7 @@ _v1 = (TYPE)1.0L; \ _v2 = (TYPE)-1.0L; \ if (H5T__bit_cmp(sizeof(TYPE), INFO.perm, &_v1, &_v2, _pad_mask, &(INFO.sign)) < 0) \ - HGOTO_ERROR(H5E_DATATYPE, H5E_CANTINIT, FAIL, "failed to detect byte order"); \ + HGOTO_ERROR(H5E_DATATYPE, H5E_CANTINIT, FAIL, "failed to determine sign bit"); \ \ /* Mantissa */ \ INFO.mpos = 0; \ @@ -126,12 +130,11 @@ _v1 = (TYPE)1.0L; \ _v2 = (TYPE)1.5L; \ if (H5T__bit_cmp(sizeof(TYPE), INFO.perm, &_v1, &_v2, _pad_mask, &(INFO.msize)) < 0) \ - HGOTO_ERROR(H5E_DATATYPE, H5E_CANTINIT, FAIL, "failed to detect byte order"); \ + HGOTO_ERROR(H5E_DATATYPE, H5E_CANTINIT, FAIL, "failed to determine mantissa"); \ INFO.msize += 1 + (unsigned)(INFO.imp ? 0 : 1) - INFO.mpos; \ \ /* Exponent */ \ - INFO.epos = INFO.mpos + INFO.msize; \ - \ + INFO.epos = INFO.mpos + INFO.msize; \ INFO.esize = INFO.sign - INFO.epos; \ \ _v1 = (TYPE)1.0L; \ @@ -466,7 +469,7 @@ H5T__init_native_float_types(void) /* Get the type's characteristics */ memset(&det, 0, sizeof(H5T_fpoint_det_t)); - DETECT_F(float, FLOAT, det); + DETECT_F(float, det); /* Allocate and fill type structure */ if (NULL == (dt = H5T__alloc())) @@ -497,7 +500,7 @@ H5T__init_native_float_types(void) /* Get the type's characteristics */ memset(&det, 0, sizeof(H5T_fpoint_det_t)); - DETECT_F(double, DOUBLE, det); + DETECT_F(double, det); /* Allocate and fill type structure */ if (NULL == (dt = H5T__alloc())) @@ -528,7 +531,7 @@ H5T__init_native_float_types(void) /* Get the type's characteristics */ memset(&det, 0, sizeof(H5T_fpoint_det_t)); - DETECT_F(long double, LDOUBLE, det); + DETECT_F(long double, det); /* Allocate and fill type structure */ if (NULL == (dt = H5T__alloc())) From b71b4426266bb7229aa42d48cb55bebcec4e3bd0 Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Tue, 7 Nov 2023 05:22:56 -0800 Subject: [PATCH 2/7] Disable fp exceptions in H5T init code The floating-point datatype initialization code can generate floating-point exceptions when it trips over signalling NaNs. The easiest fix for this is to ignore FE_INVALID exceptions while initializing the floating-point types. --- src/H5Tinit_float.c | 10 ++++++++++ src/H5private.h | 1 + 2 files changed, 11 insertions(+) diff --git a/src/H5Tinit_float.c b/src/H5Tinit_float.c index 30f6c0544ea..6a96650dbb9 100644 --- a/src/H5Tinit_float.c +++ b/src/H5Tinit_float.c @@ -461,10 +461,17 @@ H5T__init_native_float_types(void) { H5T_fpoint_det_t det; H5T_t *dt = NULL; + int fpe_flags = 0; herr_t ret_value = SUCCEED; FUNC_ENTER_PACKAGE + /* Turn off floating-point exceptions while initializing to avoid + * tripping over signalling NaNs while looking at "don't care" bits. + */ + fpe_flags = fegetexcept(); + fedisableexcept(FE_INVALID); + /* H5T_NATIVE_FLOAT */ /* Get the type's characteristics */ @@ -564,6 +571,9 @@ H5T__init_native_float_types(void) H5T_native_order_g = det.order; done: + /* Restore the original exceptions */ + feenableexcept(fpe_flags); + if (ret_value < 0) { if (dt != NULL) { dt->shared = H5FL_FREE(H5T_shared_t, dt->shared); diff --git a/src/H5private.h b/src/H5private.h index 14a0ac3225f..3aaa0d52453 100644 --- a/src/H5private.h +++ b/src/H5private.h @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include From 5e05dc1d679d71ce3a09c4072727b312d9421651 Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Tue, 7 Nov 2023 05:40:37 -0800 Subject: [PATCH 3/7] Remove IEEE changes from NAG Fortran --- config/linux-gnulibc1 | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/config/linux-gnulibc1 b/config/linux-gnulibc1 index 92f2be63df5..b4139ee1a58 100644 --- a/config/linux-gnulibc1 +++ b/config/linux-gnulibc1 @@ -183,10 +183,7 @@ case $FC_BASENAME in nagfor) F9XSUFFIXFLAG="" - # NOTE: The default is -ieee=stop, which will cause problems - # when the H5T module performs floating-point type - # introspection - AM_FCFLAGS="$AM_FCFLAGS -ieee=full" + AM_FCFLAGS="$AM_FCFLAGS" FSEARCH_DIRS="" # Production From 51296b11f19b5dedac1f2f3d0678a61086c23f7a Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Tue, 7 Nov 2023 06:51:53 -0800 Subject: [PATCH 4/7] Switch to non-GNU mechanism --- src/H5Tinit_float.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/H5Tinit_float.c b/src/H5Tinit_float.c index 6a96650dbb9..aa3f4633c49 100644 --- a/src/H5Tinit_float.c +++ b/src/H5Tinit_float.c @@ -459,9 +459,9 @@ H5T__set_precision(H5T_fpoint_det_t *d) herr_t H5_NO_UBSAN H5T__init_native_float_types(void) { + fenv_t saved_fenv; H5T_fpoint_det_t det; H5T_t *dt = NULL; - int fpe_flags = 0; herr_t ret_value = SUCCEED; FUNC_ENTER_PACKAGE @@ -469,8 +469,8 @@ H5T__init_native_float_types(void) /* Turn off floating-point exceptions while initializing to avoid * tripping over signalling NaNs while looking at "don't care" bits. */ - fpe_flags = fegetexcept(); - fedisableexcept(FE_INVALID); + if (feholdexcept(&saved_fenv) != 0) + HSYS_GOTO_ERROR(H5E_DATATYPE, H5E_CANTSET, FAIL, "can't save floating-point environment"); /* H5T_NATIVE_FLOAT */ @@ -571,8 +571,13 @@ H5T__init_native_float_types(void) H5T_native_order_g = det.order; done: - /* Restore the original exceptions */ - feenableexcept(fpe_flags); + /* Clear any FE_INVALID exceptions from NaN handling */ + if (feclearexcept(FE_INVALID) != 0) + HSYS_GOTO_ERROR(H5E_DATATYPE, H5E_CANTSET, FAIL, "can't clear floating-point exceptions"); + + /* Restore the original environment */ + if (feupdateenv(&saved_fenv) != 0) + HSYS_GOTO_ERROR(H5E_DATATYPE, H5E_CANTSET, FAIL, "can't restore floating-point environment"); if (ret_value < 0) { if (dt != NULL) { From b0afc409442884dea4e64111bf17948b2da7cc25 Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Tue, 7 Nov 2023 06:52:53 -0800 Subject: [PATCH 5/7] Remove NAG fortran IEEE note --- release_docs/RELEASE.txt | 6 ------ 1 file changed, 6 deletions(-) diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index 5dd36ea101c..e251f727bd4 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -1184,12 +1184,6 @@ Known Problems -O1 for the time being: https://forums.developer.nvidia.com/t/hdf5-no-longer-compiles-with-nv-23-9/269045. - IEEE standard arithmetic enables software to raise exceptions such as overflow, - division by zero, and other illegal operations without interrupting or halting - the program flow. The HDF5 C library intentionally performs these exceptions. - Therefore, the "-ieee=full" nagfor switch is necessary when compiling a program - to avoid stopping on an exception. - CMake files do not behave correctly with paths containing spaces. Do not use spaces in paths because the required escaping for handling spaces results in very complex and fragile build files. From 3572c9cc3a06840378cd3ad87198c908260dc0ef Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Tue, 7 Nov 2023 07:01:35 -0800 Subject: [PATCH 6/7] Add release note --- release_docs/RELEASE.txt | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index e251f727bd4..dfd70c3dbcc 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -408,6 +408,22 @@ Bug Fixes since HDF5-1.14.0 release =================================== Library ------- + - Suppressed floating-point exceptions in H5T init code + + The floating-point datatype initialization code in H5Tinit_float.c + could raise FE_INVALID exceptions while munging bits and performing + comparisons that might involve NaN. This was not a problem when the + initialization code was executed in H5detect at compile time (prior + to 1.14.3), but now that the code is executed at library startup + (1.14.3+), these exceptions can be caught by user code, as is the + default in the NAG Fortran compiler. + + Starting in 1.14.4, we now suppress floating-point exceptions while + initializing the floating-point types and clear FE_INVALID before + restoring the original environment. + + Fixes GitHub #3831 + - Fixed a file handle leak in the core VFD When opening a file with the core VFD and a file image, if the file From 198f1d0e3760d945691ee982cb31fdf286f3f1ea Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Tue, 7 Nov 2023 07:03:24 -0800 Subject: [PATCH 7/7] Fix typo --- src/H5Tinit_float.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/H5Tinit_float.c b/src/H5Tinit_float.c index aa3f4633c49..3213f00fece 100644 --- a/src/H5Tinit_float.c +++ b/src/H5Tinit_float.c @@ -467,7 +467,7 @@ H5T__init_native_float_types(void) FUNC_ENTER_PACKAGE /* Turn off floating-point exceptions while initializing to avoid - * tripping over signalling NaNs while looking at "don't care" bits. + * tripping over signaling NaNs while looking at "don't care" bits. */ if (feholdexcept(&saved_fenv) != 0) HSYS_GOTO_ERROR(H5E_DATATYPE, H5E_CANTSET, FAIL, "can't save floating-point environment");