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

Use of FE_INVALID breaks WebAssembly builds of HDF5 1.14.5 #4952

Closed
LTLA opened this issue Oct 13, 2024 · 2 comments · Fixed by #4954
Closed

Use of FE_INVALID breaks WebAssembly builds of HDF5 1.14.5 #4952

LTLA opened this issue Oct 13, 2024 · 2 comments · Fixed by #4954
Assignees
Labels
Component - Build CMake, Autotools Priority - 3. Low 🔽 Code cleanup, small feature change requests, etc. Type - Bug / Bugfix Please report security issues to [email protected] instead of creating an issue on GitHub
Milestone

Comments

@LTLA
Copy link

LTLA commented Oct 13, 2024

Describe the bug

HDF5 1.14.5 cannot compile to WebAssembly as Emscripten does not support the FE_* macros.

Expected behavior

The compilation should succeed.

Platform (please complete the following information)

  • HDF5 1.14.5.
  • Ubuntu 22.04.5 LTS
  • Emscripten 3.1.68
  • Cmake
  • Multiple configure flags (see below)
  • No MPI

Additional context

Emscripten fails on the following chunk of code:

hdf5/src/H5Tinit_float.c

Lines 611 to 613 in 6b43197

/* 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");

with:

# ... skipping Cmake logs...
[ 78%] Building C object src/CMakeFiles/hdf5-static.dir/H5Tinit_float.c.o
/home/luna/Code/js/scran.js/extern/hdf5/demo/hdf5-1.14.5/src/H5Tinit_float.c:612:23: error: use of undeclared identifier 'FE_INVALID'
  612 |     if (feclearexcept(FE_INVALID) != 0)
      |                       ^
1 error generated.

This is because many of the FE_* macros are not defined by Emscripten (emscripten-core/emscripten#13678). While I'm not familiar with the intricacies of the C standard, some reading suggests that compliant implementations do not need to define these macros.

Currently I'm modifying the HDF5 source code directly so that the compilation can complete. Would you consider adding an #ifdef FE_INVALID around this block so that it compiles out of the box?

FWIW the MRE is:

# Unzip the downloaded tarball.
tar -xf hdf5-1.14.5.tar.gz

# Avoid problems from a random package.json in a parent directory.
echo "{}" > package.json 

# Assumes you have Emscripten installed, I'm using 3.1.68. Also
# I'm using Cmake 3.27.1, but I don't think it really matters.
emcmake cmake \
    -S hdf5-1.14.5 \
    -B build-1.14.5 \
    -DCMAKE_BUILD_TYPE=Release \
    -DCMAKE_INSTALL_PREFIX=installed \
    -DBUILD_SHARED_LIBS=OFF \
    -DBUILD_TESTING=OFF \
    -DHDF5_BUILD_EXAMPLES=OFF \
    -DHDF5_BUILD_TOOLS=OFF \
    -DHDF5_BUILD_UTILS=OFF \
    -DHDF5_BUILD_CPP_LIB=ON \
    -DHDF5_ENABLE_Z_LIB_SUPPORT=ON \
    -DZLIB_USE_EXTERNAL=OFF \
    -DHDF5_ENABLE_SZIP_SUPPORT=OFF

# Reproduces the error.
cd build-1.14.5
emmake make
@bmaranville
Copy link

This is also the only usage of an FE_XX macro in the whole HDF5 codebase, as far as I can tell.

@mattjala mattjala added Component - Build CMake, Autotools Priority - 3. Low 🔽 Code cleanup, small feature change requests, etc. Type - Bug / Bugfix Please report security issues to [email protected] instead of creating an issue on GitHub labels Oct 14, 2024
@derobins
Copy link
Member

I think the suggested fix (an #ifdef FE_INVALID block around the error check/clear) is a valid solution.

@derobins derobins assigned derobins and unassigned hyoklee Oct 14, 2024
derobins added a commit to derobins/hdf5 that referenced this issue Oct 14, 2024
When we initialize the floating-point types at library startup, it's
possible to raise floating-point exceptions when we check which things
are supported. Normally, we clear these floating-point exceptions via
feclearexcept(FE_INVALID), but FE_INVALID may not be present on all
systems. Specifically, this was reported as being a problem when using
Emscripten 3.1.68 to compile HDF5 1.14.5 to WebAssembly.

We've added an #ifdef FE_INVALID block around the exception clearing
code to correct this.

Fixes HDFGroup#4952
@derobins derobins added this to the 2.0.0 milestone Oct 14, 2024
derobins added a commit that referenced this issue Oct 14, 2024
When we initialize the floating-point types at library startup, it's
possible to raise floating-point exceptions when we check which things
are supported. Normally, we clear these floating-point exceptions via
feclearexcept(FE_INVALID), but FE_INVALID may not be present on all
systems. Specifically, this was reported as being a problem when using
Emscripten 3.1.68 to compile HDF5 1.14.5 to WebAssembly.

We've added an #ifdef FE_INVALID block around the exception clearing
code to correct this.

Fixes #4952
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component - Build CMake, Autotools Priority - 3. Low 🔽 Code cleanup, small feature change requests, etc. Type - Bug / Bugfix Please report security issues to [email protected] instead of creating an issue on GitHub
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants