From 0a4fb9736f5eb9c4c8a4530b24a59ad4c6693280 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Mon, 6 Oct 2014 14:49:22 -0400 Subject: cmd/go: fix 'go vet' of package with external tests For example, fixes 'go vet syscall', which has source files in package syscall_test. Fixes issue 8511. LGTM=r R=golang-codereviews, r CC=golang-codereviews, iant https://codereview.appspot.com/152220044 --- src/cmd/go/test.bash | 14 ++++++++++++++ src/cmd/go/testdata/src/vetpkg/a_test.go | 1 + src/cmd/go/testdata/src/vetpkg/b.go | 7 +++++++ src/cmd/go/vet.go | 23 ++++++++++++++++++----- 4 files changed, 40 insertions(+), 5 deletions(-) create mode 100644 src/cmd/go/testdata/src/vetpkg/a_test.go create mode 100644 src/cmd/go/testdata/src/vetpkg/b.go (limited to 'src') diff --git a/src/cmd/go/test.bash b/src/cmd/go/test.bash index 6a72bcde0..652ef3b5b 100755 --- a/src/cmd/go/test.bash +++ b/src/cmd/go/test.bash @@ -1085,6 +1085,20 @@ fi unset GOPATH rm -rf $d +TEST go vet with external tests +d=$(mktemp -d -t testgoXXX) +export GOPATH=$(pwd)/testdata +if ./testgo vet vetpkg >$d/err 2>&1; then + echo "go vet vetpkg passes incorrectly" + ok=false +elif ! grep -q 'missing argument for Printf' $d/err; then + echo "go vet vetpkg did not find missing argument for Printf" + cat $d/err + ok=false +fi +unset GOPATH +rm -rf $d + # clean up if $started; then stop; fi rm -rf testdata/bin testdata/bin1 diff --git a/src/cmd/go/testdata/src/vetpkg/a_test.go b/src/cmd/go/testdata/src/vetpkg/a_test.go new file mode 100644 index 000000000..9b64e8e1a --- /dev/null +++ b/src/cmd/go/testdata/src/vetpkg/a_test.go @@ -0,0 +1 @@ +package p_test diff --git a/src/cmd/go/testdata/src/vetpkg/b.go b/src/cmd/go/testdata/src/vetpkg/b.go new file mode 100644 index 000000000..99e18f63d --- /dev/null +++ b/src/cmd/go/testdata/src/vetpkg/b.go @@ -0,0 +1,7 @@ +package p + +import "fmt" + +func f() { + fmt.Printf("%d") +} diff --git a/src/cmd/go/vet.go b/src/cmd/go/vet.go index ffb431837..de7befc61 100644 --- a/src/cmd/go/vet.go +++ b/src/cmd/go/vet.go @@ -4,6 +4,8 @@ package main +import "path/filepath" + func init() { addBuildFlagsNX(cmdVet) } @@ -28,10 +30,21 @@ See also: go fmt, go fix. } func runVet(cmd *Command, args []string) { - for _, pkg := range packages(args) { - // Use pkg.gofiles instead of pkg.Dir so that - // the command only applies to this package, - // not to packages in subdirectories. - run(tool("vet"), relPaths(stringList(pkg.gofiles, pkg.sfiles))) + for _, p := range packages(args) { + // Vet expects to be given a set of files all from the same package. + // Run once for package p and once for package p_test. + if len(p.GoFiles)+len(p.CgoFiles)+len(p.TestGoFiles) > 0 { + runVetFiles(p, stringList(p.GoFiles, p.CgoFiles, p.TestGoFiles, p.SFiles)) + } + if len(p.XTestGoFiles) > 0 { + runVetFiles(p, stringList(p.XTestGoFiles)) + } + } +} + +func runVetFiles(p *Package, files []string) { + for i := range files { + files[i] = filepath.Join(p.Dir, files[i]) } + run(tool("vet"), relPaths(files)) } -- cgit v1.2.1 From 1bd70feb2f014c81db749dde3fb66b40938aae19 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Mon, 6 Oct 2014 15:32:11 -0400 Subject: regexp/syntax: regenerate doc.go from re2 syntax Generated using re2/doc/mksyntaxgo. Fixes issue 8505. LGTM=iant R=r, iant CC=golang-codereviews https://codereview.appspot.com/155890043 --- src/regexp/syntax/doc.go | 48 ++++++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 24 deletions(-) (limited to 'src') diff --git a/src/regexp/syntax/doc.go b/src/regexp/syntax/doc.go index 8e72c90d3..e5e71f14f 100644 --- a/src/regexp/syntax/doc.go +++ b/src/regexp/syntax/doc.go @@ -21,8 +21,8 @@ Single characters: [^xyz] negated character class \d Perl character class \D negated Perl character class - [:alpha:] ASCII character class - [:^alpha:] negated ASCII character class + [[:alpha:]] ASCII character class + [[:^alpha:]] negated ASCII character class \pN Unicode character class (one-letter name) \p{Greek} Unicode character class \PN negated Unicode character class (one-letter name) @@ -46,14 +46,14 @@ Repetitions: x{n,}? n or more x, prefer fewer x{n}? exactly n x -Implementation restriction: The counting forms x{n} etc. (but not the other -forms x* etc.) have an upper limit of n=1000. Negative or higher explicit -counts yield the parse error ErrInvalidRepeatSize. +Implementation restriction: The counting forms x{n,m}, x{n,}, and x{n} +reject forms that create a minimum or maximum repetition count above 1000. +Unlimited repetitions are not subject to this restriction. Grouping: (re) numbered capturing group (submatch) (?Pre) named & numbered capturing group (submatch) - (?:re) non-capturing group (submatch) + (?:re) non-capturing group (?flags) set flags within current group; non-capturing (?flags:re) set flags during re; non-capturing @@ -69,7 +69,7 @@ Empty strings: $ at end of text (like \z not \Z) or line (flag m=true) \A at beginning of text \b at ASCII word boundary (\w on one side and \W, \A, or \z on the other) - \B not an ASCII word boundary + \B not at ASCII word boundary \z at end of text Escape sequences: @@ -103,29 +103,29 @@ Named character classes as character class elements: [\p{Name}] named Unicode property inside character class (== \p{Name}) [^\p{Name}] named Unicode property inside negated character class (== \P{Name}) -Perl character classes: +Perl character classes (all ASCII-only): \d digits (== [0-9]) \D not digits (== [^0-9]) \s whitespace (== [\t\n\f\r ]) \S not whitespace (== [^\t\n\f\r ]) - \w ASCII word characters (== [0-9A-Za-z_]) - \W not ASCII word characters (== [^0-9A-Za-z_]) + \w word characters (== [0-9A-Za-z_]) + \W not word characters (== [^0-9A-Za-z_]) ASCII character classes: - [:alnum:] alphanumeric (== [0-9A-Za-z]) - [:alpha:] alphabetic (== [A-Za-z]) - [:ascii:] ASCII (== [\x00-\x7F]) - [:blank:] blank (== [\t ]) - [:cntrl:] control (== [\x00-\x1F\x7F]) - [:digit:] digits (== [0-9]) - [:graph:] graphical (== [!-~] == [A-Za-z0-9!"#$%&'()*+,\-./:;<=>?@[\\\]^_`{|}~]) - [:lower:] lower case (== [a-z]) - [:print:] printable (== [ -~] == [ [:graph:]]) - [:punct:] punctuation (== [!-/:-@[-`{-~]) - [:space:] whitespace (== [\t\n\v\f\r ]) - [:upper:] upper case (== [A-Z]) - [:word:] word characters (== [0-9A-Za-z_]) - [:xdigit:] hex digit (== [0-9A-Fa-f]) + [[:alnum:]] alphanumeric (== [0-9A-Za-z]) + [[:alpha:]] alphabetic (== [A-Za-z]) + [[:ascii:]] ASCII (== [\x00-\x7F]) + [[:blank:]] blank (== [\t ]) + [[:cntrl:]] control (== [\x00-\x1F\x7F]) + [[:digit:]] digits (== [0-9]) + [[:graph:]] graphical (== [!-~] == [A-Za-z0-9!"#$%&'()*+,\-./:;<=>?@[\\\]^_`{|}~]) + [[:lower:]] lower case (== [a-z]) + [[:print:]] printable (== [ -~] == [ [:graph:]]) + [[:punct:]] punctuation (== [!-/:-@[-`{-~]) + [[:space:]] whitespace (== [\t\n\v\f\r ]) + [[:upper:]] upper case (== [A-Z]) + [[:word:]] word characters (== [0-9A-Za-z_]) + [[:xdigit:]] hex digit (== [0-9A-Fa-f]) */ package syntax -- cgit v1.2.1 From 108521744f8607c55dff4fca73342d8e72219641 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Mon, 6 Oct 2014 15:48:17 -0400 Subject: encoding/json: document that embedded interfaces look like non-embedded ones Fixes issue 8386. LGTM=r R=golang-codereviews, r CC=golang-codereviews, iant https://codereview.appspot.com/149570043 --- src/encoding/json/encode.go | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src') diff --git a/src/encoding/json/encode.go b/src/encoding/json/encode.go index b63538c92..9b7b9d5fd 100644 --- a/src/encoding/json/encode.go +++ b/src/encoding/json/encode.go @@ -93,6 +93,8 @@ import ( // as described in the next paragraph. // An anonymous struct field with a name given in its JSON tag is treated as // having that name, rather than being anonymous. +// An anonymous struct field of interface type is treated the same as having +// that type as its name, rather than being anonymous. // // The Go visibility rules for struct fields are amended for JSON when // deciding which field to marshal or unmarshal. If there are -- cgit v1.2.1 From eb32a44bc00a78ca46a69f57f600dd430c22cc2b Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Mon, 6 Oct 2014 15:49:07 -0400 Subject: net/url: document result of String Fixes issue 8742. LGTM=bradfitz R=golang-codereviews CC=adg, bradfitz, golang-codereviews, iant https://codereview.appspot.com/155910043 --- src/net/url/url.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'src') diff --git a/src/net/url/url.go b/src/net/url/url.go index 0b32cd7c8..f167408fa 100644 --- a/src/net/url/url.go +++ b/src/net/url/url.go @@ -441,6 +441,24 @@ func parseAuthority(authority string) (user *Userinfo, host string, err error) { } // String reassembles the URL into a valid URL string. +// The general form of the result is one of: +// +// scheme:opaque +// scheme://userinfo@host/path?query#fragment +// +// If u.Opaque is non-empty, String uses the first form; +// otherwise it uses the second form. +// +// In the second form, the following rules apply: +// - if u.Scheme is empty, scheme: is omitted. +// - if u.User is nil, userinfo@ is omitted. +// - if u.Host is empty, host/ is omitted. +// - if u.Scheme and u.Host are empty and u.User is nil, +// the entire scheme://userinfo@host/ is omitted. +// - if u.Host is non-empty and u.Path begins with a /, +// the form host/path does not add its own /. +// - if u.RawQuery is empty, ?query is omitted. +// - if u.Fragment is empty, #fragment is omitted. func (u *URL) String() string { var buf bytes.Buffer if u.Scheme != "" { -- cgit v1.2.1 From 591f36004682e1adf977cb5d9f12c98b560924d7 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Mon, 6 Oct 2014 15:49:19 -0400 Subject: os: make Process.Signal 'process finished' error consistent on Unix While we're here, fix the implementation of Release on both Unix and Windows: Release is supposed to make Signal an error. While we're here, make sure we never Signal pid 0. (Don't try this at home.) Fixes issue 7658. LGTM=r R=golang-codereviews, r CC=golang-codereviews, iant https://codereview.appspot.com/152240043 --- src/os/exec_unix.go | 14 +++++++++++--- src/os/exec_windows.go | 3 +++ 2 files changed, 14 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/os/exec_unix.go b/src/os/exec_unix.go index 1b1e3350b..ed97f85e2 100644 --- a/src/os/exec_unix.go +++ b/src/os/exec_unix.go @@ -34,18 +34,26 @@ func (p *Process) wait() (ps *ProcessState, err error) { return ps, nil } +var errFinished = errors.New("os: process already finished") + func (p *Process) signal(sig Signal) error { - if p.done() { - return errors.New("os: process already finished") - } if p.Pid == -1 { return errors.New("os: process already released") } + if p.Pid == 0 { + return errors.New("os: process not initialized") + } + if p.done() { + return errFinished + } s, ok := sig.(syscall.Signal) if !ok { return errors.New("os: unsupported signal type") } if e := syscall.Kill(p.Pid, s); e != nil { + if e == syscall.ESRCH { + return errFinished + } return e } return nil diff --git a/src/os/exec_windows.go b/src/os/exec_windows.go index c4f3d4f85..393393b23 100644 --- a/src/os/exec_windows.go +++ b/src/os/exec_windows.go @@ -53,6 +53,9 @@ func terminateProcess(pid, exitcode int) error { } func (p *Process) signal(sig Signal) error { + if p.handle == uintptr(syscall.InvalidHandle) { + return syscall.EINVAL + } if p.done() { return errors.New("os: process already finished") } -- cgit v1.2.1 From 746f251c178ec15bfdb422ddc55af8f343b1f593 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Mon, 6 Oct 2014 15:49:33 -0400 Subject: os: recomment MkdirAll The internal comments are not completely precise about what is going on, and they are causing confusion. Fixes issue 8283. LGTM=r R=r CC=golang-codereviews https://codereview.appspot.com/151460043 --- src/os/path.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/os/path.go b/src/os/path.go index 24a3415b4..84a3be334 100644 --- a/src/os/path.go +++ b/src/os/path.go @@ -17,7 +17,7 @@ import ( // If path is already a directory, MkdirAll does nothing // and returns nil. func MkdirAll(path string, perm FileMode) error { - // If path exists, stop with success or error. + // Fast path: if we can tell whether path is a directory or file, stop with success or error. dir, err := Stat(path) if err == nil { if dir.IsDir() { @@ -26,7 +26,7 @@ func MkdirAll(path string, perm FileMode) error { return &PathError{"mkdir", path, syscall.ENOTDIR} } - // Doesn't already exist; make sure parent does. + // Slow path: make sure parent exists and then call Mkdir for path. i := len(path) for i > 0 && IsPathSeparator(path[i-1]) { // Skip trailing path separator. i-- @@ -45,7 +45,7 @@ func MkdirAll(path string, perm FileMode) error { } } - // Now parent exists, try to create. + // Parent now exists; invoke Mkdir and use its result. err = Mkdir(path, perm) if err != nil { // Handle arguments like "foo/." by -- cgit v1.2.1 From 68c0db9ea9d37fdbbe96f106514d4793a7b7fdca Mon Sep 17 00:00:00 2001 From: Evan Kroske Date: Mon, 6 Oct 2014 17:16:39 -0400 Subject: cmd/gc: prohibit short variable declarations containing duplicate symbols Fixes issue 6764. Fixes issue 8435. LGTM=rsc R=golang-codereviews, r, gobot, rsc CC=golang-codereviews https://codereview.appspot.com/116440046 Committer: Russ Cox --- src/cmd/gc/dcl.c | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'src') diff --git a/src/cmd/gc/dcl.c b/src/cmd/gc/dcl.c index 73c2581be..cc010d901 100644 --- a/src/cmd/gc/dcl.c +++ b/src/cmd/gc/dcl.c @@ -488,6 +488,10 @@ colasdefn(NodeList *left, Node *defn) NodeList *l; Node *n; + for(l=left; l; l=l->next) + if(l->n->sym != S) + l->n->sym->flags |= SymUniq; + nnew = 0; nerr = 0; for(l=left; l; l=l->next) { @@ -499,6 +503,13 @@ colasdefn(NodeList *left, Node *defn) nerr++; continue; } + if((n->sym->flags & SymUniq) == 0) { + yyerrorl(defn->lineno, "%S repeated on left side of :=", n->sym); + n->diag++; + nerr++; + continue; + } + n->sym->flags &= ~SymUniq; if(n->sym->block == block) continue; -- cgit v1.2.1 From 06dffe0d90136e051ba908bdde851c30ebe865b7 Mon Sep 17 00:00:00 2001 From: Rob Pike Date: Mon, 6 Oct 2014 14:50:58 -0700 Subject: go/build: do not consider "android.go" to be android-specific A file name must have a non-empty underscore-separated prefix before its suffix matches GOOS. This is what the documentation already said but is not what the code did. Fixes issue 8838. This needs to be called out in the release notes. The he single affected file code.google.com/p/go.text/collate/tools/colcmp/darwin.go could use a renaming but works because it has a build tag inside. LGTM=adg, rsc R=golang-codereviews, adg, rsc CC=golang-codereviews https://codereview.appspot.com/147690043 --- src/go/build/build.go | 12 ++++++++++++ src/go/build/build_test.go | 4 ++++ 2 files changed, 16 insertions(+) (limited to 'src') diff --git a/src/go/build/build.go b/src/go/build/build.go index 5e11c9b9c..3ac798083 100644 --- a/src/go/build/build.go +++ b/src/go/build/build.go @@ -1291,6 +1291,18 @@ func (ctxt *Context) goodOSArchFile(name string, allTags map[string]bool) bool { if dot := strings.Index(name, "."); dot != -1 { name = name[:dot] } + + // Before Go 1.4, a file called "linux.go" would be equivalent to having a + // build tag "linux" in that file. For Go 1.4 and beyond, we require this + // auto-tagging to apply only to files with a non-empty prefix, so + // "foo_linux.go" is tagged but "linux.go" is not. This allows new operating + // sytems, such as android, to arrive without breaking existing code with + // innocuous source code in "android.go". The easiest fix: files without + // underscores are always included. + if !strings.ContainsRune(name, '_') { + return true + } + l := strings.Split(name, "_") if n := len(l); n > 0 && l[n-1] == "test" { l = l[:n-1] diff --git a/src/go/build/build_test.go b/src/go/build/build_test.go index 004010113..23ce89b4b 100644 --- a/src/go/build/build_test.go +++ b/src/go/build/build_test.go @@ -173,6 +173,10 @@ var matchFileTests = []struct { {ctxtAndroid, "foo_linux.go", "", true}, {ctxtAndroid, "foo_android.go", "", true}, {ctxtAndroid, "foo_plan9.go", "", false}, + {ctxtAndroid, "android.go", "", true}, + {ctxtAndroid, "plan9.go", "", true}, + {ctxtAndroid, "arm.s", "", true}, + {ctxtAndroid, "amd64.s", "", true}, } func TestMatchFile(t *testing.T) { -- cgit v1.2.1 From b69634e55db1205234ef87e3def9449f26fa4f72 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Mon, 6 Oct 2014 15:08:31 -0700 Subject: go/build: update docs for GOOS.go change Forgotten in https://codereview.appspot.com/147690043/ Update Issue 8838 LGTM=r R=r CC=golang-codereviews, rsc https://codereview.appspot.com/152220045 --- src/go/build/doc.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/go/build/doc.go b/src/go/build/doc.go index 56878f2b4..75a827bb9 100644 --- a/src/go/build/doc.go +++ b/src/go/build/doc.go @@ -108,12 +108,10 @@ // *_GOOS // *_GOARCH // *_GOOS_GOARCH -// (example: source_windows_amd64.go) or the literals: -// GOOS -// GOARCH -// (example: windows.go) where GOOS and GOARCH represent any known operating -// system and architecture values respectively, then the file is considered to -// have an implicit build constraint requiring those terms. +// (example: source_windows_amd64.go) where GOOS and GOARCH represent +// any known operating system and architecture values respectively, then +// the file is considered to have an implicit build constraint requiring +// those terms. // // To keep a file from being considered for the build: // -- cgit v1.2.1 From 694a58f1ba2dc208fcd6ec7215262d6ae19d110a Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Mon, 6 Oct 2014 15:10:51 -0700 Subject: strings: use fast path for IndexRune Noticed while reviewing https://codereview.appspot.com/147690043/ I'd never seen anybody use IndexRune before, and unsurprisingly it doesn't use the other fast paths in the strings/bytes packages. IndexByte uses assembly. Also, less code this way. LGTM=r R=r CC=golang-codereviews https://codereview.appspot.com/147700043 --- src/strings/strings.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/strings/strings.go b/src/strings/strings.go index 1b9df2e75..27d384983 100644 --- a/src/strings/strings.go +++ b/src/strings/strings.go @@ -225,13 +225,8 @@ func LastIndex(s, sep string) int { // r, or -1 if rune is not present in s. func IndexRune(s string, r rune) int { switch { - case r < 0x80: - b := byte(r) - for i := 0; i < len(s); i++ { - if s[i] == b { - return i - } - } + case r < utf8.RuneSelf: + return IndexByte(s, byte(r)) default: for i, c := range s { if c == r { -- cgit v1.2.1 From ae91212fb2a2c3d99b6521c95a8773f1d531072c Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Mon, 6 Oct 2014 19:22:48 -0400 Subject: os, syscall: test Chtimes on directories, fix on Windows Fixes issue 8090. LGTM=alex.brainman R=alex.brainman CC=golang-codereviews https://codereview.appspot.com/154020043 --- src/os/os_test.go | 47 ++++++++++++++++++++++++++++++++++-------- src/syscall/syscall_windows.go | 4 ++-- 2 files changed, 40 insertions(+), 11 deletions(-) (limited to 'src') diff --git a/src/os/os_test.go b/src/os/os_test.go index 973cc3a7b..a30a2b031 100644 --- a/src/os/os_test.go +++ b/src/os/os_test.go @@ -124,7 +124,22 @@ func newFile(testName string, t *testing.T) (f *File) { } f, err := ioutil.TempFile(dir, "_Go_"+testName) if err != nil { - t.Fatalf("open %s: %s", testName, err) + t.Fatalf("TempFile %s: %s", testName, err) + } + return +} + +func newDir(testName string, t *testing.T) (name string) { + // Use a local file system, not NFS. + // On Unix, override $TMPDIR in case the user + // has it set to an NFS-mounted directory. + dir := "" + if runtime.GOOS != "android" && runtime.GOOS != "windows" { + dir = "/tmp" + } + name, err := ioutil.TempDir(dir, "_Go_"+testName) + if err != nil { + t.Fatalf("TempDir %s: %s", testName, err) } return } @@ -755,35 +770,49 @@ func TestTruncate(t *testing.T) { } } -// Use TempDir() to make sure we're on a local file system, +// Use TempDir (via newFile) to make sure we're on a local file system, // so that timings are not distorted by latency and caching. // On NFS, timings can be off due to caching of meta-data on // NFS servers (Issue 848). func TestChtimes(t *testing.T) { f := newFile("TestChtimes", t) defer Remove(f.Name()) - defer f.Close() f.Write([]byte("hello, world\n")) f.Close() - st, err := Stat(f.Name()) + testChtimes(t, f.Name()) +} + +// Use TempDir (via newDir) to make sure we're on a local file system, +// so that timings are not distorted by latency and caching. +// On NFS, timings can be off due to caching of meta-data on +// NFS servers (Issue 848). +func TestChtimesDir(t *testing.T) { + name := newDir("TestChtimes", t) + defer RemoveAll(name) + + testChtimes(t, name) +} + +func testChtimes(t *testing.T, name string) { + st, err := Stat(name) if err != nil { - t.Fatalf("Stat %s: %s", f.Name(), err) + t.Fatalf("Stat %s: %s", name, err) } preStat := st // Move access and modification time back a second at := Atime(preStat) mt := preStat.ModTime() - err = Chtimes(f.Name(), at.Add(-time.Second), mt.Add(-time.Second)) + err = Chtimes(name, at.Add(-time.Second), mt.Add(-time.Second)) if err != nil { - t.Fatalf("Chtimes %s: %s", f.Name(), err) + t.Fatalf("Chtimes %s: %s", name, err) } - st, err = Stat(f.Name()) + st, err = Stat(name) if err != nil { - t.Fatalf("second Stat %s: %s", f.Name(), err) + t.Fatalf("second Stat %s: %s", name, err) } postStat := st diff --git a/src/syscall/syscall_windows.go b/src/syscall/syscall_windows.go index bda8214c3..e89fd096a 100644 --- a/src/syscall/syscall_windows.go +++ b/src/syscall/syscall_windows.go @@ -468,7 +468,7 @@ func Utimes(path string, tv []Timeval) (err error) { } h, e := CreateFile(pathp, FILE_WRITE_ATTRIBUTES, FILE_SHARE_WRITE, nil, - OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, 0) + OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, 0) if e != nil { return e } @@ -488,7 +488,7 @@ func UtimesNano(path string, ts []Timespec) (err error) { } h, e := CreateFile(pathp, FILE_WRITE_ATTRIBUTES, FILE_SHARE_WRITE, nil, - OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, 0) + OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, 0) if e != nil { return e } -- cgit v1.2.1 From 3471b12d97d10698f049432493ac8edc5eb65ab9 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Mon, 6 Oct 2014 20:51:05 -0400 Subject: cmd/ld: ignore .Linfo_stringNN variables in clang .o files http://build.golang.org/log/c7a91b6eac8f8daa2bd17801be273e58403a15f2 # cmd/pprof /linux-386-clang-9115aad1dc4a/go/pkg/linux_386/net.a(_all.o): sym#16: ignoring .Linfo_string0 in section 16 (type 0) /linux-386-clang-9115aad1dc4a/go/pkg/linux_386/net.a(_all.o): sym#17: ignoring .Linfo_string1 in section 16 (type 0) /linux-386-clang-9115aad1dc4a/go/pkg/linux_386/net.a(_all.o): sym#18: ignoring .Linfo_string2 in section 16 (type 0) /linux-386-clang-9115aad1dc4a/go/pkg/linux_386/net.a(_all.o): sym#20: ignoring .Linfo_string0 in section 16 (type 0) /linux-386-clang-9115aad1dc4a/go/pkg/linux_386/net.a(_all.o): sym#21: ignoring .Linfo_string1 in section 16 (type 0) ... I don't know what these are. Let's ignore them and see if we get any further. TBR=iant CC=golang-codereviews https://codereview.appspot.com/155030043 --- src/cmd/ld/ldelf.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src') diff --git a/src/cmd/ld/ldelf.c b/src/cmd/ld/ldelf.c index 38e414755..35f8b4985 100644 --- a/src/cmd/ld/ldelf.c +++ b/src/cmd/ld/ldelf.c @@ -582,6 +582,8 @@ ldelf(Biobuf *f, char *pkg, int64 len, char *pn) continue; sect = obj->sect+sym.shndx; if(sect->sym == nil) { + if(strncmp(sym.name, ".Linfo_string", 13) == 0) // clang does this + continue; diag("%s: sym#%d: ignoring %s in section %d (type %d)", pn, i, sym.name, sym.shndx, sym.type); continue; } -- cgit v1.2.1 From 049be2f16a585e45be4d8fe17660a537934488ee Mon Sep 17 00:00:00 2001 From: Jens Frederich Date: Tue, 7 Oct 2014 07:13:42 -0700 Subject: net/http: fix authentication info leakage in Referer header (potential security risk) http.Client calls URL.String() to fill in the Referer header, which may contain authentication info. This patch removes authentication info from the Referer header without introducing any API changes. A new test for net/http is also provided. This is the polished version of Alberto Garc?a Hierro's https://codereview.appspot.com/9766046/ It should handle https Referer right. Fixes issue 8417 LGTM=bradfitz R=golang-codereviews, gobot, bradfitz, mikioh.mikioh CC=golang-codereviews https://codereview.appspot.com/151430043 Committer: Brad Fitzpatrick --- src/net/http/client.go | 28 ++++++++++++++++++++++++++-- src/net/http/client_test.go | 37 +++++++++++++++++++++++++++++++++++++ src/net/http/export_test.go | 5 +++++ 3 files changed, 68 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/net/http/client.go b/src/net/http/client.go index a5a3abe61..ce884d1f0 100644 --- a/src/net/http/client.go +++ b/src/net/http/client.go @@ -101,6 +101,30 @@ type RoundTripper interface { // return true if the string includes a port. func hasPort(s string) bool { return strings.LastIndex(s, ":") > strings.LastIndex(s, "]") } +// refererForURL returns a referer without any authentication info or +// an empty string if lastReq scheme is https and newReq scheme is http. +func refererForURL(lastReq, newReq *url.URL) string { + // https://tools.ietf.org/html/rfc7231#section-5.5.2 + // "Clients SHOULD NOT include a Referer header field in a + // (non-secure) HTTP request if the referring page was + // transferred with a secure protocol." + if lastReq.Scheme == "https" && newReq.Scheme == "http" { + return "" + } + referer := lastReq.String() + if lastReq.User != nil { + // This is not very efficient, but is the best we can + // do without: + // - introducing a new method on URL + // - creating a race condition + // - copying the URL struct manually, which would cause + // maintenance problems down the line + auth := lastReq.User.String() + "@" + referer = strings.Replace(referer, auth, "", 1) + } + return referer +} + // Used in Send to implement io.ReadCloser by bundling together the // bufio.Reader through which we read the response, and the underlying // network connection. @@ -324,8 +348,8 @@ func (c *Client) doFollowingRedirects(ireq *Request, shouldRedirect func(int) bo if len(via) > 0 { // Add the Referer header. lastReq := via[len(via)-1] - if lastReq.URL.Scheme != "https" { - nreq.Header.Set("Referer", lastReq.URL.String()) + if ref := refererForURL(lastReq.URL, nreq.URL); ref != "" { + nreq.Header.Set("Referer", ref) } err = redirectChecker(nreq, via) diff --git a/src/net/http/client_test.go b/src/net/http/client_test.go index 6392c1baf..56b6563c4 100644 --- a/src/net/http/client_test.go +++ b/src/net/http/client_test.go @@ -1036,3 +1036,40 @@ func TestClientTrailers(t *testing.T) { t.Errorf("Response trailers = %#v; want %#v", res.Trailer, want) } } + +func TestReferer(t *testing.T) { + tests := []struct { + lastReq, newReq string // from -> to URLs + want string + }{ + // don't send user: + {"http://gopher@test.com", "http://link.com", "http://test.com"}, + {"https://gopher@test.com", "https://link.com", "https://test.com"}, + + // don't send a user and password: + {"http://gopher:go@test.com", "http://link.com", "http://test.com"}, + {"https://gopher:go@test.com", "https://link.com", "https://test.com"}, + + // nothing to do: + {"http://test.com", "http://link.com", "http://test.com"}, + {"https://test.com", "https://link.com", "https://test.com"}, + + // https to http doesn't send a referer: + {"https://test.com", "http://link.com", ""}, + {"https://gopher:go@test.com", "http://link.com", ""}, + } + for _, tt := range tests { + l, err := url.Parse(tt.lastReq) + if err != nil { + t.Fatal(err) + } + n, err := url.Parse(tt.newReq) + if err != nil { + t.Fatal(err) + } + r := ExportRefererForURL(l, n) + if r != tt.want { + t.Errorf("refererForURL(%q, %q) = %q; want %q", tt.lastReq, tt.newReq, r, tt.want) + } + } +} diff --git a/src/net/http/export_test.go b/src/net/http/export_test.go index a6980b538..87b6c0773 100644 --- a/src/net/http/export_test.go +++ b/src/net/http/export_test.go @@ -9,6 +9,7 @@ package http import ( "net" + "net/url" "time" ) @@ -92,6 +93,10 @@ func ResetCachedEnvironment() { var DefaultUserAgent = defaultUserAgent +func ExportRefererForURL(lastReq, newReq *url.URL) string { + return refererForURL(lastReq, newReq) +} + // SetPendingDialHooks sets the hooks that run before and after handling // pending dials. func SetPendingDialHooks(before, after func()) { -- cgit v1.2.1 From 44ae5998770a185d1f8518b6917e1c094bc62a19 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Tue, 7 Oct 2014 11:06:51 -0400 Subject: runtime: remove type-punning for Type.gc[0], gc[1] Depending on flags&KindGCProg, gc[0] and gc[1] are either pointers or inlined bitmap bits. That's not compatible with a precise garbage collector: it needs to be always pointers or never pointers. Change the inlined bitmap case to store a pointer to an out-of-line bitmap in gc[0]. The out-of-line bitmaps are dedup'ed, so that for example all pointer types share the same out-of-line bitmap. Fixes issue 8864. LGTM=r R=golang-codereviews, dvyukov, r CC=golang-codereviews, iant, khr, rlh https://codereview.appspot.com/155820043 --- src/cmd/gc/reflect.c | 25 ++++++++++++++++++++++--- src/cmd/ld/decodesym.c | 5 ++++- src/reflect/type.go | 4 ++-- src/runtime/malloc.go | 2 +- src/runtime/type.h | 2 +- 5 files changed, 30 insertions(+), 8 deletions(-) (limited to 'src') diff --git a/src/cmd/gc/reflect.c b/src/cmd/gc/reflect.c index 4892ab757..d0ebf6b48 100644 --- a/src/cmd/gc/reflect.c +++ b/src/cmd/gc/reflect.c @@ -716,9 +716,10 @@ static int dcommontype(Sym *s, int ot, Type *t) { int i, alg, sizeofAlg, gcprog; - Sym *sptr, *algsym, *zero, *gcprog0, *gcprog1; + Sym *sptr, *algsym, *zero, *gcprog0, *gcprog1, *sbits; uint8 gcmask[16]; static Sym *algarray; + uint64 x1, x2; char *p; if(ot != 0) @@ -804,8 +805,26 @@ dcommontype(Sym *s, int ot, Type *t) ot = dsymptr(s, ot, gcprog1, 0); } else { gengcmask(t, gcmask); - for(i = 0; i < 2*widthptr; i++) - ot = duint8(s, ot, gcmask[i]); + x1 = 0; + for(i=0; i<8; i++) + x1 = x1<<8 | gcmask[i]; + if(widthptr == 4) { + p = smprint("gcbits.%#016x", x1); + } else { + x2 = 0; + for(i=0; i<8; i++) + x2 = x2<<8 | gcmask[i+8]; + p = smprint("gcbits.%#016llux%016llux", x1, x2); + } + sbits = pkglookup(p, runtimepkg); + if((sbits->flags & SymUniq) == 0) { + sbits->flags |= SymUniq; + for(i = 0; i < 2*widthptr; i++) + duint8(sbits, i, gcmask[i]); + ggloblsym(sbits, 2*widthptr, DUPOK|RODATA); + } + ot = dsymptr(s, ot, sbits, 0); + ot = duintptr(s, ot, 0); } p = smprint("%-uT", t); //print("dcommontype: %s\n", p); diff --git a/src/cmd/ld/decodesym.c b/src/cmd/ld/decodesym.c index c53066942..037263dce 100644 --- a/src/cmd/ld/decodesym.c +++ b/src/cmd/ld/decodesym.c @@ -111,7 +111,10 @@ decodetype_gcprog(LSym *s) uint8* decodetype_gcmask(LSym *s) { - return (uint8*)(s->p + 1*PtrSize + 8 + 1*PtrSize); + LSym *mask; + + mask = decode_reloc_sym(s, 1*PtrSize + 8 + 1*PtrSize); + return mask->p; } // Type.ArrayType.elem and Type.SliceType.Elem diff --git a/src/reflect/type.go b/src/reflect/type.go index f099546d2..a36c0ba60 100644 --- a/src/reflect/type.go +++ b/src/reflect/type.go @@ -1523,8 +1523,8 @@ func (gc *gcProg) appendProg(t *rtype) { // The program is stored in t.gc[0], skip unroll flag. prog = (*[1 << 30]byte)(unsafe.Pointer(t.gc[0]))[1:] } else { - // The mask is embed directly in t.gc. - prog = (*[1 << 30]byte)(unsafe.Pointer(&t.gc[0]))[:] + // The mask is linked directly in t.gc. + prog = (*[2 * ptrSize]byte)(unsafe.Pointer(t.gc[0]))[:] } for i := uintptr(0); i < nptr; i++ { gc.appendWord(extractGCWord(prog, i)) diff --git a/src/runtime/malloc.go b/src/runtime/malloc.go index 99d14e314..9b4264f2b 100644 --- a/src/runtime/malloc.go +++ b/src/runtime/malloc.go @@ -270,7 +270,7 @@ func mallocgc(size uintptr, typ *_type, flags int) unsafe.Pointer { } ptrmask = (*uint8)(add(unsafe.Pointer(ptrmask), 1)) // skip the unroll flag byte } else { - ptrmask = (*uint8)(unsafe.Pointer(&typ.gc[0])) // embed mask + ptrmask = (*uint8)(unsafe.Pointer(typ.gc[0])) // pointer to unrolled mask } if size == 2*ptrSize { *xbits = *ptrmask | bitBoundary diff --git a/src/runtime/type.h b/src/runtime/type.h index de82e886f..f5b4f9d13 100644 --- a/src/runtime/type.h +++ b/src/runtime/type.h @@ -23,7 +23,7 @@ struct Type uint8 kind; void* alg; // gc stores type info required for garbage collector. - // If (kind&KindGCProg)==0, then gc directly contains sparse GC bitmap + // If (kind&KindGCProg)==0, then gc[0] points at sparse GC bitmap // (no indirection), 4 bits per word. // If (kind&KindGCProg)!=0, then gc[1] points to a compiler-generated // read-only GC program; and gc[0] points to BSS space for sparse GC bitmap. -- cgit v1.2.1 From 8fc36a23e489c4a4fe7cb2091630d48cdb04905b Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Tue, 7 Oct 2014 11:07:04 -0400 Subject: encoding/json: fix handling of null with ,string fields Fixes issue 8587. LGTM=bradfitz R=golang-codereviews, bradfitz CC=golang-codereviews, iant, r https://codereview.appspot.com/152270044 --- src/encoding/json/decode.go | 40 ++++++++++++++++++++++++++++++++++++---- src/encoding/json/decode_test.go | 21 ++++++++++++++------- 2 files changed, 50 insertions(+), 11 deletions(-) (limited to 'src') diff --git a/src/encoding/json/decode.go b/src/encoding/json/decode.go index 67ec37388..705bc2e17 100644 --- a/src/encoding/json/decode.go +++ b/src/encoding/json/decode.go @@ -173,7 +173,6 @@ type decodeState struct { scan scanner nextscan scanner // for calls to nextValue savedError error - tempstr string // scratch space to avoid some allocations useNumber bool } @@ -293,6 +292,32 @@ func (d *decodeState) value(v reflect.Value) { } } +type unquotedValue struct{} + +// valueQuoted is like value but decodes a +// quoted string literal or literal null into an interface value. +// If it finds anything other than a quoted string literal or null, +// valueQuoted returns unquotedValue{}. +func (d *decodeState) valueQuoted() interface{} { + switch op := d.scanWhile(scanSkipSpace); op { + default: + d.error(errPhase) + + case scanBeginArray: + d.array(reflect.Value{}) + + case scanBeginObject: + d.object(reflect.Value{}) + + case scanBeginLiteral: + switch v := d.literalInterface().(type) { + case nil, string: + return v + } + } + return unquotedValue{} +} + // indirect walks down v allocating pointers as needed, // until it gets to a non-pointer. // if it encounters an Unmarshaler, indirect stops and returns that. @@ -444,6 +469,8 @@ func (d *decodeState) array(v reflect.Value) { } } +var nullLiteral = []byte("null") + // object consumes an object from d.data[d.off-1:], decoding into the value v. // the first byte ('{') of the object has been read already. func (d *decodeState) object(v reflect.Value) { @@ -566,9 +593,14 @@ func (d *decodeState) object(v reflect.Value) { // Read value. if destring { - d.value(reflect.ValueOf(&d.tempstr)) - d.literalStore([]byte(d.tempstr), subv, true) - d.tempstr = "" // Zero scratch space for successive values. + switch qv := d.valueQuoted().(type) { + case nil: + d.literalStore(nullLiteral, subv, false) + case string: + d.literalStore([]byte(qv), subv, true) + default: + d.saveError(fmt.Errorf("json: invalid use of ,string struct tag, trying to unmarshal unquoted value into %v", item, v.Type())) + } } else { d.value(subv) } diff --git a/src/encoding/json/decode_test.go b/src/encoding/json/decode_test.go index d95657d72..7235969b9 100644 --- a/src/encoding/json/decode_test.go +++ b/src/encoding/json/decode_test.go @@ -1070,18 +1070,25 @@ func TestEmptyString(t *testing.T) { } } -// Test that the returned error is non-nil when trying to unmarshal null string into int, for successive ,string option -// Issue 7046 +// Test that a null for ,string is not replaced with the previous quoted string (issue 7046). +// It should also not be an error (issue 2540, issue 8587). func TestNullString(t *testing.T) { type T struct { - A int `json:",string"` - B int `json:",string"` + A int `json:",string"` + B int `json:",string"` + C *int `json:",string"` } - data := []byte(`{"A": "1", "B": null}`) + data := []byte(`{"A": "1", "B": null, "C": null}`) var s T + s.B = 1 + s.C = new(int) + *s.C = 2 err := Unmarshal(data, &s) - if err == nil { - t.Fatalf("expected error; got %v", s) + if err != nil { + t.Fatalf("Unmarshal: %v") + } + if s.B != 1 || s.C != nil { + t.Fatalf("after Unmarshal, s.B=%d, s.C=%p, want 1, nil", s.B, s.C) } } -- cgit v1.2.1 From edec49862088bc2efe851aeba6a350406f818be7 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Tue, 7 Oct 2014 11:07:18 -0400 Subject: runtime: crash if we see an invalid pointer into GC arena This will help find bugs during the release freeze. It's not clear it should be kept for the release itself. That's issue 8861. The most likely thing that would trigger this is stale pointers that previously were ignored or caused memory leaks. These were allowed due to the use of conservative collection. Now that everything is precise, we should not see them anymore. The small number check reinforces what the stack copier is already doing, catching the storage of integers in pointers. It caught issue 8864. The check is disabled if _cgo_allocate is linked into the binary, which is to say if the binary is using SWIG to allocate untyped Go memory. In that case, there are invalid pointers and there's nothing we can do about it. LGTM=rlh R=golang-codereviews, dvyukov, rlh CC=golang-codereviews, iant, khr, r https://codereview.appspot.com/148470043 --- src/cmd/cc/godefs.c | 6 ++-- src/runtime/mgc0.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 98 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/cmd/cc/godefs.c b/src/cmd/cc/godefs.c index d3ab52fde..d9f67f0ae 100644 --- a/src/cmd/cc/godefs.c +++ b/src/cmd/cc/godefs.c @@ -353,8 +353,10 @@ godefvar(Sym *s) case CSTATIC: case CEXTERN: case CGLOBL: - if(strchr(s->name, '$') != nil) // TODO(lvd) - break; + if(strchr(s->name, '$') != nil) + break; + if(strncmp(s->name, "go.weak.", 8) == 0) + break; Bprint(&outbuf, "var %U\t", s->name); printtypename(t); Bprint(&outbuf, "\n"); diff --git a/src/runtime/mgc0.c b/src/runtime/mgc0.c index 9b9bc0ef1..5876ea5c3 100644 --- a/src/runtime/mgc0.c +++ b/src/runtime/mgc0.c @@ -64,6 +64,7 @@ enum { Debug = 0, + DebugPtrs = 0, // if 1, print trace of every pointer load during GC ConcurrentSweep = 1, WorkbufSize = 4*1024, @@ -127,6 +128,9 @@ BitVector runtime·gcbssmask; Mutex runtime·gclock; +static uintptr badblock[1024]; +static int32 nbadblock; + static Workbuf* getempty(Workbuf*); static Workbuf* getfull(Workbuf*); static void putempty(Workbuf*); @@ -158,6 +162,14 @@ struct WorkData { }; WorkData runtime·work; +// Is _cgo_allocate linked into the binary? +static bool +have_cgo_allocate(void) +{ + extern byte go·weak·runtime·_cgo_allocate_internal[1]; + return go·weak·runtime·_cgo_allocate_internal != nil; +} + // scanblock scans a block of n bytes starting at pointer b for references // to other objects, scanning any it finds recursively until there are no // unscanned objects left. Instead of using an explicit recursion, it keeps @@ -167,8 +179,8 @@ WorkData runtime·work; static void scanblock(byte *b, uintptr n, byte *ptrmask) { - byte *obj, *p, *arena_start, *arena_used, **wp, *scanbuf[8], *ptrbitp, *bitp, bits, xbits, shift, cached; - uintptr i, nobj, size, idx, x, off, scanbufpos; + byte *obj, *obj0, *p, *arena_start, *arena_used, **wp, *scanbuf[8], *ptrbitp, *bitp, bits, xbits, shift, cached; + uintptr i, j, nobj, size, idx, x, off, scanbufpos; intptr ncached; Workbuf *wbuf; Iface *iface; @@ -241,6 +253,8 @@ scanblock(byte *b, uintptr n, byte *ptrmask) ptrmask = nil; // use GC bitmap for pointer info scanobj: + if(DebugPtrs) + runtime·printf("scanblock %p +%p %p\n", b, n, ptrmask); // Find bits of the beginning of the object. if(ptrmask == nil) { off = (uintptr*)b - (uintptr*)arena_start; @@ -279,6 +293,7 @@ scanblock(byte *b, uintptr n, byte *ptrmask) continue; if(bits == BitsPointer) { obj = *(byte**)(b+i); + obj0 = obj; goto markobj; } @@ -321,12 +336,20 @@ scanblock(byte *b, uintptr n, byte *ptrmask) cached >>= gcBits; ncached--; + obj0 = obj; markobj: // At this point we have extracted the next potential pointer. // Check if it points into heap. - if(obj == nil || obj < arena_start || obj >= arena_used) + if(obj == nil) + continue; + if((uintptr)obj < PhysPageSize) { + s = nil; + goto badobj; + } + if(obj < arena_start || obj >= arena_used) continue; // Mark the object. + obj = (byte*)((uintptr)obj & ~(PtrSize-1)); off = (uintptr*)obj - (uintptr*)arena_start; bitp = arena_start - off/wordsPerBitmapByte - 1; shift = (off % wordsPerBitmapByte) * gcBits; @@ -338,8 +361,40 @@ scanblock(byte *b, uintptr n, byte *ptrmask) x = k; x -= (uintptr)arena_start>>PageShift; s = runtime·mheap.spans[x]; - if(s == nil || k < s->start || obj >= s->limit || s->state != MSpanInUse) + if(s == nil || k < s->start || obj >= s->limit || s->state != MSpanInUse) { + // Stack pointers lie within the arena bounds but are not part of the GC heap. + // Ignore them. + if(s != nil && s->state == MSpanStack) + continue; + + badobj: + // If cgo_allocate is linked into the binary, it can allocate + // memory as []unsafe.Pointer that may not contain actual + // pointers and must be scanned conservatively. + // In this case alone, allow the bad pointer. + if(have_cgo_allocate() && ptrmask == nil) + continue; + + // Anything else indicates a bug somewhere. + // If we're in the middle of chasing down a different bad pointer, + // don't confuse the trace by printing about this one. + if(nbadblock > 0) + continue; + + runtime·printf("runtime: garbage collector found invalid heap pointer *(%p+%p)=%p", b, i, obj); + if(s == nil) + runtime·printf(" s=nil\n"); + else + runtime·printf(" span=%p-%p-%p state=%d\n", (uintptr)s->start<limit, (uintptr)(s->start+s->npages)<state); + if(ptrmask != nil) + runtime·throw("bad pointer"); + // Add to badblock list, which will cause the garbage collection + // to keep repeating until it has traced the chain of pointers + // leading to obj all the way back to a root. + if(nbadblock == 0) + badblock[nbadblock++] = (uintptr)b; continue; + } p = (byte*)((uintptr)s->start<sizeclass != 0) { size = s->elemsize; @@ -354,6 +409,24 @@ scanblock(byte *b, uintptr n, byte *ptrmask) obj = p; goto markobj; } + if(DebugPtrs) + runtime·printf("scan *%p = %p => base %p\n", b+i, obj0, obj); + + if(nbadblock > 0 && (uintptr)obj == badblock[nbadblock-1]) { + // Running garbage collection again because + // we want to find the path from a root to a bad pointer. + // Found possible next step; extend or finish path. + for(j=0; j= nelem(badblock)) + runtime·throw("badblock trace too long"); + badblock[nbadblock++] = (uintptr)b; + AlreadyBad:; + } // Now we have bits, bitp, and shift correct for // obj pointing at the base of the object. @@ -381,7 +454,6 @@ scanblock(byte *b, uintptr n, byte *ptrmask) // Queue the obj for scanning. PREFETCH(obj); - obj = (byte*)((uintptr)obj & ~(PtrSize-1)); p = scanbuf[scanbufpos]; scanbuf[scanbufpos++] = obj; if(scanbufpos == nelem(scanbuf)) @@ -400,6 +472,8 @@ scanblock(byte *b, uintptr n, byte *ptrmask) wp++; nobj++; } + if(DebugPtrs) + runtime·printf("end scanblock %p +%p %p\n", b, n, ptrmask); if(Debug && ptrmask == nil) { // For heap objects ensure that we did not overscan. @@ -1306,6 +1380,15 @@ runtime·gc_m(void) a.eagersweep = g->m->scalararg[2]; gc(&a); + if(nbadblock > 0) { + // Work out path from root to bad block. + for(;;) { + gc(&a); + if(nbadblock >= nelem(badblock)) + runtime·throw("cannot find path to bad pointer"); + } + } + runtime·casgstatus(gp, Gwaiting, Grunning); } @@ -1316,6 +1399,9 @@ gc(struct gc_args *args) uint64 heap0, heap1, obj; GCStats stats; + if(DebugPtrs) + runtime·printf("GC start\n"); + if(runtime·debug.allocfreetrace) runtime·tracegc(); @@ -1450,6 +1536,9 @@ gc(struct gc_args *args) runtime·mProf_GC(); g->m->traceback = 0; + + if(DebugPtrs) + runtime·printf("GC end\n"); } extern uintptr runtime·sizeof_C_MStats; -- cgit v1.2.1 From 0eb93113859ac2bc358de929306c473a6bd536ba Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Tue, 7 Oct 2014 12:03:48 -0400 Subject: cmd/gc: fix print format Fixes 386 build. TBR=r CC=golang-codereviews https://codereview.appspot.com/149620043 --- src/cmd/gc/reflect.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/cmd/gc/reflect.c b/src/cmd/gc/reflect.c index d0ebf6b48..b2ff2fbc5 100644 --- a/src/cmd/gc/reflect.c +++ b/src/cmd/gc/reflect.c @@ -809,7 +809,7 @@ dcommontype(Sym *s, int ot, Type *t) for(i=0; i<8; i++) x1 = x1<<8 | gcmask[i]; if(widthptr == 4) { - p = smprint("gcbits.%#016x", x1); + p = smprint("gcbits.%#016llux", x1); } else { x2 = 0; for(i=0; i<8; i++) -- cgit v1.2.1 From ee50d8cf05a1e23b31dcb21c51f9bcbe529afc5a Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Tue, 7 Oct 2014 12:07:40 -0400 Subject: cmd/5c, cmd/6c, cmd/8c: make failure to optimize fatal LGTM=bradfitz, dave, r R=r, bradfitz, dave CC=golang-codereviews https://codereview.appspot.com/152250044 --- src/cmd/5c/reg.c | 9 +++------ src/cmd/6c/reg.c | 14 ++++---------- src/cmd/8c/reg.c | 9 +++------ 3 files changed, 10 insertions(+), 22 deletions(-) (limited to 'src') diff --git a/src/cmd/5c/reg.c b/src/cmd/5c/reg.c index 2fbe031f4..9024d5f49 100644 --- a/src/cmd/5c/reg.c +++ b/src/cmd/5c/reg.c @@ -406,7 +406,7 @@ loop2: rgp->cost = change; nregion++; if(nregion >= NRGN) { - warn(Z, "too many regions"); + fatal(Z, "too many regions"); goto brk; } rgp++; @@ -642,11 +642,8 @@ mkvar(Addr *a, int docon) if(s) if(s->name[0] == '.') goto none; - if(nvar >= NVAR) { - if(debug['w'] > 1 && s) - warn(Z, "variable not optimized: %s", s->name); - goto none; - } + if(nvar >= NVAR) + fatal(Z, "variable not optimized: %s", s->name); i = nvar; nvar++; v = &var[i]; diff --git a/src/cmd/6c/reg.c b/src/cmd/6c/reg.c index 348d747b7..6f8d3ce14 100644 --- a/src/cmd/6c/reg.c +++ b/src/cmd/6c/reg.c @@ -585,14 +585,11 @@ loop2: } rgp->cost = change; nregion++; - if(nregion >= NRGN) { - warn(Z, "too many regions"); - goto brk; - } + if(nregion >= NRGN) + fatal(Z, "too many regions"); rgp++; } } -brk: qsort(region, nregion, sizeof(region[0]), rcmp); /* @@ -808,11 +805,8 @@ mkvar(Reg *r, Addr *a) goto out; v++; } - if(nvar >= NVAR) { - if(debug['w'] > 1 && s) - warn(Z, "variable not optimized: %s", s->name); - goto none; - } + if(nvar >= NVAR) + fatal(Z, "variable not optimized: %s", s->name); i = nvar; nvar++; v = &var[i]; diff --git a/src/cmd/8c/reg.c b/src/cmd/8c/reg.c index e6ba8bcb3..ea862f388 100644 --- a/src/cmd/8c/reg.c +++ b/src/cmd/8c/reg.c @@ -518,7 +518,7 @@ loop2: rgp->cost = change; nregion++; if(nregion >= NRGN) { - warn(Z, "too many regions"); + fatal(Z, "too many regions"); goto brk; } rgp++; @@ -746,11 +746,8 @@ mkvar(Reg *r, Addr *a) goto out; v++; } - if(nvar >= NVAR) { - if(debug['w'] > 1 && s) - warn(Z, "variable not optimized: %s", s->name); - goto none; - } + if(nvar >= NVAR) + fatal(Z, "variable not optimized: %s", s->name); i = nvar; nvar++; v = &var[i]; -- cgit v1.2.1 From 5cf79841e10a5ec2d03a32ea3a40c9932cb843eb Mon Sep 17 00:00:00 2001 From: Rob Pike Date: Tue, 7 Oct 2014 10:52:16 -0700 Subject: net/rpc: add test for issue 7689 (gob error should cause EOF) Helpfully supplied by tommi.virtanen in issue 8173. LGTM=rsc R=golang-codereviews, rsc CC=golang-codereviews https://codereview.appspot.com/151370043 --- src/net/rpc/client_test.go | 51 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) (limited to 'src') diff --git a/src/net/rpc/client_test.go b/src/net/rpc/client_test.go index bbfc1ec3a..c138c06b8 100644 --- a/src/net/rpc/client_test.go +++ b/src/net/rpc/client_test.go @@ -6,6 +6,9 @@ package rpc import ( "errors" + "fmt" + "net" + "strings" "testing" ) @@ -34,3 +37,51 @@ func TestCloseCodec(t *testing.T) { t.Error("client.Close did not close codec") } } + +// Test that errors in gob shut down the connection. Issue 7689. + +type R struct { + msg []byte // Not exported, so R does not work with gob. +} + +type S struct{} + +func (s *S) Recv(nul *struct{}, reply *R) error { + *reply = R{[]byte("foo")} + return nil +} + +func TestGobError(t *testing.T) { + defer func() { + err := recover() + if err == nil { + t.Fatal("no error") + } + if !strings.Contains("reading body EOF", err.(error).Error()) { + t.Fatal("expected `reading body EOF', got", err) + } + }() + Register(new(S)) + + listen, err := net.Listen("tcp", ":5555") + if err != nil { + panic(err) + } + go Accept(listen) + + client, err := Dial("tcp", ":5555") + if err != nil { + panic(err) + } + + var reply Reply + err = client.Call("S.Recv", &struct{}{}, &reply) + if err != nil { + panic(err) + } + + fmt.Printf("%#v\n", reply) + client.Close() + + listen.Close() +} -- cgit v1.2.1 From 70df1111b1bdba591f02e6ca0628a0a73c6427d6 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Tue, 7 Oct 2014 10:56:58 -0700 Subject: math/big: fix doc comments Fixes issue 8904. TBR=iant R=iant CC=golang-codereviews https://codereview.appspot.com/148650043 --- src/math/big/int.go | 4 ++-- src/math/big/rat.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/math/big/int.go b/src/math/big/int.go index 3998652e9..fc53719d7 100644 --- a/src/math/big/int.go +++ b/src/math/big/int.go @@ -1016,12 +1016,12 @@ func (z *Int) UnmarshalJSON(text []byte) error { return nil } -// MarshalText implements the encoding.TextMarshaler interface +// MarshalText implements the encoding.TextMarshaler interface. func (z *Int) MarshalText() (text []byte, err error) { return []byte(z.String()), nil } -// UnmarshalText implements the encoding.TextUnmarshaler interface +// UnmarshalText implements the encoding.TextUnmarshaler interface. func (z *Int) UnmarshalText(text []byte) error { if _, ok := z.SetString(string(text), 0); !ok { return fmt.Errorf("math/big: cannot unmarshal %q into a *big.Int", text) diff --git a/src/math/big/rat.go b/src/math/big/rat.go index e6ab0bb48..0bcec3025 100644 --- a/src/math/big/rat.go +++ b/src/math/big/rat.go @@ -699,12 +699,12 @@ func (z *Rat) GobDecode(buf []byte) error { return nil } -// MarshalText implements the encoding.TextMarshaler interface +// MarshalText implements the encoding.TextMarshaler interface. func (r *Rat) MarshalText() (text []byte, err error) { return []byte(r.RatString()), nil } -// UnmarshalText implements the encoding.TextUnmarshaler interface +// UnmarshalText implements the encoding.TextUnmarshaler interface. func (r *Rat) UnmarshalText(text []byte) error { if _, ok := r.SetString(string(text)); !ok { return fmt.Errorf("math/big: cannot unmarshal %q into a *big.Rat", text) -- cgit v1.2.1 From 1b7392444183c40afb85b6e20aa5f3b4a2112f27 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Tue, 7 Oct 2014 16:27:40 -0400 Subject: runtime: fix _cgo_allocate(0) Fixes a SWIG bug reported off-list. LGTM=iant R=iant CC=golang-codereviews https://codereview.appspot.com/155990043 --- src/runtime/cgocallback.go | 3 +++ 1 file changed, 3 insertions(+) (limited to 'src') diff --git a/src/runtime/cgocallback.go b/src/runtime/cgocallback.go index 1e1b57607..2c8914320 100644 --- a/src/runtime/cgocallback.go +++ b/src/runtime/cgocallback.go @@ -21,6 +21,9 @@ import "unsafe" // Either we need to add types or we need to stop using it. func _cgo_allocate_internal(len uintptr) unsafe.Pointer { + if len == 0 { + len = 1 + } ret := unsafe.Pointer(&make([]unsafe.Pointer, (len+ptrSize-1)/ptrSize)[0]) c := new(cgomal) c.alloc = ret -- cgit v1.2.1 From e158ff3646c84316ba436d4bd7a7bbe6ff1fbee4 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Tue, 7 Oct 2014 13:36:16 -0700 Subject: runtime: update heap dump format for 1.4 We no longer have full type information in the heap, so we can't dump that any more. Instead we dump ptr/noptr maps so at least we can compute graph connectivity. In addition, we still dump Iface/Eface types so together with dwarf type info we might be able to reconstruct types of most things in the heap. LGTM=dvyukov R=golang-codereviews, dvyukov, rsc, khr CC=golang-codereviews https://codereview.appspot.com/155940043 --- src/runtime/heapdump.c | 44 +++++++++++++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 13 deletions(-) (limited to 'src') diff --git a/src/runtime/heapdump.c b/src/runtime/heapdump.c index 54b9666b5..71da419f1 100644 --- a/src/runtime/heapdump.c +++ b/src/runtime/heapdump.c @@ -7,7 +7,7 @@ // finalizers, etc.) to a file. // The format of the dumped file is described at -// http://code.google.com/p/go-wiki/wiki/heapdump13 +// http://code.google.com/p/go-wiki/wiki/heapdump14 #include "runtime.h" #include "arch_GOARCH.h" @@ -27,10 +27,8 @@ extern byte runtime·ebss[]; enum { FieldKindEol = 0, FieldKindPtr = 1, - FieldKindString = 2, - FieldKindSlice = 3, - FieldKindIface = 4, - FieldKindEface = 5, + FieldKindIface = 2, + FieldKindEface = 3, TagEOF = 0, TagObject = 1, @@ -200,7 +198,6 @@ dumptype(Type *t) write(t->x->name->str, t->x->name->len); } dumpbool((t->kind & KindDirectIface) == 0 || (t->kind & KindNoPointers) == 0); - dumpfields((BitVector){0, nil}); } // dump an object @@ -210,9 +207,8 @@ dumpobj(byte *obj, uintptr size, BitVector bv) dumpbvtypes(&bv, obj); dumpint(TagObject); dumpint((uintptr)obj); - dumpint(0); // Type* - dumpint(0); // kind dumpmemrange(obj, size); + dumpfields(bv); } static void @@ -255,6 +251,7 @@ dumpbv(BitVector *bv, uintptr offset) for(i = 0; i < bv->n; i += BitsPerPointer) { switch(bv->bytedata[i/8] >> i%8 & 3) { case BitsDead: + return; case BitsScalar: break; case BitsPointer: @@ -489,16 +486,18 @@ dumproots(void) byte *p; // data segment + dumpbvtypes(&runtime·gcdatamask, runtime·data); dumpint(TagData); dumpint((uintptr)runtime·data); dumpmemrange(runtime·data, runtime·edata - runtime·data); dumpfields(runtime·gcdatamask); // bss segment + dumpbvtypes(&runtime·gcbssmask, runtime·bss); dumpint(TagBss); dumpint((uintptr)runtime·bss); dumpmemrange(runtime·bss, runtime·ebss - runtime·bss); - dumpfields(runtime·gcdatamask); + dumpfields(runtime·gcbssmask); // MSpan.types allspans = runtime·mheap.allspans; @@ -578,10 +577,29 @@ itab_callback(Itab *tab) { Type *t; - dumpint(TagItab); - dumpint((uintptr)tab); t = tab->type; - dumpbool((t->kind & KindDirectIface) == 0 || (t->kind & KindNoPointers) == 0); + // Dump a map from itab* to the type of its data field. + // We want this map so we can deduce types of interface referents. + if((t->kind & KindDirectIface) == 0) { + // indirect - data slot is a pointer to t. + dumptype(t->ptrto); + dumpint(TagItab); + dumpint((uintptr)tab); + dumpint((uintptr)t->ptrto); + } else if((t->kind & KindNoPointers) == 0) { + // t is pointer-like - data slot is a t. + dumptype(t); + dumpint(TagItab); + dumpint((uintptr)tab); + dumpint((uintptr)t); + } else { + // Data slot is a scalar. Dump type just for fun. + // With pointer-only interfaces, this shouldn't happen. + dumptype(t); + dumpint(TagItab); + dumpint((uintptr)tab); + dumpint((uintptr)t); + } } static void @@ -726,7 +744,7 @@ mdump(void) } runtime·memclr((byte*)&typecache[0], sizeof(typecache)); - hdr = (byte*)"go1.3 heap dump\n"; + hdr = (byte*)"go1.4 heap dump\n"; write(hdr, runtime·findnull(hdr)); dumpparams(); dumpitabs(); -- cgit v1.2.1 From fcec67c2f5105f59695f37c32aff2a763fe3beda Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Tue, 7 Oct 2014 15:21:00 -0700 Subject: runtime: zero pointer-looking scalararg values I have a CL which at every gc looks through data and bss sections for nonpointer data (according to gc maps) that looks like a pointer. These are potential missing roots. The only thing it finds are begnign, storing stack pointers into m0.scalararg[1] and never cleaning them up. Let's clean them up now so the test CL passes all.bash cleanly. The test CL can't be checked in because we might store pointer-looking things in nonpointer data by accident. LGTM=iant R=golang-codereviews, iant, khr CC=golang-codereviews https://codereview.appspot.com/153210043 --- src/runtime/panic.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src') diff --git a/src/runtime/panic.c b/src/runtime/panic.c index 1cd0aa865..55ad80e9b 100644 --- a/src/runtime/panic.c +++ b/src/runtime/panic.c @@ -31,6 +31,7 @@ runtime·deferproc_m(void) argp = g->m->scalararg[1]; callerpc = g->m->scalararg[2]; g->m->ptrarg[0] = nil; + g->m->scalararg[1] = 0; d = runtime·newdefer(siz); d->fn = fn; @@ -131,6 +132,7 @@ runtime·dopanic_m(void) g->m->ptrarg[0] = nil; pc = g->m->scalararg[0]; sp = g->m->scalararg[1]; + g->m->scalararg[1] = 0; if(gp->sig != 0) runtime·printf("[signal %x code=%p addr=%p pc=%p]\n", gp->sig, gp->sigcode0, gp->sigcode1, gp->sigpc); -- cgit v1.2.1 From e832252aa6d7444ff054230f6cb116096a509803 Mon Sep 17 00:00:00 2001 From: Andrew Gerrand Date: Wed, 8 Oct 2014 13:22:31 +1100 Subject: cmd/go: add ImportComment to Package struct It seems reasonable that people might want to look up the ImportComment with "go list". LGTM=r R=golang-codereviews, r CC=golang-codereviews https://codereview.appspot.com/143600043 --- src/cmd/go/doc.go | 39 ++++++++++++++++++++------------------- src/cmd/go/list.go | 39 ++++++++++++++++++++------------------- src/cmd/go/pkg.go | 22 ++++++++++++---------- 3 files changed, 52 insertions(+), 48 deletions(-) (limited to 'src') diff --git a/src/cmd/go/doc.go b/src/cmd/go/doc.go index 8e2facd04..314c69bd8 100644 --- a/src/cmd/go/doc.go +++ b/src/cmd/go/doc.go @@ -385,28 +385,29 @@ syntax of package template. The default output is equivalent to -f '{{.ImportPath}}'. The struct being passed to the template is: type Package struct { - Dir string // directory containing package sources - ImportPath string // import path of package in dir - Name string // package name - Doc string // package documentation string - Target string // install path - Goroot bool // is this package in the Go root? - Standard bool // is this package part of the standard Go library? - Stale bool // would 'go install' do anything for this package? - Root string // Go root or Go path dir containing this package + Dir string // directory containing package sources + ImportPath string // import path of package in dir + ImportComment string // path in import comment on package statement + Name string // package name + Doc string // package documentation string + Target string // install path + Goroot bool // is this package in the Go root? + Standard bool // is this package part of the standard Go library? + Stale bool // would 'go install' do anything for this package? + Root string // Go root or Go path dir containing this package // Source files - GoFiles []string // .go source files (excluding CgoFiles, TestGoFiles, XTestGoFiles) - CgoFiles []string // .go sources files that import "C" + GoFiles []string // .go source files (excluding CgoFiles, TestGoFiles, XTestGoFiles) + CgoFiles []string // .go sources files that import "C" IgnoredGoFiles []string // .go sources ignored due to build constraints - CFiles []string // .c source files - CXXFiles []string // .cc, .cxx and .cpp source files - MFiles []string // .m source files - HFiles []string // .h, .hh, .hpp and .hxx source files - SFiles []string // .s source files - SwigFiles []string // .swig files - SwigCXXFiles []string // .swigcxx files - SysoFiles []string // .syso object files to add to archive + CFiles []string // .c source files + CXXFiles []string // .cc, .cxx and .cpp source files + MFiles []string // .m source files + HFiles []string // .h, .hh, .hpp and .hxx source files + SFiles []string // .s source files + SwigFiles []string // .swig files + SwigCXXFiles []string // .swigcxx files + SysoFiles []string // .syso object files to add to archive // Cgo directives CgoCFLAGS []string // cgo: flags for C compiler diff --git a/src/cmd/go/list.go b/src/cmd/go/list.go index 0ead43502..fbf96167f 100644 --- a/src/cmd/go/list.go +++ b/src/cmd/go/list.go @@ -30,28 +30,29 @@ syntax of package template. The default output is equivalent to -f '{{.ImportPath}}'. The struct being passed to the template is: type Package struct { - Dir string // directory containing package sources - ImportPath string // import path of package in dir - Name string // package name - Doc string // package documentation string - Target string // install path - Goroot bool // is this package in the Go root? - Standard bool // is this package part of the standard Go library? - Stale bool // would 'go install' do anything for this package? - Root string // Go root or Go path dir containing this package + Dir string // directory containing package sources + ImportPath string // import path of package in dir + ImportComment string // path in import comment on package statement + Name string // package name + Doc string // package documentation string + Target string // install path + Goroot bool // is this package in the Go root? + Standard bool // is this package part of the standard Go library? + Stale bool // would 'go install' do anything for this package? + Root string // Go root or Go path dir containing this package // Source files - GoFiles []string // .go source files (excluding CgoFiles, TestGoFiles, XTestGoFiles) - CgoFiles []string // .go sources files that import "C" + GoFiles []string // .go source files (excluding CgoFiles, TestGoFiles, XTestGoFiles) + CgoFiles []string // .go sources files that import "C" IgnoredGoFiles []string // .go sources ignored due to build constraints - CFiles []string // .c source files - CXXFiles []string // .cc, .cxx and .cpp source files - MFiles []string // .m source files - HFiles []string // .h, .hh, .hpp and .hxx source files - SFiles []string // .s source files - SwigFiles []string // .swig files - SwigCXXFiles []string // .swigcxx files - SysoFiles []string // .syso object files to add to archive + CFiles []string // .c source files + CXXFiles []string // .cc, .cxx and .cpp source files + MFiles []string // .m source files + HFiles []string // .h, .hh, .hpp and .hxx source files + SFiles []string // .s source files + SwigFiles []string // .swig files + SwigCXXFiles []string // .swigcxx files + SysoFiles []string // .syso object files to add to archive // Cgo directives CgoCFLAGS []string // cgo: flags for C compiler diff --git a/src/cmd/go/pkg.go b/src/cmd/go/pkg.go index 7f7a3b04f..e17326442 100644 --- a/src/cmd/go/pkg.go +++ b/src/cmd/go/pkg.go @@ -26,16 +26,17 @@ type Package struct { // Note: These fields are part of the go command's public API. // See list.go. It is okay to add fields, but not to change or // remove existing ones. Keep in sync with list.go - Dir string `json:",omitempty"` // directory containing package sources - ImportPath string `json:",omitempty"` // import path of package in dir - Name string `json:",omitempty"` // package name - Doc string `json:",omitempty"` // package documentation string - Target string `json:",omitempty"` // install path - Goroot bool `json:",omitempty"` // is this package found in the Go root? - Standard bool `json:",omitempty"` // is this package part of the standard Go library? - Stale bool `json:",omitempty"` // would 'go install' do anything for this package? - Root string `json:",omitempty"` // Go root or Go path dir containing this package - ConflictDir string `json:",omitempty"` // Dir is hidden by this other directory + Dir string `json:",omitempty"` // directory containing package sources + ImportPath string `json:",omitempty"` // import path of package in dir + ImportComment string `json:",omitempty"` // path in import comment on package statement + Name string `json:",omitempty"` // package name + Doc string `json:",omitempty"` // package documentation string + Target string `json:",omitempty"` // install path + Goroot bool `json:",omitempty"` // is this package found in the Go root? + Standard bool `json:",omitempty"` // is this package part of the standard Go library? + Stale bool `json:",omitempty"` // would 'go install' do anything for this package? + Root string `json:",omitempty"` // Go root or Go path dir containing this package + ConflictDir string `json:",omitempty"` // Dir is hidden by this other directory // Source files GoFiles []string `json:",omitempty"` // .go source files (excluding CgoFiles, TestGoFiles, XTestGoFiles) @@ -104,6 +105,7 @@ func (p *Package) copyBuild(pp *build.Package) { p.Dir = pp.Dir p.ImportPath = pp.ImportPath + p.ImportComment = pp.ImportComment p.Name = pp.Name p.Doc = pp.Doc p.Root = pp.Root -- cgit v1.2.1 From bdd9c65ccff925967ece8a9141d3eae677104428 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Tue, 7 Oct 2014 23:08:07 -0400 Subject: net/rpc: listen on localhost, let kernel pick port This avoids a pop-up box on OS X and it avoids a test failure if something is using 5555. I apologize for not noticing this during the review. TBR=r CC=golang-codereviews https://codereview.appspot.com/152320044 --- src/net/rpc/client_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/net/rpc/client_test.go b/src/net/rpc/client_test.go index c138c06b8..d116d2acc 100644 --- a/src/net/rpc/client_test.go +++ b/src/net/rpc/client_test.go @@ -63,13 +63,13 @@ func TestGobError(t *testing.T) { }() Register(new(S)) - listen, err := net.Listen("tcp", ":5555") + listen, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { panic(err) } go Accept(listen) - client, err := Dial("tcp", ":5555") + client, err := Dial("tcp", listen.Addr().String()) if err != nil { panic(err) } -- cgit v1.2.1 From 9673ab4a490e582ee5a8dc4c91eaadbac48b215c Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Tue, 7 Oct 2014 23:17:31 -0400 Subject: runtime: clear Defer.panic before removing from G.defer list Another dangling stack pointer in a cached structure. Same as SudoG.elem and SudoG.selectdone. Definitely a fix, and the new test in freedefer makes the crash reproducible, but probably not a complete fix. I have seen one dangling pointer in a Defer.panic even after this fix; I cannot see where it could be coming from. I think this will fix the solaris build. I do not think this will fix the occasional failure on the darwin build. TBR=khr R=khr CC=golang-codereviews https://codereview.appspot.com/155080043 --- src/runtime/panic.c | 2 ++ src/runtime/panic.go | 8 ++++++++ 2 files changed, 10 insertions(+) (limited to 'src') diff --git a/src/runtime/panic.c b/src/runtime/panic.c index 55ad80e9b..24eb6dbfe 100644 --- a/src/runtime/panic.c +++ b/src/runtime/panic.c @@ -34,6 +34,8 @@ runtime·deferproc_m(void) g->m->scalararg[1] = 0; d = runtime·newdefer(siz); + if(d->panic != nil) + runtime·throw("deferproc: d->panic != nil after newdefer"); d->fn = fn; d->pc = callerpc; d->argp = argp; diff --git a/src/runtime/panic.go b/src/runtime/panic.go index 7eb2d6055..c78102f8a 100644 --- a/src/runtime/panic.go +++ b/src/runtime/panic.go @@ -188,6 +188,10 @@ func newdefer(siz int32) *_defer { // The defer cannot be used after this call. //go:nosplit func freedefer(d *_defer) { + if d._panic != nil { + // _panic must be cleared before d is unlinked from gp. + gothrow("freedefer with d._panic != nil") + } sc := deferclass(uintptr(d.siz)) if sc < uintptr(len(p{}.deferpool)) { mp := acquirem() @@ -258,6 +262,7 @@ func Goexit() { if d.started { if d._panic != nil { d._panic.aborted = true + d._panic = nil } gp._defer = d.link freedefer(d) @@ -268,6 +273,7 @@ func Goexit() { if gp._defer != d { gothrow("bad defer entry in Goexit") } + d._panic = nil gp._defer = d.link freedefer(d) // Note: we ignore recovers here because Goexit isn't a panic @@ -343,6 +349,7 @@ func gopanic(e interface{}) { if d._panic != nil { d._panic.aborted = true } + d._panic = nil gp._defer = d.link freedefer(d) continue @@ -366,6 +373,7 @@ func gopanic(e interface{}) { if gp._defer != d { gothrow("bad defer entry in panic") } + d._panic = nil gp._defer = d.link // trigger shrinkage to test stack copy. See stack_test.go:TestStackPanic -- cgit v1.2.1 From 5d002841f887436edc2b238694f04caaa696cae4 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Tue, 7 Oct 2014 23:27:25 -0400 Subject: runtime: change Windows M.thread from void* to uintptr It appears to be an opaque bit pattern more than a pointer. The Go garbage collector has discovered that for m0 it is set to 0x4c. Should fix Windows build. TBR=brainman CC=golang-codereviews https://codereview.appspot.com/149640043 --- src/runtime/asm_386.s | 3 +++ src/runtime/asm_amd64.s | 3 +++ src/runtime/asm_amd64p32.s | 3 +++ src/runtime/asm_arm.s | 3 +++ src/runtime/os_windows.c | 14 +++++++------- src/runtime/runtime.h | 4 +++- 6 files changed, 22 insertions(+), 8 deletions(-) (limited to 'src') diff --git a/src/runtime/asm_386.s b/src/runtime/asm_386.s index 1495246a2..c401741ef 100644 --- a/src/runtime/asm_386.s +++ b/src/runtime/asm_386.s @@ -479,6 +479,9 @@ TEXT runtime·atomicloaduintptr(SB), NOSPLIT, $0-8 TEXT runtime·atomicloaduint(SB), NOSPLIT, $0-8 JMP runtime·atomicload(SB) +TEXT runtime·atomicstoreuintptr(SB), NOSPLIT, $0-8 + JMP runtime·atomicstore(SB) + // bool runtime·cas64(uint64 *val, uint64 old, uint64 new) // Atomically: // if(*val == *old){ diff --git a/src/runtime/asm_amd64.s b/src/runtime/asm_amd64.s index 3f7f60841..e21270d8c 100644 --- a/src/runtime/asm_amd64.s +++ b/src/runtime/asm_amd64.s @@ -491,6 +491,9 @@ TEXT runtime·atomicloaduintptr(SB), NOSPLIT, $0-16 TEXT runtime·atomicloaduint(SB), NOSPLIT, $0-16 JMP runtime·atomicload64(SB) +TEXT runtime·atomicstoreuintptr(SB), NOSPLIT, $0-16 + JMP runtime·atomicstore64(SB) + // bool casp(void **val, void *old, void *new) // Atomically: // if(*val == old){ diff --git a/src/runtime/asm_amd64p32.s b/src/runtime/asm_amd64p32.s index 13a164256..c2bc91a3f 100644 --- a/src/runtime/asm_amd64p32.s +++ b/src/runtime/asm_amd64p32.s @@ -440,6 +440,9 @@ TEXT runtime·atomicloaduintptr(SB), NOSPLIT, $0-12 TEXT runtime·atomicloaduint(SB), NOSPLIT, $0-12 JMP runtime·atomicload(SB) +TEXT runtime·atomicstoreuintptr(SB), NOSPLIT, $0-12 + JMP runtime·atomicstore(SB) + // bool runtime·cas64(uint64 *val, uint64 old, uint64 new) // Atomically: // if(*val == *old){ diff --git a/src/runtime/asm_arm.s b/src/runtime/asm_arm.s index 36fb022f9..a1535aeec 100644 --- a/src/runtime/asm_arm.s +++ b/src/runtime/asm_arm.s @@ -724,6 +724,9 @@ TEXT runtime·atomicloaduintptr(SB),NOSPLIT,$0-8 TEXT runtime·atomicloaduint(SB),NOSPLIT,$0-8 B runtime·atomicload(SB) +TEXT runtime·atomicstoreuintptr(SB),NOSPLIT,$0-8 + B runtime·atomicstore(SB) + // AES hashing not implemented for ARM TEXT runtime·aeshash(SB),NOSPLIT,$-4-0 MOVW $0, R0 diff --git a/src/runtime/os_windows.c b/src/runtime/os_windows.c index 77f99062c..6337dde2a 100644 --- a/src/runtime/os_windows.c +++ b/src/runtime/os_windows.c @@ -268,19 +268,19 @@ runtime·mpreinit(M *mp) void runtime·minit(void) { - void *thandle; + uintptr thandle; // -1 = current process, -2 = current thread runtime·stdcall7(runtime·DuplicateHandle, -1, -2, -1, (uintptr)&thandle, 0, 0, DUPLICATE_SAME_ACCESS); - runtime·atomicstorep(&g->m->thread, thandle); + runtime·atomicstoreuintptr(&g->m->thread, thandle); } // Called from dropm to undo the effect of an minit. void runtime·unminit(void) { - runtime·stdcall1(runtime·CloseHandle, (uintptr)g->m->thread); - g->m->thread = nil; + runtime·stdcall1(runtime·CloseHandle, g->m->thread); + g->m->thread = 0; } // Described in http://www.dcl.hpi.uni-potsdam.de/research/WRK/2007/08/getting-os-information-the-kuser_shared_data-structure/ @@ -532,7 +532,7 @@ void runtime·profileloop1(void) { M *mp, *allm; - void *thread; + uintptr thread; runtime·stdcall2(runtime·SetThreadPriority, -2, THREAD_PRIORITY_HIGHEST); @@ -540,11 +540,11 @@ runtime·profileloop1(void) runtime·stdcall2(runtime·WaitForSingleObject, (uintptr)profiletimer, -1); allm = runtime·atomicloadp(&runtime·allm); for(mp = allm; mp != nil; mp = mp->alllink) { - thread = runtime·atomicloadp(&mp->thread); + thread = runtime·atomicloaduintptr(&mp->thread); // Do not profile threads blocked on Notes, // this includes idle worker threads, // idle timer thread, idle heap scavenger, etc. - if(thread == nil || mp->profilehz == 0 || mp->blocked) + if(thread == 0 || mp->profilehz == 0 || mp->blocked) continue; runtime·stdcall1(runtime·SuspendThread, (uintptr)thread); if(mp->profilehz != 0 && !mp->blocked) diff --git a/src/runtime/runtime.h b/src/runtime/runtime.h index c4d878608..27a809a07 100644 --- a/src/runtime/runtime.h +++ b/src/runtime/runtime.h @@ -371,7 +371,7 @@ struct M uintptr scalararg[4]; // scalar argument/return for mcall void* ptrarg[4]; // pointer argument/return for mcall #ifdef GOOS_windows - void* thread; // thread handle + uintptr thread; // thread handle // these are here because they are too large to be on the stack // of low-level NOSPLIT functions. LibCall libcall; @@ -885,7 +885,9 @@ void runtime·atomicstore(uint32 volatile*, uint32); void runtime·atomicstore64(uint64 volatile*, uint64); uint64 runtime·atomicload64(uint64 volatile*); void* runtime·atomicloadp(void* volatile*); +uintptr runtime·atomicloaduintptr(uintptr volatile*); void runtime·atomicstorep(void* volatile*, void*); +void runtime·atomicstoreuintptr(uintptr volatile*, uintptr); void runtime·atomicor8(byte volatile*, byte); void runtime·setg(G*); -- cgit v1.2.1 From e5f51cfff0a86f886b85e4e489217d80f65c56c0 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Tue, 7 Oct 2014 23:39:00 -0400 Subject: runtime: fix windows/amd64 build Out of stack space due to new 2-word call in freedefer. Go back to smaller function calls. TBR=brainman CC=golang-codereviews https://codereview.appspot.com/152340043 --- src/runtime/panic.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/runtime/panic.go b/src/runtime/panic.go index c78102f8a..58b14b09e 100644 --- a/src/runtime/panic.go +++ b/src/runtime/panic.go @@ -189,8 +189,7 @@ func newdefer(siz int32) *_defer { //go:nosplit func freedefer(d *_defer) { if d._panic != nil { - // _panic must be cleared before d is unlinked from gp. - gothrow("freedefer with d._panic != nil") + freedeferpanic() } sc := deferclass(uintptr(d.siz)) if sc < uintptr(len(p{}.deferpool)) { @@ -203,6 +202,13 @@ func freedefer(d *_defer) { } } +// Separate function so that it can split stack. +// Windows otherwise runs out of stack space. +func freedeferpanic() { + // _panic must be cleared before d is unlinked from gp. + gothrow("freedefer with d._panic != nil") +} + // Run a deferred function if there is one. // The compiler inserts a call to this at the end of any // function which calls defer. -- cgit v1.2.1 From fbad28ce50204ad5d57b0e04543030e3e6b63970 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Wed, 8 Oct 2014 00:03:50 -0400 Subject: runtime: clear Defer.fn before removing from the G.defer list Should fix the remaining 'invalid heap pointer' build failures. TBR=khr CC=golang-codereviews https://codereview.appspot.com/152360043 --- src/runtime/panic.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'src') diff --git a/src/runtime/panic.go b/src/runtime/panic.go index 58b14b09e..685ff5ca0 100644 --- a/src/runtime/panic.go +++ b/src/runtime/panic.go @@ -191,6 +191,9 @@ func freedefer(d *_defer) { if d._panic != nil { freedeferpanic() } + if d.fn != nil { + freedeferfn() + } sc := deferclass(uintptr(d.siz)) if sc < uintptr(len(p{}.deferpool)) { mp := acquirem() @@ -209,6 +212,11 @@ func freedeferpanic() { gothrow("freedefer with d._panic != nil") } +func freedeferfn() { + // fn must be cleared before d is unlinked from gp. + gothrow("freedefer with d.fn != nil") +} + // Run a deferred function if there is one. // The compiler inserts a call to this at the end of any // function which calls defer. @@ -241,6 +249,7 @@ func deferreturn(arg0 uintptr) { mp := acquirem() memmove(unsafe.Pointer(argp), deferArgs(d), uintptr(d.siz)) fn := d.fn + d.fn = nil gp._defer = d.link freedefer(d) releasem(mp) @@ -270,6 +279,7 @@ func Goexit() { d._panic.aborted = true d._panic = nil } + d.fn = nil gp._defer = d.link freedefer(d) continue @@ -280,6 +290,7 @@ func Goexit() { gothrow("bad defer entry in Goexit") } d._panic = nil + d.fn = nil gp._defer = d.link freedefer(d) // Note: we ignore recovers here because Goexit isn't a panic @@ -356,6 +367,7 @@ func gopanic(e interface{}) { d._panic.aborted = true } d._panic = nil + d.fn = nil gp._defer = d.link freedefer(d) continue @@ -380,6 +392,7 @@ func gopanic(e interface{}) { gothrow("bad defer entry in panic") } d._panic = nil + d.fn = nil gp._defer = d.link // trigger shrinkage to test stack copy. See stack_test.go:TestStackPanic -- cgit v1.2.1 From 756b2d33e6d87ebb7bcf44b3bb4f25d137682e8d Mon Sep 17 00:00:00 2001 From: Dmitriy Vyukov Date: Wed, 8 Oct 2014 13:51:12 +0400 Subject: runtime: faster GC scan The change contains 3 spot optimizations to scan loop: 1. Don't use byte vars, use uintptr's instead. This seems to alleviate some codegen issue, and alone accounts to a half of speedup. 2. Remove bitmap cache. Currently we cache only 1 byte, so caching is not particularly effective anyway. Removal of the cache simplifies code and positively affects regalloc. 3. Replace BitsMultiword switch with if and do debug checks only in Debug mode. I've benchmarked changes separately and ensured that each of them provides speedup on top of the previous one. This change as a whole fixes the unintentional regressions of scan loop that were introduced during development cycle. Fixes issue 8625. Fixes issue 8565. On go.benchmarks/garbage benchmark: GOMAXPROCS=1 time: -3.13% cputime: -3.22% gc-pause-one: -15.71% gc-pause-total: -15.71% GOMAXPROCS=32 time: -1.96% cputime: -4.43% gc-pause-one: -6.22% gc-pause-total: -6.22% LGTM=khr, rsc R=golang-codereviews, khr CC=golang-codereviews, rlh, rsc https://codereview.appspot.com/153990043 --- src/runtime/mgc0.c | 50 ++++++++++++++++++-------------------------------- 1 file changed, 18 insertions(+), 32 deletions(-) (limited to 'src') diff --git a/src/runtime/mgc0.c b/src/runtime/mgc0.c index 5876ea5c3..e369e5425 100644 --- a/src/runtime/mgc0.c +++ b/src/runtime/mgc0.c @@ -179,9 +179,8 @@ have_cgo_allocate(void) static void scanblock(byte *b, uintptr n, byte *ptrmask) { - byte *obj, *obj0, *p, *arena_start, *arena_used, **wp, *scanbuf[8], *ptrbitp, *bitp, bits, xbits, shift, cached; - uintptr i, j, nobj, size, idx, x, off, scanbufpos; - intptr ncached; + byte *obj, *obj0, *p, *arena_start, *arena_used, **wp, *scanbuf[8], *ptrbitp, *bitp; + uintptr i, j, nobj, size, idx, x, off, scanbufpos, bits, xbits, shift; Workbuf *wbuf; Iface *iface; Eface *eface; @@ -203,8 +202,6 @@ scanblock(byte *b, uintptr n, byte *ptrmask) scanbuf[i] = nil; ptrbitp = nil; - cached = 0; - ncached = 0; // ptrmask can have 2 possible values: // 1. nil - obtain pointer mask from GC bitmap. @@ -259,10 +256,6 @@ scanblock(byte *b, uintptr n, byte *ptrmask) if(ptrmask == nil) { off = (uintptr*)b - (uintptr*)arena_start; ptrbitp = arena_start - off/wordsPerBitmapByte - 1; - shift = (off % wordsPerBitmapByte) * gcBits; - cached = *ptrbitp >> shift; - cached &= ~bitBoundary; - ncached = (8 - shift)/gcBits; } for(i = 0; i < n; i += PtrSize) { obj = nil; @@ -273,15 +266,12 @@ scanblock(byte *b, uintptr n, byte *ptrmask) runtime·mheap.spans[(b-arena_start)>>PageShift] != runtime·mheap.spans[(b+i-arena_start)>>PageShift]) break; // Consult GC bitmap. - if(ncached <= 0) { - // Refill cache. - cached = *--ptrbitp; - ncached = 2; + bits = *ptrbitp; + if((((uintptr)b+i)%(PtrSize*wordsPerBitmapByte)) != 0) { + ptrbitp--; + bits >>= gcBits; } - bits = cached; - cached >>= gcBits; - ncached--; - if((bits&bitBoundary) != 0) + if((bits&bitBoundary) != 0 && i != 0) break; // reached beginning of the next object bits = (bits>>2)&BitsMask; if(bits == BitsDead) @@ -289,7 +279,7 @@ scanblock(byte *b, uintptr n, byte *ptrmask) } else // dense mask (stack or data) bits = (ptrmask[(i/PtrSize)/4]>>(((i/PtrSize)%4)*BitsPerPointer))&BitsMask; - if(bits == BitsScalar || bits == BitsDead) + if(bits <= BitsScalar) // BitsScalar || BitsDead continue; if(bits == BitsPointer) { obj = *(byte**)(b+i); @@ -298,43 +288,39 @@ scanblock(byte *b, uintptr n, byte *ptrmask) } // With those three out of the way, must be multi-word. - if(bits != BitsMultiWord) + if(Debug && bits != BitsMultiWord) runtime·throw("unexpected garbage collection bits"); // Find the next pair of bits. if(ptrmask == nil) { - if(ncached <= 0) { - // Refill cache. - cached = *--ptrbitp; - ncached = 2; + bits = *ptrbitp; + if((((uintptr)b+i)%(PtrSize*wordsPerBitmapByte)) == 0) { + ptrbitp--; + bits >>= gcBits; } - bits = (cached>>2)&BitsMask; + bits = (bits>>2)&BitsMask; } else bits = (ptrmask[((i+PtrSize)/PtrSize)/4]>>((((i+PtrSize)/PtrSize)%4)*BitsPerPointer))&BitsMask; - switch(bits) { - default: + if(Debug && bits != BitsIface && bits != BitsEface) runtime·throw("unexpected garbage collection bits"); - case BitsIface: + + if(bits == BitsIface) { iface = (Iface*)(b+i); if(iface->tab != nil) { typ = iface->tab->type; if(!(typ->kind&KindDirectIface) || !(typ->kind&KindNoPointers)) obj = iface->data; } - break; - case BitsEface: + } else { eface = (Eface*)(b+i); typ = eface->type; if(typ != nil) { if(!(typ->kind&KindDirectIface) || !(typ->kind&KindNoPointers)) obj = eface->data; } - break; } i += PtrSize; - cached >>= gcBits; - ncached--; obj0 = obj; markobj: -- cgit v1.2.1 From d5169fcdbca1592b8c6c7565c6b7f0e85f5deef2 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Wed, 8 Oct 2014 15:48:46 -0700 Subject: reflect: add tests for variadic method calls These tests fail when using gccgo. In gccgo using Interface on the value of a method function is implemented using a variant of MakeFunc. That approach did not correctly handle variadic functions. LGTM=r R=golang-codereviews, r CC=golang-codereviews https://codereview.appspot.com/151280043 --- src/reflect/all_test.go | 54 ++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 49 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/reflect/all_test.go b/src/reflect/all_test.go index d17ef5c5e..f13b91b74 100644 --- a/src/reflect/all_test.go +++ b/src/reflect/all_test.go @@ -1569,6 +1569,24 @@ func (p Point) Dist(scale int) int { return p.x*p.x*scale + p.y*p.y*scale } +// This will be index 2. +func (p Point) GCMethod(k int) int { + runtime.GC() + return k + p.x +} + +// This will be index 3. +func (p Point) TotalDist(points ...Point) int { + tot := 0 + for _, q := range points { + dx := q.x - p.x + dy := q.y - p.y + tot += dx*dx + dy*dy // Should call Sqrt, but it's just a test. + + } + return tot +} + func TestMethod(t *testing.T) { // Non-curried method of type. p := Point{3, 4} @@ -1751,6 +1769,37 @@ func TestMethodValue(t *testing.T) { } } +func TestVariadicMethodValue(t *testing.T) { + p := Point{3, 4} + points := []Point{{20, 21}, {22, 23}, {24, 25}} + want := int64(p.TotalDist(points[0], points[1], points[2])) + + // Curried method of value. + tfunc := TypeOf((func(...Point) int)(nil)) + v := ValueOf(p).Method(3) + if tt := v.Type(); tt != tfunc { + t.Errorf("Variadic Method Type is %s; want %s", tt, tfunc) + } + i := ValueOf(v.Interface()).Call([]Value{ValueOf(points[0]), ValueOf(points[1]), ValueOf(points[2])})[0].Int() + if i != want { + t.Errorf("Variadic Method returned %d; want %d", i, want) + } + i = ValueOf(v.Interface()).CallSlice([]Value{ValueOf(points)})[0].Int() + if i != want { + t.Errorf("Variadic Method CallSlice returned %d; want %d", i, want) + } + + f := v.Interface().(func(...Point) int) + i = int64(f(points[0], points[1], points[2])) + if i != want { + t.Errorf("Variadic Method Interface returned %d; want %d", i, want) + } + i = int64(f(points...)) + if i != want { + t.Errorf("Variadic Method Interface Slice returned %d; want %d", i, want) + } +} + // Reflect version of $GOROOT/test/method5.go // Concrete types implementing M method. @@ -3770,11 +3819,6 @@ func TestReflectFuncTraceback(t *testing.T) { f.Call([]Value{}) } -func (p Point) GCMethod(k int) int { - runtime.GC() - return k + p.x -} - func TestReflectMethodTraceback(t *testing.T) { p := Point{3, 4} m := ValueOf(p).MethodByName("GCMethod") -- cgit v1.2.1 From 68bc155352275d8e292e8c8c8b3cff1c24492d08 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Wed, 8 Oct 2014 15:57:20 -0700 Subject: runtime: delay freeing of shrunk stacks until gc is done. This change prevents confusion in the garbage collector. The collector wants to make sure that every pointer it finds isn't junk. Its criteria for junk is (among others) points to a "free" span. Because the stack shrinker modifies pointers in the heap, there is a race condition between the GC scanner and the shrinker. The GC scanner can see old pointers (pointers to freed stacks). In particular this happens with SudoG.elem pointers. Normally this is not a problem, as pointers into stack spans are ok. But if the freed stack is the last one in its span, the span is marked as "free" instead of "contains stacks". This change makes sure that even if the GC scanner sees an old pointer, the span into which it points is still marked as "contains stacks", and thus the GC doesn't complain about it. This change will make the GC pause a tiny bit slower, as the stack freeing now happens in serial with the mark pause. We could delay the freeing until the mutators start back up, but this is the simplest change for now. TBR=dvyukov CC=golang-codereviews https://codereview.appspot.com/158750043 --- src/runtime/mgc0.c | 2 ++ src/runtime/runtime.h | 1 + src/runtime/stack.c | 38 +++++++++++++++++++++++++++++++++++++- 3 files changed, 40 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/runtime/mgc0.c b/src/runtime/mgc0.c index e369e5425..0de7b1bf4 100644 --- a/src/runtime/mgc0.c +++ b/src/runtime/mgc0.c @@ -1445,6 +1445,8 @@ gc(struct gc_args *args) if(runtime·work.nproc > 1) runtime·notesleep(&runtime·work.alldone); + runtime·shrinkfinish(); + cachestats(); // next_gc calculation is tricky with concurrent sweep since we don't know size of live heap // estimate what was live heap size after previous GC (for tracing only) diff --git a/src/runtime/runtime.h b/src/runtime/runtime.h index 27a809a07..a84a32525 100644 --- a/src/runtime/runtime.h +++ b/src/runtime/runtime.h @@ -852,6 +852,7 @@ void runtime·stackinit(void); Stack runtime·stackalloc(uint32); void runtime·stackfree(Stack); void runtime·shrinkstack(G*); +void runtime·shrinkfinish(void); MCache* runtime·allocmcache(void); void runtime·freemcache(MCache*); void runtime·mallocinit(void); diff --git a/src/runtime/stack.c b/src/runtime/stack.c index 8562b9407..d1ea3ff73 100644 --- a/src/runtime/stack.c +++ b/src/runtime/stack.c @@ -36,6 +36,8 @@ MSpan runtime·stackpool[NumStackOrders]; Mutex runtime·stackpoolmu; // TODO: one lock per order? +static Stack stackfreequeue; + void runtime·stackinit(void) { @@ -656,7 +658,24 @@ copystack(G *gp, uintptr newsize) while(p < ep) *p++ = 0xfc; } - runtime·stackfree(old); + if(newsize > old.hi-old.lo) { + // growing, free stack immediately + runtime·stackfree(old); + } else { + // shrinking, queue up free operation. We can't actually free the stack + // just yet because we might run into the following situation: + // 1) GC starts, scans a SudoG but does not yet mark the SudoG.elem pointer + // 2) The stack that pointer points to is shrunk + // 3) The old stack is freed + // 4) The containing span is marked free + // 5) GC attempts to mark the SudoG.elem pointer. The marking fails because + // the pointer looks like a pointer into a free span. + // By not freeing, we prevent step #4 until GC is done. + runtime·lock(&runtime·stackpoolmu); + *(Stack*)old.lo = stackfreequeue; + stackfreequeue = old; + runtime·unlock(&runtime·stackpoolmu); + } } // round x up to a power of 2. @@ -841,6 +860,23 @@ runtime·shrinkstack(G *gp) copystack(gp, newsize); } +// Do any delayed stack freeing that was queued up during GC. +void +runtime·shrinkfinish(void) +{ + Stack s, t; + + runtime·lock(&runtime·stackpoolmu); + s = stackfreequeue; + stackfreequeue = (Stack){0,0}; + runtime·unlock(&runtime·stackpoolmu); + while(s.lo != 0) { + t = *(Stack*)s.lo; + runtime·stackfree(s); + s = t; + } +} + static void badc(void); #pragma textflag NOSPLIT -- cgit v1.2.1 From ff4dfbe28e0119e5759888401ee3d837082e1479 Mon Sep 17 00:00:00 2001 From: Michael Hudson-Doyle Date: Wed, 8 Oct 2014 15:58:56 -0700 Subject: reflect: add direct call tests to TestMakeFuncVariadic TestMakeFuncVariadic only called the variadic function via Call and CallSlice, not via a direct function call. I thought these tests would fail under gccgo tip, but they don't. Still seems worth having though. LGTM=iant R=golang-codereviews, gobot, iant CC=golang-codereviews https://codereview.appspot.com/152060043 Committer: Ian Lance Taylor --- src/reflect/all_test.go | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/reflect/all_test.go b/src/reflect/all_test.go index f13b91b74..f0cd6a412 100644 --- a/src/reflect/all_test.go +++ b/src/reflect/all_test.go @@ -1543,7 +1543,17 @@ func TestMakeFuncVariadic(t *testing.T) { fv := MakeFunc(TypeOf(fn), func(in []Value) []Value { return in[1:2] }) ValueOf(&fn).Elem().Set(fv) - r := fv.Call([]Value{ValueOf(1), ValueOf(2), ValueOf(3)})[0].Interface().([]int) + r := fn(1, 2, 3) + if r[0] != 2 || r[1] != 3 { + t.Errorf("Call returned [%v, %v]; want 2, 3", r[0], r[1]) + } + + r = fn(1, []int{2, 3}...) + if r[0] != 2 || r[1] != 3 { + t.Errorf("Call returned [%v, %v]; want 2, 3", r[0], r[1]) + } + + r = fv.Call([]Value{ValueOf(1), ValueOf(2), ValueOf(3)})[0].Interface().([]int) if r[0] != 2 || r[1] != 3 { t.Errorf("Call returned [%v, %v]; want 2, 3", r[0], r[1]) } @@ -1552,6 +1562,17 @@ func TestMakeFuncVariadic(t *testing.T) { if r[0] != 2 || r[1] != 3 { t.Errorf("Call returned [%v, %v]; want 2, 3", r[0], r[1]) } + + f := fv.Interface().(func(int, ...int) []int) + + r = f(1, 2, 3) + if r[0] != 2 || r[1] != 3 { + t.Errorf("Call returned [%v, %v]; want 2, 3", r[0], r[1]) + } + r = f(1, []int{2, 3}...) + if r[0] != 2 || r[1] != 3 { + t.Errorf("Call returned [%v, %v]; want 2, 3", r[0], r[1]) + } } type Point struct { -- cgit v1.2.1 From 0bd49e6ad4b3c9b0cf207869c597517023060ff1 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Wed, 8 Oct 2014 16:17:34 -0700 Subject: cmd/ld: don't add line number info for the final address of an FDE This makes dwardump --verify happy. Update issue 8846 LGTM=r R=golang-codereviews, r CC=golang-codereviews https://codereview.appspot.com/150370043 --- src/cmd/ld/dwarf.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/cmd/ld/dwarf.c b/src/cmd/ld/dwarf.c index 4efb0ed53..a3ba52325 100644 --- a/src/cmd/ld/dwarf.c +++ b/src/cmd/ld/dwarf.c @@ -1733,6 +1733,7 @@ writeframes(void) LSym *s; vlong fdeo, fdesize, pad; Pciter pcsp; + uint32 nextpc; if(framesec == S) framesec = linklookup(ctxt, ".dwarfframe", 0); @@ -1775,8 +1776,17 @@ writeframes(void) addrput(0); // initial location addrput(0); // address range - for(pciterinit(ctxt, &pcsp, &s->pcln->pcsp); !pcsp.done; pciternext(&pcsp)) - putpccfadelta(pcsp.nextpc - pcsp.pc, PtrSize + pcsp.value); + for(pciterinit(ctxt, &pcsp, &s->pcln->pcsp); !pcsp.done; pciternext(&pcsp)) { + nextpc = pcsp.nextpc; + // pciterinit goes up to the end of the function, + // but DWARF expects us to stop just before the end. + if(nextpc == s->size) { + nextpc--; + if(nextpc < pcsp.pc) + continue; + } + putpccfadelta(nextpc - pcsp.pc, PtrSize + pcsp.value); + } fdesize = cpos() - fdeo - 4; // exclude the length field. pad = rnd(fdesize, PtrSize) - fdesize; -- cgit v1.2.1 From 91473a63744d6fecd732be6cee30cbb2be3bb5c5 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Wed, 8 Oct 2014 17:22:34 -0700 Subject: runtime: zero a few more dead pointers. In channels, zeroing of gp.waiting is missed on a closed channel panic. m.morebuf.g is not zeroed. I don't expect the latter causes any problems, but just in case. LGTM=iant R=golang-codereviews, iant CC=golang-codereviews https://codereview.appspot.com/151610043 --- src/runtime/chan.go | 11 +++++++---- src/runtime/stack.c | 1 + 2 files changed, 8 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/runtime/chan.go b/src/runtime/chan.go index 10503f4e1..004970182 100644 --- a/src/runtime/chan.go +++ b/src/runtime/chan.go @@ -174,6 +174,10 @@ func chansend(t *chantype, c *hchan, ep unsafe.Pointer, block bool, callerpc uin goparkunlock(&c.lock, "chan send") // someone woke us up. + if mysg != gp.waiting { + gothrow("G waiting list is corrupted!") + } + gp.waiting = nil if gp.param == nil { if c.closed == 0 { gothrow("chansend: spurious wakeup") @@ -184,10 +188,6 @@ func chansend(t *chantype, c *hchan, ep unsafe.Pointer, block bool, callerpc uin if mysg.releasetime > 0 { blockevent(int64(mysg.releasetime)-t0, 2) } - if mysg != gp.waiting { - gothrow("G waiting list is corrupted!") - } - gp.waiting = nil releaseSudog(mysg) return true } @@ -410,6 +410,9 @@ func chanrecv(t *chantype, c *hchan, ep unsafe.Pointer, block bool) (selected, r goparkunlock(&c.lock, "chan receive") // someone woke us up + if mysg != gp.waiting { + gothrow("G waiting list is corrupted!") + } gp.waiting = nil if mysg.releasetime > 0 { blockevent(mysg.releasetime-t0, 2) diff --git a/src/runtime/stack.c b/src/runtime/stack.c index d1ea3ff73..e402691f4 100644 --- a/src/runtime/stack.c +++ b/src/runtime/stack.c @@ -725,6 +725,7 @@ runtime·newstack(void) g->m->morebuf.pc = (uintptr)nil; g->m->morebuf.lr = (uintptr)nil; g->m->morebuf.sp = (uintptr)nil; + g->m->morebuf.g = (G*)nil; runtime·casgstatus(gp, Grunning, Gwaiting); gp->waitreason = runtime·gostringnocopy((byte*)"stack growth"); -- cgit v1.2.1 From 01fd182471a4a06c5bdfb3184c4559fb8929bfd3 Mon Sep 17 00:00:00 2001 From: Alex Brainman Date: Thu, 9 Oct 2014 16:52:28 +1100 Subject: runtime: handle all windows exception Fixes issue 8006. LGTM=rsc R=golang-codereviews, rsc CC=golang-codereviews https://codereview.appspot.com/145150043 --- src/runtime/defs_windows.go | 3 ++ src/runtime/defs_windows_386.h | 3 ++ src/runtime/defs_windows_amd64.h | 3 ++ src/runtime/os_windows.c | 35 ++++++++++--- src/runtime/os_windows_386.c | 84 +++++++++++++++++++------------- src/runtime/os_windows_amd64.c | 97 ++++++++++++++++++++++++------------- src/runtime/sys_windows_386.s | 18 ++++++- src/runtime/sys_windows_amd64.s | 18 ++++++- src/runtime/syscall_windows_test.go | 39 +++++++++++++++ 9 files changed, 224 insertions(+), 76 deletions(-) (limited to 'src') diff --git a/src/runtime/defs_windows.go b/src/runtime/defs_windows.go index cb0f54d8a..c27cc41dc 100644 --- a/src/runtime/defs_windows.go +++ b/src/runtime/defs_windows.go @@ -59,6 +59,9 @@ const ( INFINITE = C.INFINITE WAIT_TIMEOUT = C.WAIT_TIMEOUT + + EXCEPTION_CONTINUE_EXECUTION = C.EXCEPTION_CONTINUE_EXECUTION + EXCEPTION_CONTINUE_SEARCH = C.EXCEPTION_CONTINUE_SEARCH ) type SystemInfo C.SYSTEM_INFO diff --git a/src/runtime/defs_windows_386.h b/src/runtime/defs_windows_386.h index 295e422c6..67cac0f01 100644 --- a/src/runtime/defs_windows_386.h +++ b/src/runtime/defs_windows_386.h @@ -32,6 +32,9 @@ enum { INFINITE = 0xffffffff, WAIT_TIMEOUT = 0x102, + + EXCEPTION_CONTINUE_EXECUTION = -0x1, + EXCEPTION_CONTINUE_SEARCH = 0x0, }; typedef struct SystemInfo SystemInfo; diff --git a/src/runtime/defs_windows_amd64.h b/src/runtime/defs_windows_amd64.h index 2516c8412..97cdb9ed1 100644 --- a/src/runtime/defs_windows_amd64.h +++ b/src/runtime/defs_windows_amd64.h @@ -32,6 +32,9 @@ enum { INFINITE = 0xffffffff, WAIT_TIMEOUT = 0x102, + + EXCEPTION_CONTINUE_EXECUTION = -0x1, + EXCEPTION_CONTINUE_SEARCH = 0x0, }; typedef struct SystemInfo SystemInfo; diff --git a/src/runtime/os_windows.c b/src/runtime/os_windows.c index 6337dde2a..721083550 100644 --- a/src/runtime/os_windows.c +++ b/src/runtime/os_windows.c @@ -34,6 +34,7 @@ #pragma dynimport runtime·SetEvent SetEvent "kernel32.dll" #pragma dynimport runtime·SetProcessPriorityBoost SetProcessPriorityBoost "kernel32.dll" #pragma dynimport runtime·SetThreadPriority SetThreadPriority "kernel32.dll" +#pragma dynimport runtime·SetUnhandledExceptionFilter SetUnhandledExceptionFilter "kernel32.dll" #pragma dynimport runtime·SetWaitableTimer SetWaitableTimer "kernel32.dll" #pragma dynimport runtime·Sleep Sleep "kernel32.dll" #pragma dynimport runtime·SuspendThread SuspendThread "kernel32.dll" @@ -65,6 +66,7 @@ extern void *runtime·SetConsoleCtrlHandler; extern void *runtime·SetEvent; extern void *runtime·SetProcessPriorityBoost; extern void *runtime·SetThreadPriority; +extern void *runtime·SetUnhandledExceptionFilter; extern void *runtime·SetWaitableTimer; extern void *runtime·Sleep; extern void *runtime·SuspendThread; @@ -77,7 +79,9 @@ void *runtime·GetQueuedCompletionStatusEx; extern uintptr runtime·externalthreadhandlerp; void runtime·externalthreadhandler(void); -void runtime·sigtramp(void); +void runtime·exceptiontramp(void); +void runtime·firstcontinuetramp(void); +void runtime·lastcontinuetramp(void); #pragma textflag NOSPLIT uintptr @@ -106,12 +110,28 @@ void runtime·osinit(void) { void *kernel32; + void *addVectoredContinueHandler = nil; + + kernel32 = runtime·stdcall1(runtime·LoadLibraryA, (uintptr)"kernel32.dll"); runtime·externalthreadhandlerp = (uintptr)runtime·externalthreadhandler; - runtime·stdcall2(runtime·AddVectoredExceptionHandler, 1, (uintptr)runtime·sigtramp); + runtime·stdcall2(runtime·AddVectoredExceptionHandler, 1, (uintptr)runtime·exceptiontramp); + if(kernel32 != nil) + addVectoredContinueHandler = runtime·stdcall2(runtime·GetProcAddress, (uintptr)kernel32, (uintptr)"AddVectoredContinueHandler"); + if(addVectoredContinueHandler == nil) + // use SetUnhandledExceptionFilter if VectoredContinueHandler is unavailable. + // note: SetUnhandledExceptionFilter handler won't be called, if debugging. + runtime·stdcall1(runtime·SetUnhandledExceptionFilter, (uintptr)runtime·lastcontinuetramp); + else { + runtime·stdcall2(addVectoredContinueHandler, 1, (uintptr)runtime·firstcontinuetramp); + runtime·stdcall2(addVectoredContinueHandler, 0, (uintptr)runtime·lastcontinuetramp); + } + runtime·stdcall2(runtime·SetConsoleCtrlHandler, (uintptr)runtime·ctrlhandler, 1); + runtime·stdcall1(runtime·timeBeginPeriod, 1); + runtime·ncpu = getproccount(); // Windows dynamic priority boosting assumes that a process has different types @@ -120,7 +140,6 @@ runtime·osinit(void) // In such context dynamic priority boosting does nothing but harm, so we turn it off. runtime·stdcall2(runtime·SetProcessPriorityBoost, -1, 1); - kernel32 = runtime·stdcall1(runtime·LoadLibraryA, (uintptr)"kernel32.dll"); if(kernel32 != nil) { runtime·GetQueuedCompletionStatusEx = runtime·stdcall2(runtime·GetProcAddress, (uintptr)kernel32, (uintptr)"GetQueuedCompletionStatusEx"); } @@ -475,10 +494,14 @@ runtime·issigpanic(uint32 code) void runtime·initsig(void) { - // following line keeps sigtramp alive at link stage + // following line keeps these functions alive at link stage // if there's a better way please write it here - void *p = runtime·sigtramp; - USED(p); + void *e = runtime·exceptiontramp; + void *f = runtime·firstcontinuetramp; + void *l = runtime·lastcontinuetramp; + USED(e); + USED(f); + USED(l); } uint32 diff --git a/src/runtime/os_windows_386.c b/src/runtime/os_windows_386.c index e2ae8db27..213582799 100644 --- a/src/runtime/os_windows_386.c +++ b/src/runtime/os_windows_386.c @@ -24,45 +24,63 @@ runtime·dumpregs(Context *r) runtime·printf("gs %x\n", r->SegGs); } -// Called by sigtramp from Windows VEH handler. -// Return value signals whether the exception has been handled (-1) -// or should be made available to other handlers in the chain (0). -uint32 -runtime·sighandler(ExceptionRecord *info, Context *r, G *gp) +bool +runtime·isgoexception(ExceptionRecord *info, Context *r) { - bool crash; - uintptr *sp; extern byte runtime·text[], runtime·etext[]; // Only handle exception if executing instructions in Go binary // (not Windows library code). if(r->Eip < (uint32)runtime·text || (uint32)runtime·etext < r->Eip) - return 0; - - if(gp != nil && runtime·issigpanic(info->ExceptionCode)) { - // Make it look like a call to the signal func. - // Have to pass arguments out of band since - // augmenting the stack frame would break - // the unwinding code. - gp->sig = info->ExceptionCode; - gp->sigcode0 = info->ExceptionInformation[0]; - gp->sigcode1 = info->ExceptionInformation[1]; - gp->sigpc = r->Eip; - - // Only push runtime·sigpanic if r->eip != 0. - // If r->eip == 0, probably panicked because of a - // call to a nil func. Not pushing that onto sp will - // make the trace look like a call to runtime·sigpanic instead. - // (Otherwise the trace will end at runtime·sigpanic and we - // won't get to see who faulted.) - if(r->Eip != 0) { - sp = (uintptr*)r->Esp; - *--sp = r->Eip; - r->Esp = (uintptr)sp; - } - r->Eip = (uintptr)runtime·sigpanic; - return -1; + return false; + + if(!runtime·issigpanic(info->ExceptionCode)) + return false; + + return true; +} + +// Called by sigtramp from Windows VEH handler. +// Return value signals whether the exception has been handled (EXCEPTION_CONTINUE_EXECUTION) +// or should be made available to other handlers in the chain (EXCEPTION_CONTINUE_SEARCH). +uint32 +runtime·exceptionhandler(ExceptionRecord *info, Context *r, G *gp) +{ + uintptr *sp; + + if(!runtime·isgoexception(info, r)) + return EXCEPTION_CONTINUE_SEARCH; + + // Make it look like a call to the signal func. + // Have to pass arguments out of band since + // augmenting the stack frame would break + // the unwinding code. + gp->sig = info->ExceptionCode; + gp->sigcode0 = info->ExceptionInformation[0]; + gp->sigcode1 = info->ExceptionInformation[1]; + gp->sigpc = r->Eip; + + // Only push runtime·sigpanic if r->eip != 0. + // If r->eip == 0, probably panicked because of a + // call to a nil func. Not pushing that onto sp will + // make the trace look like a call to runtime·sigpanic instead. + // (Otherwise the trace will end at runtime·sigpanic and we + // won't get to see who faulted.) + if(r->Eip != 0) { + sp = (uintptr*)r->Esp; + *--sp = r->Eip; + r->Esp = (uintptr)sp; } + r->Eip = (uintptr)runtime·sigpanic; + return EXCEPTION_CONTINUE_EXECUTION; +} + +// lastcontinuehandler is reached, because runtime cannot handle +// current exception. lastcontinuehandler will print crash info and exit. +uint32 +runtime·lastcontinuehandler(ExceptionRecord *info, Context *r, G *gp) +{ + bool crash; if(runtime·panicking) // traceback already printed runtime·exit(2); @@ -88,7 +106,7 @@ runtime·sighandler(ExceptionRecord *info, Context *r, G *gp) runtime·crash(); runtime·exit(2); - return -1; // not reached + return 0; // not reached } void diff --git a/src/runtime/os_windows_amd64.c b/src/runtime/os_windows_amd64.c index 261880d45..b96cf70d1 100644 --- a/src/runtime/os_windows_amd64.c +++ b/src/runtime/os_windows_amd64.c @@ -32,45 +32,76 @@ runtime·dumpregs(Context *r) runtime·printf("gs %X\n", (uint64)r->SegGs); } -// Called by sigtramp from Windows VEH handler. -// Return value signals whether the exception has been handled (-1) -// or should be made available to other handlers in the chain (0). -uint32 -runtime·sighandler(ExceptionRecord *info, Context *r, G *gp) +bool +runtime·isgoexception(ExceptionRecord *info, Context *r) { - bool crash; - uintptr *sp; extern byte runtime·text[], runtime·etext[]; // Only handle exception if executing instructions in Go binary // (not Windows library code). if(r->Rip < (uint64)runtime·text || (uint64)runtime·etext < r->Rip) - return 0; - - if(gp != nil && runtime·issigpanic(info->ExceptionCode)) { - // Make it look like a call to the signal func. - // Have to pass arguments out of band since - // augmenting the stack frame would break - // the unwinding code. - gp->sig = info->ExceptionCode; - gp->sigcode0 = info->ExceptionInformation[0]; - gp->sigcode1 = info->ExceptionInformation[1]; - gp->sigpc = r->Rip; - - // Only push runtime·sigpanic if r->rip != 0. - // If r->rip == 0, probably panicked because of a - // call to a nil func. Not pushing that onto sp will - // make the trace look like a call to runtime·sigpanic instead. - // (Otherwise the trace will end at runtime·sigpanic and we - // won't get to see who faulted.) - if(r->Rip != 0) { - sp = (uintptr*)r->Rsp; - *--sp = r->Rip; - r->Rsp = (uintptr)sp; - } - r->Rip = (uintptr)runtime·sigpanic; - return -1; + return false; + + if(!runtime·issigpanic(info->ExceptionCode)) + return false; + + return true; +} + +// Called by sigtramp from Windows VEH handler. +// Return value signals whether the exception has been handled (EXCEPTION_CONTINUE_EXECUTION) +// or should be made available to other handlers in the chain (EXCEPTION_CONTINUE_SEARCH). +uint32 +runtime·exceptionhandler(ExceptionRecord *info, Context *r, G *gp) +{ + uintptr *sp; + + if(!runtime·isgoexception(info, r)) + return EXCEPTION_CONTINUE_SEARCH; + + // Make it look like a call to the signal func. + // Have to pass arguments out of band since + // augmenting the stack frame would break + // the unwinding code. + gp->sig = info->ExceptionCode; + gp->sigcode0 = info->ExceptionInformation[0]; + gp->sigcode1 = info->ExceptionInformation[1]; + gp->sigpc = r->Rip; + + // Only push runtime·sigpanic if r->rip != 0. + // If r->rip == 0, probably panicked because of a + // call to a nil func. Not pushing that onto sp will + // make the trace look like a call to runtime·sigpanic instead. + // (Otherwise the trace will end at runtime·sigpanic and we + // won't get to see who faulted.) + if(r->Rip != 0) { + sp = (uintptr*)r->Rsp; + *--sp = r->Rip; + r->Rsp = (uintptr)sp; } + r->Rip = (uintptr)runtime·sigpanic; + return EXCEPTION_CONTINUE_EXECUTION; +} + +// It seems Windows searches ContinueHandler's list even +// if ExceptionHandler returns EXCEPTION_CONTINUE_EXECUTION. +// firstcontinuehandler will stop that search, +// if exceptionhandler did the same earlier. +uint32 +runtime·firstcontinuehandler(ExceptionRecord *info, Context *r, G *gp) +{ + USED(gp); + if(!runtime·isgoexception(info, r)) + return EXCEPTION_CONTINUE_SEARCH; + return EXCEPTION_CONTINUE_EXECUTION; +} + +// lastcontinuehandler is reached, because runtime cannot handle +// current exception. lastcontinuehandler will print crash info and exit. +uint32 +runtime·lastcontinuehandler(ExceptionRecord *info, Context *r, G *gp) +{ + bool crash; if(runtime·panicking) // traceback already printed runtime·exit(2); @@ -97,7 +128,7 @@ runtime·sighandler(ExceptionRecord *info, Context *r, G *gp) runtime·crash(); runtime·exit(2); - return -1; // not reached + return 0; // not reached } void diff --git a/src/runtime/sys_windows_386.s b/src/runtime/sys_windows_386.s index 1bf4d062a..932fe9dd2 100644 --- a/src/runtime/sys_windows_386.s +++ b/src/runtime/sys_windows_386.s @@ -73,6 +73,7 @@ TEXT runtime·setlasterror(SB),NOSPLIT,$0 // Called by Windows as a Vectored Exception Handler (VEH). // First argument is pointer to struct containing // exception record and context pointers. +// Handler function is stored in AX. // Return 0 for 'not handled', -1 for handled. TEXT runtime·sigtramp(SB),NOSPLIT,$0-0 MOVL ptrs+0(FP), CX @@ -84,6 +85,8 @@ TEXT runtime·sigtramp(SB),NOSPLIT,$0-0 MOVL SI, 20(SP) MOVL DI, 24(SP) + MOVL AX, SI // save handler address + // find g get_tls(DX) CMPL DX, $0 @@ -123,11 +126,10 @@ TEXT runtime·sigtramp(SB),NOSPLIT,$0-0 sigtramp_g0: MOVL 0(CX), BX // ExceptionRecord* MOVL 4(CX), CX // Context* - // call sighandler(ExceptionRecord*, Context*, G*) MOVL BX, 0(SP) MOVL CX, 4(SP) MOVL DX, 8(SP) - CALL runtime·sighandler(SB) + CALL SI // call handler // AX is set to report result back to Windows MOVL 12(SP), AX @@ -149,6 +151,18 @@ done: // RET 4 (return and pop 4 bytes parameters) BYTE $0xC2; WORD $4 RET // unreached; make assembler happy + +TEXT runtime·exceptiontramp(SB),NOSPLIT,$0 + MOVL $runtime·exceptionhandler(SB), AX + JMP runtime·sigtramp(SB) + +TEXT runtime·firstcontinuetramp(SB),NOSPLIT,$0-0 + // is never called + INT $3 + +TEXT runtime·lastcontinuetramp(SB),NOSPLIT,$0-0 + MOVL $runtime·lastcontinuehandler(SB), AX + JMP runtime·sigtramp(SB) TEXT runtime·ctrlhandler(SB),NOSPLIT,$0 PUSHL $runtime·ctrlhandler1(SB) diff --git a/src/runtime/sys_windows_amd64.s b/src/runtime/sys_windows_amd64.s index 05750398e..e6190ce68 100644 --- a/src/runtime/sys_windows_amd64.s +++ b/src/runtime/sys_windows_amd64.s @@ -99,6 +99,7 @@ TEXT runtime·setlasterror(SB),NOSPLIT,$0 // Called by Windows as a Vectored Exception Handler (VEH). // First argument is pointer to struct containing // exception record and context pointers. +// Handler function is stored in AX. // Return 0 for 'not handled', -1 for handled. TEXT runtime·sigtramp(SB),NOSPLIT,$0-0 // CX: PEXCEPTION_POINTERS ExceptionInfo @@ -116,6 +117,8 @@ TEXT runtime·sigtramp(SB),NOSPLIT,$0-0 MOVQ R14, 32(SP) MOVQ R15, 88(SP) + MOVQ AX, R15 // save handler address + // find g get_tls(DX) CMPQ DX, $0 @@ -157,11 +160,10 @@ TEXT runtime·sigtramp(SB),NOSPLIT,$0-0 sigtramp_g0: MOVQ 0(CX), BX // ExceptionRecord* MOVQ 8(CX), CX // Context* - // call sighandler(ExceptionRecord*, Context*, G*) MOVQ BX, 0(SP) MOVQ CX, 8(SP) MOVQ DX, 16(SP) - CALL runtime·sighandler(SB) + CALL R15 // call handler // AX is set to report result back to Windows MOVL 24(SP), AX @@ -187,6 +189,18 @@ done: RET +TEXT runtime·exceptiontramp(SB),NOSPLIT,$0 + MOVQ $runtime·exceptionhandler(SB), AX + JMP runtime·sigtramp(SB) + +TEXT runtime·firstcontinuetramp(SB),NOSPLIT,$0-0 + MOVQ $runtime·firstcontinuehandler(SB), AX + JMP runtime·sigtramp(SB) + +TEXT runtime·lastcontinuetramp(SB),NOSPLIT,$0-0 + MOVQ $runtime·lastcontinuehandler(SB), AX + JMP runtime·sigtramp(SB) + TEXT runtime·ctrlhandler(SB),NOSPLIT,$8 MOVQ CX, 16(SP) // spill MOVQ $runtime·ctrlhandler1(SB), CX diff --git a/src/runtime/syscall_windows_test.go b/src/runtime/syscall_windows_test.go index 9ed016ccc..ce8a9ec1b 100644 --- a/src/runtime/syscall_windows_test.go +++ b/src/runtime/syscall_windows_test.go @@ -494,3 +494,42 @@ func TestOutputDebugString(t *testing.T) { p := syscall.StringToUTF16Ptr("testing OutputDebugString") d.Proc("OutputDebugStringW").Call(uintptr(unsafe.Pointer(p))) } + +func TestRaiseException(t *testing.T) { + o := executeTest(t, raiseExceptionSource, nil) + if strings.Contains(o, "RaiseException should not return") { + t.Fatalf("RaiseException did not crash program: %v", o) + } + if !strings.Contains(o, "Exception 0xbad") { + t.Fatalf("No stack trace: %v", o) + } +} + +const raiseExceptionSource = ` +package main +import "syscall" +func main() { + const EXCEPTION_NONCONTINUABLE = 1 + mod := syscall.MustLoadDLL("kernel32.dll") + proc := mod.MustFindProc("RaiseException") + proc.Call(0xbad, EXCEPTION_NONCONTINUABLE, 0, 0) + println("RaiseException should not return") +} +` + +func TestZeroDivisionException(t *testing.T) { + o := executeTest(t, zeroDivisionExceptionSource, nil) + if !strings.Contains(o, "panic: runtime error: integer divide by zero") { + t.Fatalf("No stack trace: %v", o) + } +} + +const zeroDivisionExceptionSource = ` +package main +func main() { + x := 1 + y := 0 + z := x / y + println(z) +} +` -- cgit v1.2.1 From 76b3936b738289481b81b6a520c4f03aef6afcf1 Mon Sep 17 00:00:00 2001 From: Alex Brainman Date: Thu, 9 Oct 2014 17:24:34 +1100 Subject: undo CL 145150043 / 8b3d26697b8d That was complete failure - builders are broken, but original cl worked fine on my system. I will need access to builders to test this change properly. ??? original CL description runtime: handle all windows exception Fixes issue 8006. LGTM=rsc R=golang-codereviews, rsc CC=golang-codereviews https://codereview.appspot.com/145150043 ??? TBR=rsc R=golang-codereviews CC=golang-codereviews https://codereview.appspot.com/154180043 --- src/runtime/defs_windows.go | 3 -- src/runtime/defs_windows_386.h | 3 -- src/runtime/defs_windows_amd64.h | 3 -- src/runtime/os_windows.c | 35 +++---------- src/runtime/os_windows_386.c | 84 +++++++++++++------------------- src/runtime/os_windows_amd64.c | 97 +++++++++++++------------------------ src/runtime/sys_windows_386.s | 18 +------ src/runtime/sys_windows_amd64.s | 18 +------ src/runtime/syscall_windows_test.go | 39 --------------- 9 files changed, 76 insertions(+), 224 deletions(-) (limited to 'src') diff --git a/src/runtime/defs_windows.go b/src/runtime/defs_windows.go index c27cc41dc..cb0f54d8a 100644 --- a/src/runtime/defs_windows.go +++ b/src/runtime/defs_windows.go @@ -59,9 +59,6 @@ const ( INFINITE = C.INFINITE WAIT_TIMEOUT = C.WAIT_TIMEOUT - - EXCEPTION_CONTINUE_EXECUTION = C.EXCEPTION_CONTINUE_EXECUTION - EXCEPTION_CONTINUE_SEARCH = C.EXCEPTION_CONTINUE_SEARCH ) type SystemInfo C.SYSTEM_INFO diff --git a/src/runtime/defs_windows_386.h b/src/runtime/defs_windows_386.h index 67cac0f01..295e422c6 100644 --- a/src/runtime/defs_windows_386.h +++ b/src/runtime/defs_windows_386.h @@ -32,9 +32,6 @@ enum { INFINITE = 0xffffffff, WAIT_TIMEOUT = 0x102, - - EXCEPTION_CONTINUE_EXECUTION = -0x1, - EXCEPTION_CONTINUE_SEARCH = 0x0, }; typedef struct SystemInfo SystemInfo; diff --git a/src/runtime/defs_windows_amd64.h b/src/runtime/defs_windows_amd64.h index 97cdb9ed1..2516c8412 100644 --- a/src/runtime/defs_windows_amd64.h +++ b/src/runtime/defs_windows_amd64.h @@ -32,9 +32,6 @@ enum { INFINITE = 0xffffffff, WAIT_TIMEOUT = 0x102, - - EXCEPTION_CONTINUE_EXECUTION = -0x1, - EXCEPTION_CONTINUE_SEARCH = 0x0, }; typedef struct SystemInfo SystemInfo; diff --git a/src/runtime/os_windows.c b/src/runtime/os_windows.c index 721083550..6337dde2a 100644 --- a/src/runtime/os_windows.c +++ b/src/runtime/os_windows.c @@ -34,7 +34,6 @@ #pragma dynimport runtime·SetEvent SetEvent "kernel32.dll" #pragma dynimport runtime·SetProcessPriorityBoost SetProcessPriorityBoost "kernel32.dll" #pragma dynimport runtime·SetThreadPriority SetThreadPriority "kernel32.dll" -#pragma dynimport runtime·SetUnhandledExceptionFilter SetUnhandledExceptionFilter "kernel32.dll" #pragma dynimport runtime·SetWaitableTimer SetWaitableTimer "kernel32.dll" #pragma dynimport runtime·Sleep Sleep "kernel32.dll" #pragma dynimport runtime·SuspendThread SuspendThread "kernel32.dll" @@ -66,7 +65,6 @@ extern void *runtime·SetConsoleCtrlHandler; extern void *runtime·SetEvent; extern void *runtime·SetProcessPriorityBoost; extern void *runtime·SetThreadPriority; -extern void *runtime·SetUnhandledExceptionFilter; extern void *runtime·SetWaitableTimer; extern void *runtime·Sleep; extern void *runtime·SuspendThread; @@ -79,9 +77,7 @@ void *runtime·GetQueuedCompletionStatusEx; extern uintptr runtime·externalthreadhandlerp; void runtime·externalthreadhandler(void); -void runtime·exceptiontramp(void); -void runtime·firstcontinuetramp(void); -void runtime·lastcontinuetramp(void); +void runtime·sigtramp(void); #pragma textflag NOSPLIT uintptr @@ -110,28 +106,12 @@ void runtime·osinit(void) { void *kernel32; - void *addVectoredContinueHandler = nil; - - kernel32 = runtime·stdcall1(runtime·LoadLibraryA, (uintptr)"kernel32.dll"); runtime·externalthreadhandlerp = (uintptr)runtime·externalthreadhandler; - runtime·stdcall2(runtime·AddVectoredExceptionHandler, 1, (uintptr)runtime·exceptiontramp); - if(kernel32 != nil) - addVectoredContinueHandler = runtime·stdcall2(runtime·GetProcAddress, (uintptr)kernel32, (uintptr)"AddVectoredContinueHandler"); - if(addVectoredContinueHandler == nil) - // use SetUnhandledExceptionFilter if VectoredContinueHandler is unavailable. - // note: SetUnhandledExceptionFilter handler won't be called, if debugging. - runtime·stdcall1(runtime·SetUnhandledExceptionFilter, (uintptr)runtime·lastcontinuetramp); - else { - runtime·stdcall2(addVectoredContinueHandler, 1, (uintptr)runtime·firstcontinuetramp); - runtime·stdcall2(addVectoredContinueHandler, 0, (uintptr)runtime·lastcontinuetramp); - } - + runtime·stdcall2(runtime·AddVectoredExceptionHandler, 1, (uintptr)runtime·sigtramp); runtime·stdcall2(runtime·SetConsoleCtrlHandler, (uintptr)runtime·ctrlhandler, 1); - runtime·stdcall1(runtime·timeBeginPeriod, 1); - runtime·ncpu = getproccount(); // Windows dynamic priority boosting assumes that a process has different types @@ -140,6 +120,7 @@ runtime·osinit(void) // In such context dynamic priority boosting does nothing but harm, so we turn it off. runtime·stdcall2(runtime·SetProcessPriorityBoost, -1, 1); + kernel32 = runtime·stdcall1(runtime·LoadLibraryA, (uintptr)"kernel32.dll"); if(kernel32 != nil) { runtime·GetQueuedCompletionStatusEx = runtime·stdcall2(runtime·GetProcAddress, (uintptr)kernel32, (uintptr)"GetQueuedCompletionStatusEx"); } @@ -494,14 +475,10 @@ runtime·issigpanic(uint32 code) void runtime·initsig(void) { - // following line keeps these functions alive at link stage + // following line keeps sigtramp alive at link stage // if there's a better way please write it here - void *e = runtime·exceptiontramp; - void *f = runtime·firstcontinuetramp; - void *l = runtime·lastcontinuetramp; - USED(e); - USED(f); - USED(l); + void *p = runtime·sigtramp; + USED(p); } uint32 diff --git a/src/runtime/os_windows_386.c b/src/runtime/os_windows_386.c index 213582799..e2ae8db27 100644 --- a/src/runtime/os_windows_386.c +++ b/src/runtime/os_windows_386.c @@ -24,63 +24,45 @@ runtime·dumpregs(Context *r) runtime·printf("gs %x\n", r->SegGs); } -bool -runtime·isgoexception(ExceptionRecord *info, Context *r) +// Called by sigtramp from Windows VEH handler. +// Return value signals whether the exception has been handled (-1) +// or should be made available to other handlers in the chain (0). +uint32 +runtime·sighandler(ExceptionRecord *info, Context *r, G *gp) { + bool crash; + uintptr *sp; extern byte runtime·text[], runtime·etext[]; // Only handle exception if executing instructions in Go binary // (not Windows library code). if(r->Eip < (uint32)runtime·text || (uint32)runtime·etext < r->Eip) - return false; - - if(!runtime·issigpanic(info->ExceptionCode)) - return false; - - return true; -} - -// Called by sigtramp from Windows VEH handler. -// Return value signals whether the exception has been handled (EXCEPTION_CONTINUE_EXECUTION) -// or should be made available to other handlers in the chain (EXCEPTION_CONTINUE_SEARCH). -uint32 -runtime·exceptionhandler(ExceptionRecord *info, Context *r, G *gp) -{ - uintptr *sp; - - if(!runtime·isgoexception(info, r)) - return EXCEPTION_CONTINUE_SEARCH; - - // Make it look like a call to the signal func. - // Have to pass arguments out of band since - // augmenting the stack frame would break - // the unwinding code. - gp->sig = info->ExceptionCode; - gp->sigcode0 = info->ExceptionInformation[0]; - gp->sigcode1 = info->ExceptionInformation[1]; - gp->sigpc = r->Eip; - - // Only push runtime·sigpanic if r->eip != 0. - // If r->eip == 0, probably panicked because of a - // call to a nil func. Not pushing that onto sp will - // make the trace look like a call to runtime·sigpanic instead. - // (Otherwise the trace will end at runtime·sigpanic and we - // won't get to see who faulted.) - if(r->Eip != 0) { - sp = (uintptr*)r->Esp; - *--sp = r->Eip; - r->Esp = (uintptr)sp; + return 0; + + if(gp != nil && runtime·issigpanic(info->ExceptionCode)) { + // Make it look like a call to the signal func. + // Have to pass arguments out of band since + // augmenting the stack frame would break + // the unwinding code. + gp->sig = info->ExceptionCode; + gp->sigcode0 = info->ExceptionInformation[0]; + gp->sigcode1 = info->ExceptionInformation[1]; + gp->sigpc = r->Eip; + + // Only push runtime·sigpanic if r->eip != 0. + // If r->eip == 0, probably panicked because of a + // call to a nil func. Not pushing that onto sp will + // make the trace look like a call to runtime·sigpanic instead. + // (Otherwise the trace will end at runtime·sigpanic and we + // won't get to see who faulted.) + if(r->Eip != 0) { + sp = (uintptr*)r->Esp; + *--sp = r->Eip; + r->Esp = (uintptr)sp; + } + r->Eip = (uintptr)runtime·sigpanic; + return -1; } - r->Eip = (uintptr)runtime·sigpanic; - return EXCEPTION_CONTINUE_EXECUTION; -} - -// lastcontinuehandler is reached, because runtime cannot handle -// current exception. lastcontinuehandler will print crash info and exit. -uint32 -runtime·lastcontinuehandler(ExceptionRecord *info, Context *r, G *gp) -{ - bool crash; if(runtime·panicking) // traceback already printed runtime·exit(2); @@ -106,7 +88,7 @@ runtime·lastcontinuehandler(ExceptionRecord *info, Context *r, G *gp) runtime·crash(); runtime·exit(2); - return 0; // not reached + return -1; // not reached } void diff --git a/src/runtime/os_windows_amd64.c b/src/runtime/os_windows_amd64.c index b96cf70d1..261880d45 100644 --- a/src/runtime/os_windows_amd64.c +++ b/src/runtime/os_windows_amd64.c @@ -32,76 +32,45 @@ runtime·dumpregs(Context *r) runtime·printf("gs %X\n", (uint64)r->SegGs); } -bool -runtime·isgoexception(ExceptionRecord *info, Context *r) +// Called by sigtramp from Windows VEH handler. +// Return value signals whether the exception has been handled (-1) +// or should be made available to other handlers in the chain (0). +uint32 +runtime·sighandler(ExceptionRecord *info, Context *r, G *gp) { + bool crash; + uintptr *sp; extern byte runtime·text[], runtime·etext[]; // Only handle exception if executing instructions in Go binary // (not Windows library code). if(r->Rip < (uint64)runtime·text || (uint64)runtime·etext < r->Rip) - return false; - - if(!runtime·issigpanic(info->ExceptionCode)) - return false; - - return true; -} - -// Called by sigtramp from Windows VEH handler. -// Return value signals whether the exception has been handled (EXCEPTION_CONTINUE_EXECUTION) -// or should be made available to other handlers in the chain (EXCEPTION_CONTINUE_SEARCH). -uint32 -runtime·exceptionhandler(ExceptionRecord *info, Context *r, G *gp) -{ - uintptr *sp; - - if(!runtime·isgoexception(info, r)) - return EXCEPTION_CONTINUE_SEARCH; - - // Make it look like a call to the signal func. - // Have to pass arguments out of band since - // augmenting the stack frame would break - // the unwinding code. - gp->sig = info->ExceptionCode; - gp->sigcode0 = info->ExceptionInformation[0]; - gp->sigcode1 = info->ExceptionInformation[1]; - gp->sigpc = r->Rip; - - // Only push runtime·sigpanic if r->rip != 0. - // If r->rip == 0, probably panicked because of a - // call to a nil func. Not pushing that onto sp will - // make the trace look like a call to runtime·sigpanic instead. - // (Otherwise the trace will end at runtime·sigpanic and we - // won't get to see who faulted.) - if(r->Rip != 0) { - sp = (uintptr*)r->Rsp; - *--sp = r->Rip; - r->Rsp = (uintptr)sp; + return 0; + + if(gp != nil && runtime·issigpanic(info->ExceptionCode)) { + // Make it look like a call to the signal func. + // Have to pass arguments out of band since + // augmenting the stack frame would break + // the unwinding code. + gp->sig = info->ExceptionCode; + gp->sigcode0 = info->ExceptionInformation[0]; + gp->sigcode1 = info->ExceptionInformation[1]; + gp->sigpc = r->Rip; + + // Only push runtime·sigpanic if r->rip != 0. + // If r->rip == 0, probably panicked because of a + // call to a nil func. Not pushing that onto sp will + // make the trace look like a call to runtime·sigpanic instead. + // (Otherwise the trace will end at runtime·sigpanic and we + // won't get to see who faulted.) + if(r->Rip != 0) { + sp = (uintptr*)r->Rsp; + *--sp = r->Rip; + r->Rsp = (uintptr)sp; + } + r->Rip = (uintptr)runtime·sigpanic; + return -1; } - r->Rip = (uintptr)runtime·sigpanic; - return EXCEPTION_CONTINUE_EXECUTION; -} - -// It seems Windows searches ContinueHandler's list even -// if ExceptionHandler returns EXCEPTION_CONTINUE_EXECUTION. -// firstcontinuehandler will stop that search, -// if exceptionhandler did the same earlier. -uint32 -runtime·firstcontinuehandler(ExceptionRecord *info, Context *r, G *gp) -{ - USED(gp); - if(!runtime·isgoexception(info, r)) - return EXCEPTION_CONTINUE_SEARCH; - return EXCEPTION_CONTINUE_EXECUTION; -} - -// lastcontinuehandler is reached, because runtime cannot handle -// current exception. lastcontinuehandler will print crash info and exit. -uint32 -runtime·lastcontinuehandler(ExceptionRecord *info, Context *r, G *gp) -{ - bool crash; if(runtime·panicking) // traceback already printed runtime·exit(2); @@ -128,7 +97,7 @@ runtime·lastcontinuehandler(ExceptionRecord *info, Context *r, G *gp) runtime·crash(); runtime·exit(2); - return 0; // not reached + return -1; // not reached } void diff --git a/src/runtime/sys_windows_386.s b/src/runtime/sys_windows_386.s index 932fe9dd2..1bf4d062a 100644 --- a/src/runtime/sys_windows_386.s +++ b/src/runtime/sys_windows_386.s @@ -73,7 +73,6 @@ TEXT runtime·setlasterror(SB),NOSPLIT,$0 // Called by Windows as a Vectored Exception Handler (VEH). // First argument is pointer to struct containing // exception record and context pointers. -// Handler function is stored in AX. // Return 0 for 'not handled', -1 for handled. TEXT runtime·sigtramp(SB),NOSPLIT,$0-0 MOVL ptrs+0(FP), CX @@ -85,8 +84,6 @@ TEXT runtime·sigtramp(SB),NOSPLIT,$0-0 MOVL SI, 20(SP) MOVL DI, 24(SP) - MOVL AX, SI // save handler address - // find g get_tls(DX) CMPL DX, $0 @@ -126,10 +123,11 @@ TEXT runtime·sigtramp(SB),NOSPLIT,$0-0 sigtramp_g0: MOVL 0(CX), BX // ExceptionRecord* MOVL 4(CX), CX // Context* + // call sighandler(ExceptionRecord*, Context*, G*) MOVL BX, 0(SP) MOVL CX, 4(SP) MOVL DX, 8(SP) - CALL SI // call handler + CALL runtime·sighandler(SB) // AX is set to report result back to Windows MOVL 12(SP), AX @@ -151,18 +149,6 @@ done: // RET 4 (return and pop 4 bytes parameters) BYTE $0xC2; WORD $4 RET // unreached; make assembler happy - -TEXT runtime·exceptiontramp(SB),NOSPLIT,$0 - MOVL $runtime·exceptionhandler(SB), AX - JMP runtime·sigtramp(SB) - -TEXT runtime·firstcontinuetramp(SB),NOSPLIT,$0-0 - // is never called - INT $3 - -TEXT runtime·lastcontinuetramp(SB),NOSPLIT,$0-0 - MOVL $runtime·lastcontinuehandler(SB), AX - JMP runtime·sigtramp(SB) TEXT runtime·ctrlhandler(SB),NOSPLIT,$0 PUSHL $runtime·ctrlhandler1(SB) diff --git a/src/runtime/sys_windows_amd64.s b/src/runtime/sys_windows_amd64.s index e6190ce68..05750398e 100644 --- a/src/runtime/sys_windows_amd64.s +++ b/src/runtime/sys_windows_amd64.s @@ -99,7 +99,6 @@ TEXT runtime·setlasterror(SB),NOSPLIT,$0 // Called by Windows as a Vectored Exception Handler (VEH). // First argument is pointer to struct containing // exception record and context pointers. -// Handler function is stored in AX. // Return 0 for 'not handled', -1 for handled. TEXT runtime·sigtramp(SB),NOSPLIT,$0-0 // CX: PEXCEPTION_POINTERS ExceptionInfo @@ -117,8 +116,6 @@ TEXT runtime·sigtramp(SB),NOSPLIT,$0-0 MOVQ R14, 32(SP) MOVQ R15, 88(SP) - MOVQ AX, R15 // save handler address - // find g get_tls(DX) CMPQ DX, $0 @@ -160,10 +157,11 @@ TEXT runtime·sigtramp(SB),NOSPLIT,$0-0 sigtramp_g0: MOVQ 0(CX), BX // ExceptionRecord* MOVQ 8(CX), CX // Context* + // call sighandler(ExceptionRecord*, Context*, G*) MOVQ BX, 0(SP) MOVQ CX, 8(SP) MOVQ DX, 16(SP) - CALL R15 // call handler + CALL runtime·sighandler(SB) // AX is set to report result back to Windows MOVL 24(SP), AX @@ -189,18 +187,6 @@ done: RET -TEXT runtime·exceptiontramp(SB),NOSPLIT,$0 - MOVQ $runtime·exceptionhandler(SB), AX - JMP runtime·sigtramp(SB) - -TEXT runtime·firstcontinuetramp(SB),NOSPLIT,$0-0 - MOVQ $runtime·firstcontinuehandler(SB), AX - JMP runtime·sigtramp(SB) - -TEXT runtime·lastcontinuetramp(SB),NOSPLIT,$0-0 - MOVQ $runtime·lastcontinuehandler(SB), AX - JMP runtime·sigtramp(SB) - TEXT runtime·ctrlhandler(SB),NOSPLIT,$8 MOVQ CX, 16(SP) // spill MOVQ $runtime·ctrlhandler1(SB), CX diff --git a/src/runtime/syscall_windows_test.go b/src/runtime/syscall_windows_test.go index ce8a9ec1b..9ed016ccc 100644 --- a/src/runtime/syscall_windows_test.go +++ b/src/runtime/syscall_windows_test.go @@ -494,42 +494,3 @@ func TestOutputDebugString(t *testing.T) { p := syscall.StringToUTF16Ptr("testing OutputDebugString") d.Proc("OutputDebugStringW").Call(uintptr(unsafe.Pointer(p))) } - -func TestRaiseException(t *testing.T) { - o := executeTest(t, raiseExceptionSource, nil) - if strings.Contains(o, "RaiseException should not return") { - t.Fatalf("RaiseException did not crash program: %v", o) - } - if !strings.Contains(o, "Exception 0xbad") { - t.Fatalf("No stack trace: %v", o) - } -} - -const raiseExceptionSource = ` -package main -import "syscall" -func main() { - const EXCEPTION_NONCONTINUABLE = 1 - mod := syscall.MustLoadDLL("kernel32.dll") - proc := mod.MustFindProc("RaiseException") - proc.Call(0xbad, EXCEPTION_NONCONTINUABLE, 0, 0) - println("RaiseException should not return") -} -` - -func TestZeroDivisionException(t *testing.T) { - o := executeTest(t, zeroDivisionExceptionSource, nil) - if !strings.Contains(o, "panic: runtime error: integer divide by zero") { - t.Fatalf("No stack trace: %v", o) - } -} - -const zeroDivisionExceptionSource = ` -package main -func main() { - x := 1 - y := 0 - z := x / y - println(z) -} -` -- cgit v1.2.1 From 6c72d4a5203ac3a1176a12c2ef9a24958ffeb322 Mon Sep 17 00:00:00 2001 From: David du Colombier <0intro@gmail.com> Date: Thu, 9 Oct 2014 11:12:03 +0200 Subject: net/rpc: skip TestGobError on Plan 9 LGTM=bradfitz R=rsc, bradfitz CC=aram, golang-codereviews https://codereview.appspot.com/154140043 --- src/net/rpc/client_test.go | 3 +++ 1 file changed, 3 insertions(+) (limited to 'src') diff --git a/src/net/rpc/client_test.go b/src/net/rpc/client_test.go index d116d2acc..fb838eb34 100644 --- a/src/net/rpc/client_test.go +++ b/src/net/rpc/client_test.go @@ -52,6 +52,9 @@ func (s *S) Recv(nul *struct{}, reply *R) error { } func TestGobError(t *testing.T) { + if runtime.GOOS == "plan9" { + t.Skip("skipping test; see http://golang.org/issue/8908") + } defer func() { err := recover() if err == nil { -- cgit v1.2.1 From c41a8909f5cc8b7cb2ec09a74d0cae6b8f087fc9 Mon Sep 17 00:00:00 2001 From: David du Colombier <0intro@gmail.com> Date: Thu, 9 Oct 2014 11:21:21 +0200 Subject: net/rpc: fix build LGTM=adg R=golang-codereviews, adg CC=golang-codereviews https://codereview.appspot.com/151620043 --- src/net/rpc/client_test.go | 1 + 1 file changed, 1 insertion(+) (limited to 'src') diff --git a/src/net/rpc/client_test.go b/src/net/rpc/client_test.go index fb838eb34..5dd111b29 100644 --- a/src/net/rpc/client_test.go +++ b/src/net/rpc/client_test.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "net" + "runtime" "strings" "testing" ) -- cgit v1.2.1 From e04979c0c2779a7b4830ffcd694480e588941b22 Mon Sep 17 00:00:00 2001 From: Dmitriy Vyukov Date: Thu, 9 Oct 2014 17:05:38 +0400 Subject: runtime: add comment to mgc0.h Missed that comment in CL 153990043. LGTM=khr R=khr CC=golang-codereviews https://codereview.appspot.com/156010043 --- src/runtime/mgc0.h | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src') diff --git a/src/runtime/mgc0.h b/src/runtime/mgc0.h index 10f24d009..64f818914 100644 --- a/src/runtime/mgc0.h +++ b/src/runtime/mgc0.h @@ -42,6 +42,8 @@ enum { BitsMask = (1< Date: Thu, 9 Oct 2014 14:38:45 -0700 Subject: debug/elf: add comments explaining applyRelocations for amd64/arm64 LGTM=bradfitz R=golang-codereviews, bradfitz CC=golang-codereviews https://codereview.appspot.com/155190043 --- src/debug/elf/file.go | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'src') diff --git a/src/debug/elf/file.go b/src/debug/elf/file.go index c908e7a88..de8a3a24f 100644 --- a/src/debug/elf/file.go +++ b/src/debug/elf/file.go @@ -564,6 +564,10 @@ func (f *File) applyRelocationsAMD64(dst []byte, rels []byte) error { continue } + // There are relocations, so this must be a normal + // object file, and we only look at section symbols, + // so we assume that the symbol value is 0. + switch t { case R_X86_64_64: if rela.Off+8 >= uint64(len(dst)) || rela.Addend < 0 { @@ -646,6 +650,10 @@ func (f *File) applyRelocationsARM64(dst []byte, rels []byte) error { continue } + // There are relocations, so this must be a normal + // object file, and we only look at section symbols, + // so we assume that the symbol value is 0. + switch t { case R_AARCH64_ABS64: if rela.Off+8 >= uint64(len(dst)) || rela.Addend < 0 { -- cgit v1.2.1 From 24815bcce4ab6e6c7cf41f68d2b3b86561e0e2b6 Mon Sep 17 00:00:00 2001 From: Ron Hashimoto Date: Fri, 10 Oct 2014 09:21:32 +1100 Subject: net: disable SIO_UDP_CONNRESET behavior on windows. Fixes issue 5834. LGTM=alex.brainman R=golang-codereviews, bradfitz, alex.brainman, mikioh.mikioh, in60jp, iant CC=golang-codereviews https://codereview.appspot.com/149510043 Committer: Alex Brainman --- src/net/fd_windows.go | 12 ++++++++++++ src/net/udp_test.go | 36 ++++++++++++++++++++++++++++++++++++ src/syscall/ztypes_windows.go | 1 + 3 files changed, 49 insertions(+) (limited to 'src') diff --git a/src/net/fd_windows.go b/src/net/fd_windows.go index 6d69e0624..f3a534a1d 100644 --- a/src/net/fd_windows.go +++ b/src/net/fd_windows.go @@ -294,6 +294,18 @@ func (fd *netFD) init() error { fd.skipSyncNotif = true } } + // Disable SIO_UDP_CONNRESET behavior. + // http://support.microsoft.com/kb/263823 + switch fd.net { + case "udp", "udp4", "udp6": + ret := uint32(0) + flag := uint32(0) + size := uint32(unsafe.Sizeof(flag)) + err := syscall.WSAIoctl(fd.sysfd, syscall.SIO_UDP_CONNRESET, (*byte)(unsafe.Pointer(&flag)), size, nil, 0, &ret, nil, 0) + if err != nil { + return os.NewSyscallError("WSAIoctl", err) + } + } fd.rop.mode = 'r' fd.wop.mode = 'w' fd.rop.fd = fd diff --git a/src/net/udp_test.go b/src/net/udp_test.go index e1778779c..a102acf6c 100644 --- a/src/net/udp_test.go +++ b/src/net/udp_test.go @@ -9,6 +9,7 @@ import ( "runtime" "strings" "testing" + "time" ) func TestResolveUDPAddr(t *testing.T) { @@ -34,6 +35,41 @@ func TestResolveUDPAddr(t *testing.T) { } } +func TestReadFromUDP(t *testing.T) { + ra, err := ResolveUDPAddr("udp", "127.0.0.1:7") + if err != nil { + t.Fatal(err) + } + + la, err := ResolveUDPAddr("udp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + + c, err := ListenUDP("udp", la) + if err != nil { + t.Fatal(err) + } + defer c.Close() + + _, err = c.WriteToUDP([]byte("a"), ra) + if err != nil { + t.Fatal(err) + } + + err = c.SetDeadline(time.Now().Add(100 * time.Millisecond)) + if err != nil { + t.Fatal(err) + } + b := make([]byte, 1) + _, _, err = c.ReadFromUDP(b) + if err == nil { + t.Fatal("ReadFromUDP should fail") + } else if !isTimeout(err) { + t.Fatal(err) + } +} + func TestWriteToUDP(t *testing.T) { switch runtime.GOOS { case "plan9": diff --git a/src/syscall/ztypes_windows.go b/src/syscall/ztypes_windows.go index 1363da01a..4c8a99ab9 100644 --- a/src/syscall/ztypes_windows.go +++ b/src/syscall/ztypes_windows.go @@ -547,6 +547,7 @@ const ( IOC_WS2 = 0x08000000 SIO_GET_EXTENSION_FUNCTION_POINTER = IOC_INOUT | IOC_WS2 | 6 SIO_KEEPALIVE_VALS = IOC_IN | IOC_VENDOR | 4 + SIO_UDP_CONNRESET = IOC_IN | IOC_VENDOR | 12 // cf. http://support.microsoft.com/default.aspx?scid=kb;en-us;257460 -- cgit v1.2.1 From 0fab6a60e66dd7b90ca0ff2e22ab9ee225a5a4e2 Mon Sep 17 00:00:00 2001 From: Alex Brainman Date: Fri, 10 Oct 2014 09:46:41 +1100 Subject: net: skip new TestReadFromUDP on nacl and plan9 (fixes build) TBR=0intro R=golang-codereviews CC=golang-codereviews https://codereview.appspot.com/157820043 --- src/net/udp_test.go | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'src') diff --git a/src/net/udp_test.go b/src/net/udp_test.go index a102acf6c..aa5751557 100644 --- a/src/net/udp_test.go +++ b/src/net/udp_test.go @@ -36,6 +36,11 @@ func TestResolveUDPAddr(t *testing.T) { } func TestReadFromUDP(t *testing.T) { + switch runtime.GOOS { + case "nacl", "plan9": + t.Skipf("skipping test on %q", runtime.GOOS) + } + ra, err := ResolveUDPAddr("udp", "127.0.0.1:7") if err != nil { t.Fatal(err) -- cgit v1.2.1 From 8c13bc63d348c5a6ad2188786594cd1bd9e772e5 Mon Sep 17 00:00:00 2001 From: Adam Langley Date: Thu, 9 Oct 2014 17:37:40 -0700 Subject: encoding/asn1: fix explicitly tagged Times. https://codereview.appspot.com/153770043/ tried to fix the case where a implicitly tagged Time, that happened to have the same tag as GENERALIZEDTIME, shouldn't be parsed as a GENERALIZEDTIME. It did so, mistakenly, by testing whether params.tag != nil. But explicitly tagged values also have a non-nil tag and there the inner tag actually does encode the type of the value. This change instead tests whether the tag class is UNIVERSAL before assuming that the tag contains type information. LGTM=iant R=iant CC=golang-codereviews https://codereview.appspot.com/152380044 --- src/encoding/asn1/asn1.go | 4 ++-- src/encoding/asn1/asn1_test.go | 48 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/encoding/asn1/asn1.go b/src/encoding/asn1/asn1.go index 3aeb3dcc4..8b3d1b341 100644 --- a/src/encoding/asn1/asn1.go +++ b/src/encoding/asn1/asn1.go @@ -640,7 +640,7 @@ func parseField(v reflect.Value, bytes []byte, initOffset int, params fieldParam // when it sees a string, so if we see a different string type on the // wire, we change the universal type to match. if universalTag == tagPrintableString { - if params.tag == nil { + if t.class == classUniversal { switch t.tag { case tagIA5String, tagGeneralString, tagT61String, tagUTF8String: universalTag = t.tag @@ -652,7 +652,7 @@ func parseField(v reflect.Value, bytes []byte, initOffset int, params fieldParam // Special case for time: UTCTime and GeneralizedTime both map to the // Go type time.Time. - if universalTag == tagUTCTime && params.tag == nil && t.tag == tagGeneralizedTime { + if universalTag == tagUTCTime && t.tag == tagGeneralizedTime && t.class == classUniversal { universalTag = tagGeneralizedTime } diff --git a/src/encoding/asn1/asn1_test.go b/src/encoding/asn1/asn1_test.go index b94d59d36..4e864d08a 100644 --- a/src/encoding/asn1/asn1_test.go +++ b/src/encoding/asn1/asn1_test.go @@ -817,3 +817,51 @@ func TestStringSlice(t *testing.T) { } } } + +type explicitTaggedTimeTest struct { + Time time.Time `asn1:"explicit,tag:0"` +} + +var explicitTaggedTimeTestData = []struct { + in []byte + out explicitTaggedTimeTest +}{ + {[]byte{0x30, 0x11, 0xa0, 0xf, 0x17, 0xd, '9', '1', '0', '5', '0', '6', '1', '6', '4', '5', '4', '0', 'Z'}, + explicitTaggedTimeTest{time.Date(1991, 05, 06, 16, 45, 40, 0, time.UTC)}}, + {[]byte{0x30, 0x17, 0xa0, 0xf, 0x18, 0x13, '2', '0', '1', '0', '0', '1', '0', '2', '0', '3', '0', '4', '0', '5', '+', '0', '6', '0', '7'}, + explicitTaggedTimeTest{time.Date(2010, 01, 02, 03, 04, 05, 0, time.FixedZone("", 6*60*60+7*60))}}, +} + +func TestExplicitTaggedTime(t *testing.T) { + // Test that a time.Time will match either tagUTCTime or + // tagGeneralizedTime. + for i, test := range explicitTaggedTimeTestData { + var got explicitTaggedTimeTest + _, err := Unmarshal(test.in, &got) + if err != nil { + t.Errorf("Unmarshal failed at index %d %v", i, err) + } + if !got.Time.Equal(test.out.Time) { + t.Errorf("#%d: got %v, want %v", i, got.Time, test.out.Time) + } + } +} + +type implicitTaggedTimeTest struct { + Time time.Time `asn1:"tag:24"` +} + +func TestImplicitTaggedTime(t *testing.T) { + // An implicitly tagged time value, that happens to have an implicit + // tag equal to a GENERALIZEDTIME, should still be parsed as a UTCTime. + // (There's no "timeType" in fieldParameters to determine what type of + // time should be expected when implicitly tagged.) + der := []byte{0x30, 0x0f, 0x80 | 24, 0xd, '9', '1', '0', '5', '0', '6', '1', '6', '4', '5', '4', '0', 'Z'} + var result implicitTaggedTimeTest + if _, err := Unmarshal(der, &result); err != nil { + t.Fatalf("Error while parsing: %s", err) + } + if expected := time.Date(1991, 05, 06, 16, 45, 40, 0, time.UTC); !result.Time.Equal(expected) { + t.Errorf("Wrong result. Got %v, want %v", result.Time, expected) + } +} -- cgit v1.2.1 From a179a1934e4266e5331664877a2e3d8d34ffcc84 Mon Sep 17 00:00:00 2001 From: Alex Brainman Date: Fri, 10 Oct 2014 13:12:32 +1100 Subject: net: link skipped TestReadFromUDP to the issue LGTM=minux R=bradfitz, minux CC=golang-codereviews https://codereview.appspot.com/154220043 --- src/net/udp_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/net/udp_test.go b/src/net/udp_test.go index aa5751557..125bbca6c 100644 --- a/src/net/udp_test.go +++ b/src/net/udp_test.go @@ -38,7 +38,7 @@ func TestResolveUDPAddr(t *testing.T) { func TestReadFromUDP(t *testing.T) { switch runtime.GOOS { case "nacl", "plan9": - t.Skipf("skipping test on %q", runtime.GOOS) + t.Skipf("skipping test on %q, see issue 8916", runtime.GOOS) } ra, err := ResolveUDPAddr("udp", "127.0.0.1:7") -- cgit v1.2.1 From a40607efac0d82103032d20bc611d47736546bb7 Mon Sep 17 00:00:00 2001 From: Shenghou Ma Date: Fri, 10 Oct 2014 20:30:24 -0400 Subject: cmd/ld: fix off-by-one error when emitting symbol names I diffed the output of `nm -n gofmt' before and after this change, and verified that all changes are correct and all corrupted symbol names are fixed. Fixes issue 8906. LGTM=iant, cookieo9 R=golang-codereviews, iant, cookieo9 CC=golang-codereviews https://codereview.appspot.com/159750043 --- src/cmd/ld/macho.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'src') diff --git a/src/cmd/ld/macho.c b/src/cmd/ld/macho.c index 61306bb7c..fe7e10e46 100644 --- a/src/cmd/ld/macho.c +++ b/src/cmd/ld/macho.c @@ -590,8 +590,7 @@ machosymtab(void) if(strstr(s->extname, "·") == nil) { addstring(symstr, s->extname); } else { - p = s->extname; - while (*p++ != '\0') { + for(p = s->extname; *p; p++) { if((uchar)*p == 0xc2 && (uchar)*(p+1) == 0xb7) { adduint8(ctxt, symstr, '.'); p++; -- cgit v1.2.1 From cc1fecd64d6395d1692b0fc5936353d2e61b9c1c Mon Sep 17 00:00:00 2001 From: Alex Brainman Date: Sat, 11 Oct 2014 21:34:10 +1100 Subject: cmd/ld: correct pe section names if longer then 8 chars gcc 4.9.1 generates pe sections with names longer then 8 charters. From IMAGE_SECTION_HEADER definition: Name An 8-byte, null-padded UTF-8 string. There is no terminating null character if the string is exactly eight characters long. For longer names, this member contains a forward slash (/) followed by an ASCII representation of a decimal number that is an offset into the string table. Our current pe object file reader does not read string table when section names starts with /. Do that, so (issue 8811 example) c:\go\path\src\isssue8811>go build # isssue8811 isssue8811/glfw(.text): isssue8811/glfw(/76): not defined isssue8811/glfw(.text): undefined: isssue8811/glfw(/76) becomes c:\go\path\src\isssue8811>go build # isssue8811 isssue8811/glfw(.text): isssue8811/glfw(.rdata$.refptr._glfwInitialized): not defined isssue8811/glfw(.text): undefined: isssue8811/glfw(.rdata$.refptr._glfwInitialized) Small progress to Update issue 8811 LGTM=iant, jfrederich R=golang-codereviews, iant, jfrederich CC=golang-codereviews https://codereview.appspot.com/154210044 --- src/cmd/ld/ldpe.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'src') diff --git a/src/cmd/ld/ldpe.c b/src/cmd/ld/ldpe.c index 1b0591614..9257c243c 100644 --- a/src/cmd/ld/ldpe.c +++ b/src/cmd/ld/ldpe.c @@ -179,6 +179,15 @@ ldpe(Biobuf *f, char *pkg, int64 len, char *pn) Bseek(f, base+obj->fh.PointerToSymbolTable+sizeof(symbuf)*obj->fh.NumberOfSymbols, 0); if(Bread(f, obj->snames, l) != l) goto bad; + // rewrite section names if they start with / + for(i=0; i < obj->fh.NumberOfSections; i++) { + if(obj->sect[i].name == nil) + continue; + if(obj->sect[i].name[0] != '/') + continue; + l = atoi(obj->sect[i].name + 1); + obj->sect[i].name = (char*)&obj->snames[l]; + } // read symbols obj->pesym = mal(obj->fh.NumberOfSymbols*sizeof obj->pesym[0]); obj->npesym = obj->fh.NumberOfSymbols; -- cgit v1.2.1 From 1e2b6799cb92a21b4acfcd0c2fea4c8dab53d45f Mon Sep 17 00:00:00 2001 From: Alex Brainman Date: Sat, 11 Oct 2014 22:01:04 +1100 Subject: cmd/ld: do not assume that only pe section names start with '.' Our current pe object reader assumes that every symbol starting with '.' is section. It appeared to be true, until now gcc 4.9.1 generates some symbols with '.' at the front. Change that logic to check other symbol fields in addition to checking for '.'. I am not an expert here, but it seems reasonable to me. Added test, but it is only good, if tested with gcc 4.9.1. Otherwise the test PASSes regardless. Fixes issue 8811. Fixes issue 8856. LGTM=jfrederich, iant, stephen.gutekanst R=golang-codereviews, jfrederich, stephen.gutekanst, iant CC=alex.brainman, golang-codereviews https://codereview.appspot.com/152410043 --- src/cmd/ld/ldpe.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/cmd/ld/ldpe.c b/src/cmd/ld/ldpe.c index 9257c243c..4f5e51f2f 100644 --- a/src/cmd/ld/ldpe.c +++ b/src/cmd/ld/ldpe.c @@ -128,6 +128,7 @@ struct PeObj { }; static int map(PeObj *obj, PeSect *sect); +static int issect(PeSym *s); static int readsym(PeObj *obj, int i, PeSym **sym); void @@ -318,8 +319,8 @@ ldpe(Biobuf *f, char *pkg, int64 len, char *pn) // ld -r could generate multiple section symbols for the // same section but with different values, we have to take // that into account - if (obj->pesym[symindex].name[0] == '.') - rp->add += obj->pesym[symindex].value; + if(issect(&obj->pesym[symindex])) + rp->add += obj->pesym[symindex].value; } qsort(r, rsect->sh.NumberOfRelocations, sizeof r[0], rbyoff); @@ -327,12 +328,12 @@ ldpe(Biobuf *f, char *pkg, int64 len, char *pn) s->r = r; s->nr = rsect->sh.NumberOfRelocations; } - + // enter sub-symbols into symbol table. for(i=0; inpesym; i++) { if(obj->pesym[i].name == 0) continue; - if(obj->pesym[i].name[0] == '.') //skip section + if(issect(&obj->pesym[i])) continue; if(obj->pesym[i].sectnum > 0) { sect = &obj->sect[obj->pesym[i].sectnum-1]; @@ -430,6 +431,12 @@ map(PeObj *obj, PeSect *sect) return 0; } +static int +issect(PeSym *s) +{ + return s->sclass == IMAGE_SYM_CLASS_STATIC && s->type == 0 && s->name[0] == '.'; +} + static int readsym(PeObj *obj, int i, PeSym **y) { @@ -445,7 +452,7 @@ readsym(PeObj *obj, int i, PeSym **y) sym = &obj->pesym[i]; *y = sym; - if(sym->name[0] == '.') // .section + if(issect(sym)) name = obj->sect[sym->sectnum-1].sym->name; else { name = sym->name; -- cgit v1.2.1 From de92dd92e818cf8f79c589e0154a2da08106d032 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Mon, 13 Oct 2014 10:01:34 -0700 Subject: reflect: generated unrolled GC bitmask directly The code for a generated type is already generating an unrolled GC bitmask. Rather than unrolling the the source type bitmasks and copying them, just generate the required bitmask directly. Don't mark it as an unrolled GC program, since there is no need to do so. Fixes issue 8917. LGTM=rsc R=dvyukov, rsc CC=golang-codereviews https://codereview.appspot.com/156930044 --- src/reflect/all_test.go | 6 ++++++ src/reflect/type.go | 56 ++++++++++++++++++++++++++++++++----------------- 2 files changed, 43 insertions(+), 19 deletions(-) (limited to 'src') diff --git a/src/reflect/all_test.go b/src/reflect/all_test.go index f0cd6a412..6bdc9be9d 100644 --- a/src/reflect/all_test.go +++ b/src/reflect/all_test.go @@ -4018,3 +4018,9 @@ func TestInvalid(t *testing.T) { t.Errorf("field elem: IsValid=%v, Kind=%v, want false, Invalid", v.IsValid(), v.Kind()) } } + +// Issue 8917. +func TestLargeGCProg(t *testing.T) { + fv := ValueOf(func([256]*byte) {}) + fv.Call([]Value{ValueOf([256]*byte{})}) +} diff --git a/src/reflect/type.go b/src/reflect/type.go index a36c0ba60..821b60412 100644 --- a/src/reflect/type.go +++ b/src/reflect/type.go @@ -1514,20 +1514,36 @@ func (gc *gcProg) appendProg(t *rtype) { gc.size += t.size return } - nptr := t.size / unsafe.Sizeof(uintptr(0)) - var prog []byte - if t.kind&kindGCProg != 0 { - // Ensure that the runtime has unrolled GC program. - // TODO(rsc): Do not allocate. - unsafe_New(t) - // The program is stored in t.gc[0], skip unroll flag. - prog = (*[1 << 30]byte)(unsafe.Pointer(t.gc[0]))[1:] - } else { - // The mask is linked directly in t.gc. - prog = (*[2 * ptrSize]byte)(unsafe.Pointer(t.gc[0]))[:] - } - for i := uintptr(0); i < nptr; i++ { - gc.appendWord(extractGCWord(prog, i)) + switch t.Kind() { + default: + panic("reflect: non-pointer type marked as having pointers") + case Ptr, UnsafePointer, Chan, Func, Map: + gc.appendWord(bitsPointer) + case Slice: + gc.appendWord(bitsPointer) + gc.appendWord(bitsScalar) + gc.appendWord(bitsScalar) + case String: + gc.appendWord(bitsPointer) + gc.appendWord(bitsScalar) + case Array: + c := t.Len() + e := t.Elem().common() + for i := 0; i < c; i++ { + gc.appendProg(e) + } + case Interface: + gc.appendWord(bitsMultiWord) + if t.NumMethod() == 0 { + gc.appendWord(bitsEface) + } else { + gc.appendWord(bitsIface) + } + case Struct: + c := t.NumField() + for i := 0; i < c; i++ { + gc.appendProg(t.Field(i).Type.common()) + } } } @@ -1562,7 +1578,6 @@ func (gc *gcProg) finalize() unsafe.Pointer { gc.appendWord(extractGCWord(gc.gc, i)) } } - gc.gc = append([]byte{1}, gc.gc...) // prepend unroll flag return unsafe.Pointer(&gc.gc[0]) } @@ -1574,9 +1589,14 @@ func (gc *gcProg) align(a uintptr) { gc.size = align(gc.size, a) } +// These constants must stay in sync with ../runtime/mgc0.h. const ( - bitsScalar = 1 - bitsPointer = 2 + bitsScalar = 1 + bitsPointer = 2 + bitsMultiWord = 3 + + bitsIface = 2 + bitsEface = 3 ) // Make sure these routines stay in sync with ../../runtime/hashmap.go! @@ -1619,7 +1639,6 @@ func bucketOf(ktyp, etyp *rtype) *rtype { b := new(rtype) b.size = gc.size b.gc[0] = gc.finalize() - b.kind |= kindGCProg s := "bucket(" + *ktyp.string + "," + *etyp.string + ")" b.string = &s return b @@ -1821,7 +1840,6 @@ func funcLayout(t *rtype, rcvr *rtype) (frametype *rtype, argSize, retOffset uin x := new(rtype) x.size = gc.size x.gc[0] = gc.finalize() - x.kind |= kindGCProg var s string if rcvr != nil { s = "methodargs(" + *rcvr.string + ")(" + *t.string + ")" -- cgit v1.2.1 From 9d7d1d8b2867d2ce6ef44441d9d26970e3e27ce9 Mon Sep 17 00:00:00 2001 From: Rob Pike Date: Mon, 13 Oct 2014 10:27:51 -0700 Subject: net/rpc: fix mutex comment Fixes issue 8086. LGTM=bradfitz R=golang-codereviews, bradfitz CC=golang-codereviews https://codereview.appspot.com/153420044 --- src/net/rpc/client.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/net/rpc/client.go b/src/net/rpc/client.go index 21f79b068..d0c4a6921 100644 --- a/src/net/rpc/client.go +++ b/src/net/rpc/client.go @@ -41,10 +41,10 @@ type Call struct { type Client struct { codec ClientCodec - sending sync.Mutex + reqMutex sync.Mutex // protects following + request Request mutex sync.Mutex // protects following - request Request seq uint64 pending map[uint64]*Call closing bool // user has called Close @@ -69,8 +69,8 @@ type ClientCodec interface { } func (client *Client) send(call *Call) { - client.sending.Lock() - defer client.sending.Unlock() + client.reqMutex.Lock() + defer client.reqMutex.Unlock() // Register this call. client.mutex.Lock() @@ -146,7 +146,7 @@ func (client *Client) input() { } } // Terminate pending calls. - client.sending.Lock() + client.reqMutex.Lock() client.mutex.Lock() client.shutdown = true closing := client.closing @@ -162,7 +162,7 @@ func (client *Client) input() { call.done() } client.mutex.Unlock() - client.sending.Unlock() + client.reqMutex.Unlock() if debugLog && err != io.EOF && !closing { log.Println("rpc: client protocol error:", err) } -- cgit v1.2.1 From 0ab0939235f5b470e4031153b95cfabdcd4055eb Mon Sep 17 00:00:00 2001 From: David du Colombier <0intro@gmail.com> Date: Mon, 13 Oct 2014 20:39:46 +0200 Subject: os: handle 'no parent' error as IsNotExist on Plan 9 This error is returned by lib9p when removing a file without parent. It should fix TestRemoveAllRace when running on ramfs. LGTM=bradfitz, aram R=rsc, bradfitz, aram CC=golang-codereviews, mischief https://codereview.appspot.com/153410044 --- src/os/error_plan9.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/os/error_plan9.go b/src/os/error_plan9.go index 45cd74792..001cdfcf2 100644 --- a/src/os/error_plan9.go +++ b/src/os/error_plan9.go @@ -25,7 +25,8 @@ func isNotExist(err error) bool { case *LinkError: err = pe.Err } - return contains(err.Error(), "does not exist") || contains(err.Error(), "not found") || contains(err.Error(), "has been removed") + return contains(err.Error(), "does not exist") || contains(err.Error(), "not found") || + contains(err.Error(), "has been removed") || contains(err.Error(), "no parent") } func isPermission(err error) bool { -- cgit v1.2.1 From f4662d9cf2a785a6fd76c8b2680936c6bda00203 Mon Sep 17 00:00:00 2001 From: Casey Marshall Date: Mon, 13 Oct 2014 12:41:14 -0700 Subject: math/big: Fixes issue 8920 (*Rat).SetString checks for denominator. LGTM=gri R=golang-codereviews, gri CC=golang-codereviews https://codereview.appspot.com/159760043 Committer: Robert Griesemer --- src/math/big/rat.go | 3 +++ src/math/big/rat_test.go | 1 + 2 files changed, 4 insertions(+) (limited to 'src') diff --git a/src/math/big/rat.go b/src/math/big/rat.go index 0bcec3025..c5339fe44 100644 --- a/src/math/big/rat.go +++ b/src/math/big/rat.go @@ -552,6 +552,9 @@ func (z *Rat) SetString(s string) (*Rat, bool) { if z.b.abs, _, err = z.b.abs.scan(strings.NewReader(s), 10); err != nil { return nil, false } + if len(z.b.abs) == 0 { + return nil, false + } return z.norm(), true } diff --git a/src/math/big/rat_test.go b/src/math/big/rat_test.go index 598eac8cc..5dbbb3510 100644 --- a/src/math/big/rat_test.go +++ b/src/math/big/rat_test.go @@ -89,6 +89,7 @@ var setStringTests = []struct { {"53/70893980658822810696", "53/70893980658822810696", true}, {"106/141787961317645621392", "53/70893980658822810696", true}, {"204211327800791583.81095", "4084226556015831676219/20000", true}, + {in: "1/0", ok: false}, } func TestRatSetString(t *testing.T) { -- cgit v1.2.1 From d08112b182f09b6cec323a7264bbebba7bb425ff Mon Sep 17 00:00:00 2001 From: Adam Langley Date: Mon, 13 Oct 2014 18:35:53 -0700 Subject: crypto/x509: continue to recognise MaxPathLen of zero as "no value". In [1] the behaviour of encoding/asn1 with respect to marshaling optional integers was changed. Previously, a zero valued integer would be omitted when marshaling. After the change, if a default value was set then the integer would only be omitted if it was the default value. This changed the behaviour of crypto/x509 because Certificate.MaxPathLen has a default value of -1 and thus zero valued MaxPathLens would no longer be omitted when marshaling. This is arguably a bug-fix -- a value of zero for MaxPathLen is valid and meaningful and now could be expressed. However it broke users (including Docker) who were not setting MaxPathLen at all. This change again causes a zero-valued MaxPathLen to be omitted and introduces a ZeroMathPathLen member that indicates that, yes, one really does want a zero. This is ugly, but we value not breaking users. [1] https://code.google.com/p/go/source/detail?r=4218b3544610e8d9771b89126553177e32687adf LGTM=rsc R=rsc CC=golang-codereviews, golang-dev https://codereview.appspot.com/153420045 --- src/crypto/x509/x509.go | 15 ++++++++++- src/crypto/x509/x509_test.go | 63 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/crypto/x509/x509.go b/src/crypto/x509/x509.go index 6e57e913a..69a62e57d 100644 --- a/src/crypto/x509/x509.go +++ b/src/crypto/x509/x509.go @@ -494,6 +494,11 @@ type Certificate struct { BasicConstraintsValid bool // if true then the next two fields are valid. IsCA bool MaxPathLen int + // MaxPathLenZero indicates that BasicConstraintsValid==true and + // MaxPathLen==0 should be interpreted as an actual maximum path length + // of zero. Otherwise, that combination is interpreted as MaxPathLen + // not being set. + MaxPathLenZero bool SubjectKeyId []byte AuthorityKeyId []byte @@ -913,6 +918,7 @@ func parseCertificate(in *certificate) (*Certificate, error) { out.BasicConstraintsValid = true out.IsCA = constraints.IsCA out.MaxPathLen = constraints.MaxPathLen + out.MaxPathLenZero = out.MaxPathLen == 0 continue } case 17: @@ -1227,8 +1233,15 @@ func buildExtensions(template *Certificate) (ret []pkix.Extension, err error) { } if template.BasicConstraintsValid && !oidInExtensions(oidExtensionBasicConstraints, template.ExtraExtensions) { + // Leaving MaxPathLen as zero indicates that no maximum path + // length is desired, unless MaxPathLenZero is set. A value of + // -1 causes encoding/asn1 to omit the value as desired. + maxPathLen := template.MaxPathLen + if maxPathLen == 0 && !template.MaxPathLenZero { + maxPathLen = -1 + } ret[n].Id = oidExtensionBasicConstraints - ret[n].Value, err = asn1.Marshal(basicConstraints{template.IsCA, template.MaxPathLen}) + ret[n].Value, err = asn1.Marshal(basicConstraints{template.IsCA, maxPathLen}) ret[n].Critical = true if err != nil { return diff --git a/src/crypto/x509/x509_test.go b/src/crypto/x509/x509_test.go index abe86216f..4f5173fb5 100644 --- a/src/crypto/x509/x509_test.go +++ b/src/crypto/x509/x509_test.go @@ -953,6 +953,69 @@ func TestParseCertificateRequest(t *testing.T) { } } +func TestMaxPathLen(t *testing.T) { + block, _ := pem.Decode([]byte(pemPrivateKey)) + rsaPriv, err := ParsePKCS1PrivateKey(block.Bytes) + if err != nil { + t.Fatalf("Failed to parse private key: %s", err) + } + + template := &Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{ + CommonName: "Σ Acme Co", + }, + NotBefore: time.Unix(1000, 0), + NotAfter: time.Unix(100000, 0), + + BasicConstraintsValid: true, + IsCA: true, + } + + serialiseAndParse := func(template *Certificate) *Certificate { + derBytes, err := CreateCertificate(rand.Reader, template, template, &rsaPriv.PublicKey, rsaPriv) + if err != nil { + t.Fatalf("failed to create certificate: %s", err) + return nil + } + + cert, err := ParseCertificate(derBytes) + if err != nil { + t.Fatalf("failed to parse certificate: %s", err) + return nil + } + + return cert + } + + cert1 := serialiseAndParse(template) + if m := cert1.MaxPathLen; m != -1 { + t.Errorf("Omitting MaxPathLen didn't turn into -1, got %d", m) + } + if cert1.MaxPathLenZero { + t.Errorf("Omitting MaxPathLen resulted in MaxPathLenZero") + } + + template.MaxPathLen = 1 + cert2 := serialiseAndParse(template) + if m := cert2.MaxPathLen; m != 1 { + t.Errorf("Setting MaxPathLen didn't work. Got %d but set 1", m) + } + if cert2.MaxPathLenZero { + t.Errorf("Setting MaxPathLen resulted in MaxPathLenZero") + } + + template.MaxPathLen = 0 + template.MaxPathLenZero = true + cert3 := serialiseAndParse(template) + if m := cert3.MaxPathLen; m != 0 { + t.Errorf("Setting MaxPathLenZero didn't work, got %d", m) + } + if !cert3.MaxPathLenZero { + t.Errorf("Setting MaxPathLen to zero didn't result in MaxPathLenZero") + } +} + // This CSR was generated with OpenSSL: // openssl req -out CSR.csr -new -newkey rsa:2048 -nodes -keyout privateKey.key -config openssl.cnf // -- cgit v1.2.1 From 8ff4ffa5c47a6b3f98c09581fa0e43a49a3863b8 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Tue, 14 Oct 2014 09:22:47 -0700 Subject: runtime: a few optimizations of scanblock. Lowers gc pause time by 5-10% on test/bench/garbage LGTM=rsc, dvyukov R=rsc, dvyukov CC=golang-codereviews https://codereview.appspot.com/157810043 --- src/runtime/mgc0.c | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) (limited to 'src') diff --git a/src/runtime/mgc0.c b/src/runtime/mgc0.c index 0de7b1bf4..05cabe708 100644 --- a/src/runtime/mgc0.c +++ b/src/runtime/mgc0.c @@ -215,8 +215,7 @@ scanblock(byte *b, uintptr n, byte *ptrmask) for(i = 0; i < nelem(scanbuf); i++) { b = scanbuf[scanbufpos]; scanbuf[scanbufpos++] = nil; - if(scanbufpos == nelem(scanbuf)) - scanbufpos = 0; + scanbufpos %= nelem(scanbuf); if(b != nil) { n = arena_used - b; // scan until bitBoundary or BitsDead ptrmask = nil; // use GC bitmap for pointer info @@ -267,10 +266,13 @@ scanblock(byte *b, uintptr n, byte *ptrmask) break; // Consult GC bitmap. bits = *ptrbitp; - if((((uintptr)b+i)%(PtrSize*wordsPerBitmapByte)) != 0) { - ptrbitp--; - bits >>= gcBits; - } + + if(wordsPerBitmapByte != 2) + runtime·throw("alg doesn't work for wordsPerBitmapByte != 2"); + j = ((uintptr)b+i)/PtrSize & 1; + ptrbitp -= j; + bits >>= gcBits*j; + if((bits&bitBoundary) != 0 && i != 0) break; // reached beginning of the next object bits = (bits>>2)&BitsMask; @@ -293,10 +295,9 @@ scanblock(byte *b, uintptr n, byte *ptrmask) // Find the next pair of bits. if(ptrmask == nil) { bits = *ptrbitp; - if((((uintptr)b+i)%(PtrSize*wordsPerBitmapByte)) == 0) { - ptrbitp--; - bits >>= gcBits; - } + j = ((uintptr)b+i+PtrSize)/PtrSize & 1; + ptrbitp -= j; + bits >>= gcBits*j; bits = (bits>>2)&BitsMask; } else bits = (ptrmask[((i+PtrSize)/PtrSize)/4]>>((((i+PtrSize)/PtrSize)%4)*BitsPerPointer))&BitsMask; @@ -328,12 +329,13 @@ scanblock(byte *b, uintptr n, byte *ptrmask) // Check if it points into heap. if(obj == nil) continue; - if((uintptr)obj < PhysPageSize) { - s = nil; - goto badobj; - } - if(obj < arena_start || obj >= arena_used) + if(obj < arena_start || obj >= arena_used) { + if((uintptr)obj < PhysPageSize) { + s = nil; + goto badobj; + } continue; + } // Mark the object. obj = (byte*)((uintptr)obj & ~(PtrSize-1)); off = (uintptr*)obj - (uintptr*)arena_start; @@ -442,8 +444,7 @@ scanblock(byte *b, uintptr n, byte *ptrmask) PREFETCH(obj); p = scanbuf[scanbufpos]; scanbuf[scanbufpos++] = obj; - if(scanbufpos == nelem(scanbuf)) - scanbufpos = 0; + scanbufpos %= nelem(scanbuf); if(p == nil) continue; -- cgit v1.2.1 From 35ab6e03dd3628c0d78b7fc19883ff914e682aaf Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Tue, 14 Oct 2014 14:58:25 -0400 Subject: cmd/gc: fix 'make' in cmd/gc directory Right now, go tool 6g -A fails complaining about 'any' type. TBR=r CC=golang-codereviews https://codereview.appspot.com/156200044 --- src/cmd/gc/pgen.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src') diff --git a/src/cmd/gc/pgen.c b/src/cmd/gc/pgen.c index 50c03788e..39028e3f8 100644 --- a/src/cmd/gc/pgen.c +++ b/src/cmd/gc/pgen.c @@ -182,6 +182,8 @@ compile(Node *fn) yyerror("missing function body", fn); goto ret; } + if(debug['A']) + goto ret; emitptrargsmap(); goto ret; } -- cgit v1.2.1 From 14eceb73c0810d31081f7f70c68183440b52d12a Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Tue, 14 Oct 2014 16:31:09 -0400 Subject: cmd/gc: add 2-, 3-, 4-word write barrier specializations Assignments of 2-, 3-, and 4-word values were handled by individual MOV instructions (and for scalars still are). But if there are pointers involved, those assignments now go through the write barrier routine. Before this CL, they went to writebarrierfat, which calls memmove. Memmove is too much overhead for these small amounts of data. Instead, call writebarrierfat{2,3,4}, which are specialized for the specific amount of data being copied. Today the write barrier does not care which words are pointers, so size alone is enough to distinguish the cases. If we keep these distinctions in Go 1.5 we will need to expand them for all the pointer-vs-scalar possibilities, so the current 3 functions will become 3+7+15 = 25, still not a large burden (we deleted more morestack functions than that when we dropped segmented stacks). BenchmarkBinaryTree17 3250972583 3123910344 -3.91% BenchmarkFannkuch11 3067605223 2964737839 -3.35% BenchmarkFmtFprintfEmpty 101 96.0 -4.95% BenchmarkFmtFprintfString 267 235 -11.99% BenchmarkFmtFprintfInt 261 253 -3.07% BenchmarkFmtFprintfIntInt 444 402 -9.46% BenchmarkFmtFprintfPrefixedInt 374 346 -7.49% BenchmarkFmtFprintfFloat 472 449 -4.87% BenchmarkFmtManyArgs 1537 1476 -3.97% BenchmarkGobDecode 13986528 12432985 -11.11% BenchmarkGobEncode 13120323 12537420 -4.44% BenchmarkGzip 451925758 437500578 -3.19% BenchmarkGunzip 113267612 110053644 -2.84% BenchmarkHTTPClientServer 103151 77100 -25.26% BenchmarkJSONEncode 25002733 23435278 -6.27% BenchmarkJSONDecode 94213717 82568789 -12.36% BenchmarkMandelbrot200 4804246 4713070 -1.90% BenchmarkGoParse 4646114 4379456 -5.74% BenchmarkRegexpMatchEasy0_32 163 158 -3.07% BenchmarkRegexpMatchEasy0_1K 433 391 -9.70% BenchmarkRegexpMatchEasy1_32 154 138 -10.39% BenchmarkRegexpMatchEasy1_1K 1481 1132 -23.57% BenchmarkRegexpMatchMedium_32 282 270 -4.26% BenchmarkRegexpMatchMedium_1K 92421 86149 -6.79% BenchmarkRegexpMatchHard_32 5209 4718 -9.43% BenchmarkRegexpMatchHard_1K 158141 147921 -6.46% BenchmarkRevcomp 699818791 642222464 -8.23% BenchmarkTemplate 132402383 108269713 -18.23% BenchmarkTimeParse 509 478 -6.09% BenchmarkTimeFormat 462 456 -1.30% LGTM=r R=r CC=golang-codereviews https://codereview.appspot.com/156200043 --- src/cmd/gc/builtin.c | 5 ++++- src/cmd/gc/runtime.go | 3 +++ src/cmd/gc/walk.c | 32 ++++++++++++++++++++++---------- src/runtime/mgc0.go | 21 +++++++++++++++++++++ 4 files changed, 50 insertions(+), 11 deletions(-) (limited to 'src') diff --git a/src/cmd/gc/builtin.c b/src/cmd/gc/builtin.c index ee1ac1da4..17f80ebba 100644 --- a/src/cmd/gc/builtin.c +++ b/src/cmd/gc/builtin.c @@ -84,9 +84,12 @@ char *runtimeimport = "func @\"\".chansend1 (@\"\".chanType·1 *byte, @\"\".hchan·2 chan<- any, @\"\".elem·3 *any)\n" "func @\"\".closechan (@\"\".hchan·1 any)\n" "func @\"\".writebarrierptr (@\"\".dst·1 *any, @\"\".src·2 any)\n" - "func @\"\".writebarrieriface (@\"\".dst·1 *any, @\"\".src·2 any)\n" "func @\"\".writebarrierstring (@\"\".dst·1 *any, @\"\".src·2 any)\n" "func @\"\".writebarrierslice (@\"\".dst·1 *any, @\"\".src·2 any)\n" + "func @\"\".writebarrieriface (@\"\".dst·1 *any, @\"\".src·2 any)\n" + "func @\"\".writebarrierfat2 (@\"\".dst·1 *any, @\"\".src·2 any)\n" + "func @\"\".writebarrierfat3 (@\"\".dst·1 *any, @\"\".src·2 any)\n" + "func @\"\".writebarrierfat4 (@\"\".dst·1 *any, @\"\".src·2 any)\n" "func @\"\".writebarrierfat (@\"\".typ·1 *byte, @\"\".dst·2 *any, @\"\".src·3 *any)\n" "func @\"\".selectnbsend (@\"\".chanType·2 *byte, @\"\".hchan·3 chan<- any, @\"\".elem·4 *any) (? bool)\n" "func @\"\".selectnbrecv (@\"\".chanType·2 *byte, @\"\".elem·3 *any, @\"\".hchan·4 <-chan any) (? bool)\n" diff --git a/src/cmd/gc/runtime.go b/src/cmd/gc/runtime.go index fa927a58a..6ee5e2e36 100644 --- a/src/cmd/gc/runtime.go +++ b/src/cmd/gc/runtime.go @@ -112,6 +112,9 @@ func writebarrierptr(dst *any, src any) func writebarrierstring(dst *any, src any) func writebarrierslice(dst *any, src any) func writebarrieriface(dst *any, src any) +func writebarrierfat2(dst *any, src any) +func writebarrierfat3(dst *any, src any) +func writebarrierfat4(dst *any, src any) func writebarrierfat(typ *byte, dst *any, src *any) func selectnbsend(chanType *byte, hchan chan<- any, elem *any) bool diff --git a/src/cmd/gc/walk.c b/src/cmd/gc/walk.c index 713348c0c..5b5385d50 100644 --- a/src/cmd/gc/walk.c +++ b/src/cmd/gc/walk.c @@ -2040,21 +2040,32 @@ static Node* applywritebarrier(Node *n, NodeList **init) { Node *l, *r; + Type *t; if(n->left && n->right && needwritebarrier(n->left, n->right)) { + t = n->left->type; l = nod(OADDR, n->left, N); l->etype = 1; // addr does not escape - if(n->left->type->width == widthptr) { - n = mkcall1(writebarrierfn("writebarrierptr", n->left->type, n->right->type), T, init, + if(t->width == widthptr) { + n = mkcall1(writebarrierfn("writebarrierptr", t, n->right->type), T, init, + l, n->right); + } else if(t->etype == TSTRING) { + n = mkcall1(writebarrierfn("writebarrierstring", t, n->right->type), T, init, + l, n->right); + } else if(isslice(t)) { + n = mkcall1(writebarrierfn("writebarrierslice", t, n->right->type), T, init, + l, n->right); + } else if(isinter(t)) { + n = mkcall1(writebarrierfn("writebarrieriface", t, n->right->type), T, init, l, n->right); - } else if(n->left->type->etype == TSTRING) { - n = mkcall1(writebarrierfn("writebarrierstring", n->left->type, n->right->type), T, init, + } else if(t->width == 2*widthptr) { + n = mkcall1(writebarrierfn("writebarrierfat2", t, n->right->type), T, init, l, n->right); - } else if(isslice(n->left->type)) { - n = mkcall1(writebarrierfn("writebarrierslice", n->left->type, n->right->type), T, init, + } else if(t->width == 3*widthptr) { + n = mkcall1(writebarrierfn("writebarrierfat3", t, n->right->type), T, init, l, n->right); - } else if(isinter(n->left->type)) { - n = mkcall1(writebarrierfn("writebarrieriface", n->left->type, n->right->type), T, init, + } else if(t->width == 4*widthptr) { + n = mkcall1(writebarrierfn("writebarrierfat4", t, n->right->type), T, init, l, n->right); } else { r = n->right; @@ -2062,8 +2073,9 @@ applywritebarrier(Node *n, NodeList **init) r = r->left; r = nod(OADDR, r, N); r->etype = 1; // addr does not escape - n = mkcall1(writebarrierfn("writebarrierfat", n->left->type, r->left->type), T, init, - typename(n->left->type), l, r); + //warnl(n->lineno, "writebarrierfat %T %N", t, r); + n = mkcall1(writebarrierfn("writebarrierfat", t, r->left->type), T, init, + typename(t), l, r); } } return n; diff --git a/src/runtime/mgc0.go b/src/runtime/mgc0.go index 0e17599c2..3152b1fe1 100644 --- a/src/runtime/mgc0.go +++ b/src/runtime/mgc0.go @@ -109,6 +109,27 @@ func writebarrieriface(dst *[2]uintptr, src [2]uintptr) { dst[1] = src[1] } +//go:nosplit +func writebarrierfat2(dst *[2]uintptr, src [2]uintptr) { + dst[0] = src[0] + dst[1] = src[1] +} + +//go:nosplit +func writebarrierfat3(dst *[3]uintptr, src [3]uintptr) { + dst[0] = src[0] + dst[1] = src[1] + dst[2] = src[2] +} + +//go:nosplit +func writebarrierfat4(dst *[4]uintptr, src [4]uintptr) { + dst[0] = src[0] + dst[1] = src[1] + dst[2] = src[2] + dst[3] = src[3] +} + //go:nosplit func writebarrierfat(typ *_type, dst, src unsafe.Pointer) { memmove(dst, src, typ.size) -- cgit v1.2.1 From 7bc1b0d51a175a0ad567ada08d9e2024d9c6549e Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Tue, 14 Oct 2014 14:09:56 -0700 Subject: math/big: Allow non-prime modulus for ModInverse The inverse is defined whenever the element and the modulus are relatively prime. The code already handles this situation, but the spec does not. Test that it does indeed work. Fixes issue 8875 LGTM=agl R=agl CC=golang-codereviews https://codereview.appspot.com/155010043 --- src/math/big/int.go | 15 ++++++++------- src/math/big/int_test.go | 36 ++++++++++++++++++++++++++---------- 2 files changed, 34 insertions(+), 17 deletions(-) (limited to 'src') diff --git a/src/math/big/int.go b/src/math/big/int.go index fc53719d7..d22e39e7c 100644 --- a/src/math/big/int.go +++ b/src/math/big/int.go @@ -752,15 +752,16 @@ func (z *Int) Rand(rnd *rand.Rand, n *Int) *Int { return z } -// ModInverse sets z to the multiplicative inverse of g in the group ℤ/pℤ (where -// p is a prime) and returns z. -func (z *Int) ModInverse(g, p *Int) *Int { +// ModInverse sets z to the multiplicative inverse of g in the ring ℤ/nℤ +// and returns z. If g and n are not relatively prime, the result is undefined. +func (z *Int) ModInverse(g, n *Int) *Int { var d Int - d.GCD(z, nil, g, p) - // x and y are such that g*x + p*y = d. Since p is prime, d = 1. Taking - // that modulo p results in g*x = 1, therefore x is the inverse element. + d.GCD(z, nil, g, n) + // x and y are such that g*x + n*y = d. Since g and n are + // relatively prime, d = 1. Taking that modulo n results in + // g*x = 1, therefore x is the inverse element. if z.neg { - z.Add(z, p) + z.Add(z, n) } return z } diff --git a/src/math/big/int_test.go b/src/math/big/int_test.go index ec05fbb1c..6070cf325 100644 --- a/src/math/big/int_test.go +++ b/src/math/big/int_test.go @@ -1448,24 +1448,40 @@ func TestNot(t *testing.T) { var modInverseTests = []struct { element string - prime string + modulus string }{ - {"1", "7"}, - {"1", "13"}, + {"1234567", "458948883992"}, {"239487239847", "2410312426921032588552076022197566074856950548502459942654116941958108831682612228890093858261341614673227141477904012196503648957050582631942730706805009223062734745341073406696246014589361659774041027169249453200378729434170325843778659198143763193776859869524088940195577346119843545301547043747207749969763750084308926339295559968882457872412993810129130294592999947926365264059284647209730384947211681434464714438488520940127459844288859336526896320919633919"}, } func TestModInverse(t *testing.T) { - var element, prime Int + var element, modulus, gcd, inverse Int one := NewInt(1) for i, test := range modInverseTests { (&element).SetString(test.element, 10) - (&prime).SetString(test.prime, 10) - inverse := new(Int).ModInverse(&element, &prime) - inverse.Mul(inverse, &element) - inverse.Mod(inverse, &prime) - if inverse.Cmp(one) != 0 { - t.Errorf("#%d: failed (e·e^(-1)=%s)", i, inverse) + (&modulus).SetString(test.modulus, 10) + (&inverse).ModInverse(&element, &modulus) + (&inverse).Mul(&inverse, &element) + (&inverse).Mod(&inverse, &modulus) + if (&inverse).Cmp(one) != 0 { + t.Errorf("#%d: failed (e·e^(-1)=%s)", i, &inverse) + } + } + // exhaustive test for small values + for n := 2; n < 100; n++ { + (&modulus).SetInt64(int64(n)) + for x := 1; x < n; x++ { + (&element).SetInt64(int64(x)) + (&gcd).GCD(nil, nil, &element, &modulus) + if (&gcd).Cmp(one) != 0 { + continue + } + (&inverse).ModInverse(&element, &modulus) + (&inverse).Mul(&inverse, &element) + (&inverse).Mod(&inverse, &modulus) + if (&inverse).Cmp(one) != 0 { + t.Errorf("ModInverse(%d,%d)*%d%%%d=%d, not 1", &element, &modulus, &element, &modulus, &inverse) + } } } } -- cgit v1.2.1 From 1e77469a67f3a9905a43aaa0f2c189c7bcd1e558 Mon Sep 17 00:00:00 2001 From: Alex Brainman Date: Wed, 15 Oct 2014 11:11:11 +1100 Subject: runtime: handle all windows exception (second attempt) includes undo of 22318cd31d7d and also: - always use SetUnhandledExceptionFilter on windows-386; - crash when receive EXCEPTION_BREAKPOINT in exception handler. Fixes issue 8006. LGTM=rsc R=golang-codereviews, rsc CC=golang-codereviews https://codereview.appspot.com/155360043 --- src/runtime/defs_windows.go | 4 ++ src/runtime/defs_windows_386.h | 4 ++ src/runtime/defs_windows_amd64.h | 4 ++ src/runtime/os_windows.c | 38 ++++++++++++--- src/runtime/os_windows_386.c | 84 +++++++++++++++++++------------- src/runtime/os_windows_amd64.c | 97 ++++++++++++++++++++++++------------- src/runtime/sys_windows_386.s | 18 ++++++- src/runtime/sys_windows_amd64.s | 18 ++++++- src/runtime/syscall_windows_test.go | 39 +++++++++++++++ 9 files changed, 230 insertions(+), 76 deletions(-) (limited to 'src') diff --git a/src/runtime/defs_windows.go b/src/runtime/defs_windows.go index cb0f54d8a..7ce679741 100644 --- a/src/runtime/defs_windows.go +++ b/src/runtime/defs_windows.go @@ -49,6 +49,7 @@ const ( CONTEXT_FULL = C.CONTEXT_FULL EXCEPTION_ACCESS_VIOLATION = C.STATUS_ACCESS_VIOLATION + EXCEPTION_BREAKPOINT = C.STATUS_BREAKPOINT EXCEPTION_FLT_DENORMAL_OPERAND = C.STATUS_FLOAT_DENORMAL_OPERAND EXCEPTION_FLT_DIVIDE_BY_ZERO = C.STATUS_FLOAT_DIVIDE_BY_ZERO EXCEPTION_FLT_INEXACT_RESULT = C.STATUS_FLOAT_INEXACT_RESULT @@ -59,6 +60,9 @@ const ( INFINITE = C.INFINITE WAIT_TIMEOUT = C.WAIT_TIMEOUT + + EXCEPTION_CONTINUE_EXECUTION = C.EXCEPTION_CONTINUE_EXECUTION + EXCEPTION_CONTINUE_SEARCH = C.EXCEPTION_CONTINUE_SEARCH ) type SystemInfo C.SYSTEM_INFO diff --git a/src/runtime/defs_windows_386.h b/src/runtime/defs_windows_386.h index 295e422c6..2317c04f6 100644 --- a/src/runtime/defs_windows_386.h +++ b/src/runtime/defs_windows_386.h @@ -22,6 +22,7 @@ enum { CONTEXT_FULL = 0x10007, EXCEPTION_ACCESS_VIOLATION = 0xc0000005, + EXCEPTION_BREAKPOINT = 0x80000003, EXCEPTION_FLT_DENORMAL_OPERAND = 0xc000008d, EXCEPTION_FLT_DIVIDE_BY_ZERO = 0xc000008e, EXCEPTION_FLT_INEXACT_RESULT = 0xc000008f, @@ -32,6 +33,9 @@ enum { INFINITE = 0xffffffff, WAIT_TIMEOUT = 0x102, + + EXCEPTION_CONTINUE_EXECUTION = -0x1, + EXCEPTION_CONTINUE_SEARCH = 0x0, }; typedef struct SystemInfo SystemInfo; diff --git a/src/runtime/defs_windows_amd64.h b/src/runtime/defs_windows_amd64.h index 2516c8412..7f37a7a8c 100644 --- a/src/runtime/defs_windows_amd64.h +++ b/src/runtime/defs_windows_amd64.h @@ -22,6 +22,7 @@ enum { CONTEXT_FULL = 0x10000b, EXCEPTION_ACCESS_VIOLATION = 0xc0000005, + EXCEPTION_BREAKPOINT = 0x80000003, EXCEPTION_FLT_DENORMAL_OPERAND = 0xc000008d, EXCEPTION_FLT_DIVIDE_BY_ZERO = 0xc000008e, EXCEPTION_FLT_INEXACT_RESULT = 0xc000008f, @@ -32,6 +33,9 @@ enum { INFINITE = 0xffffffff, WAIT_TIMEOUT = 0x102, + + EXCEPTION_CONTINUE_EXECUTION = -0x1, + EXCEPTION_CONTINUE_SEARCH = 0x0, }; typedef struct SystemInfo SystemInfo; diff --git a/src/runtime/os_windows.c b/src/runtime/os_windows.c index 6337dde2a..b8b8eda5f 100644 --- a/src/runtime/os_windows.c +++ b/src/runtime/os_windows.c @@ -34,6 +34,7 @@ #pragma dynimport runtime·SetEvent SetEvent "kernel32.dll" #pragma dynimport runtime·SetProcessPriorityBoost SetProcessPriorityBoost "kernel32.dll" #pragma dynimport runtime·SetThreadPriority SetThreadPriority "kernel32.dll" +#pragma dynimport runtime·SetUnhandledExceptionFilter SetUnhandledExceptionFilter "kernel32.dll" #pragma dynimport runtime·SetWaitableTimer SetWaitableTimer "kernel32.dll" #pragma dynimport runtime·Sleep Sleep "kernel32.dll" #pragma dynimport runtime·SuspendThread SuspendThread "kernel32.dll" @@ -65,6 +66,7 @@ extern void *runtime·SetConsoleCtrlHandler; extern void *runtime·SetEvent; extern void *runtime·SetProcessPriorityBoost; extern void *runtime·SetThreadPriority; +extern void *runtime·SetUnhandledExceptionFilter; extern void *runtime·SetWaitableTimer; extern void *runtime·Sleep; extern void *runtime·SuspendThread; @@ -77,7 +79,9 @@ void *runtime·GetQueuedCompletionStatusEx; extern uintptr runtime·externalthreadhandlerp; void runtime·externalthreadhandler(void); -void runtime·sigtramp(void); +void runtime·exceptiontramp(void); +void runtime·firstcontinuetramp(void); +void runtime·lastcontinuetramp(void); #pragma textflag NOSPLIT uintptr @@ -106,12 +110,30 @@ void runtime·osinit(void) { void *kernel32; + void *addVectoredContinueHandler; + + kernel32 = runtime·stdcall1(runtime·LoadLibraryA, (uintptr)"kernel32.dll"); runtime·externalthreadhandlerp = (uintptr)runtime·externalthreadhandler; - runtime·stdcall2(runtime·AddVectoredExceptionHandler, 1, (uintptr)runtime·sigtramp); + runtime·stdcall2(runtime·AddVectoredExceptionHandler, 1, (uintptr)runtime·exceptiontramp); + addVectoredContinueHandler = nil; + if(kernel32 != nil) + addVectoredContinueHandler = runtime·stdcall2(runtime·GetProcAddress, (uintptr)kernel32, (uintptr)"AddVectoredContinueHandler"); + if(addVectoredContinueHandler == nil || sizeof(void*) == 4) { + // use SetUnhandledExceptionFilter for windows-386 or + // if VectoredContinueHandler is unavailable. + // note: SetUnhandledExceptionFilter handler won't be called, if debugging. + runtime·stdcall1(runtime·SetUnhandledExceptionFilter, (uintptr)runtime·lastcontinuetramp); + } else { + runtime·stdcall2(addVectoredContinueHandler, 1, (uintptr)runtime·firstcontinuetramp); + runtime·stdcall2(addVectoredContinueHandler, 0, (uintptr)runtime·lastcontinuetramp); + } + runtime·stdcall2(runtime·SetConsoleCtrlHandler, (uintptr)runtime·ctrlhandler, 1); + runtime·stdcall1(runtime·timeBeginPeriod, 1); + runtime·ncpu = getproccount(); // Windows dynamic priority boosting assumes that a process has different types @@ -120,7 +142,6 @@ runtime·osinit(void) // In such context dynamic priority boosting does nothing but harm, so we turn it off. runtime·stdcall2(runtime·SetProcessPriorityBoost, -1, 1); - kernel32 = runtime·stdcall1(runtime·LoadLibraryA, (uintptr)"kernel32.dll"); if(kernel32 != nil) { runtime·GetQueuedCompletionStatusEx = runtime·stdcall2(runtime·GetProcAddress, (uintptr)kernel32, (uintptr)"GetQueuedCompletionStatusEx"); } @@ -467,6 +488,7 @@ runtime·issigpanic(uint32 code) case EXCEPTION_FLT_INEXACT_RESULT: case EXCEPTION_FLT_OVERFLOW: case EXCEPTION_FLT_UNDERFLOW: + case EXCEPTION_BREAKPOINT: return 1; } return 0; @@ -475,10 +497,14 @@ runtime·issigpanic(uint32 code) void runtime·initsig(void) { - // following line keeps sigtramp alive at link stage + // following line keeps these functions alive at link stage // if there's a better way please write it here - void *p = runtime·sigtramp; - USED(p); + void *e = runtime·exceptiontramp; + void *f = runtime·firstcontinuetramp; + void *l = runtime·lastcontinuetramp; + USED(e); + USED(f); + USED(l); } uint32 diff --git a/src/runtime/os_windows_386.c b/src/runtime/os_windows_386.c index e2ae8db27..213582799 100644 --- a/src/runtime/os_windows_386.c +++ b/src/runtime/os_windows_386.c @@ -24,45 +24,63 @@ runtime·dumpregs(Context *r) runtime·printf("gs %x\n", r->SegGs); } -// Called by sigtramp from Windows VEH handler. -// Return value signals whether the exception has been handled (-1) -// or should be made available to other handlers in the chain (0). -uint32 -runtime·sighandler(ExceptionRecord *info, Context *r, G *gp) +bool +runtime·isgoexception(ExceptionRecord *info, Context *r) { - bool crash; - uintptr *sp; extern byte runtime·text[], runtime·etext[]; // Only handle exception if executing instructions in Go binary // (not Windows library code). if(r->Eip < (uint32)runtime·text || (uint32)runtime·etext < r->Eip) - return 0; - - if(gp != nil && runtime·issigpanic(info->ExceptionCode)) { - // Make it look like a call to the signal func. - // Have to pass arguments out of band since - // augmenting the stack frame would break - // the unwinding code. - gp->sig = info->ExceptionCode; - gp->sigcode0 = info->ExceptionInformation[0]; - gp->sigcode1 = info->ExceptionInformation[1]; - gp->sigpc = r->Eip; - - // Only push runtime·sigpanic if r->eip != 0. - // If r->eip == 0, probably panicked because of a - // call to a nil func. Not pushing that onto sp will - // make the trace look like a call to runtime·sigpanic instead. - // (Otherwise the trace will end at runtime·sigpanic and we - // won't get to see who faulted.) - if(r->Eip != 0) { - sp = (uintptr*)r->Esp; - *--sp = r->Eip; - r->Esp = (uintptr)sp; - } - r->Eip = (uintptr)runtime·sigpanic; - return -1; + return false; + + if(!runtime·issigpanic(info->ExceptionCode)) + return false; + + return true; +} + +// Called by sigtramp from Windows VEH handler. +// Return value signals whether the exception has been handled (EXCEPTION_CONTINUE_EXECUTION) +// or should be made available to other handlers in the chain (EXCEPTION_CONTINUE_SEARCH). +uint32 +runtime·exceptionhandler(ExceptionRecord *info, Context *r, G *gp) +{ + uintptr *sp; + + if(!runtime·isgoexception(info, r)) + return EXCEPTION_CONTINUE_SEARCH; + + // Make it look like a call to the signal func. + // Have to pass arguments out of band since + // augmenting the stack frame would break + // the unwinding code. + gp->sig = info->ExceptionCode; + gp->sigcode0 = info->ExceptionInformation[0]; + gp->sigcode1 = info->ExceptionInformation[1]; + gp->sigpc = r->Eip; + + // Only push runtime·sigpanic if r->eip != 0. + // If r->eip == 0, probably panicked because of a + // call to a nil func. Not pushing that onto sp will + // make the trace look like a call to runtime·sigpanic instead. + // (Otherwise the trace will end at runtime·sigpanic and we + // won't get to see who faulted.) + if(r->Eip != 0) { + sp = (uintptr*)r->Esp; + *--sp = r->Eip; + r->Esp = (uintptr)sp; } + r->Eip = (uintptr)runtime·sigpanic; + return EXCEPTION_CONTINUE_EXECUTION; +} + +// lastcontinuehandler is reached, because runtime cannot handle +// current exception. lastcontinuehandler will print crash info and exit. +uint32 +runtime·lastcontinuehandler(ExceptionRecord *info, Context *r, G *gp) +{ + bool crash; if(runtime·panicking) // traceback already printed runtime·exit(2); @@ -88,7 +106,7 @@ runtime·sighandler(ExceptionRecord *info, Context *r, G *gp) runtime·crash(); runtime·exit(2); - return -1; // not reached + return 0; // not reached } void diff --git a/src/runtime/os_windows_amd64.c b/src/runtime/os_windows_amd64.c index 261880d45..b96cf70d1 100644 --- a/src/runtime/os_windows_amd64.c +++ b/src/runtime/os_windows_amd64.c @@ -32,45 +32,76 @@ runtime·dumpregs(Context *r) runtime·printf("gs %X\n", (uint64)r->SegGs); } -// Called by sigtramp from Windows VEH handler. -// Return value signals whether the exception has been handled (-1) -// or should be made available to other handlers in the chain (0). -uint32 -runtime·sighandler(ExceptionRecord *info, Context *r, G *gp) +bool +runtime·isgoexception(ExceptionRecord *info, Context *r) { - bool crash; - uintptr *sp; extern byte runtime·text[], runtime·etext[]; // Only handle exception if executing instructions in Go binary // (not Windows library code). if(r->Rip < (uint64)runtime·text || (uint64)runtime·etext < r->Rip) - return 0; - - if(gp != nil && runtime·issigpanic(info->ExceptionCode)) { - // Make it look like a call to the signal func. - // Have to pass arguments out of band since - // augmenting the stack frame would break - // the unwinding code. - gp->sig = info->ExceptionCode; - gp->sigcode0 = info->ExceptionInformation[0]; - gp->sigcode1 = info->ExceptionInformation[1]; - gp->sigpc = r->Rip; - - // Only push runtime·sigpanic if r->rip != 0. - // If r->rip == 0, probably panicked because of a - // call to a nil func. Not pushing that onto sp will - // make the trace look like a call to runtime·sigpanic instead. - // (Otherwise the trace will end at runtime·sigpanic and we - // won't get to see who faulted.) - if(r->Rip != 0) { - sp = (uintptr*)r->Rsp; - *--sp = r->Rip; - r->Rsp = (uintptr)sp; - } - r->Rip = (uintptr)runtime·sigpanic; - return -1; + return false; + + if(!runtime·issigpanic(info->ExceptionCode)) + return false; + + return true; +} + +// Called by sigtramp from Windows VEH handler. +// Return value signals whether the exception has been handled (EXCEPTION_CONTINUE_EXECUTION) +// or should be made available to other handlers in the chain (EXCEPTION_CONTINUE_SEARCH). +uint32 +runtime·exceptionhandler(ExceptionRecord *info, Context *r, G *gp) +{ + uintptr *sp; + + if(!runtime·isgoexception(info, r)) + return EXCEPTION_CONTINUE_SEARCH; + + // Make it look like a call to the signal func. + // Have to pass arguments out of band since + // augmenting the stack frame would break + // the unwinding code. + gp->sig = info->ExceptionCode; + gp->sigcode0 = info->ExceptionInformation[0]; + gp->sigcode1 = info->ExceptionInformation[1]; + gp->sigpc = r->Rip; + + // Only push runtime·sigpanic if r->rip != 0. + // If r->rip == 0, probably panicked because of a + // call to a nil func. Not pushing that onto sp will + // make the trace look like a call to runtime·sigpanic instead. + // (Otherwise the trace will end at runtime·sigpanic and we + // won't get to see who faulted.) + if(r->Rip != 0) { + sp = (uintptr*)r->Rsp; + *--sp = r->Rip; + r->Rsp = (uintptr)sp; } + r->Rip = (uintptr)runtime·sigpanic; + return EXCEPTION_CONTINUE_EXECUTION; +} + +// It seems Windows searches ContinueHandler's list even +// if ExceptionHandler returns EXCEPTION_CONTINUE_EXECUTION. +// firstcontinuehandler will stop that search, +// if exceptionhandler did the same earlier. +uint32 +runtime·firstcontinuehandler(ExceptionRecord *info, Context *r, G *gp) +{ + USED(gp); + if(!runtime·isgoexception(info, r)) + return EXCEPTION_CONTINUE_SEARCH; + return EXCEPTION_CONTINUE_EXECUTION; +} + +// lastcontinuehandler is reached, because runtime cannot handle +// current exception. lastcontinuehandler will print crash info and exit. +uint32 +runtime·lastcontinuehandler(ExceptionRecord *info, Context *r, G *gp) +{ + bool crash; if(runtime·panicking) // traceback already printed runtime·exit(2); @@ -97,7 +128,7 @@ runtime·sighandler(ExceptionRecord *info, Context *r, G *gp) runtime·crash(); runtime·exit(2); - return -1; // not reached + return 0; // not reached } void diff --git a/src/runtime/sys_windows_386.s b/src/runtime/sys_windows_386.s index 1bf4d062a..932fe9dd2 100644 --- a/src/runtime/sys_windows_386.s +++ b/src/runtime/sys_windows_386.s @@ -73,6 +73,7 @@ TEXT runtime·setlasterror(SB),NOSPLIT,$0 // Called by Windows as a Vectored Exception Handler (VEH). // First argument is pointer to struct containing // exception record and context pointers. +// Handler function is stored in AX. // Return 0 for 'not handled', -1 for handled. TEXT runtime·sigtramp(SB),NOSPLIT,$0-0 MOVL ptrs+0(FP), CX @@ -84,6 +85,8 @@ TEXT runtime·sigtramp(SB),NOSPLIT,$0-0 MOVL SI, 20(SP) MOVL DI, 24(SP) + MOVL AX, SI // save handler address + // find g get_tls(DX) CMPL DX, $0 @@ -123,11 +126,10 @@ TEXT runtime·sigtramp(SB),NOSPLIT,$0-0 sigtramp_g0: MOVL 0(CX), BX // ExceptionRecord* MOVL 4(CX), CX // Context* - // call sighandler(ExceptionRecord*, Context*, G*) MOVL BX, 0(SP) MOVL CX, 4(SP) MOVL DX, 8(SP) - CALL runtime·sighandler(SB) + CALL SI // call handler // AX is set to report result back to Windows MOVL 12(SP), AX @@ -149,6 +151,18 @@ done: // RET 4 (return and pop 4 bytes parameters) BYTE $0xC2; WORD $4 RET // unreached; make assembler happy + +TEXT runtime·exceptiontramp(SB),NOSPLIT,$0 + MOVL $runtime·exceptionhandler(SB), AX + JMP runtime·sigtramp(SB) + +TEXT runtime·firstcontinuetramp(SB),NOSPLIT,$0-0 + // is never called + INT $3 + +TEXT runtime·lastcontinuetramp(SB),NOSPLIT,$0-0 + MOVL $runtime·lastcontinuehandler(SB), AX + JMP runtime·sigtramp(SB) TEXT runtime·ctrlhandler(SB),NOSPLIT,$0 PUSHL $runtime·ctrlhandler1(SB) diff --git a/src/runtime/sys_windows_amd64.s b/src/runtime/sys_windows_amd64.s index 05750398e..e6190ce68 100644 --- a/src/runtime/sys_windows_amd64.s +++ b/src/runtime/sys_windows_amd64.s @@ -99,6 +99,7 @@ TEXT runtime·setlasterror(SB),NOSPLIT,$0 // Called by Windows as a Vectored Exception Handler (VEH). // First argument is pointer to struct containing // exception record and context pointers. +// Handler function is stored in AX. // Return 0 for 'not handled', -1 for handled. TEXT runtime·sigtramp(SB),NOSPLIT,$0-0 // CX: PEXCEPTION_POINTERS ExceptionInfo @@ -116,6 +117,8 @@ TEXT runtime·sigtramp(SB),NOSPLIT,$0-0 MOVQ R14, 32(SP) MOVQ R15, 88(SP) + MOVQ AX, R15 // save handler address + // find g get_tls(DX) CMPQ DX, $0 @@ -157,11 +160,10 @@ TEXT runtime·sigtramp(SB),NOSPLIT,$0-0 sigtramp_g0: MOVQ 0(CX), BX // ExceptionRecord* MOVQ 8(CX), CX // Context* - // call sighandler(ExceptionRecord*, Context*, G*) MOVQ BX, 0(SP) MOVQ CX, 8(SP) MOVQ DX, 16(SP) - CALL runtime·sighandler(SB) + CALL R15 // call handler // AX is set to report result back to Windows MOVL 24(SP), AX @@ -187,6 +189,18 @@ done: RET +TEXT runtime·exceptiontramp(SB),NOSPLIT,$0 + MOVQ $runtime·exceptionhandler(SB), AX + JMP runtime·sigtramp(SB) + +TEXT runtime·firstcontinuetramp(SB),NOSPLIT,$0-0 + MOVQ $runtime·firstcontinuehandler(SB), AX + JMP runtime·sigtramp(SB) + +TEXT runtime·lastcontinuetramp(SB),NOSPLIT,$0-0 + MOVQ $runtime·lastcontinuehandler(SB), AX + JMP runtime·sigtramp(SB) + TEXT runtime·ctrlhandler(SB),NOSPLIT,$8 MOVQ CX, 16(SP) // spill MOVQ $runtime·ctrlhandler1(SB), CX diff --git a/src/runtime/syscall_windows_test.go b/src/runtime/syscall_windows_test.go index 9ed016ccc..ce8a9ec1b 100644 --- a/src/runtime/syscall_windows_test.go +++ b/src/runtime/syscall_windows_test.go @@ -494,3 +494,42 @@ func TestOutputDebugString(t *testing.T) { p := syscall.StringToUTF16Ptr("testing OutputDebugString") d.Proc("OutputDebugStringW").Call(uintptr(unsafe.Pointer(p))) } + +func TestRaiseException(t *testing.T) { + o := executeTest(t, raiseExceptionSource, nil) + if strings.Contains(o, "RaiseException should not return") { + t.Fatalf("RaiseException did not crash program: %v", o) + } + if !strings.Contains(o, "Exception 0xbad") { + t.Fatalf("No stack trace: %v", o) + } +} + +const raiseExceptionSource = ` +package main +import "syscall" +func main() { + const EXCEPTION_NONCONTINUABLE = 1 + mod := syscall.MustLoadDLL("kernel32.dll") + proc := mod.MustFindProc("RaiseException") + proc.Call(0xbad, EXCEPTION_NONCONTINUABLE, 0, 0) + println("RaiseException should not return") +} +` + +func TestZeroDivisionException(t *testing.T) { + o := executeTest(t, zeroDivisionExceptionSource, nil) + if !strings.Contains(o, "panic: runtime error: integer divide by zero") { + t.Fatalf("No stack trace: %v", o) + } +} + +const zeroDivisionExceptionSource = ` +package main +func main() { + x := 1 + y := 0 + z := x / y + println(z) +} +` -- cgit v1.2.1 From 9ddeb99a986ff9c6dacc218359d9aa09111cbfa6 Mon Sep 17 00:00:00 2001 From: Chris Manghane Date: Tue, 14 Oct 2014 19:12:10 -0700 Subject: cmd/gc: check for initialization cycles in method values Fixes issue 7960. LGTM=rsc R=rsc CC=golang-codereviews, gri https://codereview.appspot.com/159800045 --- src/cmd/gc/sinit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/cmd/gc/sinit.c b/src/cmd/gc/sinit.c index 508747e5a..f050026d9 100644 --- a/src/cmd/gc/sinit.c +++ b/src/cmd/gc/sinit.c @@ -207,7 +207,7 @@ init2(Node *n, NodeList **out) if(n->op == OCLOSURE) init2list(n->closure->nbody, out); - if(n->op == ODOTMETH) + if(n->op == ODOTMETH || n->op == OCALLPART) init2(n->type->nname, out); } -- cgit v1.2.1 From 3d4a92a2e6eb8efa60e84bd53be2280288e0bf2f Mon Sep 17 00:00:00 2001 From: Rob Pike Date: Tue, 14 Oct 2014 20:03:35 -0700 Subject: encoding/gob: make encoding structs a little faster FieldByIndex never returns an invalid Value, so the validity test can be avoided if the field is not indirect. BenchmarkGobEncode 12768642 12424022 -2.70% BenchmarkGobEncode 60.11 61.78 1.03x LGTM=rsc R=rsc CC=golang-codereviews https://codereview.appspot.com/158890045 --- src/encoding/gob/encode.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/encoding/gob/encode.go b/src/encoding/gob/encode.go index b7bf8b002..04a85410c 100644 --- a/src/encoding/gob/encode.go +++ b/src/encoding/gob/encode.go @@ -281,15 +281,16 @@ func (enc *Encoder) encodeStruct(b *bytes.Buffer, engine *encEngine, value refle field := value.FieldByIndex(instr.index) if instr.indir > 0 { field = encIndirect(field, instr.indir) - } - if !valid(field) { - continue + // TODO: Is field guaranteed valid? If so we could avoid this check. + if !valid(field) { + continue + } } instr.op(instr, state, field) } } -// encodeArray encodes the array whose 0th element is at p. +// encodeArray encodes an array. func (enc *Encoder) encodeArray(b *bytes.Buffer, value reflect.Value, op encOp, elemIndir int, length int) { state := enc.newEncoderState(b) defer enc.freeEncoderState(state) @@ -300,6 +301,7 @@ func (enc *Encoder) encodeArray(b *bytes.Buffer, value reflect.Value, op encOp, elem := value.Index(i) if elemIndir > 0 { elem = encIndirect(elem, elemIndir) + // TODO: Is elem guaranteed valid? If so we could avoid this check. if !valid(elem) { errorf("encodeArray: nil element") } -- cgit v1.2.1 From a789400a8e4b1ad09076edacc1efeaef2446e619 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Tue, 14 Oct 2014 23:24:32 -0400 Subject: cmd/gc, runtime: fix race, nacl for writebarrier changes The racewalk code was not updated for the new write barriers. Make it more future-proof. The new write barrier code assumed that +1 pointer would be aligned properly for any type that might follow, but that's not true on 32-bit systems where some types are 64-bit aligned. The only system like that today is nacl/amd64p32. Insert a dummy pointer so that the ambiguously typed value is at +2 pointers, which is always max-aligned. LGTM=r R=r CC=golang-codereviews, iant, khr https://codereview.appspot.com/158890046 --- src/cmd/gc/builtin.c | 6 +++--- src/cmd/gc/racewalk.c | 7 +------ src/cmd/gc/runtime.go | 10 +++++++--- src/cmd/gc/walk.c | 6 +++--- src/runtime/mgc0.go | 6 +++--- 5 files changed, 17 insertions(+), 18 deletions(-) (limited to 'src') diff --git a/src/cmd/gc/builtin.c b/src/cmd/gc/builtin.c index 17f80ebba..5fbb4f0cf 100644 --- a/src/cmd/gc/builtin.c +++ b/src/cmd/gc/builtin.c @@ -87,9 +87,9 @@ char *runtimeimport = "func @\"\".writebarrierstring (@\"\".dst·1 *any, @\"\".src·2 any)\n" "func @\"\".writebarrierslice (@\"\".dst·1 *any, @\"\".src·2 any)\n" "func @\"\".writebarrieriface (@\"\".dst·1 *any, @\"\".src·2 any)\n" - "func @\"\".writebarrierfat2 (@\"\".dst·1 *any, @\"\".src·2 any)\n" - "func @\"\".writebarrierfat3 (@\"\".dst·1 *any, @\"\".src·2 any)\n" - "func @\"\".writebarrierfat4 (@\"\".dst·1 *any, @\"\".src·2 any)\n" + "func @\"\".writebarrierfat2 (@\"\".dst·1 *any, _ *byte, @\"\".src·3 any)\n" + "func @\"\".writebarrierfat3 (@\"\".dst·1 *any, _ *byte, @\"\".src·3 any)\n" + "func @\"\".writebarrierfat4 (@\"\".dst·1 *any, _ *byte, @\"\".src·3 any)\n" "func @\"\".writebarrierfat (@\"\".typ·1 *byte, @\"\".dst·2 *any, @\"\".src·3 *any)\n" "func @\"\".selectnbsend (@\"\".chanType·2 *byte, @\"\".hchan·3 chan<- any, @\"\".elem·4 *any) (? bool)\n" "func @\"\".selectnbrecv (@\"\".chanType·2 *byte, @\"\".elem·3 *any, @\"\".hchan·4 <-chan any) (? bool)\n" diff --git a/src/cmd/gc/racewalk.c b/src/cmd/gc/racewalk.c index cb98ca247..c9e27fe56 100644 --- a/src/cmd/gc/racewalk.c +++ b/src/cmd/gc/racewalk.c @@ -210,12 +210,7 @@ racewalknode(Node **np, NodeList **init, int wr, int skip) case OCALLFUNC: // Instrument dst argument of runtime.writebarrier* calls // as we do not instrument runtime code. - if(n->left->sym != S && n->left->sym->pkg == runtimepkg && - (strcmp(n->left->sym->name, "writebarrierptr") == 0 || - strcmp(n->left->sym->name, "writebarrierstring") == 0 || - strcmp(n->left->sym->name, "writebarrierslice") == 0 || - strcmp(n->left->sym->name, "writebarrieriface") == 0 || - strcmp(n->left->sym->name, "writebarrierfat") == 0)) { + if(n->left->sym != S && n->left->sym->pkg == runtimepkg && strncmp(n->left->sym->name, "writebarrier", 12) == 0) { // Find the dst argument. // The list can be reordered, so it's not necessary just the first or the second element. for(l = n->list; l; l = l->next) { diff --git a/src/cmd/gc/runtime.go b/src/cmd/gc/runtime.go index 6ee5e2e36..86afe67f1 100644 --- a/src/cmd/gc/runtime.go +++ b/src/cmd/gc/runtime.go @@ -112,9 +112,13 @@ func writebarrierptr(dst *any, src any) func writebarrierstring(dst *any, src any) func writebarrierslice(dst *any, src any) func writebarrieriface(dst *any, src any) -func writebarrierfat2(dst *any, src any) -func writebarrierfat3(dst *any, src any) -func writebarrierfat4(dst *any, src any) + +// The unused *byte argument makes sure that src is 2-pointer-aligned, +// which is the maximum alignment on NaCl amd64p32 +// (and possibly on 32-bit systems if we start 64-bit aligning uint64s). +func writebarrierfat2(dst *any, _ *byte, src any) +func writebarrierfat3(dst *any, _ *byte, src any) +func writebarrierfat4(dst *any, _ *byte, src any) func writebarrierfat(typ *byte, dst *any, src *any) func selectnbsend(chanType *byte, hchan chan<- any, elem *any) bool diff --git a/src/cmd/gc/walk.c b/src/cmd/gc/walk.c index 5b5385d50..241d7d74a 100644 --- a/src/cmd/gc/walk.c +++ b/src/cmd/gc/walk.c @@ -2060,13 +2060,13 @@ applywritebarrier(Node *n, NodeList **init) l, n->right); } else if(t->width == 2*widthptr) { n = mkcall1(writebarrierfn("writebarrierfat2", t, n->right->type), T, init, - l, n->right); + l, nodnil(), n->right); } else if(t->width == 3*widthptr) { n = mkcall1(writebarrierfn("writebarrierfat3", t, n->right->type), T, init, - l, n->right); + l, nodnil(), n->right); } else if(t->width == 4*widthptr) { n = mkcall1(writebarrierfn("writebarrierfat4", t, n->right->type), T, init, - l, n->right); + l, nodnil(), n->right); } else { r = n->right; while(r->op == OCONVNOP) diff --git a/src/runtime/mgc0.go b/src/runtime/mgc0.go index 3152b1fe1..3a7204b54 100644 --- a/src/runtime/mgc0.go +++ b/src/runtime/mgc0.go @@ -110,20 +110,20 @@ func writebarrieriface(dst *[2]uintptr, src [2]uintptr) { } //go:nosplit -func writebarrierfat2(dst *[2]uintptr, src [2]uintptr) { +func writebarrierfat2(dst *[2]uintptr, _ *byte, src [2]uintptr) { dst[0] = src[0] dst[1] = src[1] } //go:nosplit -func writebarrierfat3(dst *[3]uintptr, src [3]uintptr) { +func writebarrierfat3(dst *[3]uintptr, _ *byte, src [3]uintptr) { dst[0] = src[0] dst[1] = src[1] dst[2] = src[2] } //go:nosplit -func writebarrierfat4(dst *[4]uintptr, src [4]uintptr) { +func writebarrierfat4(dst *[4]uintptr, _ *byte, src [4]uintptr) { dst[0] = src[0] dst[1] = src[1] dst[2] = src[2] -- cgit v1.2.1 From f617e5520bbfe677fed06a1fd2bfedd5ef61a5ef Mon Sep 17 00:00:00 2001 From: Jens Frederich Date: Tue, 14 Oct 2014 23:24:58 -0400 Subject: go/build: Return MultiplePackageError on importing a dir containing multiple packages When the Import function in go/build encounters a directory without any buildable Go source files, it returns a handy NoGoError. Now if, instead it encounters multiple Go source files from multiple packages, it returns a handy MultiplePackageError. A new test for NoGoError and MultiplePackageError is also provided. Fixes issue 8286. LGTM=adg, rsc R=bradfitz, rsc, adg CC=golang-codereviews https://codereview.appspot.com/155050043 Committer: Russ Cox --- src/go/build/build.go | 15 ++++++++++++++- src/go/build/build_test.go | 14 ++++++++++++++ src/go/build/testdata/empty/dummy | 0 src/go/build/testdata/multi/file.go | 5 +++++ src/go/build/testdata/multi/file_appengine.go | 5 +++++ 5 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 src/go/build/testdata/empty/dummy create mode 100644 src/go/build/testdata/multi/file.go create mode 100644 src/go/build/testdata/multi/file_appengine.go (limited to 'src') diff --git a/src/go/build/build.go b/src/go/build/build.go index 3ac798083..7a51cf3c0 100644 --- a/src/go/build/build.go +++ b/src/go/build/build.go @@ -417,6 +417,19 @@ func (e *NoGoError) Error() string { return "no buildable Go source files in " + e.Dir } +// MultiplePackageError describes a directory containing +// multiple buildable Go source files for multiple packages. +type MultiplePackageError struct { + Dir string // directory containing files + Packages []string // package names found + Files []string // corresponding files: Files[i] declares package Packages[i] +} + +func (e *MultiplePackageError) Error() string { + // Error string limited to two entries for compatibility. + return fmt.Sprintf("found packages %s (%s) and %s (%s) in %s", e.Packages[0], e.Files[0], e.Packages[1], e.Files[1], e.Dir) +} + func nameExt(name string) string { i := strings.LastIndex(name, ".") if i < 0 { @@ -675,7 +688,7 @@ Found: p.Name = pkg firstFile = name } else if pkg != p.Name { - return p, fmt.Errorf("found packages %s (%s) and %s (%s) in %s", p.Name, firstFile, pkg, name, p.Dir) + return p, &MultiplePackageError{p.Dir, []string{firstFile, name}, []string{p.Name, pkg}} } if pf.Doc != nil && p.Doc == "" { p.Doc = doc.Synopsis(pf.Doc.Text()) diff --git a/src/go/build/build_test.go b/src/go/build/build_test.go index 23ce89b4b..43d09cbd1 100644 --- a/src/go/build/build_test.go +++ b/src/go/build/build_test.go @@ -85,6 +85,20 @@ func TestEmptyImport(t *testing.T) { } } +func TestEmptyFolderImport(t *testing.T) { + _, err := Import(".", "testdata/empty", 0) + if _, ok := err.(*NoGoError); !ok { + t.Fatal(`Import("testdata/empty") did not return NoGoError.`) + } +} + +func TestMultiplePackageImport(t *testing.T) { + _, err := Import(".", "testdata/multi", 0) + if _, ok := err.(*MultiplePackageError); !ok { + t.Fatal(`Import("testdata/multi") did not return MultiplePackageError.`) + } +} + func TestLocalDirectory(t *testing.T) { cwd, err := os.Getwd() if err != nil { diff --git a/src/go/build/testdata/empty/dummy b/src/go/build/testdata/empty/dummy new file mode 100644 index 000000000..e69de29bb diff --git a/src/go/build/testdata/multi/file.go b/src/go/build/testdata/multi/file.go new file mode 100644 index 000000000..ee946eb2a --- /dev/null +++ b/src/go/build/testdata/multi/file.go @@ -0,0 +1,5 @@ +// Test data - not compiled. + +package main + +func main() {} diff --git a/src/go/build/testdata/multi/file_appengine.go b/src/go/build/testdata/multi/file_appengine.go new file mode 100644 index 000000000..4ea31e703 --- /dev/null +++ b/src/go/build/testdata/multi/file_appengine.go @@ -0,0 +1,5 @@ +// Test data - not compiled. + +package test_package + +func init() {} -- cgit v1.2.1 From 601416627d7e4d21399144e43ca539707badbf61 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Tue, 14 Oct 2014 23:25:12 -0400 Subject: liblink: require DATA lines to be ordered by offset, with no overlap The assembler could give a better error, but this one is good enough for now. Fixes issue 8880. LGTM=r R=r CC=golang-codereviews https://codereview.appspot.com/153610043 --- src/cmd/5a/lex.c | 1 + src/cmd/6a/lex.c | 1 + src/cmd/8a/lex.c | 1 + src/liblink/data.c | 2 ++ 4 files changed, 5 insertions(+) (limited to 'src') diff --git a/src/cmd/5a/lex.c b/src/cmd/5a/lex.c index 84a17d155..9c6970947 100644 --- a/src/cmd/5a/lex.c +++ b/src/cmd/5a/lex.c @@ -85,6 +85,7 @@ main(int argc, char *argv[]) ctxt = linknew(&linkarm); ctxt->diag = yyerror; ctxt->bso = &bstdout; + ctxt->enforce_data_order = 1; Binit(&bstdout, 1, OWRITE); listinit5(); fmtinstall('L', Lconv); diff --git a/src/cmd/6a/lex.c b/src/cmd/6a/lex.c index b50e1622e..8973d6974 100644 --- a/src/cmd/6a/lex.c +++ b/src/cmd/6a/lex.c @@ -101,6 +101,7 @@ main(int argc, char *argv[]) ctxt = linknew(thelinkarch); ctxt->diag = yyerror; ctxt->bso = &bstdout; + ctxt->enforce_data_order = 1; Binit(&bstdout, 1, OWRITE); listinit6(); fmtinstall('L', Lconv); diff --git a/src/cmd/8a/lex.c b/src/cmd/8a/lex.c index 807e48cb5..6ce6a18ab 100644 --- a/src/cmd/8a/lex.c +++ b/src/cmd/8a/lex.c @@ -90,6 +90,7 @@ main(int argc, char *argv[]) ctxt = linknew(&link386); ctxt->diag = yyerror; ctxt->bso = &bstdout; + ctxt->enforce_data_order = 1; Binit(&bstdout, 1, OWRITE); listinit8(); fmtinstall('L', Lconv); diff --git a/src/liblink/data.c b/src/liblink/data.c index 4504f4171..e5efa2eb2 100644 --- a/src/liblink/data.c +++ b/src/liblink/data.c @@ -83,6 +83,8 @@ savedata(Link *ctxt, LSym *s, Prog *p, char *pn) siz = ctxt->arch->datasize(p); if(off < 0 || siz < 0 || off >= 1<<30 || siz >= 100) mangle(pn); + if(ctxt->enforce_data_order && off < s->np) + ctxt->diag("data out of order (already have %d)\n%P", p); symgrow(ctxt, s, off+siz); if(p->to.type == ctxt->arch->D_FCONST) { -- cgit v1.2.1 From c5799fafe77a6561653f8faccfa7ff5128c968ca Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Wed, 15 Oct 2014 06:20:55 -0700 Subject: crypto/x509: correct field name in comment Fixes issue 8936. LGTM=bradfitz R=agl, bradfitz CC=golang-codereviews https://codereview.appspot.com/152590043 --- src/crypto/x509/x509.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/crypto/x509/x509.go b/src/crypto/x509/x509.go index 69a62e57d..7a37b98e3 100644 --- a/src/crypto/x509/x509.go +++ b/src/crypto/x509/x509.go @@ -1670,7 +1670,7 @@ var oidExtensionRequest = asn1.ObjectIdentifier{1, 2, 840, 113549, 1, 9, 14} // CreateCertificateRequest creates a new certificate based on a template. The // following members of template are used: Subject, Attributes, -// SignatureAlgorithm, Extension, DNSNames, EmailAddresses, and IPAddresses. +// SignatureAlgorithm, Extensions, DNSNames, EmailAddresses, and IPAddresses. // The private key is the private key of the signer. // // The returned slice is the certificate request in DER encoding. -- cgit v1.2.1 From 713940ccabd51693d5a5bbdfa3acfae9e7235e35 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 15 Oct 2014 17:51:12 +0200 Subject: net/http: don't reuse a server connection after any Write errors Fixes Issue 8534 LGTM=adg R=adg CC=golang-codereviews https://codereview.appspot.com/149340044 --- src/net/http/serve_test.go | 97 ++++++++++++++++++++++++++++++++++++++++++++++ src/net/http/server.go | 32 +++++++++++++-- 2 files changed, 126 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go index 702bffdc1..bb44ac853 100644 --- a/src/net/http/serve_test.go +++ b/src/net/http/serve_test.go @@ -2659,6 +2659,103 @@ func TestCloseWrite(t *testing.T) { } } +// This verifies that a handler can Flush and then Hijack. +// +// An similar test crashed once during development, but it was only +// testing this tangentially and temporarily until another TODO was +// fixed. +// +// So add an explicit test for this. +func TestServerFlushAndHijack(t *testing.T) { + defer afterTest(t) + ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { + io.WriteString(w, "Hello, ") + w.(Flusher).Flush() + conn, buf, _ := w.(Hijacker).Hijack() + buf.WriteString("6\r\nworld!\r\n0\r\n\r\n") + if err := buf.Flush(); err != nil { + t.Error(err) + } + if err := conn.Close(); err != nil { + t.Error(err) + } + })) + defer ts.Close() + res, err := Get(ts.URL) + if err != nil { + t.Fatal(err) + } + defer res.Body.Close() + all, err := ioutil.ReadAll(res.Body) + if err != nil { + t.Fatal(err) + } + if want := "Hello, world!"; string(all) != want { + t.Errorf("Got %q; want %q", all, want) + } +} + +// golang.org/issue/8534 -- the Server shouldn't reuse a connection +// for keep-alive after it's seen any Write error (e.g. a timeout) on +// that net.Conn. +// +// To test, verify we don't timeout or see fewer unique client +// addresses (== unique connections) than requests. +func TestServerKeepAliveAfterWriteError(t *testing.T) { + if testing.Short() { + t.Skip("skipping in -short mode") + } + defer afterTest(t) + const numReq = 3 + addrc := make(chan string, numReq) + ts := httptest.NewUnstartedServer(HandlerFunc(func(w ResponseWriter, r *Request) { + addrc <- r.RemoteAddr + time.Sleep(500 * time.Millisecond) + w.(Flusher).Flush() + })) + ts.Config.WriteTimeout = 250 * time.Millisecond + ts.Start() + defer ts.Close() + + errc := make(chan error, numReq) + go func() { + defer close(errc) + for i := 0; i < numReq; i++ { + res, err := Get(ts.URL) + if res != nil { + res.Body.Close() + } + errc <- err + } + }() + + timeout := time.NewTimer(numReq * 2 * time.Second) // 4x overkill + defer timeout.Stop() + addrSeen := map[string]bool{} + numOkay := 0 + for { + select { + case v := <-addrc: + addrSeen[v] = true + case err, ok := <-errc: + if !ok { + if len(addrSeen) != numReq { + t.Errorf("saw %d unique client addresses; want %d", len(addrSeen), numReq) + } + if numOkay != 0 { + t.Errorf("got %d successful client requests; want 0", numOkay) + } + return + } + if err == nil { + numOkay++ + } + case <-timeout.C: + t.Fatal("timeout waiting for requests to complete") + } + } +} + func BenchmarkClientServer(b *testing.B) { b.ReportAllocs() b.StopTimer() diff --git a/src/net/http/server.go b/src/net/http/server.go index b5959f732..008d5aa7a 100644 --- a/src/net/http/server.go +++ b/src/net/http/server.go @@ -114,6 +114,8 @@ type conn struct { remoteAddr string // network address of remote side server *Server // the Server on which the connection arrived rwc net.Conn // i/o connection + w io.Writer // checkConnErrorWriter's copy of wrc, not zeroed on Hijack + werr error // any errors writing to w sr liveSwitchReader // where the LimitReader reads from; usually the rwc lr *io.LimitedReader // io.LimitReader(sr) buf *bufio.ReadWriter // buffered(lr,rwc), reading from bufio->limitReader->sr->rwc @@ -432,13 +434,14 @@ func (srv *Server) newConn(rwc net.Conn) (c *conn, err error) { c.remoteAddr = rwc.RemoteAddr().String() c.server = srv c.rwc = rwc + c.w = rwc if debugServerConnections { c.rwc = newLoggingConn("server", c.rwc) } c.sr = liveSwitchReader{r: c.rwc} c.lr = io.LimitReader(&c.sr, noLimit).(*io.LimitedReader) br := newBufioReader(c.lr) - bw := newBufioWriterSize(c.rwc, 4<<10) + bw := newBufioWriterSize(checkConnErrorWriter{c}, 4<<10) c.buf = bufio.NewReadWriter(br, bw) return c, nil } @@ -956,8 +959,10 @@ func (w *response) bodyAllowed() bool { // 2. (*response).w, a *bufio.Writer of bufferBeforeChunkingSize bytes // 3. chunkWriter.Writer (whose writeHeader finalizes Content-Length/Type) // and which writes the chunk headers, if needed. -// 4. conn.buf, a bufio.Writer of default (4kB) bytes -// 5. the rwc, the net.Conn. +// 4. conn.buf, a bufio.Writer of default (4kB) bytes, writing to -> +// 5. checkConnErrorWriter{c}, which notes any non-nil error on Write +// and populates c.werr with it if so. but otherwise writes to: +// 6. the rwc, the net.Conn. // // TODO(bradfitz): short-circuit some of the buffering when the // initial header contains both a Content-Type and Content-Length. @@ -1027,6 +1032,12 @@ func (w *response) finishRequest() { // Did not write enough. Avoid getting out of sync. w.closeAfterReply = true } + + // There was some error writing to the underlying connection + // during the request, so don't re-use this conn. + if w.conn.werr != nil { + w.closeAfterReply = true + } } func (w *response) Flush() { @@ -2068,3 +2079,18 @@ func (c *loggingConn) Close() (err error) { log.Printf("%s.Close() = %v", c.name, err) return } + +// checkConnErrorWriter writes to c.rwc and records any write errors to c.werr. +// It only contains one field (and a pointer field at that), so it +// fits in an interface value without an extra allocation. +type checkConnErrorWriter struct { + c *conn +} + +func (w checkConnErrorWriter) Write(p []byte) (n int, err error) { + n, err = w.c.w.Write(p) // c.w == c.rwc, except after a hijack, when rwc is nil. + if err != nil && w.c.werr == nil { + w.c.werr = err + } + return +} -- cgit v1.2.1 From efd6ae7278334d7f37762b136a1d160f2da24ed5 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 15 Oct 2014 17:51:30 +0200 Subject: net/http: don't send implicit gzip Accept-Encoding on Range requests The http package by default adds "Accept-Encoding: gzip" to outgoing requests, unless it's a bad idea, or the user requested otherwise. Only when the http package adds its own implicit Accept-Encoding header does the http package also transparently un-gzip the response. If the user requested part of a document (e.g. bytes 40 to 50), it appears that Github/Varnish send: range(gzip(content), 40, 50) And not: gzip(range(content, 40, 50)) The RFC 2616 set of replacements (with the purpose of clarifying ambiguities since 1999) has an RFC about Range requests (http://tools.ietf.org/html/rfc7233) but does not mention the interaction with encodings. Regardless of whether range(gzip(content)) or gzip(range(content)) is correct, this change prevents the Go package from asking for gzip in requests if we're also asking for Range, avoiding the issue. If the user cared, they can do it themselves. But Go transparently un-gzipping a fragment of gzip is never useful. Fixes Issue 8923 LGTM=adg R=adg CC=golang-codereviews https://codereview.appspot.com/155420044 --- src/net/http/response_test.go | 28 ++++++++++++++++++++++++++++ src/net/http/transport.go | 11 +++++++++-- src/net/http/transport_test.go | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/net/http/response_test.go b/src/net/http/response_test.go index 2dd0fad11..06e940d9a 100644 --- a/src/net/http/response_test.go +++ b/src/net/http/response_test.go @@ -377,6 +377,34 @@ some body`, "Body here\n", }, + + // 206 Partial Content. golang.org/issue/8923 + { + "HTTP/1.1 206 Partial Content\r\n" + + "Content-Type: text/plain; charset=utf-8\r\n" + + "Accept-Ranges: bytes\r\n" + + "Content-Range: bytes 0-5/1862\r\n" + + "Content-Length: 6\r\n\r\n" + + "foobar", + + Response{ + Status: "206 Partial Content", + StatusCode: 206, + Proto: "HTTP/1.1", + ProtoMajor: 1, + ProtoMinor: 1, + Request: dummyReq("GET"), + Header: Header{ + "Accept-Ranges": []string{"bytes"}, + "Content-Length": []string{"6"}, + "Content-Type": []string{"text/plain; charset=utf-8"}, + "Content-Range": []string{"bytes 0-5/1862"}, + }, + ContentLength: 6, + }, + + "foobar", + }, } func TestReadResponse(t *testing.T) { diff --git a/src/net/http/transport.go b/src/net/http/transport.go index 70e574fc8..782f7cd39 100644 --- a/src/net/http/transport.go +++ b/src/net/http/transport.go @@ -1040,11 +1040,14 @@ func (pc *persistConn) roundTrip(req *transportRequest) (resp *Response, err err } // Ask for a compressed version if the caller didn't set their - // own value for Accept-Encoding. We only attempted to + // own value for Accept-Encoding. We only attempt to // uncompress the gzip stream if we were the layer that // requested it. requestedGzip := false - if !pc.t.DisableCompression && req.Header.Get("Accept-Encoding") == "" && req.Method != "HEAD" { + if !pc.t.DisableCompression && + req.Header.Get("Accept-Encoding") == "" && + req.Header.Get("Range") == "" && + req.Method != "HEAD" { // Request gzip only, not deflate. Deflate is ambiguous and // not as universally supported anyway. // See: http://www.gzip.org/zlib/zlib_faq.html#faq38 @@ -1053,6 +1056,10 @@ func (pc *persistConn) roundTrip(req *transportRequest) (resp *Response, err err // due to a bug in nginx: // http://trac.nginx.org/nginx/ticket/358 // http://golang.org/issue/5522 + // + // We don't request gzip if the request is for a range, since + // auto-decoding a portion of a gzipped document will just fail + // anyway. See http://golang.org/issue/8923 requestedGzip = true req.extraHeaders().Set("Accept-Encoding", "gzip") } diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go index 66fcc3c7d..defa63370 100644 --- a/src/net/http/transport_test.go +++ b/src/net/http/transport_test.go @@ -2216,6 +2216,39 @@ func TestTransportCloseIdleConnsThenReturn(t *testing.T) { wantIdle("after final put", 1) } +// This tests that an client requesting a content range won't also +// implicitly ask for gzip support. If they want that, they need to do it +// on their own. +// golang.org/issue/8923 +func TestTransportRangeAndGzip(t *testing.T) { + defer afterTest(t) + reqc := make(chan *Request, 1) + ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { + reqc <- r + })) + defer ts.Close() + + req, _ := NewRequest("GET", ts.URL, nil) + req.Header.Set("Range", "bytes=7-11") + res, err := DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + + select { + case r := <-reqc: + if strings.Contains(r.Header.Get("Accept-Encoding"), "gzip") { + t.Error("Transport advertised gzip support in the Accept header") + } + if r.Header.Get("Range") == "" { + t.Error("no Range in request") + } + case <-time.After(10 * time.Second): + t.Fatal("timeout") + } + res.Body.Close() +} + func wantBody(res *http.Response, err error, want string) error { if err != nil { return err -- cgit v1.2.1 From a5906fdac49996df7009c1ab4c60683df9e1469f Mon Sep 17 00:00:00 2001 From: Chris Manghane Date: Wed, 15 Oct 2014 09:55:13 -0700 Subject: cmd/gc: blank methods are not permitted in interface types Fixes issue 6606. LGTM=rsc R=rsc CC=golang-codereviews, gri https://codereview.appspot.com/156210044 --- src/cmd/gc/dcl.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'src') diff --git a/src/cmd/gc/dcl.c b/src/cmd/gc/dcl.c index cc010d901..dfcf47520 100644 --- a/src/cmd/gc/dcl.c +++ b/src/cmd/gc/dcl.c @@ -558,6 +558,9 @@ ifacedcl(Node *n) if(n->op != ODCLFIELD || n->right == N) fatal("ifacedcl"); + if(isblank(n->left)) + yyerror("methods must have a unique non-blank name"); + dclcontext = PPARAM; markdcl(); funcdepth++; -- cgit v1.2.1 From dad1b5c702a5408d47fdc7d622913be348a22731 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Wed, 15 Oct 2014 13:09:59 -0400 Subject: os/exec: document that Stdin goroutine must finish in Wait Fixes issue 7990. LGTM=iant, bradfitz R=bradfitz, iant, robryk CC=golang-codereviews https://codereview.appspot.com/156220043 --- src/os/exec/exec.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/os/exec/exec.go b/src/os/exec/exec.go index 4aded4171..72b4905d5 100644 --- a/src/os/exec/exec.go +++ b/src/os/exec/exec.go @@ -55,8 +55,15 @@ type Cmd struct { // calling process's current directory. Dir string - // Stdin specifies the process's standard input. If Stdin is - // nil, the process reads from the null device (os.DevNull). + // Stdin specifies the process's standard input. + // If Stdin is nil, the process reads from the null device (os.DevNull). + // If Stdin is an *os.File, the process's standard input is connected + // directly to that file. + // Otherwise, during the execution of the command a separate + // goroutine reads from Stdin and delivers that data to the command + // over a pipe. In this case, Wait does not complete until the goroutine + // stops copying, either because it has reached the end of Stdin + // (EOF or a read error) or because writing to the pipe returned an error. Stdin io.Reader // Stdout and Stderr specify the process's standard output and error. -- cgit v1.2.1 From ae4161ab2a8b51aca660c5ab6b23dd277aa77044 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Wed, 15 Oct 2014 13:10:14 -0400 Subject: database/sql: add Drivers, returning list of registered drivers Fixes issue 7969. LGTM=bradfitz R=bradfitz CC=golang-codereviews https://codereview.appspot.com/158950043 --- src/database/sql/fakedb_test.go | 22 ++++++++++++++++++++++ src/database/sql/sql.go | 11 +++++++++++ 2 files changed, 33 insertions(+) (limited to 'src') diff --git a/src/database/sql/fakedb_test.go b/src/database/sql/fakedb_test.go index c7db0dd77..171c322d4 100644 --- a/src/database/sql/fakedb_test.go +++ b/src/database/sql/fakedb_test.go @@ -10,6 +10,7 @@ import ( "fmt" "io" "log" + "sort" "strconv" "strings" "sync" @@ -126,6 +127,27 @@ func init() { Register("test", fdriver) } +func contains(list []string, y string) bool { + for _, x := range list { + if x == y { + return true + } + } + return false +} + +type Dummy struct { + driver.Driver +} + +func TestDrivers(t *testing.T) { + Register("invalid", Dummy{}) + all := Drivers() + if len(all) < 2 || !sort.StringsAreSorted(all) || !contains(all, "test") || !contains(all, "invalid") { + t.Fatalf("Drivers = %v, want sorted list with at least [invalid, test]", all) + } +} + // Supports dsn forms: // // ; (only currently supported option is `badConn`, diff --git a/src/database/sql/sql.go b/src/database/sql/sql.go index 731b7a7f7..ad9179cf7 100644 --- a/src/database/sql/sql.go +++ b/src/database/sql/sql.go @@ -18,6 +18,7 @@ import ( "fmt" "io" "runtime" + "sort" "sync" ) @@ -36,6 +37,16 @@ func Register(name string, driver driver.Driver) { drivers[name] = driver } +// Drivers returns a sorted list of the names of the registered drivers. +func Drivers() []string { + var list []string + for name := range drivers { + list = append(list, name) + } + sort.Strings(list) + return list +} + // RawBytes is a byte slice that holds a reference to memory owned by // the database itself. After a Scan into a RawBytes, the slice is only // valid until the next call to Next, Scan, or Close. -- cgit v1.2.1 From 40c2dd45e4056e903046d02982a961d0e3fbf0b9 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Wed, 15 Oct 2014 13:12:16 -0400 Subject: runtime: remove hand-generated ptr bitmaps for reflectcall A Go prototype can be used instead now, and the compiler will do a better job than we will doing it by hand. (We got it wrong in amd64p32, causing the current build breakage.) The auto-prototype-matching only applies to functions without an explicit package path, so the TEXT lines for reflectcall and callXX are s/runtime?/?/. LGTM=khr R=khr CC=golang-codereviews, iant, r https://codereview.appspot.com/153600043 --- src/runtime/asm_386.s | 72 ++++++++++++++++++------------------------- src/runtime/asm_amd64.s | 72 ++++++++++++++++++------------------------- src/runtime/asm_amd64p32.s | 77 +++++++++++++++++++--------------------------- src/runtime/asm_arm.s | 70 +++++++++++++++++------------------------ src/runtime/stubs.go | 30 ++++++++++++++++++ 5 files changed, 151 insertions(+), 170 deletions(-) (limited to 'src') diff --git a/src/runtime/asm_386.s b/src/runtime/asm_386.s index c401741ef..b0ed2d8ce 100644 --- a/src/runtime/asm_386.s +++ b/src/runtime/asm_386.s @@ -353,7 +353,7 @@ TEXT runtime·morestack_noctxt(SB),NOSPLIT,$0-0 JMP AX // Note: can't just "JMP NAME(SB)" - bad inlining results. -TEXT runtime·reflectcall(SB), NOSPLIT, $0-16 +TEXT ·reflectcall(SB), NOSPLIT, $0-16 MOVL argsize+8(FP), CX DISPATCH(runtime·call16, 16) DISPATCH(runtime·call32, 32) @@ -385,21 +385,9 @@ TEXT runtime·reflectcall(SB), NOSPLIT, $0-16 MOVL $runtime·badreflectcall(SB), AX JMP AX -// Argument map for the callXX frames. Each has one stack map. -DATA gcargs_reflectcall<>+0x00(SB)/4, $1 // 1 stackmap -DATA gcargs_reflectcall<>+0x04(SB)/4, $8 // 4 words -DATA gcargs_reflectcall<>+0x08(SB)/1, $(const_BitsPointer+(const_BitsPointer<<2)+(const_BitsScalar<<4)+(const_BitsScalar<<6)) -GLOBL gcargs_reflectcall<>(SB),RODATA,$12 - -// callXX frames have no locals -DATA gclocals_reflectcall<>+0x00(SB)/4, $1 // 1 stackmap -DATA gclocals_reflectcall<>+0x04(SB)/4, $0 // 0 locals -GLOBL gclocals_reflectcall<>(SB),RODATA,$8 - #define CALLFN(NAME,MAXSIZE) \ -TEXT NAME(SB), WRAPPER, $MAXSIZE-16; \ - FUNCDATA $FUNCDATA_ArgsPointerMaps,gcargs_reflectcall<>(SB); \ - FUNCDATA $FUNCDATA_LocalsPointerMaps,gclocals_reflectcall<>(SB);\ +TEXT NAME(SB), WRAPPER, $MAXSIZE-16; \ + NO_LOCAL_POINTERS; \ /* copy arguments to stack */ \ MOVL argptr+4(FP), SI; \ MOVL argsize+8(FP), CX; \ @@ -421,33 +409,33 @@ TEXT NAME(SB), WRAPPER, $MAXSIZE-16; \ REP;MOVSB; \ RET -CALLFN(runtime·call16, 16) -CALLFN(runtime·call32, 32) -CALLFN(runtime·call64, 64) -CALLFN(runtime·call128, 128) -CALLFN(runtime·call256, 256) -CALLFN(runtime·call512, 512) -CALLFN(runtime·call1024, 1024) -CALLFN(runtime·call2048, 2048) -CALLFN(runtime·call4096, 4096) -CALLFN(runtime·call8192, 8192) -CALLFN(runtime·call16384, 16384) -CALLFN(runtime·call32768, 32768) -CALLFN(runtime·call65536, 65536) -CALLFN(runtime·call131072, 131072) -CALLFN(runtime·call262144, 262144) -CALLFN(runtime·call524288, 524288) -CALLFN(runtime·call1048576, 1048576) -CALLFN(runtime·call2097152, 2097152) -CALLFN(runtime·call4194304, 4194304) -CALLFN(runtime·call8388608, 8388608) -CALLFN(runtime·call16777216, 16777216) -CALLFN(runtime·call33554432, 33554432) -CALLFN(runtime·call67108864, 67108864) -CALLFN(runtime·call134217728, 134217728) -CALLFN(runtime·call268435456, 268435456) -CALLFN(runtime·call536870912, 536870912) -CALLFN(runtime·call1073741824, 1073741824) +CALLFN(·call16, 16) +CALLFN(·call32, 32) +CALLFN(·call64, 64) +CALLFN(·call128, 128) +CALLFN(·call256, 256) +CALLFN(·call512, 512) +CALLFN(·call1024, 1024) +CALLFN(·call2048, 2048) +CALLFN(·call4096, 4096) +CALLFN(·call8192, 8192) +CALLFN(·call16384, 16384) +CALLFN(·call32768, 32768) +CALLFN(·call65536, 65536) +CALLFN(·call131072, 131072) +CALLFN(·call262144, 262144) +CALLFN(·call524288, 524288) +CALLFN(·call1048576, 1048576) +CALLFN(·call2097152, 2097152) +CALLFN(·call4194304, 4194304) +CALLFN(·call8388608, 8388608) +CALLFN(·call16777216, 16777216) +CALLFN(·call33554432, 33554432) +CALLFN(·call67108864, 67108864) +CALLFN(·call134217728, 134217728) +CALLFN(·call268435456, 268435456) +CALLFN(·call536870912, 536870912) +CALLFN(·call1073741824, 1073741824) // bool cas(int32 *val, int32 old, int32 new) // Atomically: diff --git a/src/runtime/asm_amd64.s b/src/runtime/asm_amd64.s index e21270d8c..2ee331208 100644 --- a/src/runtime/asm_amd64.s +++ b/src/runtime/asm_amd64.s @@ -339,11 +339,11 @@ TEXT runtime·morestack_noctxt(SB),NOSPLIT,$0 #define DISPATCH(NAME,MAXSIZE) \ CMPQ CX, $MAXSIZE; \ JA 3(PC); \ - MOVQ $NAME(SB), AX; \ + MOVQ $NAME(SB), AX; \ JMP AX // Note: can't just "JMP NAME(SB)" - bad inlining results. -TEXT runtime·reflectcall(SB), NOSPLIT, $0-24 +TEXT ·reflectcall(SB), NOSPLIT, $0-24 MOVLQZX argsize+16(FP), CX DISPATCH(runtime·call16, 16) DISPATCH(runtime·call32, 32) @@ -375,21 +375,9 @@ TEXT runtime·reflectcall(SB), NOSPLIT, $0-24 MOVQ $runtime·badreflectcall(SB), AX JMP AX -// Argument map for the callXX frames. Each has one stack map. -DATA gcargs_reflectcall<>+0x00(SB)/4, $1 // 1 stackmap -DATA gcargs_reflectcall<>+0x04(SB)/4, $6 // 3 words -DATA gcargs_reflectcall<>+0x08(SB)/1, $(const_BitsPointer+(const_BitsPointer<<2)+(const_BitsScalar<<4)) -GLOBL gcargs_reflectcall<>(SB),RODATA,$12 - -// callXX frames have no locals -DATA gclocals_reflectcall<>+0x00(SB)/4, $1 // 1 stackmap -DATA gclocals_reflectcall<>+0x04(SB)/4, $0 // 0 locals -GLOBL gclocals_reflectcall<>(SB),RODATA,$8 - #define CALLFN(NAME,MAXSIZE) \ TEXT NAME(SB), WRAPPER, $MAXSIZE-24; \ - FUNCDATA $FUNCDATA_ArgsPointerMaps,gcargs_reflectcall<>(SB); \ - FUNCDATA $FUNCDATA_LocalsPointerMaps,gclocals_reflectcall<>(SB);\ + NO_LOCAL_POINTERS; \ /* copy arguments to stack */ \ MOVQ argptr+8(FP), SI; \ MOVLQZX argsize+16(FP), CX; \ @@ -410,33 +398,33 @@ TEXT NAME(SB), WRAPPER, $MAXSIZE-24; \ REP;MOVSB; \ RET -CALLFN(runtime·call16, 16) -CALLFN(runtime·call32, 32) -CALLFN(runtime·call64, 64) -CALLFN(runtime·call128, 128) -CALLFN(runtime·call256, 256) -CALLFN(runtime·call512, 512) -CALLFN(runtime·call1024, 1024) -CALLFN(runtime·call2048, 2048) -CALLFN(runtime·call4096, 4096) -CALLFN(runtime·call8192, 8192) -CALLFN(runtime·call16384, 16384) -CALLFN(runtime·call32768, 32768) -CALLFN(runtime·call65536, 65536) -CALLFN(runtime·call131072, 131072) -CALLFN(runtime·call262144, 262144) -CALLFN(runtime·call524288, 524288) -CALLFN(runtime·call1048576, 1048576) -CALLFN(runtime·call2097152, 2097152) -CALLFN(runtime·call4194304, 4194304) -CALLFN(runtime·call8388608, 8388608) -CALLFN(runtime·call16777216, 16777216) -CALLFN(runtime·call33554432, 33554432) -CALLFN(runtime·call67108864, 67108864) -CALLFN(runtime·call134217728, 134217728) -CALLFN(runtime·call268435456, 268435456) -CALLFN(runtime·call536870912, 536870912) -CALLFN(runtime·call1073741824, 1073741824) +CALLFN(·call16, 16) +CALLFN(·call32, 32) +CALLFN(·call64, 64) +CALLFN(·call128, 128) +CALLFN(·call256, 256) +CALLFN(·call512, 512) +CALLFN(·call1024, 1024) +CALLFN(·call2048, 2048) +CALLFN(·call4096, 4096) +CALLFN(·call8192, 8192) +CALLFN(·call16384, 16384) +CALLFN(·call32768, 32768) +CALLFN(·call65536, 65536) +CALLFN(·call131072, 131072) +CALLFN(·call262144, 262144) +CALLFN(·call524288, 524288) +CALLFN(·call1048576, 1048576) +CALLFN(·call2097152, 2097152) +CALLFN(·call4194304, 4194304) +CALLFN(·call8388608, 8388608) +CALLFN(·call16777216, 16777216) +CALLFN(·call33554432, 33554432) +CALLFN(·call67108864, 67108864) +CALLFN(·call134217728, 134217728) +CALLFN(·call268435456, 268435456) +CALLFN(·call536870912, 536870912) +CALLFN(·call1073741824, 1073741824) // bool cas(int32 *val, int32 old, int32 new) // Atomically: diff --git a/src/runtime/asm_amd64p32.s b/src/runtime/asm_amd64p32.s index c2bc91a3f..e27f67e1e 100644 --- a/src/runtime/asm_amd64p32.s +++ b/src/runtime/asm_amd64p32.s @@ -310,11 +310,11 @@ TEXT runtime·morestack_noctxt(SB),NOSPLIT,$0 #define DISPATCH(NAME,MAXSIZE) \ CMPL CX, $MAXSIZE; \ JA 3(PC); \ - MOVL $NAME(SB), AX; \ + MOVL $NAME(SB), AX; \ JMP AX // Note: can't just "JMP NAME(SB)" - bad inlining results. -TEXT runtime·reflectcall(SB), NOSPLIT, $0-16 +TEXT ·reflectcall(SB), NOSPLIT, $0-16 MOVLQZX argsize+8(FP), CX DISPATCH(runtime·call16, 16) DISPATCH(runtime·call32, 32) @@ -346,22 +346,9 @@ TEXT runtime·reflectcall(SB), NOSPLIT, $0-16 MOVL $runtime·badreflectcall(SB), AX JMP AX -// Argument map for the callXX frames. Each has one stack map. -DATA gcargs_reflectcall<>+0x00(SB)/4, $1 // 1 stackmap -DATA gcargs_reflectcall<>+0x04(SB)/4, $10 // 5 words -DATA gcargs_reflectcall<>+0x08(SB)/1, $(const_BitsPointer+(const_BitsPointer<<2)+(const_BitsScalar<<4)+(const_BitsScalar<<6)) -DATA gcargs_reflectcall<>+0x09(SB)/1, $(const_BitsPointer) -GLOBL gcargs_reflectcall<>(SB),RODATA,$12 - -// callXX frames have no locals -DATA gclocals_reflectcall<>+0x00(SB)/4, $1 // 1 stackmap -DATA gclocals_reflectcall<>+0x04(SB)/4, $0 // 0 locals -GLOBL gclocals_reflectcall<>(SB),RODATA,$8 - #define CALLFN(NAME,MAXSIZE) \ TEXT NAME(SB), WRAPPER, $MAXSIZE-16; \ - FUNCDATA $FUNCDATA_ArgsPointerMaps,gcargs_reflectcall<>(SB); \ - FUNCDATA $FUNCDATA_LocalsPointerMaps,gclocals_reflectcall<>(SB);\ + NO_LOCAL_POINTERS; \ /* copy arguments to stack */ \ MOVL argptr+4(FP), SI; \ MOVL argsize+8(FP), CX; \ @@ -369,8 +356,8 @@ TEXT NAME(SB), WRAPPER, $MAXSIZE-16; \ REP;MOVSB; \ /* call function */ \ MOVL f+0(FP), DX; \ - MOVL (DX), AX; \ - CALL AX; \ + MOVL (DX), AX; \ + CALL AX; \ /* copy return values back */ \ MOVL argptr+4(FP), DI; \ MOVL argsize+8(FP), CX; \ @@ -382,33 +369,33 @@ TEXT NAME(SB), WRAPPER, $MAXSIZE-16; \ REP;MOVSB; \ RET -CALLFN(runtime·call16, 16) -CALLFN(runtime·call32, 32) -CALLFN(runtime·call64, 64) -CALLFN(runtime·call128, 128) -CALLFN(runtime·call256, 256) -CALLFN(runtime·call512, 512) -CALLFN(runtime·call1024, 1024) -CALLFN(runtime·call2048, 2048) -CALLFN(runtime·call4096, 4096) -CALLFN(runtime·call8192, 8192) -CALLFN(runtime·call16384, 16384) -CALLFN(runtime·call32768, 32768) -CALLFN(runtime·call65536, 65536) -CALLFN(runtime·call131072, 131072) -CALLFN(runtime·call262144, 262144) -CALLFN(runtime·call524288, 524288) -CALLFN(runtime·call1048576, 1048576) -CALLFN(runtime·call2097152, 2097152) -CALLFN(runtime·call4194304, 4194304) -CALLFN(runtime·call8388608, 8388608) -CALLFN(runtime·call16777216, 16777216) -CALLFN(runtime·call33554432, 33554432) -CALLFN(runtime·call67108864, 67108864) -CALLFN(runtime·call134217728, 134217728) -CALLFN(runtime·call268435456, 268435456) -CALLFN(runtime·call536870912, 536870912) -CALLFN(runtime·call1073741824, 1073741824) +CALLFN(·call16, 16) +CALLFN(·call32, 32) +CALLFN(·call64, 64) +CALLFN(·call128, 128) +CALLFN(·call256, 256) +CALLFN(·call512, 512) +CALLFN(·call1024, 1024) +CALLFN(·call2048, 2048) +CALLFN(·call4096, 4096) +CALLFN(·call8192, 8192) +CALLFN(·call16384, 16384) +CALLFN(·call32768, 32768) +CALLFN(·call65536, 65536) +CALLFN(·call131072, 131072) +CALLFN(·call262144, 262144) +CALLFN(·call524288, 524288) +CALLFN(·call1048576, 1048576) +CALLFN(·call2097152, 2097152) +CALLFN(·call4194304, 4194304) +CALLFN(·call8388608, 8388608) +CALLFN(·call16777216, 16777216) +CALLFN(·call33554432, 33554432) +CALLFN(·call67108864, 67108864) +CALLFN(·call134217728, 134217728) +CALLFN(·call268435456, 268435456) +CALLFN(·call536870912, 536870912) +CALLFN(·call1073741824, 1073741824) // bool cas(int32 *val, int32 old, int32 new) // Atomically: diff --git a/src/runtime/asm_arm.s b/src/runtime/asm_arm.s index a1535aeec..b21441488 100644 --- a/src/runtime/asm_arm.s +++ b/src/runtime/asm_arm.s @@ -343,7 +343,7 @@ TEXT runtime·morestack_noctxt(SB),NOSPLIT,$-4-0 MOVW $NAME(SB), R1; \ B (R1) -TEXT runtime·reflectcall(SB),NOSPLIT,$-4-16 +TEXT ·reflectcall(SB),NOSPLIT,$-4-16 MOVW argsize+8(FP), R0 DISPATCH(runtime·call16, 16) DISPATCH(runtime·call32, 32) @@ -375,21 +375,9 @@ TEXT runtime·reflectcall(SB),NOSPLIT,$-4-16 MOVW $runtime·badreflectcall(SB), R1 B (R1) -// Argument map for the callXX frames. Each has one stack map. -DATA gcargs_reflectcall<>+0x00(SB)/4, $1 // 1 stackmap -DATA gcargs_reflectcall<>+0x04(SB)/4, $8 // 4 words -DATA gcargs_reflectcall<>+0x08(SB)/1, $(const_BitsPointer+(const_BitsPointer<<2)+(const_BitsScalar<<4)+(const_BitsScalar<<6)) -GLOBL gcargs_reflectcall<>(SB),RODATA,$12 - -// callXX frames have no locals -DATA gclocals_reflectcall<>+0x00(SB)/4, $1 // 1 stackmap -DATA gclocals_reflectcall<>+0x04(SB)/4, $0 // 0 locals -GLOBL gclocals_reflectcall<>(SB),RODATA,$8 - #define CALLFN(NAME,MAXSIZE) \ TEXT NAME(SB), WRAPPER, $MAXSIZE-16; \ - FUNCDATA $FUNCDATA_ArgsPointerMaps,gcargs_reflectcall<>(SB); \ - FUNCDATA $FUNCDATA_LocalsPointerMaps,gclocals_reflectcall<>(SB);\ + NO_LOCAL_POINTERS; \ /* copy arguments to stack */ \ MOVW argptr+4(FP), R0; \ MOVW argsize+8(FP), R2; \ @@ -420,33 +408,33 @@ TEXT NAME(SB), WRAPPER, $MAXSIZE-16; \ SUB $1, R2, R2; \ B -5(PC) \ -CALLFN(runtime·call16, 16) -CALLFN(runtime·call32, 32) -CALLFN(runtime·call64, 64) -CALLFN(runtime·call128, 128) -CALLFN(runtime·call256, 256) -CALLFN(runtime·call512, 512) -CALLFN(runtime·call1024, 1024) -CALLFN(runtime·call2048, 2048) -CALLFN(runtime·call4096, 4096) -CALLFN(runtime·call8192, 8192) -CALLFN(runtime·call16384, 16384) -CALLFN(runtime·call32768, 32768) -CALLFN(runtime·call65536, 65536) -CALLFN(runtime·call131072, 131072) -CALLFN(runtime·call262144, 262144) -CALLFN(runtime·call524288, 524288) -CALLFN(runtime·call1048576, 1048576) -CALLFN(runtime·call2097152, 2097152) -CALLFN(runtime·call4194304, 4194304) -CALLFN(runtime·call8388608, 8388608) -CALLFN(runtime·call16777216, 16777216) -CALLFN(runtime·call33554432, 33554432) -CALLFN(runtime·call67108864, 67108864) -CALLFN(runtime·call134217728, 134217728) -CALLFN(runtime·call268435456, 268435456) -CALLFN(runtime·call536870912, 536870912) -CALLFN(runtime·call1073741824, 1073741824) +CALLFN(·call16, 16) +CALLFN(·call32, 32) +CALLFN(·call64, 64) +CALLFN(·call128, 128) +CALLFN(·call256, 256) +CALLFN(·call512, 512) +CALLFN(·call1024, 1024) +CALLFN(·call2048, 2048) +CALLFN(·call4096, 4096) +CALLFN(·call8192, 8192) +CALLFN(·call16384, 16384) +CALLFN(·call32768, 32768) +CALLFN(·call65536, 65536) +CALLFN(·call131072, 131072) +CALLFN(·call262144, 262144) +CALLFN(·call524288, 524288) +CALLFN(·call1048576, 1048576) +CALLFN(·call2097152, 2097152) +CALLFN(·call4194304, 4194304) +CALLFN(·call8388608, 8388608) +CALLFN(·call16777216, 16777216) +CALLFN(·call33554432, 33554432) +CALLFN(·call67108864, 67108864) +CALLFN(·call134217728, 134217728) +CALLFN(·call268435456, 268435456) +CALLFN(·call536870912, 536870912) +CALLFN(·call1073741824, 1073741824) // void jmpdefer(fn, sp); // called from deferreturn. diff --git a/src/runtime/stubs.go b/src/runtime/stubs.go index c6a9cf9f5..6561094ff 100644 --- a/src/runtime/stubs.go +++ b/src/runtime/stubs.go @@ -252,3 +252,33 @@ func return0() // thunk to call time.now. func timenow() (sec int64, nsec int32) + +// in asm_*.s +// not called directly; definitions here supply type information for traceback. +func call16(fn, arg unsafe.Pointer, n, retoffset uint32) +func call32(fn, arg unsafe.Pointer, n, retoffset uint32) +func call64(fn, arg unsafe.Pointer, n, retoffset uint32) +func call128(fn, arg unsafe.Pointer, n, retoffset uint32) +func call256(fn, arg unsafe.Pointer, n, retoffset uint32) +func call512(fn, arg unsafe.Pointer, n, retoffset uint32) +func call1024(fn, arg unsafe.Pointer, n, retoffset uint32) +func call2048(fn, arg unsafe.Pointer, n, retoffset uint32) +func call4096(fn, arg unsafe.Pointer, n, retoffset uint32) +func call8192(fn, arg unsafe.Pointer, n, retoffset uint32) +func call16384(fn, arg unsafe.Pointer, n, retoffset uint32) +func call32768(fn, arg unsafe.Pointer, n, retoffset uint32) +func call65536(fn, arg unsafe.Pointer, n, retoffset uint32) +func call131072(fn, arg unsafe.Pointer, n, retoffset uint32) +func call262144(fn, arg unsafe.Pointer, n, retoffset uint32) +func call524288(fn, arg unsafe.Pointer, n, retoffset uint32) +func call1048576(fn, arg unsafe.Pointer, n, retoffset uint32) +func call2097152(fn, arg unsafe.Pointer, n, retoffset uint32) +func call4194304(fn, arg unsafe.Pointer, n, retoffset uint32) +func call8388608(fn, arg unsafe.Pointer, n, retoffset uint32) +func call16777216(fn, arg unsafe.Pointer, n, retoffset uint32) +func call33554432(fn, arg unsafe.Pointer, n, retoffset uint32) +func call67108864(fn, arg unsafe.Pointer, n, retoffset uint32) +func call134217728(fn, arg unsafe.Pointer, n, retoffset uint32) +func call268435456(fn, arg unsafe.Pointer, n, retoffset uint32) +func call536870912(fn, arg unsafe.Pointer, n, retoffset uint32) +func call1073741824(fn, arg unsafe.Pointer, n, retoffset uint32) -- cgit v1.2.1 From 975a1633cf3a40984fd5a7c924eed78f91f36c52 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Wed, 15 Oct 2014 13:33:00 -0400 Subject: reflect: add fast path for FieldByIndex with len(index) = 1 LGTM=r R=r CC=golang-codereviews https://codereview.appspot.com/152640043 --- src/reflect/value.go | 3 +++ 1 file changed, 3 insertions(+) (limited to 'src') diff --git a/src/reflect/value.go b/src/reflect/value.go index 9c65ee270..8c320f11b 100644 --- a/src/reflect/value.go +++ b/src/reflect/value.go @@ -857,6 +857,9 @@ func (v Value) Field(i int) Value { // FieldByIndex returns the nested field corresponding to index. // It panics if v's Kind is not struct. func (v Value) FieldByIndex(index []int) Value { + if len(index) == 1 { + return v.Field(index[0]) + } v.mustBe(Struct) for i, x := range index { if i > 0 { -- cgit v1.2.1 From 4dace57a215037f6a66d37e0480d8e071f19ff91 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Wed, 15 Oct 2014 14:24:18 -0400 Subject: reflect: shorten value to 3 words scalar is no longer needed, now that interfaces always hold pointers. Comparing best of 5 with TurboBoost turned off, on a 2012 Retina MacBook Pro Core i5. Still not completely confident in these numbers, but the gob and template improvements seem real. benchmark old ns/op new ns/op delta BenchmarkBinaryTree17 3819892491 3803008185 -0.44% BenchmarkFannkuch11 3623876405 3611776426 -0.33% BenchmarkFmtFprintfEmpty 119 118 -0.84% BenchmarkFmtFprintfString 294 292 -0.68% BenchmarkFmtFprintfInt 310 304 -1.94% BenchmarkFmtFprintfIntInt 513 507 -1.17% BenchmarkFmtFprintfPrefixedInt 427 426 -0.23% BenchmarkFmtFprintfFloat 562 554 -1.42% BenchmarkFmtManyArgs 1873 1832 -2.19% BenchmarkGobDecode 15824504 14746565 -6.81% BenchmarkGobEncode 14347378 14208743 -0.97% BenchmarkGzip 537229271 537973492 +0.14% BenchmarkGunzip 134996775 135406149 +0.30% BenchmarkHTTPClientServer 119065 116937 -1.79% BenchmarkJSONEncode 29134359 28928099 -0.71% BenchmarkJSONDecode 106867289 105770161 -1.03% BenchmarkMandelbrot200 5798475 5791433 -0.12% BenchmarkGoParse 5299169 5379201 +1.51% BenchmarkRegexpMatchEasy0_32 195 195 +0.00% BenchmarkRegexpMatchEasy0_1K 477 477 +0.00% BenchmarkRegexpMatchEasy1_32 170 170 +0.00% BenchmarkRegexpMatchEasy1_1K 1412 1397 -1.06% BenchmarkRegexpMatchMedium_32 336 337 +0.30% BenchmarkRegexpMatchMedium_1K 109025 108977 -0.04% BenchmarkRegexpMatchHard_32 5854 5856 +0.03% BenchmarkRegexpMatchHard_1K 184914 184748 -0.09% BenchmarkRevcomp 829233526 836598734 +0.89% BenchmarkTemplate 142055312 137016166 -3.55% BenchmarkTimeParse 598 597 -0.17% BenchmarkTimeFormat 564 568 +0.71% Fixes issue 7425. LGTM=r R=golang-codereviews, r CC=golang-codereviews, iant, khr https://codereview.appspot.com/158890043 --- src/reflect/makefunc.go | 6 +- src/reflect/type.go | 10 +- src/reflect/value.go | 365 +++++++++++------------------------------------- 3 files changed, 91 insertions(+), 290 deletions(-) (limited to 'src') diff --git a/src/reflect/makefunc.go b/src/reflect/makefunc.go index bdb8c21d7..1072c7fab 100644 --- a/src/reflect/makefunc.go +++ b/src/reflect/makefunc.go @@ -60,7 +60,7 @@ func MakeFunc(typ Type, fn func(args []Value) (results []Value)) Value { impl := &makeFuncImpl{code: code, stack: stack, typ: ftyp, fn: fn} - return Value{t, unsafe.Pointer(impl), 0, flag(Func) << flagKindShift} + return Value{t, unsafe.Pointer(impl), flag(Func) << flagKindShift} } // makeFuncStub is an assembly function that is the code half of @@ -92,7 +92,7 @@ func makeMethodValue(op string, v Value) Value { // Ignoring the flagMethod bit, v describes the receiver, not the method type. fl := v.flag & (flagRO | flagAddr | flagIndir) fl |= flag(v.typ.Kind()) << flagKindShift - rcvr := Value{v.typ, v.ptr, v.scalar, fl} + rcvr := Value{v.typ, v.ptr, fl} // v.Type returns the actual type of the method value. funcType := v.Type().(*rtype) @@ -118,7 +118,7 @@ func makeMethodValue(op string, v Value) Value { // but we want Interface() and other operations to fail early. methodReceiver(op, fv.rcvr, fv.method) - return Value{funcType, unsafe.Pointer(fv), 0, v.flag&flagRO | flag(Func)<> (field.offset * 8) } - return Value{typ, ptr, scalar, fl} + return Value{typ, ptr, fl} } // FieldByIndex returns the nested field corresponding to index. @@ -904,15 +798,9 @@ func (v Value) Float() float64 { k := v.kind() switch k { case Float32: - if v.flag&flagIndir != 0 { - return float64(*(*float32)(v.ptr)) - } - return float64(*(*float32)(unsafe.Pointer(&v.scalar))) + return float64(*(*float32)(v.ptr)) case Float64: - if v.flag&flagIndir != 0 { - return *(*float64)(v.ptr) - } - return *(*float64)(unsafe.Pointer(&v.scalar)) + return *(*float64)(v.ptr) } panic(&ValueError{"reflect.Value.Float", k}) } @@ -935,12 +823,10 @@ func (v Value) Index(i int) Value { offset := uintptr(i) * typ.size var val unsafe.Pointer - var scalar uintptr - switch { - case fl&flagIndir != 0: + if fl&flagIndir != 0 { // Indirect. Just bump pointer. val = unsafe.Pointer(uintptr(v.ptr) + offset) - case typ.pointers(): + } else { if offset != 0 { // This is an array stored inline in an interface value. // And the array element type has pointers. @@ -951,14 +837,8 @@ func (v Value) Index(i int) Value { panic("reflect: internal error: unexpected array index") } val = v.ptr - case bigEndian: - // Direct. Discard leading bytes. - scalar = v.scalar << (offset * 8) - default: - // Direct. Discard leading bytes. - scalar = v.scalar >> (offset * 8) } - return Value{typ, val, scalar, fl} + return Value{typ, val, fl} case Slice: // Element flag same as Elem of Ptr. @@ -972,7 +852,7 @@ func (v Value) Index(i int) Value { typ := tt.elem fl |= flag(typ.Kind()) << flagKindShift val := unsafe.Pointer(uintptr(s.Data) + uintptr(i)*typ.size) - return Value{typ, val, 0, fl} + return Value{typ, val, fl} case String: fl := v.flag&flagRO | flag(Uint8< cap { @@ -1708,7 +1565,7 @@ func (v Value) Slice(i, j int) Value { } fl := v.flag&flagRO | flagIndir | flag(Slice)< interface @@ -2618,7 +2419,7 @@ func cvtT2I(v Value, typ Type) Value { } else { ifaceE2I(typ.(*rtype), x, unsafe.Pointer(target)) } - return Value{typ.common(), unsafe.Pointer(target), 0, v.flag&flagRO | flagIndir | flag(Interface)< interface -- cgit v1.2.1 From 4d314181e21d951a420d1f2740ff3e863435d1f5 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Wed, 15 Oct 2014 14:33:52 -0400 Subject: cmd/gc: do not copy via temporary for writebarrierfat{2,3,4} The general writebarrierfat needs a temporary for src, because we need to pass the address of the temporary to the writebarrierfat routine. But the new fixed-size ones pass the value directly and don't need to introduce the temporary. Magnifies some of the effect of the custom write barrier change. Comparing best of 5 with TurboBoost turned off, on a 2012 Retina MacBook Pro Core i5. Still not completely confident in these numbers, but the fmt, regexp, and revcomp improvements seem real. benchmark old ns/op new ns/op delta BenchmarkBinaryTree17 3942965521 3929654940 -0.34% BenchmarkFannkuch11 3707543350 3699566011 -0.22% BenchmarkFmtFprintfEmpty 119 119 +0.00% BenchmarkFmtFprintfString 295 296 +0.34% BenchmarkFmtFprintfInt 313 314 +0.32% BenchmarkFmtFprintfIntInt 517 484 -6.38% BenchmarkFmtFprintfPrefixedInt 439 429 -2.28% BenchmarkFmtFprintfFloat 571 569 -0.35% BenchmarkFmtManyArgs 1899 1820 -4.16% BenchmarkGobDecode 15507208 15325649 -1.17% BenchmarkGobEncode 14811710 14715434 -0.65% BenchmarkGzip 561144467 549624323 -2.05% BenchmarkGunzip 137377667 137691087 +0.23% BenchmarkHTTPClientServer 126632 124717 -1.51% BenchmarkJSONEncode 29944112 29526629 -1.39% BenchmarkJSONDecode 108954913 107339551 -1.48% BenchmarkMandelbrot200 5828755 5821659 -0.12% BenchmarkGoParse 5577437 5521895 -1.00% BenchmarkRegexpMatchEasy0_32 198 193 -2.53% BenchmarkRegexpMatchEasy0_1K 486 469 -3.50% BenchmarkRegexpMatchEasy1_32 175 167 -4.57% BenchmarkRegexpMatchEasy1_1K 1450 1419 -2.14% BenchmarkRegexpMatchMedium_32 344 338 -1.74% BenchmarkRegexpMatchMedium_1K 112088 109855 -1.99% BenchmarkRegexpMatchHard_32 6078 6003 -1.23% BenchmarkRegexpMatchHard_1K 191166 187499 -1.92% BenchmarkRevcomp 854870445 799012851 -6.53% BenchmarkTemplate 141572691 141508105 -0.05% BenchmarkTimeParse 604 603 -0.17% BenchmarkTimeFormat 579 560 -3.28% LGTM=r R=r CC=golang-codereviews https://codereview.appspot.com/155450043 --- src/cmd/gc/order.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/cmd/gc/order.c b/src/cmd/gc/order.c index 3027ed27d..76820fde7 100644 --- a/src/cmd/gc/order.c +++ b/src/cmd/gc/order.c @@ -438,6 +438,9 @@ ordercall(Node *n, Order *order) // cases they are also typically registerizable, so not much harm done. // And this only applies to the multiple-assignment form. // We could do a more precise analysis if needed, like in walk.c. +// +// Ordermapassign also inserts these temporaries if needed for +// calling writebarrierfat with a pointer to n->right. static void ordermapassign(Node *n, Order *order) { @@ -451,7 +454,8 @@ ordermapassign(Node *n, Order *order) case OAS: order->out = list(order->out, n); - if((n->left->op == OINDEXMAP || (needwritebarrier(n->left, n->right) && n->left->type->width > widthptr)) && !isaddrokay(n->right)) { + // We call writebarrierfat only for values > 4 pointers long. See walk.c. + if((n->left->op == OINDEXMAP || (needwritebarrier(n->left, n->right) && n->left->type->width > 4*widthptr)) && !isaddrokay(n->right)) { m = n->left; n->left = ordertemp(m->type, order, 0); a = nod(OAS, m, n->left); -- cgit v1.2.1 From 25c63c2c30f4807b073f849d248144d97893ce70 Mon Sep 17 00:00:00 2001 From: Chris Manghane Date: Wed, 15 Oct 2014 13:13:37 -0700 Subject: cmd/go: add '_go_' suffix to go files compiled by gccgo to avoid naming conflicts Fixes issue 8828. LGTM=rsc R=rsc CC=golang-codereviews https://codereview.appspot.com/154410043 --- src/cmd/go/build.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/cmd/go/build.go b/src/cmd/go/build.go index 9c7b42650..49b84709e 100644 --- a/src/cmd/go/build.go +++ b/src/cmd/go/build.go @@ -1852,7 +1852,7 @@ func (gccgoToolchain) linker() string { } func (gccgoToolchain) gc(b *builder, p *Package, archive, obj string, importArgs []string, gofiles []string) (ofile string, output []byte, err error) { - out := p.Name + ".o" + out := "_go_.o" ofile = obj + out gcargs := []string{"-g"} gcargs = append(gcargs, b.gccArchArgs()...) -- cgit v1.2.1