[Qemu-devel] [PATCH for 4.1 0/4] target/mips: Fixes for 4.1

[Qemu-devel] [PATCH for 4.1 0/4] target/mips: Fixes for 4.1

[Aleksandar Markovic]

From: Aleksandar Markovic <amarkovic@wavecomp.com>

At the moment, this includes fixes for problems in switch statements
found by GCC 8.3 improved code analysis features.

Aleksandar Markovic (4):
  target/mips: Add 'fall through' comments for handling nanoMips' SHXS,
    SWXS
  target/mips: Add missing 'break' for a case of MTHC0 handling
  target/mips: Add missing 'break' for certain cases of MFTR handling
  target/mips: Add missing 'break' for certain cases of MTTR handling

 target/mips/translate.c | 7 +++++++
 1 file changed, 7 insertions(+)

-- 
2.7.4

[Qemu-devel] [PATCH for 4.1 v2] linux-user: Fix structure target_ucontext for MIPS

[Aleksandar Markovic]

From: Aleksandar Markovic <amarkovic@wavecomp.com>

Structure ucontext for MIPS is defined in the following way in
Linux kernel:

(arch/mips/include/uapi/asm/ucontext.h, lines 54-64)

struct ucontext {
    /* Historic fields matching asm-generic */
    unsigned long       uc_flags;
    struct ucontext     *uc_link;
    stack_t             uc_stack;
    struct sigcontext   uc_mcontext;
    sigset_t            uc_sigmask;

    /* Extended context structures may follow ucontext */
    unsigned long long	uc_extcontext[0];
};

Fix the structure target_ucontext for MIPS to reflect the definition
above, except the correction for field uc_extcontext, which will
follow at some later time.

Fixes: 94c5495d

Reported-by: Dragan Mladjenovic <dmladjenovic@wavecomp.com>
Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>

---

v2: rectified a commit message mistake with
    s/target_sigset_t/target_ucontext
---
 linux-user/mips/signal.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/linux-user/mips/signal.c b/linux-user/mips/signal.c
index 6aa303e..455a8a2 100644
--- a/linux-user/mips/signal.c
+++ b/linux-user/mips/signal.c
@@ -71,10 +71,9 @@ struct sigframe {
 };
 
 struct target_ucontext {
-    target_ulong tuc_flags;
-    target_ulong tuc_link;
+    abi_ulong tuc_flags;
+    abi_ulong tuc_link;
     target_stack_t tuc_stack;
-    target_ulong pad0;
     struct target_sigcontext tuc_mcontext;
     target_sigset_t tuc_sigmask;
 };
-- 
2.7.4

Re: [Qemu-devel] [PATCH for 4.1 v2] linux-user: Fix structure target_ucontext for MIPS

[Laurent Vivier]

Le 12/07/2019 à 13:37, Aleksandar Markovic a écrit :
> From: Aleksandar Markovic <amarkovic@wavecomp.com>
> 
> Structure ucontext for MIPS is defined in the following way in
> Linux kernel:
> 
> (arch/mips/include/uapi/asm/ucontext.h, lines 54-64)
> 
> struct ucontext {
>     /* Historic fields matching asm-generic */
>     unsigned long       uc_flags;
>     struct ucontext     *uc_link;
>     stack_t             uc_stack;
>     struct sigcontext   uc_mcontext;
>     sigset_t            uc_sigmask;
> 
>     /* Extended context structures may follow ucontext */
>     unsigned long long	uc_extcontext[0];
> };
> 
> Fix the structure target_ucontext for MIPS to reflect the definition
> above, except the correction for field uc_extcontext, which will
> follow at some later time.
> 
> Fixes: 94c5495d
> 
> Reported-by: Dragan Mladjenovic <dmladjenovic@wavecomp.com>
> Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> 
> ---
> 
> v2: rectified a commit message mistake with
>     s/target_sigset_t/target_ucontext
> ---
>  linux-user/mips/signal.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/linux-user/mips/signal.c b/linux-user/mips/signal.c
> index 6aa303e..455a8a2 100644
> --- a/linux-user/mips/signal.c
> +++ b/linux-user/mips/signal.c
> @@ -71,10 +71,9 @@ struct sigframe {
>  };
>  
>  struct target_ucontext {
> -    target_ulong tuc_flags;
> -    target_ulong tuc_link;
> +    abi_ulong tuc_flags;
> +    abi_ulong tuc_link;
>      target_stack_t tuc_stack;
> -    target_ulong pad0;
>      struct target_sigcontext tuc_mcontext;
>      target_sigset_t tuc_sigmask;
>  };
> 

