Skip to content

Fix asan jit #8711

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
Closed

Fix asan jit #8711

wants to merge 2 commits into from

Conversation

iluuu1994
Copy link
Member

The PR GH-8636 broke ASAN builds with Tracing JIT:
https://github.com/php/php-src/runs/6741952211?check_suite_focus=true

The second commit will not be merged, it's just here to demonstrate that the builds succeeds again.

/cc @dstogov @chen-hu-97

If you have a solution for this you can also create a PR and I'll close this one.

@iluuu1994
Copy link
Member Author

This seems to also fix GH-8696.

@dstogov
Copy link
Member

dstogov commented Jun 6, 2022

@iluuu1994 can you reproduce the problem locally (I can not) and guess what is going wrong?

@iluuu1994
Copy link
Member Author

@dstogov Unfortunately not, I don't know a lot of x86 and I could reproduce neither the ASAN failure nor the i386 segfaults, but given that there are two build failures I don't think it's a fluke.

@dstogov
Copy link
Member

dstogov commented Jun 6, 2022

@iluuu1994 OK. Please revert this.
@chen-hu-97 Please try to find the source of the problem and we will merge the patch once again.

@iluuu1994 iluuu1994 closed this in 3f557eb Jun 6, 2022
@chen-hu-97
Copy link
Contributor

@iluuu1994 OK. Please revert this. @chen-hu-97 Please try to find the source of the problem and we will merge the patch once again.

Thanks @iluuu1994 @dstogov, let me figure out how to manually trigger ASAN builds in github to reproduce this issue. Then I will try to find the source of the problem.

@iluuu1994
Copy link
Member Author

@chen-hu-97 Basically you can cherry-pick 350430a and then create a PR in your fork or https://github.com/php/php-src. Then it should be triggered on push. It takes quite a while to complete, so you might also want to comment out the Test and just run Test Tracing JIT so it finishes faster.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants