Re: [Qemu-devel] [PATCH 5/5] tricore: reset DisasContext before generating code

[Hofstetter, Georg (EFS-GH2)]

Hi Sebastian,

in translate.c:gen_mtcr() code accesses hflags within the structure:
    if ((ctx->hflags & TRICORE_HFLAG_KUU) == TRICORE_HFLAG_SM) {
        /* since we're caching PSW make this a special case */

The code expects the HFLAG set for kernel mode in (i guess) preparation for 
supporting operation modes.
There is no code modifying these flags.
The flags were introduced in 0aaeb11 and there it looks like it was expected to 
be zeroed - maybe you know a bit more.

As having a stack variable uninitialized is generally a bad idea, so my 
proposal was to zero it as a whole, as it would be for bss variables.
Alternatively we could initialize them explicitly to zero or TRICORE_HFLAG_SM.

    ctx.hflags = TRICORE_HFLAG_SM;

Not sure though why a cpu state flag is within DisasContext.

Regards,
Georg


-----Ursprüngliche Nachricht-----
Von: Bastian Koppelmann <address@hidden> 
Gesendet: Mittwoch, 5. Juni 2019 11:02
An: David Brenken <address@hidden>; address@hidden
Cc: Biermanski, Lars (EFS-GH3) <address@hidden>; Hofstetter, Georg (EFS-GH2) 
<address@hidden>; Brenken, David (EFS-GH2) <address@hidden>; Rasche, Robert 
(EFS-GH2) <address@hidden>; Konopik, Andreas (EFS-GH2) <address@hidden>
Betreff: Re: [Qemu-devel] [PATCH 5/5] tricore: reset DisasContext before 
generating code

Hi,

On 6/5/19 8:11 AM, David Brenken wrote:
> From: Georg Hofstetter <address@hidden>
>
> Signed-off-by: Andreas Konopik <address@hidden>
> Signed-off-by: David Brenken <address@hidden>
> Signed-off-by: Georg Hofstetter <address@hidden>
> Signed-off-by: Robert Rasche <address@hidden>
> Signed-off-by: Lars Biermanski <address@hidden>
>
> ---
>   target/tricore/translate.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/target/tricore/translate.c b/target/tricore/translate.c 
> index db09f82c31..cdbc00d654 100644
> --- a/target/tricore/translate.c
> +++ b/target/tricore/translate.c
> @@ -8811,6 +8811,7 @@ void gen_intermediate_code(CPUState *cs, 
> TranslationBlock *tb, int max_insns)
>       target_ulong pc_start;
>       int num_insns = 0;
>   
> +    memset(&ctx, 0x00, sizeof(DisasContext));
>       pc_start = tb->pc;
>       ctx.pc = pc_start;
>       ctx.saved_pc = -1;

To me this looks like fixing a symptom instead of the root cause. Which commit 
did you bisect to? Do you have a reproducer?

Cheers,

Bastian