On Thu, 19 Aug 2021 at 17:57, Richard Henderson
<richard.hender...@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
