Re: [PATCH 3/4] target/arm: Take an exception if PC is misaligned

[Peter Maydell]

On Thu, 19 Aug 2021 at 17:57, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 8/19/21 6:50 AM, Richard Henderson wrote:
> > On 8/19/21 3:40 AM, Peter Maydell wrote:
> >>>       uint32_t insn;
> >>>       bool is_16bit;
> >>>
> >>> -    if (arm_pre_translate_insn(dc)) {
> >>> +    if (arm_check_ss_active(dc) || arm_check_kernelpage(dc)) {
> >>
> >>
> >> Is it not possible to get a misaligned PC in the Thumb case ?
> >
> > No.  The thumb bit is always removed, leaving all pc aligned mod 2.
> > Both BXWritePC and BranchWritePC do this, as do we in gen_bx and store_reg.
>
> Do you think it's worth an assert here to make sure we never miss a case?  I 
> did an audit
> of the exception code and it looks like we mask everything correctly there, 
> but...

(Did you check the M-profile code too? That also architecturally I think
should never let PC have the low bit set; hopefully the code I wrote
actually ensures that...)

I guess an assert() is more helpful than ploughing ahead with
a misaligned PC value. If we don't assert we should at least have
a comment saying misaligned Thumb PCs are architecturally impossible.

If we do go for the assert, then the comment in arm_cpu_gdb_write_register()
about why we don't let GDB set bit 0 in the PC would need updating (there
would now be two reasons). We should probably also fail the migration if
we get an unaligned Thumb PC in the inbound data.

-- PMM