Skip to content

Fix GH-16770: Tracing JIT type mismatch when returning UNDEF #16784

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

Closed
wants to merge 2 commits into from

Conversation

nielsdos
Copy link
Member

@nielsdos nielsdos commented Nov 13, 2024

When returning an UNDEF value, it actually becomes NULL. The following code took this into account:

if (return_value_info.type & MAY_BE_UNDEF) {
return_value_info.type &= ~MAY_BE_UNDEF;
return_value_info.type |= MAY_BE_NULL;
}

But the stack does not update the type to NULL, causing a mismatch.
Alternative would be to update the code I quoted above I think.

When returning an UNDEF value, it actually becomes NULL.
The following code took this into account:
https://github.com/php/php-src/blob/28344e0445bc2abae8dc5f1376aa0ff350e6d66d/ext/opcache/jit/zend_jit_trace.c#L2196-L2199

But the stack does not update the type to NULL, causing a mismatch.
@nielsdos nielsdos linked an issue Nov 13, 2024 that may be closed by this pull request
@nielsdos
Copy link
Member Author

nielsdos commented Nov 13, 2024

I'm not sure if updating the abstract stack types is enough, or if this may cause issues with JIT codegen (e.g. do we need codegen to actually change the type info of the zval or is this already handled?).

@nielsdos nielsdos marked this pull request as ready for review November 13, 2024 18:40
@nielsdos nielsdos requested a review from dstogov as a code owner November 13, 2024 18:40
Copy link
Member

@dstogov dstogov left a comment

Choose a reason for hiding this comment

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

The fix is right, but I would move this check into ZEND_RETURN handler.

					case ZEND_RETURN:
						op1_info = OP1_INFO();
						CHECK_OP1_TRACE_TYPE();
						if (opline->op1_type == IS_CONST) {
							res_type = Z_TYPE_P(RT_CONSTANT(opline, opline->op1));
						} else if (op1_type != IS_UNKNOWN) {
							res_type = op1_type;
+							if (res_type == IS_UNDEF) {
+								res_type = IS_NULL;
+							}
						}

@nielsdos nielsdos closed this in cbb3b93 Nov 14, 2024
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.

Tracing JIT type mismatch when returning UNDEF
2 participants