From: Joel Brobecker <brobecker@adacore.com>
To: Dominik Vogt <vogt@linux.vnet.ibm.com>
Cc: gdb-patches@sourceware.org, Mike Frysinger <vapier@gentoo.org>
Subject: [SIM patch] Re: [PATCH 3/2] Fix invalid left shift of negative value.
Date: Sun, 06 Dec 2015 14:17:00 -0000 [thread overview]
Message-ID: <20151206141732.GH10969@adacore.com> (raw)
In-Reply-To: <20151130085341.GA4137@linux.vnet.ibm.com>
> 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
next prev parent reply other threads:[~2015-12-06 14:17 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-11-10 11:16 [PATCH 0/2] " Dominik Vogt
2015-11-10 11:17 ` [PATCH 1/2] " Dominik Vogt
2015-11-10 11:18 ` [PATCH 2/2] " Dominik Vogt
2015-11-10 22:42 ` [PATCH 0/2] " Kevin Buettner
2015-11-11 17:23 ` Ulrich Weigand
2015-11-11 19:27 ` Kevin Buettner
2015-11-17 5:09 ` Kevin Buettner
2015-11-17 14:13 ` Pedro Alves
2015-11-17 17:33 ` Paul_Koning
2015-11-17 15:10 ` [PATCH 3/2] " Dominik Vogt
2015-11-17 17:49 ` Kevin Buettner
2015-11-30 8:54 ` Dominik Vogt
2015-12-06 14:17 ` Joel Brobecker [this message]
2015-12-15 13:15 ` [SIM patch] " Andreas Arnez
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20151206141732.GH10969@adacore.com \
--to=brobecker@adacore.com \
--cc=gdb-patches@sourceware.org \
--cc=vapier@gentoo.org \
--cc=vogt@linux.vnet.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox