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 #2030: Segmentation fault with autoloaders bailing out #2037

Merged
merged 2 commits into from
May 11, 2023
Merged

Conversation

bwoebi
Copy link
Collaborator

@bwoebi bwoebi commented May 5, 2023

Description

We fix this issue by ensuring that the opline is always connected to its proper execute_data and popping newer entries if they aren't matching.

Note for the reviewer: the PHP 7 and PHP 8 changes are identical.

Readiness checklist

  • (only for Members) Changelog has been added to the release document.
  • Tests added for this feature/bug.

Reviewer checklist

  • Appropriate labels assigned.
  • Milestone is set.
  • Changelog has been added to the release document. For community contributors the reviewer is in charge of this task.

@bwoebi bwoebi added this to the 0.87.0 milestone May 5, 2023
@bwoebi bwoebi requested a review from a team as a code owner May 5, 2023 14:16
We fix this issue by ensuring that the opline is always connected to its proper execute_data and popping newer entries if they aren't matching.

Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>
--TEST--
Assert bailouts are gracefully handled within class autoloading
--SKIPIF--
<?php if (PHP_VERSION_ID >= 70300 && PHP_VERSION_ID < 70400) die('skip: Bailing out in autoloaders is fundamentally broken in PHP 7.3'); ?>
Copy link
Collaborator

Choose a reason for hiding this comment

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

question (non-blocking): What would be the expected behaviour in PHP 7.3, segfault?
Do we document such things somewhere, just in case another customer experiences this problem?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, this segfaults. It's plain and simple a bug in PHP 7.3 itself.

For now I'm not aware of such documentation.

Copy link
Collaborator

@morrisonlevi morrisonlevi left a comment

Choose a reason for hiding this comment

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

Approved.

}
if (prev_exception_hook) {
prev_exception_hook(ex);
}
}

void zai_interceptor_reset_resolver() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor nit.

Suggested change
void zai_interceptor_reset_resolver() {
void zai_interceptor_reset_resolver(void) {

}
if (prev_exception_hook) {
prev_exception_hook(ex);
}
}

void zai_interceptor_reset_resolver() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

And matching nit.

Suggested change
void zai_interceptor_reset_resolver() {
void zai_interceptor_reset_resolver(void) {

Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>
@bwoebi bwoebi merged commit 6f6ff33 into master May 11, 2023
1 of 35 checks passed
@bwoebi bwoebi deleted the bob/fix-2030 branch May 11, 2023 12:41
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.

None yet

3 participants