Page MenuHomeFreeBSD

Initialize lists of signals using C99 designators.
ClosedPublic

Authored by brooks on Aug 22 2016, 10:33 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mar 30 2024, 1:51 AM
Unknown Object (File)
Dec 20 2023, 3:18 AM
Unknown Object (File)
Sep 13 2023, 10:06 PM
Unknown Object (File)
Sep 12 2023, 5:44 PM
Unknown Object (File)
Jun 25 2023, 10:50 PM
Unknown Object (File)
Jun 19 2023, 10:11 PM
Unknown Object (File)
Jun 10 2023, 2:09 PM
Unknown Object (File)
May 27 2023, 8:39 PM
Subscribers

Details

Summary

This avoids the possiblity of the slots being misaligned and allows sparse
initialization in the future.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

brooks retitled this revision from to Initialize lists of signals using C99 designators..
brooks updated this object.
brooks edited the test plan for this revision. (Show Details)
brooks added reviewers: jilles, jhb.

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.

jilles edited edge metadata.

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.

This revision is now accepted and ready to land.Aug 23 2016, 9:23 PM

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.)

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.)

This revision was automatically updated to reflect the committed changes.
brooks marked 2 inline comments as done.