This avoids the possiblity of the slots being misaligned and allows sparse
initialization in the future.
Details
- Reviewers
jilles jhb - Commits
- rS305262: Initialize lists of signals using C99 designators
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Are you planning on bumping NSIG? The next two slots are already used in FreeBSD (SIGTHR and SIGLIBRT) but aren't documented here. Alternatively, are you planning on removing some signals in an alternate ABI?
For CHERI we've bumped NSIG and currently add SIGTHR and SIGLIBRT to the various lists. It's not clear to me if that's desirable or if we should just do a sparse initialization. Mostly though it seems like bad form to use a comment when we could use a designators.
lib/libc/gen/siglist.c | ||
---|---|---|
39 ↗ | (On Diff #19559) | I'm not sure it makes sense to retain this entry since I suspect it exists for padding. |
So I've assumed that bumping NSIG is a bit fraught with peril since sys_siglist, etc. are exposed in the ABI. However, I have made libsysdecode handle SIGTHR and SIGLBRT (as well as names for real-time signals) in a pending update to libsysdecode. I used a similar approach when implementing proc_sig2str() for libproc. I guess in general I considered the current APIs to be "legacy" since they aren't able to handle things like SIGRT<n> and prefer to use the libsysdecode API going forward.
https://github.com/freebsd/freebsd/compare/master...bsdjhb:libsysdecode
Is the next batch of libsysdecode changes I expect to hit the tree relatively soon. It needs a tinderbox run and new manpages, but I've already checked for regressions in truss/kdump on both amd64 and i386 already.
If we're planning to abandon this interface in place then skipping this change probably makes sense. In CheriBSD we may have not worried about the size of sys_siglist and friends in the API.
If we aren't going to be able to update this interface, then we should probably slap deprecation annotations on it one of these days so we can eventually remove it on new architectures.
This change is good but I do not see any follow-up changes.
Changing NSIG is an ABI breakage because of copy relocations. Non-PIE executables hard-code the size of sys_signame and sys_siglist when they use it, and have rtld copy libc's data to the executable's data segment so they can access it directly. Without extra work like SVN r255108 for sys_errlist, this can even break even libc's internal use of the tables.
A second problem with sys_signame and sys_siglist is that pointers in relocatable data segments have to be fixed up by rtld, even if the process will never use the tables. These are relative relocations which are fast to resolve but they still dirty memory pages.
An interface based on functions can have a stable ABI independent of the number of signals and can store the strings more efficiently. There is no reason to use sys_siglist because strsignal() exists but there is no good alternative to sys_signame. (Note that for /bin/sh I will not add additional DSOs so libsysdecode APIs are useless to me.)
lib/libc/gen/siglist.c | ||
---|---|---|
39 ↗ | (On Diff #19559) | This entry ensures that no null pointer results for the index 0 which is sometimes valid in addition to a signal number. Removing it seems to give potential for null pointer dereferences for little gain. |
We could move a libsysdecode-like API into libc with another name. Solaris has str2sig(), though it just returns "KILL", not "SIGKILL". (That is a good match for sys_signame though.)