Re: [Qemu-devel] [qemu-s390x] [PATCH v3 8/9] target/s390x: Skip wout, cout helpers if op helper does not return

[David Hildenbrand <david@redhat.com>]

On 03/10/2018 21:39, Richard Henderson wrote:
> When op raises an exception, it may not have initialized the output
> temps that would be written back by wout or cout.
> 
> Cc: qemu-s390x@nongnu.org
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/s390x/translate.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index 7363aabf3a..7fad3ad8e9 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -6164,11 +6164,13 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s)
>      if (insn->help_op) {
>          ret = insn->help_op(s, &o);
>      }
> -    if (insn->help_wout) {
> -        insn->help_wout(s, &f, &o);
> -    }
> -    if (insn->help_cout) {
> -        insn->help_cout(s, &o);
> +    if (ret != DISAS_NORETURN) {
> +        if (insn->help_wout) {
> +            insn->help_wout(s, &f, &o);
> +        }
> +        if (insn->help_cout) {
> +            insn->help_cout(s, &o);
> +        }
>      }
>  
>      /* Free any temporaries created by the helpers.  */
> 

What about things like LPSW/LPWSE ? They certainly don't imply that we
had an exception.

(these two don't use wout/cout, so it is still fine, but I would prefer
a comment somewhere because otherwise it is really easy to miss that
DISAS_NORETURN makes us skip these handlers)

Apart from that

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David / dhildenb