Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix SIGINT handler to use sigaction with SA_ONSTACK flag #7312

Closed

Conversation

NikolajBjorner
Copy link
Contributor

@NikolajBjorner NikolajBjorner commented Jul 27, 2024

Related to #7305

Update SIGINT signal handler to use sigaction with SA_ONSTACK flag.

  • Replace signal function with sigaction function in src/util/scoped_ctrl_c.cpp to register the SIGINT handler.
  • Set the SA_ONSTACK flag for the SIGINT signal handler in src/util/scoped_ctrl_c.cpp.
  • Update on_ctrl_c function to use sigaction in src/util/scoped_ctrl_c.cpp.
  • Modify scoped_ctrl_c struct in src/util/scoped_ctrl_c.h to include struct sigaction m_old_action instead of void (STD_CALL *m_old_handler)(int).

For more details, open the Copilot Workspace session.

Related to Z3Prover#7305

Update SIGINT signal handler to use `sigaction` with `SA_ONSTACK` flag.

* Replace `signal` function with `sigaction` function in `src/util/scoped_ctrl_c.cpp` to register the SIGINT handler.
* Set the `SA_ONSTACK` flag for the SIGINT signal handler in `src/util/scoped_ctrl_c.cpp`.
* Update `on_ctrl_c` function to use `sigaction` in `src/util/scoped_ctrl_c.cpp`.
* Modify `scoped_ctrl_c` struct in `src/util/scoped_ctrl_c.h` to include `struct sigaction m_old_action` instead of `void (STD_CALL *m_old_handler)(int)`.

---

For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/Z3Prover/z3/issues/7305?shareId=XXXX-XXXX-XXXX-XXXX).
m_old_handler = signal(SIGINT, on_ctrl_c);
struct sigaction sa;
sa.sa_handler = on_ctrl_c;
sa.sa_flags = SA_ONSTACK;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but without calling sigaltstack, isn't this useless?

My man page says:

Call the signal handler on an alternate signal stack
provided by sigaltstack(2). If an alternate stack is not
available, the default stack will be used. This flag is
meaningful only when establishing a signal handler.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is what Github WorkSpaces provided in the first shot.
Besides looking suspicious, it also isn't cross-platform. For Windows, we would not use this.
Then also, I am not sure if the code in 'shell' is broken because it only considers sigint.
Altogether more than a quick change, but something that you as the highest esteemed and glorious expert in operating sophisticated machinery may find a spare moment to consider when not occupied near sunny grass lawns or ocean side camps (it is August, isn't it?).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants