You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
tool.scheduler.unit_tests seems to be broken on ARM/AArch32 since bd7f3eb (@derekbruening, May 2023).
The problem seems to be that memref_is_nop_instr calls decode in a way that always decodes as A32 so a T32 NOP instruction is not recognised. So I can make it pass by inserting if (*(uint *)record.instr.encoding == 0xe320f000) return true; into that function.
Would it make sense to rewrite memref_is_nop_instr so that instead of using the decoder it just recognises the encodings, like how next_recordworks? Or is there a better way?
The text was updated successfully, but these errors were encountered:
There will be many usage patterns that call decode on record.instr.encoding so we need to solve that usage pattern.
E.g., the drmemtrace view tool does the exact same thing: so won't it get all the Thumb-mode instructions wrong? The same thing with the opcode_mix tool.
This affects PR #7114 which is making a library for all analyzers for decoding: @abhinav92003
The tracer makes sure to call dr_app_pc_as_jump_target on recording PC values so that raw2trace decodes them properly. What about the instruction addresses in the trace records: do those have LSB set for thumb?
Since record.instr.encoding is a field reference, we would have to add some wrapper users have to call: if the LSB is set in the record.instr.addr this wrapper could use that to also set the LSB in .encoding. We'd have to add this to memref.h and document that it should be used instead of the .encoding directly, and update all our uses.
(The module mapper decode PC would also need to be adjusted.)
(This all assumes setting the LSB is the way to go; that seems better than explicitly setting the ISA mode somehow on every single record, right?)
If decode_from_copy() is what is already used, and record.instr.addr has LSB=1 set (does it today? seems like it should), we could make sure decode_from_copy() uses the LSB-mode-setting logic on the original PC and then not need a wrapper. We would document that anyone decoding the record.instr.encoding should use decode_from_copy().
tool.scheduler.unit_tests
seems to be broken on ARM/AArch32 since bd7f3eb (@derekbruening, May 2023).The problem seems to be that
memref_is_nop_instr
callsdecode
in a way that always decodes as A32 so a T32 NOP instruction is not recognised. So I can make it pass by insertingif (*(uint *)record.instr.encoding == 0xe320f000) return true;
into that function.Would it make sense to rewrite
memref_is_nop_instr
so that instead of using the decoder it just recognises the encodings, like hownext_record
works? Or is there a better way?The text was updated successfully, but these errors were encountered: