Skip to content

Commit

Permalink
cmd/compile: don't allow NaNs in floating-point constant ops
Browse files Browse the repository at this point in the history
We store 32-bit floating point constants in a 64-bit field, by
converting that 32-bit float to 64-bit float to store it, and convert
it back to use it.

That works for *almost* all floating-point constants. The exception is
signaling NaNs. The round trip described above means we can't represent
a 32-bit signaling NaN, because conversions strip the signaling bit.

To fix this issue, just forbid NaNs as floating-point constants in SSA
form. This shouldn't affect any real-world code, as people seldom
constant-propagate NaNs (except in test code).

Additionally, NaNs are somewhat underspecified (which of the many NaNs
do you get when dividing 0/0?), so when cross-compiling there's a
danger of using the compiler machine's NaN regime for some math, and
the target machine's NaN regime for other math. Better to use the
target machine's NaN regime always.

This has been a bug since 1.10, and there's an easy workaround
(declare a global varaible containing the signaling NaN pattern, and
use that as the argument to math.Float32frombits) so we'll fix it in
1.15.

Fixes #36400
Update #36399

Change-Id: Icf155e743281560eda2eed953d19a829552ccfda
Reviewed-on: https://go-review.googlesource.com/c/go/+/213477
Run-TryBot: Keith Randall <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Josh Bleecher Snyder <[email protected]>
  • Loading branch information
randall77 committed Feb 25, 2020
1 parent 0fb1a49 commit 2aa7c6c
Show file tree
Hide file tree
Showing 11 changed files with 200 additions and 26 deletions.
60 changes: 60 additions & 0 deletions src/cmd/compile/internal/gc/float_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,66 @@ func TestFloat32StoreToLoadConstantFold(t *testing.T) {
}
}

// Signaling NaN values as constants.
const (
snan32bits uint32 = 0x7f800001
snan64bits uint64 = 0x7ff0000000000001
)

// Signaling NaNs as variables.
var snan32bitsVar uint32 = snan32bits
var snan64bitsVar uint64 = snan64bits

func TestFloatSignalingNaN(t *testing.T) {
// Make sure we generate a signaling NaN from a constant properly.
// See issue 36400.
f32 := math.Float32frombits(snan32bits)
g32 := math.Float32frombits(snan32bitsVar)
x32 := math.Float32bits(f32)
y32 := math.Float32bits(g32)
if x32 != y32 {
t.Errorf("got %x, want %x (diff=%x)", x32, y32, x32^y32)
}

f64 := math.Float64frombits(snan64bits)
g64 := math.Float64frombits(snan64bitsVar)
x64 := math.Float64bits(f64)
y64 := math.Float64bits(g64)
if x64 != y64 {
t.Errorf("got %x, want %x (diff=%x)", x64, y64, x64^y64)
}
}

func TestFloatSignalingNaNConversion(t *testing.T) {
// Test to make sure when we convert a signaling NaN, it converts to a quiet NaN.
// See issue 36399.
s32 := math.Float32frombits(snan32bitsVar)
q64 := float64(s32)
if math.Float64bits(q64)>>52&1 == 0 {
t.Errorf("got signaling NaN, want quiet NaN")
}
s64 := math.Float64frombits(snan64bitsVar)
q32 := float32(s64)
if math.Float32bits(q32)>>22&1 == 0 {
t.Errorf("got signaling NaN, want quiet NaN")
}
}

func TestFloatSignalingNaNConversionConst(t *testing.T) {
// Test to make sure when we convert a signaling NaN, it converts to a quiet NaN.
// See issue 36399 and 36400.
s32 := math.Float32frombits(snan32bits)
q64 := float64(s32)
if math.Float64bits(q64)>>52&1 == 0 {
t.Errorf("got signaling NaN, want quiet NaN")
}
s64 := math.Float64frombits(snan64bits)
q32 := float32(s64)
if math.Float32bits(q32)>>22&1 == 0 {
t.Errorf("got signaling NaN, want quiet NaN")
}
}

var sinkFloat float64

