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.

Sign up for GitHub

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

[Patch] FPE Issues in H5T #230

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[Patch] FPE Issues in H5T #230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
335 changes: 335 additions & 0 deletions recipe/3837.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,335 @@
From 0134f60fe1431887e7b318d60b38ae7cbd1ff485 Mon Sep 17 00:00:00 2001
From: Dana Robinson <[email protected]>
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 <[email protected]>
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 <ctype.h>
#include <errno.h>
#include <fcntl.h>
+#include <fenv.h>
#include <float.h>
#include <math.h>
#include <setjmp.h>

From 5e05dc1d679d71ce3a09c4072727b312d9421651 Mon Sep 17 00:00:00 2001
From: Dana Robinson <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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");
5 changes: 4 additions & 1 deletion recipe/meta.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{% set version = "1.14.3" %}
{% set maj_min_ver = ".".join(version.split(".")[:2]) %}
{% set build = 5 %}
{% set build = 6 %}

# recipe-lint fails if mpi is undefined
{% set mpi = mpi or 'nompi' %}
Expand Down Expand Up @@ -39,6 +39,9 @@ source:
url: https://support.hdfgroup.org/ftp/HDF5/releases/hdf5-{{ maj_min_ver }}/hdf5-{{ version }}/src/hdf5-{{ version }}.tar.gz
sha256: 09cdb287aa7a89148c1638dd20891fdbae08102cf433ef128fd345338aa237c7
patches:
# FPE issue in H5T
# https://github.com/HDFGroup/hdf5/pull/3837.patch
- 3837.patch
# Atomicity tests seem to fail for openmpi
# This seems to be a known bug
# https://github.com/HDFGroup/hdf5/issues/2196
Expand Down
Loading