ASSERT FAILURE: .../raw2trace.cpp:3056: *pc - desc->pc_ == instr_length(dcontext, instr) ()

[egrimley-arm]

Reproduce like this on a 32-bit Arm system:

cat > t.s <<'EOF'
        .arch   armv7-a
        .syntax unified
        .text
        .thumb
        .thumb_func
        .global _start
        .type   _start, %function
_start:
        ldr.w   r0, [sp] // Note the ".w"!
        mov     r7, #248 // SYS_exit_group
        svc     #0
EOF
gcc t.s -nostdlib
rm -rf drmemtrace.a.out.*
bin32/drrun -t drmemtrace -offline ./a.out
bin32/drrun -t drmemtrace -indir drmemtrace.a.out.*

Get:

ASSERT FAILURE: .../dynamorio/clients/drcachesim/tracer/raw2trace.cpp:3056: *pc - desc->pc_ == instr_length(dcontext, instr) ()

It appears that DEBUG_ASSERT(*pc - desc->pc_ == instr_length(dcontext, instr)) incorrectly assumes that the instruction will be encoded in a particular way. Some Thumb instructions have both a 2-byte and a 4-byte encoding, and the 4-byte encoding seems to be used instead of an alternative 2-byte encoding in the C library code that comes with Debian 10 so you might be able to reproduce this with an a.out built from int main() { return 0; }; I could.

The fix is to delete that line with that DEBUG_ASSERT?

[derekbruening]

If it has the wrong ISA mode then it's going to get everything else wrong. No, please do not remove the assert. Sounds like there's a bug in setting the mode somewhere.

[egrimley-arm]

I'm not talking about the two ISA modes: there are multiple Thumb encodings for LDR (immediate).

[derekbruening]

Oh you mean encoding fidelity is lost: that's because #4016 is missing for AArchXX and so the raw bits are lost; also xref #3271 and #3339. #4016 is what we want to solve this.

[egrimley-arm]

Just to get tests passing, do you think we could do something like the following?

#ifndef ARM /* FIXME i#4016: Reencoding a T32 instruction can change its length from 4 to 2. */
    DEBUG_ASSERT(*pc - desc->pc_ == instr_length(dcontext, instr));
#endif

[derekbruening]

OK.