func BenchmarkMul2(b *testing.B) {
Expand Down
10 changes: 9 additions & 1 deletion src/cmd/compile/internal/ssa/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,15 +141,23 @@ func checkFunc(f *Func) {
f.Fatalf("bad int32 AuxInt value for %v", v)
}
canHaveAuxInt = true
case auxInt64, auxFloat64:
case auxInt64:
canHaveAuxInt = true
case auxInt128:
// AuxInt must be zero, so leave canHaveAuxInt set to false.
case auxFloat32:
canHaveAuxInt = true
if math.IsNaN(v.AuxFloat()) {
f.Fatalf("value %v has an AuxInt that encodes a NaN", v)
}
if !isExactFloat32(v.AuxFloat()) {
f.Fatalf("value %v has an AuxInt value that is not an exact float32", v)
}
case auxFloat64:
canHaveAuxInt = true
if math.IsNaN(v.AuxFloat()) {
f.Fatalf("value %v has an AuxInt that encodes a NaN", v)
}
case auxString, auxSym, auxTyp, auxArchSpecific:
canHaveAux = true
case auxSymOff, auxSymValAndOff, auxTypSize:
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/ssa/gen/PPC64.rules
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@

// Constant folding
(FABS (FMOVDconst [x])) -> (FMOVDconst [auxFrom64F(math.Abs(auxTo64F(x)))])
(FSQRT (FMOVDconst [x])) -> (FMOVDconst [auxFrom64F(math.Sqrt(auxTo64F(x)))])
(FSQRT (FMOVDconst [x])) && auxTo64F(x) >= 0 -> (FMOVDconst [auxFrom64F(math.Sqrt(auxTo64F(x)))])
(FFLOOR (FMOVDconst [x])) -> (FMOVDconst [auxFrom64F(math.Floor(auxTo64F(x)))])
(FCEIL (FMOVDconst [x])) -> (FMOVDconst [auxFrom64F(math.Ceil(auxTo64F(x)))])
(FTRUNC (FMOVDconst [x])) -> (FMOVDconst [auxFrom64F(math.Trunc(auxTo64F(x)))])
Expand Down
21 changes: 11 additions & 10 deletions src/cmd/compile/internal/ssa/gen/Wasm.rules
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@
(I64Or (I64Const [x]) (I64Const [y])) -> (I64Const [x | y])
(I64Xor (I64Const [x]) (I64Const [y])) -> (I64Const [x ^ y])
(F64Add (F64Const [x]) (F64Const [y])) -> (F64Const [auxFrom64F(auxTo64F(x) + auxTo64F(y))])
(F64Mul (F64Const [x]) (F64Const [y])) -> (F64Const [auxFrom64F(auxTo64F(x) * auxTo64F(y))])
(F64Mul (F64Const [x]) (F64Const [y])) && !math.IsNaN(auxTo64F(x) * auxTo64F(y)) -> (F64Const [auxFrom64F(auxTo64F(x) * auxTo64F(y))])
(I64Eq (I64Const [x]) (I64Const [y])) && x == y -> (I64Const [1])
(I64Eq (I64Const [x]) (I64Const [y])) && x != y -> (I64Const [0])
(I64Ne (I64Const [x]) (I64Const [y])) && x == y -> (I64Const [0])
Expand All @@ -382,15 +382,16 @@
(I64ShrU (I64Const [x]) (I64Const [y])) -> (I64Const [int64(uint64(x) >> uint64(y))])
(I64ShrS (I64Const [x]) (I64Const [y])) -> (I64Const [x >> uint64(y)])

(I64Add (I64Const [x]) y) -> (I64Add y (I64Const [x]))
(I64Mul (I64Const [x]) y) -> (I64Mul y (I64Const [x]))
(I64And (I64Const [x]) y) -> (I64And y (I64Const [x]))
(I64Or (I64Const [x]) y) -> (I64Or y (I64Const [x]))
(I64Xor (I64Const [x]) y) -> (I64Xor y (I64Const [x]))
(F64Add (F64Const [x]) y) -> (F64Add y (F64Const [x]))
(F64Mul (F64Const [x]) y) -> (F64Mul y (F64Const [x]))
(I64Eq (I64Const [x]) y) -> (I64Eq y (I64Const [x]))
(I64Ne (I64Const [x]) y) -> (I64Ne y (I64Const [x]))
// TODO: declare these operations as commutative and get rid of these rules?
(I64Add (I64Const [x]) y) && y.Op != OpWasmI64Const -> (I64Add y (I64Const [x]))
(I64Mul (I64Const [x]) y) && y.Op != OpWasmI64Const -> (I64Mul y (I64Const [x]))
(I64And (I64Const [x]) y) && y.Op != OpWasmI64Const -> (I64And y (I64Const [x]))
(I64Or (I64Const [x]) y) && y.Op != OpWasmI64Const -> (I64Or y (I64Const [x]))
(I64Xor (I64Const [x]) y) && y.Op != OpWasmI64Const -> (I64Xor y (I64Const [x]))
(F64Add (F64Const [x]) y) && y.Op != OpWasmF64Const -> (F64Add y (F64Const [x]))
(F64Mul (F64Const [x]) y) && y.Op != OpWasmF64Const -> (F64Mul y (F64Const [x]))
(I64Eq (I64Const [x]) y) && y.Op != OpWasmI64Const -> (I64Eq y (I64Const [x]))
(I64Ne (I64Const [x]) y) && y.Op != OpWasmI64Const -> (I64Ne y (I64Const [x]))

(I64Eq x (I64Const [0])) -> (I64Eqz x)
(I64Ne x (I64Const [0])) -> (I64Eqz (I64Eqz x))
Expand Down
14 changes: 7 additions & 7 deletions src/cmd/compile/internal/ssa/gen/generic.rules
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,8 @@
(Mul16 (Const16 [c]) (Const16 [d])) -> (Const16 [int64(int16(c*d))])
(Mul32 (Const32 [c]) (Const32 [d])) -> (Const32 [int64(int32(c*d))])
(Mul64 (Const64 [c]) (Const64 [d])) -> (Const64 [c*d])
(Mul32F (Const32F [c]) (Const32F [d])) -> (Const32F [auxFrom32F(auxTo32F(c) * auxTo32F(d))])
(Mul64F (Const64F [c]) (Const64F [d])) -> (Const64F [auxFrom64F(auxTo64F(c) * auxTo64F(d))])
(Mul32F (Const32F [c]) (Const32F [d])) && !math.IsNaN(float64(auxTo32F(c) * auxTo32F(d))) -> (Const32F [auxFrom32F(auxTo32F(c) * auxTo32F(d))])
(Mul64F (Const64F [c]) (Const64F [d])) && !math.IsNaN(auxTo64F(c) * auxTo64F(d)) -> (Const64F [auxFrom64F(auxTo64F(c) * auxTo64F(d))])

(And8 (Const8 [c]) (Const8 [d])) -> (Const8 [int64(int8(c&d))])
(And16 (Const16 [c]) (Const16 [d])) -> (Const16 [int64(int16(c&d))])
Expand All @@ -144,8 +144,8 @@
(Div16u (Const16 [c]) (Const16 [d])) && d != 0 -> (Const16 [int64(int16(uint16(c)/uint16(d)))])
(Div32u (Const32 [c]) (Const32 [d])) && d != 0 -> (Const32 [int64(int32(uint32(c)/uint32(d)))])
(Div64u (Const64 [c]) (Const64 [d])) && d != 0 -> (Const64 [int64(uint64(c)/uint64(d))])
(Div32F (Const32F [c]) (Const32F [d])) -> (Const32F [auxFrom32F(auxTo32F(c) / auxTo32F(d))])
(Div64F (Const64F [c]) (Const64F [d])) -> (Const64F [auxFrom64F(auxTo64F(c) / auxTo64F(d))])
(Div32F (Const32F [c]) (Const32F [d])) && !math.IsNaN(float64(auxTo32F(c) / auxTo32F(d))) -> (Const32F [auxFrom32F(auxTo32F(c) / auxTo32F(d))])
(Div64F (Const64F [c]) (Const64F [d])) && !math.IsNaN(auxTo64F(c) / auxTo64F(d)) -> (Const64F [auxFrom64F(auxTo64F(c) / auxTo64F(d))])
(Select0 (Div128u (Const64 [0]) lo y)) -> (Div64u lo y)
(Select1 (Div128u (Const64 [0]) lo y)) -> (Mod64u lo y)

Expand Down Expand Up @@ -588,8 +588,8 @@
-> x

// Pass constants through math.Float{32,64}bits and math.Float{32,64}frombits
(Load <t1> p1 (Store {t2} p2 (Const64 [x]) _)) && isSamePtr(p1,p2) && sizeof(t2) == 8 && is64BitFloat(t1) -> (Const64F [x])
(Load <t1> p1 (Store {t2} p2 (Const32 [x]) _)) && isSamePtr(p1,p2) && sizeof(t2) == 4 && is32BitFloat(t1) -> (Const32F [auxFrom32F(math.Float32frombits(uint32(x)))])
(Load <t1> p1 (Store {t2} p2 (Const64 [x]) _)) && isSamePtr(p1,p2) && sizeof(t2) == 8 && is64BitFloat(t1) && !math.IsNaN(math.Float64frombits(uint64(x))) -> (Const64F [x])
(Load <t1> p1 (Store {t2} p2 (Const32 [x]) _)) && isSamePtr(p1,p2) && sizeof(t2) == 4 && is32BitFloat(t1) && !math.IsNaN(float64(math.Float32frombits(uint32(x)))) -> (Const32F [auxFrom32F(math.Float32frombits(uint32(x)))])
(Load <t1> p1 (Store {t2} p2 (Const64F [x]) _)) && isSamePtr(p1,p2) && sizeof(t2) == 8 && is64BitInt(t1) -> (Const64 [x])
(Load <t1> p1 (Store {t2} p2 (Const32F [x]) _)) && isSamePtr(p1,p2) && sizeof(t2) == 4 && is32BitInt(t1) -> (Const32 [int64(int32(math.Float32bits(auxTo32F(x))))])

Expand Down Expand Up @@ -1858,7 +1858,7 @@
(Div32F x (Const32F <t> [c])) && reciprocalExact32(auxTo32F(c)) -> (Mul32F x (Const32F <t> [auxFrom32F(1/auxTo32F(c))]))
(Div64F x (Const64F <t> [c])) && reciprocalExact64(auxTo64F(c)) -> (Mul64F x (Const64F <t> [auxFrom64F(1/auxTo64F(c))]))

(Sqrt (Const64F [c])) -> (Const64F [auxFrom64F(math.Sqrt(auxTo64F(c)))])
(Sqrt (Const64F [c])) && !math.IsNaN(math.Sqrt(auxTo64F(c))) -> (Const64F [auxFrom64F(math.Sqrt(auxTo64F(c)))])

// recognize runtime.newobject and don't Zero/Nilcheck it
(Zero (Load (OffPtr [c] (SP)) mem) mem)
Expand Down
7 changes: 6 additions & 1 deletion src/cmd/compile/internal/ssa/gen/genericOps.go
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,12 @@ var genericOps = []opData{
{name: "Const32", aux: "Int32"}, // auxint is sign-extended 32 bits
// Note: ConstX are sign-extended even when the type of the value is unsigned.
// For instance, uint8(0xaa) is stored as auxint=0xffffffffffffffaa.
{name: "Const64", aux: "Int64"}, // value is auxint
{name: "Const64", aux: "Int64"}, // value is auxint
// Note: for both Const32F and Const64F, we disallow encoding NaNs.
// Signaling NaNs are tricky because if you do anything with them, they become quiet.
// Particularly, converting a 32 bit sNaN to 64 bit and back converts it to a qNaN.
// See issue 36399 and 36400.
// Encodings of +inf, -inf, and -0 are fine.
{name: "Const32F", aux: "Float32"}, // value is math.Float64frombits(uint64(auxint)) and is exactly representable as float 32
{name: "Const64F", aux: "Float64"}, // value is math.Float64frombits(uint64(auxint))
{name: "ConstInterface"}, // nil interface
Expand Down
6 changes: 6 additions & 0 deletions src/cmd/compile/internal/ssa/rewrite.go
Original file line number Diff line number Diff line change
Expand Up @@ -487,11 +487,17 @@ func DivisionNeedsFixUp(v *Value) bool {

// auxFrom64F encodes a float64 value so it can be stored in an AuxInt.
func auxFrom64F(f float64) int64 {
if f != f {
panic("can't encode a NaN in AuxInt field")
}
return int64(math.Float64bits(f))
}

// auxFrom32F encodes a float32 value so it can be stored in an AuxInt.
func auxFrom32F(f float32) int64 {
if f != f {
panic("can't encode a NaN in AuxInt field")
}
return int64(math.Float64bits(extend32Fto64F(f)))
}

Expand Down
4 changes: 4 additions & 0 deletions src/cmd/compile/internal/ssa/rewritePPC64.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 2aa7c6c

Please sign in to comment.