[SIM patch] Re: [PATCH 3/2] Fix invalid left shift of negative value.
Joel Brobecker
brobecker@adacore.com
Sun Dec 6 14:17:00 GMT 2015
> 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.
Sorry about the delay in answering this. I see Mike is in Cc, and
he usually reviews sim patches very quickly. It looks good to me,
but normally, Mike does the approving. Nevertheless, in the interest
of moving forward, if Mike isn't available or hasn't answered by
next Sunday, please feel free to push your patch.
>
> > 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
--
Joel
More information about the Gdb-patches
mailing list