Skip to content

Conversation

@robn
Copy link
Member

@robn robn commented Nov 7, 2025

Motivation and Context

The abigail ABI checker is very good, but is hard to use locally, not least because it's output is very dependent on the version used. It's great for CI to catch unexpected changes, less good when you're doing things that are actively trying to make changes and you want to see what the results are.

This PR adds a lighter alternative: produce the list of symbols visible in the shared objects for the linker to use. This won't cover every possible ABI break, but will at least show when a symbol is added or removed. And, it can be run easily; it only needs the common nm utility and any changes are immediately visible via git diff.

Description

It's very simple. Run make syms. Once the '.so' file is generated, it runs nm -DUj to get the list of exported symbols, sorts them, and writes them to files in the lib dirs. If they've changed, git will notice, and they can be inspected and either the errant thing fixed, or the changes committed.

I haven't hooked it up as part of any standard build yet though, because it seems a bit too variable. For example, presence of strlcpy and strlcat varies depending on the target libc, and there are obvious differences between platforms. It might need to get smarter, eg understand differences based on config or platform, but I don't want to get into the weeds. I'm aiming for a simple early warning, not a whole complex replacement for checkabi.

I wrote this to support a side project to clean up all our library exports (we currently expose symbols from transitive internal dependencies, which is a lot of cruft that shouldn't be part of our ABI). So this might be better left until I've finished that cleanup. Or, maybe its better to just merge the code but not the outputs yet. Discuss!

How Has This Been Tested?

It is its own thing, so no testing outside it.

After a completed build, run it:

$ make syms
  SYMS      libnvpair.so
  SYMS      libuutil.so
  SYMS      libzfs_core.so
  SYMS      libzfs.so
  SYMS      libzfsbootenv.so
  SYMS      libzpool.so

or verbosely:

$ make syms V=1
nm -DUj /home/robn/code/zfs/.libs/libnvpair.so | env LC_ALL=C sort >> lib/libnvpair/libnvpair.syms
nm -DUj /home/robn/code/zfs/.libs/libuutil.so | env LC_ALL=C sort >> lib/libuutil/libuutil.syms
nm -DUj /home/robn/code/zfs/.libs/libzfs_core.so | env LC_ALL=C sort >> lib/libzfs_core/libzfs_core.syms
nm -DUj /home/robn/code/zfs/.libs/libzfs.so | env LC_ALL=C sort >> lib/libzfs/libzfs.syms
nm -DUj /home/robn/code/zfs/.libs/libzfsbootenv.so | env LC_ALL=C sort >> lib/libzfsbootenv/libzfsbootenv.syms
nm -DUj /home/robn/code/zfs/.libs/libzpool.so | env LC_ALL=C sort >> lib/libzpool/libzpool.syms

So I make a change for some reason. Maybe I don't want to be able to increment 8-bit values anymore.

diff --git lib/libspl/atomic.c lib/libspl/atomic.c
index b5cc4ab2a5..3a0f869bee 100644
--- lib/libspl/atomic.c
+++ lib/libspl/atomic.c
@@ -36,7 +36,6 @@
 		(void) __atomic_add_fetch(target, 1, __ATOMIC_SEQ_CST);	\
 	}
 
-ATOMIC_INC(8, uint8_t)
 ATOMIC_INC(16, uint16_t)
 ATOMIC_INC(32, uint32_t)
 ATOMIC_INC(64, uint64_t)

make syms, and then the diff shows:

diff --git lib/libuutil/libuutil.syms lib/libuutil/libuutil.syms
index cd39cfa680..0b3bece7ab 100644
--- lib/libuutil/libuutil.syms
+++ lib/libuutil/libuutil.syms
@@ -68,7 +68,6 @@ atomic_inc_32
 atomic_inc_32_nv
 atomic_inc_64
 atomic_inc_64_nv
-atomic_inc_8
 atomic_inc_8_nv
 atomic_inc_uchar
 atomic_inc_uchar_nv
diff --git lib/libzfs/libzfs.syms lib/libzfs/libzfs.syms
index 67c9d511d8..b8e24ae43b 100644
--- lib/libzfs/libzfs.syms
+++ lib/libzfs/libzfs.syms
@@ -68,7 +68,6 @@ atomic_inc_32
 atomic_inc_32_nv
 atomic_inc_64
 atomic_inc_64_nv
-atomic_inc_8
 atomic_inc_8_nv
 atomic_inc_uchar
 atomic_inc_uchar_nv
diff --git lib/libzfs_core/libzfs_core.syms lib/libzfs_core/libzfs_core.syms
index 2f30365cbf..176b8059e9 100644
--- lib/libzfs_core/libzfs_core.syms
+++ lib/libzfs_core/libzfs_core.syms
@@ -68,7 +68,6 @@ atomic_inc_32
 atomic_inc_32_nv
 atomic_inc_64
 atomic_inc_64_nv
-atomic_inc_8
 atomic_inc_8_nv
 atomic_inc_uchar
 atomic_inc_uchar_nv
diff --git lib/libzpool/libzpool.syms lib/libzpool/libzpool.syms
index ca6aaaf0db..ea7a0e2ade 100644
--- lib/libzpool/libzpool.syms
+++ lib/libzpool/libzpool.syms
@@ -298,7 +298,6 @@ atomic_inc_32
 atomic_inc_32_nv
 atomic_inc_64
 atomic_inc_64_nv
-atomic_inc_8
 atomic_inc_8_nv
 atomic_inc_uchar
 atomic_inc_uchar_nv

(as you see, we export too much, but that's another PR another day).

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Quality assurance (non-breaking change which makes the code more robust against bugs)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

robn added 2 commits November 7, 2025 13:33
The abigail ABI checker is very good, but is hard to use locally, not
least because it's output is very dependent on the version used. It's
great for CI to catch unexpected changes, less good when you're doing
things that are actively trying to make changes and you want to see what
the results are.

This commit adds a lighter alternative: produce the list of symbols
visible in the shared objects for the linker to use. This won't cover
every possible ABI break, but will at least show when a symbol is added
or removed. And, it can be run easily; it only needs the common `nm`
utility and any changes are immediately visible via `git diff`.

Sponsored-by: https://despairlabs.com/sponsor/
Signed-off-by: Rob Norris <[email protected]>
@behlendorf
Copy link
Contributor

we currently expose symbols from transitive internal dependencies, which is a lot of cruft that shouldn't be part of our ABI). So this might be better left until I've finished that cleanup.

My initial inclination is that we'd want to get those internal dependencies cleaned up first. That way, we'll start with symbol files without all the noise. If it's helpful to the effort I'd be okay with pulling in the Makefile target but not the symbol files for now. Then at least it would be easier to generate them. Although, in practice I'm not sure how handy that would be.

@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Nov 8, 2025
@robn
Copy link
Member Author

robn commented Nov 8, 2025

Yep, same thought me. Cool, lets just hold onto it for a minute.

@robn robn marked this pull request as draft November 8, 2025 04:33
@github-actions github-actions bot added Status: Work in Progress Not yet ready for general review and removed Status: Code Review Needed Ready for review and testing labels Nov 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Status: Work in Progress Not yet ready for general review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants