Fix issue with duplicate signal entries in the list - #293
Conversation
SIGABRT and SIGIOT are the same signal on most platforms and this meant that the "previous" signal handler was getting registered correctly the first time, and then overwritten to the same function the second time causing an infinite loop. This adds a static assert to ensure that all signals in the list are unique. It also adds another test to make sure that other things that override the signal handler can still properly chain back.
There was a problem hiding this comment.
Pull request overview
This PR addresses an infinite-loop failure mode in the POSIX backtrace signal handler when the same signal value appears more than once in the handled-signal list (e.g., SIGABRT vs SIGIOT aliases). It adds compile-time validation of signal uniqueness and introduces a manual test to verify signal-handler chaining behavior when another handler overrides SIGSEGV.
Changes:
- Add a
static_assertto ensure the backtrace signal list contains no duplicate signal values. - Refactor the POSIX signal list into a
constexpr std::arrayand adjust handler installation logic. - Update/add manual crash tests to exercise SIGABRT and an override+chain scenario for SIGSEGV.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| Tests/SupportTests.cc | Updates the manual crash test to use raise(SIGABRT) and adds a new manual test that overrides SIGSEGV and attempts to chain back to the previous handler. |
| Fleece/Support/Backtrace+signals-posix.cc | Adds compile-time uniqueness checking for the handled signals and refactors signal registration/default-action lookup. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
pasin
left a comment
pasin
left a comment
There was a problem hiding this comment.
Per my understanding, the main point of this PR is to prevent a mistake to add a duplicate signals in the signals. Look good to me.
SIGABRT and SIGIOT are the same signal on most platforms and this meant that the "previous" signal handler was getting registered correctly the first time, and then overwritten to the same function the second time causing an infinite loop. This adds a static assert to ensure that all signals in the list are unique. It also adds another test to make sure that other things that override the signal handler can still properly chain back.