Skip to content

Panic with abort() #8590

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

Merged
merged 1 commit into from
May 21, 2022
Merged

Panic with abort() #8590

merged 1 commit into from
May 21, 2022

Conversation

arnaud-lb
Copy link
Member

@arnaud-lb arnaud-lb commented May 20, 2022

This changes zend_mm_panic() so that abort() is used instead of exit(1).

The goal is to make panics more detectable by run-tests even when zend_mm_panic() was not able to write to stderr (see #8588).

An other effect is that abort() will also skip normal process termination (e.g. atexit functions, flushing stdio streams). I think that this is desirable in case of memory corruption.

@arnaud-lb arnaud-lb marked this pull request as ready for review May 20, 2022 16:00
@morrisonlevi
Copy link
Contributor

Can you explain how run-tests gets more insights for abort than non-zero exit?

@arnaud-lb
Copy link
Member Author

Currently run-tests ignores the exit status, and only cares about the termination signal. So exit(1) here was ignored by run-tests.

We should also change run-tests so that it doesn't ignore the exit status anymore.

@bwoebi
Copy link
Member

bwoebi commented May 20, 2022

@morrisonlevi We have some tests which are testing error scenarios and thus returning error codes. We'd need to teach run-tests to have an expected status code then. (which may make sense, but is more work to do)

@arnaud-lb arnaud-lb merged commit 9683812 into php:master May 21, 2022
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.

4 participants