Uh oh!
There was an error while loading. Please reload this page.
[ARM64] Change INS_bkpt to INS_brk for INS_BREAKPOINT - #892
Conversation
False positive?
|
BruceForstall
commented
Dec 17, 2019
Since arm64 shouldn't ever use bkpt, can you also:
? |
xiangzhai
commented
Dec 17, 2019
Thanks for your review! I updated the patch as you suggested, please review it again. Cheers, |
xiangzhai
commented
Dec 17, 2019
How to ci-help? |
BruceForstall
commented
Dec 18, 2019
Unfortunately, it looks like the situation is a little more complicated that it should be. I reproduced the failure and your fix on Linux, and it looks good. I tried the existing code and your fix on Windows, and it doesn't work. With the existing code (using bkpt), setting COMPlus_JitHalt to some function and running a test, the Windows windbg debugger works as desired, and stops at that instruction. With your change, windbg shows an illegal instruction exception, and kills the process. So, I think we need to leave the existing code, but #ifdef your fix (for defining INS_BREAKPOINT especially) under TARGET_UNIX. |
xiangzhai
commented
Dec 18, 2019
Updated as you suggested. Please review it again. Cheers, |
xiangzhai
commented
Dec 18, 2019
Any suggestion? |
BruceForstall
commented
Dec 18, 2019
@xiangzhai It looks like you ran into a pre-existing problem with the formatting jobs (see #990 for some discussion). I would normally encourage you to look at https://github.com/dotnet/jitutils and run "jit-format" over your code. However, it looks like that hasn't been properly updated to work with the new dotnet/runtime repo. For now, you might need to just wait until this gets fixed before re-testing. |
BruceForstall
left a comment
There was a problem hiding this comment.
Code change looks good to me. Thanks!
xiangzhai
commented
Dec 18, 2019
Thanks for your review and teaching! Cheers, |
TamarChristinaArm
commented
Dec 18, 2019
Something is odd here.. I was wondering how this was working at all on Windows if this was really a the encoding for The immediate effects what kind of signal the kernel raises on Linux, but for both I would suggest renaming |
TamarChristinaArm
commented
Dec 18, 2019
It seems the issue for GDB is that whenever it traps the GDB is expecting the breakpoint to be set through |
BruceForstall
commented
Dec 18, 2019
@TamarChristinaArm Thanks for looking. I did see that windbg disassembled what the JIT calls |
TamarChristinaArm
commented
Dec 18, 2019
Yeah, for GDB it seems that |
Thanks for your teaching!
Cheers, |
xiangzhai
commented
Dec 19, 2019
How to retrigger the CI checkers? #990 has been merged. Could you please help me to retrigger the test? Thanks, |
BruceForstall
commented
Dec 19, 2019
Looks like I managed to re-trigger it. cc @CarolEidt |
BruceForstall
commented
Dec 19, 2019
(If you click on "Details", there is a "Re-run" link on failing jobs) |
xiangzhai
commented
Dec 20, 2019
The "Re-run" link is not available to me. Perhaps I do not have the access permission? |
xiangzhai
commented
Dec 24, 2019
#990 fixed the issue or not?
Regression? Thanks, |
xiangzhai
commented
Dec 25, 2019
Merry Christmas :) |
BruceForstall
commented
Jan 2, 2020
/azp run |
|
You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list. |
BruceForstall
commented
Jan 2, 2020
/azp list |
BruceForstall
commented
Jan 2, 2020
I'm going to close and re-open to (hopefully) get re-testing. Note that the Windows ARM jobs are still broken (#702). |
BruceForstall
commented
Jan 2, 2020
Looks like there are actual formatting job issues: |
xiangzhai
commented
Jan 3, 2020
Welcome back :)
Before is someone might want to right alignment? Could I change the Thanks, |
BruceForstall
commented
Jan 3, 2020
You could try that. Or, you could set yourself up to run Or, you can download the patch files created by the failing CI job and apply those. From the failing Linux formatting job, it gives these instructions: Unfortunately, it looks like Azure DevOps changed their UI. But the bottom-line is: find the format.patch artifacts stored with the CI job (they will also have OS / architecture in their name), and apply them. |
xiangzhai
commented
Jan 6, 2020
No format issue any more. But I have no idea about other failures. False positive? Thanks, |
sandreenko
commented
Jan 6, 2020
Most likely from what I see, arm and crossgen-comparison are known. x64 failed during "Download native test artifacts" which I have not seen before, but it is clearly infra. |
xiangzhai
commented
Jan 6, 2020
Hi @sandreenko Thanks for your kind response! The patch is accepted and ready to land? Cheers, |
BruceForstall
commented
Jan 6, 2020
Since it looks like infrastructure issues in the CI failures, I'll go ahead and merge it. @xiangzhai Thanks for your patience and various iterations! |
xiangzhai
commented
Jan 7, 2020
Hi, You are welcome :) Cheers, |
#606
Cheers,
Leslie Zhai