On 03/03/2017 09:03 PM, Alex Bennée wrote:
We *should* have retaddr == 0 for this case, which indicates that we
should not attempt to restore state. Are you seeing a non-zero value?
Actually looking at xtensa I see:
Attempt to resolve CPU state @ 0x0 while translating
So maybe I should check just that - but I don't see where we ensure we
always pass zero.
cpu_ld*_cmmu, in include/exec/cpu_ldst_template.h, has
return glue(glue(glue(cpu_ld, USUFFIX), MEMSUFFIX), _ra)(env, ptr, 0);
so there's your zero.
#0 0x00005555555e3712 in cpu_restore_state (cpu=cpu@entry=0x555556032600,
retaddr=retaddr@entry=0) at /home/alex/lsrc/qemu/qemu.git/translate-all.c:338
#1 0x000055555564cb38 in tlb_fill (cs=cs@entry=0x555556032600,
vaddr=vaddr@entry=537034752, access_type=access_type@entry=MMU_INST_FETCH,
mmu_idx=mmu_idx@entry=1, retaddr=retaddr@entry=0) at
/home/alex/lsrc/qemu/qemu.git/target/xtensa/op_helper.c:73
#2 0x000055555562d604 in helper_ret_ldb_cmmu (env=env@entry=0x55555603a890,
addr=537034752, oi=<optimised out>, retaddr=retaddr@entry=0) at
/home/alex/lsrc/qemu/qemu.git/softmmu_template.h:127
Confirmed. This is a simple bug in xtensa -- failure to check retaddr == 0
before calling cpu_restore_state.
That said, I do wonder if it wouldn't be better to move that check inside
cpu_restore_state instead. Put the check there now, but leave the follow-on
cleanup for the next devel cycle.
It would also save auditing the other usages of cpu_restore_state in the tree.
Is this in fact linux-user, not softmmu, as you imply from tlb_fill?
Because handle_cpu_signal will in fact pass a genuine non-zero retaddr
for the SIGSEGV resulting from a cpu_ld*_code from a non-mapped
address.
I think that is another call chain that might trip us up. Peter
mentioned he'd hit it. This one is definitely softmmu.
This is really easy to reproduce on any guest with a call to a null function
pointer.
r~
