On Mon, 19 Aug 2019 at 22:38, Richard Henderson <richard.hender...@linaro.org> wrote: > > This fixes an exiting bug with the T5 encoding of SUBS PC, LR, #IMM, > in that it may be executed from user mode as with any other encoding > of SUBS, not as ERET. > > Signed-off-by: Richard Henderson <richard.hender...@linaro.org> > --- > target/arm/translate.c | 119 +++++++++++++---------------------------- > target/arm/a32.decode | 8 +++ > target/arm/t32.decode | 5 ++ > 3 files changed, 50 insertions(+), 82 deletions(-)
> +static bool trans_HVC(DisasContext *s, arg_HVC *a) > +{ > + if (!ENABLE_ARCH_7 || IS_USER(s) || arm_dc_feature(s, ARM_FEATURE_M)) { > + return false; > + } > + gen_hvc(s, a->imm); > + return true; > +} I was wondering about for these trans_ functions the difference between returning 'false' and calling unallocated_encoding() and then returning 'true'. If I understand the decodetree right this will only make a difference when the pattern is inside a {} group. So for instance here we have { [...] { HVC 1111 0111 1110 .... 1000 .... .... .... \ &i imm=%imm16_16_0 CPS 1111 0011 1010 1111 1000 0 imod:2 M:1 A:1 I:1 F:1 mode:5 \ &cps UDF 1111 0111 1111 ---- 1010 ---- ---- ---- } B_cond_thumb 1111 0. cond:4 ...... 10.0 ............ &ci imm=%imm21 } which means that if the HVC returns 'false' we'll end up trying the insn as a B_cond_thumb. In this case the trans function for the B_cond_thumb pattern will correctly return false itself for a->cond >= 0xe, so it makes no difference. But maybe it would be more robust for a pattern like HVC to be self-contained so it doesn't fall through for cases that really do belong to it but happen to be required to UNDEF (like IS_USER() == true) ? OTOH I suppose you could say that when you're writing patterns like the B_cond_thumb one you know you've underdecoded and must catch all the theoretical overlaps by writing checks in the trans function, so as long as you do that correctly you're fine. I guess this isn't a request for a change, just wondering if there is a general principle for how we should structure this sort of thing. I didn't run into it with the VFP stuff because none of the decode needed groups. thanks -- PMM