Stop exporting the built-in filter class structs by brtnfld · Pull Request #6764 · HDFGroup/hdf5

brtnfld · 2026-10-21T04:47:37Z

Describe your changes

libhdf5 exported the class structs of its built-in filters (H5Z_DEFLATE, H5Z_SHUFFLE, H5Z_FLETCHER32, H5Z_NBIT, H5Z_SCALEOFFSET, H5Z_SZIP). They are declared in the private H5Zpkg.h, but H5_DLLVAR and default symbol visibility put them in the shared library's exported symbols.

Filter plugins built from a copy of HDF5's filter source define globals with the same names; netCDF-C's deflate, shuffle and fletcher32 plugins do. On Linux, when such a plugin is loaded into a process that already has libhdf5, the plugin's reference to its own struct binds to libhdf5's copy, so H5PLget_plugin_info() returns libhdf5's struct and filter function. This has been silent because the structs matched. #6710 changes the built-in structs to H5Z_class3_t with version 3; netCDF-C's NCZarr loader then rejects the plugins (libnczarr/zplugins.c:413) and the netCDF CI job fails.

This makes the six structs static, the same way the VFD and native VOL class structs already are. Each filter file now registers its own filter through H5Z__<filter>_register() (like H5FD__sec2_register()), so no pointer to the struct leaves its file; the szip encoder check moved into H5Z__szip_register(). Internal linkage keeps them out of the export table with any compiler, without visibility attributes.

test/dsets.c: new test_filter_class_symbols() checks with dlsym(RTLD_DEFAULT, ...) that none of the six names is exported. It fails on develop and passes with this change, and is skipped where RTLD_DEFAULT is not available.
With Implement thread-safe filters #6710's changes plus this fix, netCDF-C's nczarr_test passes 34/34 locally (23 fail without it).

Once #6710 merges, this needs a rebase onto the H5Z_class3_t structs; the change itself is the same.

Issue ticket number (GitHub or JIRA)

Related to #6710 (netCDF CI failure).

github-actions · 2026-10-21T04:51:39Z

Review Checklist

This PR touches the following areas. Each needs a sign-off
from its listed owners before merging.

src ✅ — @jhendersonHDF
test ✅ — @jhendersonHDF
release_docs ✅ — @hyoklee

✅ All areas have been signed off.

jhendersonHDF · 2026-10-21T15:36:31Z

+ *-------------------------------------------------------------------------
+ */
+const H5Z_class2_t *
+H5Z__scaleoffset_class(void)


Just as a design suggestion, these functions could alternatively self-register the filter so that they don't return a pointer to internal details from a function that is exposed

I agree, I was going to make the same suggestion, create H5Z__deflate_register(), etc. analogous to H5FD__sec2_register(). These functions would then call H5Z_register with their private class structs. While the way it is now works, it is slightly strange (if legal) to pass the address of a static variable outside the scope of that variable. I think it would be more straightforward to use e.g. H5Z__deflate_register(), and it would be more in line with the rest of the library.

Done in c621bc5. Each filter file now has H5Z__<filter>_register(), like H5FD__sec2_register(), and the szip encoder check moved into H5Z__szip_register(). The structs never leave their files now; test/dsets.c gets the szip class through H5Z_find().

The class structs of the built-in filters (H5Z_DEFLATE, H5Z_SHUFFLE, H5Z_FLETCHER32, H5Z_NBIT, H5Z_SCALEOFFSET, H5Z_SZIP) were exported from libhdf5. Filter plugins built from a copy of HDF5's filter source, such as netCDF-C's deflate, shuffle and fletcher32 plugins, define globals with the same names, and on ELF platforms the plugin's references to its own struct bound to libhdf5's copy instead. Make the structs static, as the VFD and native VOL class structs already are, and reach them through package accessors. Add a test that checks none of the names is exported.

Replace the H5Z__<filter>_class() accessors with H5Z__<filter>_register() functions that call H5Z_register() on the file's static class struct, as H5FD__sec2_register() and the other VFDs do, so no pointer to the internal struct leaves its file. The szip encoder check moves into H5Z__szip_register(), and test/dsets.c looks the szip class up with H5Z_find().

jhendersonHDF · 2026-10-21T20:48:43Z

