cmd/compile: minor cleanup

Follow-up to review comments on CL 41797.

Mask the input to set2 and set3, so that at the very least,
we won't corrupt the rest of the flags in case of a bad input.
It also seems more semantically appropriate.

Do minor cleanup in addrescapes. I started on larger cleanup,
but it wasn't clear that it was an improvement.

Add warning comments and sanity checks to Initorder and Class constants,
to attempt to prevent them from overflowing their allotted flag bits.

Passes toolstash-check.

Change-Id: I57b9661ba36f56406aa7a1d8da9b7c70338f9119
Reviewed-on: https://go-review.googlesource.com/41817
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/cmd/compile/internal/gc/bitset.go b/src/cmd/compile/internal/gc/bitset.go
index 89e6fe85c88f23e8fc937717404e5c5adae5db5b..90babd5a9f6bce92b653e0eb75acc356857fc489 100644 (file)
--- a/src/cmd/compile/internal/gc/bitset.go
+++ b/src/cmd/compile/internal/gc/bitset.go
@@ -28,20 +28,22 @@ func (f bitset32) get2(shift uint8) uint8 {
        return uint8(f>>shift) & 3
 }
 
+// set2 sets two bits in f using the bottom two bits of b.
 func (f *bitset32) set2(shift uint8, b uint8) {
        // Clear old bits.
        *(*uint32)(f) &^= 3 << shift
        // Set new bits.
-       *(*uint32)(f) |= uint32(b) << shift
+       *(*uint32)(f) |= uint32(b&3) << shift
 }
 
 func (f bitset32) get3(shift uint8) uint8 {
        return uint8(f>>shift) & 7
 }
 
+// set3 sets three bits in f using the bottom three bits of b.
 func (f *bitset32) set3(shift uint8, b uint8) {
        // Clear old bits.
        *(*uint32)(f) &^= 7 << shift
        // Set new bits.
-       *(*uint32)(f) |= uint32(b) << shift
+       *(*uint32)(f) |= uint32(b&7) << shift
 }

diff --git a/src/cmd/compile/internal/gc/gen.go b/src/cmd/compile/internal/gc/gen.go
index fffc5b76dd0b3b16f1811505cd17e549617dbf0a..941c41502ae44c9c83b449e9bae6b9ee9231b3c5 100644 (file)
--- a/src/cmd/compile/internal/gc/gen.go
+++ b/src/cmd/compile/internal/gc/gen.go
@@ -24,10 +24,11 @@ func Sysfunc(name string) *obj.LSym {
 // to be taken.
 func addrescapes(n *Node) {
        switch n.Op {
-       // probably a type error already.
-       // dump("addrescapes", n);
        default:
-               break
+               // Unexpected Op, probably due to a previous type error. Ignore.
+
+       case OIND, ODOTPTR:
+               // Nothing to do.
 
        case ONAME:
                if n == nodfp {
@@ -73,9 +74,6 @@ func addrescapes(n *Node) {
                Curfn = oldfn
                lineno = ln
 
-       case OIND, ODOTPTR:
-               break
-
        // ODOTPTR has already been introduced,
        // so these are the non-pointer ODOT and OINDEX.
        // In &x[0], if x is a slice, then x does not

diff --git a/src/cmd/compile/internal/gc/go.go b/src/cmd/compile/internal/gc/go.go
index 46cec3e8bc2a2f408031ff0ae46825cbf907e29a..f795ce0bf0e4a4349b7eab4f330103002f45ab42 100644 (file)
--- a/src/cmd/compile/internal/gc/go.go
+++ b/src/cmd/compile/internal/gc/go.go
@@ -31,17 +31,25 @@ func isRuntimePkg(p *types.Pkg) bool {
 type Class uint8
 
 const (
-       Pxxx      Class = iota
-       PEXTERN         // global variable
-       PAUTO           // local variables
-       PAUTOHEAP       // local variable or parameter moved to heap
-       PPARAM          // input arguments
-       PPARAMOUT       // output results
-       PFUNC           // global function
+       Pxxx      Class = iota // no class; used during ssa conversion to indicate pseudo-variables
+       PEXTERN                // global variable
+       PAUTO                  // local variables
+       PAUTOHEAP              // local variable or parameter moved to heap
+       PPARAM                 // input arguments
+       PPARAMOUT              // output results
+       PFUNC                  // global function
 
        PDISCARD // discard during parse of duplicate import
+       // Careful: Class is stored in three bits in Node.flags.
+       // Adding a new Class will overflow that.
 )
 
+func init() {
+       if PDISCARD != 7 {
+               panic("PDISCARD changed; does all Class values still fit in three bits?")
+       }
+}
+
 // note this is the runtime representation
 // of the compilers arrays.
 //

diff --git a/src/cmd/compile/internal/gc/sinit.go b/src/cmd/compile/internal/gc/sinit.go
index e8f3d70bd101f1dcfbcd40972942b9fd128b45c3..1a1dbc0de72973be5d9475f87d023ba904e3bc72 100644 (file)
--- a/src/cmd/compile/internal/gc/sinit.go
+++ b/src/cmd/compile/internal/gc/sinit.go
@@ -9,11 +9,12 @@ import (
        "fmt"
 )
 
-// static initialization
+// Static initialization ordering state.
+// These values are stored in two bits in Node.flags.
 const (
-       InitNotStarted = 0
-       InitDone       = 1
-       InitPending    = 2
+       InitNotStarted = iota
+       InitDone
+       InitPending
 )
 
 type InitEntry struct {