i#1569 AArch64: Print instruction if encoding fails. #2315
Conversation
As the parser for AArch64 is not complete yet dumping instructions that could not be encoded makes it easier to diagnose encoding problems.
|
Xref #2112 |
|
Make sure any issue hit on Travis or Appveyor has the Hotlist-Travis label. At some point we'll have a Travis Fixit and try to get all-hand-on-board for a burst of flaky test fixing, using that label to find all such problems. |
|
I added the label. |
| @@ -46,7 +46,6 @@ | |||
|
|
|||
| #include "codec.h" | |||
|
|
|||
| #define ENCFAIL (uint)0 /* a value that is not a valid instruction */ | |||
There was a problem hiding this comment.
Isn't ENCFAIL used in codec.c below? Wouldn't this make it no longer compile?
There was a problem hiding this comment.
I moved the define to codec.h, which is included by codec.c
There was a problem hiding this comment.
Hmm, I misread it, thought it was moved to encode.c.
core/arch/aarch64/encode.c
Outdated
| @@ -160,6 +160,11 @@ instr_encode_arch(dcontext_t *dcontext, instr_t *instr, byte *copy_pc, byte *fin | |||
| CLIENT_ASSERT(instr_operands_valid(instr), "instr_encode error: operands invalid"); | |||
|
|
|||
| *(uint *)copy_pc = encode_common(final_pc, instr); | |||
| if (*(uint *)copy_pc == ENCFAIL) { | |||
| IF_DEBUG(instr_disassemble(dcontext, instr, STDERR)); | |||
There was a problem hiding this comment.
Everything printed should go through the checks for stderr_mask. How about instr_disassemble_to_buffer and then print via SYSLOG_INTERNAL_ERROR which has the proper checks and also tees the print to the log file.
There was a problem hiding this comment.
Done in latest commit. I've also added a short message using SYSLOG_INTERNAL_ERROR, it should be much better now than just using instr_disassemble :)
core/arch/aarch64/encode.c
Outdated
| /* We were unable to encode this instruction. */ | ||
| IF_DEBUG({ |
There was a problem hiding this comment.
DODEBUG is what's generally used but the difference is minor.
|
I think my recent change to static_signal may have made it flaky on Travis. |
|
It looks like the bottom of the commit message was not fully edited on the squash-and-merge: please remember to edit the message, as github just squashes the different commit messages together. |
|
There's also weird whitespace: |
As the parser for AArch64 is not complete yet dumping instructions that could
not be encoded makes it easier to diagnose encoding problems.