llnl / llnl/zfp

Undefined behaviour crash on NaN/inf/-inf input

Open
#242 9 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C++
Stars
885
Forks
181
PR merge metrics
No merged PRs in 30d

Description

Sorry another UB bug:

I'm aware that zfp doesn't currently support round-tripping non-finite values, but it would be good if it didn't crash hard on NaN/inf/-inf input, in particular since client libraries like xarray / zarr will often pad data with NaNs, sometimes at a fairly low level in the library e.g. to achieve uniform chunk sizes before writing to zarr.

As in https://github.com/LLNL/zfp/issues/241, with clang I get SIGILL crashes, and with UBSAN enabled it's attributed to, e.g.:

third_party/zfp/src/template/encodef.c:57:17: runtime error: nan is outside the range of representable values of type 'int'
    #0 0x55cf45e3edf7 in zfp_encode_block_float_4 third_party/zfp/src/template/encodef.c:98
    #1 0x55cf45e3f916 in zfp_encode_partial_block_strided_float_4 third_party/zfp/src/template/encode4.c:88:10
    #2 0x55cf45e480bc in compress_strided_float_4 third_party/zfp/src/template/compress.c:105:13
    #3 0x55cf45e464b2 in zfp_compress third_party/zfp/src/zfp.c:1116:3
...
SUMMARY: UndefinedBehaviorSanitizer: float-cast-overflow third_party/zfp/src/template/encodef.c:57:17

Assuming we want to allow some default conversion to happen silently, this can be worked around via -fno-strict-float-cast-overflow -fno-sanitize=float-cast-overflow, but I'm reluctant to enable these too widely in case it masks some other bug.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reproducing the NaN, inf, and -inf cases with UBSAN, then inspect third_party/zfp/src/template/encodef.c around line 57 and the callers in encode4.c and compress.c. Confirm the behavior through zfp_compress and define completion as avoiding the reported float-cast-overflow and SIGILL crashes, with the intended handling of non-finite values agreed before implementation.

Written by the indexing model from the issue text.

Assessment

Tech stack
c
Domain
data
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.