Skip to content

Fix segfault in JIT#18289

Closed
realFlowControl wants to merge 1 commit into
php:PHP-8.4from
realFlowControl:florian/fix-opline-in-jit
Closed

Fix segfault in JIT#18289
realFlowControl wants to merge 1 commit into
php:PHP-8.4from
realFlowControl:florian/fix-opline-in-jit

Conversation

@realFlowControl

Copy link
Copy Markdown
Contributor

This PR saves the opline, as otherwise it can be a dangling pointer

@realFlowControl realFlowControl force-pushed the florian/fix-opline-in-jit branch from ce29717 to abac1f6 Compare April 10, 2025 05:46
@realFlowControl realFlowControl changed the base branch from master to PHP-8.4 April 10, 2025 05:47
@realFlowControl realFlowControl marked this pull request as ready for review April 10, 2025 14:50
@dstogov

dstogov commented Apr 11, 2025

Copy link
Copy Markdown
Member

Hi, can you demonstrate the problem with a test case?

@morrisonlevi

Copy link
Copy Markdown
Contributor

No easy reproducer at this time. The issue is happening with Datadog's allocation profiler. We're working on verifying the issue manually with the customer which hit the issue.

The rough idea at this time: if this path is taken, and allocations start happening then the profiler may gather a sample. Then we read a bad opline when walking the stack and collecting file and line information.

@dstogov

dstogov commented Apr 11, 2025

Copy link
Copy Markdown
Member

@arnaud-lb doesn't this look similar to the problem you are fixing in #18297
@realFlowControl @morrisonlevi can you check the fix from that PR.

@arnaud-lb arnaud-lb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@dstogov I think this is a different issue.

This looks similar the classic issue where emitting an error from the allocator crashes because EX(opline) is not initialized (we crash when fetching the file/line for the error message). The usual fix is to save opline in op handlers that may allocate.

In this case the op handler is zend_jit_func_counter_helper() or zend_jit_loop_counter_helper(), which call zend_jit_hot_func(), so saving opline there seems fine.

For zend_jit_trace_counter_helper() we save in zend_jit_trace_hot_root() (here).

@dstogov dstogov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

OK. You convinced me. Approved.

@arnaud-lb arnaud-lb closed this in 061b46e Apr 15, 2025
@arnaud-lb

Copy link
Copy Markdown
Member

Thank you @realFlowControl!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants