On 06/11/15 15:50, Paolo Bonzini wrote: > This is reported by Coverity. The algorithm description at > ftp://ftp.icm.edu.pl/packages/ggi/doc/hw/sparc/Sparc.pdf suggests > that the 32-bit parts of rs2, after the left shift, is treated > as a 64-bit integer. Bits 32 and above are used to do the > saturating truncation. > > Use a cast to unsigned in order to placate ubsan for negative > inputs, and do so in the other fpack* instructions too. > > Signed-off-by: Paolo Bonzini <[email protected]> > --- > target-sparc/vis_helper.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/target-sparc/vis_helper.c b/target-sparc/vis_helper.c > index 383cc8b..a1c3343 100644 > --- a/target-sparc/vis_helper.c > +++ b/target-sparc/vis_helper.c > @@ -404,7 +404,8 @@ uint32_t helper_fpack16(uint64_t gsr, uint64_t rs2) > for (byte = 0; byte < 4; byte++) { > uint32_t val; > int16_t src = rs2 >> (byte * 16); > - int32_t scaled = src << scale; > + /* The cast to unsigned avoids undefined behavior for negative src. > */ > + int32_t scaled = (uint32_t)src << scale; > int32_t from_fixed = scaled >> 7; > > val = (from_fixed < 0 ? 0 : > @@ -426,7 +427,8 @@ uint64_t helper_fpack32(uint64_t gsr, uint64_t rs1, > uint64_t rs2) > for (word = 0; word < 2; word++) { > uint64_t val; > int32_t src = rs2 >> (word * 32); > - int64_t scaled = (int64_t)src << scale; > + /* The cast to unsigned avoids undefined behavior for negative src. > */ > + int64_t scaled = (uint64_t)src << scale; > int64_t from_fixed = scaled >> 23; > > val = (from_fixed < 0 ? 0 : > @@ -447,7 +449,8 @@ uint32_t helper_fpackfix(uint64_t gsr, uint64_t rs2) > for (word = 0; word < 2; word++) { > uint32_t val; > int32_t src = rs2 >> (word * 32); > - int64_t scaled = src << scale; > + /* The cast to unsigned avoids undefined behavior for negative src. > */ > + int64_t scaled = (uint64_t)src << scale; > int64_t from_fixed = scaled >> 16; > > val = (from_fixed < -32768 ? -32768 :
Acked-by: Mark Cave-Ayland <[email protected]> ATB, Mark.
