Skip to content

Make an installed SZ3 usable through find_package, stop a crash in the HDF5 filter, and vendor Zstd - #150

Open
ayzk wants to merge 65 commits into
masterfrom
zstd-export-fix
Open

ayzk wants to merge 65 commits into
masterfrom
zstd-export-fix

Conversation

@ayzk

@ayzk ayzk commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator

An SZ3 that has been installed cannot be used through find_package(SZ3), and a source build
cannot be configured without network access. Nothing in this repository ever tried either: every
test reaches SZ3 through add_subdirectory or FetchContent, and neither reads SZ3Config.cmake.
So these have been shipping since at least v3.3.2, and the GROMACS merge request that links an
installed SZ3 (gromacs!5560) has been
stuck in Draft since November waiting on us — GROMACS 2027 replaces XTC with H5MD and reaches lossy
compression through our filter. Spack builds SZ3 the same way, so every Spack consumer met the same
wall.

90 files, +47,069 / −241. 64 of those files and +46,065 of those lines are the vendored Zstd
source; our own change is 26 files, +1,004 / −241. The three CMake files it touches end at 285
lines against master's 246.

find_package(SZ3) could not produce a working target

  • SZ3Targets.cmake named PkgConfig::ZSTD, an imported target that exists only inside the build
    that created it. Every consumer of an SZ3 built against a system Zstd — the default on Linux and
    macOS, and what Spack builds — died at generate time with The link interface of target "SZ3::SZ3" contains: PkgConfig::ZSTD but the target was not found.
  • An installed SZ3 exported only the namespaced names, so target_link_libraries(x PRIVATE hdf5sz3)
    — what GROMACS writes, and what works against an in-tree SZ3 — became -lhdf5sz3.