+
+#if defined(H5_HAVE_DLFCN_H) && defined(RTLD_DEFAULT)
+    for (u = 0; u < NELMTS(names); u++)
+        if (dlsym(RTLD_DEFAULT, names[u]) != NULL) {


This is maybe fine as a quick hack but consider that projects already appear to be exporting these symbols publicly as well, so if HDF5 is being built as a sub-project and this test is run, it has the potential to fail for reasons unrelated to what this is testing for.

Good point, fixed in 4b7e5a6. The test now looks the names up in libhdf5's own shared object (found with dladdr() on H5open, opened with RTLD_NOLOAD) and only fails if the symbol found is inside libhdf5, so a definition elsewhere in the process doesn't count. It's skipped for static builds.

I checked it by preloading a library that exports its own H5Z_DEFLATE: the test passes against this PR's library and still fails when libhdf5 exports the structs. Incidentally, with that preload the unfixed libhdf5 itself breaks: its own references to H5Z_DEFLATE bind to the foreign definition, and dsets segfaults in the filter tests. This PR fixes that too.

Do you think we need a test for this, in general, or should I remove it?

Generally speaking I would avoid testing for specific symbol names at all since it can be hard to predict what is going to be exported at any given time, especially when dlopen() is involved (let's say the library dlopens a filter like blosc which may in turn dlopen its own copy of an H5Z_SZIP).

…lass structs) # Conflicts: # release_docs/CHANGELOG.md

dlsym(RTLD_DEFAULT, ...) searches the whole process, so the test failed when another object, such as a project that builds HDF5 as a sub-project, defines one of these names. Look the names up in the library's own shared object instead and only count a symbol found inside it. Skip the test when the library is not a shared object.

hyoklee · 2026-10-21T23:53:02Z

+
+   The class structs of the built-in filters (H5Z_DEFLATE, H5Z_SHUFFLE, H5Z_FLETCHER32,
+   H5Z_NBIT, H5Z_SCALEOFFSET and H5Z_SZIP) were exported from the shared library. Filter
+   plugins built from a copy of HDF5's filter source, such as netCDF-C's deflate, shuffle


shuffle -> shuffle,

Applied in 02a95af, thanks.

hyoklee · 2026-10-21T23:53:07Z

+   The class structs of the built-in filters (H5Z_DEFLATE, H5Z_SHUFFLE, H5Z_FLETCHER32,
+   H5Z_NBIT, H5Z_SCALEOFFSET and H5Z_SZIP) were exported from the shared library. Filter
+   plugins built from a copy of HDF5's filter source, such as netCDF-C's deflate, shuffle
+   and fletcher32 plugins, define globals with the same names, and on ELF platforms the


ELF platforms the -> ELF platforms where the

Good catch: "bound" was meant as the verb, and it is easy to misread. "Where" would leave the clause without a verb, though, so I reworded it instead: "...on ELF platforms the plugin's references to its own struct were resolved to the library's copy instead." (02a95af). Does that read clearly now?

hyoklee · 2026-10-21T23:53:29Z

+### Stopped exporting the built-in filter class structs
+
+   The class structs of the built-in filters (H5Z_DEFLATE, H5Z_SHUFFLE, H5Z_FLETCHER32,
+   H5Z_NBIT, H5Z_SCALEOFFSET and H5Z_SZIP) were exported from the shared library. Filter


H5Z_SCALEOFFSET and H5Z_SZIP -> `H5Z_SCALEOFFSET, and H5Z_SZIP

Thanks, applied in 02a95af. The CHANGELOG uses the serial comma elsewhere, so this is more consistent.

Address review: add the serial commas the rest of the CHANGELOG uses, and replace "references ... bound to the library's copy", which read as a noun phrase, with "were resolved to the library's copy".

brtnfld · 2026-10-22T00:00:36Z

Thanks for the careful read, @hyoklee. All three are addressed in 02a95af.

One small request for future wording nits: could you post them as suggestion blocks (the "Insert a suggestion" button in the review comment box)? Then I can apply them straight from the PR in one batch commit instead of editing each one by hand. Thanks!

hyoklee

Reviewed documentation only

Testing for specific exported symbol names is fragile: what a process exports depends on what else is loaded, including filter plugins that dlopen their own copies of these names.

brtnfld · 2026-10-22T02:37:50Z

Removed. Whether a given name is exported depends on what else is loaded, so the test was checking the environment as much as the fix.

github-actions · 2026-10-22T15:00:43Z

🎉 All checklist items are signed off — @brtnfld, this PR is ready to merge.

brtnfld requested review from fortnern, glennsong09, hyoklee, jhendersonHDF, lrknox, mattjala and vchoi-hdfgroup as code owners October 21, 2026 04:47

github-project-automation Bot added this to HDF5 - TRIAGE & TRACK Oct 21, 2026

github-project-automation Bot moved this to To be triaged in HDF5 - TRIAGE & TRACK Oct 21, 2026

github-actions Bot assigned brtnfld Oct 21, 2026

github-actions Bot removed request for fortnern, glennsong09, jhendersonHDF, lrknox and vchoi-hdfgroup October 21, 2026 04:51

brtnfld requested review from fortnern and removed request for mattjala October 21, 2026 13:49

jhendersonHDF reviewed Oct 21, 2026

View reviewed changes

brtnfld requested a review from jhendersonHDF October 21, 2026 16:44

github-actions Bot removed the request for review from fortnern October 21, 2026 16:45

fortnern previously approved these changes Oct 21, 2026

View reviewed changes

brtnfld dismissed fortnern’s stale review via fd765b0 October 21, 2026 17:49

brtnfld force-pushed the fix/h5z-static-filter-classes branch from c621bc5 to fd765b0 Compare October 21, 2026 17:49

github-actions Bot requested a review from fortnern October 21, 2026 17:49

fortnern previously approved these changes Oct 21, 2026

View reviewed changes

brtnfld added 2 commits October 21, 2026 13:56

brtnfld dismissed fortnern’s stale review via 4c55ae5 October 21, 2026 18:59

brtnfld force-pushed the fix/h5z-static-filter-classes branch from fd765b0 to 4c55ae5 Compare October 21, 2026 18:59

github-actions Bot requested a review from fortnern October 21, 2026 19:16

jhendersonHDF mentioned this pull request Oct 21, 2026

Skip dsets missing-filter test for non-default-compatible VFDs #6763

Merged

2 of 3 tasks

Merge branch 'develop' into fix/h5z-static-filter-classes

bd87949

github-actions Bot removed the request for review from jhendersonHDF October 21, 2026 20:32

jhendersonHDF reviewed Oct 21, 2026

View reviewed changes

hyoklee added a commit to hyoklee/hdf5 that referenced this pull request Oct 21, 2026

Merge upstream PR HDFGroup#6764 (Stop exporting the built-in filter c…

62cbed5

…lass structs) # Conflicts: # release_docs/CHANGELOG.md

hyoklee previously approved these changes Oct 21, 2026

View reviewed changes

CHANGELOG: serial commas, and clarify how plugin references resolved

02a95af

Address review: add the serial commas the rest of the CHANGELOG uses, and replace "references ... bound to the library's copy", which read as a noun phrase, with "were resolved to the library's copy".

brtnfld dismissed hyoklee’s stale review via 02a95af October 21, 2026 23:57

github-actions Bot requested a review from hyoklee October 21, 2026 23:57

hyoklee previously approved these changes Oct 22, 2026

View reviewed changes

Remove test_filter_class_symbols

b385b04

Testing for specific exported symbol names is fragile: what a process exports depends on what else is loaded, including filter plugins that dlopen their own copies of these names.

brtnfld dismissed hyoklee’s stale review via b385b04 October 22, 2026 02:02

github-actions Bot requested a review from hyoklee October 22, 2026 02:03

hyoklee approved these changes Oct 22, 2026

View reviewed changes

jhendersonHDF approved these changes Oct 22, 2026

View reviewed changes

github-actions Bot added the checklist-complete All review checklist areas are signed off label Oct 22, 2026

Conversation

brtnfld commented Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Describe your changes

Issue ticket number (GitHub or JIRA)

Uh oh!

github-actions Bot commented Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brtnfld Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jhendersonHDF Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hyoklee Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brtnfld commented Oct 22, 2026

Uh oh!

hyoklee left a comment

Choose a reason for hiding this comment

Uh oh!

brtnfld commented Oct 22, 2026

Uh oh!

github-actions Bot commented Oct 22, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

brtnfld commented Oct 21, 2026 •

edited

Loading

github-actions Bot commented Oct 21, 2026 •

edited

Loading

brtnfld Oct 21, 2026 •

edited

Loading

jhendersonHDF Oct 22, 2026 •

edited

Loading

hyoklee Oct 21, 2026 •

edited

Loading