+The GCC 6 releases include a complete implementation of the Go 1.6.1 +user libraries. The Go 1.6 runtime is not fully merged, but that +should not be visible to Go programs. +
+ ++The GCC 7 releases are expected to include a complete implementation +of the Go 1.8 user libraries. As with earlier releases, the Go 1.8 +runtime is not fully merged, but that should not be visible to Go +programs. +
+@@ -158,23 +171,6 @@ cd objdir ../gccgo/configure --prefix=/opt/gccgo --enable-languages=c,c++,go --with-ld=/opt/gold/bin/ld make make install - - -
-Current versions of Ubuntu and versions of GCC before 4.8 disagree on -where system libraries and header files are found. This is not a -gccgo issue. When building older versions of GCC, setting these -environment variables while configuring and building gccgo may fix the -problem. -
- --LIBRARY_PATH=/usr/lib/x86_64-linux-gnu -C_INCLUDE_PATH=/usr/include/x86_64-linux-gnu -CPLUS_INCLUDE_PATH=/usr/include/x86_64-linux-gnu -export LIBRARY_PATH C_INCLUDE_PATH CPLUS_INCLUDE_PATH
-Basic types map directly: an int in Go is an int
-in C, an int32 is an int32_t,
-etc. Go byte is equivalent to C unsigned
-char.
-Pointers in Go are pointers in C. A Go struct is the same as C
-struct with the same fields and types.
+Basic types map directly: an int32 in Go is
+an int32_t in C, an int64 is
+an int64_t, etc.
+The Go type int is an integer that is the same size as a
+pointer, and as such corresponds to the C type intptr_t.
+Go byte is equivalent to C unsigned char.
+Pointers in Go are pointers in C.
+A Go struct is the same as C struct with the
+same fields and types.
@@ -380,7 +379,7 @@
struct __go_string {
const unsigned char *__data;
- int __length;
+ intptr_t __length;
};
@@ -400,8 +399,8 @@
struct __go_slice {
void *__values;
- int __count;
- int __capacity;
+ intptr_t __count;
+ intptr_t __capacity;
};
@@ -526,15 +525,3 @@ This procedure is full of unstated caveats and restrictions and we make no
guarantee that it will not change in the future. It is more useful as a
starting point for real Go code than as a regular procedure.
-
-
-The gccgo compiler has been ported to
-RTEMS. RTEMS is a real-time executive
-that provides a high performance environment for embedded applications
-on a range of processors and embedded hardware. The current gccgo
-port is for x86. The goal is to extend the port to most of the
-
-architectures supported by RTEMS. For more information on the port,
-as well as instructions on how to install it, please see this
-RTEMS Wiki page.
diff --git a/doc/go1.8.html b/doc/go1.8.html
index 337f13d630f5a36a98c38bafd18c27715aaecf24..bc40378a6a51257c904820a2a95a03250c6c9bb6 100644
--- a/doc/go1.8.html
+++ b/doc/go1.8.html
@@ -435,11 +435,11 @@
- Go now supports a “plugin” build mode for generating
- plugins written in Go, and a
+ Go now provides early support for plugins with a “plugin”
+ build mode for generating plugins written in Go, and a
new plugin package for
- loading such plugins at run time. Plugin support is only currently
- available on Linux.
+ loading such plugins at run time. Plugin support is currently only
+ available on Linux. Please report any issues.
+ The signature of the
+ MainStart
+ function has changed, as allowed by the documentation. It is an
+ internal detail and not part of the Go 1 compatibility promise.
+ If you're not calling MainStart directly but see
+ errors, that likely means you set the
+ normally-empty GOROOT environment variable and it
+ doesn't match the version of your go command's binary.
cd to
+the src subdirectory and run make.bash (or,
+on Windows, make.bat).
diff --git a/misc/cgo/test/issue18146.go b/misc/cgo/test/issue18146.go index ffb04e9037ba4e1db7e35fabda86ef9011e2587a..3c600463f0335447f8a36c44fb20f607519f2b64 100644 --- a/misc/cgo/test/issue18146.go +++ b/misc/cgo/test/issue18146.go @@ -73,7 +73,7 @@ }() } runtime.GOMAXPROCS(threads) argv := append(os.Args, "-test.run=NoSuchTestExists") - if err := syscall.Exec(os.Args[0], argv, nil); err != nil { + if err := syscall.Exec(os.Args[0], argv, os.Environ()); err != nil { t.Fatal(err) } } diff --git a/misc/cgo/test/sigaltstack.go b/misc/cgo/test/sigaltstack.go index b16adc7d88f9822081c4ee969085e916c85fd47e..2b7a1ec9ad0a01e0dcb34f37ca756afbc94011c3 100644 --- a/misc/cgo/test/sigaltstack.go +++ b/misc/cgo/test/sigaltstack.go @@ -17,7 +17,7 @@ static stack_t oss; static char signalStack[SIGSTKSZ]; -static void changeSignalStack() { +static void changeSignalStack(void) { stack_t ss; memset(&ss, 0, sizeof ss); ss.ss_sp = signalStack; @@ -29,7 +29,7 @@ abort(); } } -static void restoreSignalStack() { +static void restoreSignalStack(void) { #if (defined(__x86_64__) || defined(__i386__)) && defined(__APPLE__) // The Darwin C library enforces a minimum that the kernel does not. // This is OK since we allocated this much space in mpreinit, @@ -42,7 +42,7 @@ abort(); } } -static int zero() { +static int zero(void) { return 0; } */ diff --git a/misc/cgo/testsanitizers/msan_shared.go b/misc/cgo/testsanitizers/msan_shared.go new file mode 100644 index 0000000000000000000000000000000000000000..966947cac359802072372ee044bd40323472b884 --- /dev/null +++ b/misc/cgo/testsanitizers/msan_shared.go @@ -0,0 +1,12 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// This program segfaulted during libpreinit when built with -msan: +// http://golang.org/issue/18707 + +package main + +import "C" + +func main() {} diff --git a/misc/cgo/testsanitizers/test.bash b/misc/cgo/testsanitizers/test.bash index dfc6d3819a284413c64cb517f9fa2dc998897df8..4da85020d89800a3db00594b1b7937b04b6646c3 100755 --- a/misc/cgo/testsanitizers/test.bash +++ b/misc/cgo/testsanitizers/test.bash @@ -68,6 +68,25 @@ fi status=0 +testmsanshared() { + goos=$(go env GOOS) + suffix="-installsuffix testsanitizers" + libext="so" + if [ "$goos" == "darwin" ]; then + libext="dylib" + fi + go build -msan -buildmode=c-shared $suffix -o ${TMPDIR}/libmsanshared.$libext msan_shared.go + + echo 'int main() { return 0; }' > ${TMPDIR}/testmsanshared.c + $CC $(go env GOGCCFLAGS) -fsanitize=memory -o ${TMPDIR}/testmsanshared ${TMPDIR}/testmsanshared.c ${TMPDIR}/libmsanshared.$libext + + if ! LD_LIBRARY_PATH=. ${TMPDIR}/testmsanshared; then + echo "FAIL: msan_shared" + status=1 + fi + rm -f ${TMPDIR}/{testmsanshared,testmsanshared.c,libmsanshared.$libext} +} + if test "$msan" = "yes"; then if ! go build -msan std; then echo "FAIL: build -msan std" @@ -108,6 +127,8 @@ if go run -msan msan_fail.go 2>/dev/null; then echo "FAIL: msan_fail" status=1 fi + + testmsanshared fi if test "$tsan" = "yes"; then diff --git a/misc/cgo/testshared/src/depBase/dep.go b/misc/cgo/testshared/src/depBase/dep.go index a518b4efe2955e785b7c46fddf9513bf89902624..9f86710db01a4438fe4ab7fb7e8f3b13829592f5 100644 --- a/misc/cgo/testshared/src/depBase/dep.go +++ b/misc/cgo/testshared/src/depBase/dep.go @@ -5,6 +5,8 @@ "os" "reflect" ) +var SlicePtr interface{} = &[]int{} + var V int = 1 var HasMask []string = []string{"hi"} diff --git a/misc/cgo/testshared/src/exe/exe.go b/misc/cgo/testshared/src/exe/exe.go index 433727112b13166d1bdc71a5c82e73e32b7b422b..84302a811f0919b9035b8da22814581af1d122a0 100644 --- a/misc/cgo/testshared/src/exe/exe.go +++ b/misc/cgo/testshared/src/exe/exe.go @@ -19,6 +19,8 @@ func F() *C { return nil } +var slicePtr interface{} = &[]int{} + func main() { defer depBase.ImplementedInAsm() // This code below causes various go.itab.* symbols to be generated in @@ -31,5 +33,12 @@ var c *C if reflect.TypeOf(F).Out(0) != reflect.TypeOf(c) { panic("bad reflection results, see golang.org/issue/18252") + } + + sp := reflect.New(reflect.TypeOf(slicePtr).Elem()) + s := sp.Interface() + + if reflect.TypeOf(s) != reflect.TypeOf(slicePtr) { + panic("bad reflection results, see golang.org/issue/18729") } } diff --git a/src/cmd/compile/internal/gc/util.go b/src/cmd/compile/internal/gc/util.go index bb5cede5a60da75f700594c4aede4dacfa39ae01..c62bd00808f21c625343a495cebc31aab4747b30 100644 --- a/src/cmd/compile/internal/gc/util.go +++ b/src/cmd/compile/internal/gc/util.go @@ -57,8 +57,13 @@ if err != nil { Fatalf("%v", err) } atExit(func() { - runtime.GC() // profile all outstanding allocations - if err := pprof.WriteHeapProfile(f); err != nil { + // Profile all outstanding allocations. + runtime.GC() + // compilebench parses the memory profile to extract memstats, + // which are only written in the legacy pprof format. + // See golang.org/issue/18641 and runtime/pprof/pprof.go:writeHeap. + const writeLegacyFormat = 1 + if err := pprof.Lookup("heap").WriteTo(f, writeLegacyFormat); err != nil { Fatalf("%v", err) } }) diff --git a/src/cmd/compile/internal/ssa/gen/S390X.rules b/src/cmd/compile/internal/ssa/gen/S390X.rules index 3e0533a95142d3bb8f883a8b609339086bca9ce7..be0d581fe099289c09ee805370af5588848cdee7 100644 --- a/src/cmd/compile/internal/ssa/gen/S390X.rules +++ b/src/cmd/compile/internal/ssa/gen/S390X.rules @@ -885,9 +885,9 @@ (MOVDEQ _ x (FlagEQ)) -> x (MOVDEQ y _ (FlagLT)) -> y (MOVDEQ y _ (FlagGT)) -> y -(MOVDNE _ y (FlagEQ)) -> y -(MOVDNE x _ (FlagLT)) -> x -(MOVDNE x _ (FlagGT)) -> x +(MOVDNE y _ (FlagEQ)) -> y +(MOVDNE _ x (FlagLT)) -> x +(MOVDNE _ x (FlagGT)) -> x (MOVDLT y _ (FlagEQ)) -> y (MOVDLT _ x (FlagLT)) -> x diff --git a/src/cmd/compile/internal/ssa/nilcheck.go b/src/cmd/compile/internal/ssa/nilcheck.go index 9f58db664b144499911a8924071b8b45b450e9b7..0a34cd1ae642305e44a516e646413e0cf74ba20e 100644 --- a/src/cmd/compile/internal/ssa/nilcheck.go +++ b/src/cmd/compile/internal/ssa/nilcheck.go @@ -82,7 +82,7 @@ } } } - // Next, process values in the block. + // Next, eliminate any redundant nil checks in this block. i := 0 for _, v := range b.Values { b.Values[i] = v @@ -105,19 +105,31 @@ if f.Config.Debug_checknil() && v.Line > 1 { f.Config.Warnl(v.Line, "removed nil check") } v.reset(OpUnknown) + // TODO: f.freeValue(v) i-- continue } - // Record the fact that we know ptr is non nil, and remember to - // undo that information when this dominator subtree is done. - nonNilValues[ptr.ID] = true - work = append(work, bp{op: ClearPtr, ptr: ptr}) } } for j := i; j < len(b.Values); j++ { b.Values[j] = nil } b.Values = b.Values[:i] + + // Finally, find redundant nil checks for subsequent blocks. + // Note that we can't add these until the loop above is done, as the + // values in the block are not ordered in any way when this pass runs. + // This was the cause of issue #18725. + for _, v := range b.Values { + if v.Op != OpNilCheck { + continue + } + ptr := v.Args[0] + // Record the fact that we know ptr is non nil, and remember to + // undo that information when this dominator subtree is done. + nonNilValues[ptr.ID] = true + work = append(work, bp{op: ClearPtr, ptr: ptr}) + } // Add all dominated blocks to the work list. for w := sdom[node.block.ID].child; w != nil; w = sdom[w.ID].sibling { diff --git a/src/cmd/compile/internal/ssa/rewriteS390X.go b/src/cmd/compile/internal/ssa/rewriteS390X.go index 7d023bcf8ba38f18b04fac62a4cd22694d622182..5acaf2dbdc4fcb1466c36afa07112dcbd8f4de07 100644 --- a/src/cmd/compile/internal/ssa/rewriteS390X.go +++ b/src/cmd/compile/internal/ssa/rewriteS390X.go @@ -9847,11 +9847,11 @@ v.AddArg(y) v.AddArg(cmp) return true } - // match: (MOVDNE _ y (FlagEQ)) + // match: (MOVDNE y _ (FlagEQ)) // cond: // result: y for { - y := v.Args[1] + y := v.Args[0] v_2 := v.Args[2] if v_2.Op != OpS390XFlagEQ { break @@ -9861,11 +9861,11 @@ v.Type = y.Type v.AddArg(y) return true } - // match: (MOVDNE x _ (FlagLT)) + // match: (MOVDNE _ x (FlagLT)) // cond: // result: x for { - x := v.Args[0] + x := v.Args[1] v_2 := v.Args[2] if v_2.Op != OpS390XFlagLT { break @@ -9875,11 +9875,11 @@ v.Type = x.Type v.AddArg(x) return true } - // match: (MOVDNE x _ (FlagGT)) + // match: (MOVDNE _ x (FlagGT)) // cond: // result: x for { - x := v.Args[0] + x := v.Args[1] v_2 := v.Args[2] if v_2.Op != OpS390XFlagGT { break diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go index 5727eb094e5473a1b5eadaed70a53e16aa9f0ef9..f26c3660e4b809a1ea76d13b4d1a2176fdf419d4 100644 --- a/src/cmd/go/go_test.go +++ b/src/cmd/go/go_test.go @@ -3787,3 +3787,26 @@ tg.tempFile("go/src/p/p.go", `package p`) tg.setenv("GOPATH", tg.path("go")) tg.run("build", "p") } + +// Issue 18778. +func TestDotDotDotOutsideGOPATH(t *testing.T) { + tg := testgo(t) + defer tg.cleanup() + + tg.tempFile("pkgs/a.go", `package x`) + tg.tempFile("pkgs/a_test.go", `package x_test +import "testing" +func TestX(t *testing.T) {}`) + + tg.tempFile("pkgs/a/a.go", `package a`) + tg.tempFile("pkgs/a/a_test.go", `package a_test +import "testing" +func TestA(t *testing.T) {}`) + + tg.cd(tg.path("pkgs")) + tg.run("build", "./...") + tg.run("test", "./...") + tg.run("list", "./...") + tg.grepStdout("pkgs$", "expected package not listed") + tg.grepStdout("pkgs/a", "expected package not listed") +} diff --git a/src/cmd/go/pkg.go b/src/cmd/go/pkg.go index d69fa5118f520c374f305b02a8f422b7df861bc2..e40f9420c7e2217f9e126cf5f636c282c532d959 100644 --- a/src/cmd/go/pkg.go +++ b/src/cmd/go/pkg.go @@ -429,7 +429,7 @@ func cleanImport(path string) string { orig := path path = pathpkg.Clean(path) - if strings.HasPrefix(orig, "./") && path != ".." && path != "." && !strings.HasPrefix(path, "../") { + if strings.HasPrefix(orig, "./") && path != ".." && !strings.HasPrefix(path, "../") { path = "./" + path } return path diff --git a/src/cmd/link/internal/ld/dwarf.go b/src/cmd/link/internal/ld/dwarf.go index 61d3e4fb720ecd1299540652816fcc2e11a90807..22d2c548c373e0c3b91f4805e8035fd3f64192cf 100644 --- a/src/cmd/link/internal/ld/dwarf.go +++ b/src/cmd/link/internal/ld/dwarf.go @@ -1080,7 +1080,7 @@ epc = s.Value + s.Size epcs = s dsym := ctxt.Syms.Lookup(dwarf.InfoPrefix+s.Name, int(s.Version)) - dsym.Attr |= AttrHidden + dsym.Attr |= AttrHidden | AttrReachable dsym.Type = obj.SDWARFINFO for _, r := range dsym.R { if r.Type == obj.R_DWARFREF && r.Sym.Size == 0 { diff --git a/src/database/sql/sql.go b/src/database/sql/sql.go index 0fa7c34a13e3bb7ee209b9b01a33befe2f414e80..feb91223a9e9b99f48fa65e041afd6f7d309e791 100644 --- a/src/database/sql/sql.go +++ b/src/database/sql/sql.go @@ -1357,16 +1357,7 @@ txi: txi, cancel: cancel, ctx: ctx, } - go func(tx *Tx) { - select { - case <-tx.ctx.Done(): - if !tx.isDone() { - // Discard and close the connection used to ensure the transaction - // is closed and the resources are released. - tx.rollback(true) - } - } - }(tx) + go tx.awaitDone() return tx, nil } @@ -1388,6 +1379,11 @@ // by the call to Commit or Rollback. type Tx struct { db *DB + // closemu prevents the transaction from closing while there + // is an active query. It is held for read during queries + // and exclusively during close. + closemu sync.RWMutex + // dc is owned exclusively until Commit or Rollback, at which point // it's returned with putConn. dc *driverConn @@ -1413,6 +1409,20 @@ // ctx lives for the life of the transaction. ctx context.Context } +// awaitDone blocks until the context in Tx is canceled and rolls back +// the transaction if it's not already done. +func (tx *Tx) awaitDone() { + // Wait for either the transaction to be committed or rolled + // back, or for the associated context to be closed. + <-tx.ctx.Done() + + // Discard and close the connection used to ensure the + // transaction is closed and the resources are released. This + // rollback does nothing if the transaction has already been + // committed or rolled back. + tx.rollback(true) +} + func (tx *Tx) isDone() bool { return atomic.LoadInt32(&tx.done) != 0 } @@ -1424,15 +1434,30 @@ // close returns the connection to the pool and // must only be called by Tx.rollback or Tx.Commit. func (tx *Tx) close(err error) { + tx.closemu.Lock() + defer tx.closemu.Unlock() + tx.db.putConn(tx.dc, err) tx.cancel() tx.dc = nil tx.txi = nil } +// hookTxGrabConn specifies an optional hook to be called on +// a successful call to (*Tx).grabConn. For tests. +var hookTxGrabConn func() + func (tx *Tx) grabConn(ctx context.Context) (*driverConn, error) { + select { + default: + case <-ctx.Done(): + return nil, ctx.Err() + } if tx.isDone() { return nil, ErrTxDone + } + if hookTxGrabConn != nil { // test hook + hookTxGrabConn() } return tx.dc, nil } @@ -1503,6 +1528,9 @@ // The provided context will be used for the preparation of the context, not // for the execution of the returned statement. The returned statement // will run in the transaction context. func (tx *Tx) PrepareContext(ctx context.Context, query string) (*Stmt, error) { + tx.closemu.RLock() + defer tx.closemu.RUnlock() + // TODO(bradfitz): We could be more efficient here and either // provide a method to take an existing Stmt (created on // perhaps a different Conn), and re-create it on this Conn if @@ -1567,6 +1595,9 @@ // // The returned statement operates within the transaction and will be closed // when the transaction has been committed or rolled back. func (tx *Tx) StmtContext(ctx context.Context, stmt *Stmt) *Stmt { + tx.closemu.RLock() + defer tx.closemu.RUnlock() + // TODO(bradfitz): optimize this. Currently this re-prepares // each time. This is fine for now to illustrate the API but // we should really cache already-prepared statements @@ -1618,6 +1649,9 @@ // ExecContext executes a query that doesn't return rows. // For example: an INSERT and UPDATE. func (tx *Tx) ExecContext(ctx context.Context, query string, args ...interface{}) (Result, error) { + tx.closemu.RLock() + defer tx.closemu.RUnlock() + dc, err := tx.grabConn(ctx) if err != nil { return nil, err @@ -1661,6 +1695,9 @@ } // QueryContext executes a query that returns rows, typically a SELECT. func (tx *Tx) QueryContext(ctx context.Context, query string, args ...interface{}) (*Rows, error) { + tx.closemu.RLock() + defer tx.closemu.RUnlock() + dc, err := tx.grabConn(ctx) if err != nil { return nil, err @@ -2038,25 +2075,21 @@ // closed value is 1 when the Rows is closed. // Use atomic operations on value when checking value. closed int32 - ctxClose chan struct{} // closed when Rows is closed, may be null. + cancel func() // called when Rows is closed, may be nil. lastcols []driver.Value lasterr error // non-nil only if closed is true closeStmt *driverStmt // if non-nil, statement to Close on close } func (rs *Rows) initContextClose(ctx context.Context) { - if ctx.Done() == context.Background().Done() { - return - } + ctx, rs.cancel = context.WithCancel(ctx) + go rs.awaitDone(ctx) +} - rs.ctxClose = make(chan struct{}) - go func() { - select { - case <-ctx.Done(): - rs.Close() - case <-rs.ctxClose: - } - }() +// awaitDone blocks until the rows are closed or the context canceled. +func (rs *Rows) awaitDone(ctx context.Context) { + <-ctx.Done() + rs.Close() } // Next prepares the next result row for reading with the Scan method. It @@ -2314,7 +2347,9 @@ } return nil } -var rowsCloseHook func(*Rows, *error) +// rowsCloseHook returns a function so tests may install the +// hook throug a test only mutex. +var rowsCloseHook = func() func(*Rows, *error) { return nil } func (rs *Rows) isClosed() bool { return atomic.LoadInt32(&rs.closed) != 0 @@ -2328,13 +2363,15 @@ func (rs *Rows) Close() error { if !atomic.CompareAndSwapInt32(&rs.closed, 0, 1) { return nil } - if rs.ctxClose != nil { - close(rs.ctxClose) - } + err := rs.rowsi.Close() - if fn := rowsCloseHook; fn != nil { + if fn := rowsCloseHook(); fn != nil { fn(rs, &err) } + if rs.cancel != nil { + rs.cancel() + } + if rs.closeStmt != nil { rs.closeStmt.Close() } diff --git a/src/database/sql/sql_test.go b/src/database/sql/sql_test.go index 63e1292cb1f2c50591f2e1f63f2606a879de5e95..898df3b455b22cac915bdb19510cc9ba22f83cbe 100644 --- a/src/database/sql/sql_test.go +++ b/src/database/sql/sql_test.go @@ -14,6 +14,7 @@ "reflect" "runtime" "strings" "sync" + "sync/atomic" "testing" "time" ) @@ -326,9 +327,7 @@ } // And verify that the final rows.Next() call, which hit EOF, // also closed the rows connection. - if n := db.numFreeConns(); n != 1 { - t.Fatalf("free conns after query hitting EOF = %d; want 1", n) - } + waitForFree(t, db, 5*time.Second, 1) if prepares := numPrepares(t, db) - prepares0; prepares != 1 { t.Errorf("executed %d Prepare statements; want 1", prepares) } @@ -345,6 +344,18 @@ } return false } +// waitForFree checks db.numFreeConns until either it equals want or +// the maxWait time elapses. +func waitForFree(t *testing.T, db *DB, maxWait time.Duration, want int) { + var numFree int + if !waitCondition(maxWait, 5*time.Millisecond, func() bool { + numFree = db.numFreeConns() + return numFree == want + }) { + t.Fatalf("free conns after hitting EOF = %d; want %d", numFree, want) + } +} + func TestQueryContextWait(t *testing.T) { db := newTestDB(t, "people") defer closeDB(t, db) @@ -361,9 +372,7 @@ t.Fatalf("expected QueryContext to error with context deadline exceeded but returned %v", err) } // Verify closed rows connection after error condition. - if n := db.numFreeConns(); n != 1 { - t.Fatalf("free conns after query hitting EOF = %d; want 1", n) - } + waitForFree(t, db, 5*time.Second, 1) if prepares := numPrepares(t, db) - prepares0; prepares != 1 { t.Errorf("executed %d Prepare statements; want 1", prepares) } @@ -388,13 +397,7 @@ if err != context.DeadlineExceeded { t.Fatalf("expected QueryContext to error with context deadline exceeded but returned %v", err) } - var numFree int - if !waitCondition(5*time.Second, 5*time.Millisecond, func() bool { - numFree = db.numFreeConns() - return numFree == 0 - }) { - t.Fatalf("free conns after hitting EOF = %d; want 0", numFree) - } + waitForFree(t, db, 5*time.Second, 0) // Ensure the dropped connection allows more connections to be made. // Checked on DB Close. @@ -471,9 +474,7 @@ } // And verify that the final rows.Next() call, which hit EOF, // also closed the rows connection. - if n := db.numFreeConns(); n != 1 { - t.Fatalf("free conns after query hitting EOF = %d; want 1", n) - } + waitForFree(t, db, 5*time.Second, 1) if prepares := numPrepares(t, db) - prepares0; prepares != 1 { t.Errorf("executed %d Prepare statements; want 1", prepares) } @@ -1135,6 +1136,24 @@ t.Errorf("statement close mismatch: made %d, closed %d", made, closed) } } +var atomicRowsCloseHook atomic.Value // of func(*Rows, *error) + +func init() { + rowsCloseHook = func() func(*Rows, *error) { + fn, _ := atomicRowsCloseHook.Load().(func(*Rows, *error)) + return fn + } +} + +func setRowsCloseHook(fn func(*Rows, *error)) { + if fn == nil { + // Can't change an atomic.Value back to nil, so set it to this + // no-op func instead. + fn = func(*Rows, *error) {} + } + atomicRowsCloseHook.Store(fn) +} + // Test issue 6651 func TestIssue6651(t *testing.T) { db := newTestDB(t, "people") @@ -1147,6 +1166,7 @@ rowsCursorNextHook = func(dest []driver.Value) error { return fmt.Errorf(want) } defer func() { rowsCursorNextHook = nil }() + err := db.QueryRow("SELECT|people|name|").Scan(&v) if err == nil || err.Error() != want { t.Errorf("error = %q; want %q", err, want) @@ -1154,10 +1174,10 @@ } rowsCursorNextHook = nil want = "error in rows.Close" - rowsCloseHook = func(rows *Rows, err *error) { + setRowsCloseHook(func(rows *Rows, err *error) { *err = fmt.Errorf(want) - } - defer func() { rowsCloseHook = nil }() + }) + defer setRowsCloseHook(nil) err = db.QueryRow("SELECT|people|name|").Scan(&v) if err == nil || err.Error() != want { t.Errorf("error = %q; want %q", err, want) @@ -1830,7 +1850,9 @@ t.Errorf("db connections opened = %d; want <= 2", openDelta) db.dumpDeps(t) } - if len(stmt.css) > nquery { + if !waitCondition(5*time.Second, 5*time.Millisecond, func() bool { + return len(stmt.css) <= nquery + }) { t.Errorf("len(stmt.css) = %d; want <= %d", len(stmt.css), nquery) } @@ -2576,10 +2598,10 @@ stmt, err := db.Prepare("SELECT|people|name|") if err != nil { t.Fatal(err) } - rowsCloseHook = func(rows *Rows, err *error) { + setRowsCloseHook(func(rows *Rows, err *error) { *err = driver.ErrBadConn - } - defer func() { rowsCloseHook = nil }() + }) + defer setRowsCloseHook(nil) for i := 0; i < 10; i++ { rows, err := stmt.Query() if err != nil { @@ -2642,7 +2664,10 @@ tx, err := db.BeginTx(ctx, nil) if err != nil { return } - rows, err := tx.QueryContext(ctx, "WAIT|"+qwait+"|SELECT|people|name|") + // This is expected to give a cancel error many, but not all the time. + // Test failure will happen with a panic or other race condition being + // reported. + rows, _ := tx.QueryContext(ctx, "WAIT|"+qwait+"|SELECT|people|name|") if rows != nil { rows.Close() } @@ -2653,6 +2678,56 @@ }() } wg.Wait() time.Sleep(milliWait * 3 * time.Millisecond) +} + +// TestIssue18719 closes the context right before use. The sql.driverConn +// will nil out the ci on close in a lock, but if another process uses it right after +// it will panic with on the nil ref. +// +// See https://golang.org/cl/35550 . +func TestIssue18719(t *testing.T) { + db := newTestDB(t, "people") + defer closeDB(t, db) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + tx, err := db.BeginTx(ctx, nil) + if err != nil { + t.Fatal(err) + } + + hookTxGrabConn = func() { + cancel() + + // Wait for the context to cancel and tx to rollback. + for tx.isDone() == false { + time.Sleep(time.Millisecond * 3) + } + } + defer func() { hookTxGrabConn = nil }() + + // This call will grab the connection and cancel the context + // after it has done so. Code after must deal with the canceled state. + rows, err := tx.QueryContext(ctx, "SELECT|people|name|") + if err != nil { + rows.Close() + t.Fatalf("expected error %v but got %v", nil, err) + } + + // Rows may be ignored because it will be closed when the context is canceled. + + // Do not explicitly rollback. The rollback will happen from the + // canceled context. + + // Wait for connections to return to pool. + var numOpen int + if !waitCondition(5*time.Second, 5*time.Millisecond, func() bool { + numOpen = db.numOpenConns() + return numOpen == 0 + }) { + t.Fatalf("open conns after hitting EOF = %d; want 0", numOpen) + } } func TestConcurrency(t *testing.T) { diff --git a/src/go/printer/nodes.go b/src/go/printer/nodes.go index 11f26d45ea39429afb6eed2f4ee03588812dbaf5..ea432860a8e5ac2b159b48fbb8941182108fec00 100644 --- a/src/go/printer/nodes.go +++ b/src/go/printer/nodes.go @@ -733,7 +733,7 @@ p.print(x) case *ast.FuncLit: p.expr(x.Type) - p.adjBlock(p.distanceFrom(x.Type.Pos()), blank, x.Body) + p.funcBody(p.distanceFrom(x.Type.Pos()), blank, x.Body) case *ast.ParenExpr: if _, hasParens := x.X.(*ast.ParenExpr); hasParens { @@ -825,6 +825,7 @@ // composite literal elements that are composite literals themselves may have the type omitted if x.Type != nil { p.expr1(x.Type, token.HighestPrec, depth) } + p.level++ p.print(x.Lbrace, token.LBRACE) p.exprList(x.Lbrace, x.Elts, 1, commaTerm, x.Rbrace) // do not insert extra line break following a /*-style comment @@ -837,6 +838,7 @@ if len(x.Elts) > 0 { mode |= noExtraBlank } p.print(mode, x.Rbrace, token.RBRACE, mode) + p.level-- case *ast.Ellipsis: p.print(token.ELLIPSIS) @@ -1557,17 +1559,22 @@ } return bodySize } -// adjBlock prints an "adjacent" block (e.g., a for-loop or function body) following -// a header (e.g., a for-loop control clause or function signature) of given headerSize. +// funcBody prints a function body following a function header of given headerSize. // If the header's and block's size are "small enough" and the block is "simple enough", // the block is printed on the current line, without line breaks, spaced from the header // by sep. Otherwise the block's opening "{" is printed on the current line, followed by // lines for the block's statements and its closing "}". // -func (p *printer) adjBlock(headerSize int, sep whiteSpace, b *ast.BlockStmt) { +func (p *printer) funcBody(headerSize int, sep whiteSpace, b *ast.BlockStmt) { if b == nil { return } + + // save/restore composite literal nesting level + defer func(level int) { + p.level = level + }(p.level) + p.level = 0 const maxSize = 100 if headerSize+p.bodySize(b, maxSize) <= maxSize { @@ -1613,7 +1620,7 @@ p.print(blank) } p.expr(d.Name) p.signature(d.Type.Params, d.Type.Results) - p.adjBlock(p.distanceFrom(d.Pos()), vtab, d.Body) + p.funcBody(p.distanceFrom(d.Pos()), vtab, d.Body) } func (p *printer) decl(decl ast.Decl) { diff --git a/src/go/printer/printer.go b/src/go/printer/printer.go index eabf23e8b28691ec975c0fdcb5aee2584081f893..be61dad590eea02012364ae32c9c602d256f9c11 100644 --- a/src/go/printer/printer.go +++ b/src/go/printer/printer.go @@ -58,6 +58,7 @@ // Current state output []byte // raw printer result indent int // current indentation + level int // level == 0: outside composite literal; level > 0: inside composite literal mode pmode // current printer mode impliedSemi bool // if set, a linebreak implies a semicolon lastTok token.Token // last token printed (token.ILLEGAL if it's whitespace) @@ -744,15 +745,19 @@ // If the last comment is a /*-style comment and the next item // follows on the same line but is not a comma, and not a "closing" // token immediately following its corresponding "opening" token, // add an extra separator unless explicitly disabled. Use a blank - // as separator unless we have pending linebreaks and they are not - // disabled, in which case we want a linebreak (issue 15137). + // as separator unless we have pending linebreaks, they are not + // disabled, and we are outside a composite literal, in which case + // we want a linebreak (issue 15137). + // TODO(gri) This has become overly complicated. We should be able + // to track whether we're inside an expression or statement and + // use that information to decide more directly. needsLinebreak := false if p.mode&noExtraBlank == 0 && last.Text[1] == '*' && p.lineFor(last.Pos()) == next.Line && tok != token.COMMA && (tok != token.RPAREN || p.prevOpen == token.LPAREN) && (tok != token.RBRACK || p.prevOpen == token.LBRACK) { - if p.containsLinebreak() && p.mode&noExtraLinebreak == 0 { + if p.containsLinebreak() && p.mode&noExtraLinebreak == 0 && p.level == 0 { needsLinebreak = true } else { p.writeByte(' ', 1) diff --git a/src/go/printer/testdata/comments2.golden b/src/go/printer/testdata/comments2.golden index 7676a26c1259eec0a56ca1a3c2930bc593f86de6..8b3a94ddcd03a0ac75ab7e6c2bbfa5dbb88c2b95 100644 --- a/src/go/printer/testdata/comments2.golden +++ b/src/go/printer/testdata/comments2.golden @@ -103,3 +103,62 @@ label: mask := uint64(1)<