From 799da9cee79e7ee89156ddc3c8bea44fdf1ac252 Mon Sep 17 00:00:00 2001 From: Rob Pike Date: Wed, 29 Oct 2014 08:15:58 -0700 Subject: [PATCH 01/25] doc/go1.4.html: half of the small library changes LGTM=rsc R=golang-codereviews, rsc CC=golang-codereviews https://golang.org/cl/165090043 --- doc/go1.4.html | 170 +++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 137 insertions(+), 33 deletions(-) diff --git a/doc/go1.4.html b/doc/go1.4.html index 7e670c47cb..9fa86c31ae 100644 --- a/doc/go1.4.html +++ b/doc/go1.4.html @@ -487,6 +487,16 @@ need to know about the new location. All tools and services maintained by the Go have been updated.

+ +

SWIG

+ +

+Due to the runtime changes in this release, Go 1.4 will require SWIG 3.0.3. +At time of writing that has not yet been released, but we expect it to be by +Go 1.4's release date. +TODO +

+

Miscellany

@@ -549,7 +559,7 @@ TODO major changes 

-encoding/gob: remove unsafe (CL 102680045)
+bufio: handling of empty tokens at EOF changed, may require scanner change (CL 145390043)
 syscall: now frozen (CL 129820043); go.sys subrepo created: http://golang.org/s/go1.4-syscall
