From 6bc593805f2c2b0ef8b194cc186b804237744cb4 Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Wed, 8 Mar 2017 11:36:30 +0200 Subject: [PATCH] cmd/vet: eliminate "might be too small for shift" warnings Determine int, uint and uintptr bit sizes from GOARCH environment variable if it is set. Otherwise use host-specific sizes. Fixes #19321 Change-Id: I494b8e4b49b59d32794f50ff2ce06ba040cb8460 Reviewed-on: https://go-review.googlesource.com/37950 Run-TryBot: Josh Bleecher Snyder Reviewed-by: Josh Bleecher Snyder --- src/cmd/vet/all/whitelist/32bit.txt | 16 ++++ src/cmd/vet/all/whitelist/64bit.txt | 26 ------- src/cmd/vet/all/whitelist/all.txt | 18 ----- src/cmd/vet/shift.go | 16 ++-- src/cmd/vet/testdata/shift.go | 111 +++++++++++++++++----------- src/cmd/vet/types.go | 5 ++ src/go/types/sizes.go | 6 +- 7 files changed, 102 insertions(+), 96 deletions(-) create mode 100644 src/cmd/vet/all/whitelist/32bit.txt delete mode 100644 src/cmd/vet/all/whitelist/64bit.txt diff --git a/src/cmd/vet/all/whitelist/32bit.txt b/src/cmd/vet/all/whitelist/32bit.txt new file mode 100644 index 0000000000..8728ee1c57 --- /dev/null +++ b/src/cmd/vet/all/whitelist/32bit.txt @@ -0,0 +1,16 @@ +// 32bit-specific vet whitelist. See readme.txt for details. + +// TODO: fix these warnings after the CL 37950 . +math/big/float.go: x[i] (32 bits) too small for shift of 32 +math/big/nat.go: Word(rand.Uint32()) (32 bits) too small for shift of 32 +runtime/malloc.go: uintptr(i) (32 bits) too small for shift of 40 +runtime/malloc.go: uintptr(i) (32 bits) too small for shift of 40 +runtime/malloc.go: uintptr(i) (32 bits) too small for shift of 40 +sync/atomic/atomic_test.go: uintptr(seed + i) (32 bits) too small for shift of 32 +sync/atomic/atomic_test.go: uintptr(seed+i) << 32 (32 bits) too small for shift of 32 +sync/atomic/atomic_test.go: uintptr(seed + i) (32 bits) too small for shift of 32 +sync/atomic/atomic_test.go: old (32 bits) too small for shift of 32 +sync/atomic/atomic_test.go: old << 32 (32 bits) too small for shift of 32 +sync/atomic/atomic_test.go: old (32 bits) too small for shift of 32 +sync/atomic/atomic_test.go: v (32 bits) too small for shift of 32 +sync/atomic/atomic_test.go: v (32 bits) too small for shift of 32 diff --git a/src/cmd/vet/all/whitelist/64bit.txt b/src/cmd/vet/all/whitelist/64bit.txt deleted file mode 100644 index 39855f7d70..0000000000 --- a/src/cmd/vet/all/whitelist/64bit.txt +++ /dev/null @@ -1,26 +0,0 @@ -// 64-bit-specific vet whitelist. See readme.txt for details. - -// False positives. - -// Clever const tricks outwit the "large shift" check. -runtime/hashmap.go: hash might be too small for shift of 56 -runtime/hashmap.go: hash might be too small for shift of 56 -runtime/hashmap.go: hash might be too small for shift of 56 -runtime/hashmap.go: hash might be too small for shift of 56 -runtime/hashmap.go: hash might be too small for shift of 56 -runtime/hashmap.go: hash might be too small for shift of 56 -runtime/hashmap_fast.go: hash might be too small for shift of 56 -runtime/hashmap_fast.go: hash might be too small for shift of 56 -math/big/arith.go: x might be too small for shift of 32 -math/big/arith.go: y might be too small for shift of 32 -math/big/arith.go: w0 might be too small for shift of 32 -math/big/arith.go: t might be too small for shift of 32 -math/big/arith.go: w1 might be too small for shift of 32 -math/big/arith.go: v might be too small for shift of 32 -math/big/arith.go: un10 might be too small for shift of 32 -math/big/arith.go: (xi&yi | (xi|yi)&^zi) might be too small for shift of 63 -math/big/arith.go: (yi&^xi | (yi|^xi)&zi) might be too small for shift of 63 -math/big/arith.go: xi &^ zi might be too small for shift of 63 -math/big/arith.go: (zi &^ xi) might be too small for shift of 63 -math/big/nat.go: yi might be too small for shift of 60 -math/big/nat.go: yi might be too small for shift of 60 diff --git a/src/cmd/vet/all/whitelist/all.txt b/src/cmd/vet/all/whitelist/all.txt index 6323c4c2b6..8f6251d716 100644 --- a/src/cmd/vet/all/whitelist/all.txt +++ b/src/cmd/vet/all/whitelist/all.txt @@ -51,24 +51,6 @@ encoding/xml/marshal_test.go: method MarshalXML(e *Encoder, start StartElement) encoding/xml/read.go: method UnmarshalXML(d *Decoder, start StartElement) error should have signature UnmarshalXML(*xml.Decoder, xml.StartElement) error encoding/xml/read_test.go: method UnmarshalXML(d *Decoder, start StartElement) error should have signature UnmarshalXML(*xml.Decoder, xml.StartElement) error -// Lots of false positives from the "large shift" check. -// Mostly code that uses clever const tricks to determine -// or use the size of an int or pointer (and related values). -image/png/paeth.go: x might be too small for shift of 63 -math/big/float.go: x[i] might be too small for shift of 32 -math/big/nat.go: Word(rand.Uint32()) might be too small for shift of 32 -runtime/malloc.go: uintptr(i) might be too small for shift of 40 -runtime/malloc.go: uintptr(i) might be too small for shift of 40 -runtime/malloc.go: uintptr(i) might be too small for shift of 40 -sync/atomic/atomic_test.go: uintptr(seed + i) might be too small for shift of 32 -sync/atomic/atomic_test.go: uintptr(seed+i) << 32 might be too small for shift of 32 -sync/atomic/atomic_test.go: uintptr(seed + i) might be too small for shift of 32 -sync/atomic/atomic_test.go: old might be too small for shift of 32 -sync/atomic/atomic_test.go: old << 32 might be too small for shift of 32 -sync/atomic/atomic_test.go: old might be too small for shift of 32 -sync/atomic/atomic_test.go: v might be too small for shift of 32 -sync/atomic/atomic_test.go: v might be too small for shift of 32 - // Long struct tags used to test reflect internals cmd/link/link_test.go: struct field tag "\n\tLondon. Michaelmas term lately over, and the Lord Chancellor sitting in Lincoln’s Inn Hall. Implacable November weather. As much mud in the streets as if the waters had but newly retired from the face of the earth, and it would not be wonderful to meet a Megalosaurus, forty feet long or so, waddling like an elephantine lizard up Holborn Hill. Smoke lowering down from chimney-pots, making a soft black drizzle, with flakes of soot in it as big as full-grown snowflakes—gone into mourning, one might imagine, for the death of the sun. Dogs, undistinguishable in mire. Horses, scarcely better; splashed to their very blinkers. Foot passengers, jostling one another’s umbrellas in a general infection of ill temper, and losing their foot-hold at street-corners, where tens of thousands of other foot passengers have been slipping and sliding since the day broke (if this day ever broke), adding new deposits to the crust upon crust of mud, sticking at those points tenaciously to the pavement, and accumulating at compound interest.\n\n\tFog everywhere. Fog up the river, where it flows among green aits and meadows; fog down the river, where it rolls defiled among the tiers of shipping and the waterside pollutions of a great (and dirty) city. Fog on the Essex marshes, fog on the Kentish heights. Fog creeping into the cabooses of collier-brigs; fog lying out on the yards and hovering in the rigging of great ships; fog drooping on the gunwales of barges and small boats. Fog in the eyes and throats of ancient Greenwich pensioners, wheezing by the firesides of their wards; fog in the stem and bowl of the afternoon pipe of the wrathful skipper, down in his close cabin; fog cruelly pinching the toes and fingers of his shivering little ‘prentice boy on deck. Chance people on the bridges peeping over the parapets into a nether sky of fog, with fog all round them, as if they were up in a balloon and hanging in the misty clouds.\n\n\tGas looming through the fog in divers places in the streets, much as the sun may, from the spongey fields, be seen to loom by husbandman and ploughboy. Most of the shops lighted two hours before their time—as the gas seems to know, for it has a haggard and unwilling look.\n\n\tThe raw afternoon is rawest, and the dense fog is densest, and the muddy streets are muddiest near that leaden-headed old obstruction, appropriate ornament for the threshold of a leaden-headed old corporation, Temple Bar. And hard by Temple Bar, in Lincoln’s Inn Hall, at the very heart of the fog, sits the Lord High Chancellor in his High Court of Chancery." not compatible with reflect.StructTag.Get: bad syntax for struct tag key cmd/link/link_test.go: struct field tag "\n\tIt was grand to see how the wind awoke, and bent the trees, and drove the rain before it like a cloud of smoke; and to hear the solemn thunder, and to see the lightning; and while thinking with awe of the tremendous powers by which our little lives are encompassed, to consider how beneficent they are, and how upon the smallest flower and leaf there was already a freshness poured from all this seeming rage, which seemed to make creation new again." not compatible with reflect.StructTag.Get: bad syntax for struct tag key diff --git a/src/cmd/vet/shift.go b/src/cmd/vet/shift.go index 83009ecff7..200e20fd9d 100644 --- a/src/cmd/vet/shift.go +++ b/src/cmd/vet/shift.go @@ -87,7 +87,6 @@ func checkLongShift(f *File, node ast.Node, x, y ast.Expr) { return } var size int64 - var msg string switch b.Kind() { case types.Uint8, types.Int8: size = 8 @@ -97,15 +96,20 @@ func checkLongShift(f *File, node ast.Node, x, y ast.Expr) { size = 32 case types.Uint64, types.Int64: size = 64 - case types.Int, types.Uint, types.Uintptr: - // These types may be as small as 32 bits, but no smaller. - size = 32 - msg = "might be " + case types.Int, types.Uint: + size = uintBitSize + case types.Uintptr: + size = uintptrBitSize default: return } if amt >= size { ident := f.gofmt(x) - f.Badf(node.Pos(), "%s %stoo small for shift of %d", ident, msg, amt) + f.Badf(node.Pos(), "%s (%d bits) too small for shift of %d", ident, size, amt) } } + +var ( + uintBitSize = 8 * archSizes.Sizeof(types.Typ[types.Uint]) + uintptrBitSize = 8 * archSizes.Sizeof(types.Typ[types.Uintptr]) +) diff --git a/src/cmd/vet/testdata/shift.go b/src/cmd/vet/testdata/shift.go index 50d628d2c4..40c8c8aa4f 100644 --- a/src/cmd/vet/testdata/shift.go +++ b/src/cmd/vet/testdata/shift.go @@ -11,72 +11,93 @@ import "unsafe" func ShiftTest() { var i8 int8 _ = i8 << 7 - _ = (i8 + 1) << 8 // ERROR "\(i8 \+ 1\) too small for shift of 8" - _ = i8 << (7 + 1) // ERROR "i8 too small for shift of 8" - _ = i8 >> 8 // ERROR "i8 too small for shift of 8" - i8 <<= 8 // ERROR "i8 too small for shift of 8" - i8 >>= 8 // ERROR "i8 too small for shift of 8" + _ = (i8 + 1) << 8 // ERROR ".i8 . 1. .8 bits. too small for shift of 8" + _ = i8 << (7 + 1) // ERROR "i8 .8 bits. too small for shift of 8" + _ = i8 >> 8 // ERROR "i8 .8 bits. too small for shift of 8" + i8 <<= 8 // ERROR "i8 .8 bits. too small for shift of 8" + i8 >>= 8 // ERROR "i8 .8 bits. too small for shift of 8" var i16 int16 _ = i16 << 15 - _ = i16 << 16 // ERROR "i16 too small for shift of 16" - _ = i16 >> 16 // ERROR "i16 too small for shift of 16" - i16 <<= 16 // ERROR "i16 too small for shift of 16" - i16 >>= 16 // ERROR "i16 too small for shift of 16" + _ = i16 << 16 // ERROR "i16 .16 bits. too small for shift of 16" + _ = i16 >> 16 // ERROR "i16 .16 bits. too small for shift of 16" + i16 <<= 16 // ERROR "i16 .16 bits. too small for shift of 16" + i16 >>= 16 // ERROR "i16 .16 bits. too small for shift of 16" var i32 int32 _ = i32 << 31 - _ = i32 << 32 // ERROR "i32 too small for shift of 32" - _ = i32 >> 32 // ERROR "i32 too small for shift of 32" - i32 <<= 32 // ERROR "i32 too small for shift of 32" - i32 >>= 32 // ERROR "i32 too small for shift of 32" + _ = i32 << 32 // ERROR "i32 .32 bits. too small for shift of 32" + _ = i32 >> 32 // ERROR "i32 .32 bits. too small for shift of 32" + i32 <<= 32 // ERROR "i32 .32 bits. too small for shift of 32" + i32 >>= 32 // ERROR "i32 .32 bits. too small for shift of 32" var i64 int64 _ = i64 << 63 - _ = i64 << 64 // ERROR "i64 too small for shift of 64" - _ = i64 >> 64 // ERROR "i64 too small for shift of 64" - i64 <<= 64 // ERROR "i64 too small for shift of 64" - i64 >>= 64 // ERROR "i64 too small for shift of 64" + _ = i64 << 64 // ERROR "i64 .64 bits. too small for shift of 64" + _ = i64 >> 64 // ERROR "i64 .64 bits. too small for shift of 64" + i64 <<= 64 // ERROR "i64 .64 bits. too small for shift of 64" + i64 >>= 64 // ERROR "i64 .64 bits. too small for shift of 64" var u8 uint8 _ = u8 << 7 - _ = u8 << 8 // ERROR "u8 too small for shift of 8" - _ = u8 >> 8 // ERROR "u8 too small for shift of 8" - u8 <<= 8 // ERROR "u8 too small for shift of 8" - u8 >>= 8 // ERROR "u8 too small for shift of 8" + _ = u8 << 8 // ERROR "u8 .8 bits. too small for shift of 8" + _ = u8 >> 8 // ERROR "u8 .8 bits. too small for shift of 8" + u8 <<= 8 // ERROR "u8 .8 bits. too small for shift of 8" + u8 >>= 8 // ERROR "u8 .8 bits. too small for shift of 8" var u16 uint16 _ = u16 << 15 - _ = u16 << 16 // ERROR "u16 too small for shift of 16" - _ = u16 >> 16 // ERROR "u16 too small for shift of 16" - u16 <<= 16 // ERROR "u16 too small for shift of 16" - u16 >>= 16 // ERROR "u16 too small for shift of 16" + _ = u16 << 16 // ERROR "u16 .16 bits. too small for shift of 16" + _ = u16 >> 16 // ERROR "u16 .16 bits. too small for shift of 16" + u16 <<= 16 // ERROR "u16 .16 bits. too small for shift of 16" + u16 >>= 16 // ERROR "u16 .16 bits. too small for shift of 16" var u32 uint32 _ = u32 << 31 - _ = u32 << 32 // ERROR "u32 too small for shift of 32" - _ = u32 >> 32 // ERROR "u32 too small for shift of 32" - u32 <<= 32 // ERROR "u32 too small for shift of 32" - u32 >>= 32 // ERROR "u32 too small for shift of 32" + _ = u32 << 32 // ERROR "u32 .32 bits. too small for shift of 32" + _ = u32 >> 32 // ERROR "u32 .32 bits. too small for shift of 32" + u32 <<= 32 // ERROR "u32 .32 bits. too small for shift of 32" + u32 >>= 32 // ERROR "u32 .32 bits. too small for shift of 32" var u64 uint64 _ = u64 << 63 - _ = u64 << 64 // ERROR "u64 too small for shift of 64" - _ = u64 >> 64 // ERROR "u64 too small for shift of 64" - u64 <<= 64 // ERROR "u64 too small for shift of 64" - u64 >>= 64 // ERROR "u64 too small for shift of 64" + _ = u64 << 64 // ERROR "u64 .64 bits. too small for shift of 64" + _ = u64 >> 64 // ERROR "u64 .64 bits. too small for shift of 64" + u64 <<= 64 // ERROR "u64 .64 bits. too small for shift of 64" + u64 >>= 64 // ERROR "u64 .64 bits. too small for shift of 64" _ = u64 << u64 // Non-constant shifts should succeed. + var i int _ = i << 31 - _ = i << 32 // ERROR "i might be too small for shift of 32" - _ = i >> 32 // ERROR "i might be too small for shift of 32" - i <<= 32 // ERROR "i might be too small for shift of 32" - i >>= 32 // ERROR "i might be too small for shift of 32" + const in = 8 * unsafe.Sizeof(i) + _ = i << in // ERROR "too small for shift" + _ = i >> in // ERROR "too small for shift" + i <<= in // ERROR "too small for shift" + i >>= in // ERROR "too small for shift" + const ix = 8*unsafe.Sizeof(i) - 1 + _ = i << ix + _ = i >> ix + i <<= ix + i >>= ix + var u uint _ = u << 31 - _ = u << 32 // ERROR "u might be too small for shift of 32" - _ = u >> 32 // ERROR "u might be too small for shift of 32" - u <<= 32 // ERROR "u might be too small for shift of 32" - u >>= 32 // ERROR "u might be too small for shift of 32" + const un = 8 * unsafe.Sizeof(u) + _ = u << un // ERROR "too small for shift" + _ = u >> un // ERROR "too small for shift" + u <<= un // ERROR "too small for shift" + u >>= un // ERROR "too small for shift" + const ux = 8*unsafe.Sizeof(u) - 1 + _ = u << ux + _ = u >> ux + u <<= ux + u >>= ux + var p uintptr _ = p << 31 - _ = p << 32 // ERROR "p might be too small for shift of 32" - _ = p >> 32 // ERROR "p might be too small for shift of 32" - p <<= 32 // ERROR "p might be too small for shift of 32" - p >>= 32 // ERROR "p might be too small for shift of 32" + const pn = 8 * unsafe.Sizeof(p) + _ = p << pn // ERROR "too small for shift" + _ = p >> pn // ERROR "too small for shift" + p <<= pn // ERROR "too small for shift" + p >>= pn // ERROR "too small for shift" + const px = 8*unsafe.Sizeof(p) - 1 + _ = p << px + _ = p >> px + p <<= px + p >>= px const oneIf64Bit = ^uint(0) >> 63 // allow large shifts of constants; they are used for 32/64 bit compatibility tricks diff --git a/src/cmd/vet/types.go b/src/cmd/vet/types.go index 3a5e71c87c..77bd1c1c99 100644 --- a/src/cmd/vet/types.go +++ b/src/cmd/vet/types.go @@ -8,6 +8,7 @@ package main import ( "go/ast" + "go/build" "go/importer" "go/token" "go/types" @@ -80,6 +81,8 @@ func (pkg *Package) check(fs *token.FileSet, astFiles []*ast.File) error { // By providing a Config with our own error function, it will continue // past the first error. There is no need for that function to do anything. Error: func(error) {}, + + Sizes: archSizes, } info := &types.Info{ Selections: pkg.selectors, @@ -289,3 +292,5 @@ func (f *File) hasMethod(typ types.Type, name string) bool { _, ok := obj.(*types.Func) return ok } + +var archSizes = types.SizesFor("gc", build.Default.GOARCH) diff --git a/src/go/types/sizes.go b/src/go/types/sizes.go index 4fa71b4d5e..0821a61359 100644 --- a/src/go/types/sizes.go +++ b/src/go/types/sizes.go @@ -181,7 +181,11 @@ func SizesFor(compiler, arch string) Sizes { if compiler != "gc" { return nil } - return gcArchSizes[arch] + s, ok := gcArchSizes[arch] + if !ok { + return nil + } + return s } // stdSizes is used if Config.Sizes == nil. -- 2.48.1