[Qemu-devel] [PATCH 0/3] target-arm: three minor coverity fixes

[Qemu-devel] [PATCH 0/3] target-arm: three minor coverity fixes

[Peter Maydell]

This patchset fixes three minor coverity-reported issues in target-arm:
two instances of a ?: expression whose conditional is always true, and
a missing ULL suffix on a "1 << ..." expression.

Peter Maydell (3):
  target-arm: Add ULL suffix to calculation of page size
  target-arm/translate-a64.c: Remove dead ?: in disas_simd_3same_int()
  target-arm/translate-a64.c: Fix dead ?: in
    handle_simd_shift_fpint_conv()

 target-arm/helper.c        | 2 +-
 target-arm/translate-a64.c | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

-- 
1.8.5.4

[Qemu-devel] [PATCH 1/3] target-arm: Add ULL suffix to calculation of page size

[Peter Maydell]

The maximum block size for AArch64 address translation is 2GB. This means
that we need a ULL suffix on our shift to avoid shifting into the sign
bit of a signed 32 bit integer.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index ec031f5..cbad223 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3926,7 +3926,7 @@ static int get_phys_addr_lpae(CPUARMState *env, target_ulong address,
          * These are basically the same thing, although the number
          * of bits we pull in from the vaddr varies.
          */
-        page_size = (1 << ((granule_sz * (4 - level)) + 3));
+        page_size = (1ULL << ((granule_sz * (4 - level)) + 3));
         descaddr |= (address & (page_size - 1));
         /* Extract attributes from the descriptor and merge with table attrs */
         if (arm_feature(env, ARM_FEATURE_V8)) {
-- 
1.8.5.4

[Qemu-devel] [PATCH 2/3] target-arm/translate-a64.c: Remove dead ?: in disas_simd_3same_int()

[Peter Maydell]

In disas_simd_3same_int(), none of the instructions permit is_q
to be false with size == 3 (this would be a vector operation with
a one-element vector, and the instruction set encodes those as
scalar operations). Replace the always-true ?: check with an
assert.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate-a64.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 9f964df..4c9e237 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -8997,7 +8997,8 @@ static void disas_simd_3same_int(DisasContext *s, uint32_t insn)
     }
 
     if (size == 3) {
-        for (pass = 0; pass < (is_q ? 2 : 1); pass++) {
+        assert(is_q);
+        for (pass = 0; pass < 2; pass++) {
             TCGv_i64 tcg_op1 = tcg_temp_new_i64();
             TCGv_i64 tcg_op2 = tcg_temp_new_i64();
             TCGv_i64 tcg_res = tcg_temp_new_i64();
-- 
1.8.5.4

[Qemu-devel] [PATCH 3/3] target-arm/translate-a64.c: Fix dead ?: in handle_simd_shift_fpint_conv()

[Peter Maydell]

In handle_simd_shift_fpint_conv(), the combination of is_double == true,
is_scalar == false and is_q == false is an unallocated encoding; the
'both parts false' case of the nested ?: expression for calculating
maxpass is therefore unreachable and can be removed.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate-a64.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 4c9e237..5542d7d 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -6484,7 +6484,7 @@ static void handle_simd_shift_fpint_conv(DisasContext *s, bool is_scalar,
     tcg_shift = tcg_const_i32(fracbits);
 
     if (is_double) {
-        int maxpass = is_scalar ? 1 : is_q ? 2 : 1;
+        int maxpass = is_scalar ? 1 : 2;
 
         for (pass = 0; pass < maxpass; pass++) {
             TCGv_i64 tcg_op = tcg_temp_new_i64();
-- 
1.8.5.4

Re: [Qemu-devel] [PATCH 1/3] target-arm: Add ULL suffix to calculation of page size

[Peter Crosthwaite]

On Sun, Jun 8, 2014 at 6:11 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> The maximum block size for AArch64 address translation is 2GB. This means
> that we need a ULL suffix on our shift to avoid shifting into the sign
> bit of a signed 32 bit integer.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

> ---
>  target-arm/helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index ec031f5..cbad223 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3926,7 +3926,7 @@ static int get_phys_addr_lpae(CPUARMState *env, target_ulong address,
>           * These are basically the same thing, although the number
>           * of bits we pull in from the vaddr varies.
>           */
> -        page_size = (1 << ((granule_sz * (4 - level)) + 3));
> +        page_size = (1ULL << ((granule_sz * (4 - level)) + 3));
>          descaddr |= (address & (page_size - 1));
>          /* Extract attributes from the descriptor and merge with table attrs */
>          if (arm_feature(env, ARM_FEATURE_V8)) {
> --
> 1.8.5.4
>
>

Re: [Qemu-devel] [PATCH 2/3] target-arm/translate-a64.c: Remove dead ?: in disas_simd_3same_int()

[Peter Crosthwaite]

On Sun, Jun 8, 2014 at 6:11 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> In disas_simd_3same_int(), none of the instructions permit is_q
> to be false with size == 3 (this would be a vector operation with
> a one-element vector, and the instruction set encodes those as
> scalar operations). Replace the always-true ?: check with an
> assert.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

> ---
>  target-arm/translate-a64.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> index 9f964df..4c9e237 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -8997,7 +8997,8 @@ static void disas_simd_3same_int(DisasContext *s, uint32_t insn)
>      }
>
>      if (size == 3) {
> -        for (pass = 0; pass < (is_q ? 2 : 1); pass++) {
> +        assert(is_q);
> +        for (pass = 0; pass < 2; pass++) {
>              TCGv_i64 tcg_op1 = tcg_temp_new_i64();
>              TCGv_i64 tcg_op2 = tcg_temp_new_i64();
>              TCGv_i64 tcg_res = tcg_temp_new_i64();
> --
> 1.8.5.4
>
>

Re: [Qemu-devel] [PATCH 3/3] target-arm/translate-a64.c: Fix dead ?: in handle_simd_shift_fpint_conv()

[Peter Crosthwaite]

On Sun, Jun 8, 2014 at 6:11 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> In handle_simd_shift_fpint_conv(), the combination of is_double == true,
> is_scalar == false and is_q == false is an unallocated encoding; the
> 'both parts false' case of the nested ?: expression for calculating
> maxpass is therefore unreachable and can be removed.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

> ---
>  target-arm/translate-a64.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> index 4c9e237..5542d7d 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -6484,7 +6484,7 @@ static void handle_simd_shift_fpint_conv(DisasContext *s, bool is_scalar,
>      tcg_shift = tcg_const_i32(fracbits);
>
>      if (is_double) {
> -        int maxpass = is_scalar ? 1 : is_q ? 2 : 1;
> +        int maxpass = is_scalar ? 1 : 2;
>
>          for (pass = 0; pass < maxpass; pass++) {
>              TCGv_i64 tcg_op = tcg_temp_new_i64();
> --
> 1.8.5.4
>
>