@@ -562,37 +572,131 @@ See the relevant package documentation for more information about each change. - -
-
-cmd/6l, liblink: use pc-relative addressing for all memory references, so that linking Go binaries at high addresses works (CL 125140043). This cuts the maximum size of a Go binary's text+data+bss from 4GB to 2GB.
-
-bufio: handling of empty tokens at EOF changed, may require scanner change (CL 145390043)
-compress/flate, compress/gzip, compress/zlib: Reset support (https://codereview.appspot.com/97140043)
-crypto/tls: add support for ALPN (RFC 7301) (CL 108710046)
-crypto/tls: support programmatic selection of server certificates (CL 107400043)
-encoding/asn1: optional elements with a default value will now only be omitted if they have that value (CL 86960045)
-fmt: print type *map[T]T as &map[k:v] (CL 154870043)
-encoding/csv: do not quote empty strings, quote \. (CL 164760043)
-net/http: add Request.BasicAuth method (CL 76540043)
-net/http: add Transport.DialTLS hook (CL 137940043)
-net/http/httputil: add ReverseProxy.ErrorLog (CL 132750043)
-os: implement symlink support for windows (CL 86160044)
-reflect: add type.Comparable (CL 144020043)
-reflect: Value is one word smaller
-runtime: implement monotonic clocks on windows (CL 108700045)
-runtime: MemStats.Mallocs now counts very small allocations missed in Go 1.3. This may break tests using runtime.ReadMemStats or testing.AllocsPerRun by giving a more accurate answer than Go 1.3 did (CL 143150043).
-runtime/race: freebsd is supported (CL 107270043)
-runtime: add PauseEnd array to MemStats and GCStats (CL 153670043)
-swig: Due to runtime changes Go 1.4 will require SWIG 3.0.3 (not yet released)
-sync/atomic: add Value (CL 136710045)
-syscall: Setuid, Setgid are disabled on linux platforms. On linux those syscalls operate on the calling thread, not the whole process. This does not match the semantics of other platforms, nor the expectations of the caller, so the operations have been disabled until issue 1435 is resolved (CL 106170043)
-testing: add Coverage (CL 98150043)
-testing: add TestMain support (CL 148770043)
-text/scanner: add IsIdentRune field of Scanner. (CL 108030044)
-text/template: allow comparison of signed and unsigned integers (CL 149780043)
-time: use the micro symbol (µ (U+00B5)) to print microsecond duration (CL 105030046)
-
From 8db71d4ee89a505c375b550eb8fb8cc33bbabc03 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Wed, 29 Oct 2014 15:14:04 -0400 Subject: [PATCH 02/25] runtime: update comment for Callers Attempt to clear up confusion about how to turn the PCs reported by Callers into the file and line number people actually want. Fixes #7690. LGTM=r, chris.cs.guy R=r, chris.cs.guy CC=golang-codereviews https://golang.org/cl/163550043 --- src/runtime/extern.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/runtime/extern.go b/src/runtime/extern.go index 1b8052bb56..6cc5df810c 100644 --- a/src/runtime/extern.go +++ b/src/runtime/extern.go @@ -117,11 +117,20 @@ func Caller(skip int) (pc uintptr, file string, line int, ok bool) { return } -// Callers fills the slice pc with the program counters of function invocations +// Callers fills the slice pc with the return program counters of function invocations // on the calling goroutine's stack. The argument skip is the number of stack frames // to skip before recording in pc, with 0 identifying the frame for Callers itself and // 1 identifying the caller of Callers. // It returns the number of entries written to pc. +// +// Note that since each slice entry pc[i] is a return program counter, +// looking up the file and line for pc[i] (for example, using (*Func).FileLine) +// will return the file and line number of the instruction immediately +// following the call. +// To look up the file and line number of the call itself, use pc[i]-1. +// As an exception to this rule, if pc[i-1] corresponds to the function +// runtime.sigpanic, then pc[i] is the program counter of a faulting +// instruction and should be used without any subtraction. func Callers(skip int, pc []uintptr) int { // runtime.callers uses pc.array==nil as a signal // to print a stack trace. Pick off 0-length pc here From a22c11b9957cf3f0d66dd6d1d38172d5ac0ec54a Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Wed, 29 Oct 2014 15:14:24 -0400 Subject: [PATCH 03/25] runtime: fix line number in first stack frame in printed stack trace Originally traceback was only used for printing the stack when an unexpected signal came in. In that case, the initial PC is taken from the signal and should be used unaltered. For the callers, the PC is the return address, which might be on the line after the call; we subtract 1 to get to the CALL instruction. Traceback is now used for a variety of things, and for almost all of those the initial PC is a return address, whether from getcallerpc, or gp->sched.pc, or gp->syscallpc. In those cases, we need to subtract 1 from this initial PC, but the traceback code had a hard rule "never subtract 1 from the initial PC", left over from the signal handling days. Change gentraceback to take a flag that specifies whether we are tracing a trap. Change traceback to default to "starting with a return PC", which is the overwhelmingly common case. Add tracebacktrap, like traceback but starting with a trap PC. Use tracebacktrap in signal handlers. Fixes #7690. LGTM=iant, r R=r, iant CC=golang-codereviews https://golang.org/cl/167810044 --- src/runtime/heapdump.c | 2 +- src/runtime/mgc0.c | 4 +-- src/runtime/mprof.go | 2 +- src/runtime/os_plan9_386.c | 2 +- src/runtime/os_plan9_amd64.c | 2 +- src/runtime/os_windows_386.c | 2 +- src/runtime/os_windows_amd64.c | 2 +- src/runtime/proc.c | 6 ++--- src/runtime/runtime.h | 8 +++++- src/runtime/signal_386.c | 2 +- src/runtime/signal_amd64x.c | 2 +- src/runtime/signal_arm.c | 2 +- src/runtime/stack.c | 2 +- src/runtime/traceback.go | 33 ++++++++++++++++------- test/fixedbugs/issue7690.go | 49 ++++++++++++++++++++++++++++++++++ 15 files changed, 95 insertions(+), 25 deletions(-) create mode 100644 test/fixedbugs/issue7690.go diff --git a/src/runtime/heapdump.c b/src/runtime/heapdump.c index 71da419f15..94a4bd2be5 100644 --- a/src/runtime/heapdump.c +++ b/src/runtime/heapdump.c @@ -413,7 +413,7 @@ dumpgoroutine(G *gp) child.sp = nil; child.depth = 0; fn = dumpframe; - runtime·gentraceback(pc, sp, lr, gp, 0, nil, 0x7fffffff, &fn, &child, false); + runtime·gentraceback(pc, sp, lr, gp, 0, nil, 0x7fffffff, &fn, &child, 0); // dump defer & panic records for(d = gp->defer; d != nil; d = d->link) { diff --git a/src/runtime/mgc0.c b/src/runtime/mgc0.c index 1b41bf9a79..7754bad89d 100644 --- a/src/runtime/mgc0.c +++ b/src/runtime/mgc0.c @@ -774,7 +774,7 @@ scanstack(G *gp) runtime·throw("can't scan gchelper stack"); fn = scanframe; - runtime·gentraceback(~(uintptr)0, ~(uintptr)0, 0, gp, 0, nil, 0x7fffffff, &fn, nil, false); + runtime·gentraceback(~(uintptr)0, ~(uintptr)0, 0, gp, 0, nil, 0x7fffffff, &fn, nil, 0); runtime·tracebackdefers(gp, &fn, nil); } @@ -1964,7 +1964,7 @@ runtime·getgcmask(byte *p, Type *t, byte **mask, uintptr *len) frame.fn = nil; frame.sp = (uintptr)p; cb = getgcmaskcb; - runtime·gentraceback(g->m->curg->sched.pc, g->m->curg->sched.sp, 0, g->m->curg, 0, nil, 1000, &cb, &frame, false); + runtime·gentraceback(g->m->curg->sched.pc, g->m->curg->sched.sp, 0, g->m->curg, 0, nil, 1000, &cb, &frame, 0); if(frame.fn != nil) { Func *f; StackMap *stackmap; diff --git a/src/runtime/mprof.go b/src/runtime/mprof.go index 803da56670..d64e3be695 100644 --- a/src/runtime/mprof.go +++ b/src/runtime/mprof.go @@ -562,7 +562,7 @@ func GoroutineProfile(p []StackRecord) (n int, ok bool) { } func saveg(pc, sp uintptr, gp *g, r *StackRecord) { - n := gentraceback(pc, sp, 0, gp, 0, &r.Stack0[0], len(r.Stack0), nil, nil, false) + n := gentraceback(pc, sp, 0, gp, 0, &r.Stack0[0], len(r.Stack0), nil, nil, 0) if n < len(r.Stack0) { r.Stack0[n] = 0 } diff --git a/src/runtime/os_plan9_386.c b/src/runtime/os_plan9_386.c index 3490862244..42c6d161c7 100644 --- a/src/runtime/os_plan9_386.c +++ b/src/runtime/os_plan9_386.c @@ -114,7 +114,7 @@ Throw: if(runtime·gotraceback(&crash)) { runtime·goroutineheader(gp); - runtime·traceback(ureg->pc, ureg->sp, 0, gp); + runtime·tracebacktrap(ureg->pc, ureg->sp, 0, gp); runtime·tracebackothers(gp); runtime·printf("\n"); runtime·dumpregs(ureg); diff --git a/src/runtime/os_plan9_amd64.c b/src/runtime/os_plan9_amd64.c index 6b0f8ae3a2..a9dc0eb966 100644 --- a/src/runtime/os_plan9_amd64.c +++ b/src/runtime/os_plan9_amd64.c @@ -122,7 +122,7 @@ Throw: if(runtime·gotraceback(&crash)) { runtime·goroutineheader(gp); - runtime·traceback(ureg->ip, ureg->sp, 0, gp); + runtime·tracebacktrap(ureg->ip, ureg->sp, 0, gp); runtime·tracebackothers(gp); runtime·printf("\n"); runtime·dumpregs(ureg); diff --git a/src/runtime/os_windows_386.c b/src/runtime/os_windows_386.c index 213582799b..9962f0dc2e 100644 --- a/src/runtime/os_windows_386.c +++ b/src/runtime/os_windows_386.c @@ -97,7 +97,7 @@ runtime·lastcontinuehandler(ExceptionRecord *info, Context *r, G *gp) runtime·printf("\n"); if(runtime·gotraceback(&crash)){ - runtime·traceback(r->Eip, r->Esp, 0, gp); + runtime·tracebacktrap(r->Eip, r->Esp, 0, gp); runtime·tracebackothers(gp); runtime·dumpregs(r); } diff --git a/src/runtime/os_windows_amd64.c b/src/runtime/os_windows_amd64.c index b96cf70d1e..e4617e4cef 100644 --- a/src/runtime/os_windows_amd64.c +++ b/src/runtime/os_windows_amd64.c @@ -119,7 +119,7 @@ runtime·lastcontinuehandler(ExceptionRecord *info, Context *r, G *gp) runtime·printf("\n"); if(runtime·gotraceback(&crash)){ - runtime·traceback(r->Rip, r->Rsp, 0, gp); + runtime·tracebacktrap(r->Rip, r->Rsp, 0, gp); runtime·tracebackothers(gp); runtime·dumpregs(r); } diff --git a/src/runtime/proc.c b/src/runtime/proc.c index 52f7ef3a5b..b46f67065a 100644 --- a/src/runtime/proc.c +++ b/src/runtime/proc.c @@ -2532,7 +2532,7 @@ runtime·sigprof(uint8 *pc, uint8 *sp, uint8 *lr, G *gp, M *mp) n = 0; if(traceback) - n = runtime·gentraceback((uintptr)pc, (uintptr)sp, (uintptr)lr, gp, 0, stk, nelem(stk), nil, nil, false); + n = runtime·gentraceback((uintptr)pc, (uintptr)sp, (uintptr)lr, gp, 0, stk, nelem(stk), nil, nil, TraceTrap); if(!traceback || n <= 0) { // Normal traceback is impossible or has failed. // See if it falls into several common cases. @@ -2542,13 +2542,13 @@ runtime·sigprof(uint8 *pc, uint8 *sp, uint8 *lr, G *gp, M *mp) // Cgo, we can't unwind and symbolize arbitrary C code, // so instead collect Go stack that leads to the cgo call. // This is especially important on windows, since all syscalls are cgo calls. - n = runtime·gentraceback(mp->curg->syscallpc, mp->curg->syscallsp, 0, mp->curg, 0, stk, nelem(stk), nil, nil, false); + n = runtime·gentraceback(mp->curg->syscallpc, mp->curg->syscallsp, 0, mp->curg, 0, stk, nelem(stk), nil, nil, 0); } #ifdef GOOS_windows if(n == 0 && mp->libcallg != nil && mp->libcallpc != 0 && mp->libcallsp != 0) { // Libcall, i.e. runtime syscall on windows. // Collect Go stack that leads to the call. - n = runtime·gentraceback(mp->libcallpc, mp->libcallsp, 0, mp->libcallg, 0, stk, nelem(stk), nil, nil, false); + n = runtime·gentraceback(mp->libcallpc, mp->libcallsp, 0, mp->libcallg, 0, stk, nelem(stk), nil, nil, 0); } #endif if(n == 0) { diff --git a/src/runtime/runtime.h b/src/runtime/runtime.h index 2a60740063..977c4547df 100644 --- a/src/runtime/runtime.h +++ b/src/runtime/runtime.h @@ -719,9 +719,15 @@ struct Stkframe BitVector* argmap; // force use of this argmap }; -intgo runtime·gentraceback(uintptr, uintptr, uintptr, G*, intgo, uintptr*, intgo, bool(**)(Stkframe*, void*), void*, bool); +enum +{ + TraceRuntimeFrames = 1<<0, // include frames for internal runtime functions. + TraceTrap = 1<<1, // the initial PC, SP are from a trap, not a return PC from a call +}; +intgo runtime·gentraceback(uintptr, uintptr, uintptr, G*, intgo, uintptr*, intgo, bool(**)(Stkframe*, void*), void*, uintgo); void runtime·tracebackdefers(G*, bool(**)(Stkframe*, void*), void*); void runtime·traceback(uintptr pc, uintptr sp, uintptr lr, G* gp); +void runtime·tracebacktrap(uintptr pc, uintptr sp, uintptr lr, G* gp); void runtime·tracebackothers(G*); bool runtime·haszeroargs(uintptr pc); bool runtime·topofstack(Func*); diff --git a/src/runtime/signal_386.c b/src/runtime/signal_386.c index d55e304528..30a7488bd7 100644 --- a/src/runtime/signal_386.c +++ b/src/runtime/signal_386.c @@ -109,7 +109,7 @@ runtime·sighandler(int32 sig, Siginfo *info, void *ctxt, G *gp) if(runtime·gotraceback(&crash)){ runtime·goroutineheader(gp); - runtime·traceback(SIG_EIP(info, ctxt), SIG_ESP(info, ctxt), 0, gp); + runtime·tracebacktrap(SIG_EIP(info, ctxt), SIG_ESP(info, ctxt), 0, gp); runtime·tracebackothers(gp); runtime·printf("\n"); runtime·dumpregs(info, ctxt); diff --git a/src/runtime/signal_amd64x.c b/src/runtime/signal_amd64x.c index 44e68cecfc..feb4afcce3 100644 --- a/src/runtime/signal_amd64x.c +++ b/src/runtime/signal_amd64x.c @@ -143,7 +143,7 @@ runtime·sighandler(int32 sig, Siginfo *info, void *ctxt, G *gp) if(runtime·gotraceback(&crash)){ runtime·goroutineheader(gp); - runtime·traceback(SIG_RIP(info, ctxt), SIG_RSP(info, ctxt), 0, gp); + runtime·tracebacktrap(SIG_RIP(info, ctxt), SIG_RSP(info, ctxt), 0, gp); runtime·tracebackothers(gp); runtime·printf("\n"); runtime·dumpregs(info, ctxt); diff --git a/src/runtime/signal_arm.c b/src/runtime/signal_arm.c index 3571cf3ac6..afad5e7d16 100644 --- a/src/runtime/signal_arm.c +++ b/src/runtime/signal_arm.c @@ -108,7 +108,7 @@ runtime·sighandler(int32 sig, Siginfo *info, void *ctxt, G *gp) if(runtime·gotraceback(&crash)){ runtime·goroutineheader(gp); - runtime·traceback(SIG_PC(info, ctxt), SIG_SP(info, ctxt), SIG_LR(info, ctxt), gp); + runtime·tracebacktrap(SIG_PC(info, ctxt), SIG_SP(info, ctxt), SIG_LR(info, ctxt), gp); runtime·tracebackothers(gp); runtime·printf("\n"); runtime·dumpregs(info, ctxt); diff --git a/src/runtime/stack.c b/src/runtime/stack.c index ed8f4f8727..072bc242bc 100644 --- a/src/runtime/stack.c +++ b/src/runtime/stack.c @@ -620,7 +620,7 @@ copystack(G *gp, uintptr newsize) adjinfo.old = old; adjinfo.delta = new.hi - old.hi; cb = adjustframe; - runtime·gentraceback(~(uintptr)0, ~(uintptr)0, 0, gp, 0, nil, 0x7fffffff, &cb, &adjinfo, false); + runtime·gentraceback(~(uintptr)0, ~(uintptr)0, 0, gp, 0, nil, 0x7fffffff, &cb, &adjinfo, 0); // adjust other miscellaneous things that have pointers into stacks. adjustctxt(gp, &adjinfo); diff --git a/src/runtime/traceback.go b/src/runtime/traceback.go index 24dc3eea95..834435b400 100644 --- a/src/runtime/traceback.go +++ b/src/runtime/traceback.go @@ -96,7 +96,7 @@ func tracebackdefers(gp *g, callback func(*stkframe, unsafe.Pointer) bool, v uns // the runtime.Callers function (pcbuf != nil), as well as the garbage // collector (callback != nil). A little clunky to merge these, but avoids // duplicating the code and all its subtlety. -func gentraceback(pc0 uintptr, sp0 uintptr, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max int, callback func(*stkframe, unsafe.Pointer) bool, v unsafe.Pointer, printall bool) int { +func gentraceback(pc0 uintptr, sp0 uintptr, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max int, callback func(*stkframe, unsafe.Pointer) bool, v unsafe.Pointer, flags uint) int { if goexitPC == 0 { gothrow("gentraceback before goexitPC initialization") } @@ -297,13 +297,13 @@ func gentraceback(pc0 uintptr, sp0 uintptr, lr0 uintptr, gp *g, skip int, pcbuf } } if printing { - if printall || showframe(f, gp) { + if (flags&_TraceRuntimeFrames) != 0 || showframe(f, gp) { // Print during crash. // main(0x1, 0x2, 0x3) // /home/rsc/go/src/runtime/x.go:23 +0xf // tracepc := frame.pc // back up to CALL instruction for funcline. - if n > 0 && frame.pc > f.entry && !waspanic { + if (n > 0 || flags&_TraceTrap == 0) && frame.pc > f.entry && !waspanic { tracepc-- } print(gofuncname(f), "(") @@ -475,17 +475,32 @@ func printcreatedby(gp *g) { } func traceback(pc uintptr, sp uintptr, lr uintptr, gp *g) { + traceback1(pc, sp, lr, gp, 0) +} + +// tracebacktrap is like traceback but expects that the PC and SP were obtained +// from a trap, not from gp->sched or gp->syscallpc/gp->syscallsp or getcallerpc/getcallersp. +// Because they are from a trap instead of from a saved pair, +// the initial PC must not be rewound to the previous instruction. +// (All the saved pairs record a PC that is a return address, so we +// rewind it into the CALL instruction.) +func tracebacktrap(pc uintptr, sp uintptr, lr uintptr, gp *g) { + traceback1(pc, sp, lr, gp, _TraceTrap) +} + +func traceback1(pc uintptr, sp uintptr, lr uintptr, gp *g, flags uint) { var n int if readgstatus(gp)&^_Gscan == _Gsyscall { - // Override signal registers if blocked in system call. + // Override registers if blocked in system call. pc = gp.syscallpc sp = gp.syscallsp + flags &^= _TraceTrap } // Print traceback. By default, omits runtime frames. // If that means we print nothing at all, repeat forcing all frames printed. - n = gentraceback(pc, sp, lr, gp, 0, nil, _TracebackMaxFrames, nil, nil, false) - if n == 0 { - n = gentraceback(pc, sp, lr, gp, 0, nil, _TracebackMaxFrames, nil, nil, true) + n = gentraceback(pc, sp, lr, gp, 0, nil, _TracebackMaxFrames, nil, nil, flags) + if n == 0 && (flags&_TraceRuntimeFrames) == 0 { + n = gentraceback(pc, sp, lr, gp, 0, nil, _TracebackMaxFrames, nil, nil, flags|_TraceRuntimeFrames) } if n == _TracebackMaxFrames { print("...additional frames elided...\n") @@ -496,11 +511,11 @@ func traceback(pc uintptr, sp uintptr, lr uintptr, gp *g) { func callers(skip int, pcbuf *uintptr, m int) int { sp := getcallersp(unsafe.Pointer(&skip)) pc := uintptr(getcallerpc(unsafe.Pointer(&skip))) - return gentraceback(pc, sp, 0, getg(), skip, pcbuf, m, nil, nil, false) + return gentraceback(pc, sp, 0, getg(), skip, pcbuf, m, nil, nil, 0) } func gcallers(gp *g, skip int, pcbuf *uintptr, m int) int { - return gentraceback(^uintptr(0), ^uintptr(0), 0, gp, skip, pcbuf, m, nil, nil, false) + return gentraceback(^uintptr(0), ^uintptr(0), 0, gp, skip, pcbuf, m, nil, nil, 0) } func showframe(f *_func, gp *g) bool { diff --git a/test/fixedbugs/issue7690.go b/test/fixedbugs/issue7690.go new file mode 100644 index 0000000000..4ad9e8622a --- /dev/null +++ b/test/fixedbugs/issue7690.go @@ -0,0 +1,49 @@ +// run + +// Copyright 2014 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. + +// issue 7690 - Stack and other routines did not back up initial PC +// into CALL instruction, instead reporting line number of next instruction, +// which might be on a different line. + +package main + +import ( + "bytes" + "regexp" + "runtime" + "strconv" +) + +func main() { + buf1 := make([]byte, 1000) + buf2 := make([]byte, 1000) + + runtime.Stack(buf1, false) // CALL is last instruction on this line + n := runtime.Stack(buf2, false) // CALL is followed by load of result from stack + + buf1 = buf1[:bytes.IndexByte(buf1, 0)] + buf2 = buf2[:n] + + re := regexp.MustCompile(`(?m)^main\.main\(\)\n.*/issue7690.go:([0-9]+)`) + m1 := re.FindStringSubmatch(string(buf1)) + if m1 == nil { + println("BUG: cannot find main.main in first trace") + return + } + m2 := re.FindStringSubmatch(string(buf2)) + if m2 == nil { + println("BUG: cannot find main.main in second trace") + return + } + + n1, _ := strconv.Atoi(m1[1]) + n2, _ := strconv.Atoi(m2[1]) + if n1+1 != n2 { + println("BUG: expect runtime.Stack on back to back lines, have", n1, n2) + println(string(buf1)) + println(string(buf2)) + } +} From 97b24a05dff7adef3a0fb463a575b705be985468 Mon Sep 17 00:00:00 2001 From: Rob Pike Date: Wed, 29 Oct 2014 13:07:34 -0700 Subject: [PATCH 04/25] doc/go1.4.html: gccgo status LGTM=iant, cmang R=cmang, iant, rsc CC=golang-codereviews https://golang.org/cl/169760043 --- doc/go1.4.html | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/doc/go1.4.html b/doc/go1.4.html index 9fa86c31ae..e2458f2efe 100644 --- a/doc/go1.4.html +++ b/doc/go1.4.html @@ -18,8 +18,7 @@ Stacks are now contiguous, reallocated when necessary rather than linking on new "segments"; this release therefore eliminates the notorious "hot stack split" problem. There are some new tools available including support in the go command -for build-time source code generation -and TODO. +for build-time source code generation. The release also adds support for ARM processors on Android and Native Client (NaCl) and AMD64 on Plan 9. As always, Go 1.4 keeps the promise @@ -281,7 +280,9 @@ More information about these changes is in the assembly docum

Status of gccgo

-TODO gccgo news +The release schedules for the GCC and Go projects do not coincide. +GCC release 4.9 contains the Go 1.2 version of gccgo. +The next release, GCC 5, will likely have the Go 1.4 version of gccgo.

Internal packages

@@ -593,13 +594,11 @@ of the Config struct.
  • -Also in the crypto/tls package, -the server now supports +Also in the crypto/tls package, the server now supports TLS_FALLBACK_SCSV -to help clients detect fallback attacks like -POODLE. -(The crypto/tls package's client has never supported SSLv3, so it is not -vulnerable to the POODLE attack.) +to help clients detect fallback attacks. +(The Go client does not support fallback at all, so it is not vulnerable to +those attacks.)
  • From 3eadbb02afa1494821de000ee280e00c3c398f1d Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Wed, 29 Oct 2014 18:07:24 -0400 Subject: [PATCH 05/25] cmd/objdump: use cmd/internal/objfile This removes a bunch of ugly duplicate code. The end goal is to factor the disassembly code into cmd/internal/objfile too, so that pprof can use it, but one step at a time. LGTM=r, iant R=r, alex.brainman, iant CC=golang-codereviews https://golang.org/cl/149400043 --- src/cmd/internal/objfile/elf.go | 25 +++ src/cmd/internal/objfile/goobj.go | 12 ++ src/cmd/internal/objfile/macho.go | 24 +++ src/cmd/internal/objfile/objfile.go | 10 + src/cmd/internal/objfile/pe.go | 31 +++ src/cmd/internal/objfile/plan9obj.go | 22 +++ src/cmd/objdump/Makefile | 10 - src/cmd/objdump/elf.go | 65 ------- src/cmd/objdump/macho.go | 77 -------- src/cmd/objdump/main.go | 277 +++------------------------ src/cmd/objdump/pe.go | 99 ---------- src/cmd/objdump/plan9obj.go | 70 ------- 12 files changed, 154 insertions(+), 568 deletions(-) delete mode 100644 src/cmd/objdump/Makefile delete mode 100644 src/cmd/objdump/elf.go delete mode 100644 src/cmd/objdump/macho.go delete mode 100644 src/cmd/objdump/pe.go delete mode 100644 src/cmd/objdump/plan9obj.go diff --git a/src/cmd/internal/objfile/elf.go b/src/cmd/internal/objfile/elf.go index 8495fa7532..17755b84d2 100644 --- a/src/cmd/internal/objfile/elf.go +++ b/src/cmd/internal/objfile/elf.go @@ -8,6 +8,7 @@ package objfile import ( "debug/elf" + "fmt" "os" ) @@ -77,3 +78,27 @@ func (f *elfFile) pcln() (textStart uint64, symtab, pclntab []byte, err error) { } return textStart, symtab, pclntab, nil } + +func (f *elfFile) text() (textStart uint64, text []byte, err error) { + sect := f.elf.Section(".text") + if sect == nil { + return 0, nil, fmt.Errorf("text section not found") + } + textStart = sect.Addr + text, err = sect.Data() + return +} + +func (f *elfFile) goarch() string { + switch f.elf.Machine { + case elf.EM_386: + return "386" + case elf.EM_X86_64: + return "amd64" + case elf.EM_ARM: + return "arm" + case elf.EM_PPC64: + return "power64" + } + return "" +} diff --git a/src/cmd/internal/objfile/goobj.go b/src/cmd/internal/objfile/goobj.go index a4f49ebe44..a1d773023d 100644 --- a/src/cmd/internal/objfile/goobj.go +++ b/src/cmd/internal/objfile/goobj.go @@ -79,3 +79,15 @@ func (f *goobjFile) symbols() ([]Sym, error) { func (f *goobjFile) pcln() (textStart uint64, symtab, pclntab []byte, err error) { return 0, nil, nil, fmt.Errorf("pcln not available in go object file") } + +// text does not make sense for Go object files, because +// each function has a separate section. +func (f *goobjFile) text() (textStart uint64, text []byte, err error) { + return 0, nil, fmt.Errorf("text not available in go object file") +} + +// goarch makes sense but is not exposed in debug/goobj's API, +// and we don't need it yet for any users of internal/objfile. +func (f *goobjFile) goarch() string { + return "GOARCH unimplemented for debug/goobj files" +} diff --git a/src/cmd/internal/objfile/macho.go b/src/cmd/internal/objfile/macho.go index f845792ffa..7dd84a339d 100644 --- a/src/cmd/internal/objfile/macho.go +++ b/src/cmd/internal/objfile/macho.go @@ -85,6 +85,30 @@ func (f *machoFile) pcln() (textStart uint64, symtab, pclntab []byte, err error) return textStart, symtab, pclntab, nil } +func (f *machoFile) text() (textStart uint64, text []byte, err error) { + sect := f.macho.Section("__text") + if sect == nil { + return 0, nil, fmt.Errorf("text section not found") + } + textStart = sect.Addr + text, err = sect.Data() + return +} + +func (f *machoFile) goarch() string { + switch f.macho.Cpu { + case macho.Cpu386: + return "386" + case macho.CpuAmd64: + return "amd64" + case macho.CpuArm: + return "arm" + case macho.CpuPpc64: + return "power64" + } + return "" +} + type uint64s []uint64 func (x uint64s) Len() int { return len(x) } diff --git a/src/cmd/internal/objfile/objfile.go b/src/cmd/internal/objfile/objfile.go index 09fa63e60b..3d4a5d27cd 100644 --- a/src/cmd/internal/objfile/objfile.go +++ b/src/cmd/internal/objfile/objfile.go @@ -14,6 +14,8 @@ import ( type rawFile interface { symbols() (syms []Sym, err error) pcln() (textStart uint64, symtab, pclntab []byte, err error) + text() (textStart uint64, text []byte, err error) + goarch() string } // A File is an opened executable file. @@ -70,3 +72,11 @@ func (f *File) PCLineTable() (*gosym.Table, error) { } return gosym.NewTable(symtab, gosym.NewLineTable(pclntab, textStart)) } + +func (f *File) Text() (uint64, []byte, error) { + return f.raw.text() +} + +func (f *File) GOARCH() string { + return f.raw.goarch() +} diff --git a/src/cmd/internal/objfile/pe.go b/src/cmd/internal/objfile/pe.go index 868709eaf9..67e59c226b 100644 --- a/src/cmd/internal/objfile/pe.go +++ b/src/cmd/internal/objfile/pe.go @@ -133,6 +133,25 @@ func (f *peFile) pcln() (textStart uint64, symtab, pclntab []byte, err error) { return textStart, symtab, pclntab, nil } +func (f *peFile) text() (textStart uint64, text []byte, err error) { + var imageBase uint64 + switch oh := f.pe.OptionalHeader.(type) { + case *pe.OptionalHeader32: + imageBase = uint64(oh.ImageBase) + case *pe.OptionalHeader64: + imageBase = oh.ImageBase + default: + return 0, nil, fmt.Errorf("pe file format not recognized") + } + sect := f.pe.Section(".text") + if sect == nil { + return 0, nil, fmt.Errorf("text section not found") + } + textStart = imageBase + uint64(sect.VirtualAddress) + text, err = sect.Data() + return +} + func findPESymbol(f *pe.File, name string) (*pe.Symbol, error) { for _, s := range f.Symbols { if s.Name != name { @@ -168,3 +187,15 @@ func loadPETable(f *pe.File, sname, ename string) ([]byte, error) { } return data[ssym.Value:esym.Value], nil } + +func (f *peFile) goarch() string { + // Not sure how to get the info we want from PE header. + // Look in symbol table for telltale rt0 symbol. + if _, err := findPESymbol(f.pe, "_rt0_386_windows"); err == nil { + return "386" + } + if _, err := findPESymbol(f.pe, "_rt0_amd64_windows"); err == nil { + return "amd64" + } + return "" +} diff --git a/src/cmd/internal/objfile/plan9obj.go b/src/cmd/internal/objfile/plan9obj.go index 80744f82a8..eb6cba5eb1 100644 --- a/src/cmd/internal/objfile/plan9obj.go +++ b/src/cmd/internal/objfile/plan9obj.go @@ -88,6 +88,16 @@ func (f *plan9File) pcln() (textStart uint64, symtab, pclntab []byte, err error) return textStart, symtab, pclntab, nil } +func (f *plan9File) text() (textStart uint64, text []byte, err error) { + sect := f.plan9.Section("text") + if sect == nil { + return 0, nil, fmt.Errorf("text section not found") + } + textStart = f.plan9.LoadAddress + f.plan9.HdrSize + text, err = sect.Data() + return +} + func findPlan9Symbol(f *plan9obj.File, name string) (*plan9obj.Sym, error) { syms, err := f.Symbols() if err != nil { @@ -122,3 +132,15 @@ func loadPlan9Table(f *plan9obj.File, sname, ename string) ([]byte, error) { textStart := f.LoadAddress + f.HdrSize return data[ssym.Value-textStart : esym.Value-textStart], nil } + +func (f *plan9File) goarch() string { + switch f.plan9.Magic { + case plan9obj.Magic386: + return "386" + case plan9obj.MagicAMD64: + return "amd64" + case plan9obj.MagicARM: + return "arm" + } + return "" +} diff --git a/src/cmd/objdump/Makefile b/src/cmd/objdump/Makefile deleted file mode 100644 index 1b66c26bab..0000000000 --- a/src/cmd/objdump/Makefile +++ /dev/null @@ -1,10 +0,0 @@ -all: x86.go armasm.go - -x86.go: bundle - ./bundle -p main -x x86_ rsc.io/x86/x86asm | gofmt >x86.go - -armasm.go: bundle - ./bundle -p main -x arm_ rsc.io/arm/armasm | gofmt >armasm.go - -bundle: - go build -o bundle code.google.com/p/rsc/cmd/bundle diff --git a/src/cmd/objdump/elf.go b/src/cmd/objdump/elf.go deleted file mode 100644 index 906e903532..0000000000 --- a/src/cmd/objdump/elf.go +++ /dev/null @@ -1,65 +0,0 @@ -// Copyright 2013 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. - -// Parsing of ELF executables (Linux, FreeBSD, and so on). - -package main - -import ( - "debug/elf" - "os" -) - -func elfSymbols(f *os.File) (syms []Sym, goarch string) { - p, err := elf.NewFile(f) - if err != nil { - errorf("parsing %s: %v", f.Name(), err) - return - } - - elfSyms, err := p.Symbols() - if err != nil { - errorf("parsing %s: %v", f.Name(), err) - return - } - - switch p.Machine { - case elf.EM_X86_64: - goarch = "amd64" - case elf.EM_386: - goarch = "386" - case elf.EM_ARM: - goarch = "arm" - } - - for _, s := range elfSyms { - sym := Sym{Addr: s.Value, Name: s.Name, Size: int64(s.Size), Code: '?'} - switch s.Section { - case elf.SHN_UNDEF: - sym.Code = 'U' - case elf.SHN_COMMON: - sym.Code = 'B' - default: - i := int(s.Section) - if i < 0 || i >= len(p.Sections) { - break - } - sect := p.Sections[i] - switch sect.Flags & (elf.SHF_WRITE | elf.SHF_ALLOC | elf.SHF_EXECINSTR) { - case elf.SHF_ALLOC | elf.SHF_EXECINSTR: - sym.Code = 'T' - case elf.SHF_ALLOC: - sym.Code = 'R' - case elf.SHF_ALLOC | elf.SHF_WRITE: - sym.Code = 'D' - } - } - if elf.ST_BIND(s.Info) == elf.STB_LOCAL { - sym.Code += 'a' - 'A' - } - syms = append(syms, sym) - } - - return -} diff --git a/src/cmd/objdump/macho.go b/src/cmd/objdump/macho.go deleted file mode 100644 index 6e0ad223d4..0000000000 --- a/src/cmd/objdump/macho.go +++ /dev/null @@ -1,77 +0,0 @@ -// Copyright 2013 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. - -// Parsing of Mach-O executables (OS X). - -package main - -import ( - "debug/macho" - "os" - "sort" -) - -func machoSymbols(f *os.File) (syms []Sym, goarch string) { - p, err := macho.NewFile(f) - if err != nil { - errorf("parsing %s: %v", f.Name(), err) - return - } - - if p.Symtab == nil { - errorf("%s: no symbol table", f.Name()) - return - } - - switch p.Cpu { - case macho.Cpu386: - goarch = "386" - case macho.CpuAmd64: - goarch = "amd64" - case macho.CpuArm: - goarch = "arm" - } - - // Build sorted list of addresses of all symbols. - // We infer the size of a symbol by looking at where the next symbol begins. - var addrs []uint64 - for _, s := range p.Symtab.Syms { - addrs = append(addrs, s.Value) - } - sort.Sort(uint64s(addrs)) - - for _, s := range p.Symtab.Syms { - sym := Sym{Name: s.Name, Addr: s.Value, Code: '?'} - i := sort.Search(len(addrs), func(x int) bool { return addrs[x] > s.Value }) - if i < len(addrs) { - sym.Size = int64(addrs[i] - s.Value) - } - if s.Sect == 0 { - sym.Code = 'U' - } else if int(s.Sect) <= len(p.Sections) { - sect := p.Sections[s.Sect-1] - switch sect.Seg { - case "__TEXT": - sym.Code = 'R' - case "__DATA": - sym.Code = 'D' - } - switch sect.Seg + " " + sect.Name { - case "__TEXT __text": - sym.Code = 'T' - case "__DATA __bss", "__DATA __noptrbss": - sym.Code = 'B' - } - } - syms = append(syms, sym) - } - - return -} - -type uint64s []uint64 - -func (x uint64s) Len() int { return len(x) } -func (x uint64s) Swap(i, j int) { x[i], x[j] = x[j], x[i] } -func (x uint64s) Less(i, j int) bool { return x[i] < x[j] } diff --git a/src/cmd/objdump/main.go b/src/cmd/objdump/main.go index 0f66f20a40..0f125c98bf 100644 --- a/src/cmd/objdump/main.go +++ b/src/cmd/objdump/main.go @@ -33,12 +33,7 @@ package main import ( "bufio" - "bytes" - "debug/elf" "debug/gosym" - "debug/macho" - "debug/pe" - "debug/plan9obj" "encoding/binary" "flag" "fmt" @@ -51,6 +46,8 @@ import ( "strings" "text/tabwriter" + "cmd/internal/objfile" + "cmd/internal/rsc.io/arm/armasm" "cmd/internal/rsc.io/x86/x86asm" ) @@ -85,21 +82,33 @@ func main() { symRE = re } - f, err := os.Open(flag.Arg(0)) + f, err := objfile.Open(flag.Arg(0)) if err != nil { log.Fatal(err) } - textStart, textData, symtab, pclntab, err := loadTables(f) + syms, err := f.Symbols() if err != nil { log.Fatalf("reading %s: %v", flag.Arg(0), err) } - syms, goarch, err := loadSymbols(f) + tab, err := f.PCLineTable() if err != nil { log.Fatalf("reading %s: %v", flag.Arg(0), err) } + textStart, textBytes, err := f.Text() + if err != nil { + log.Fatalf("reading %s: %v", flag.Arg(0), err) + } + + goarch := f.GOARCH() + + disasm := disasms[goarch] + if disasm == nil { + log.Fatalf("reading %s: unknown architecture", flag.Arg(0)) + } + // Filter out section symbols, overwriting syms in place. keep := syms[:0] for _, sym := range syms { @@ -112,37 +121,27 @@ func main() { } syms = keep - disasm := disasms[goarch] - if disasm == nil { - log.Fatalf("reading %s: unknown architecture", flag.Arg(0)) - } - + sort.Sort(ByAddr(syms)) lookup := func(addr uint64) (string, uint64) { - i := sort.Search(len(syms), func(i int) bool { return syms[i].Addr > addr }) + i := sort.Search(len(syms), func(i int) bool { return addr < syms[i].Addr }) if i > 0 { s := syms[i-1] - if s.Addr <= addr && addr < s.Addr+uint64(s.Size) && s.Name != "runtime.etext" && s.Name != "etext" && s.Name != "_etext" { + if s.Addr != 0 && s.Addr <= addr && addr < s.Addr+uint64(s.Size) { return s.Name, s.Addr } } return "", 0 } - pcln := gosym.NewLineTable(pclntab, textStart) - tab, err := gosym.NewTable(symtab, pcln) - if err != nil { - log.Fatalf("reading %s: %v", flag.Arg(0), err) - } - if flag.NArg() == 1 { // disassembly of entire object - our format - dump(tab, lookup, disasm, goarch, syms, textData, textStart) - os.Exit(exitCode) + dump(tab, lookup, disasm, goarch, syms, textBytes, textStart) + os.Exit(0) } // disassembly of specific piece of object - gnu objdump format for pprof - gnuDump(tab, lookup, disasm, textData, textStart) - os.Exit(exitCode) + gnuDump(tab, lookup, disasm, textBytes, textStart) + os.Exit(0) } // base returns the final element in the path. @@ -153,13 +152,13 @@ func base(path string) string { return path } -func dump(tab *gosym.Table, lookup lookupFunc, disasm disasmFunc, goarch string, syms []Sym, textData []byte, textStart uint64) { +func dump(tab *gosym.Table, lookup lookupFunc, disasm disasmFunc, goarch string, syms []objfile.Sym, textData []byte, textStart uint64) { stdout := bufio.NewWriter(os.Stdout) defer stdout.Flush() printed := false for _, sym := range syms { - if (sym.Code != 'T' && sym.Code != 't') || sym.Size == 0 || sym.Name == "_text" || sym.Name == "text" || sym.Addr < textStart || symRE != nil && !symRE.MatchString(sym.Name) { + if (sym.Code != 'T' && sym.Code != 't') || sym.Size == 0 || sym.Addr < textStart || symRE != nil && !symRE.MatchString(sym.Name) { continue } if sym.Addr >= textStart+uint64(len(textData)) || sym.Addr+uint64(sym.Size) > textStart+uint64(len(textData)) { @@ -307,224 +306,8 @@ func gnuDump(tab *gosym.Table, lookup lookupFunc, disasm disasmFunc, textData [] flush(end) } -func loadTables(f *os.File) (textStart uint64, textData, symtab, pclntab []byte, err error) { - if obj, err := elf.NewFile(f); err == nil { - if sect := obj.Section(".text"); sect != nil { - textStart = sect.Addr - textData, _ = sect.Data() - } - if sect := obj.Section(".gosymtab"); sect != nil { - if symtab, err = sect.Data(); err != nil { - return 0, nil, nil, nil, err - } - } - if sect := obj.Section(".gopclntab"); sect != nil { - if pclntab, err = sect.Data(); err != nil { - return 0, nil, nil, nil, err - } - } - return textStart, textData, symtab, pclntab, nil - } +type ByAddr []objfile.Sym - if obj, err := macho.NewFile(f); err == nil { - if sect := obj.Section("__text"); sect != nil { - textStart = sect.Addr - textData, _ = sect.Data() - } - if sect := obj.Section("__gosymtab"); sect != nil { - if symtab, err = sect.Data(); err != nil { - return 0, nil, nil, nil, err - } - } - if sect := obj.Section("__gopclntab"); sect != nil { - if pclntab, err = sect.Data(); err != nil { - return 0, nil, nil, nil, err - } - } - return textStart, textData, symtab, pclntab, nil - } - - if obj, err := pe.NewFile(f); err == nil { - var imageBase uint64 - switch oh := obj.OptionalHeader.(type) { - case *pe.OptionalHeader32: - imageBase = uint64(oh.ImageBase) - case *pe.OptionalHeader64: - imageBase = oh.ImageBase - default: - return 0, nil, nil, nil, fmt.Errorf("pe file format not recognized") - } - if sect := obj.Section(".text"); sect != nil { - textStart = imageBase + uint64(sect.VirtualAddress) - textData, _ = sect.Data() - } - if pclntab, err = loadPETable(obj, "runtime.pclntab", "runtime.epclntab"); err != nil { - // We didn't find the symbols, so look for the names used in 1.3 and earlier. - // TODO: Remove code looking for the old symbols when we no longer care about 1.3. - var err2 error - if pclntab, err2 = loadPETable(obj, "pclntab", "epclntab"); err2 != nil { - return 0, nil, nil, nil, err - } - } - if symtab, err = loadPETable(obj, "runtime.symtab", "runtime.esymtab"); err != nil { - // Same as above. - var err2 error - if symtab, err2 = loadPETable(obj, "symtab", "esymtab"); err2 != nil { - return 0, nil, nil, nil, err - } - } - return textStart, textData, symtab, pclntab, nil - } - - if obj, err := plan9obj.NewFile(f); err == nil { - textStart = obj.LoadAddress + obj.HdrSize - if sect := obj.Section("text"); sect != nil { - textData, _ = sect.Data() - } - if pclntab, err = loadPlan9Table(obj, "runtime.pclntab", "runtime.epclntab"); err != nil { - // We didn't find the symbols, so look for the names used in 1.3 and earlier. - // TODO: Remove code looking for the old symbols when we no longer care about 1.3. - var err2 error - if pclntab, err2 = loadPlan9Table(obj, "pclntab", "epclntab"); err2 != nil { - return 0, nil, nil, nil, err - } - } - if symtab, err = loadPlan9Table(obj, "runtime.symtab", "runtime.esymtab"); err != nil { - // Same as above. - var err2 error - if symtab, err2 = loadPlan9Table(obj, "symtab", "esymtab"); err2 != nil { - return 0, nil, nil, nil, err - } - } - return textStart, textData, symtab, pclntab, nil - } - - return 0, nil, nil, nil, fmt.Errorf("unrecognized binary format") -} - -func findPESymbol(f *pe.File, name string) (*pe.Symbol, error) { - for _, s := range f.Symbols { - if s.Name != name { - continue - } - if s.SectionNumber <= 0 { - return nil, fmt.Errorf("symbol %s: invalid section number %d", name, s.SectionNumber) - } - if len(f.Sections) < int(s.SectionNumber) { - return nil, fmt.Errorf("symbol %s: section number %d is larger than max %d", name, s.SectionNumber, len(f.Sections)) - } - return s, nil - } - return nil, fmt.Errorf("no %s symbol found", name) -} - -func loadPETable(f *pe.File, sname, ename string) ([]byte, error) { - ssym, err := findPESymbol(f, sname) - if err != nil { - return nil, err - } - esym, err := findPESymbol(f, ename) - if err != nil { - return nil, err - } - if ssym.SectionNumber != esym.SectionNumber { - return nil, fmt.Errorf("%s and %s symbols must be in the same section", sname, ename) - } - sect := f.Sections[ssym.SectionNumber-1] - data, err := sect.Data() - if err != nil { - return nil, err - } - return data[ssym.Value:esym.Value], nil -} - -func findPlan9Symbol(f *plan9obj.File, name string) (*plan9obj.Sym, error) { - syms, err := f.Symbols() - if err != nil { - return nil, err - } - for _, s := range syms { - if s.Name != name { - continue - } - return &s, nil - } - return nil, fmt.Errorf("no %s symbol found", name) -} - -func loadPlan9Table(f *plan9obj.File, sname, ename string) ([]byte, error) { - ssym, err := findPlan9Symbol(f, sname) - if err != nil { - return nil, err - } - esym, err := findPlan9Symbol(f, ename) - if err != nil { - return nil, err - } - sect := f.Section("text") - if sect == nil { - return nil, err - } - data, err := sect.Data() - if err != nil { - return nil, err - } - textStart := f.LoadAddress + f.HdrSize - return data[ssym.Value-textStart : esym.Value-textStart], nil -} - -// TODO(rsc): This code is taken from cmd/nm. Arrange some way to share the code. - -var exitCode = 0 - -func errorf(format string, args ...interface{}) { - log.Printf(format, args...) - exitCode = 1 -} - -func loadSymbols(f *os.File) (syms []Sym, goarch string, err error) { - f.Seek(0, 0) - buf := make([]byte, 16) - io.ReadFull(f, buf) - f.Seek(0, 0) - - for _, p := range parsers { - if bytes.HasPrefix(buf, p.prefix) { - syms, goarch = p.parse(f) - sort.Sort(byAddr(syms)) - return - } - } - err = fmt.Errorf("unknown file format") - return -} - -type Sym struct { - Addr uint64 - Size int64 - Code rune - Name string - Type string -} - -var parsers = []struct { - prefix []byte - parse func(*os.File) ([]Sym, string) -}{ - {[]byte("\x7FELF"), elfSymbols}, - {[]byte("\xFE\xED\xFA\xCE"), machoSymbols}, - {[]byte("\xFE\xED\xFA\xCF"), machoSymbols}, - {[]byte("\xCE\xFA\xED\xFE"), machoSymbols}, - {[]byte("\xCF\xFA\xED\xFE"), machoSymbols}, - {[]byte("MZ"), peSymbols}, - {[]byte("\x00\x00\x01\xEB"), plan9Symbols}, // 386 - {[]byte("\x00\x00\x04\x07"), plan9Symbols}, // mips - {[]byte("\x00\x00\x06\x47"), plan9Symbols}, // arm - {[]byte("\x00\x00\x8A\x97"), plan9Symbols}, // amd64 -} - -type byAddr []Sym - -func (x byAddr) Len() int { return len(x) } -func (x byAddr) Swap(i, j int) { x[i], x[j] = x[j], x[i] } -func (x byAddr) Less(i, j int) bool { return x[i].Addr < x[j].Addr } +func (x ByAddr) Less(i, j int) bool { return x[i].Addr < x[j].Addr } +func (x ByAddr) Len() int { return len(x) } +func (x ByAddr) Swap(i, j int) { x[i], x[j] = x[j], x[i] } diff --git a/src/cmd/objdump/pe.go b/src/cmd/objdump/pe.go deleted file mode 100644 index 38190095a3..0000000000 --- a/src/cmd/objdump/pe.go +++ /dev/null @@ -1,99 +0,0 @@ -// Copyright 2013 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. - -// Parsing of PE executables (Microsoft Windows). - -package main - -import ( - "debug/pe" - "os" - "sort" -) - -func peSymbols(f *os.File) (syms []Sym, goarch string) { - p, err := pe.NewFile(f) - if err != nil { - errorf("parsing %s: %v", f.Name(), err) - return - } - - // Build sorted list of addresses of all symbols. - // We infer the size of a symbol by looking at where the next symbol begins. - var addrs []uint64 - - var imageBase uint64 - switch oh := p.OptionalHeader.(type) { - case *pe.OptionalHeader32: - imageBase = uint64(oh.ImageBase) - goarch = "386" - case *pe.OptionalHeader64: - imageBase = oh.ImageBase - goarch = "amd64" - default: - errorf("parsing %s: file format not recognized", f.Name()) - return - } - - for _, s := range p.Symbols { - const ( - N_UNDEF = 0 // An undefined (extern) symbol - N_ABS = -1 // An absolute symbol (e_value is a constant, not an address) - N_DEBUG = -2 // A debugging symbol - ) - sym := Sym{Name: s.Name, Addr: uint64(s.Value), Code: '?'} - switch s.SectionNumber { - case N_UNDEF: - sym.Code = 'U' - case N_ABS: - sym.Code = 'C' - case N_DEBUG: - sym.Code = '?' - default: - if s.SectionNumber < 0 { - errorf("parsing %s: invalid section number %d", f.Name(), s.SectionNumber) - return - } - if len(p.Sections) < int(s.SectionNumber) { - errorf("parsing %s: section number %d is large then max %d", f.Name(), s.SectionNumber, len(p.Sections)) - return - } - sect := p.Sections[s.SectionNumber-1] - const ( - text = 0x20 - data = 0x40 - bss = 0x80 - permX = 0x20000000 - permR = 0x40000000 - permW = 0x80000000 - ) - ch := sect.Characteristics - switch { - case ch&text != 0: - sym.Code = 'T' - case ch&data != 0: - if ch&permW == 0 { - sym.Code = 'R' - } else { - sym.Code = 'D' - } - case ch&bss != 0: - sym.Code = 'B' - } - sym.Addr += imageBase + uint64(sect.VirtualAddress) - } - syms = append(syms, sym) - addrs = append(addrs, sym.Addr) - } - - sort.Sort(uint64s(addrs)) - for i := range syms { - j := sort.Search(len(addrs), func(x int) bool { return addrs[x] > syms[i].Addr }) - if j < len(addrs) { - syms[i].Size = int64(addrs[j] - syms[i].Addr) - } - } - - return -} diff --git a/src/cmd/objdump/plan9obj.go b/src/cmd/objdump/plan9obj.go deleted file mode 100644 index f851d4158c..0000000000 --- a/src/cmd/objdump/plan9obj.go +++ /dev/null @@ -1,70 +0,0 @@ -// Copyright 2014 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. - -// Parsing of Plan 9 a.out executables. - -package main - -import ( - "debug/plan9obj" - "os" - "sort" -) - -var validSymType = map[rune]bool{ - 'T': true, - 't': true, - 'D': true, - 'd': true, - 'B': true, - 'b': true, -} - -func plan9Symbols(f *os.File) (syms []Sym, goarch string) { - p, err := plan9obj.NewFile(f) - if err != nil { - errorf("parsing %s: %v", f.Name(), err) - return - } - - plan9Syms, err := p.Symbols() - if err != nil { - errorf("parsing %s: %v", f.Name(), err) - return - } - - switch p.Magic { - case plan9obj.MagicAMD64: - goarch = "amd64" - case plan9obj.Magic386: - goarch = "386" - case plan9obj.MagicARM: - goarch = "arm" - } - - // Build sorted list of addresses of all symbols. - // We infer the size of a symbol by looking at where the next symbol begins. - var addrs []uint64 - for _, s := range plan9Syms { - if !validSymType[s.Type] { - continue - } - addrs = append(addrs, s.Value) - } - sort.Sort(uint64s(addrs)) - - for _, s := range plan9Syms { - if !validSymType[s.Type] { - continue - } - sym := Sym{Addr: s.Value, Name: s.Name, Code: rune(s.Type)} - i := sort.Search(len(addrs), func(x int) bool { return addrs[x] > s.Value }) - if i < len(addrs) { - sym.Size = int64(addrs[i] - s.Value) - } - syms = append(syms, sym) - } - - return -} From bfe459adc98724354ffefa624b73815adaa711d8 Mon Sep 17 00:00:00 2001 From: Rob Pike Date: Wed, 29 Oct 2014 15:35:48 -0700 Subject: [PATCH 06/25] doc/go1.4.html: final library changes First draft now complete. LGTM=rsc R=golang-codereviews, rsc CC=golang-codereviews https://golang.org/cl/170750043 --- doc/go1.4.html | 150 ++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 129 insertions(+), 21 deletions(-) diff --git a/doc/go1.4.html b/doc/go1.4.html index e2458f2efe..19bad0065d 100644 --- a/doc/go1.4.html +++ b/doc/go1.4.html @@ -466,11 +466,6 @@ rebuild the standard library and commands, to avoid overwriting the installation -

    Changes to godoc

    -

    -TODO godoc news -

    -

    Changes to package source layout

    @@ -555,14 +550,57 @@ There are no new packages in this release.

    Major changes to the library

    +

    bufio.Scanner

    +

    -TODO major changes +The Scanner type in the +bufio package +has had a bug fixed that may require changes to custom +split functions. +The bug made it impossible to generate an empty token at EOF; the fix +changes the end conditions seen by the split function. +Previously, scanning stopped at EOF if there was no more data. +As of 1.4, the split function will be called once at EOF after input is exhausted, +so the split function can generate a final empty token +as the documentation already promised.

    -
    -bufio: handling of empty tokens at EOF changed, may require scanner change (CL 145390043)
-syscall: now frozen (CL 129820043); go.sys subrepo created: http://golang.org/s/go1.4-syscall
-
    +

    +Updating: Custom split functions may need to be modified to +handle empty tokens at EOF as desired. +

    + +

    syscall

    + +

    +The syscall package is now frozen except +for changes needed to maintain the core repository. +In particular, it will no longer be extended to support new or different system calls +that are not used by the core. +The reasons are described at length in a +separate document. +

    + +

    +A new subrepository, go.sys, +has been created to serve as the location for new developments to support system +calls on all kernels. +It has a nicer structure, with three packages that each hold the implementation of +system calls for one of +Unix, +Windows and +Plan 9. +These packages will be curated more generously, accepting all reasonable changes +that reflect kernel interfaces in those operating systems. +See the documentation and the article mentioned above for more information. +

    + +

    +Updating: Existing programs are not affected as the syscall +package is largely unchanged from the 1.3 release. +Future development that requires system calls not in the syscall package +should build on go.sys instead. +

    Minor changes to the library

    @@ -629,11 +667,28 @@ For instance, &map[string]int{"one": 1} now prints &map[one: 1] rather than as a hexadecimal pointer value.
    • -
  • TODO net/http: add Request.BasicAuth method ( https://codereview.appspot.com/76540043)
    • +
  • +The net/http package's +Request type +has a new BasicAuth method +that returns the username and password from authenticated requests using the +HTTP Basic Authentication +Scheme. +
    • -
  • TODO net/http: add Transport.DialTLS hook ( https://codereview.appspot.com/137940043)
    • +
  • The net/http package's +Transport type +has a new DialTLS function +that simplifies setting up TLS connections. +
    • -
  • TODO net/http/httputil: add ReverseProxy.ErrorLog ( https://codereview.appspot.com/132750043)
    • +
  • +The net/http/httputil package's +ReverseProxy type +has a new field, +ErrorLog, that +provides user control of logging. +
  • The os package @@ -656,21 +711,74 @@ because of changes to the implementation of interfaces in the runtime. This saves memory but has no semantic effect.
    • -
  • TODO runtime: implement monotonic clocks on windows ( https://codereview.appspot.com/108700045)
    • +
  • +The runtime package +now implements monotonic clocks on Windows, +as it already did for the other systems. +
    • -
  • TODO runtime: MemStats.Mallocs now counts very small allocations missed in Go 1.3. This may break tests using runtime.ReadMemStats or testing.AllocsPerRun by giving a more accurate answer than Go 1.3 did ( https://codereview.appspot.com/143150043).
    • +
  • +The runtime package's +Mallocs counter +now counts very small allocations that were missed in Go 1.3. +This may break tests using ReadMemStats +or AllocsPerRun +due to the more accurate answer. +
    • -
  • TODO runtime/race: freebsd is supported ( https://codereview.appspot.com/107270043)
    • +
  • +In the runtime package, +an array PauseEnd +has been added to the +MemStats +and GCStats structs. +This array is a circular buffer of times when garbage collection pauses ended. +The corresponding pause durations are already recorded in +PauseNs +
    • -
  • TODO runtime: add PauseEnd array to MemStats and GCStats ( https://codereview.appspot.com/153670043)
    • +
  • +The runtime/race package +now supports FreeBSD, which means the +go command's -race +flag now works on FreeBSD. +
    • -
  • TODO sync/atomic: add Value ( https://codereview.appspot.com/136710045)
    • +
  • +The sync/atomic package +has a new type, Value. +Value provides an efficient mechanism for atomic loads and +stores of values of arbitrary type. +
    • -
  • TODO syscall: Setuid, Setgid are disabled on linux platforms. On linux those syscalls operate on the calling thread, not the whole process. This does not match the semantics of other platforms, nor the expectations of the caller, so the operations have been disabled until issue 1435 is resolved ( https://codereview.appspot.com/106170043)
    • +
  • +In the syscall package's +implementation on Linux, the +Setuid +and Setgid have been disabled +because those system calls operate on the calling thread, not the whole process, which is +different from other platforms and not the expected result. +
    • -
  • TODO testing: add Coverage ( https://codereview.appspot.com/98150043)
    • +
  • +The testing package +has a new facility to provide more control over running a set of tests. +If the test code contains a function +
    +func TestMain(m *testing.M) 
+
    -
  • TODO testing: add TestMain support ( https://codereview.appspot.com/148770043)
    • +that function will be called instead of running the tests directly. +The M struct contains methods to access and run the tests. + + +
  • +Also in the testing package, +a new Coverage +function reports the current test coverage fraction, +enabling individual tests to report how much they are contributing to the +overall coverage. +
  • The text/scanner package's From f9c4c16dce621f1834943f3ccda0d0a079f7b1a4 Mon Sep 17 00:00:00 2001 From: Alex Brainman Date: Thu, 30 Oct 2014 10:24:37 +1100 Subject: [PATCH 07/25] runtime: make TestCgoExternalThreadPanic run on windows LGTM=rsc R=golang-codereviews, bradfitz, rsc CC=golang-codereviews https://golang.org/cl/163540043 --- src/runtime/crash_cgo_test.go | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/src/runtime/crash_cgo_test.go b/src/runtime/crash_cgo_test.go index 5958ad8914..972eedc624 100644 --- a/src/runtime/crash_cgo_test.go +++ b/src/runtime/crash_cgo_test.go @@ -36,10 +36,14 @@ func TestCgoTraceback(t *testing.T) { } func TestCgoExternalThreadPanic(t *testing.T) { - if runtime.GOOS == "windows" || runtime.GOOS == "plan9" { + if runtime.GOOS == "plan9" { t.Skipf("no pthreads on %s", runtime.GOOS) } - got := executeTest(t, cgoExternalThreadPanicSource, nil, "main.c", cgoExternalThreadPanicC) + csrc := cgoExternalThreadPanicC + if runtime.GOOS == "windows" { + csrc = cgoExternalThreadPanicC_windows + } + got := executeTest(t, cgoExternalThreadPanicSource, nil, "main.c", csrc) want := "panic: BOOM" if !strings.Contains(got, want) { t.Fatalf("want failure containing %q. output:\n%s\n", want, got) @@ -169,3 +173,24 @@ start(void) printf("pthread_create failed\n"); } ` + +const cgoExternalThreadPanicC_windows = ` +#include +#include + +void gopanic(void); + +static void* +die(void* x) +{ + gopanic(); + return 0; +} + +void +start(void) +{ + if(_beginthreadex(0, 0, die, 0, 0, 0) != 0) + printf("_beginthreadex failed\n"); +} +` From a5a07331444f9b48a5e09728e3d0085cfbfb2222 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Wed, 29 Oct 2014 20:37:44 -0400 Subject: [PATCH 08/25] runtime: change top-most return PC from goexit to goexit+PCQuantum If you get a stack of PCs from Callers, it would be expected that every PC is immediately after a call instruction, so to find the line of the call, you look up the line for PC-1. CL 163550043 now explicitly documents that. The most common exception to this is the top-most return PC on the stack, which is the entry address of the runtime.goexit function. Subtracting 1 from that PC will end up in a different function entirely. To remove this special case, make the top-most return PC goexit+PCQuantum and then implement goexit in assembly so that the first instruction can be skipped. Fixes #7690. LGTM=r R=r CC=golang-codereviews https://golang.org/cl/170720043 --- src/runtime/asm_386.s | 6 ++++++ src/runtime/asm_amd64.s | 6 ++++++ src/runtime/asm_amd64p32.s | 6 ++++++ src/runtime/asm_arm.s | 6 ++++++ src/runtime/proc.c | 8 +++----- 5 files changed, 27 insertions(+), 5 deletions(-) diff --git a/src/runtime/asm_386.s b/src/runtime/asm_386.s index 0d46a9eff7..b4b81d7397 100644 --- a/src/runtime/asm_386.s +++ b/src/runtime/asm_386.s @@ -2284,3 +2284,9 @@ TEXT _cgo_topofstack(SB),NOSPLIT,$0 MOVL m_curg(AX), AX MOVL (g_stack+stack_hi)(AX), AX RET + +// The top-most function running on a goroutine +// returns to goexit+PCQuantum. +TEXT runtime·goexit(SB),NOSPLIT,$0-0 + BYTE $0x90 // NOP + CALL runtime·goexit1(SB) // does not return diff --git a/src/runtime/asm_amd64.s b/src/runtime/asm_amd64.s index a9b082beb8..39d7c78f23 100644 --- a/src/runtime/asm_amd64.s +++ b/src/runtime/asm_amd64.s @@ -2229,3 +2229,9 @@ TEXT _cgo_topofstack(SB),NOSPLIT,$0 MOVQ m_curg(AX), AX MOVQ (g_stack+stack_hi)(AX), AX RET + +// The top-most function running on a goroutine +// returns to goexit+PCQuantum. +TEXT runtime·goexit(SB),NOSPLIT,$0-0 + BYTE $0x90 // NOP + CALL runtime·goexit1(SB) // does not return diff --git a/src/runtime/asm_amd64p32.s b/src/runtime/asm_amd64p32.s index 28875bc55a..a1116b5d47 100644 --- a/src/runtime/asm_amd64p32.s +++ b/src/runtime/asm_amd64p32.s @@ -1079,3 +1079,9 @@ TEXT runtime·fastrand1(SB), NOSPLIT, $0-4 TEXT runtime·return0(SB), NOSPLIT, $0 MOVL $0, AX RET + +// The top-most function running on a goroutine +// returns to goexit+PCQuantum. +TEXT runtime·goexit(SB),NOSPLIT,$0-0 + BYTE $0x90 // NOP + CALL runtime·goexit1(SB) // does not return diff --git a/src/runtime/asm_arm.s b/src/runtime/asm_arm.s index e94b4c1ff6..0f3b5eeb8b 100644 --- a/src/runtime/asm_arm.s +++ b/src/runtime/asm_arm.s @@ -1320,3 +1320,9 @@ TEXT _cgo_topofstack(SB),NOSPLIT,$8 MOVW saveG-8(SP), g MOVW saveR11-4(SP), R11 RET + +// The top-most function running on a goroutine +// returns to goexit+PCQuantum. +TEXT runtime·goexit(SB),NOSPLIT,$-4-0 + MOVW R0, R0 // NOP + BL runtime·goexit1(SB) // does not return diff --git a/src/runtime/proc.c b/src/runtime/proc.c index b46f67065a..4be51e1e16 100644 --- a/src/runtime/proc.c +++ b/src/runtime/proc.c @@ -1643,12 +1643,10 @@ runtime·gosched_m(G *gp) } // Finishes execution of the current goroutine. -// Need to mark it as nosplit, because it runs with sp > stackbase. -// Since it does not return it does not matter. But if it is preempted -// at the split stack check, GC will complain about inconsistent sp. +// Must be NOSPLIT because it is called from Go. #pragma textflag NOSPLIT void -runtime·goexit(void) +runtime·goexit1(void) { void (*fn)(G*); @@ -2192,7 +2190,7 @@ runtime·newproc1(FuncVal *fn, byte *argp, int32 narg, int32 nret, void *callerp runtime·memclr((byte*)&newg->sched, sizeof newg->sched); newg->sched.sp = (uintptr)sp; - newg->sched.pc = (uintptr)runtime·goexit; + newg->sched.pc = (uintptr)runtime·goexit + PCQuantum; // +PCQuantum so that previous instruction is in same function newg->sched.g = newg; runtime·gostartcallfn(&newg->sched, fn); newg->gopc = (uintptr)callerpc; From ca230d2d6ffeaef0be2f58fd46ba6ed34a8dbf46 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Wed, 29 Oct 2014 21:02:58 -0400 Subject: [PATCH 09/25] cmd/objdump: disable test failing on arm5 TBR=adg CC=golang-codereviews https://golang.org/cl/167890043 --- src/cmd/objdump/objdump_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cmd/objdump/objdump_test.go b/src/cmd/objdump/objdump_test.go index 0a2d2565a7..ffaaa5b437 100644 --- a/src/cmd/objdump/objdump_test.go +++ b/src/cmd/objdump/objdump_test.go @@ -143,7 +143,7 @@ var x86Need = []string{ var armNeed = []string{ "fmthello.go:6", "TEXT main.main(SB)", - "B.LS main.main(SB)", + //"B.LS main.main(SB)", // TODO(rsc): restore; golang.org/issue/9021 "BL fmt.Println(SB)", "RET", } From 9cd8cb0b17e4bbff7cfce29631dc78c9cafd2e8f Mon Sep 17 00:00:00 2001 From: Andrew Gerrand Date: Thu, 30 Oct 2014 12:33:57 +1100 Subject: [PATCH 10/25] tag go1.4beta1 TBR=rsc R=r, rsc CC=golang-codereviews https://golang.org/cl/164240043 --- .hgtags | 1 + 1 file changed, 1 insertion(+) diff --git a/.hgtags b/.hgtags index 5a5c4aed44..edd2163f70 100644 --- a/.hgtags +++ b/.hgtags @@ -135,3 +135,4 @@ f8b50ad4cac4d4c4ecf48324b4f512f65e82cc1c go1.3beta1 85518b1d6f8d6e16133b9ed2c9db6807522d37de go1.3.2 f44017549ff9c3cc5eef74ebe7276cd0dfc066b6 go1.3.3 f44017549ff9c3cc5eef74ebe7276cd0dfc066b6 release +1fdfd7dfaedb1b7702141617e621ab7328a236a1 go1.4beta1 From 70dc39e17dc47ecb58a398d077fcdce6046be5dd Mon Sep 17 00:00:00 2001 From: Mikio Hara Date: Thu, 30 Oct 2014 14:15:00 +0900 Subject: [PATCH 11/25] doc/go1.4.html: fix typo LGTM=adg R=r, adg CC=golang-codereviews https://golang.org/cl/165890043 --- doc/go1.4.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/go1.4.html b/doc/go1.4.html index 19bad0065d..9f65aaf24c 100644 --- a/doc/go1.4.html +++ b/doc/go1.4.html @@ -621,7 +621,7 @@ for the decompressors, allowing them to reuse buffers and improve performance.
  • The crypto/tls package -now supports APLN as defined in RFC 7301. +now supports ALPN as defined in RFC 7301.
  • From f21a02a179ca7335ee864512f9afb2c34d6c1850 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Thu, 30 Oct 2014 10:58:31 -0300 Subject: [PATCH 12/25] doc/go1.4.html: tweak http.Transport.DialTLS wording It doesn't simplify, because it wasn't even possible before. LGTM=r R=r CC=golang-codereviews https://golang.org/cl/164250043 --- doc/go1.4.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/go1.4.html b/doc/go1.4.html index 9f65aaf24c..cb2280cb4d 100644 --- a/doc/go1.4.html +++ b/doc/go1.4.html @@ -678,8 +678,8 @@ Scheme.
  • The net/http package's Transport type -has a new DialTLS function -that simplifies setting up TLS connections. +has a new DialTLS hook +that allows customizing the behavior of outbound TLS connections.
  • From 09f6f05c1fdd394ec512642e0cf086e0cf2d3d79 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Thu, 30 Oct 2014 14:01:14 -0400 Subject: [PATCH 13/25] cmd/cgo: avoid worklist nondeterminism. + Regression test. Fixes #9026. LGTM=rsc R=rsc CC=golang-codereviews https://golang.org/cl/162490043 --- misc/cgo/test/cgo_test.go | 1 + misc/cgo/test/issue9026.go | 33 +++++++++++++++++++++++++++++++++ src/cmd/cgo/gcc.go | 24 +++++++++++++++--------- 3 files changed, 49 insertions(+), 9 deletions(-) create mode 100644 misc/cgo/test/issue9026.go diff --git a/misc/cgo/test/cgo_test.go b/misc/cgo/test/cgo_test.go index 3b289ba7b5..fbdfac87ac 100644 --- a/misc/cgo/test/cgo_test.go +++ b/misc/cgo/test/cgo_test.go @@ -62,5 +62,6 @@ func Test8517(t *testing.T) { test8517(t) } func Test8811(t *testing.T) { test8811(t) } func TestReturnAfterGrow(t *testing.T) { testReturnAfterGrow(t) } func TestReturnAfterGrowFromGo(t *testing.T) { testReturnAfterGrowFromGo(t) } +func Test9026(t *testing.T) { test9026(t) } func BenchmarkCgoCall(b *testing.B) { benchCgoCall(b) } diff --git a/misc/cgo/test/issue9026.go b/misc/cgo/test/issue9026.go new file mode 100644 index 0000000000..b17440452f --- /dev/null +++ b/misc/cgo/test/issue9026.go @@ -0,0 +1,33 @@ +package cgotest + +/* +typedef struct {} git_merge_file_input; + +typedef struct {} git_merge_file_options; + +int git_merge_file( + git_merge_file_input *in, + git_merge_file_options *opts) {} +*/ +import "C" +import ( + "fmt" + "testing" +) + +func test9026(t *testing.T) { + var in C.git_merge_file_input + var opts *C.git_merge_file_options + C.git_merge_file(&in, opts) + + // Test that the generated type names are deterministic. + // (Previously this would fail about 10% of the time.) + // + // Brittle: the assertion may fail spuriously when the algorithm + // changes, but should remain stable otherwise. + got := fmt.Sprintf("%T %T", in, opts) + want := "cgotest._Ctype_struct___12 *cgotest._Ctype_struct___13" + if got != want { + t.Errorf("Non-deterministic type names: got %s, want %s", got, want) + } +} diff --git a/src/cmd/cgo/gcc.go b/src/cmd/cgo/gcc.go index d77d56c22a..abdd369d71 100644 --- a/src/cmd/cgo/gcc.go +++ b/src/cmd/cgo/gcc.go @@ -944,6 +944,8 @@ type typeConv struct { // Map from types to incomplete pointers to those types. ptrs map[dwarf.Type][]*Type + // Keys of ptrs in insertion order (deterministic worklist) + ptrKeys []dwarf.Type // Predeclared types. bool ast.Expr @@ -1061,16 +1063,17 @@ func (tr *TypeRepr) Set(repr string, fargs ...interface{}) { func (c *typeConv) FinishType(pos token.Pos) { // Completing one pointer type might produce more to complete. // Keep looping until they're all done. - for len(c.ptrs) > 0 { - for dtype := range c.ptrs { - // Note Type might invalidate c.ptrs[dtype]. - t := c.Type(dtype, pos) - for _, ptr := range c.ptrs[dtype] { - ptr.Go.(*ast.StarExpr).X = t.Go - ptr.C.Set("%s*", t.C) - } - delete(c.ptrs, dtype) + for len(c.ptrKeys) > 0 { + dtype := c.ptrKeys[0] + c.ptrKeys = c.ptrKeys[1:] + + // Note Type might invalidate c.ptrs[dtype]. + t := c.Type(dtype, pos) + for _, ptr := range c.ptrs[dtype] { + ptr.Go.(*ast.StarExpr).X = t.Go + ptr.C.Set("%s*", t.C) } + c.ptrs[dtype] = nil // retain the map key } } @@ -1237,6 +1240,9 @@ func (c *typeConv) Type(dtype dwarf.Type, pos token.Pos) *Type { // Placeholder initialization; completed in FinishType. t.Go = &ast.StarExpr{} t.C.Set("*") + if _, ok := c.ptrs[dt.Type]; !ok { + c.ptrKeys = append(c.ptrKeys, dt.Type) + } c.ptrs[dt.Type] = append(c.ptrs[dt.Type], t) case *dwarf.QualType: From a14ae4451700a690a2ca075d585d55d60f0d46e3 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Thu, 30 Oct 2014 14:08:55 -0400 Subject: [PATCH 14/25] misc/cgo/test: fix bad C test code that fails on some configurations LGTM=rsc R=rsc CC=golang-codereviews https://golang.org/cl/169800043 --- misc/cgo/test/issue9026.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/misc/cgo/test/issue9026.go b/misc/cgo/test/issue9026.go index b17440452f..b5d975f17a 100644 --- a/misc/cgo/test/issue9026.go +++ b/misc/cgo/test/issue9026.go @@ -5,7 +5,7 @@ typedef struct {} git_merge_file_input; typedef struct {} git_merge_file_options; -int git_merge_file( +void git_merge_file( git_merge_file_input *in, git_merge_file_options *opts) {} */ From 014665ce0feee15726a8ef05cc568e752babb6ba Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Thu, 30 Oct 2014 13:15:43 -0700 Subject: [PATCH 15/25] A+C: add Jed Denlea (Fastly corporate CLA) LGTM=iant R=golang-codereviews, iant CC=adg, golang-codereviews https://golang.org/cl/165170043 --- AUTHORS | 1 + CONTRIBUTORS | 1 + 2 files changed, 2 insertions(+) diff --git a/AUTHORS b/AUTHORS index 03e686e751..89fd171794 100644 --- a/AUTHORS +++ b/AUTHORS @@ -156,6 +156,7 @@ Evan Shaw Ewan Chou Fabrizio Milo Fan Hongjian +Fastly, Inc. Fatih Arslan Fazlul Shahriar Felix Geisendörfer diff --git a/CONTRIBUTORS b/CONTRIBUTORS index 7679f78742..67c8b1d5cd 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -299,6 +299,7 @@ Jason Del Ponte Jason Travis Jay Weisskopf Jean-Marc Eurin +Jed Denlea Jeff Hodges Jeff R. Allen Jeff Sickel From 51fcd3cf1bc4b9553da586396b028bfe98d3bff5 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Thu, 30 Oct 2014 17:19:29 -0300 Subject: [PATCH 16/25] A+C: Nathan P Finch (individual CLA) TBR=iant R=iant CC=golang-codereviews https://golang.org/cl/155560045 --- AUTHORS | 1 + CONTRIBUTORS | 1 + 2 files changed, 2 insertions(+) diff --git a/AUTHORS b/AUTHORS index 89fd171794..3caf870358 100644 --- a/AUTHORS +++ b/AUTHORS @@ -316,6 +316,7 @@ Moriyoshi Koizumi Môshe van der Sterre Nan Deng Nathan John Youngman +Nathan P Finch ngmoco, LLC Nicholas Katsaros Nicholas Presta diff --git a/CONTRIBUTORS b/CONTRIBUTORS index 67c8b1d5cd..b4435b0e6d 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -445,6 +445,7 @@ Môshe van der Sterre Mrunal Patel Nan Deng Nathan John Youngman +Nathan P Finch Nicholas Katsaros Nicholas Presta Nicholas Sullivan From 692ad844b632074d9e71b6e59fab227c82da77b5 Mon Sep 17 00:00:00 2001 From: Nathan P Finch Date: Thu, 30 Oct 2014 13:20:43 -0700 Subject: [PATCH 17/25] cmd/go: fix minor typo LGTM=bradfitz R=golang-codereviews, bradfitz CC=golang-codereviews https://golang.org/cl/170770043 --- src/cmd/go/doc.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cmd/go/doc.go b/src/cmd/go/doc.go index 314c69bd8c..946b18875e 100644 --- a/src/cmd/go/doc.go +++ b/src/cmd/go/doc.go @@ -247,7 +247,7 @@ The arguments are space-separated tokens or double-quoted strings passed to the generator as individual arguments when it is run. Quoted strings use Go syntax and are evaluated before execution; a -quoted string appears a single argument to the generator. +quoted string appears as a single argument to the generator. Go generate sets several variables when it runs the generator: From baa5d26f629a38335df010a1b5098ebdec8af3b8 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Fri, 31 Oct 2014 00:48:57 -0300 Subject: [PATCH 18/25] sync/atomic: fix comment referencing Value.Store's argument name Fixes #9029 LGTM=adg, r R=r, adg CC=golang-codereviews https://golang.org/cl/161630044 --- src/sync/atomic/value.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sync/atomic/value.go b/src/sync/atomic/value.go index ab46d9a240..ab3aa11285 100644 --- a/src/sync/atomic/value.go +++ b/src/sync/atomic/value.go @@ -38,7 +38,7 @@ func (v *Value) Load() (x interface{}) { return } -// Store sets the value of the Value to v. +// Store sets the value of the Value to x. // All calls to Store for a given Value must use values of the same concrete type. // Store of an inconsistent type panics, as does Store(nil). func (v *Value) Store(x interface{}) { From 1965ba6e6fce0b643a48f08d6d78c0c2c7822dd5 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Fri, 31 Oct 2014 09:37:11 -0700 Subject: [PATCH 19/25] A+C: add Gabriel Aszalos (individual CLA) LGTM=bradfitz R=adg, bradfitz CC=golang-codereviews https://golang.org/cl/162580043 --- AUTHORS | 1 + CONTRIBUTORS | 1 + 2 files changed, 2 insertions(+) diff --git a/AUTHORS b/AUTHORS index 3caf870358..cea02ac3ec 100644 --- a/AUTHORS +++ b/AUTHORS @@ -167,6 +167,7 @@ Francisco Souza Frederick Kelly Mayle III Fredrik Enestad Frithjof Schulze +Gabriel Aszalos Gary Burd Gautham Thambidorai Georg Reinke diff --git a/CONTRIBUTORS b/CONTRIBUTORS index b4435b0e6d..8c5ceea8a6 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -241,6 +241,7 @@ Fredrik Enestad Frithjof Schulze Fumitoshi Ukai Gaal Yahas +Gabriel Aszalos Gary Burd Gautham Thambidorai Georg Reinke From 2074046d00719c0ec0cbc4857726e9a55b71b63f Mon Sep 17 00:00:00 2001 From: Gabriel Aszalos Date: Fri, 31 Oct 2014 09:38:41 -0700 Subject: [PATCH 20/25] cmd/go: fixed typo in doc and generator LGTM=iant R=golang-codereviews, iant, bradfitz CC=golang-codereviews https://golang.org/cl/163690043 --- src/cmd/go/doc.go | 2 +- src/cmd/go/generate.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cmd/go/doc.go b/src/cmd/go/doc.go index 946b18875e..cf3a54565a 100644 --- a/src/cmd/go/doc.go +++ b/src/cmd/go/doc.go @@ -260,7 +260,7 @@ Go generate sets several variables when it runs the generator: $GOPACKAGE The name of the package of the file containing the directive. -Other than variable substition and quoted-string evaluation, no +Other than variable substitution and quoted-string evaluation, no special processing such as "globbing" is performed on the command line. diff --git a/src/cmd/go/generate.go b/src/cmd/go/generate.go index 4227abbe7c..a83cce8f7a 100644 --- a/src/cmd/go/generate.go +++ b/src/cmd/go/generate.go @@ -58,7 +58,7 @@ Go generate sets several variables when it runs the generator: $GOPACKAGE The name of the package of the file containing the directive. -Other than variable substition and quoted-string evaluation, no +Other than variable substitution and quoted-string evaluation, no special processing such as "globbing" is performed on the command line. From 9dc1cce38db0229e97c1ee8d9929f0457f1af385 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Fri, 31 Oct 2014 09:49:42 -0700 Subject: [PATCH 21/25] database/sql: make TestDrivers not crash on second run Using -test.cpu=1,1 made it crash before. Fixes #9024 LGTM=iant R=adg, iant CC=golang-codereviews https://golang.org/cl/169860043 --- src/database/sql/fakedb_test.go | 2 ++ src/database/sql/sql.go | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/src/database/sql/fakedb_test.go b/src/database/sql/fakedb_test.go index 171c322d49..a993fd46ed 100644 --- a/src/database/sql/fakedb_test.go +++ b/src/database/sql/fakedb_test.go @@ -141,6 +141,8 @@ type Dummy struct { } func TestDrivers(t *testing.T) { + unregisterAllDrivers() + Register("test", fdriver) Register("invalid", Dummy{}) all := Drivers() if len(all) < 2 || !sort.StringsAreSorted(all) || !contains(all, "test") || !contains(all, "invalid") { diff --git a/src/database/sql/sql.go b/src/database/sql/sql.go index ad9179cf7d..6e6f246aee 100644 --- a/src/database/sql/sql.go +++ b/src/database/sql/sql.go @@ -37,6 +37,11 @@ func Register(name string, driver driver.Driver) { drivers[name] = driver } +func unregisterAllDrivers() { + // For tests. + drivers = make(map[string]driver.Driver) +} + // Drivers returns a sorted list of the names of the registered drivers. func Drivers() []string { var list []string From 8985c091e4a83eef27ed2a474e1dd34eae43db3a Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Fri, 31 Oct 2014 10:20:36 -0700 Subject: [PATCH 22/25] net/http: add missing newline in list of leaked goroutines LGTM=bradfitz R=bradfitz CC=golang-codereviews https://golang.org/cl/168860044 --- src/net/http/main_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/net/http/main_test.go b/src/net/http/main_test.go index 9f1dfc3727..b8c71fd19f 100644 --- a/src/net/http/main_test.go +++ b/src/net/http/main_test.go @@ -70,7 +70,7 @@ func goroutineLeaked() bool { } fmt.Fprintf(os.Stderr, "Too many goroutines running after net/http test(s).\n") for stack, count := range stackCount { - fmt.Fprintf(os.Stderr, "%d instances of:\n%s", count, stack) + fmt.Fprintf(os.Stderr, "%d instances of:\n%s\n", count, stack) } return true } From 6a0a47539072b8ba4a7cfa585e4a0d71fba8c4c5 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Sat, 1 Nov 2014 08:27:55 -0700 Subject: [PATCH 23/25] A+C: add Benoit Sigoure (individual CLA) LGTM=bradfitz R=golang-codereviews, bradfitz CC=golang-codereviews https://golang.org/cl/164410043 --- AUTHORS | 1 + CONTRIBUTORS | 1 + 2 files changed, 2 insertions(+) diff --git a/AUTHORS b/AUTHORS index cea02ac3ec..48a262bbd7 100644 --- a/AUTHORS +++ b/AUTHORS @@ -65,6 +65,7 @@ Aulus Egnatius Varialus Ben Olive Benjamin Black Benny Siegert +Benoit Sigoure Berengar Lehr Billie Harold Cleek Bjorn Tillenius diff --git a/CONTRIBUTORS b/CONTRIBUTORS index 8c5ceea8a6..ec69858b60 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -104,6 +104,7 @@ Ben Lynn Ben Olive Benjamin Black Benny Siegert +Benoit Sigoure Berengar Lehr Bill Neubauer Bill Thiede From 8e01fc7e9b531398e868d9899c91a9f052f015c7 Mon Sep 17 00:00:00 2001 From: Benoit Sigoure Date: Sat, 1 Nov 2014 08:28:09 -0700 Subject: [PATCH 24/25] misc: Increase issue 6997's test timeout to prevent spurious failures. On heavily loaded build servers, a 5 second timeout is too aggressive, which causes this test to fail spuriously. LGTM=iant R=iant CC=golang-codereviews, sqweek https://golang.org/cl/170850043 --- misc/cgo/test/issue6997_linux.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/misc/cgo/test/issue6997_linux.go b/misc/cgo/test/issue6997_linux.go index 871bd517a7..5455f0c536 100644 --- a/misc/cgo/test/issue6997_linux.go +++ b/misc/cgo/test/issue6997_linux.go @@ -34,7 +34,7 @@ func test6997(t *testing.T) { if r == 0 { t.Error("pthread finished but wasn't cancelled??") } - case <-time.After(5 * time.Second): + case <-time.After(30 * time.Second): t.Error("hung in pthread_cancel/pthread_join") } } From b802240300a33024c1a47fdf2c5260a3fad0155b Mon Sep 17 00:00:00 2001 From: Andrew Gerrand Date: Mon, 3 Nov 2014 17:01:17 +1100 Subject: [PATCH 25/25] doc: document go get -f flag in 1.4 release notes LGTM=r, rsc R=r, rsc, adg CC=golang-codereviews https://golang.org/cl/168890043 --- doc/go1.4.html | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/doc/go1.4.html b/doc/go1.4.html index cb2280cb4d..3310117a4d 100644 --- a/doc/go1.4.html +++ b/doc/go1.4.html @@ -371,6 +371,15 @@ fails because of this check, the mis-imported package has been copied to the loc and should be removed manually.

    +

    +To complement this new feature, a check has been added at update time to verify +that the local package's remote repository matches that of its custom import. +The go get -u command will fail to +update a package if its remote repository has changed since it was first +downloaded. +The new -f flag overrides this check. +

    +

    Further information is in the design document.