Fix #2030: Segmentation fault with autoloaders bailing out #2037
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| --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'); ?> | ||
| --ENV-- | ||
| DD_TRACE_DEBUG=1 | ||
| --FILE-- | ||
| <?php | ||
|
|
||
| ddtrace\trace_function("x", function() { class C extends D {} print "Should not appear...\n"; }); | ||
|
|
||
| function x() {} | ||
|
|
||
| spl_autoload_register(function() use (&$s) { | ||
| if ($s) { | ||
| trigger_error("No D", E_USER_ERROR); | ||
| } else { | ||
| $s = true; | ||
| x(); | ||
| class B {} | ||
| print "Leaving Autoloader\n"; | ||
| } | ||
| }); | ||
|
|
||
| class A extends B {} | ||
|
|
||
| ?> | ||
| --EXPECTF-- | ||
| Error raised in ddtrace's closure defined at %s:%d for x(): No D in %s | ||
| Leaving Autoloader | ||
| Flushing trace of size 2 to send-queue for %s | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -818,9 +818,16 @@ static void zai_hook_memory_dtor(zval *zv) { | |
| efree(Z_PTR_P(zv)); | ||
| } | ||
|
|
||
| #if PHP_VERSION_ID < 80200 | ||
| void zai_interceptor_reset_resolver(void); | ||
| #endif | ||
| void zai_interceptor_activate(void) { | ||
| zend_hash_init(&zai_hook_memory, 8, nothing, zai_hook_memory_dtor, 0); | ||
| zend_hash_init(&zai_interceptor_implicit_generators, 8, nothing, NULL, 0); | ||
|
|
||
| #if PHP_VERSION_ID < 80200 | ||
| zai_interceptor_reset_resolver(); | ||
| #endif | ||
| } | ||
|
|
||
| void zai_interceptor_deactivate(void) { | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -82,7 +82,7 @@ PHP_FUNCTION(zai_interceptor_resolve_after_class_alias) { | |||||
|
|
||||||
| static zend_op zai_interceptor_post_declare_op; | ||||||
| ZEND_TLS zend_op zai_interceptor_post_declare_ops[4]; | ||||||
| struct zai_interceptor_opline { const zend_op *op; const zend_execute_data *execute_data; struct zai_interceptor_opline *prev; }; | ||||||
| ZEND_TLS struct zai_interceptor_opline zai_interceptor_opline_before_binding = {0}; | ||||||
| static void zai_interceptor_install_post_declare_op(zend_execute_data *execute_data) { | ||||||
| // We replace the current opline *before* it is executed. Thus we need to preserve opline data first: | ||||||
|
|
@@ -110,10 +110,24 @@ static void zai_interceptor_install_post_declare_op(zend_execute_data *execute_d | |||||
| zai_interceptor_opline_before_binding.prev = backup; | ||||||
| } | ||||||
| zai_interceptor_opline_before_binding.op = EX(opline); | ||||||
| zai_interceptor_opline_before_binding.execute_data = execute_data; | ||||||
| EX(opline) = zai_interceptor_post_declare_ops; | ||||||
| } | ||||||
|
|
||||||
| static void zai_interceptor_pop_opline_before_binding(zend_execute_data *execute_data) { | ||||||
| // Normally the zai_interceptor_opline_before_binding stack should be in sync with the actual executing stack, but it might not after bailouts | ||||||
| if (execute_data) { | ||||||
| if (zai_interceptor_opline_before_binding.execute_data == execute_data) { | ||||||
| return; | ||||||
| } | ||||||
|
|
||||||
| while (zai_interceptor_opline_before_binding.prev && zai_interceptor_opline_before_binding.prev->execute_data != execute_data) { | ||||||
| struct zai_interceptor_opline *backup = zai_interceptor_opline_before_binding.prev; | ||||||
| zai_interceptor_opline_before_binding = *backup; | ||||||
| efree(backup); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| struct zai_interceptor_opline *backup = zai_interceptor_opline_before_binding.prev; | ||||||
| if (backup) { | ||||||
| zai_interceptor_opline_before_binding = *backup; | ||||||
|
|
@@ -154,8 +168,9 @@ static int zai_interceptor_post_declare_handler(zend_execute_data *execute_data) | |||||
| } | ||||||
| } | ||||||
| // preserve offset | ||||||
| zai_interceptor_pop_opline_before_binding(execute_data); | ||||||
| EX(opline) = zai_interceptor_opline_before_binding.op + (EX(opline) - &zai_interceptor_post_declare_ops[0]); | ||||||
| zai_interceptor_pop_opline_before_binding(NULL); | ||||||
| return ZEND_USER_OPCODE_CONTINUE; | ||||||
| } else if (prev_post_declare_handler) { | ||||||
| return prev_post_declare_handler(execute_data); | ||||||
|
|
@@ -237,8 +252,9 @@ static user_opcode_handler_t prev_handle_exception_handler; | |||||
| static int zai_interceptor_handle_exception_handler(zend_execute_data *execute_data) { | ||||||
| // not everything goes through zend_throw_exception_hook, in particular when zend_rethrow_exception alone is used (e.g. during zend_call_function) | ||||||
| if (EG(opline_before_exception) == zai_interceptor_post_declare_ops) { | ||||||
| zai_interceptor_pop_opline_before_binding(execute_data); | ||||||
| EG(opline_before_exception) = zai_interceptor_opline_before_binding.op; | ||||||
| zai_interceptor_pop_opline_before_binding(NULL); | ||||||
| } | ||||||
|
|
||||||
| return prev_handle_exception_handler ? prev_handle_exception_handler(execute_data) : ZEND_USER_OPCODE_DISPATCH; | ||||||
|
|
@@ -249,14 +265,20 @@ static void zai_interceptor_exception_hook(zend_object *ex) { | |||||
| zend_function *func = EG(current_execute_data)->func; | ||||||
| if (func && ZEND_USER_CODE(func->type) && EG(current_execute_data)->opline == zai_interceptor_post_declare_ops) { | ||||||
| // called right before setting EG(opline_before_exception), reset to original value to ensure correct throw_op handling | ||||||
| zai_interceptor_pop_opline_before_binding(EG(current_execute_data)); | ||||||
| EG(current_execute_data)->opline = zai_interceptor_opline_before_binding.op; | ||||||
| zai_interceptor_pop_opline_before_binding(NULL); | ||||||
| } | ||||||
| if (prev_exception_hook) { | ||||||
| prev_exception_hook(ex); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| void zai_interceptor_reset_resolver() { | ||||||
|
There was a problem hiding this comment. And matching nit.
Suggested change
|
||||||
| // reset in case a prior request had a bailout | ||||||
| zai_interceptor_opline_before_binding = (struct zai_interceptor_opline){0}; | ||||||
| } | ||||||
|
|
||||||
| static void *opcache_handle; | ||||||
| static void zai_interceptor_find_opcache_handle(void *ext) { | ||||||
| zend_extension *extension = (zend_extension *)ext; | ||||||
|
|
||||||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.