On Mon, 19 Aug 2019 at 22:38, Richard Henderson
<[email protected]> 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 <[email protected]>
> ---
> 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