Applied to my linux-user branch.

Thanks,
Laurent

[Qemu-devel] [PATCH for 4.1 1/4] target/mips: Add 'fall through' comments for handling nanoMips' SHXS, SWXS

[Aleksandar Markovic]

From: Aleksandar Markovic <amarkovic@wavecomp.com>

This was found by GCC 8.3 static analysis.

Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
---
 target/mips/translate.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index f96f141..2be5e2d 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -20136,12 +20136,14 @@ static void gen_p_lsx(DisasContext *ctx, int rd, int rs, int rt)
         switch (extract32(ctx->opcode, 7, 4)) {
         case NM_SHXS:
             check_nms(ctx);
+            /* fall through */
         case NM_LHXS:
         case NM_LHUXS:
             tcg_gen_shli_tl(t0, t0, 1);
             break;
         case NM_SWXS:
             check_nms(ctx);
+            /* fall through */
         case NM_LWXS:
         case NM_LWC1XS:
         case NM_SWC1XS:
-- 
2.7.4

[Qemu-devel] [PATCH for 4.1 2/4] target/mips: Add missing 'break' for a case of MTHC0 handling

[Aleksandar Markovic]

From: Aleksandar Markovic <amarkovic@wavecomp.com>

This was found by GCC 8.3 static analysis.

Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
---
 target/mips/translate.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 2be5e2d..59d4acd 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -6745,6 +6745,7 @@ static void gen_mthc0(DisasContext *ctx, TCGv arg, int reg, int sel)
         default:
             goto cp0_unimplemented;
         }