A config file must not end its caller's configure. find_package(HDF5 ... REQUIRED),
find_package(MPI COMPONENTS CXX REQUIRED) and find_package(GSL REQUIRED) all called
message(FATAL_ERROR) from inside a QUIET optional probe, taking down GMX_USE_SZ3=AUTO
GROMACS's default, whose contract is to fall back to its own copy. The MPI search should not have
existed: nm -u libhdf5sz3.so finds no MPI symbol, and a parallel HDF5 brings its own MPI through
its package config. Resolving MPI a second time only added an independently chosen implementation,
which put libmpi.so.12 and libmpi.so.40 in one binary
(gromacs#5505).

The install leaked the build machine into the consumer

  • ${HDF5_INCLUDE_DIRS} was PUBLIC with no $<BUILD_INTERFACE:> guard, so a consumer who found
    HDF5 1.10.6 compiled against the build machine's 2.2.0 headers.
  • The installed libhdf5sz3.so had no RUNPATH — ldd reported libhdf5.so.320 => not found.
  • find_dependency(OpenMP) without COMPONENTS CXX told a C-only consumer SZ3_FOUND=1, which
    then failed at link. An HDF5 filter is exactly what a C program links.

A crash in released code

set_SZ3_conf_to_H5 chose between H5Pset_filter and H5Pmodify_filter on H5Zfilter_avail(),
which answers whether the filter is registered with the library, not whether it is on this
property list. With HDF5_PLUGIN_PATH set it always answers yes, so a freshly created property
list went to H5Pmodify_filter and faulted inside HDF5:

#0  H5Z_modify ()  #1  H5P_modify_filter ()  #2  H5Pmodify_filter ()
#3  set_SZ3_conf_to_H5 ()  #4  main ()

Deterministic, and reproduced on HDF5 1.10.6, 1.14.6 and 2.2.0. It was introduced in v3.3.1:
v3.3.0 branched on H5Pget_filter_by_id(propertyList, ...), which asks the right question. v3.3.1,
v3.3.2 and this branch's parent all crash; v3.3.0 does not.

get_SZ3_conf_from_H5 had the same predicate and also ignored H5Pget_filter_by_id's return, so
reading from a list with no SZ3 filter silently overwrote the caller's Config with zeros while
reporting success.

Zstd is now a private dependency, vendored rather than downloaded

The bundled Zstd was fetched at configure time from a GitHub release URL, with no URL_HASH. That
build cannot be configured offline at all — the failure is Build step for zstdfetched failed
which rules out air-gapped clusters, most distribution build systems, and any reproducible build.

Zstd 1.5.6 is now vendored in tools/zstd/lib (lib/common, lib/compress, lib/decompress;
deprecated/, legacy/ and dictBuilder/ dropped, 39 compiled files down to 25, −29.8% on that
target's build time). It stays what it already was — a private static libsz3_zstd with hidden
symbols — and it no longer installs a header.

Lossless_zstd.hpp declares the four Simple-API functions it calls instead of including zstd.h,
so nothing SZ3 installs answers to zstd.h and a consumer needs no Zstd include directory. A
ctest compiles those declarations against the real header in both include orders and fails if they
drift.

Resolution stays pkg-config, as on master, falling back to the vendored copy;
SZ3_USE_BUNDLED_ZSTD=ON forces it. A consumer of a system-Zstd install resolves it with one
find_library, so nothing about the build machine's Zstd is written into the installed package.
Four cases that failed before now work:
an offline build with no Zstd present, a consumer against a Homebrew prefix, a consumer after the
Zstd prefix used at SZ3 build time was deleted, and an hdf5plugin-style direct compile with no Zstd
include directory on the command line.

1.5.6 decompresses 2–22% faster than the 1.4.5 it replaces on the payloads measured. The 32-bit
ZSTD_CURRENT_MAX defect (facebook/zstd#4129) cannot be reached from SZ3, which holds no
ZSTD_CCtx across calls; the reasoning and the condition that would expire it are recorded in
tools/zstd/CMakeLists.txt.

Smaller fixes

  • GSL removed. Its only consumer was Wavelet.hpp, which has no includer anywhere in the tree;
    PreprocessorInterface's only virtual is commented out, so it overrode nothing. It is also not a
    round trip — it pads to the next power of two, transforms all of it, and copies back only the
    original length. Keeping the header without the dependency would leave an installed public header
    that #includes <gsl/gsl_wavelet.h> and cannot compile, so both go.
  • A registration entry point (H5Z_SZ3_initialize/_finalize), a configurable plugin install
    directory, and a filter version string that h5dump -pH and h5ls -v now show.
  • Config::load rejects a negative dimension count. char's signedness is implementation
    defined; where it is signed, a cd_values byte of 0xF3 arrives as −13 and passed N > 4. The
    later guards did not stop it either: with bitWidth 0 the dim_bytes computation wraps to 0, so
    a negative N reached bytes2vector, which asked for 1.8e19 elements and threw a bare
    std::length_error with no diagnosis.
  • Config::load is no longer the first thing a foreign file meets. On read, the payload's magic
    and data version are checked before cd_values is parsed under the layout this build assumes. A
    v3.2.0, v3.2.1 or v3.3.0 file was previously refused by Config::load's own guards, with
    "invalid number of dimensions" — which reads as a corrupt file rather than a version mismatch. It
    now names both versions. The magic is a precondition rather than a requirement: a chunk the
    conf.num < 20 path stores raw has no header, and conf.num is not known until cd_values has
    been parsed, so files older than v3.2.0 carry no magic and are still refused by Config::load.
  • Three error paths no longer write to stdout. sz.hpp, SZImplOMP.hpp and Iterator.hpp each
    emitted the same error two or three times, once of them to stdout. Inside the HDF5 filter that
    lands in whatever the user redirected, so h5dump -d /ds f.h5 > out.txt got SZ3's diagnostic
    written into the data file. The exception carries the whole message; the filter turns what()
    into an entry in HDF5's error stack.
  • dsz3FromHDF5 checks the read before using it. H5Dread's status was stored and ignored: on
    failure the tool printed twenty uninitialized floats as "reconstructed data", wrote them into a
    new HDF5 file, and reached a trailing check that reported the failure and called exit(0). A
    caller saw a complete output file and a success status. It now returns 1 and writes nothing.
  • CI fails if the serialized format changes without SZ3_DATA_VERSION. v3.3.0 and v3.3.1
    shipped different layouts under the same declared 3.3.0 and nothing caught it, because the only
    digest CI had asks whether Linux and macOS agree with each other and a format change moves both
    together. tools/test/data_format_digest.txt pins the bytes to the declared version.

Not changed

The compressed format. SZ3_DATA_VERSION is untouched, and all seven algorithms across 1D/2D/3D
cross-decompress bit-identically between v3.3.2 and this branch in both directions.

Verification

Reviewed over six rounds by an agent briefed as a GROMACS maintainer, which found eleven defects in
the first round, two regressions introduced by the fixes in the third, and two more when the patch
was cut down in the fifth.

  • GROMACS, ~/gmx unmodified, EXTERNAL / AUTO / INTERNAL / OFF × serial / MPI, plus
    EXTERNAL-serial against the vendored-Zstd install: nine configurations, 526/526 h5md tests
    each
    . AUTO against a declining SZ3 falls back to internal, as it must.
  • Every failure above has a control that fails on master and passes here.
  • Compression correctness against the installed package, not the build tree: real MD trajectory
    (4187×3341×3, 53.4 MiB), direct API and HDF5 filter, max error inside the bound, filter output
    bit-identical to the API, on both Zstd paths.
  • Access modes: tools/H5Z-SZ3/test/filter_access_modes.sh asserts 31 properties across
    h5repack, h5dump, h5ls and four link/registration shapes an application can take — including
    that h5repack with the plugin unreachable exits 0 and quietly writes an unfiltered copy. CI
    runs it.
  • Zstd: system, bundled, a different version, no pkg-config. HDF5: serial and parallel, in
    both module and config mode. Shapes: static, shared, relocated prefix, add_subdirectory.
    CMake 3.28 and 4.4 in both roles. macOS on a real host.
  • Every header compiles standalone, 56 of them, under libc++ (clang, macOS) and libstdc++
    (g++ 13.3, Linux).
  • CI gains the step that would have caught all of this: install SZ3, build a separate project
    against it, link an executable and run it. Against the pre-fix tree, its assertions fire.

Windows has no development host here, so every change was read against MSVC and MinGW
semantics rather than run locally. CI covers it: Build & Test (Windows) and
Build & Test (Windows MinGW) both pass, and MSVC defaults to the vendored Zstd, so that is the
path the MSVC job exercises.

🤖 Generated with Claude Code

Copilot AI lite review requested due to automatic review settings September 17, 2026 22:35

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The bundled Zstd target publicly propagates private visibility overrides that can interfere with consumers’ own Zstd headers and linking.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR makes installed SZ3 packages consumable via find_package(SZ3) and fixes HDF5 filter property-list handling to prevent crashes.

Changes:

  • Reworks exported dependencies, target aliases, ABI checks, and RPATH handling.
  • Bundles Zstd privately at version 1.5.7.
  • Adds regression coverage for installed-package consumption and HDF5 behavior.
File summaries
File Description
CMakeLists.txt Configures packaging metadata and install RPATH behavior.
tools/zstd/CMakeLists.txt Builds and installs private bundled Zstd.
tools/H5Z-SZ3/CMakeLists.txt Updates HDF5 detection, ABI checks, and exports.
SZ3Config.cmake.in Resolves dependencies and exposes compatible targets.
tools/H5Z-SZ3/src/H5Z_SZ3.cpp Detects filters on individual property lists safely.
include/SZ3/lossless/Lossless_zstd.hpp Selects bundled or system Zstd headers.
.github/workflows/cmake.yml Adds installed-package and bundling regression checks.
Review details
  • Files reviewed: 7/7 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tools/zstd/CMakeLists.txt Outdated
Comment on lines +102 to +111
target_compile_definitions(zstd
PUBLIC
SZ3_BUNDLED_ZSTD=1
ZSTDLIB_VISIBILITY=
ZSTDERRORLIB_VISIBILITY=
PRIVATE
# The x86-64 assembly Huffman decoder arrived in 1.5.x as a .S that would need
# enable_language(ASM) and an architecture guard, and that MSVC -- the one platform where
# this bundled copy is the default -- never builds. Take the C path everywhere.
ZSTD_DISABLE_ASM=1)
ayzk and others added 3 commits September 17, 2026 18:52
set_SZ3_conf_to_H5 and get_SZ3_conf_from_H5 both branched on H5Zfilter_avail(),
which reports whether the filter is registered with the library, not whether it
is on this property list. Once H5Zregister() has run or the plugin has been
loaded it answers yes for every list, so a freshly created dataset creation
property list took the modify path in set_ and faulted inside HDF5, while get_
read from a list with no filter on it.

get_ also ignored H5Pget_filter_by_id's return. A failed read leaves cd_nelmts
at its input value, which is not zero, so the caller's Config was overwritten
from the zero-filled buffer instead of being left alone.

Co-Authored-By: Claude Opus 5 <[email protected]>
find_package(SZ3) on an installed SZ3 did not work. The export named imported
targets that exist only inside the build that made them (PkgConfig::ZSTD,
MPI::MPI_CXX), it offered only the namespaced names, its config file ended the
consumer's configure from inside an optional probe, it baked in this machine's
HDF5 include path, and the filter it installed recorded no RUNPATH. The bundled
Zstd installed a libzstd and a zstd.h over the ones already in the prefix and
exported its symbols into the consumer's process.

CI now installs SZ3 and builds a separate project against it, which is how all
of the above were found.

Co-Authored-By: Claude Opus 5 <[email protected]>
find_package(GSL) defined SZ3_ENABLE_GSL and linked GSL::gsl for one header,
preprocessor/Wavelet.hpp, which nothing in the tree includes -- no ALGO uses it,
no pipeline composes it, no test builds it, and no CI job enables GSL, so it has
never been compiled. Its cost was a fatal find_package(GSL REQUIRED) inside
every consumer's optional find_package(SZ3) probe, because libgsl-dev is common
enough on a build host that a packaged SZ3 records GSL_FOUND=TRUE.

The header goes with the dependency rather than staying behind: with nothing
defining SZ3_ENABLE_GSL, every line of it is inside a block no supported build
can enter. It also discards the transform coefficients past index n, so it is
not a round trip for any length that is not a power of two. git has it if
someone wants to bring it back with a composition and a test.

Co-Authored-By: Claude Opus 5 <[email protected]>
ayzk and others added 7 commits September 17, 2026 22:51
…and a version

Three things an application that links the filter, rather than reaching it through
HDF5_PLUGIN_PATH, could not do.

H5Z_SZ3_initialize()/H5Z_SZ3_finalize() follow H5Z-ZFP's shape: an H5Zfilter_avail
guard, H5Zregister, and an unregister that only takes back what this library put there.
Until now nothing in the public header was a function a consumer had to call, so
GROMACS linked SZ3::hdf5sz3 without referencing a single symbol from it, and on a
compiler driver that passes --as-needed the dependency was dropped outright.

initialize() separates the two ways it can succeed: 1 when it registered the filter,
0 when one was already there and it deferred. Zero is worth logging. HDF5 holds one
filter per id, and a different build of this same filter -- the copy hdf5plugin ships
under id 32024, say -- writes cd_values this one rejects and files this one cannot read.

The install now also drops a copy of the library in lib/plugin, which HDF5 can be
pointed at on its own. Pointing HDF5_PLUGIN_PATH at the whole install lib/ instead is
destructive -- the variable replaces HDF5's compiled-in plugin directory rather than
extending it -- and expensive, because HDF5 dlopens every lib*.so in each directory on
the path. H5Z_SZ3_PLUGIN_INSTALL_DIR follows H5Z-ZFP's cache-variable precedent.

The H5Z_class2_t name now carries the version, because that string is what lands in the
file, what h5dump and h5ls report back with no plugin present at all, and what HDF5
names in "required filter '...' is not registered".

Co-Authored-By: Claude Opus 5 <[email protected]>
h5repack is how most users apply a filter without writing code, and it drives
H5Z_sz3_set_local through cd_values typed on a command line, which set_SZ3_conf_to_H5
never does. h5dump and h5ls are what they reach for when it goes wrong. Neither was
covered, nor were the three shapes an application can take: linking and calling
H5Z_SZ3_initialize(), linking but relying on HDF5_PLUGIN_PATH, and doing both.

The negative cases are the point. h5repack applied with the filter unreachable exits 0
and writes an unfiltered copy; h5repack -f NONE in the same state exits 0 and drops the
dataset outright. Both are asserted here so that a change making them worse is visible.

The script asserts which path was taken rather than only an exit code, and the DT_NEEDED
check reports itself inconclusive rather than passing when the compiler driver keeps
unreferenced libraries anyway.

Co-Authored-By: Claude Opus 5 <[email protected]>
The bundled copy is not a fallback for rare platforms; it is what most people who build SZ3
from source get. pkg_search_module(ZSTD) needs libzstd.pc, which ships in libzstd-dev --
Priority optional on Debian and Ubuntu, not installed by default -- while the libzstd1
runtime it would link is Priority required and always present (checked on Ubuntu 24.04:
`apt-cache show libzstd1 libzstd-dev`). macOS carries no zstd.h in /usr/include or the Xcode
SDK, and Windows carries neither. So `git clone && cmake && make` reaches the bundled copy on
all three unless the builder installed a development package on purpose. It was 1.4.5, from
2020.

1.5.6 rather than the current 1.5.7, on compression THROUGHPUT rather than output size.
First, Zstd on its own -- Lossless_zstd timed directly, no file I/O in the timed region, on lab1 (x86-64, BMI2),
pinned to one core, variants interleaved, best of 5 over 4 passes:

  92,989,003 B payload -- the exact bytes ALGO_INTERP hands Zstd for a 1.07 GB HACC field
    zstd     output bytes   zstd ratio    compress MB/s    decompress MB/s
    1.4.5      90,125,005       1.0318            568.6             1248.0
    1.5.6      90,125,789       1.0318            672.3             1496.7
    1.5.7      90,641,684       1.0259            420.7             1689.3

  167,865,204 B of raw MD floats -- what ALGO_LOSSLESS hands Zstd
    1.4.5     155,106,115       1.0823            774.8             1222.4
    1.5.6     155,108,273       1.0822            940.9             1485.7
    1.5.7     155,100,676       1.0823            565.1             1238.8

Read those as two separate axes, because only one of them moves much, and then read the
third table below, because throughput measured on Zstd alone is not what a user feels.

  Throughput, Zstd alone (MB/s): over four payloads 1.5.6 compresses 15-38% faster and
  decompresses 2-27% faster than 1.4.5, while 1.5.7 compresses 26-30% SLOWER than 1.4.5.

  Output size (bytes): all three sit within about half a percent of each other. 1.5.6 is
  within 0.002% of 1.4.5 on every payload. 1.5.7 ranges from 0.573% more bytes on the first
  payload above (90,125,005 B -> 90,641,684 B, ratio 1.0318 -> 1.0259) to 0.004% fewer on
  the second.

What that is worth end to end depends entirely on the algorithm, because SZ3 interpolates,
quantises and Huffman-codes before Zstd sees anything. Measured, not inferred: a Timer around
the one lossless call in SZGenericCompressor, against the SZ_compress/SZ_decompress time the
sz3 CLI already reports (which excludes file I/O). SZ3_DEBUG_TIMINGS could not answer this --
built with it ON, the default path prints no stage timings at all, every timer.stop() on that
path being absent or commented out. Seconds, best of 5, one quiet machine:

  ALGO_INTERP_LORENZO, the default path -- Zstd is a few percent of the work
    dataset  zstd     SZ_compress   Zstd share    SZ_decompress   Zstd share
    md       1.4.5    0.7826 s         4.2 %        0.7128 s         2.2 %
    md       1.5.6    0.7742 s         3.4 %        0.7015 s         2.2 %
    md       1.5.7    0.8073 s         7.0 %        0.7011 s         2.2 %
    hacc     1.4.5    1.3432 s         4.5 %        1.0896 s         1.2 %
    hacc     1.5.6    1.3391 s         3.7 %        1.0773 s         1.2 %
    hacc     1.5.7    1.3808 s         6.9 %        1.0839 s         1.8 %

  ALGO_LOSSLESS -- Zstd is the whole algorithm (SZDispatcher calls Lossless_zstd directly)
    md       1.4.5    0.2861 s                      0.1722 s
    md       1.5.6    0.2500 s  (-12.6 %)           0.1604 s  (-6.9 %)
    md       1.5.7    0.3701 s  (+29.4 %)           0.1852 s  (+7.5 %)
    hacc     1.4.5    0.7264 s                      0.2661 s
    hacc     1.5.6    0.6485 s  (-10.7 %)           0.2544 s  (-4.4 %)
    hacc     1.5.7    0.7903 s   (+8.8 %)           0.2681 s  (+0.8 %)

So 1.5.7's Zstd regression costs only 2.8-3.2% of SZ_compress on the default algorithm, and
nothing on decompression; it costs 8.8-29.4% on ALGO_LOSSLESS, where Zstd is all there is.
1.5.6 is faster than 1.4.5 in every cell of both tables. That, rather than any disqualifying
fault in 1.5.7, is why 1.5.6: it is the fastest of the three everywhere measured and matches
1.4.5's output size. If 1.5.7's one listed fix -- "compression bug in 32-bit mode associated
with long-lasting sessions" -- matters for 32-bit builds, the cost of taking 1.5.7 instead is
about 3% of SZ_compress on the default path and up to 29% on ALGO_LOSSLESS, and that is a
judgement call rather than a measurement.

Where the throughput went: 1.5.7's changelog lists "compression ratio improvement for dfast,
aka levels 3 and 4" and "better block boundaries", and level 3 is the level Lossless_zstd
uses. Building 1.4.5, 1.5.0, 1.5.2, 1.5.4, 1.5.5, 1.5.6 and 1.5.7 with Zstd's own makefile
puts the drop at 1.5.7 and nowhere earlier, and Zstd's own build of 1.5.7 is as slow as this
one, so it is upstream's trade rather than a mistake in how this file builds it.

Streams cross between the versions unchanged. Every ALGO_* against 1D, 2D and 3D framings of
a 4187x3341x3 MD trajectory and a 256 MiB HACC field, at three error bounds -- 126
combinations -- compressed with each build and decompressed with both: all 252 reconstructions
byte-identical to the one from the build that wrote the stream. Compressed SZ3 file size over
those 126, in bytes against the same 1.4.5 baseline: 1.5.6 mean -0.027%, best -0.770%, worst
+0.045%; 1.5.7 mean -0.155%, best -1.855%, worst +1.041%. Neither is a throughput figure, and
neither is large: 1.5.6's worst case is md 3D ALGO_INTERP_LORENZO at eb 1e-2, 19,115,368 B ->
19,123,929 B, a ratio of 8.782 -> 8.778.

zstd_decompress.c was listed twice; it is listed once now.

ZSTD_DISABLE_ASM is new and required: 1.5.x added decompress/huf_decompress_amd64.S, and
without it huf_decompress.c leaves two HUF_decompress4X*_fast_asm_loop symbols undefined on
x86-64. Assembling that file would need enable_language(ASM), for a language MSVC does not
assemble. The cost was measured, not assumed: built both ways on a quiet machine, twelve
samples per case, the assembly loop decompresses 0.1-2.0% faster on three payloads and 10.6%
faster on the fourth -- and the C loop still leaves 1.5.6 faster than 1.4.5 everywhere.

The CI half is that nothing said which Zstd was under test. The Linux job installed no
development package, so whether it exercised the system path or the bundled one was whatever
the runner image happened to carry -- and either way it never said so. It now installs
libzstd-dev and asserts sz3 links a system libzstd with no bundled archive built; the bundled
step, which already existed to keep that copy out of the install prefix, now asserts the
converse and round-trips testfloat_8_8_128.dat through it, because an archive that exists is
not an archive that works. Both assertions were run against real builds of each kind, so each
is known to fail on the other path rather than passing regardless.

Two checks in that job were written `! grep ...`. Bash exempts a !-inverted command from
set -e, so unless it is the last line of the step its failure is dropped: neither could ever
have failed. They are `if grep ...; then exit 1; fi` now.

Co-Authored-By: Claude Opus 5 <[email protected]>
A consumer of an installed SZ3 could not compile unless zstd.h was on their include path, and
outside /usr/include -- Homebrew, conda, Spack -- it is not, because the header lives in a
development package they have no reason to have installed. The export then made it worse by
naming the build machine's libzstd, which stops existing the moment that prefix does. And the
bundled copy was downloaded at configure time from a URL with no hash, so an offline build
failed outright and no build was reproducible.

Five changes, because they are one problem seen from five sides.

Lossless_zstd.hpp declares the four Simple-API functions SZ3 calls instead of including
<zstd.h>. They are byte-identical in every release from 1.4.5 to 1.5.7 and covered by Zstd's
stable-API guarantee. An installed SZ3 now needs Zstd's library and never its header, which
fixes the consumer compile however SZ3 was built. SZ3_USE_ZSTD_HEADER takes the real header
instead. SZImpl.hpp, SZImplOMP.hpp and SZDispatcher.hpp called ZSTD_compressBound directly,
reaching a declaration they never made through whatever chain of headers happened to include
this one; they call Lossless_zstd::compress_bound now, so every Zstd symbol is in one file.
Not a virtual on LosslessInterface: a new pure virtual breaks every out-of-tree implementer.

test_zstd_decl compiles those declarations and the real zstd.h into one translation unit, and
zstd_decl_header_first.cpp does it in the opposite order, so a future divergence is a build
failure rather than a silent ABI mismatch. Verified by breaking it on purpose: changing
ZSTD_compressBound's parameter to int fails with "conflicting types for 'ZSTD_compressBound'".
Both include orders sit behind clang-format off/on, because clang-format sorts includes and
sorting them made both files test the same order -- caught by re-running it, not by luck.

tools/zstd holds the source now. FetchContent downloaded zstd-1.5.6.tar.gz at configure time
with no URL_HASH: offline builds failed, nothing verified what arrived, and a distro packager
had to patch the mechanism out. lib/common, lib/compress and lib/decompress are copied byte for
byte from the release tarball (SHA256 8c29e06c...), so diff -r against it proves the copy is
unmodified -- which is why whole directories were copied rather than files picked out of them.
lib/deprecated, lib/legacy and lib/dictBuilder are not here: SZ3 calls none of them and they
were 14 of the 39 files this used to compile. ZSTD_LEGACY_SUPPORT=0 is now stated rather than
left to the default, because lib/legacy is gone and two decompress files include it when it is
set. The library's form is unchanged: STATIC, OUTPUT_NAME sz3_zstd, hidden visibility, both
visibility macros, position-independent. That is what keeps its symbols from interposing on a
consumer's own Zstd, and it was not touched.

Nothing includes zstd.h any more, so the SZ3/bundled_zstd/ staging and its install(FILES) are
gone. They existed only to keep the bundled header off a consumer's include path; with no
header installed at all there is nothing to keep off it.

Finding Zstd is now a cascade -- find_package(zstd CONFIG), then pkg-config, then find_library,
then the vendored copy -- because no one of them is everywhere. zstdConfig.cmake is absent on
RHEL/Rocky/Alma 8-10, Fedora <=43, Ubuntu 20.04 and 22.04, Debian bullseye and default Spack;
libzstd.pc needs libzstd-dev, which Debian and Ubuntu do not install by default. find_library
needs neither, and since SZ3 no longer needs a header it is now a sufficient answer.
tools/zstd can be deleted outright and a system build still works.

The export carries only what a consumer can resolve. A system Zstd goes in under
$<BUILD_INTERFACE:> -- the link as well as the includes, which is the half that was missing --
and SZ3Config.cmake runs the same cascade again on the consumer's machine, soft-failing with
SZ3_NOT_FOUND_MESSAGE and SZ3_FOUND=FALSE rather than ending a configure that may well be an
optional QUIET probe. The vendored Zstd needs none of that: it exports as SZ3::zstd under
${_IMPORT_PREFIX}, so it relocates with the prefix.

1.5.6 rather than 1.5.7 still, and after vendoring there is no URL left to carry why, so the
reasoning and its expiry condition are in tools/zstd/CMakeLists.txt. In short: 1.5.7's only
listed correctness fix caps a 32-bit window index that SZ3 cannot drive past its limit, because
SZ3 calls one-shot ZSTD_compress, which builds a fresh ZSTD_CCtx per call. Measured: 5 GB
through one-shot calls reaches max curr = srcSize + 2 at 64 MiB, 256 MiB and 1 GiB per call,
while the same 5 GB through one reused ZSTD_CCtx reaches ZSTD_CURRENT_MAX. The same defect is
in the 1.4.5 this replaced. It expires if SZ3 ever holds a ZSTD_CCtx across calls.

CI asserts which Zstd each leg took from SZ3_ZSTD_PROVIDER rather than inferring it from ldd,
checks that no zstd.h is installed under any name, that the generated export names no libzstd
at all, that a consumer still builds after the build tree is deleted, and that a packager who
deletes tools/zstd can still build against the system Zstd.

Co-Authored-By: Claude Opus 5 <[email protected]>
h5repack, h5dump and h5ls cover how a user reaches this filter without writing
any code, so the suite cannot report a result without them. libhdf5-dev does not
carry them, hdf5-tools does, and the runner had neither: the suite answered with
twelve `command not found` failures rather than one actionable line.

Co-Authored-By: Claude Opus 5 <[email protected]>
char's signedness is implementation-defined. Where it is signed, a cd_values
byte of 0xF3 reaches N as -13 and passes `N > 4`. The later guards do not stop
it either: with bitWidth 0 the dim_bytes computation wraps to 0, so a negative
N reaches bytes2vector, which asks for 1.8e19 elements and throws a bare
std::length_error with no diagnosis of what was wrong with the input.

Co-Authored-By: Claude Opus 5 <[email protected]>
The comment claimed the array was the layout hdf5plugin ships, nine elements
against this filter's eight. Neither number holds: hdf5plugin ships v3.1.7,
which writes fourteen for that dataset, and this filter writes fifteen. The
count is dataset-dependent and has been 9, 11, 13, 14 and 15 across releases,
so it identifies nothing and the check does not rest on it.

Co-Authored-By: Claude Opus 5 <[email protected]>
@ayzk ayzk changed the title Make an installed SZ3 usable through find_package, and stop a crash in the HDF5 filter Make an installed SZ3 usable through find_package, stop a crash in the HDF5 filter, and vendor Zstd Sep 18, 2026
ayzk and others added 10 commits September 18, 2026 10:52
Proving the export outlives the build tree meant deleting build/, but the steps
that run sz3 on the downloaded dataset have working-directory: build. The job
had never reached them before, because it stopped earlier on missing HDF5 tools.
Moving the directory aside proves the same thing: it genuinely does not exist
while the consumer configures, builds and links.

Co-Authored-By: Claude Opus 5 <[email protected]>
The build resolved Zstd through three routes and the consumer re-ran all
three plus a build-time search hint. One route each is enough.

Build side: pkg-config only, as on master. find_package(zstd CONFIG) was
never the only route to a system Zstd -- zstd's own Makefile and CMake
installs both write libzstd.pc -- and the find_library rung never won in
any of the nine environments measured, because the one packaging shape
that has a libzstd without a .pc (Debian's libzstd1 with no libzstd-dev)
has no dev symlink for find_library to find either.

Consumer side: find_library only. Its suffix order prefers a shared Zstd
without the BUILD_SHARED_LIBS reasoning the imported-target route needed,
and it searches exactly the prefixes find_package(SZ3) itself came from.
The build-time libdir hint goes with it, which takes the last
build-machine path out of the installed package.

CMakeLists.txt 252 -> 151, SZ3Config.cmake.in 84 -> 46,
tools/zstd/CMakeLists.txt 133 -> 88.

Co-Authored-By: Claude Opus 5 <[email protected]>
The path emitted the same error three times: a printf to stdout, a std::cerr,
and the throw. The stdout write is the damaging one -- inside the HDF5 filter it
lands in whatever the user redirected, so `h5dump -d /ds f.h5 > out.txt` gets
SZ3's diagnostic written into the data file. The other two are duplicates: the
filter already turns what() into an entry in HDF5's error stack, and the CLI
prints it.

The message keeps what the printf carried -- the program version, the data
version it reads, and the version found -- so nothing diagnostic is lost.

Co-Authored-By: Claude Opus 5 <[email protected]>
The version check in SZImplOMP.hpp is a copy of the serial one, so the OpenMP
decompression path wrote the same diagnostic to the caller's stdout. The
iterator's dimension check did it with std::cout, and threw away the two counts
it printed rather than putting them in the message.

Iterator.hpp gains <sstream>, which it now uses directly. Its print() keeps
std::cout: printing is what the caller asks it for.

Co-Authored-By: Claude Opus 5 <[email protected]>
H5Dread's status was stored and then ignored: on failure the tool printed twenty
uninitialized floats as "reconstructed data", wrote them into a new HDF5 file,
and reached a trailing check that reported the failure and called exit(0). A
caller saw a complete output file and a success status.

The status is checked where it is produced, so nothing downstream runs on a
buffer that was never filled, and ERROR() returns non-zero like every other
failure in this file. The trailing check is unreachable once the reads bail out.

Co-Authored-By: Claude Opus 5 <[email protected]>
cd_values carries neither a magic number nor a version, and its layout has
changed five times across released versions, so on read it was parsed under this
build's assumed layout before anything established the data came from a
compatible SZ3. What refused a v3.2.0, v3.2.1 or v3.3.0 file was Config::load's
own guards -- one of which, the dimension count, did not catch a negative value
until this branch fixed it -- and the message they produce, "invalid number of
dimensions", reads as a corrupt file rather than a version mismatch.

Consulting the payload header first refuses those three with the version that
wrote them and the version this build reads.

The magic number is a precondition rather than a requirement: a chunk the
conf.num < 20 path stores raw has no header, and conf.num is not known until
cd_values has been parsed, so a missing magic cannot be told apart from a
pass-through chunk here. Files older than v3.2.0 carry no magic and so are still
refused by Config::load, as before.

Co-Authored-By: Claude Opus 5 <[email protected]>
A reader compares SZ3_DATA_VERSION for exact equality, so a format change that
does not move it produces files an older build accepts and then misreads.
v3.3.0 and v3.3.1 shipped different layouts under the same declared 3.3.0 and
nothing caught it: the only digest CI had asks whether Linux and macOS agree
with each other, and a format change moves both together.

The recorded digest is of a bundled-Zstd build, reusing the tree the step above
already produces. The bytes depend on which Zstd compressed them -- system Zstd
1.5.5 and 1.5.7 differ by two bytes on this input -- and only the bundled one is
pinned by this repository, so anything else would make the check depend on the
runner image.

Demonstrated in all four states: unchanged passes, a serialized field added
without a version bump fails, a version bumped without a row fails, and
reverting passes again.

Co-Authored-By: Claude Opus 5 <[email protected]>
A third of this branch's added lines were comments. Most restated their own code
or recounted how a defect was found. What is kept is the set a later edit would
trip over: that Lossless_zstd.hpp declares Zstd rather than including it, that
H5Zfilter_avail() registers rather than reports, that the magic number is a
precondition and not a requirement, that char's signedness is why a dimension
count is compared as unsigned, why the bundled Zstd is named libsz3_zstd with
hidden symbols, and the two shell forms in CI that look like checks and cannot
fail.

Two clang-format reflows of untouched lines are reverted with them.

Co-Authored-By: Claude Opus 5 <[email protected]>
The comments said what happens. A comment on a guard is read by someone about to
remove the guard, so it has to say what not to write: not `N > 4`, not
#include <zstd.h>, not H5Zfilter_avail(), and no printf inside a filter whose
stdout is the user's output file.

Co-Authored-By: Claude Opus 5 <[email protected]>
Two of these stacked negatives -- "never needs ... would not have", "which they
do not ... not its header" -- and trailed into references a reader has no way to
resolve at that point. The instruction is the first sentence; the rest is one
reason, stated positively.

Co-Authored-By: Claude Opus 5 <[email protected]>
ayzk and others added 4 commits September 18, 2026 15:22
It guarded against Zstd changing the signatures of its four Simple-API
functions. Those are ABI-stable across 1.x and the assumption is taken as given,
which removes the only thing in the tree that needed a real zstd.h and with it
the SZ3_ZSTD_HEADER_DIRS search and its can-be-empty branch.

Co-Authored-By: Claude Opus 5 <[email protected]>
SZ3_ZSTD_PROVIDER was a string saying what SZ3Targets.cmake already shows: the
vendored build exports SZ3::zstd carrying its own location under the install
prefix, and the system build exports nothing for Zstd at all. Two records of one
fact can disagree; one cannot.

SZ3Config.cmake now asks whether SZ3::zstd exists, and the three CI checks ask
the export instead of grepping for the string.

Co-Authored-By: Claude Opus 5 <[email protected]>
Two option() calls differing only in their default value. option() leaves an
existing cache entry alone, so -DSZ3_USE_BUNDLED_ZSTD still wins either way.

Co-Authored-By: Claude Opus 5 <[email protected]>
ayzk and others added 30 commits September 18, 2026 16:41
sz3ToHDF5, dsz3FromHDF5, convertBinToHDF5 and cdvalueHelper all print a usage
line and assert nothing. They are command-line tools that happened to live in a
directory called test, which left the one thing that does assert --
filterAccessModes.sh -- indistinguishable from them.

The four move to tools/H5Z-SZ3/tools. test/ keeps the suite and the ctest
wrapper that installs a prefix for it to run against.

Co-Authored-By: Claude Opus 5 <[email protected]>
…atic

tools/H5Z-SZ3/CMakeLists.txt added test/ unconditionally, so the filter's tests
were configured whenever the filter was, while the rest of the tree's tests sit
behind BUILD_TESTING. The guard inside the directory was doing that job instead.
Now the directory is gated and the guard asks only what it should: whether
h5repack, h5dump and h5ls are there.

sz3_filter_on_plist sat in an anonymous namespace while everything else
file-local in that file is static.

Co-Authored-By: Claude Opus 5 <[email protected]>
The pxd mirrored a one-argument load; the real one takes the remaining buffer
length as well. Cython trusts a cdef extern declaration rather than checking it,
so this only shows up as a compile failure the first time anything calls it --
nothing in sz.pyx does. Checked the rest of the mirror while there: every other
signature and field type matches.

Co-Authored-By: Claude Opus 5 <[email protected]>
build-linux had grown to fourteen steps and three hundred lines: a build, a
consumer, an export audit, a packager's deletion, the bundled Zstd's promises, a
format digest, and a round trip. A red X on it said nothing about which.

  cmake.yml      one full-feature build per platform: configure everything on,
                 build, ctest, round-trip the smoke dataset, compare digests
  packaging.yml  what an installed SZ3 hands someone else -- the export, the
                 consumer shapes, the bundled Zstd, the format digest
  hdf5.yml       the filter against more than one HDF5: three versions, a
                 parallel one, and two installed side by side

No step was dropped. Three are new, all in hdf5.yml: a second HDF5 the build
does not see, and a consumer that has to compile against its own rather than
ours -- which is the case the BUILD_INTERFACE guard exists for, and which no
single-HDF5 machine can show.

Co-Authored-By: Claude Opus 5 <[email protected]>
The name is written into every file's object header, so it costs bytes there and
makes two patch releases produce different headers for identical data. The
library version already implies the format version, and cd_values carries the
format version where a decoder can act on it.

Of the filters registered with HDF5, one puts any version in this field and none
puts a format version.

Co-Authored-By: Claude Opus 5 <[email protected]>
A header-only library sees more of a compiler than most: every template it has
is instantiated in the consumer's translation unit, so a warning or a language
difference shows up there rather than here. Everything until now built under
whichever compiler the runner shipped.

gcc-12, gcc-14, clang-16 and clang-18, each with BUILD_SHARED_LIBS on and off,
and an assertion that the leg produced the shape it asked for. Quoted in the
matrix because YAML reads a bare ON as a boolean, which would have made that
assertion compare against "true" and never fire.

Co-Authored-By: Claude Opus 5 <[email protected]>
HDF5 writes this string into every dataset that uses the filter and quotes it
back when the filter is missing, which is the moment someone needs to know where
to get it. Measured: at 100 and at 1000 datasets, lengthening it costs zero
bytes -- the object header absorbs it.

Co-Authored-By: Claude Opus 5 <[email protected]>
Every template SZ3 has is instantiated in the consumer's translation unit, so
its warnings land in their build. GROMACS carries 24 suppression lines for
them.

Renames constructor parameters that shadowed the member they initialise,
following the trailing-underscore spelling already used in BlockwiseIterator
and Iterator; drops the names of parameters nothing reads; gives six files the
newline they were missing; marks the xdr helpers inline, since a static in a
header that only a template calls is one clang flags as unneeded; parenthesises
the && inside || in HuffmanEncoderV2, which already meant what it looked like;
and moves the H5Z filter callbacks out of the public header, where every
consumer saw them declared and never defined.

Co-Authored-By: Claude Opus 5 <[email protected]>
…or it

Splitting build-linux into three left packaging.yml with only BUILD_TYPE while
the steps that moved into it still read DIMS, MODE and TOL. GitHub interpolates
an undeclared key to the empty string, so the compressor ran as

  sz3 -f -i tools/sz3/testfloat_8_8_128.dat -z _n.sz3 -3  -M

and answered with its usage text and a non-zero exit -- which reads as a
compression failure rather than a workflow that lost its arguments.

An interpolation cannot be made to fail, so the step that feeds those values to
a program now names a key that is missing before running it, and a check over
every workflow catches one that was never declared or is declared empty. Run
against the three files as the split left them it reports the eight references
that broke this job.

Co-Authored-By: Claude Opus 5 <[email protected]>
SZ3Targets named OpenMP::OpenMP_CXX, and SZ3Config asked FindOpenMP for the CXX
component to supply it. FindOpenMP can only produce a target for a language the
consumer enabled, so a C-only project got

  Could NOT find OpenMP (missing: OpenMP_CXX_FOUND CXX)
  SZ3Config.cmake:51 (find_dependency)

and its configure ended there. That is what the parallel-HDF5 job hit: its
consumer is C, for mpicc, and MPI had nothing to do with it.

The export no longer names the target. SZ3Config asks for whichever of C and
C++ the consumer enabled -- one runtime serves both on every compiler SZ3 builds
under -- and appends it to SZ3::SZ3, the way the Zstd dependency above it is
already handled.

The check that should have caught this was written as find_package(SZ3 QUIET)
with the executable inside if (SZ3_FOUND), so it built nothing in exactly the
case where a C-only consumer is broken, and passed. It is REQUIRED now, and the
binary it names has to exist.

Co-Authored-By: Claude Opus 5 <[email protected]>
The new shared=OFF matrix legs turned 16 of the 31 access-mode checks red on
their first run, and they were right to: BUILD_SHARED_LIBS=OFF installs
libhdf5sz3.a, there is no shared object for HDF5 to dlopen, and every mode that
goes through HDF5_PLUGIN_PATH has nothing to reach.

Not a configure-time error for that combination: the archive is what an
application links to call H5Z_SZ3_initialize() itself, which is a whole access
mode and still passes, and the matrix leg asserts that archive exists. Not a
plugin forced shared regardless either: that overrides what the flag was set
for, and H5Z_SZ3_PLUGIN_INSTALL_DIR already offers an install with no plugin,
so this suite has to describe that tree whichever way the question is settled.

So the modes are skipped, each one named, each one counted, and the total
asserted at the bottom, because a suite that quietly reports fewer checks is
what this file exists to prevent. What decides the skip is the shape of the
installed library rather than the plugin's absence: a shared hdf5sz3 with an
empty plugin directory is the install rule being wrong and now says so.

Under MSYS2 the prefix reached cmake as /d/a/... , which a native Windows cmake
does not read, so it found SZ3 through the build tree on PATH instead and
failed on an SZ3Targets.cmake that only exists once installed. Paths handed to
cmake and to HDF5 go through cygpath where there is one.

Co-Authored-By: Claude Opus 5 <[email protected]>
integration_test.yml has had this since it was written; cmake.yml, packaging.yml
and hdf5.yml never got it. Three pushes to this branch inside 24 minutes each
started a full set, and all three ran to completion, around 40 jobs spent on two
revisions nobody was waiting for.

Co-Authored-By: Claude Opus 5 <[email protected]>
…ey are absent

::set-output is deprecated and warned about on every run. When it stops working
the two jobs will still finish, the compare job will read two empty strings, and
"" != "" is false -- it would print that Linux and macOS produced identical
output having compared nothing at all.

The digests go through $GITHUB_OUTPUT instead, each job refuses to publish an
empty one, and the compare job refuses to accept one.

Co-Authored-By: Claude Opus 5 <[email protected]>
An installed SZ3 is reached two ways by GROMACS: mainline finds it with
find_package(SZ3) behind GMX_USE_SZ3, and the H5MD prototype vendors it and
writes SZ3-compressed trajectories through it. Neither was covered.

gromacsConsumerMatrix.sh configures GROMACS at each value of GMX_USE_SZ3 and
asserts which library was wired in, including that AUTO falls back when SZ3
declines rather than the configure ending. gromacsH5mdRoundtrip.sh writes a real
trajectory through the filter, repacks it, reads it back with a reader that
links no GROMACS and no SZ3, and compares every coordinate with the float32 the
compressor was given.

The workflow runs them weekly and on demand, not on push: it is 1.5 hours of
runner time and it depends on gitlab.com and a 70 MB download.

Co-Authored-By: Claude Opus 5 <[email protected]>
SZGenericCompressor::decompress reads quant_inds_size straight from the
decompressed frame and hands it to encoder.decode as targetLength, which
allocated out(targetLength) before any check. A 1736-byte file with
quant_inds_size patched to 2^31 committed ~8 GB (measured) before the
decode walk failed; 2^50 only then hit bad_alloc. Reachable from h5dump,
h5repack and h5py, which run the same decompress.

Two bounds, because the count has two ceilings:

- HuffmanEncoder::decode: below the root every code is at least one bit,
  so encodedLength * 8 is the most symbols the stream can hold. Reject a
  larger targetLength (guarding the * 8 against overflow) before sizing
  out. This covers every decode caller for a multi-leaf tree.

- SZGenericCompressor::decompress: a one-leaf tree stores its symbols in
  zero bits, so decode has no stream length to check against; cap
  quant_inds_size at conf.num, the element count decData already holds,
  which no valid frame exceeds.

Co-Authored-By: Claude Opus 5 <[email protected]>
usage() ends in exit(0), and the argument path called it for every
parse error, so a missing dimension, unknown -M mode, or negative/zero/
absurd dimension printed usage to stdout and reported success. A script
checking the exit code saw a bad command succeed.

Route argument errors through main()'s existing catch instead: they now
print "sz3: <what was wrong>" to stderr and exit 1, matching the fix in
2e908f3 for algorithm exceptions. Dimensions are parsed and validated
in one place (parse_dim), rejecting signs, non-digits, trailing junk,
zero, and overflow. Unknown -M modes are rejected at parse time rather
than silently falling back to ABS. No arguments, -h, --help, -h2, and
-v still print help to stdout and exit 0.

Co-Authored-By: Claude Opus 5 <[email protected]>
Each of these reported success in a state it was written to catch.

cmake.yml: build-windows configured without BUILD_TESTING, so its "Run CTest"
step found no tests and ctest exited 0. Set BUILD_TESTING there, and pass
--no-tests=error everywhere so an empty ctest can never be green again.

packaging.yml: the macOS step grepped otool -L for '@rpath/libhdf5', which
matches the library's own LC_ID_DYLIB (@rpath/libhdf5sz3.dylib) on every shared
build, so the step reduced to "the dylib carries some LC_RPATH". A dylib whose
libhdf5 comes through an @rpath that no LC_RPATH resolves passed it. dlopen
answers the question the step is named after.

packaging.yml: two export checks globbed Export/*/SZ3Targets.cmake straight into
grep. When the glob matches nothing it reaches grep as a literal, grep exits 2,
and the `if` reads that as "clean" -- with SZ3::zstd sitting in the file.

packaging.yml, hdf5.yml: BUILD_TESTING=ON in two jobs that never run ctest. It
also makes FetchContent install googletest into the prefix those jobs go on to
inspect as an installed SZ3.

gromacs.yml: sed exits 0 when it matches nothing, so an unreadable project()
line left SZ3_EXPECT_VERSION empty and the round trip asserted "H5Z-SZ3-",
which the vendored 3.1.8 satisfies. Fail instead, in the workflow and in the
script.

hdf5.yml: restore the line continuations in the _mpi consumer's cmake call.

Co-Authored-By: Claude Opus 5 <[email protected]>
RegressionPredictor::load reads coeff_size out of the decompressed frame and
hands it to HuffmanEncoder::decode as targetLength. Reachable through
ALGO_LORENZO_REG, whose default predictor set is Lorenzo + Regression.

The encoder-side ceiling added in 98e05f4 does not cover this call. Below the
root every Huffman code is at least one bit, so encodedLength * 8 bounds a
multi-leaf stream -- but a single-leaf tree spends zero bits per symbol, and
that is exactly the tree a smooth field produces here: once the first block is
in, every later block's coefficients predict from the previous block's, so the
deltas are all the zero bin. A 143-byte ALGO_LORENZO_REG file with coeff_size
patched to 2^28 committed 1.07 GB (measured) and then decompressed without
complaint, because nothing downstream reads past the coefficients it needs.

The count's own ceiling comes from the block grid, not from the stream:
pred_and_quantize_coefficients() appends N linear terms plus one independent
term per block it is committed on, and the caller walks
prod ceil(dims[i] / blockSize) blocks, so (N + 1) * that product is the most a
stream this predictor wrote can carry. A predictor cannot work that out on its
own -- load() gets a buffer, not a Config -- so the two decompositions that
drive predictors now pass the count down, which is also how the time-series
case gets its own (spatial-only) grid rather than an over-estimate.

Co-Authored-By: Claude Opus 5 <[email protected]>
ComposedPredictor::load reads selection_size out of the decompressed frame and
hands it to HuffmanEncoder::decode, the same shape as the regression count
fixed in the previous commit and reached through the same ALGO_LORENZO_REG
path. The single-leaf case is if anything more likely here: one predictor
usually wins every block, so the selection vector is constant and its tree
holds one leaf, which spends zero bits per symbol and leaves the encoder's
encodedLength * 8 ceiling nothing to measure. A 143-byte file with
selection_size patched to 2^28 committed 1.07 GB (measured).

Its ceiling is the block count itself, with no multiplier: precompress_block_
commit() appends exactly one selection per block, and predecompress() consumes
one per block, so a stream cannot hold more selections than the caller has
blocks to spend them on. The composed predictors run over that same block
walk, so they inherit the ceiling unchanged.

Co-Authored-By: Claude Opus 5 <[email protected]>
Four steps made more than one claim, so a red X on them named at most one.
The consumer step in packaging.yml built six unrelated consumer shapes; the
hdf5.yml matrix step configured, built, ctested, installed and consumed in
one; the OpenMP step checked an error bound and a 24-cell thread-count matrix;
the bundled-Zstd step mixed "it used the bundled one" with "none of it
escaped". Each is now one step per claim, or one script that names every
assertion it makes and asserts its own total, the way filterAccessModes.sh
already does.

Four scripts, all runnable by hand with no GitHub context:

  consumeInstalledSZ3.sh   11 checks, replaces three byte-identical copies of
                           the consumer CMakeLists/main.cpp heredoc pair
  bundledZstdIsolation.sh   9 checks on what a bundled-Zstd build hands out
  openmpThreadCounts.sh    25 checks on OpenMP stream framing
  checkMaxError.sh          2 checks, replaces four copies of the parse

Two of the assertions they replace could not fail. `for lib in _bprefix/lib/*.so`
passed on an unmatched glob, and the include-directory walks passed on an empty
list; both now count what they inspected. The OpenMP suite gained the check that
makes the other 24 mean anything: a binary built without OpenMP writes the same
stream at every thread count and passed all 24 of them.

The two `for v in DIMS MODE TOL` runtime guards are gone. check_workflow_env.py
already flags every one of those references statically, at the line, for
undeclared and empty-valued keys alike.

Left alone on purpose: gromacs.yml's H5Z_SZ3_initialize step, whose four
assertions are the controls and the result of one before/after experiment;
cmake.yml's workflow-env job, whose failures do not correlate with the build's,
so folding it into build-linux would report them under a name that says
"Build & Test (Linux)"; and the "Create build dir"/`cmake ..` idiom outside the
one Linux job restructured here, because converging it across the pwsh and
msys2 jobs is unverifiable churn.

Co-Authored-By: Claude Opus 5 <[email protected]>
Registering filterAccessModes.sh as a ctest sent it through the MinGW job for
the first time, where 14 of its 31 checks went red. Both causes are the suite
reading Windows as if it were POSIX; the filter itself came through clean.

Four h5dump checks asked for dataset /ds. An argument shaped like an absolute
POSIX path is rewritten by the MSYS2 runtime before a native program sees it,
so h5dump was handed D:/a/_temp/msys64/ds and reported no such link. HDF5
resolves an unrooted name against the root group, so the name loses its slash
and the assertion stays exactly as strong.

The other ten follow from one loader error: app.exe could not find
libhdf5sz3.dll. Windows has no RPATH, the install puts the DLL in <prefix>/bin
and only the import library in <prefix>/lib, and nothing had put that directory
on PATH -- so every application shape failed to start, and the two reader
checks that read what those shapes write failed with them. The suite now puts
<prefix>/bin on PATH, which is what a Windows consumer has to do, and the
README says so where it previously claimed an application that links
SZ3::hdf5sz3 need set nothing.

consumeInstalledSZ3.sh runs a consumer that links the filter the same way and
carries the same assumption, though no job runs it on Windows today.

No check was loosened and no skip was added: the total stays 31. Three checks
that passed in that run passed for the wrong reason -- h5dump-data-noplugin-
fails and h5dump-data-with-plugin-clean on the mangled dataset name, and
app-init-file-needs-filter on a file the app never got far enough to write.

Co-Authored-By: Claude Opus 5 <[email protected]>
decode() opens the code section by reading an 8-byte length off the stream and
then uses that, not the targetLength its caller passed, to size its output and
to drive its walk. The cap SZGenericCompressor::decompress puts on
quant_inds_size therefore never reaches the number that does the work.
Reachable from an untrusted file through ALGO_BIOMD, and so through h5dump,
h5repack and h5py on any HDF5 file carrying the filter.

On the constant path the field is a symbol count, handed straight to
std::vector<T>(len, ...). A 29-byte frame with it patched to 2^28 returned
268435456 values where the caller asked for 64, and committed 1029 MB
(/usr/bin/time -v, gcc 13.3). Nothing measures it: a one-symbol tree spends no
bits on the bins, so there is no stream length to check it against. encode()
writes the num_bin it was handed and the compressor records that same count and
hands it back as targetLength, so the two must be equal -- which is what the
commented-out assert next to it claimed, and the encode path bears out.

Everywhere else the field counts bits, `out` is sized targetLength, and the walk
runs on the bits; the buffer check ran only after the walk, too late to keep it
inside either. ASan on a 47-byte frame: heap-buffer-overflow, WRITE of size 4,
0 bytes after a 256-byte region. On the fixed-length path equality is again the
bound, since encode() records mbft * num_bin for the same num_bin; holding len
there makes the walk emit floor(len / mbft) == targetLength values and no more.

Below the root every code costs at least one bit, so `len` bits cannot carry
more than `len` symbols whichever path is taken. That ceiling -- the one
HuffmanEncoder::decode already applies -- now gates these walks as well, which
is what keeps their work proportional to the stream rather than to whatever
count the caller was handed.

The Huffman path has no equality of its own -- a code is anywhere from 1 to
tree.limit bits -- and bounding the length would not have been enough anyway.
Every code below the root costs at least one bit, so a tree whose codes are
shorter than the ones that wrote the stream turns the bits the stream really
holds into more symbols than the caller asked for. Both walks now stop at the
caller's count as well as at the stream's bits, and a stream that cannot fill
that count is rejected rather than half-filled. The cached-codebook walk was
already driven by the count, but read ahead of it without limit; past the last
code byte it now shifts in zeros instead of touching memory, and afterwards
checks that no code actually consumed them.

Co-Authored-By: Claude Opus 5 <[email protected]>
Sweeping the rest of the file for the same shape -- a count read off the stream
that sizes or drives something -- turns up three more in loadAsDFSOrder, two of
them heap corruption. All are reached the same way as the length field fixed in
the previous commit: ALGO_BIOMD on an untrusted file.

tree.n sizes the node pool, ht.reserve(2 * n), but nothing ties the walk that
fills it to n: the walk pushes one node per DFS bit and runs until its stack
empties. A full binary tree with n leaves has exactly 2n - 1 nodes, so a
well-formed stream stays under the reserve and the raw Node* held in the stack
and in the children already linked stay valid -- which is the only reason the
walk works at all. A 533-byte tree section declaring n = 2 and then an all-zero
bitstream pushes a node per bit and never pops, and the vector reallocates out
from under those pointers: ASan reports heap-use-after-free, READ of size 8,
and the walk writes through them too. The walk now stops at 2n - 1 nodes, which
is both the shape of the tree n describes and what the reserve already assumed.

tree.maxval sizes veclen and veccode, which dfs_vec then indexes by the leaf
values -- read as tree.mbft raw bits, with nothing tying them to maxval. A
25-byte tree section declaring maxval = 2 and leaf values 200 and 201: ASan
reports heap-buffer-overflow, WRITE of size 1, 198 bytes after a 2-byte region.
addElementInVector() only ever enters values from [0, maxval), so requiring
that of a leaf turns away no tree the encoder wrote.

constructHuffmanTree() collapses a one-leaf tree to maxval == 1, and encode()
keys off that to spend no bits on the bins, so n == 1 and maxval == 1 are
settled together before either is written. A stream that pairs n == 1 with any
other maxval sends decode down the general walk into a root with only one
child, where the first 1 bit steps onto a null pointer: SEGV on a 30-byte frame.

Two smaller ones while here. mbft is a leaf's width in bits and is rebuilt by
shifting into a T, so a width the type cannot hold is undefined behaviour;
preprocess_encode() never raises it past what a T holds. And readBit took its
bit index as an int while the caller counts bits over a whole tree section,
which for a large enough section truncates to a negative offset.

Left alone, and worth their own pass: n still reserves two Nodes per declared
leaf before the walk can disagree, so a stream can still ask for ~384 bytes of
node pool per byte of tree section; maxval under the 2^28 ceiling can still size
veccode and veclen at about 1.3 GB between them; and dfs_vec and dfs_mp recurse
once per level with a uchar depth, so a tree deeper than 255 levels wraps it
before any of the above notices. All three are bounded by the stream's own
size, which is what the earlier commits on this branch settled for.

Co-Authored-By: Claude Opus 5 <[email protected]>
XtcBasedEncoder is libxdrf.cpp from GROMACS turned into a header, and it carries
that code's habits. Unlike the rest of this family, the three defects below fire
on valid data rather than on crafted input: an ASan+UBSan sz3 run over ALGO_BIOMDXTC
on peg-1chain (50 frames x 13406 atoms, ABS 1e-3, gcc 13.3) reports sixteen, and
ctest's SZ3_BioMD.InputsTooShortForOneTriplet aborts under ASan.

The header is seven 4-byte ints and an 8-byte count, written at whatever byte of
the compressor's buffer the decomposition and the encoder's own save() left off at.
That byte is odd for every trajectory tried, and both sides reached it by pointing
an unsigned int* or a uint64_t* at it: undefined, and exactly the shape that
survives until a vectorising -O3, LTO, or a target that faults instead of
tolerating. Those sixteen reports are all of this one kind -- three stores at the
minInt loop, three at maxInt, one for smallIdx, one for the count, and their eight
mirrors in decode. memcpy moves the same bytes with no alignment demand, so the
format is untouched; the int*/uint64_t* cursors are gone and the byte cursor that
was already there carries the whole header.

sizeInt is maxInt - minInt + 1 on both sides. An input with no complete triplet
never enters the scan that fills them, so they stay at the INT_MAX and INT_MIN
seeds -- and the range guard above passes, because the float difference of the
seeds is negative rather than large. INT_MIN - INT_MAX then overflows a signed int,
which UBSan reports six times on a one-float input, three in encode and three in
decode reading the seeds back off the stream. The destination is unsigned, so
doing the arithmetic there is the same value by a defined route: 2 for the seeds,
and the same bits as the wrap for every other input.

The run loop squares three differences and compares against smaller * smaller. The
differences are bounded by smallNum, but smallNum reaches 2^23 at the top of
magicInts, so the squares overflow. Reached from a plain float input: 16 atoms
16000.0 apart in every axis quantise ~8e6 bins apart, which puts every neighbour
distance past the end of the table, and UBSan reports -8000000 * -8000000 and
6658042 * 6658042. Squaring in unsigned and reading the sum back as int is the
wrap the signed version was relying on, so the run is cut in the same places.

Last, the genuine memory error. magicInts holds 73 entries and the encoder writes
LASTIDX -- 73 -- whenever no entry reaches minDiff, which is every input with fewer
than two triplets. The encoder clamps its own lookups and guards the rest behind
isSmaller != 0; the decoder's sizeSmall assignment sits outside that guard and
subscripted the table at 73. ASan: global-buffer-overflow, and UBSan: index 73 out
of bounds for type 'int [73]'. Clamping cannot change a well-formed decode, because
a stream that names LASTIDX carries no run and so never reads sizeSmall. The two
lookups either side of it take the same clamp, which also holds the index in range
when isSmaller walks it there over a long stream.

Nothing here may change what the encoder emits, and nothing does. ALGO_BIOMD and
ALGO_BIOMDXTC over peg-1chain, ifabp-water and adk-equilibrium at ABS 1e-2, 1e-3
and 1e-4 give the same 18 sha256 sums, compression ratios and maximum errors before
and after, as does the 16-atom input that trips the square.

Co-Authored-By: Claude Opus 5 <[email protected]>
decode() opens with a 36-byte header -- six coordinate extremes, smallIdx, and an
8-byte count of the packed bytes that follow -- and then copies that many bytes out
of the compressed buffer into one of its own. The count was measured against the
destination only, and the destination is sized from the caller's targetLength:
malloc(targetLength * 1.2 * sizeof(int)), about 4.8 bytes per element. The packed
bytes run about one per element, so a frame could claim roughly four times what it
carried and the memcpy read the difference off the end of the buffer the lossless
layer allocated. Reachable from an untrusted file through ALGO_BIOMD and
ALGO_BIOMDXTC, and so through h5dump, h5repack and h5py on any HDF5 file carrying
the filter.

A 5009-byte ALGO_BIOMDXTC frame over 3000 floats, with that one field patched from
4874 to 14400 -- the largest the destination check accepts -- decompressed without
complaint. Under ASan: heap-buffer-overflow, READ of size 14400, 0 bytes after the
4959-byte region Lossless_bypass::decompress allocated.

The bound the check was missing is the source: what is left of the compressed
buffer once the header is off it. That cannot turn away a stream this encoder
wrote, because encode() puts the packed bytes immediately after the header and is
the last thing SZGenericCompressor::compress writes into the frame -- so at this
point remaining_length is exactly the header plus the count, and the new check is
an equality for every well-formed stream. The header is now checked to be present
before any of it is read, which is also what makes that subtraction safe.

The cursor accounting at the other end of the function belongs to the same
statement and was likewise not being made. Every read went through a local pointer,
so `bytes` never moved, the consumed count came out zero, and remaining_length was
never charged -- leaving the check just above the return with nothing to check.
encode() advances its cursor and the other encoders' decode() do too. Nothing
depended on the old behaviour: SZGenericCompressor::decompress is the only caller
that can reach this decode -- SZAlgoBioMD.hpp is the one wiring, and no test
constructs the encoder directly -- and it reads neither the cursor nor the count
again, going straight on to postprocess_decode(), releasing the buffer, and
decomposition.decompress().

Co-Authored-By: Claude Opus 5 <[email protected]>
…drives

A round of decode() ends with a flag bit and, when it is set, five more that carry
the run: how many further triplets this round decodes in one go. Those five bits
went straight into the loop that writes them. Two things then went wrong at once,
and each needs its own bound.

The value itself. Five raw bits give up to 31; the decoder takes the remainder mod
three off as isSmaller, leaving a multiple of three up to 30, ten triplets. The
encoder's run loop tests `run < CHAR_BIT * 3` at the top and adds three per turn,
so 24 is the largest run it can hold, and it writes run + isSmaller + 1, at most
26. 27 through 31 are values no stream it wrote can carry.

And the loop. Bounding the count is not enough, which is the lesson from
HuffmanEncoderV2: the `while (i < numTriplets)` test is only at the top, so a run
of any length starting on the last triplet still writes past the end. Both
destinations are sized from the frame -- quantData holds targetLength bins and the
index buffer targetLength ints -- and the run writes three ints into each per
triplet, plus three more on the first. The bound that holds is the frame's own
remaining triplets: the encoder opens a run only when another triplet follows, and
its run loop re-tests `i < numTriplets` before taking each one, so i never passes
numTriplets and a stream it wrote never names more triplets than the frame has room
for. run carries over to the next round when the flag bit is 0, so the test sits
where the run is used rather than only where it is read.

A 30-bin ALGO_BIOMDXTC frame whose last of ten triplets claims a run of 30, through
SZ_decompress under ASan: heap-buffer-overflow, WRITE of size 4, 8 bytes after the
120-byte index buffer, then five more -- two writes and three reads past the same
buffer, and a WRITE 0 bytes after the 120-byte quantData -- with halt_on_error=0
letting it run on. About thirty ints past the end of both, as the shape predicts.

Co-Authored-By: Claude Opus 5 <[email protected]>
smallIdx names an entry of magicInts, and it is also the bit width receiveints
reads a run body at. It arrives on the stream, and from then on every round can
move it one step in either direction, with nothing holding it: 256 steps up and
receiveints, which splits its bit count into bytes on a local int[32], writes off
the end of that array. The previous pass clamped the three magicInts lookups but
deliberately left the bit count alone, because clamping it would have changed what
a well-formed stream decodes to. Rejecting instead does not.

The bound is the table's own extent, [FIRSTIDX, LASTIDX], and it is the encoder
that fixes it. The seed is FIRSTIDX, raised only by a scan that stops at LASTIDX,
so the field on the stream is in range to begin with. For the drift, the encoder
computes maxIdx = min(LASTIDX - 1, smallIdx + CHAR_BIT) and minIdx = maxIdx -
CHAR_BIT once, before its loop, and never recomputes them; inside the loop it takes
a step up only while smallIdx < maxIdx and a step down only while smallIdx >
minIdx. So after the first round smallIdx lies in [minIdx, maxIdx], and that window
is inside the table: maxIdx is at most LASTIDX - 1 by construction, and minIdx is
either the seed, which is at least FIRSTIDX, or LASTIDX - 1 - CHAR_BIT, which is
64. The only value outside the window the encoder ever holds is the LASTIDX it may
have been seeded with, which the range already admits. A stream this encoder wrote
therefore never leaves [FIRSTIDX, LASTIDX], and at LASTIDX receiveints fills ten of
its thirty-two entries.

Tightening the initial check from [0, LASTIDX] to [FIRSTIDX, LASTIDX] costs nothing
for the same reason: the scan that produces the field starts at FIRSTIDX.

A 900-bin ALGO_BIOMDXTC frame whose rounds each ask for one step up walks smallIdx
from 9 to 269 and then decodes one run body. Under UBSan: index 32 out of bounds
for type 'int [32]', then 33; under ASan: stack-buffer-overflow, WRITE of size 4 at
offset 160 of a [32, 160) frame object, and three more past it.

Co-Authored-By: Claude Opus 5 <[email protected]>
receivebits() is the whole of the decoder's contact with the packed bytes, and it walked them as
buffer->data[buffer->index++] with nothing testing that cursor against the allocation. How far it
walks is decided by the stream: the coordinate extremes in the header set the width of every base
triplet, and smallIdx sets the width of every run body. Reachable from an untrusted file through
ALGO_BIOMDXTC, and so through h5dump, h5repack and h5py on any HDF5 file carrying the filter.

Three bins is the tightest frame there is. buffer.data is malloc(targetLength * 1.2 * sizeof(int)),
which truncates to twelve bytes, and a header naming minInt 0 and maxInt 2147483646 on each axis
makes sizeInt 2^31 - 1, so sizeofint returns 32 and the three base coordinates take all twelve. The
flag bit that follows then reads data[12]. Through SZ_decompress under ASan: heap-buffer-overflow,
READ of size 1, 0 bytes after the 12-byte region.

The bound is the allocation and not the packed byte count beside it in the header, because the
allocation is what the cursor runs off and is a fact of this process rather than a number the stream
supplied -- and it is the wider of the two, the count having already been measured against it. The
buffer now carries that allocation in bits, less every bit read so far, so the test is one
comparison against the width being asked for, and it refuses exactly the reads that would leave the
buffer and nothing else. A well-formed stream is never one of those: the encoder emitted these bits
through sendbits into a buffer the same targetLength * 1.2 * sizeof(int) sized, sendbits and
receivebits carry lastbits identically so the decode takes back byte for byte what the encode put
in, and decode() has already held the packed count inside the allocation. An instrumented decode
puts numbers on it. The cursor stops at exactly the packed byte count on every frame of peg-1chain,
ifabp-water and adk-equilibrium, between 22 and 43 per cent of what was allocated; and on the
tightest well-formed shape that exists it stops at 12 of 12 -- three bins whose base triplet is the
widest sizeofint can describe without overflowing, 3 * 30 bits, plus the flag and the run field, 96
bits against 12 bytes. That one is still accepted, with nothing to spare, which is what says the
bound is not a byte too tight.

The test is in receivebits and not at its call sites because receiveints calls it in a loop, so a
test outside would let the round that runs off the end do so before the next test was reached. It
costs a compare, a subtract and a branch that is never taken, per call rather than per bit: adk-
equilibrium at ABS 1e-4, 82 MB of packed bytes over 42 M elements, decodes in 0.400 s where it
decoded in 0.390, 2.8 per cent, and 1.9 per cent of the wall clock end to end. Counting bits rather
than bytes is what keeps it that low -- the same test written as the bytes this call will take,
which has to work out whether a partial tail needs another one, cost 6.7 per cent -- and most of
what is left is the extra word in DataBuffer rather than the test: carrying the word and not testing
it costs 1.8.

Two operations on the path to this were already undefined, bounding the cursor leaves both
reachable, since both run before the first byte is read, and so both are fixed here. sizeofint
doubles num until it passes size, so a size of 2^30 or more takes it past INT_MAX; UBSan on the
frame above reports a left shift of negative value -2147483648. Doubling in unsigned returns the
same width for every size, and the encoder's range guard holds every size it asks about below 2^30,
so no stream it wrote came near. receivebits then formed its mask as 1 << 32 -- shift exponent 32 is
too large for 32-bit type 'int'. Building the mask in unsigned gives the identical mask at every
width below 32, which is where that same guard holds a well-formed stream; comparing the width as
unsigned lets the one test cover a width that arrives negative as well.

Nothing here may change what the encoder emits or what the decoder returns, and nothing does.
ALGO_BIOMD and ALGO_BIOMDXTC over peg-1chain, ifabp-water and adk-equilibrium at ABS 1e-2, 1e-3 and
1e-4 give the same eighteen pairs of sha256 sums, compressed and decompressed, as before -- and as
the two passes before this one. 240 further round trips, over twenty frame sizes from one triplet
up, four value distributions and three error bounds, are byte-identical on both sides too.

Co-Authored-By: Claude Opus 5 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants