Re: [PATCH 3/2] Fix invalid left shift of negative value.

Dominik Vogt
2015-11-30T08:54:08Z

On Tue, Nov 17, 2015 at 04:09:57PM +0100, Dominik Vogt wrote:
> And another one:
>
> On Tue, Nov 10, 2015 at 12:16:38PM +0100, Dominik Vogt wrote:
> > The following series of patches fixes all occurences of
> > left-shifting negative constants in C code which is undefined by
> > the C standard. The patches have been tested on s390x, covering
> > only a small subset of the changes.
>
> Changes in sim/.

Ping.

> sim/arm/ChangeLog
>
> * thumbemu.c (handle_T2_insn): Fix left shift of negative value.
> * armemu.c (handle_v6_insn): Likewise.
>
> sim/avr/ChangeLog
>
> * interp.c (sign_ext): Fix left shift of negative value.
>
> sim/mips/ChangeLog
>
> * micromips.igen (process_isa_mode): Fix left shift of negative value.
>
> sim/msp430/ChangeLog
>
> * msp430-sim.c (get_op, put_op): Fix left shift of negative value.
>
> sim/v850/ChangeLog
>
> * simops.c (v850_bins): Fix left shift of negative value.

> >From 5855e92b06a55ec2cba580788361fbbcc0f1c754 Mon Sep 17 00:00:00 2001
> From: Dominik Vogt <vogt@linux.vnet.ibm.com>
> Date: Fri, 30 Oct 2015 15:19:28 +0100
> Subject: [PATCH 3/2] sim: Fix left shift of negative value.
>
> ---
> sim/arm/armemu.c | 40 ++++++++++++++++++++--------------------
> sim/arm/thumbemu.c | 16 ++++++++--------
> sim/avr/interp.c | 2 +-
> sim/mips/micromips.igen | 2 +-
> sim/msp430/msp430-sim.c | 4 ++--
> sim/v850/simops.c | 2 +-
> 6 files changed, 33 insertions(+), 33 deletions(-)
>
> diff --git a/sim/arm/armemu.c b/sim/arm/armemu.c
> index f2a84eb..3826c78 100644
> --- a/sim/arm/armemu.c
> +++ b/sim/arm/armemu.c
> @@ -351,11 +351,11 @@ handle_v6_insn (ARMul_State * state, ARMword instr)
> {
> n = (val1 >> i) & 0xFFFF;
> if (n & 0x8000)
> - n |= -1 << 16;
> + n |= -(1 << 16);
>
> m = (val2 >> i) & 0xFFFF;
> if (m & 0x8000)
> - m |= -1 << 16;
> + m |= -(1 << 16);
>
> r = n + m;
>
> @@ -371,11 +371,11 @@ handle_v6_insn (ARMul_State * state, ARMword instr)
> case 0xF3: /* QASX<c> <Rd>,<Rn>,<Rm>. */
> n = val1 & 0xFFFF;
> if (n & 0x8000)
> - n |= -1 << 16;
> + n |= -(1 << 16);
>
> m = (val2 >> 16) & 0xFFFF;
> if (m & 0x8000)
> - m |= -1 << 16;
> + m |= -(1 << 16);
>
> r = n - m;
>
> @@ -388,11 +388,11 @@ handle_v6_insn (ARMul_State * state, ARMword instr)
>
> n = (val1 >> 16) & 0xFFFF;
> if (n & 0x8000)
> - n |= -1 << 16;
> + n |= -(1 << 16);
>
> m = val2 & 0xFFFF;
> if (m & 0x8000)
> - m |= -1 << 16;
> + m |= -(1 << 16);
>
> r = n + m;
>
> @@ -407,11 +407,11 @@ handle_v6_insn (ARMul_State * state, ARMword instr)
> case 0xF5: /* QSAX<c> <Rd>,<Rn>,<Rm>. */
> n = val1 & 0xFFFF;
> if (n & 0x8000)
> - n |= -1 << 16;
> + n |= -(1 << 16);
>
> m = (val2 >> 16) & 0xFFFF;
> if (m & 0x8000)
> - m |= -1 << 16;
> + m |= -(1 << 16);
>
> r = n + m;
>
> @@ -424,11 +424,11 @@ handle_v6_insn (ARMul_State * state, ARMword instr)
>
> n = (val1 >> 16) & 0xFFFF;
> if (n & 0x8000)
> - n |= -1 << 16;
> + n |= -(1 << 16);
>
> m = val2 & 0xFFFF;
> if (m & 0x8000)
> - m |= -1 << 16;
> + m |= -(1 << 16);
>
> r = n - m;
>
> @@ -447,11 +447,11 @@ handle_v6_insn (ARMul_State * state, ARMword instr)
> {
> n = (val1 >> i) & 0xFFFF;
> if (n & 0x8000)
> - n |= -1 << 16;
> + n |= -(1 << 16);
>
> m = (val2 >> i) & 0xFFFF;
> if (m & 0x8000)
> - m |= -1 << 16;
> + m |= -(1 << 16);
>
> r = n - m;
>
> @@ -471,11 +471,11 @@ handle_v6_insn (ARMul_State * state, ARMword instr)
> {
> n = (val1 >> i) & 0xFF;
> if (n & 0x80)
> - n |= -1 << 8;
> + n |= - (1 << 8);
>
> m = (val2 >> i) & 0xFF;
> if (m & 0x80)
> - m |= -1 << 8;
> + m |= - (1 << 8);
>
> r = n + m;
>
> @@ -495,11 +495,11 @@ handle_v6_insn (ARMul_State * state, ARMword instr)
> {
> n = (val1 >> i) & 0xFF;
> if (n & 0x80)
> - n |= -1 << 8;
> + n |= - (1 << 8);
>
> m = (val2 >> i) & 0xFF;
> if (m & 0x80)
> - m |= -1 << 8;
> + m |= - (1 << 8);
>
> r = n - m;
>
> @@ -951,14 +951,14 @@ handle_v6_insn (ARMul_State * state, ARMword instr)
> state->Emulate = FALSE;
> }
>
> - mask = -1 << lsb;
> - mask &= ~(-1 << (msb + 1));
> + mask = -(1 << lsb);
> + mask &= ~(-(1 << (msb + 1)));
> state->Reg[Rd] &= ~ mask;
>
> Rn = BITS (0, 3);
> if (Rn != 0xF)
> {
> - ARMword val = state->Reg[Rn] & ~(-1 << ((msb + 1) - lsb));
> + ARMword val = state->Reg[Rn] & ~(-(1 << ((msb + 1) - lsb)));
> state->Reg[Rd] |= val << lsb;
> }
> return 1;
> @@ -1036,7 +1036,7 @@ handle_v6_insn (ARMul_State * state, ARMword instr)
>
> val = state->Reg[Rn];
> val >>= lsb;
> - val &= ~(-1 << (widthm1 + 1));
> + val &= ~(-(1 << (widthm1 + 1)));
>
> state->Reg[Rd] = val;
>
> diff --git a/sim/arm/thumbemu.c b/sim/arm/thumbemu.c
> index 2d26bf6..72929c7 100644
> --- a/sim/arm/thumbemu.c
> +++ b/sim/arm/thumbemu.c
> @@ -204,7 +204,7 @@ handle_T2_insn (ARMul_State * state,
>
> simm32 = (J1 << 19) | (J2 << 18) | (imm6 << 12) | (imm11 << 1);
> if (S)
> - simm32 |= (-1 << 20);
> + simm32 |= -(1 << 20);
> break;
> }
>
> @@ -217,7 +217,7 @@ handle_T2_insn (ARMul_State * state,
>
> simm32 = (I1 << 23) | (I2 << 22) | (imm10 << 12) | (imm11 << 1);
> if (S)
> - simm32 |= (-1 << 24);
> + simm32 |= -(1 << 24);
> break;
> }
>
> @@ -230,7 +230,7 @@ handle_T2_insn (ARMul_State * state,
>
> simm32 = (I1 << 23) | (I2 << 22) | (imm10h << 12) | (imm10l << 2);
> if (S)
> - simm32 |= (-1 << 24);
> + simm32 |= -(1 << 24);
>
> CLEART;
> state->Reg[14] = (pc + 4) | 1;
> @@ -246,7 +246,7 @@ handle_T2_insn (ARMul_State * state,
>
> simm32 = (I1 << 23) | (I2 << 22) | (imm10 << 12) | (imm11 << 1);
> if (S)
> - simm32 |= (-1 << 24);
> + simm32 |= -(1 << 24);
> state->Reg[14] = (pc + 4) | 1;
> break;
> }
> @@ -1078,7 +1078,7 @@ handle_T2_insn (ARMul_State * state,
> ARMword Rn = tBITS (0, 3);
> ARMword msbit = ntBITS (0, 5);
> ARMword lsbit = (ntBITS (12, 14) << 2) | ntBITS (6, 7);
> - ARMword mask = -1 << lsbit;
> + ARMword mask = -(1 << lsbit);
>
> tASSERT (tBIT (4) == 0);
> tASSERT (ntBIT (15) == 0);
> @@ -1489,7 +1489,7 @@ handle_T2_insn (ARMul_State * state,
>
> state->Reg[Rt] = ARMul_LoadByte (state, address);
> if (state->Reg[Rt] & 0x80)
> - state->Reg[Rt] |= -1 << 8;
> + state->Reg[Rt] |= -(1 << 8);
>
> * pvalid = t_resolved;
> break;
> @@ -1542,7 +1542,7 @@ handle_T2_insn (ARMul_State * state,
>
> state->Reg[Rt] = ARMul_LoadHalfWord (state, address);
> if (state->Reg[Rt] & 0x8000)
> - state->Reg[Rt] |= -1 << 16;
> + state->Reg[Rt] |= -(1 << 16);
>
> * pvalid = t_branch;
> break;
> @@ -1564,7 +1564,7 @@ handle_T2_insn (ARMul_State * state,
> val = state->Reg[Rm];
> val = (val >> ror) | (val << (32 - ror));
> if (val & 0x8000)
> - val |= -1 << 16;
> + val |= -(1 << 16);
> state->Reg[Rd] = val;
> }
> else
> diff --git a/sim/avr/interp.c b/sim/avr/interp.c
> index a6588d3..bdb4e42 100644
> --- a/sim/avr/interp.c
> +++ b/sim/avr/interp.c
> @@ -231,7 +231,7 @@ static byte sram[MAX_AVR_SRAM];
> static int sign_ext (word val, int nb_bits)
> {
> if (val & (1 << (nb_bits - 1)))
> - return val | (-1 << nb_bits);
> + return val | -(1 << nb_bits);
> return val;
> }
>
> diff --git a/sim/mips/micromips.igen b/sim/mips/micromips.igen
> index 2c62376..f24220e 100644
> --- a/sim/mips/micromips.igen
> +++ b/sim/mips/micromips.igen
> @@ -54,7 +54,7 @@
> :function:::address_word:process_isa_mode:address_word target
> {
> SD->isa_mode = target & 0x1;
> - return (target & (-1 << 1));
> + return (target & (-(1 << 1)));
> }
>
> :function:::address_word:do_micromips_jalr:int rt, int rs, address_word nia, int delayslot_instruction_size
> diff --git a/sim/msp430/msp430-sim.c b/sim/msp430/msp430-sim.c
> index f32cb69..6a7effa 100644
> --- a/sim/msp430/msp430-sim.c
> +++ b/sim/msp430/msp430-sim.c
> @@ -366,7 +366,7 @@ get_op (SIM_DESC sd, MSP430_Opcode_Decoded *opc, int n)
>
> /* Index values are signed. */
> if (addr & (1 << (sign - 1)))
> - addr |= -1 << sign;
> + addr |= -(1 << sign);
>
> addr += reg;
>
> @@ -559,7 +559,7 @@ put_op (SIM_DESC sd, MSP430_Opcode_Decoded *opc, int n, int val)
>
> /* Index values are signed. */
> if (addr & (1 << (sign - 1)))
> - addr |= -1 << sign;
> + addr |= -(1 << sign);
>
> addr += reg;
>
> diff --git a/sim/v850/simops.c b/sim/v850/simops.c
> index b8b3856..40d578e 100644
> --- a/sim/v850/simops.c
> +++ b/sim/v850/simops.c
> @@ -3317,7 +3317,7 @@ v850_bins (SIM_DESC sd, unsigned int source, unsigned int lsb, unsigned int msb,
> pos = lsb;
> width = (msb - lsb) + 1;
>
> - mask = ~ (-1 << width);
> + mask = ~ (-(1 << width));
> source &= mask;
> mask <<= pos;
> result = (* dest) & ~ mask;
> --
> 2.3.0

Ciao

Dominik ^_^ ^_^

--

Dominik Vogt
IBM Germany