+        break;
     case CP0_REGISTER_17:
         switch (sel) {
         case 0:
-- 
2.7.4

Re: [Qemu-devel] [PATCH for 4.1 2/4] target/mips: Add missing 'break' for a case of MTHC0 handling

[Philippe Mathieu-Daudé]

Hi Aleksandar,

On 7/12/19 1:37 PM, Aleksandar Markovic wrote:
> From: Aleksandar Markovic <amarkovic@wavecomp.com>
> 
> This was found by GCC 8.3 static analysis.
> 

I think you can amend:

Fixes: 5fb2dcd1792
Reported-by: Stefan Weil <sw@weilnetz.de>

> Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> ---
>  target/mips/translate.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 2be5e2d..59d4acd 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -6745,6 +6745,7 @@ static void gen_mthc0(DisasContext *ctx, TCGv arg, int reg, int sel)
>          default:
>              goto cp0_unimplemented;
>          }
> +        break;
>      case CP0_REGISTER_17:
>          switch (sel) {
>          case 0:
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Thanks,

Phil.

Re: [Qemu-devel] ?==?utf-8?q? ?==?utf-8?q? [PATCH for 4.1?==?utf-8?q? 2/4]?==?utf-8?q? target/mips: Add missing 'break' for a case of MTHC0 handling

[Aleksandar Markovic]

> From: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> > From: Aleksandar Markovic <amarkovic@wavecomp.com>
> >
> > This was found by GCC 8.3 static analysis.
> >
> 
> I think you can amend:
> 
> Fixes: 5fb2dcd1792
> Reported-by: Stefan Weil <sw@weilnetz.de>
>

Yes, indeed. This will be added in the next version.

Thanks,
Aleksandar

[Qemu-devel] [PATCH for 4.1 3/4] target/mips: Add missing 'break' for certain cases of MFTR handling

[Aleksandar Markovic]

From: Aleksandar Markovic <amarkovic@wavecomp.com>

This was found by GCC 8.3 static analysis.

Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
---
 target/mips/translate.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 59d4acd..b1cf5f0 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -9826,6 +9826,7 @@ static void gen_mftr(CPUMIPSState *env, DisasContext *ctx, int rt, int rd,
                 gen_mfc0(ctx, t0, rt, sel);
                 break;
             }
+            break;
         case 12:
             switch (sel) {
             case 0:
@@ -9835,6 +9836,7 @@ static void gen_mftr(CPUMIPSState *env, DisasContext *ctx, int rt, int rd,
                 gen_mfc0(ctx, t0, rt, sel);
                 break;
             }
+            break;
         case 13:
             switch (sel) {
             case 0:
-- 
2.7.4

Re: [Qemu-devel] [PATCH for 4.1 3/4] target/mips: Add missing 'break' for certain cases of MFTR handling

[Philippe Mathieu-Daudé]

On 7/12/19 1:37 PM, Aleksandar Markovic wrote:
> From: Aleksandar Markovic <amarkovic@wavecomp.com>
> 
> This was found by GCC 8.3 static analysis.
> 

Again, please amend:

Fixes: ead9360e2fb
Reported-by: Stefan Weil <sw@weilnetz.de>

> Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> ---
>  target/mips/translate.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 59d4acd..b1cf5f0 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -9826,6 +9826,7 @@ static void gen_mftr(CPUMIPSState *env, DisasContext *ctx, int rt, int rd,
>                  gen_mfc0(ctx, t0, rt, sel);
>                  break;
>              }
> +            break;
>          case 12:
>              switch (sel) {
>              case 0:
> @@ -9835,6 +9836,7 @@ static void gen_mftr(CPUMIPSState *env, DisasContext *ctx, int rt, int rd,
>                  gen_mfc0(ctx, t0, rt, sel);
>                  break;
>              }
> +            break;
>          case 13:
>              switch (sel) {
>              case 0:
> 

12 years missing...

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [Qemu-devel] ?==?utf-8?q? ?==?utf-8?q? [PATCH for 4.1?==?utf-8?q? 3/4]?==?utf-8?q? target/mips: Add missing 'break' for certain cases of MFTR handling

[Aleksandar Markovic]

> From: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> > From: Aleksandar Markovic <amarkovic@wavecomp.com>
> > 
> > This was found by GCC 8.3 static analysis.
> >
> 
> Again, please amend:
> 
> Fixes: ead9360e2fb

Who would think that this bug existed since 2007? ...étrange.

Thanks,
Aleksandar

[Qemu-devel] [PATCH for 4.1 4/4] target/mips: Add missing 'break' for certain cases of MTTR handling

[Aleksandar Markovic]

From: Aleksandar Markovic <amarkovic@wavecomp.com>

This was found by GCC 8.3 static analysis.

Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
---
 target/mips/translate.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index b1cf5f0..ca62800 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -10055,6 +10055,7 @@ static void gen_mttr(CPUMIPSState *env, DisasContext *ctx, int rd, int rt,
                 gen_mtc0(ctx, t0, rd, sel);
                 break;
             }
+            break;
         case 12:
             switch (sel) {
             case 0:
@@ -10064,6 +10065,7 @@ static void gen_mttr(CPUMIPSState *env, DisasContext *ctx, int rd, int rt,
                 gen_mtc0(ctx, t0, rd, sel);
                 break;
             }
+            break;
         case 13:
             switch (sel) {
             case 0:
-- 
2.7.4

Re: [Qemu-devel] [PATCH for 4.1 4/4] target/mips: Add missing 'break' for certain cases of MTTR handling

[Philippe Mathieu-Daudé]

On 7/12/19 1:37 PM, Aleksandar Markovic wrote:
> From: Aleksandar Markovic <amarkovic@wavecomp.com>
> 
> This was found by GCC 8.3 static analysis.
> 

Similar to the previous one:

Fixes: ead9360e2fb
Reported-by: Stefan Weil <sw@weilnetz.de>

> Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Thanks for cleaning this!

Regards,

Phil.

> ---
>  target/mips/translate.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index b1cf5f0..ca62800 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -10055,6 +10055,7 @@ static void gen_mttr(CPUMIPSState *env, DisasContext *ctx, int rd, int rt,
>                  gen_mtc0(ctx, t0, rd, sel);
>                  break;
>              }
> +            break;
>          case 12:
>              switch (sel) {
>              case 0:
> @@ -10064,6 +10065,7 @@ static void gen_mttr(CPUMIPSState *env, DisasContext *ctx, int rd, int rt,
>                  gen_mtc0(ctx, t0, rd, sel);
>                  break;
>              }
> +            break;
>          case 13:
>              switch (sel) {
>              case 0:
>