Skip to content

Fix use-of-uninitialized-value of EG(last_fatal_error_backtrace) with ZTS#17639

Merged
iluuu1994 merged 1 commit into
php:masterfrom
iluuu1994:fix-uouv-last_fatal_error_backtrace
Jan 31, 2025
Merged

Fix use-of-uninitialized-value of EG(last_fatal_error_backtrace) with ZTS#17639
iluuu1994 merged 1 commit into
php:masterfrom
iluuu1994:fix-uouv-last_fatal_error_backtrace

Conversation

@iluuu1994
Copy link
Copy Markdown
Member

@iluuu1994 iluuu1994 commented Jan 30, 2025

Static variables are zeroed, but ts memory is not. Hence, we need to do it ourselves.

See https://github.com/php/php-src/actions/runs/13043905843/job/36391169998.

@iluuu1994 iluuu1994 requested a review from TimWolla January 30, 2025 17:00
@iluuu1994 iluuu1994 requested a review from dstogov as a code owner January 30, 2025 17:00
TimWolla
TimWolla approved these changes Jan 30, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@TimWolla TimWolla left a comment

Choose a reason for hiding this comment

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

Static variables are zeroed, but ts memory is not.

Makes me wonder if the first thing happening in GINIT should be a memset() to avoid this kind of bug.

Patch LGTM. /cc @ericnorris

@iluuu1994
Copy link
Copy Markdown
Member Author

iluuu1994 commented Jan 30, 2025

Hmm... unfortunately executor_globals_ctor() is called twice, once by tsrm_update_active_threads() and once by zend_post_startup(). The ini initialization happens in-between, so fatal_error_backtrace_on is initialized to false again. We could just initialize it to true, but this might lead to slight differences between zts/nts before ini's are loaded.

@iluuu1994
Fix use-of-uninitialized-value of EG(last_fatal_error_backtrace) with…
c7bff1e 
… ZTS

Static variables are zeroed, but ts memory is not. Hence, we need to do
it ourselves.
@iluuu1994 iluuu1994 force-pushed the fix-uouv-last_fatal_error_backtrace branch from 9931ba7 to c7bff1e Compare January 31, 2025 11:26
@iluuu1994
Copy link
Copy Markdown
Member Author

iluuu1994 commented Jan 31, 2025

Honestly, I have no idea why executor_globals_ctor() is being called twice, but I now moved the initialization directly after ts_allocate_fast_id(), which seems to work fine.

@iluuu1994 iluuu1994 merged commit 16c9652 into php:master Jan 31, 2025
@ericnorris
Copy link
Copy Markdown
Contributor

ericnorris commented Feb 3, 2025

Sorry, I ended up being a bit busier than usual at the end of last week. Thanks @iluuu1994!

@paresy paresy mentioned this pull request Mar 1, 2026
Open
@paresy paresy mentioned this pull request May 15, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TimWolla TimWolla TimWolla approved these changes

@dstogov dstogov Awaiting requested review from dstogov dstogov is a code owner

Assignees

No one assigned

Labels

Category: Engine

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@iluuu1994 @ericnorris @TimWolla