From 7ab4b5586d37513bfa48f769773007ff8e9b732d Mon Sep 17 00:00:00 2001 From: fanzha02 Date: Fri, 15 Jun 2018 10:20:00 +0000 Subject: cmd/internal/obj/arm64: add CONSTRAINED UNPREDICTABLE behavior check for some load/store According to ARM64 manual, it is "constrained unpredictable behavior" if the src and dst registers of some load/store instructions are same. In order to completely prevent such unpredictable behavior, adding the check for load/store instructions that are supported by the assembler in the assembler. Add test cases. Update #25823 Change-Id: I64c14ad99ee543d778e7ec8ae6516a532293dbb3 Reviewed-on: https://go-review.googlesource.com/120660 Run-TryBot: Cherry Zhang TryBot-Result: Gobot Gobot Reviewed-by: Cherry Zhang --- src/cmd/internal/obj/arm64/asm7.go | 68 +++++++++++++++++++++++++++++++++++++- 1 file changed, 67 insertions(+), 1 deletion(-) (limited to 'src/cmd/internal') diff --git a/src/cmd/internal/obj/arm64/asm7.go b/src/cmd/internal/obj/arm64/asm7.go index 7507976257..09ffc5dccf 100644 --- a/src/cmd/internal/obj/arm64/asm7.go +++ b/src/cmd/internal/obj/arm64/asm7.go @@ -1085,6 +1085,23 @@ func (c *ctxt7) regoff(a *obj.Addr) uint32 { return uint32(c.instoffset) } +func isSTLXRop(op obj.As) bool { + switch op { + case ASTLXR, ASTLXRW, ASTLXRB, ASTLXRH, + ASTXR, ASTXRW, ASTXRB, ASTXRH: + return true + } + return false +} + +func isSTXPop(op obj.As) bool { + switch op { + case ASTXP, ASTLXP, ASTXPW, ASTLXPW: + return true + } + return false +} + func isRegShiftOrExt(a *obj.Addr) bool { return (a.Index-obj.RBaseARM64)®_EXT != 0 || (a.Index-obj.RBaseARM64)®_LSL != 0 } @@ -2502,6 +2519,17 @@ func SYSARG4(op1 int, Cn int, Cm int, op2 int) int { return SYSARG5(0, op1, Cn, Cm, op2) } +// checkUnpredictable checks if the sourse and transfer registers are the same register. +// ARM64 manual says it is "constrained unpredictable" if the src and dst registers of STP/LDP are same. +func (c *ctxt7) checkUnpredictable(p *obj.Prog, isload bool, wback bool, rn int16, rt1 int16, rt2 int16) { + if wback && rn != REGSP && (rn == rt1 || rn == rt2) { + c.ctxt.Diag("constrained unpredictable behavior: %v", p) + } + if isload && rt1 == rt2 { + c.ctxt.Diag("constrained unpredictable behavior: %v", p) + } +} + /* checkindex checks if index >= 0 && index <= maxindex */ func (c *ctxt7) checkindex(p *obj.Prog, index, maxindex int) { if index < 0 || index > maxindex { @@ -2940,6 +2968,10 @@ func (c *ctxt7) asmout(p *obj.Prog, o *Optab, out []uint32) { } case 22: /* movT (R)O!,R; movT O(R)!, R -> ldrT */ + if p.As != AFMOVS && p.As != AFMOVD && p.From.Reg != REGSP && p.From.Reg == p.To.Reg { + c.ctxt.Diag("constrained unpredictable behavior: %v", p) + } + v := int32(p.From.Offset) if v < -256 || v > 255 { @@ -2954,6 +2986,10 @@ func (c *ctxt7) asmout(p *obj.Prog, o *Optab, out []uint32) { o1 |= ((uint32(v) & 0x1FF) << 12) | (uint32(p.From.Reg&31) << 5) | uint32(p.To.Reg&31) case 23: /* movT R,(R)O!; movT O(R)!, R -> strT */ + if p.As != AFMOVS && p.As != AFMOVD && p.To.Reg != REGSP && p.From.Reg == p.To.Reg { + c.ctxt.Diag("constrained unpredictable behavior: %v", p) + } + v := int32(p.To.Offset) if v < -256 || v > 255 { @@ -3551,6 +3587,9 @@ func (c *ctxt7) asmout(p *obj.Prog, o *Optab, out []uint32) { o1 |= 0x1F << 16 o1 |= uint32(p.From.Reg&31) << 5 if p.As == ALDXP || p.As == ALDXPW || p.As == ALDAXP || p.As == ALDAXPW { + if int(p.To.Reg) == int(p.To.Offset) { + c.ctxt.Diag("constrained unpredictable behavior: %v", p) + } o1 |= uint32(p.To.Offset&31) << 10 } else { o1 |= 0x1F << 10 @@ -3558,6 +3597,19 @@ func (c *ctxt7) asmout(p *obj.Prog, o *Optab, out []uint32) { o1 |= uint32(p.To.Reg & 31) case 59: /* stxr/stlxr/stxp/stlxp */ + s := p.RegTo2 + n := p.To.Reg + t := p.From.Reg + if isSTLXRop(p.As) { + if s == t || (s == n && n != REGSP) { + c.ctxt.Diag("constrained unpredictable behavior: %v", p) + } + } else if isSTXPop(p.As) { + t2 := int16(p.From.Offset) + if (s == t || s == t2) || (s == n && n != REGSP) { + c.ctxt.Diag("constrained unpredictable behavior: %v", p) + } + } o1 = c.opstore(p, p.As) if p.RegTo2 != obj.REG_NONE { @@ -3565,7 +3617,7 @@ func (c *ctxt7) asmout(p *obj.Prog, o *Optab, out []uint32) { } else { o1 |= 0x1F << 16 } - if p.As == ASTXP || p.As == ASTXPW || p.As == ASTLXP || p.As == ASTLXPW { + if isSTXPop(p.As) { o1 |= uint32(p.From.Offset&31) << 10 } o1 |= uint32(p.To.Reg&31)<<5 | uint32(p.From.Reg&31) @@ -6177,6 +6229,20 @@ func (c *ctxt7) opextr(p *obj.Prog, a obj.As, v int32, rn int, rm int, rt int) u /* genrate instruction encoding for LDP/LDPW/LDPSW/STP/STPW */ func (c *ctxt7) opldpstp(p *obj.Prog, o *Optab, vo int32, rbase, rl, rh, ldp uint32) uint32 { + wback := false + if o.scond == C_XPOST || o.scond == C_XPRE { + wback = true + } + switch p.As { + case ALDP, ALDPW, ALDPSW: + c.checkUnpredictable(p, true, wback, p.From.Reg, p.To.Reg, int16(p.To.Offset)) + case ASTP, ASTPW: + if wback == true { + c.checkUnpredictable(p, false, true, p.To.Reg, p.From.Reg, int16(p.From.Offset)) + } + case AFLDPD, AFLDPS: + c.checkUnpredictable(p, true, false, p.From.Reg, p.To.Reg, int16(p.To.Offset)) + } var ret uint32 // check offset switch p.As { -- cgit v1.3-5-g9baa From e7f5f3eca42a98340e4eb4fc5d490a9aa4bd5054 Mon Sep 17 00:00:00 2001 From: fanzha02 Date: Mon, 10 Sep 2018 02:22:48 +0000 Subject: cmd/internal/obj/arm64: add error report for invalid base register The current assembler accepts the non-integer register as the base register, which should be an illegal combination. Add the test cases. Change-Id: Ia21596bbb5b1e212e34bd3a170748ae788860422 Reviewed-on: https://go-review.googlesource.com/134575 Reviewed-by: Cherry Zhang Run-TryBot: Cherry Zhang TryBot-Result: Gobot Gobot --- src/cmd/asm/internal/asm/testdata/arm64error.s | 2 ++ src/cmd/internal/obj/arm64/asm7.go | 8 ++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) (limited to 'src/cmd/internal') diff --git a/src/cmd/asm/internal/asm/testdata/arm64error.s b/src/cmd/asm/internal/asm/testdata/arm64error.s index bbdce479c5..357db80222 100644 --- a/src/cmd/asm/internal/asm/testdata/arm64error.s +++ b/src/cmd/asm/internal/asm/testdata/arm64error.s @@ -110,4 +110,6 @@ TEXT errors(SB),$0 FLDPD (R1), (F2, F2) // ERROR "constrained unpredictable behavior" FLDPS (R2), (F3, F3) // ERROR "constrained unpredictable behavior" FSTPD (R1, R2), (R0) // ERROR "invalid register pair" + FMOVS (F2), F0 // ERROR "illegal combination" + FMOVD F0, (F1) // ERROR "illegal combination" RET diff --git a/src/cmd/internal/obj/arm64/asm7.go b/src/cmd/internal/obj/arm64/asm7.go index 09ffc5dccf..46fdcdcf7d 100644 --- a/src/cmd/internal/obj/arm64/asm7.go +++ b/src/cmd/internal/obj/arm64/asm7.go @@ -1426,6 +1426,10 @@ func (c *ctxt7) aclass(a *obj.Addr) int { return C_LIST case obj.TYPE_MEM: + // The base register should be an integer register. + if int16(REG_F0) <= a.Reg && a.Reg <= int16(REG_V31) { + break + } switch a.Name { case obj.NAME_EXTERN, obj.NAME_STATIC: if a.Sym == nil { @@ -2968,7 +2972,7 @@ func (c *ctxt7) asmout(p *obj.Prog, o *Optab, out []uint32) { } case 22: /* movT (R)O!,R; movT O(R)!, R -> ldrT */ - if p.As != AFMOVS && p.As != AFMOVD && p.From.Reg != REGSP && p.From.Reg == p.To.Reg { + if p.From.Reg != REGSP && p.From.Reg == p.To.Reg { c.ctxt.Diag("constrained unpredictable behavior: %v", p) } @@ -2986,7 +2990,7 @@ func (c *ctxt7) asmout(p *obj.Prog, o *Optab, out []uint32) { o1 |= ((uint32(v) & 0x1FF) << 12) | (uint32(p.From.Reg&31) << 5) | uint32(p.To.Reg&31) case 23: /* movT R,(R)O!; movT O(R)!, R -> strT */ - if p.As != AFMOVS && p.As != AFMOVD && p.To.Reg != REGSP && p.From.Reg == p.To.Reg { + if p.To.Reg != REGSP && p.From.Reg == p.To.Reg { c.ctxt.Diag("constrained unpredictable behavior: %v", p) } -- cgit v1.3-5-g9baa From da0d1a44bac379f5acedb1933f85400de08f4ac6 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 26 Sep 2018 21:10:21 +0000 Subject: all: use strings.ReplaceAll and bytes.ReplaceAll where applicable I omitted vendor directories and anything necessary for bootstrapping. (Tested by bootstrapping with Go 1.4) Updates #27864 Change-Id: I7d9b68d0372d3a34dee22966cca323513ece7e8a Reviewed-on: https://go-review.googlesource.com/137856 Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot Reviewed-by: Ian Lance Taylor --- src/cmd/fix/main.go | 2 +- src/cmd/fix/typecheck.go | 4 ++-- src/cmd/go/go_test.go | 10 +++++----- src/cmd/go/internal/envcmd/env.go | 2 +- src/cmd/go/internal/get/vcs.go | 2 +- src/cmd/go/internal/modconv/convert_test.go | 2 +- src/cmd/go/internal/modfetch/codehost/codehost.go | 2 +- src/cmd/go/internal/modfetch/coderepo_test.go | 12 ++++++------ src/cmd/go/internal/modfetch/proxy.go | 2 +- src/cmd/go/internal/modload/import_test.go | 2 +- src/cmd/go/internal/modload/query_test.go | 2 +- src/cmd/go/internal/search/search.go | 4 ++-- src/cmd/go/internal/work/build.go | 4 ++-- src/cmd/go/internal/work/exec.go | 10 +++++----- src/cmd/go/proxy_test.go | 4 ++-- src/cmd/go/script_test.go | 12 ++++++------ src/cmd/go/testdata/addmod.go | 2 +- src/cmd/go/vendor_test.go | 2 +- src/cmd/gofmt/gofmt_test.go | 2 +- src/cmd/internal/goobj/read.go | 2 +- src/cmd/trace/trace.go | 2 +- src/cmd/vet/main.go | 2 +- src/cmd/vet/vet_test.go | 2 +- src/crypto/rsa/pss_test.go | 2 +- src/crypto/x509/root_darwin_arm_gen.go | 6 +++--- src/encoding/base32/base32_test.go | 10 +++++----- src/encoding/base64/base64_test.go | 8 ++++---- src/flag/flag.go | 2 +- src/go/constant/value_test.go | 2 +- src/go/doc/doc_test.go | 6 +++--- src/go/printer/example_test.go | 4 ++-- src/html/template/js.go | 2 +- src/html/template/url.go | 2 +- src/mime/multipart/formdata_test.go | 8 ++++---- src/mime/multipart/multipart_test.go | 6 +++--- src/net/http/cgi/child.go | 2 +- src/net/http/httputil/dump_test.go | 2 +- src/net/http/readrequest_test.go | 2 +- src/net/http/request_test.go | 6 +++--- src/net/http/serve_test.go | 2 +- src/net/lookup_windows_test.go | 2 +- src/net/mail/message_test.go | 4 ++-- src/net/net_windows_test.go | 2 +- src/net/url/example_test.go | 2 +- src/net/url/url_test.go | 6 +++--- src/os/path_windows_test.go | 6 +++--- src/path/filepath/path.go | 4 ++-- src/path/filepath/path_test.go | 2 +- src/path/filepath/path_windows.go | 2 +- src/path/filepath/path_windows_test.go | 2 +- src/runtime/pprof/internal/profile/profile.go | 2 +- src/runtime/pprof/pprof_test.go | 2 +- src/runtime/runtime-gdb_test.go | 2 +- src/strings/example_test.go | 2 +- src/testing/sub_test.go | 4 ++-- src/text/template/exec.go | 2 +- 56 files changed, 104 insertions(+), 104 deletions(-) (limited to 'src/cmd/internal') diff --git a/src/cmd/fix/main.go b/src/cmd/fix/main.go index f06abae171..f54a5e0d96 100644 --- a/src/cmd/fix/main.go +++ b/src/cmd/fix/main.go @@ -52,7 +52,7 @@ func usage() { fmt.Fprintf(os.Stderr, "\n%s\n", f.name) } desc := strings.TrimSpace(f.desc) - desc = strings.Replace(desc, "\n", "\n\t", -1) + desc = strings.ReplaceAll(desc, "\n", "\n\t") fmt.Fprintf(os.Stderr, "\t%s\n", desc) } os.Exit(2) diff --git a/src/cmd/fix/typecheck.go b/src/cmd/fix/typecheck.go index eafb626c74..66e0cdcec0 100644 --- a/src/cmd/fix/typecheck.go +++ b/src/cmd/fix/typecheck.go @@ -193,12 +193,12 @@ func typecheck(cfg *TypeConfig, f *ast.File) (typeof map[interface{}]string, ass var params, results []string for _, p := range fn.Type.Params.List { t := gofmt(p.Type) - t = strings.Replace(t, "_Ctype_", "C.", -1) + t = strings.ReplaceAll(t, "_Ctype_", "C.") params = append(params, t) } for _, r := range fn.Type.Results.List { t := gofmt(r.Type) - t = strings.Replace(t, "_Ctype_", "C.", -1) + t = strings.ReplaceAll(t, "_Ctype_", "C.") results = append(results, t) } cfg.External["C."+fn.Name.Name[7:]] = joinFunc(params, results) diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go index 962b52fd3d..139ee73ae0 100644 --- a/src/cmd/go/go_test.go +++ b/src/cmd/go/go_test.go @@ -1090,7 +1090,7 @@ func testMove(t *testing.T, vcs, url, base, config string) { path := tg.path(filepath.Join("src", config)) data, err := ioutil.ReadFile(path) tg.must(err) - data = bytes.Replace(data, []byte(base), []byte(base+"XXX"), -1) + data = bytes.ReplaceAll(data, []byte(base), []byte(base+"XXX")) tg.must(ioutil.WriteFile(path, data, 0644)) } if vcs == "git" { @@ -2360,14 +2360,14 @@ func TestShadowingLogic(t *testing.T) { // The math in root1 is not "math" because the standard math is. tg.run("list", "-f", "({{.ImportPath}}) ({{.ConflictDir}})", "./testdata/shadow/root1/src/math") - pwdForwardSlash := strings.Replace(pwd, string(os.PathSeparator), "/", -1) + pwdForwardSlash := strings.ReplaceAll(pwd, string(os.PathSeparator), "/") if !strings.HasPrefix(pwdForwardSlash, "/") { pwdForwardSlash = "/" + pwdForwardSlash } // The output will have makeImportValid applies, but we only // bother to deal with characters we might reasonably see. for _, r := range " :" { - pwdForwardSlash = strings.Replace(pwdForwardSlash, string(r), "_", -1) + pwdForwardSlash = strings.ReplaceAll(pwdForwardSlash, string(r), "_") } want := "(_" + pwdForwardSlash + "/testdata/shadow/root1/src/math) (" + filepath.Join(runtime.GOROOT(), "src", "math") + ")" if strings.TrimSpace(tg.getStdout()) != want { @@ -2557,7 +2557,7 @@ func TestCoverageErrorLine(t *testing.T) { // It's OK that stderr2 drops the character position in the error, // because of the //line directive (see golang.org/issue/22662). - stderr = strings.Replace(stderr, "p.go:4:2:", "p.go:4:", -1) + stderr = strings.ReplaceAll(stderr, "p.go:4:2:", "p.go:4:") if stderr != stderr2 { t.Logf("test -cover changed error messages:\nbefore:\n%s\n\nafter:\n%s", stderr, stderr2) t.Skip("golang.org/issue/22660") @@ -6171,7 +6171,7 @@ func TestCDAndGOPATHAreDifferent(t *testing.T) { testCDAndGOPATHAreDifferent(tg, cd, gopath) if runtime.GOOS == "windows" { - testCDAndGOPATHAreDifferent(tg, cd, strings.Replace(gopath, `\`, `/`, -1)) + testCDAndGOPATHAreDifferent(tg, cd, strings.ReplaceAll(gopath, `\`, `/`)) testCDAndGOPATHAreDifferent(tg, cd, strings.ToUpper(gopath)) testCDAndGOPATHAreDifferent(tg, cd, strings.ToLower(gopath)) } diff --git a/src/cmd/go/internal/envcmd/env.go b/src/cmd/go/internal/envcmd/env.go index afadbade38..85a42e0519 100644 --- a/src/cmd/go/internal/envcmd/env.go +++ b/src/cmd/go/internal/envcmd/env.go @@ -203,7 +203,7 @@ func runEnv(cmd *base.Command, args []string) { fmt.Printf("%s=\"%s\"\n", e.Name, e.Value) case "plan9": if strings.IndexByte(e.Value, '\x00') < 0 { - fmt.Printf("%s='%s'\n", e.Name, strings.Replace(e.Value, "'", "''", -1)) + fmt.Printf("%s='%s'\n", e.Name, strings.ReplaceAll(e.Value, "'", "''")) } else { v := strings.Split(e.Value, "\x00") fmt.Printf("%s=(", e.Name) diff --git a/src/cmd/go/internal/get/vcs.go b/src/cmd/go/internal/get/vcs.go index 0f7b623ec3..173934b84e 100644 --- a/src/cmd/go/internal/get/vcs.go +++ b/src/cmd/go/internal/get/vcs.go @@ -964,7 +964,7 @@ func matchGoImport(imports []metaImport, importPath string) (metaImport, error) // expand rewrites s to replace {k} with match[k] for each key k in match. func expand(match map[string]string, s string) string { for k, v := range match { - s = strings.Replace(s, "{"+k+"}", v, -1) + s = strings.ReplaceAll(s, "{"+k+"}", v) } return s } diff --git a/src/cmd/go/internal/modconv/convert_test.go b/src/cmd/go/internal/modconv/convert_test.go index ad27abb8ef..4d55d73f21 100644 --- a/src/cmd/go/internal/modconv/convert_test.go +++ b/src/cmd/go/internal/modconv/convert_test.go @@ -146,7 +146,7 @@ func TestConvertLegacyConfig(t *testing.T) { } for _, tt := range tests { - t.Run(strings.Replace(tt.path, "/", "_", -1)+"_"+tt.vers, func(t *testing.T) { + t.Run(strings.ReplaceAll(tt.path, "/", "_")+"_"+tt.vers, func(t *testing.T) { f, err := modfile.Parse("golden", []byte(tt.gomod), nil) if err != nil { t.Fatal(err) diff --git a/src/cmd/go/internal/modfetch/codehost/codehost.go b/src/cmd/go/internal/modfetch/codehost/codehost.go index 4103ddc717..4205cd26bd 100644 --- a/src/cmd/go/internal/modfetch/codehost/codehost.go +++ b/src/cmd/go/internal/modfetch/codehost/codehost.go @@ -185,7 +185,7 @@ func (e *RunError) Error() string { text := e.Cmd + ": " + e.Err.Error() stderr := bytes.TrimRight(e.Stderr, "\n") if len(stderr) > 0 { - text += ":\n\t" + strings.Replace(string(stderr), "\n", "\n\t", -1) + text += ":\n\t" + strings.ReplaceAll(string(stderr), "\n", "\n\t") } return text } diff --git a/src/cmd/go/internal/modfetch/coderepo_test.go b/src/cmd/go/internal/modfetch/coderepo_test.go index 79b82786cb..0b62b9ee76 100644 --- a/src/cmd/go/internal/modfetch/coderepo_test.go +++ b/src/cmd/go/internal/modfetch/coderepo_test.go @@ -423,7 +423,7 @@ func TestCodeRepo(t *testing.T) { } } } - t.Run(strings.Replace(tt.path, "/", "_", -1)+"/"+tt.rev, f) + t.Run(strings.ReplaceAll(tt.path, "/", "_")+"/"+tt.rev, f) if strings.HasPrefix(tt.path, vgotest1git) { for _, alt := range altVgotests { // Note: Communicating with f through tt; should be cleaned up. @@ -442,7 +442,7 @@ func TestCodeRepo(t *testing.T) { tt.rev = remap(tt.rev, m) tt.gomoderr = remap(tt.gomoderr, m) tt.ziperr = remap(tt.ziperr, m) - t.Run(strings.Replace(tt.path, "/", "_", -1)+"/"+tt.rev, f) + t.Run(strings.ReplaceAll(tt.path, "/", "_")+"/"+tt.rev, f) tt = old } } @@ -473,9 +473,9 @@ func remap(name string, m map[string]string) string { } } for k, v := range m { - name = strings.Replace(name, k, v, -1) + name = strings.ReplaceAll(name, k, v) if codehost.AllHex(k) { - name = strings.Replace(name, k[:12], v[:12], -1) + name = strings.ReplaceAll(name, k[:12], v[:12]) } } return name @@ -522,7 +522,7 @@ func TestCodeRepoVersions(t *testing.T) { } defer os.RemoveAll(tmpdir) for _, tt := range codeRepoVersionsTests { - t.Run(strings.Replace(tt.path, "/", "_", -1), func(t *testing.T) { + t.Run(strings.ReplaceAll(tt.path, "/", "_"), func(t *testing.T) { repo, err := Lookup(tt.path) if err != nil { t.Fatalf("Lookup(%q): %v", tt.path, err) @@ -570,7 +570,7 @@ func TestLatest(t *testing.T) { } defer os.RemoveAll(tmpdir) for _, tt := range latestTests { - name := strings.Replace(tt.path, "/", "_", -1) + name := strings.ReplaceAll(tt.path, "/", "_") t.Run(name, func(t *testing.T) { repo, err := Lookup(tt.path) if err != nil { diff --git a/src/cmd/go/internal/modfetch/proxy.go b/src/cmd/go/internal/modfetch/proxy.go index 5f856b80d2..7c78502f31 100644 --- a/src/cmd/go/internal/modfetch/proxy.go +++ b/src/cmd/go/internal/modfetch/proxy.go @@ -248,5 +248,5 @@ func (p *proxyRepo) Zip(version string, tmpdir string) (tmpfile string, err erro // That is, it escapes things like ? and # (which really shouldn't appear anyway). // It does not escape / to %2F: our REST API is designed so that / can be left as is. func pathEscape(s string) string { - return strings.Replace(url.PathEscape(s), "%2F", "/", -1) + return strings.ReplaceAll(url.PathEscape(s), "%2F", "/") } diff --git a/src/cmd/go/internal/modload/import_test.go b/src/cmd/go/internal/modload/import_test.go index 3f4ddab436..9422a3d960 100644 --- a/src/cmd/go/internal/modload/import_test.go +++ b/src/cmd/go/internal/modload/import_test.go @@ -45,7 +45,7 @@ func TestImport(t *testing.T) { testenv.MustHaveExternalNetwork(t) for _, tt := range importTests { - t.Run(strings.Replace(tt.path, "/", "_", -1), func(t *testing.T) { + t.Run(strings.ReplaceAll(tt.path, "/", "_"), func(t *testing.T) { // Note that there is no build list, so Import should always fail. m, dir, err := Import(tt.path) if err == nil { diff --git a/src/cmd/go/internal/modload/query_test.go b/src/cmd/go/internal/modload/query_test.go index 7f3ffabef7..9b07383217 100644 --- a/src/cmd/go/internal/modload/query_test.go +++ b/src/cmd/go/internal/modload/query_test.go @@ -132,7 +132,7 @@ func TestQuery(t *testing.T) { ok, _ := path.Match(allow, m.Version) return ok } - t.Run(strings.Replace(tt.path, "/", "_", -1)+"/"+tt.query+"/"+allow, func(t *testing.T) { + t.Run(strings.ReplaceAll(tt.path, "/", "_")+"/"+tt.query+"/"+allow, func(t *testing.T) { info, err := Query(tt.path, tt.query, allowed) if tt.err != "" { if err != nil && err.Error() == tt.err { diff --git a/src/cmd/go/internal/search/search.go b/src/cmd/go/internal/search/search.go index 60ae73696b..0ca60e7349 100644 --- a/src/cmd/go/internal/search/search.go +++ b/src/cmd/go/internal/search/search.go @@ -275,7 +275,7 @@ func MatchPattern(pattern string) func(name string) bool { case strings.HasSuffix(re, `/\.\.\.`): re = strings.TrimSuffix(re, `/\.\.\.`) + `(/\.\.\.)?` } - re = strings.Replace(re, `\.\.\.`, `[^`+vendorChar+`]*`, -1) + re = strings.ReplaceAll(re, `\.\.\.`, `[^`+vendorChar+`]*`) reg := regexp.MustCompile(`^` + re + `$`) @@ -353,7 +353,7 @@ func CleanPatterns(patterns []string) []string { // as a courtesy to Windows developers, rewrite \ to / // in command-line arguments. Handles .\... and so on. if filepath.Separator == '\\' { - a = strings.Replace(a, `\`, `/`, -1) + a = strings.ReplaceAll(a, `\`, `/`) } // Put argument in canonical form, but preserve leading ./. diff --git a/src/cmd/go/internal/work/build.go b/src/cmd/go/internal/work/build.go index dd482b677d..145b87513a 100644 --- a/src/cmd/go/internal/work/build.go +++ b/src/cmd/go/internal/work/build.go @@ -398,10 +398,10 @@ func libname(args []string, pkgs []*load.Package) (string, error) { arg = bp.ImportPath } } - appendName(strings.Replace(arg, "/", "-", -1)) + appendName(strings.ReplaceAll(arg, "/", "-")) } else { for _, pkg := range pkgs { - appendName(strings.Replace(pkg.ImportPath, "/", "-", -1)) + appendName(strings.ReplaceAll(pkg.ImportPath, "/", "-")) } } } else if haveNonMeta { // have both meta package and a non-meta one diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go index 01414a3d57..158f5f3b17 100644 --- a/src/cmd/go/internal/work/exec.go +++ b/src/cmd/go/internal/work/exec.go @@ -1705,14 +1705,14 @@ func (b *Builder) fmtcmd(dir string, format string, args ...interface{}) string if dir[len(dir)-1] == filepath.Separator { dot += string(filepath.Separator) } - cmd = strings.Replace(" "+cmd, " "+dir, dot, -1)[1:] + cmd = strings.ReplaceAll(" "+cmd, " "+dir, dot)[1:] if b.scriptDir != dir { b.scriptDir = dir cmd = "cd " + dir + "\n" + cmd } } if b.WorkDir != "" { - cmd = strings.Replace(cmd, b.WorkDir, "$WORK", -1) + cmd = strings.ReplaceAll(cmd, b.WorkDir, "$WORK") } return cmd } @@ -1754,10 +1754,10 @@ func (b *Builder) showOutput(a *Action, dir, desc, out string) { prefix := "# " + desc suffix := "\n" + out if reldir := base.ShortPath(dir); reldir != dir { - suffix = strings.Replace(suffix, " "+dir, " "+reldir, -1) - suffix = strings.Replace(suffix, "\n"+dir, "\n"+reldir, -1) + suffix = strings.ReplaceAll(suffix, " "+dir, " "+reldir) + suffix = strings.ReplaceAll(suffix, "\n"+dir, "\n"+reldir) } - suffix = strings.Replace(suffix, " "+b.WorkDir, " $WORK", -1) + suffix = strings.ReplaceAll(suffix, " "+b.WorkDir, " $WORK") if a != nil && a.output != nil { a.output = append(a.output, prefix...) diff --git a/src/cmd/go/proxy_test.go b/src/cmd/go/proxy_test.go index 212e5aa08f..97fc4b0e80 100644 --- a/src/cmd/go/proxy_test.go +++ b/src/cmd/go/proxy_test.go @@ -78,7 +78,7 @@ func readModList() { if i < 0 { continue } - encPath := strings.Replace(name[:i], "_", "/", -1) + encPath := strings.ReplaceAll(name[:i], "_", "/") path, err := module.DecodePath(encPath) if err != nil { fmt.Fprintf(os.Stderr, "go proxy_test: %v\n", err) @@ -256,7 +256,7 @@ func readArchive(path, vers string) *txtar.Archive { return nil } - prefix := strings.Replace(enc, "/", "_", -1) + prefix := strings.ReplaceAll(enc, "/", "_") name := filepath.Join(cmdGoDir, "testdata/mod", prefix+"_"+encVers+".txt") a := archiveCache.Do(name, func() interface{} { a, err := txtar.ParseFile(name) diff --git a/src/cmd/go/script_test.go b/src/cmd/go/script_test.go index 7c083a87b9..31c6ede2a5 100644 --- a/src/cmd/go/script_test.go +++ b/src/cmd/go/script_test.go @@ -329,7 +329,7 @@ func (ts *testScript) cmdAddcrlf(neg bool, args []string) { file = ts.mkabs(file) data, err := ioutil.ReadFile(file) ts.check(err) - ts.check(ioutil.WriteFile(file, bytes.Replace(data, []byte("\n"), []byte("\r\n"), -1), 0666)) + ts.check(ioutil.WriteFile(file, bytes.ReplaceAll(data, []byte("\n"), []byte("\r\n")), 0666)) } } @@ -630,7 +630,7 @@ func scriptMatch(ts *testScript, neg bool, args []string, text, name string) { } // Matching against workdir would be misleading. - text = strings.Replace(text, ts.workdir, "$WORK", -1) + text = strings.ReplaceAll(text, ts.workdir, "$WORK") if neg { if re.MatchString(text) { @@ -691,7 +691,7 @@ func (ts *testScript) cmdSymlink(neg bool, args []string) { // abbrev abbreviates the actual work directory in the string s to the literal string "$WORK". func (ts *testScript) abbrev(s string) string { - s = strings.Replace(s, ts.workdir, "$WORK", -1) + s = strings.ReplaceAll(s, ts.workdir, "$WORK") if *testWork { // Expose actual $WORK value in environment dump on first line of work script, // so that the user can find out what directory -testwork left behind. @@ -885,17 +885,17 @@ var diffTests = []struct { func TestDiff(t *testing.T) { for _, tt := range diffTests { // Turn spaces into \n. - text1 := strings.Replace(tt.text1, " ", "\n", -1) + text1 := strings.ReplaceAll(tt.text1, " ", "\n") if text1 != "" { text1 += "\n" } - text2 := strings.Replace(tt.text2, " ", "\n", -1) + text2 := strings.ReplaceAll(tt.text2, " ", "\n") if text2 != "" { text2 += "\n" } out := diff(text1, text2) // Cut final \n, cut spaces, turn remaining \n into spaces. - out = strings.Replace(strings.Replace(strings.TrimSuffix(out, "\n"), " ", "", -1), "\n", " ", -1) + out = strings.ReplaceAll(strings.ReplaceAll(strings.TrimSuffix(out, "\n"), " ", ""), "\n", " ") if out != tt.diff { t.Errorf("diff(%q, %q) = %q, want %q", text1, text2, out, tt.diff) } diff --git a/src/cmd/go/testdata/addmod.go b/src/cmd/go/testdata/addmod.go index 19850af0f3..8bb6056a54 100644 --- a/src/cmd/go/testdata/addmod.go +++ b/src/cmd/go/testdata/addmod.go @@ -142,7 +142,7 @@ func main() { } data := txtar.Format(a) - target := filepath.Join("mod", strings.Replace(path, "/", "_", -1)+"_"+vers+".txt") + target := filepath.Join("mod", strings.ReplaceAll(path, "/", "_")+"_"+vers+".txt") if err := ioutil.WriteFile(target, data, 0666); err != nil { log.Printf("%s: %v", arg, err) exitCode = 1 diff --git a/src/cmd/go/vendor_test.go b/src/cmd/go/vendor_test.go index 22aa643b00..c302d7e9b5 100644 --- a/src/cmd/go/vendor_test.go +++ b/src/cmd/go/vendor_test.go @@ -37,7 +37,7 @@ func TestVendorImports(t *testing.T) { vend/x/vendor/p/p [notfound] vend/x/vendor/r [] ` - want = strings.Replace(want+"\t", "\n\t\t", "\n", -1) + want = strings.ReplaceAll(want+"\t", "\n\t\t", "\n") want = strings.TrimPrefix(want, "\n") have := tg.stdout.String() diff --git a/src/cmd/gofmt/gofmt_test.go b/src/cmd/gofmt/gofmt_test.go index 16b653b646..3008365cd2 100644 --- a/src/cmd/gofmt/gofmt_test.go +++ b/src/cmd/gofmt/gofmt_test.go @@ -200,7 +200,7 @@ func TestDiff(t *testing.T) { } if runtime.GOOS == "windows" { - b = bytes.Replace(b, []byte{'\r', '\n'}, []byte{'\n'}, -1) + b = bytes.ReplaceAll(b, []byte{'\r', '\n'}, []byte{'\n'}) } bs := bytes.SplitN(b, []byte{'\n'}, 3) diff --git a/src/cmd/internal/goobj/read.go b/src/cmd/internal/goobj/read.go index e39180cad6..2d618eefa5 100644 --- a/src/cmd/internal/goobj/read.go +++ b/src/cmd/internal/goobj/read.go @@ -293,7 +293,7 @@ func (r *objReader) readRef() { // In a symbol name in an object file, "". denotes the // prefix for the package in which the object file has been found. // Expand it. - name = strings.Replace(name, `"".`, r.pkgprefix, -1) + name = strings.ReplaceAll(name, `"".`, r.pkgprefix) // An individual object file only records version 0 (extern) or 1 (static). // To make static symbols unique across all files being read, we diff --git a/src/cmd/trace/trace.go b/src/cmd/trace/trace.go index 676b9ffa5a..07fc4333eb 100644 --- a/src/cmd/trace/trace.go +++ b/src/cmd/trace/trace.go @@ -38,7 +38,7 @@ func httpTrace(w http.ResponseWriter, r *http.Request) { http.Error(w, err.Error(), http.StatusInternalServerError) return } - html := strings.Replace(templTrace, "{{PARAMS}}", r.Form.Encode(), -1) + html := strings.ReplaceAll(templTrace, "{{PARAMS}}", r.Form.Encode()) w.Write([]byte(html)) } diff --git a/src/cmd/vet/main.go b/src/cmd/vet/main.go index 646adf4d76..6e885121c8 100644 --- a/src/cmd/vet/main.go +++ b/src/cmd/vet/main.go @@ -273,7 +273,7 @@ func main() { // Accept space-separated tags because that matches // the go command's other subcommands. // Accept commas because go tool vet traditionally has. - tagList = strings.Fields(strings.Replace(*tags, ",", " ", -1)) + tagList = strings.Fields(strings.ReplaceAll(*tags, ",", " ")) initPrintFlags() initUnusedFlags() diff --git a/src/cmd/vet/vet_test.go b/src/cmd/vet/vet_test.go index 90665d77bc..df84d6cc98 100644 --- a/src/cmd/vet/vet_test.go +++ b/src/cmd/vet/vet_test.go @@ -243,7 +243,7 @@ func errorCheck(outStr string, wantAuto bool, fullshort ...string) (err error) { for i := range out { for j := 0; j < len(fullshort); j += 2 { full, short := fullshort[j], fullshort[j+1] - out[i] = strings.Replace(out[i], full, short, -1) + out[i] = strings.ReplaceAll(out[i], full, short) } } diff --git a/src/crypto/rsa/pss_test.go b/src/crypto/rsa/pss_test.go index cae24e58c6..dfa8d8bb5a 100644 --- a/src/crypto/rsa/pss_test.go +++ b/src/crypto/rsa/pss_test.go @@ -103,7 +103,7 @@ func TestPSSGolden(t *testing.T) { switch { case len(line) == 0: if len(partialValue) > 0 { - values <- strings.Replace(partialValue, " ", "", -1) + values <- strings.ReplaceAll(partialValue, " ", "") partialValue = "" lastWasValue = true } diff --git a/src/crypto/x509/root_darwin_arm_gen.go b/src/crypto/x509/root_darwin_arm_gen.go index b5580d6f02..bc44124a78 100644 --- a/src/crypto/x509/root_darwin_arm_gen.go +++ b/src/crypto/x509/root_darwin_arm_gen.go @@ -154,9 +154,9 @@ func fetchCertIDs() ([]certID, error) { values := regexp.MustCompile("(?s)(.*?)").FindAllStringSubmatch(row, -1) name := values[cols["Certificate name"]][1] fingerprint := values[cols["Fingerprint (SHA-256)"]][1] - fingerprint = strings.Replace(fingerprint, "
", "", -1) - fingerprint = strings.Replace(fingerprint, "\n", "", -1) - fingerprint = strings.Replace(fingerprint, " ", "", -1) + fingerprint = strings.ReplaceAll(fingerprint, "
", "") + fingerprint = strings.ReplaceAll(fingerprint, "\n", "") + fingerprint = strings.ReplaceAll(fingerprint, " ", "") fingerprint = strings.ToLower(fingerprint) ids = append(ids, certID{ diff --git a/src/encoding/base32/base32_test.go b/src/encoding/base32/base32_test.go index c5506ed4de..b74054ba40 100644 --- a/src/encoding/base32/base32_test.go +++ b/src/encoding/base32/base32_test.go @@ -425,7 +425,7 @@ IZJAOZSWY2LUEBSXG43FEBRWS3DMOVWSAZDPNRXXEZJAMV2SAZTVM5UWC5BANZ2WY3DBBJYGC4TJMF NZ2CYIDTOVXHIIDJNYFGG5LMOBQSA4LVNEQG6ZTGNFRWSYJAMRSXGZLSOVXHIIDNN5WGY2LUEBQW42 LNEBUWIIDFON2CA3DBMJXXE5LNFY== ====` - encodedShort := strings.Replace(encoded, "\n", "", -1) + encodedShort := strings.ReplaceAll(encoded, "\n", "") dec := NewDecoder(StdEncoding, strings.NewReader(encoded)) res1, err := ioutil.ReadAll(dec) @@ -465,7 +465,7 @@ func TestWithCustomPadding(t *testing.T) { for _, testcase := range pairs { defaultPadding := StdEncoding.EncodeToString([]byte(testcase.decoded)) customPadding := StdEncoding.WithPadding('@').EncodeToString([]byte(testcase.decoded)) - expected := strings.Replace(defaultPadding, "=", "@", -1) + expected := strings.ReplaceAll(defaultPadding, "=", "@") if expected != customPadding { t.Errorf("Expected custom %s, got %s", expected, customPadding) @@ -675,7 +675,7 @@ func TestWithoutPaddingClose(t *testing.T) { expected := testpair.encoded if encoding.padChar == NoPadding { - expected = strings.Replace(expected, "=", "", -1) + expected = strings.ReplaceAll(expected, "=", "") } res := buf.String() @@ -697,7 +697,7 @@ func TestDecodeReadAll(t *testing.T) { for encIndex, encoding := range encodings { encoded := pair.encoded if encoding.padChar == NoPadding { - encoded = strings.Replace(encoded, "=", "", -1) + encoded = strings.ReplaceAll(encoded, "=", "") } decReader, err := ioutil.ReadAll(NewDecoder(encoding, strings.NewReader(encoded))) @@ -723,7 +723,7 @@ func TestDecodeSmallBuffer(t *testing.T) { for encIndex, encoding := range encodings { encoded := pair.encoded if encoding.padChar == NoPadding { - encoded = strings.Replace(encoded, "=", "", -1) + encoded = strings.ReplaceAll(encoded, "=", "") } decoder := NewDecoder(encoding, strings.NewReader(encoded)) diff --git a/src/encoding/base64/base64_test.go b/src/encoding/base64/base64_test.go index f019654f5b..f7f312ca39 100644 --- a/src/encoding/base64/base64_test.go +++ b/src/encoding/base64/base64_test.go @@ -53,8 +53,8 @@ func stdRef(ref string) string { // Convert a reference string to URL-encoding func urlRef(ref string) string { - ref = strings.Replace(ref, "+", "-", -1) - ref = strings.Replace(ref, "/", "_", -1) + ref = strings.ReplaceAll(ref, "+", "-") + ref = strings.ReplaceAll(ref, "/", "_") return ref } @@ -72,7 +72,7 @@ func rawURLRef(ref string) string { var funnyEncoding = NewEncoding(encodeStd).WithPadding(rune('@')) func funnyRef(ref string) string { - return strings.Replace(ref, "=", "@", -1) + return strings.ReplaceAll(ref, "=", "@") } type encodingTest struct { @@ -418,7 +418,7 @@ j+mSARB/17pKVXYWHXjsj7yIex0PadzXMO1zT5KHoNA3HT8ietoGhgjsfA+CSnvvqh/jJtqsrwOv 2b6NGNzXfTYexzJ+nU7/ALkf4P8Awv6P9KvTQQ4AgyDqCF85Pho3CTB7eHwXoH+LT65uZbX9X+o2 bqbPb06551Y4 ` - encodedShort := strings.Replace(encoded, "\n", "", -1) + encodedShort := strings.ReplaceAll(encoded, "\n", "") dec := NewDecoder(StdEncoding, strings.NewReader(encoded)) res1, err := ioutil.ReadAll(dec) diff --git a/src/flag/flag.go b/src/flag/flag.go index 2cd7829c1a..ae84e1f775 100644 --- a/src/flag/flag.go +++ b/src/flag/flag.go @@ -472,7 +472,7 @@ func (f *FlagSet) PrintDefaults() { // for both 4- and 8-space tab stops. s += "\n \t" } - s += strings.Replace(usage, "\n", "\n \t", -1) + s += strings.ReplaceAll(usage, "\n", "\n \t") if !isZeroValue(flag, flag.DefValue) { if _, ok := flag.Value.(*stringValue); ok { diff --git a/src/go/constant/value_test.go b/src/go/constant/value_test.go index e6fca76e18..68b87eaa55 100644 --- a/src/go/constant/value_test.go +++ b/src/go/constant/value_test.go @@ -296,7 +296,7 @@ func val(lit string) Value { switch first, last := lit[0], lit[len(lit)-1]; { case first == '"' || first == '`': tok = token.STRING - lit = strings.Replace(lit, "_", " ", -1) + lit = strings.ReplaceAll(lit, "_", " ") case first == '\'': tok = token.CHAR case last == 'i': diff --git a/src/go/doc/doc_test.go b/src/go/doc/doc_test.go index 902a79f63f..0b2d2b63cc 100644 --- a/src/go/doc/doc_test.go +++ b/src/go/doc/doc_test.go @@ -40,7 +40,7 @@ func readTemplate(filename string) *template.Template { func nodeFmt(node interface{}, fset *token.FileSet) string { var buf bytes.Buffer printer.Fprint(&buf, fset, node) - return strings.Replace(strings.TrimSpace(buf.String()), "\n", "\n\t", -1) + return strings.ReplaceAll(strings.TrimSpace(buf.String()), "\n", "\n\t") } func synopsisFmt(s string) string { @@ -53,7 +53,7 @@ func synopsisFmt(s string) string { } s = strings.TrimSpace(s) + " ..." } - return "// " + strings.Replace(s, "\n", " ", -1) + return "// " + strings.ReplaceAll(s, "\n", " ") } func indentFmt(indent, s string) string { @@ -62,7 +62,7 @@ func indentFmt(indent, s string) string { end = "\n" s = s[:len(s)-1] } - return indent + strings.Replace(s, "\n", "\n"+indent, -1) + end + return indent + strings.ReplaceAll(s, "\n", "\n"+indent) + end } func isGoFile(fi os.FileInfo) bool { diff --git a/src/go/printer/example_test.go b/src/go/printer/example_test.go index e570040ba1..30816931a8 100644 --- a/src/go/printer/example_test.go +++ b/src/go/printer/example_test.go @@ -48,7 +48,7 @@ func ExampleFprint() { // and trim leading and trailing white space. s := buf.String() s = s[1 : len(s)-1] - s = strings.TrimSpace(strings.Replace(s, "\n\t", "\n", -1)) + s = strings.TrimSpace(strings.ReplaceAll(s, "\n\t", "\n")) // Print the cleaned-up body text to stdout. fmt.Println(s) @@ -61,7 +61,7 @@ func ExampleFprint() { // // s := buf.String() // s = s[1 : len(s)-1] - // s = strings.TrimSpace(strings.Replace(s, "\n\t", "\n", -1)) + // s = strings.TrimSpace(strings.ReplaceAll(s, "\n\t", "\n")) // // fmt.Println(s) } diff --git a/src/html/template/js.go b/src/html/template/js.go index 33a18b4186..2291f47c33 100644 --- a/src/html/template/js.go +++ b/src/html/template/js.go @@ -172,7 +172,7 @@ func jsValEscaper(args ...interface{}) string { // turning into // x//* error marshaling y: // second line of error message */null - return fmt.Sprintf(" /* %s */null ", strings.Replace(err.Error(), "*/", "* /", -1)) + return fmt.Sprintf(" /* %s */null ", strings.ReplaceAll(err.Error(), "*/", "* /")) } // TODO: maybe post-process output to prevent it from containing diff --git a/src/html/template/url.go b/src/html/template/url.go index f0516300de..8a4f727e50 100644 --- a/src/html/template/url.go +++ b/src/html/template/url.go @@ -156,7 +156,7 @@ func srcsetFilterAndEscaper(args ...interface{}) string { s = b.String() } // Additionally, commas separate one source from another. - return strings.Replace(s, ",", "%2c", -1) + return strings.ReplaceAll(s, ",", "%2c") } var b bytes.Buffer diff --git a/src/mime/multipart/formdata_test.go b/src/mime/multipart/formdata_test.go index 2d6a830cb6..105a82c417 100644 --- a/src/mime/multipart/formdata_test.go +++ b/src/mime/multipart/formdata_test.go @@ -13,7 +13,7 @@ import ( ) func TestReadForm(t *testing.T) { - b := strings.NewReader(strings.Replace(message, "\n", "\r\n", -1)) + b := strings.NewReader(strings.ReplaceAll(message, "\n", "\r\n")) r := NewReader(b, boundary) f, err := r.ReadForm(25) if err != nil { @@ -39,7 +39,7 @@ func TestReadForm(t *testing.T) { } func TestReadFormWithNamelessFile(t *testing.T) { - b := strings.NewReader(strings.Replace(messageWithFileWithoutName, "\n", "\r\n", -1)) + b := strings.NewReader(strings.ReplaceAll(messageWithFileWithoutName, "\n", "\r\n")) r := NewReader(b, boundary) f, err := r.ReadForm(25) if err != nil { @@ -54,7 +54,7 @@ func TestReadFormWithNamelessFile(t *testing.T) { func TestReadFormWithTextContentType(t *testing.T) { // From https://github.com/golang/go/issues/24041 - b := strings.NewReader(strings.Replace(messageWithTextContentType, "\n", "\r\n", -1)) + b := strings.NewReader(strings.ReplaceAll(messageWithTextContentType, "\n", "\r\n")) r := NewReader(b, boundary) f, err := r.ReadForm(25) if err != nil { @@ -184,7 +184,7 @@ Content-Disposition: form-data; name="largetext" --MyBoundary-- ` - testBody := strings.Replace(message, "\n", "\r\n", -1) + testBody := strings.ReplaceAll(message, "\n", "\r\n") testCases := []struct { name string maxMemory int64 diff --git a/src/mime/multipart/multipart_test.go b/src/mime/multipart/multipart_test.go index abe1cc8e77..7bf606765c 100644 --- a/src/mime/multipart/multipart_test.go +++ b/src/mime/multipart/multipart_test.go @@ -105,7 +105,7 @@ never read data useless trailer ` - testBody = strings.Replace(testBody, "\n", sep, -1) + testBody = strings.ReplaceAll(testBody, "\n", sep) return strings.Replace(testBody, "[longline]", longLine, 1) } @@ -151,7 +151,7 @@ func testMultipart(t *testing.T, r io.Reader, onlyNewlines bool) { adjustNewlines := func(s string) string { if onlyNewlines { - return strings.Replace(s, "\r\n", "\n", -1) + return strings.ReplaceAll(s, "\r\n", "\n") } return s } @@ -299,7 +299,7 @@ foo-bar: baz Oh no, premature EOF! ` - body := strings.Replace(testBody, "\n", "\r\n", -1) + body := strings.ReplaceAll(testBody, "\n", "\r\n") bodyReader := strings.NewReader(body) r := NewReader(bodyReader, "MyBoundary") diff --git a/src/net/http/cgi/child.go b/src/net/http/cgi/child.go index da12ac3498..10325c2eb5 100644 --- a/src/net/http/cgi/child.go +++ b/src/net/http/cgi/child.go @@ -86,7 +86,7 @@ func RequestFromMap(params map[string]string) (*http.Request, error) { if !strings.HasPrefix(k, "HTTP_") || k == "HTTP_HOST" { continue } - r.Header.Add(strings.Replace(k[5:], "_", "-", -1), v) + r.Header.Add(strings.ReplaceAll(k[5:], "_", "-"), v) } // TODO: cookies. parsing them isn't exported, though. diff --git a/src/net/http/httputil/dump_test.go b/src/net/http/httputil/dump_test.go index 5703a7fb86..63312dd885 100644 --- a/src/net/http/httputil/dump_test.go +++ b/src/net/http/httputil/dump_test.go @@ -370,7 +370,7 @@ func TestDumpResponse(t *testing.T) { } got := string(gotb) got = strings.TrimSpace(got) - got = strings.Replace(got, "\r", "", -1) + got = strings.ReplaceAll(got, "\r", "") if got != tt.want { t.Errorf("%d.\nDumpResponse got:\n%s\n\nWant:\n%s\n", i, got, tt.want) diff --git a/src/net/http/readrequest_test.go b/src/net/http/readrequest_test.go index 18eed345a8..517a8189e1 100644 --- a/src/net/http/readrequest_test.go +++ b/src/net/http/readrequest_test.go @@ -438,7 +438,7 @@ func TestReadRequest(t *testing.T) { // reqBytes treats req as a request (with \n delimiters) and returns it with \r\n delimiters, // ending in \r\n\r\n func reqBytes(req string) []byte { - return []byte(strings.Replace(strings.TrimSpace(req), "\n", "\r\n", -1) + "\r\n\r\n") + return []byte(strings.ReplaceAll(strings.TrimSpace(req), "\n", "\r\n") + "\r\n\r\n") } var badRequestTests = []struct { diff --git a/src/net/http/request_test.go b/src/net/http/request_test.go index 7a83ae5b1c..e8005571df 100644 --- a/src/net/http/request_test.go +++ b/src/net/http/request_test.go @@ -878,7 +878,7 @@ func testMissingFile(t *testing.T, req *Request) { } func newTestMultipartRequest(t *testing.T) *Request { - b := strings.NewReader(strings.Replace(message, "\n", "\r\n", -1)) + b := strings.NewReader(strings.ReplaceAll(message, "\n", "\r\n")) req, err := NewRequest("POST", "/", b) if err != nil { t.Fatal("NewRequest:", err) @@ -970,8 +970,8 @@ Content-Disposition: form-data; name="textb" ` func benchmarkReadRequest(b *testing.B, request string) { - request = request + "\n" // final \n - request = strings.Replace(request, "\n", "\r\n", -1) // expand \n to \r\n + request = request + "\n" // final \n + request = strings.ReplaceAll(request, "\n", "\r\n") // expand \n to \r\n b.SetBytes(int64(len(request))) r := bufio.NewReader(&infiniteReader{buf: []byte(request)}) b.ReportAllocs() diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go index 8dae95678d..b12fcf4f9e 100644 --- a/src/net/http/serve_test.go +++ b/src/net/http/serve_test.go @@ -130,7 +130,7 @@ func (c *testConn) Close() error { // reqBytes treats req as a request (with \n delimiters) and returns it with \r\n delimiters, // ending in \r\n\r\n func reqBytes(req string) []byte { - return []byte(strings.Replace(strings.TrimSpace(req), "\n", "\r\n", -1) + "\r\n\r\n") + return []byte(strings.ReplaceAll(strings.TrimSpace(req), "\n", "\r\n") + "\r\n\r\n") } type handlerTest struct { diff --git a/src/net/lookup_windows_test.go b/src/net/lookup_windows_test.go index cebb2d0558..d3748f28c3 100644 --- a/src/net/lookup_windows_test.go +++ b/src/net/lookup_windows_test.go @@ -150,7 +150,7 @@ func nslookup(qtype, name string) (string, error) { if err := cmd.Run(); err != nil { return "", err } - r := strings.Replace(out.String(), "\r\n", "\n", -1) + r := strings.ReplaceAll(out.String(), "\r\n", "\n") // nslookup stderr output contains also debug information such as // "Non-authoritative answer" and it doesn't return the correct errcode if strings.Contains(err.String(), "can't find") { diff --git a/src/net/mail/message_test.go b/src/net/mail/message_test.go index b19da52c42..14ac9192a4 100644 --- a/src/net/mail/message_test.go +++ b/src/net/mail/message_test.go @@ -668,9 +668,9 @@ func TestAddressParser(t *testing.T) { switch charset { case "iso-8859-15": - in = bytes.Replace(in, []byte("\xf6"), []byte("ö"), -1) + in = bytes.ReplaceAll(in, []byte("\xf6"), []byte("ö")) case "windows-1252": - in = bytes.Replace(in, []byte("\xe9"), []byte("é"), -1) + in = bytes.ReplaceAll(in, []byte("\xe9"), []byte("é")) } return bytes.NewReader(in), nil diff --git a/src/net/net_windows_test.go b/src/net/net_windows_test.go index 8dfd312980..8aa719f433 100644 --- a/src/net/net_windows_test.go +++ b/src/net/net_windows_test.go @@ -571,7 +571,7 @@ func TestInterfaceHardwareAddrWithGetmac(t *testing.T) { // skip these return } - addr = strings.Replace(addr, "-", ":", -1) + addr = strings.ReplaceAll(addr, "-", ":") cname := getValue("Connection Name") want[cname] = addr group = make(map[string]string) diff --git a/src/net/url/example_test.go b/src/net/url/example_test.go index d8eb6dcd20..ad67f5328a 100644 --- a/src/net/url/example_test.go +++ b/src/net/url/example_test.go @@ -219,5 +219,5 @@ func toJSON(m interface{}) string { if err != nil { log.Fatal(err) } - return strings.Replace(string(js), ",", ", ", -1) + return strings.ReplaceAll(string(js), ",", ", ") } diff --git a/src/net/url/url_test.go b/src/net/url/url_test.go index 5d3f91248f..7c4ada245a 100644 --- a/src/net/url/url_test.go +++ b/src/net/url/url_test.go @@ -848,18 +848,18 @@ func TestUnescape(t *testing.T) { in := tt.in out := tt.out if strings.Contains(tt.in, "+") { - in = strings.Replace(tt.in, "+", "%20", -1) + in = strings.ReplaceAll(tt.in, "+", "%20") actual, err := PathUnescape(in) if actual != tt.out || (err != nil) != (tt.err != nil) { t.Errorf("PathUnescape(%q) = %q, %s; want %q, %s", in, actual, err, tt.out, tt.err) } if tt.err == nil { - s, err := QueryUnescape(strings.Replace(tt.in, "+", "XXX", -1)) + s, err := QueryUnescape(strings.ReplaceAll(tt.in, "+", "XXX")) if err != nil { continue } in = tt.in - out = strings.Replace(s, "XXX", "+", -1) + out = strings.ReplaceAll(s, "XXX", "+") } } diff --git a/src/os/path_windows_test.go b/src/os/path_windows_test.go index 00a3e63bf3..f1745ad132 100644 --- a/src/os/path_windows_test.go +++ b/src/os/path_windows_test.go @@ -38,10 +38,10 @@ func TestFixLongPath(t *testing.T) { {`\\?\c:\long\foo.txt`, `\\?\c:\long\foo.txt`}, {`\\?\c:\long/foo.txt`, `\\?\c:\long/foo.txt`}, } { - in := strings.Replace(test.in, "long", veryLong, -1) - want := strings.Replace(test.want, "long", veryLong, -1) + in := strings.ReplaceAll(test.in, "long", veryLong) + want := strings.ReplaceAll(test.want, "long", veryLong) if got := os.FixLongPath(in); got != want { - got = strings.Replace(got, veryLong, "long", -1) + got = strings.ReplaceAll(got, veryLong, "long") t.Errorf("fixLongPath(%q) = %q; want %q", test.in, got, test.want) } } diff --git a/src/path/filepath/path.go b/src/path/filepath/path.go index 1508137a33..aba1717e7d 100644 --- a/src/path/filepath/path.go +++ b/src/path/filepath/path.go @@ -166,7 +166,7 @@ func ToSlash(path string) string { if Separator == '/' { return path } - return strings.Replace(path, string(Separator), "/", -1) + return strings.ReplaceAll(path, string(Separator), "/") } // FromSlash returns the result of replacing each slash ('/') character @@ -176,7 +176,7 @@ func FromSlash(path string) string { if Separator == '/' { return path } - return strings.Replace(path, "/", string(Separator), -1) + return strings.ReplaceAll(path, "/", string(Separator)) } // SplitList splits a list of paths joined by the OS-specific ListSeparator, diff --git a/src/path/filepath/path_test.go b/src/path/filepath/path_test.go index a221a3d4fa..e1b5ad1d40 100644 --- a/src/path/filepath/path_test.go +++ b/src/path/filepath/path_test.go @@ -1062,7 +1062,7 @@ func TestAbs(t *testing.T) { } for _, path := range absTests { - path = strings.Replace(path, "$", root, -1) + path = strings.ReplaceAll(path, "$", root) info, err := os.Stat(path) if err != nil { t.Errorf("%s: %s", path, err) diff --git a/src/path/filepath/path_windows.go b/src/path/filepath/path_windows.go index 519b6ebc32..6a144d9e0b 100644 --- a/src/path/filepath/path_windows.go +++ b/src/path/filepath/path_windows.go @@ -100,7 +100,7 @@ func splitList(path string) []string { // Remove quotes. for i, s := range list { - list[i] = strings.Replace(s, `"`, ``, -1) + list[i] = strings.ReplaceAll(s, `"`, ``) } return list diff --git a/src/path/filepath/path_windows_test.go b/src/path/filepath/path_windows_test.go index e36a3c9b64..63eab18116 100644 --- a/src/path/filepath/path_windows_test.go +++ b/src/path/filepath/path_windows_test.go @@ -431,7 +431,7 @@ func TestToNorm(t *testing.T) { t.Fatal(err) } - err = os.MkdirAll(strings.Replace(testPath, "{{tmp}}", ctmp, -1), 0777) + err = os.MkdirAll(strings.ReplaceAll(testPath, "{{tmp}}", ctmp), 0777) if err != nil { t.Fatal(err) } diff --git a/src/runtime/pprof/internal/profile/profile.go b/src/runtime/pprof/internal/profile/profile.go index 64c3e3f054..863bd403a4 100644 --- a/src/runtime/pprof/internal/profile/profile.go +++ b/src/runtime/pprof/internal/profile/profile.go @@ -200,7 +200,7 @@ var libRx = regexp.MustCompile(`([.]so$|[.]so[._][0-9]+)`) // first. func (p *Profile) setMain() { for i := 0; i < len(p.Mapping); i++ { - file := strings.TrimSpace(strings.Replace(p.Mapping[i].File, "(deleted)", "", -1)) + file := strings.TrimSpace(strings.ReplaceAll(p.Mapping[i].File, "(deleted)", "")) if len(file) == 0 { continue } diff --git a/src/runtime/pprof/pprof_test.go b/src/runtime/pprof/pprof_test.go index 126ba50054..593924183f 100644 --- a/src/runtime/pprof/pprof_test.go +++ b/src/runtime/pprof/pprof_test.go @@ -602,7 +602,7 @@ func TestBlockProfile(t *testing.T) { } for _, test := range tests { - if !regexp.MustCompile(strings.Replace(test.re, "\t", "\t+", -1)).MatchString(prof) { + if !regexp.MustCompile(strings.ReplaceAll(test.re, "\t", "\t+")).MatchString(prof) { t.Errorf("Bad %v entry, expect:\n%v\ngot:\n%v", test.name, test.re, prof) } } diff --git a/src/runtime/runtime-gdb_test.go b/src/runtime/runtime-gdb_test.go index 1ed6b39303..26f507159b 100644 --- a/src/runtime/runtime-gdb_test.go +++ b/src/runtime/runtime-gdb_test.go @@ -490,7 +490,7 @@ func TestGdbConst(t *testing.T) { } got, _ := exec.Command("gdb", args...).CombinedOutput() - sgot := strings.Replace(string(got), "\r\n", "\n", -1) + sgot := strings.ReplaceAll(string(got), "\r\n", "\n") t.Logf("output %q", sgot) diff --git a/src/strings/example_test.go b/src/strings/example_test.go index 607e4a0a70..103ef51f29 100644 --- a/src/strings/example_test.go +++ b/src/strings/example_test.go @@ -199,7 +199,7 @@ func ExampleRepeat() { func ExampleReplace() { fmt.Println(strings.Replace("oink oink oink", "k", "ky", 2)) - fmt.Println(strings.Replace("oink oink oink", "oink", "moo", -1)) + fmt.Println(strings.ReplaceAll("oink oink oink", "oink", "moo")) // Output: // oinky oinky oink // moo moo moo diff --git a/src/testing/sub_test.go b/src/testing/sub_test.go index 9af3909b35..29803c06e2 100644 --- a/src/testing/sub_test.go +++ b/src/testing/sub_test.go @@ -594,8 +594,8 @@ func TestBRun(t *T) { func makeRegexp(s string) string { s = regexp.QuoteMeta(s) - s = strings.Replace(s, ":NNN:", `:\d\d\d:`, -1) - s = strings.Replace(s, "N\\.NNs", `\d*\.\d*s`, -1) + s = strings.ReplaceAll(s, ":NNN:", `:\d\d\d:`) + s = strings.ReplaceAll(s, "N\\.NNs", `\d*\.\d*s`) return s } diff --git a/src/text/template/exec.go b/src/text/template/exec.go index 214f72d51b..1d04c2982f 100644 --- a/src/text/template/exec.go +++ b/src/text/template/exec.go @@ -102,7 +102,7 @@ func (s *state) at(node parse.Node) { // doublePercent returns the string with %'s replaced by %%, if necessary, // so it can be used safely inside a Printf format string. func doublePercent(str string) string { - return strings.Replace(str, "%", "%%", -1) + return strings.ReplaceAll(str, "%", "%%") } // TODO: It would be nice if ExecError was more broken down, but -- cgit v1.3-5-g9baa From 1c8943bd59157878141faab0c93848f45d3d51d1 Mon Sep 17 00:00:00 2001 From: Alessandro Arzilli Date: Tue, 25 Sep 2018 11:52:24 +0200 Subject: cmd/link: move DIE of global variables to their compile unit The DIEs for global variables were all assigned to the first emitted compile unit in debug_info, regardless of what it was. Move them instead to their respective compile units. Change-Id: If794fa0ba4702f5b959c6e8c16119b16e7ecf6d8 Reviewed-on: https://go-review.googlesource.com/137235 Reviewed-by: Than McIntosh --- src/cmd/compile/internal/ssa/stmtlines_test.go | 3 + src/cmd/internal/dwarf/dwarf.go | 13 +++ src/cmd/link/dwarf_test.go | 3 + src/cmd/link/internal/ld/dwarf.go | 126 +++++++++++++------------ src/cmd/link/internal/objfile/objfile.go | 2 +- 5 files changed, 88 insertions(+), 59 deletions(-) (limited to 'src/cmd/internal') diff --git a/src/cmd/compile/internal/ssa/stmtlines_test.go b/src/cmd/compile/internal/ssa/stmtlines_test.go index 1081f83f6d..c0fc7adab5 100644 --- a/src/cmd/compile/internal/ssa/stmtlines_test.go +++ b/src/cmd/compile/internal/ssa/stmtlines_test.go @@ -62,6 +62,9 @@ func TestStmtLines(t *testing.T) { if pkgname == "runtime" { continue } + if e.Val(dwarf.AttrStmtList) == nil { + continue + } lrdr, err := dw.LineReader(e) must(err) diff --git a/src/cmd/internal/dwarf/dwarf.go b/src/cmd/internal/dwarf/dwarf.go index 96fb2b765b..355091feda 100644 --- a/src/cmd/internal/dwarf/dwarf.go +++ b/src/cmd/internal/dwarf/dwarf.go @@ -304,6 +304,7 @@ const ( const ( DW_ABRV_NULL = iota DW_ABRV_COMPUNIT + DW_ABRV_COMPUNIT_TEXTLESS DW_ABRV_FUNCTION DW_ABRV_FUNCTION_ABSTRACT DW_ABRV_FUNCTION_CONCRETE @@ -368,6 +369,18 @@ var abbrevs = [DW_NABRV]dwAbbrev{ }, }, + /* COMPUNIT_TEXTLESS */ + { + DW_TAG_compile_unit, + DW_CHILDREN_yes, + []dwAttrForm{ + {DW_AT_name, DW_FORM_string}, + {DW_AT_language, DW_FORM_data1}, + {DW_AT_comp_dir, DW_FORM_string}, + {DW_AT_producer, DW_FORM_string}, + }, + }, + /* FUNCTION */ { DW_TAG_subprogram, diff --git a/src/cmd/link/dwarf_test.go b/src/cmd/link/dwarf_test.go index ff11689bbc..2c01456f6b 100644 --- a/src/cmd/link/dwarf_test.go +++ b/src/cmd/link/dwarf_test.go @@ -122,6 +122,9 @@ func testDWARF(t *testing.T, buildmode string, expectDWARF bool, env ...string) r.SkipChildren() continue } + if cu.Val(dwarf.AttrStmtList) == nil { + continue + } lr, err := d.LineReader(cu) if err != nil { t.Fatal(err) diff --git a/src/cmd/link/internal/ld/dwarf.go b/src/cmd/link/internal/ld/dwarf.go index 2164fa80a0..743f4cedd4 100644 --- a/src/cmd/link/internal/ld/dwarf.go +++ b/src/cmd/link/internal/ld/dwarf.go @@ -5,7 +5,7 @@ // TODO/NICETOHAVE: // - eliminate DW_CLS_ if not used // - package info in compilation units -// - assign global variables and types to their packages +// - assign types to their packages // - gdb uses c syntax, meaning clumsy quoting is needed for go identifiers. eg // ptype struct '[]uint8' and qualifiers need to be quoted away // - file:line info for variables @@ -106,15 +106,8 @@ func writeabbrev(ctxt *Link) *sym.Symbol { return s } -/* - * Root DIEs for compilation units, types and global variables. - */ -var dwroot dwarf.DWDie - var dwtypes dwarf.DWDie -var dwglobals dwarf.DWDie - func newattr(die *dwarf.DWDie, attr uint16, cls int, value int64, data interface{}) *dwarf.DWAttr { a := new(dwarf.DWAttr) a.Link = die.Attr @@ -835,7 +828,11 @@ func synthesizechantypes(ctxt *Link, die *dwarf.DWDie) { } func dwarfDefineGlobal(ctxt *Link, s *sym.Symbol, str string, v int64, gotype *sym.Symbol) { - dv := newdie(ctxt, &dwglobals, dwarf.DW_ABRV_VARIABLE, str, int(s.Version)) + lib := s.Lib + if lib == nil { + lib = ctxt.LibraryByPkg["runtime"] + } + dv := newdie(ctxt, ctxt.compUnitByPackage[lib].dwinfo, dwarf.DW_ABRV_VARIABLE, str, int(s.Version)) newabslocexprattr(dv, v, s) if s.Version == 0 { newattr(dv, dwarf.DW_AT_external, dwarf.DW_CLS_FLAG, 1, 0) @@ -910,10 +907,11 @@ func calcCompUnitRanges(ctxt *Link) { } } -func movetomodule(parent *dwarf.DWDie) { - die := dwroot.Child.Child +func movetomodule(ctxt *Link, parent *dwarf.DWDie) { + runtimelib := ctxt.LibraryByPkg["runtime"] + die := ctxt.compUnitByPackage[runtimelib].dwinfo.Child if die == nil { - dwroot.Child.Child = parent.Child + ctxt.compUnitByPackage[runtimelib].dwinfo.Child = parent.Child return } for die.Link != nil { @@ -1067,7 +1065,7 @@ func importInfoSymbol(ctxt *Link, dsym *sym.Symbol) { } } -func writelines(ctxt *Link, unit *compilationUnit, ls *sym.Symbol) (dwinfo *dwarf.DWDie) { +func writelines(ctxt *Link, unit *compilationUnit, ls *sym.Symbol) { var dwarfctxt dwarf.Context = dwctxt{ctxt} is_stmt := uint8(1) // initially = recommended default_is_stmt = 1, tracks is_stmt toggles. @@ -1076,29 +1074,7 @@ func writelines(ctxt *Link, unit *compilationUnit, ls *sym.Symbol) (dwinfo *dwar headerstart := int64(-1) headerend := int64(-1) - lang := dwarf.DW_LANG_Go - - dwinfo = newdie(ctxt, &dwroot, dwarf.DW_ABRV_COMPUNIT, unit.lib.Pkg, 0) - newattr(dwinfo, dwarf.DW_AT_language, dwarf.DW_CLS_CONSTANT, int64(lang), 0) - newattr(dwinfo, dwarf.DW_AT_stmt_list, dwarf.DW_CLS_PTR, ls.Size, ls) - // OS X linker requires compilation dir or absolute path in comp unit name to output debug info. - compDir := getCompilationDir() - // TODO: Make this be the actual compilation directory, not - // the linker directory. If we move CU construction into the - // compiler, this should happen naturally. - newattr(dwinfo, dwarf.DW_AT_comp_dir, dwarf.DW_CLS_STRING, int64(len(compDir)), compDir) - producerExtra := ctxt.Syms.Lookup(dwarf.CUInfoPrefix+"producer."+unit.lib.Pkg, 0) - producer := "Go cmd/compile " + objabi.Version - if len(producerExtra.P) > 0 { - // We put a semicolon before the flags to clearly - // separate them from the version, which can be long - // and have lots of weird things in it in development - // versions. We promise not to put a semicolon in the - // version, so it should be safe for readers to scan - // forward to the semicolon. - producer += "; " + string(producerExtra.P) - } - newattr(dwinfo, dwarf.DW_AT_producer, dwarf.DW_CLS_STRING, int64(len(producer)), producer) + newattr(unit.dwinfo, dwarf.DW_AT_stmt_list, dwarf.DW_CLS_PTR, ls.Size, ls) // Write .debug_line Line Number Program Header (sec 6.2.4) // Fields marked with (*) must be changed for 64-bit dwarf @@ -1300,8 +1276,6 @@ func writelines(ctxt *Link, unit *compilationUnit, ls *sym.Symbol) (dwinfo *dwar } } } - - return dwinfo } // writepcranges generates the DW_AT_ranges table for compilation unit cu. @@ -1468,15 +1442,13 @@ func writeinfo(ctxt *Link, syms []*sym.Symbol, units []*compilationUnit, abbrevs var dwarfctxt dwarf.Context = dwctxt{ctxt} - // Re-index per-package information by its CU die. - unitByDIE := make(map[*dwarf.DWDie]*compilationUnit) for _, u := range units { - unitByDIE[u.dwinfo] = u - } - - for compunit := dwroot.Child; compunit != nil; compunit = compunit.Link { + compunit := u.dwinfo s := dtolsym(compunit.Sym) - u := unitByDIE[compunit] + + if len(u.lib.Textp) == 0 && u.dwinfo.Child == nil { + continue + } // Write .debug_info Compilation Unit Header (sec 7.5.1) // Fields marked with (*) must be changed for 64-bit dwarf @@ -1536,7 +1508,11 @@ func writepub(ctxt *Link, sname string, ispub func(*dwarf.DWDie) bool, syms []*s s.Type = sym.SDWARFSECT syms = append(syms, s) - for compunit := dwroot.Child; compunit != nil; compunit = compunit.Link { + for _, u := range ctxt.compUnits { + if len(u.lib.Textp) == 0 && u.dwinfo.Child == nil { + continue + } + compunit := u.dwinfo sectionstart := s.Size culength := uint32(getattr(compunit, dwarf.DW_AT_byte_size).Value) + 4 @@ -1671,13 +1647,10 @@ func dwarfGenerateDebugInfo(ctxt *Link) { defgotype(ctxt, lookupOrDiag(ctxt, typ)) } - // Create DIEs for global variables and the types they use. - genasmsym(ctxt, defdwsymb) + // fake root DIE for compile unit DIEs + var dwroot dwarf.DWDie for _, lib := range ctxt.Library { - if len(lib.Textp) == 0 { - continue - } unit := &compilationUnit{lib: lib} if s := ctxt.Syms.ROLookup(dwarf.ConstInfoPrefix+lib.Pkg, 0); s != nil { importInfoSymbol(ctxt, s) @@ -1686,6 +1659,31 @@ func dwarfGenerateDebugInfo(ctxt *Link) { ctxt.compUnits = append(ctxt.compUnits, unit) ctxt.compUnitByPackage[lib] = unit + unit.dwinfo = newdie(ctxt, &dwroot, dwarf.DW_ABRV_COMPUNIT, unit.lib.Pkg, 0) + newattr(unit.dwinfo, dwarf.DW_AT_language, dwarf.DW_CLS_CONSTANT, int64(dwarf.DW_LANG_Go), 0) + // OS X linker requires compilation dir or absolute path in comp unit name to output debug info. + compDir := getCompilationDir() + // TODO: Make this be the actual compilation directory, not + // the linker directory. If we move CU construction into the + // compiler, this should happen naturally. + newattr(unit.dwinfo, dwarf.DW_AT_comp_dir, dwarf.DW_CLS_STRING, int64(len(compDir)), compDir) + producerExtra := ctxt.Syms.Lookup(dwarf.CUInfoPrefix+"producer."+unit.lib.Pkg, 0) + producer := "Go cmd/compile " + objabi.Version + if len(producerExtra.P) > 0 { + // We put a semicolon before the flags to clearly + // separate them from the version, which can be long + // and have lots of weird things in it in development + // versions. We promise not to put a semicolon in the + // version, so it should be safe for readers to scan + // forward to the semicolon. + producer += "; " + string(producerExtra.P) + } + newattr(unit.dwinfo, dwarf.DW_AT_producer, dwarf.DW_CLS_STRING, int64(len(producer)), producer) + + if len(lib.Textp) == 0 { + unit.dwinfo.Abbrev = dwarf.DW_ABRV_COMPUNIT_TEXTLESS + } + // Scan all functions in this compilation unit, create DIEs for all // referenced types, create the file table for debug_line, find all // referenced abstract functions. @@ -1726,6 +1724,9 @@ func dwarfGenerateDebugInfo(ctxt *Link) { } } + // Create DIEs for global variables and the types they use. + genasmsym(ctxt, defdwsymb) + synthesizestringtypes(ctxt, dwtypes.Child) synthesizeslicetypes(ctxt, dwtypes.Child) synthesizemaptypes(ctxt, dwtypes.Child) @@ -1758,19 +1759,19 @@ func dwarfGenerateDebugSyms(ctxt *Link) { debugRanges.Attr |= sym.AttrReachable syms = append(syms, debugLine) for _, u := range ctxt.compUnits { - u.dwinfo = writelines(ctxt, u, debugLine) + reversetree(&u.dwinfo.Child) + if u.dwinfo.Abbrev == dwarf.DW_ABRV_COMPUNIT_TEXTLESS { + continue + } + writelines(ctxt, u, debugLine) writepcranges(ctxt, u.dwinfo, u.lib.Textp[0], u.pcs, debugRanges) } // newdie adds DIEs to the *beginning* of the parent's DIE list. // Now that we're done creating DIEs, reverse the trees so DIEs // appear in the order they were created. - reversetree(&dwroot.Child) reversetree(&dwtypes.Child) - reversetree(&dwglobals.Child) - - movetomodule(&dwtypes) - movetomodule(&dwglobals) + movetomodule(ctxt, &dwtypes) // Need to reorder symbols so sym.SDWARFINFO is after all sym.SDWARFSECT // (but we need to generate dies before writepub) @@ -2005,5 +2006,14 @@ func (v compilationUnitByStartPC) Len() int { return len(v) } func (v compilationUnitByStartPC) Swap(i, j int) { v[i], v[j] = v[j], v[i] } func (v compilationUnitByStartPC) Less(i, j int) bool { - return v[i].lib.Textp[0].Value < v[j].lib.Textp[0].Value + switch { + case len(v[i].lib.Textp) == 0 && len(v[j].lib.Textp) == 0: + return v[i].lib.Pkg < v[j].lib.Pkg + case len(v[i].lib.Textp) != 0 && len(v[j].lib.Textp) == 0: + return true + case len(v[i].lib.Textp) == 0 && len(v[j].lib.Textp) != 0: + return false + default: + return v[i].lib.Textp[0].Value < v[j].lib.Textp[0].Value + } } diff --git a/src/cmd/link/internal/objfile/objfile.go b/src/cmd/link/internal/objfile/objfile.go index e3800de304..3a8923b073 100644 --- a/src/cmd/link/internal/objfile/objfile.go +++ b/src/cmd/link/internal/objfile/objfile.go @@ -203,6 +203,7 @@ func (r *objReader) readSym() { overwrite: s.File = pkg + s.Lib = r.lib if dupok { s.Attr |= sym.AttrDuplicateOK } @@ -320,7 +321,6 @@ overwrite: s.FuncInfo.IsStmtSym = r.syms.Lookup(dwarf.IsStmtPrefix+s.Name, int(s.Version)) - s.Lib = r.lib if !dupok { if s.Attr.OnList() { log.Fatalf("symbol %s listed multiple times", s.Name) -- cgit v1.3-5-g9baa From c3304edf10223364bc1989bc1daa6908dbdd9c03 Mon Sep 17 00:00:00 2001 From: Ben Shi Date: Sat, 29 Sep 2018 08:35:32 +0000 Subject: cmd/internal/obj/arm: delete unnecessary code In the arm assembler, "AMOVW" never falls into optab case 13, so the check "if p.As == AMOVW" is useless. Change-Id: Iec241d5b4cffb358a1477f470619dc9a6287884a Reviewed-on: https://go-review.googlesource.com/c/138575 Run-TryBot: Ben Shi TryBot-Result: Gobot Gobot Reviewed-by: Cherry Zhang --- src/cmd/internal/obj/arm/asm5.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/cmd/internal') diff --git a/src/cmd/internal/obj/arm/asm5.go b/src/cmd/internal/obj/arm/asm5.go index 3427ea9161..dd6d9265c4 100644 --- a/src/cmd/internal/obj/arm/asm5.go +++ b/src/cmd/internal/obj/arm/asm5.go @@ -2007,7 +2007,7 @@ func (c *ctxt5) asmout(p *obj.Prog, o *Optab, out []uint32) { o2 = c.oprrr(p, p.As, int(p.Scond)) o2 |= REGTMP & 15 r := int(p.Reg) - if p.As == AMOVW || p.As == AMVN { + if p.As == AMVN { r = 0 } else if r == 0 { r = int(p.To.Reg) -- cgit v1.3-5-g9baa From a3a69afff85c94be8b6419b03efd5dc14f2425f9 Mon Sep 17 00:00:00 2001 From: Clément Chigot Date: Fri, 28 Sep 2018 14:34:00 +0200 Subject: cmd/dist: add AIX operating system. This commit adds AIX operating system to cmd/dist package for ppc64 architecture. The stack guard is increased because of syscalls made inside the runtime which need a larger stack. Disable cmd/vet/all tests until aix/ppc64 is fully available. Change-Id: I7e3caf86724249ae564a152d90c1cbd4de288814 Reviewed-on: https://go-review.googlesource.com/c/138715 Run-TryBot: Ian Lance Taylor TryBot-Result: Gobot Gobot Reviewed-by: Ian Lance Taylor --- src/cmd/dist/build.go | 2 ++ src/cmd/dist/buildruntime.go | 4 ++++ src/cmd/dist/main.go | 3 +++ src/cmd/internal/objabi/head.go | 5 +++++ src/cmd/vet/all/main.go | 6 ++++++ 5 files changed, 20 insertions(+) (limited to 'src/cmd/internal') diff --git a/src/cmd/dist/build.go b/src/cmd/dist/build.go index b27d3aac4d..49f4a5e6a7 100644 --- a/src/cmd/dist/build.go +++ b/src/cmd/dist/build.go @@ -87,6 +87,7 @@ var okgoos = []string{ "openbsd", "plan9", "windows", + "aix", } // find reports the first index of p in l[0:n], or else -1. @@ -1388,6 +1389,7 @@ func checkNotStale(goBinary string, targets ...string) { // single point of truth for supported platforms. This list is used // by 'go tool dist list'. var cgoEnabled = map[string]bool{ + "aix/ppc64": false, "darwin/386": true, "darwin/amd64": true, "darwin/arm": true, diff --git a/src/cmd/dist/buildruntime.go b/src/cmd/dist/buildruntime.go index acf2230cb4..10d1552c94 100644 --- a/src/cmd/dist/buildruntime.go +++ b/src/cmd/dist/buildruntime.go @@ -87,6 +87,10 @@ func mkzbootstrap(file string) { // stack guard size. Larger multipliers are used for non-optimized // builds that have larger stack frames. func stackGuardMultiplier() int { + // On AIX, a larger stack is needed for syscalls + if goos == "aix" { + return 2 + } for _, s := range strings.Split(os.Getenv("GO_GCFLAGS"), " ") { if s == "-N" { return 2 diff --git a/src/cmd/dist/main.go b/src/cmd/dist/main.go index 37e37e2733..bf08869afb 100644 --- a/src/cmd/dist/main.go +++ b/src/cmd/dist/main.go @@ -81,6 +81,9 @@ func main() { } case "windows": exe = ".exe" + case "aix": + // uname -m doesn't work under AIX + gohostarch = "ppc64" } sysinit() diff --git a/src/cmd/internal/objabi/head.go b/src/cmd/internal/objabi/head.go index 23c7b62daf..db2221d6b1 100644 --- a/src/cmd/internal/objabi/head.go +++ b/src/cmd/internal/objabi/head.go @@ -48,10 +48,13 @@ const ( Hplan9 Hsolaris Hwindows + Haix ) func (h *HeadType) Set(s string) error { switch s { + case "aix": + *h = Haix case "darwin": *h = Hdarwin case "dragonfly": @@ -82,6 +85,8 @@ func (h *HeadType) Set(s string) error { func (h *HeadType) String() string { switch *h { + case Haix: + return "aix" case Hdarwin: return "darwin" case Hdragonfly: diff --git a/src/cmd/vet/all/main.go b/src/cmd/vet/all/main.go index 24dfafd7bf..7e4a68101f 100644 --- a/src/cmd/vet/all/main.go +++ b/src/cmd/vet/all/main.go @@ -204,6 +204,12 @@ func (p platform) vet() { return } + if p.os == "aix" && p.arch == "ppc64" { + // TODO(aix): enable as soon as the aix/ppc64 port has fully landed + fmt.Println("skipping aix/ppc64") + return + } + var buf bytes.Buffer fmt.Fprintf(&buf, "go run main.go -p %s\n", p) -- cgit v1.3-5-g9baa From cbafcc55e80d5b444e659a892b739c04a27980d3 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Sat, 1 Sep 2018 20:16:39 -0700 Subject: cmd/compile,runtime: implement stack objects Rework how the compiler+runtime handles stack-allocated variables whose address is taken. Direct references to such variables work as before. References through pointers, however, use a new mechanism. The new mechanism is more precise than the old "ambiguously live" mechanism. It computes liveness at runtime based on the actual references among objects on the stack. Each function records all of its address-taken objects in a FUNCDATA. These are called "stack objects". The runtime then uses that information while scanning a stack to find all of the stack objects on a stack. It then does a mark phase on the stack objects, using all the pointers found on the stack (and ancillary structures, like defer records) as the root set. Only stack objects which are found to be live during this mark phase will be scanned and thus retain any heap objects they point to. A subsequent CL will remove all the "ambiguously live" logic from the compiler, so that the stack object tracing will be required. For this CL, the stack tracing is all redundant with the current ambiguously live logic. Update #22350 Change-Id: Ide19f1f71a5b6ec8c4d54f8f66f0e9a98344772f Reviewed-on: https://go-review.googlesource.com/c/134155 Reviewed-by: Austin Clements --- src/cmd/compile/internal/gc/obj.go | 5 +- src/cmd/compile/internal/gc/pgen.go | 20 +++ src/cmd/compile/internal/gc/ssa.go | 48 +++++- src/cmd/internal/obj/link.go | 7 +- src/cmd/internal/objabi/funcdata.go | 1 + src/reflect/all_test.go | 3 +- src/runtime/funcdata.h | 1 + src/runtime/mbitmap.go | 21 ++- src/runtime/mgcmark.go | 127 ++++++++++++-- src/runtime/mgcstack.go | 330 ++++++++++++++++++++++++++++++++++++ src/runtime/mheap.go | 2 +- src/runtime/stack.go | 68 +++++++- src/runtime/symtab.go | 1 + 13 files changed, 607 insertions(+), 27 deletions(-) create mode 100644 src/runtime/mgcstack.go (limited to 'src/cmd/internal') diff --git a/src/cmd/compile/internal/gc/obj.go b/src/cmd/compile/internal/gc/obj.go index fb749d171f..19862c03aa 100644 --- a/src/cmd/compile/internal/gc/obj.go +++ b/src/cmd/compile/internal/gc/obj.go @@ -281,7 +281,7 @@ func dumpglobls() { funcsyms = nil } -// addGCLocals adds gcargs and gclocals symbols to Ctxt.Data. +// addGCLocals adds gcargs, gclocals, gcregs, and stack object symbols to Ctxt.Data. // It takes care not to add any duplicates. // Though the object file format handles duplicates efficiently, // storing only a single copy of the data, @@ -299,6 +299,9 @@ func addGCLocals() { Ctxt.Data = append(Ctxt.Data, gcsym) seen[gcsym.Name] = true } + if x := s.Func.StackObjects; x != nil { + ggloblsym(x, int32(len(x.P)), obj.RODATA|obj.LOCAL) + } } } diff --git a/src/cmd/compile/internal/gc/pgen.go b/src/cmd/compile/internal/gc/pgen.go index 563eb9e966..e6bbf04400 100644 --- a/src/cmd/compile/internal/gc/pgen.go +++ b/src/cmd/compile/internal/gc/pgen.go @@ -233,6 +233,26 @@ func compile(fn *Node) { // Set up the function's LSym early to avoid data races with the assemblers. fn.Func.initLSym() + // Make sure type syms are declared for all types that might + // be types of stack objects. We need to do this here + // because symbols must be allocated before the parallel + // phase of the compiler. + if fn.Func.lsym != nil { // not func _(){} + for _, n := range fn.Func.Dcl { + switch n.Class() { + case PPARAM, PPARAMOUT, PAUTO: + if livenessShouldTrack(n) && n.Addrtaken() { + dtypesym(n.Type) + // Also make sure we allocate a linker symbol + // for the stack object data, for the same reason. + if fn.Func.lsym.Func.StackObjects == nil { + fn.Func.lsym.Func.StackObjects = lookup(fmt.Sprintf("%s.stkobj", fn.funcname())).Linksym() + } + } + } + } + } + if compilenow() { compileSSA(fn, 0) } else { diff --git a/src/cmd/compile/internal/gc/ssa.go b/src/cmd/compile/internal/gc/ssa.go index eee3a71ba3..663042754e 100644 --- a/src/cmd/compile/internal/gc/ssa.go +++ b/src/cmd/compile/internal/gc/ssa.go @@ -16,6 +16,7 @@ import ( "cmd/compile/internal/ssa" "cmd/compile/internal/types" "cmd/internal/obj" + "cmd/internal/objabi" "cmd/internal/src" "cmd/internal/sys" ) @@ -4933,6 +4934,51 @@ func (s *SSAGenState) DebugFriendlySetPosFrom(v *ssa.Value) { } } +// byXoffset implements sort.Interface for []*Node using Xoffset as the ordering. +type byXoffset []*Node + +func (s byXoffset) Len() int { return len(s) } +func (s byXoffset) Less(i, j int) bool { return s[i].Xoffset < s[j].Xoffset } +func (s byXoffset) Swap(i, j int) { s[i], s[j] = s[j], s[i] } + +func emitStackObjects(e *ssafn, pp *Progs) { + var vars []*Node + for _, n := range e.curfn.Func.Dcl { + if livenessShouldTrack(n) && n.Addrtaken() { + vars = append(vars, n) + } + } + if len(vars) == 0 { + return + } + + // Sort variables from lowest to highest address. + sort.Sort(byXoffset(vars)) + + // Populate the stack object data. + // Format must match runtime/stack.go:stackObjectRecord. + x := e.curfn.Func.lsym.Func.StackObjects + off := 0 + off = duintptr(x, off, uint64(len(vars))) + for _, v := range vars { + // Note: arguments and return values have non-negative Xoffset, + // in which case the offset is relative to argp. + // Locals have a negative Xoffset, in which case the offset is relative to varp. + off = duintptr(x, off, uint64(v.Xoffset)) + if !typesym(v.Type).Siggen() { + Fatalf("stack object's type symbol not generated for type %s", v.Type) + } + off = dsymptr(x, off, dtypesym(v.Type), 0) + } + + // Emit a funcdata pointing at the stack object data. + p := pp.Prog(obj.AFUNCDATA) + Addrconst(&p.From, objabi.FUNCDATA_StackObjects) + p.To.Type = obj.TYPE_MEM + p.To.Name = obj.NAME_EXTERN + p.To.Sym = x +} + // genssa appends entries to pp for each instruction in f. func genssa(f *ssa.Func, pp *Progs) { var s SSAGenState @@ -4940,6 +4986,7 @@ func genssa(f *ssa.Func, pp *Progs) { e := f.Frontend().(*ssafn) s.livenessMap = liveness(e, f) + emitStackObjects(e, pp) // Remember where each block starts. s.bstart = make([]*obj.Prog, f.NumBlocks()) @@ -5054,7 +5101,6 @@ func genssa(f *ssa.Func, pp *Progs) { } } } - // Emit control flow instructions for block var next *ssa.Block if i < len(f.Blocks)-1 && Debug['N'] == 0 { diff --git a/src/cmd/internal/obj/link.go b/src/cmd/internal/obj/link.go index 132f7836ef..354bda5e48 100644 --- a/src/cmd/internal/obj/link.go +++ b/src/cmd/internal/obj/link.go @@ -402,9 +402,10 @@ type FuncInfo struct { dwarfAbsFnSym *LSym dwarfIsStmtSym *LSym - GCArgs LSym - GCLocals LSym - GCRegs LSym + GCArgs LSym + GCLocals LSym + GCRegs LSym + StackObjects *LSym } // Attribute is a set of symbol attributes. diff --git a/src/cmd/internal/objabi/funcdata.go b/src/cmd/internal/objabi/funcdata.go index a7827125bf..231d11b185 100644 --- a/src/cmd/internal/objabi/funcdata.go +++ b/src/cmd/internal/objabi/funcdata.go @@ -18,6 +18,7 @@ const ( FUNCDATA_LocalsPointerMaps = 1 FUNCDATA_InlTree = 2 FUNCDATA_RegPointerMaps = 3 + FUNCDATA_StackObjects = 4 // ArgsSizeUnknown is set in Func.argsize to mark all functions // whose argument size is unknown (C vararg functions, and diff --git a/src/reflect/all_test.go b/src/reflect/all_test.go index 5b8bbad383..c463b61c57 100644 --- a/src/reflect/all_test.go +++ b/src/reflect/all_test.go @@ -5988,7 +5988,8 @@ func TestFuncLayout(t *testing.T) { func verifyGCBits(t *testing.T, typ Type, bits []byte) { heapBits := GCBits(New(typ).Interface()) if !bytes.Equal(heapBits, bits) { - t.Errorf("heapBits incorrect for %v\nhave %v\nwant %v", typ, heapBits, bits) + _, _, line, _ := runtime.Caller(1) + t.Errorf("line %d: heapBits incorrect for %v\nhave %v\nwant %v", line, typ, heapBits, bits) } } diff --git a/src/runtime/funcdata.h b/src/runtime/funcdata.h index e6e0306e65..1ee67c8683 100644 --- a/src/runtime/funcdata.h +++ b/src/runtime/funcdata.h @@ -16,6 +16,7 @@ #define FUNCDATA_LocalsPointerMaps 1 #define FUNCDATA_InlTree 2 #define FUNCDATA_RegPointerMaps 3 +#define FUNCDATA_StackObjects 4 // Pseudo-assembly statements. diff --git a/src/runtime/mbitmap.go b/src/runtime/mbitmap.go index 553d83f7f2..5301e692e0 100644 --- a/src/runtime/mbitmap.go +++ b/src/runtime/mbitmap.go @@ -1911,6 +1911,20 @@ Run: return totalBits } +// materializeGCProg allocates space for the (1-bit) pointer bitmask +// for an object of size ptrdata. Then it fills that space with the +// pointer bitmask specified by the program prog. +// The bitmask starts at s.startAddr. +// The result must be deallocated with dematerializeGCProg. +func materializeGCProg(ptrdata uintptr, prog *byte) *mspan { + s := mheap_.allocManual((ptrdata/(8*sys.PtrSize)+pageSize-1)/pageSize, &memstats.gc_sys) + runGCProg(addb(prog, 4), nil, (*byte)(unsafe.Pointer(s.startAddr)), 1) + return s +} +func dematerializeGCProg(s *mspan) { + mheap_.freeManual(s, &memstats.gc_sys) +} + func dumpGCProg(p *byte) { nptr := 0 for { @@ -2037,7 +2051,12 @@ func getgcmask(ep interface{}) (mask []byte) { _g_ := getg() gentraceback(_g_.m.curg.sched.pc, _g_.m.curg.sched.sp, 0, _g_.m.curg, 0, nil, 1000, getgcmaskcb, noescape(unsafe.Pointer(&frame)), 0) if frame.fn.valid() { - locals, _ := getStackMap(&frame, nil, false) + // TODO: once stack objects are enabled (and their pointers + // are no longer described by the stack pointermap directly), + // tests using this will probably need fixing. We might need + // to loop through the stackobjects and if we're inside one, + // use the pointermap from that object. + locals, _, _ := getStackMap(&frame, nil, false) if locals.n == 0 { return } diff --git a/src/runtime/mgcmark.go b/src/runtime/mgcmark.go index 34e9776d27..d4dcfb6cb9 100644 --- a/src/runtime/mgcmark.go +++ b/src/runtime/mgcmark.go @@ -169,7 +169,7 @@ func markroot(gcw *gcWork, i uint32) { case i == fixedRootFinalizers: for fb := allfin; fb != nil; fb = fb.alllink { cnt := uintptr(atomic.Load(&fb.cnt)) - scanblock(uintptr(unsafe.Pointer(&fb.fin[0])), cnt*unsafe.Sizeof(fb.fin[0]), &finptrmask[0], gcw) + scanblock(uintptr(unsafe.Pointer(&fb.fin[0])), cnt*unsafe.Sizeof(fb.fin[0]), &finptrmask[0], gcw, nil) } case i == fixedRootFreeGStacks: @@ -248,7 +248,7 @@ func markrootBlock(b0, n0 uintptr, ptrmask0 *uint8, gcw *gcWork, shard int) { } // Scan this shard. - scanblock(b, n, ptrmask, gcw) + scanblock(b, n, ptrmask, gcw, nil) } // markrootFreeGStacks frees stacks of dead Gs. @@ -349,7 +349,7 @@ func markrootSpans(gcw *gcWork, shard int) { scanobject(p, gcw) // The special itself is a root. - scanblock(uintptr(unsafe.Pointer(&spf.fn)), sys.PtrSize, &oneptrmask[0], gcw) + scanblock(uintptr(unsafe.Pointer(&spf.fn)), sys.PtrSize, &oneptrmask[0], gcw, nil) } unlock(&s.speciallock) @@ -689,42 +689,136 @@ func scanstack(gp *g, gcw *gcWork) { // Shrink the stack if not much of it is being used. shrinkstack(gp) + var state stackScanState + state.stack = gp.stack + + if stackTraceDebug { + println("stack trace goroutine", gp.goid) + } + // Scan the saved context register. This is effectively a live // register that gets moved back and forth between the // register and sched.ctxt without a write barrier. if gp.sched.ctxt != nil { - scanblock(uintptr(unsafe.Pointer(&gp.sched.ctxt)), sys.PtrSize, &oneptrmask[0], gcw) + scanblock(uintptr(unsafe.Pointer(&gp.sched.ctxt)), sys.PtrSize, &oneptrmask[0], gcw, &state) } - // Scan the stack. - var cache pcvalueCache + // Scan the stack. Accumulate a list of stack objects. scanframe := func(frame *stkframe, unused unsafe.Pointer) bool { - scanframeworker(frame, &cache, gcw) + scanframeworker(frame, &state, gcw) return true } gentraceback(^uintptr(0), ^uintptr(0), 0, gp, 0, nil, 0x7fffffff, scanframe, nil, 0) tracebackdefers(gp, scanframe, nil) + + // Find and scan all reachable stack objects. + state.buildIndex() + for { + p := state.getPtr() + if p == 0 { + break + } + obj := state.findObject(p) + if obj == nil { + continue + } + t := obj.typ + if t == nil { + // We've already scanned this object. + continue + } + obj.setType(nil) // Don't scan it again. + if stackTraceDebug { + println(" live stkobj at", hex(state.stack.lo+uintptr(obj.off)), "of type", t.string()) + } + gcdata := t.gcdata + var s *mspan + if t.kind&kindGCProg != 0 { + // This path is pretty unlikely, an object large enough + // to have a GC program allocated on the stack. + // We need some space to unpack the program into a straight + // bitmask, which we allocate/free here. + // TODO: it would be nice if there were a way to run a GC + // program without having to store all its bits. We'd have + // to change from a Lempel-Ziv style program to something else. + // Or we can forbid putting objects on stacks if they require + // a gc program (see issue 27447). + s = materializeGCProg(t.ptrdata, gcdata) + gcdata = (*byte)(unsafe.Pointer(s.startAddr)) + } + + scanblock(state.stack.lo+uintptr(obj.off), t.ptrdata, gcdata, gcw, &state) + + if s != nil { + dematerializeGCProg(s) + } + } + + // Deallocate object buffers. + // (Pointer buffers were all deallocated in the loop above.) + for state.head != nil { + x := state.head + state.head = x.next + if stackTraceDebug { + for _, obj := range x.obj[:x.nobj] { + if obj.typ == nil { // reachable + continue + } + println(" dead stkobj at", hex(gp.stack.lo+uintptr(obj.off)), "of type", obj.typ.string()) + // Note: not necessarily really dead - only reachable-from-ptr dead. + } + } + x.nobj = 0 + putempty((*workbuf)(unsafe.Pointer(x))) + } + if state.buf != nil || state.freeBuf != nil { + throw("remaining pointer buffers") + } + gp.gcscanvalid = true } // Scan a stack frame: local variables and function arguments/results. //go:nowritebarrier -func scanframeworker(frame *stkframe, cache *pcvalueCache, gcw *gcWork) { +func scanframeworker(frame *stkframe, state *stackScanState, gcw *gcWork) { if _DebugGC > 1 && frame.continpc != 0 { print("scanframe ", funcname(frame.fn), "\n") } - locals, args := getStackMap(frame, cache, false) + locals, args, objs := getStackMap(frame, &state.cache, false) // Scan local variables if stack frame has been allocated. if locals.n > 0 { size := uintptr(locals.n) * sys.PtrSize - scanblock(frame.varp-size, size, locals.bytedata, gcw) + scanblock(frame.varp-size, size, locals.bytedata, gcw, state) } // Scan arguments. if args.n > 0 { - scanblock(frame.argp, uintptr(args.n)*sys.PtrSize, args.bytedata, gcw) + scanblock(frame.argp, uintptr(args.n)*sys.PtrSize, args.bytedata, gcw, state) + } + + // Add all stack objects to the stack object list. + if frame.varp != 0 { + // varp is 0 for defers, where there are no locals. + // In that case, there can't be a pointer to its args, either. + // (And all args would be scanned above anyway.) + for _, obj := range objs { + off := obj.off + base := frame.varp // locals base pointer + if off >= 0 { + base = frame.argp // arguments and return values base pointer + } + ptr := base + uintptr(off) + if ptr < frame.sp { + // object hasn't been allocated in the frame yet. + continue + } + if stackTraceDebug { + println("stkobj at", hex(ptr), "of type", obj.typ.string()) + } + state.addObject(ptr, obj.typ) + } } } @@ -939,8 +1033,9 @@ func gcDrainN(gcw *gcWork, scanWork int64) int64 { // This is used to scan non-heap roots, so it does not update // gcw.bytesMarked or gcw.scanWork. // +// If stk != nil, possible stack pointers are also reported to stk.putPtr. //go:nowritebarrier -func scanblock(b0, n0 uintptr, ptrmask *uint8, gcw *gcWork) { +func scanblock(b0, n0 uintptr, ptrmask *uint8, gcw *gcWork, stk *stackScanState) { // Use local copies of original parameters, so that a stack trace // due to one of the throws below shows the original block // base and extent. @@ -957,10 +1052,12 @@ func scanblock(b0, n0 uintptr, ptrmask *uint8, gcw *gcWork) { for j := 0; j < 8 && i < n; j++ { if bits&1 != 0 { // Same work as in scanobject; see comments there. - obj := *(*uintptr)(unsafe.Pointer(b + i)) - if obj != 0 { - if obj, span, objIndex := findObject(obj, b, i); obj != 0 { + p := *(*uintptr)(unsafe.Pointer(b + i)) + if p != 0 { + if obj, span, objIndex := findObject(p, b, i); obj != 0 { greyobject(obj, b, i, span, gcw, objIndex) + } else if stk != nil && p >= stk.stack.lo && p < stk.stack.hi { + stk.putPtr(p) } } } diff --git a/src/runtime/mgcstack.go b/src/runtime/mgcstack.go new file mode 100644 index 0000000000..86e60d4381 --- /dev/null +++ b/src/runtime/mgcstack.go @@ -0,0 +1,330 @@ +// Copyright 2018 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Garbage collector: stack objects and stack tracing +// See the design doc at https://docs.google.com/document/d/1un-Jn47yByHL7I0aVIP_uVCMxjdM5mpelJhiKlIqxkE/edit?usp=sharing +// Also see issue 22350. + +// Stack tracing solves the problem of determining which parts of the +// stack are live and should be scanned. It runs as part of scanning +// a single goroutine stack. +// +// Normally determining which parts of the stack are live is easy to +// do statically, as user code has explicit references (reads and +// writes) to stack variables. The compiler can do a simple dataflow +// analysis to determine liveness of stack variables at every point in +// the code. See cmd/compile/internal/gc/plive.go for that analysis. +// +// However, when we take the address of a stack variable, determining +// whether that variable is still live is less clear. We can still +// look for static accesses, but accesses through a pointer to the +// variable are difficult in general to track statically. That pointer +// can be passed among functions on the stack, conditionally retained, +// etc. +// +// Instead, we will track pointers to stack variables dynamically. +// All pointers to stack-allocated variables will themselves be on the +// stack somewhere (or in associated locations, like defer records), so +// we can find them all efficiently. +// +// Stack tracing is organized as a mini garbage collection tracing +// pass. The objects in this garbage collection are all the variables +// on the stack whose address is taken, and which themselves contain a +// pointer. We call these variables "stack objects". +// +// We begin by determining all the stack objects on the stack and all +// the statically live pointers that may point into the stack. We then +// process each pointer to see if it points to a stack object. If it +// does, we scan that stack object. It may contain pointers into the +// heap, in which case those pointers are passed to the main garbage +// collection. It may also contain pointers into the stack, in which +// case we add them to our set of stack pointers. +// +// Once we're done processing all the pointers (including the ones we +// added during processing), we've found all the stack objects that +// are live. Any dead stack objects are not scanned and their contents +// will not keep heap objects live. Unlike the main garbage +// collection, we can't sweep the dead stack objects; they live on in +// a moribund state until the stack frame that contains them is +// popped. +// +// A stack can look like this: +// +// +----------+ +// | foo() | +// | +------+ | +// | | A | | <---\ +// | +------+ | | +// | | | +// | +------+ | | +// | | B | | | +// | +------+ | | +// | | | +// +----------+ | +// | bar() | | +// | +------+ | | +// | | C | | <-\ | +// | +----|-+ | | | +// | | | | | +// | +----v-+ | | | +// | | D ---------/ +// | +------+ | | +// | | | +// +----------+ | +// | baz() | | +// | +------+ | | +// | | E -------/ +// | +------+ | +// | ^ | +// | F: --/ | +// | | +// +----------+ +// +// foo() calls bar() calls baz(). Each has a frame on the stack. +// foo() has stack objects A and B. +// bar() has stack objects C and D, with C pointing to D and D pointing to A. +// baz() has a stack object E pointing to C, and a local variable F pointing to E. +// +// Starting from the pointer in local variable F, we will eventually +// scan all of E, C, D, and A (in that order). B is never scanned +// because there is no live pointer to it. If B is also statically +// dead (meaning that foo() never accesses B again after it calls +// bar()), then B's pointers into the heap are not considered live. + +package runtime + +import ( + "runtime/internal/sys" + "unsafe" +) + +const stackTraceDebug = false + +// Buffer for pointers found during stack tracing. +// Must be smaller than or equal to workbuf. +// +//go:notinheap +type stackWorkBuf struct { + stackWorkBufHdr + obj [(_WorkbufSize - unsafe.Sizeof(stackWorkBufHdr{})) / sys.PtrSize]uintptr +} + +// Header declaration must come after the buf declaration above, because of issue #14620. +// +//go:notinheap +type stackWorkBufHdr struct { + workbufhdr + next *stackWorkBuf // linked list of workbufs + // Note: we could theoretically repurpose lfnode.next as this next pointer. + // It would save 1 word, but that probably isn't worth busting open + // the lfnode API. +} + +// Buffer for stack objects found on a goroutine stack. +// Must be smaller than or equal to workbuf. +// +//go:notinheap +type stackObjectBuf struct { + stackObjectBufHdr + obj [(_WorkbufSize - unsafe.Sizeof(stackObjectBufHdr{})) / unsafe.Sizeof(stackObject{})]stackObject +} + +//go:notinheap +type stackObjectBufHdr struct { + workbufhdr + next *stackObjectBuf +} + +func init() { + if unsafe.Sizeof(stackWorkBuf{}) > unsafe.Sizeof(workbuf{}) { + panic("stackWorkBuf too big") + } + if unsafe.Sizeof(stackObjectBuf{}) > unsafe.Sizeof(workbuf{}) { + panic("stackObjectBuf too big") + } +} + +// A stackObject represents a variable on the stack that has had +// its address taken. +// +//go:notinheap +type stackObject struct { + off uint32 // offset above stack.lo + size uint32 // size of object + typ *_type // type info (for ptr/nonptr bits). nil if object has been scanned. + left *stackObject // objects with lower addresses + right *stackObject // objects with higher addresses +} + +// obj.typ = typ, but with no write barrier. +//go:nowritebarrier +func (obj *stackObject) setType(typ *_type) { + // Types of stack objects are always in read-only memory, not the heap. + // So not using a write barrier is ok. + *(*uintptr)(unsafe.Pointer(&obj.typ)) = uintptr(unsafe.Pointer(typ)) +} + +// A stackScanState keeps track of the state used during the GC walk +// of a goroutine. +// +//go:notinheap +type stackScanState struct { + cache pcvalueCache + + // stack limits + stack stack + + // buf contains the set of possible pointers to stack objects. + // Organized as a LIFO linked list of buffers. + // All buffers except possibly the head buffer are full. + buf *stackWorkBuf + freeBuf *stackWorkBuf // keep around one free buffer for allocation hysteresis + + // list of stack objects + // Objects are in increasing address order. + head *stackObjectBuf + tail *stackObjectBuf + nobjs int + + // root of binary tree for fast object lookup by address + // Initialized by buildIndex. + root *stackObject +} + +// Add p as a potential pointer to a stack object. +// p must be a stack address. +func (s *stackScanState) putPtr(p uintptr) { + if p < s.stack.lo || p >= s.stack.hi { + throw("address not a stack address") + } + buf := s.buf + if buf == nil { + // Initial setup. + buf = (*stackWorkBuf)(unsafe.Pointer(getempty())) + buf.nobj = 0 + buf.next = nil + s.buf = buf + } else if buf.nobj == len(buf.obj) { + if s.freeBuf != nil { + buf = s.freeBuf + s.freeBuf = nil + } else { + buf = (*stackWorkBuf)(unsafe.Pointer(getempty())) + } + buf.nobj = 0 + buf.next = s.buf + s.buf = buf + } + buf.obj[buf.nobj] = p + buf.nobj++ +} + +// Remove and return a potential pointer to a stack object. +// Returns 0 if there are no more pointers available. +func (s *stackScanState) getPtr() uintptr { + buf := s.buf + if buf == nil { + // Never had any data. + return 0 + } + if buf.nobj == 0 { + if s.freeBuf != nil { + // Free old freeBuf. + putempty((*workbuf)(unsafe.Pointer(s.freeBuf))) + } + // Move buf to the freeBuf. + s.freeBuf = buf + buf = buf.next + s.buf = buf + if buf == nil { + // No more data. + putempty((*workbuf)(unsafe.Pointer(s.freeBuf))) + s.freeBuf = nil + return 0 + } + } + buf.nobj-- + return buf.obj[buf.nobj] +} + +// addObject adds a stack object at addr of type typ to the set of stack objects. +func (s *stackScanState) addObject(addr uintptr, typ *_type) { + x := s.tail + if x == nil { + // initial setup + x = (*stackObjectBuf)(unsafe.Pointer(getempty())) + x.next = nil + s.head = x + s.tail = x + } + if x.nobj > 0 && uint32(addr-s.stack.lo) < x.obj[x.nobj-1].off+x.obj[x.nobj-1].size { + throw("objects added out of order or overlapping") + } + if x.nobj == len(x.obj) { + // full buffer - allocate a new buffer, add to end of linked list + y := (*stackObjectBuf)(unsafe.Pointer(getempty())) + y.next = nil + x.next = y + s.tail = y + x = y + } + obj := &x.obj[x.nobj] + x.nobj++ + obj.off = uint32(addr - s.stack.lo) + obj.size = uint32(typ.size) + obj.setType(typ) + // obj.left and obj.right will be initalized by buildIndex before use. + s.nobjs++ +} + +// buildIndex initializes s.root to a binary search tree. +// It should be called after all addObject calls but before +// any call of findObject. +func (s *stackScanState) buildIndex() { + s.root, _, _ = binarySearchTree(s.head, 0, s.nobjs) +} + +// Build a binary search tree with the n objects in the list +// x.obj[idx], x.obj[idx+1], ..., x.next.obj[0], ... +// Returns the root of that tree, and the buf+idx of the nth object after x.obj[idx]. +// (The first object that was not included in the binary search tree.) +// If n == 0, returns nil, x. +func binarySearchTree(x *stackObjectBuf, idx int, n int) (root *stackObject, restBuf *stackObjectBuf, restIdx int) { + if n == 0 { + return nil, x, idx + } + var left, right *stackObject + left, x, idx = binarySearchTree(x, idx, n/2) + root = &x.obj[idx] + idx++ + if idx == len(x.obj) { + x = x.next + idx = 0 + } + right, x, idx = binarySearchTree(x, idx, n-n/2-1) + root.left = left + root.right = right + return root, x, idx +} + +// findObject returns the stack object containing address a, if any. +// Must have called buildIndex previously. +func (s *stackScanState) findObject(a uintptr) *stackObject { + off := uint32(a - s.stack.lo) + obj := s.root + for { + if obj == nil { + return nil + } + if off < obj.off { + obj = obj.left + continue + } + if off >= obj.off+obj.size { + obj = obj.right + continue + } + return obj + } +} diff --git a/src/runtime/mheap.go b/src/runtime/mheap.go index e29af677a2..7a11bdc058 100644 --- a/src/runtime/mheap.go +++ b/src/runtime/mheap.go @@ -1437,7 +1437,7 @@ func addfinalizer(p unsafe.Pointer, f *funcval, nret uintptr, fint *_type, ot *p scanobject(base, gcw) // Mark the finalizer itself, since the // special isn't part of the GC'd heap. - scanblock(uintptr(unsafe.Pointer(&s.fn)), sys.PtrSize, &oneptrmask[0], gcw) + scanblock(uintptr(unsafe.Pointer(&s.fn)), sys.PtrSize, &oneptrmask[0], gcw, nil) releasem(mp) } return true diff --git a/src/runtime/stack.go b/src/runtime/stack.go index 582e94e9d0..b815aa859e 100644 --- a/src/runtime/stack.go +++ b/src/runtime/stack.go @@ -625,7 +625,7 @@ func adjustframe(frame *stkframe, arg unsafe.Pointer) bool { return true } - locals, args := getStackMap(frame, &adjinfo.cache, true) + locals, args, objs := getStackMap(frame, &adjinfo.cache, true) // Adjust local variables if stack frame has been allocated. if locals.n > 0 { @@ -663,6 +663,42 @@ func adjustframe(frame *stkframe, arg unsafe.Pointer) bool { } adjustpointers(unsafe.Pointer(frame.argp), &args, adjinfo, funcInfo{}) } + + // Adjust pointers in all stack objects (whether they are live or not). + // See comments in mgcmark.go:scanframeworker. + if frame.varp != 0 { + for _, obj := range objs { + off := obj.off + base := frame.varp // locals base pointer + if off >= 0 { + base = frame.argp // arguments and return values base pointer + } + p := base + uintptr(off) + if p < frame.sp { + // Object hasn't been allocated in the frame yet. + // (Happens when the stack bounds check fails and + // we call into morestack.) + continue + } + t := obj.typ + gcdata := t.gcdata + var s *mspan + if t.kind&kindGCProg != 0 { + // See comments in mgcmark.go:scanstack + s = materializeGCProg(t.ptrdata, gcdata) + gcdata = (*byte)(unsafe.Pointer(s.startAddr)) + } + for i := uintptr(0); i < t.ptrdata; i += sys.PtrSize { + if *addb(gcdata, i/(8*sys.PtrSize))>>(i/sys.PtrSize&7)&1 != 0 { + adjustpointer(adjinfo, unsafe.Pointer(p+i)) + } + } + if s != nil { + dematerializeGCProg(s) + } + } + } + return true } @@ -1136,9 +1172,9 @@ func freeStackSpans() { unlock(&stackLarge.lock) } -// getStackMap returns the locals and arguments live pointer maps for -// frame. -func getStackMap(frame *stkframe, cache *pcvalueCache, debug bool) (locals, args bitvector) { +// getStackMap returns the locals and arguments live pointer maps, and +// stack object list for frame. +func getStackMap(frame *stkframe, cache *pcvalueCache, debug bool) (locals, args bitvector, objs []stackObjectRecord) { targetpc := frame.continpc if targetpc == 0 { // Frame is dead. Return empty bitvectors. @@ -1235,9 +1271,33 @@ func getStackMap(frame *stkframe, cache *pcvalueCache, debug bool) (locals, args } } } + + // stack objects. + p := funcdata(f, _FUNCDATA_StackObjects) + if p != nil { + n := *(*uintptr)(p) + p = add(p, sys.PtrSize) + *(*slice)(unsafe.Pointer(&objs)) = slice{array: noescape(p), len: int(n), cap: int(n)} + // Note: the noescape above is needed to keep + // getStackMap from from "leaking param content: + // frame". That leak propagates up to getgcmask, then + // GCMask, then verifyGCInfo, which converts the stack + // gcinfo tests into heap gcinfo tests :( + } + return } +// A stackObjectRecord is generated by the compiler for each stack object in a stack frame. +// This record must match the generator code in cmd/compile/internal/gc/ssa.go:emitStackObjects. +type stackObjectRecord struct { + // offset in frame + // if negative, offset from varp + // if non-negative, offset from argp + off int + typ *_type +} + //go:nosplit func morestackc() { throw("attempt to execute system stack code on user stack") diff --git a/src/runtime/symtab.go b/src/runtime/symtab.go index d90ab86ffa..452e9d06ae 100644 --- a/src/runtime/symtab.go +++ b/src/runtime/symtab.go @@ -348,6 +348,7 @@ const ( _FUNCDATA_LocalsPointerMaps = 1 _FUNCDATA_InlTree = 2 _FUNCDATA_RegPointerMaps = 3 + _FUNCDATA_StackObjects = 4 _ArgsSizeUnknown = -0x80000000 ) -- cgit v1.3-5-g9baa From 9dac0a8132d7db5225b27bdd8faeb3158e624159 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Tue, 11 Sep 2018 15:14:28 -0700 Subject: runtime: on a signal, set traceback address to a deferreturn call When a function triggers a signal (like a segfault which translates to a nil pointer exception) during execution, a sigpanic handler is just below it on the stack. The function itself did not stop at a safepoint, so we have to figure out what safepoint we should use to scan its stack frame. Previously we used the site of the most recent defer to get the live variables at the signal site. That answer is not quite correct, as explained in #27518. Instead, use the site of a deferreturn call. It has all the right variables marked as live (no args, all the return values, except those that escape to the heap, in which case the corresponding PAUTOHEAP variables will be live instead). This CL requires stack objects, so that all the local variables and args referenced by the deferred closures keep the right variables alive. Fixes #27518 Change-Id: Id45d8a8666759986c203181090b962e2981e48ca Reviewed-on: https://go-review.googlesource.com/c/134637 Reviewed-by: Austin Clements Reviewed-by: Cherry Zhang --- src/cmd/internal/objabi/funcid.go | 2 +- src/cmd/link/internal/ld/pcln.go | 97 +++++++++++++++++++++++---------------- src/runtime/runtime2.go | 8 ++-- src/runtime/symtab.go | 4 +- src/runtime/traceback.go | 8 ++-- test/fixedbugs/issue27518a.go | 45 ++++++++++++++++++ test/fixedbugs/issue27518b.go | 72 +++++++++++++++++++++++++++++ 7 files changed, 188 insertions(+), 48 deletions(-) create mode 100644 test/fixedbugs/issue27518a.go create mode 100644 test/fixedbugs/issue27518b.go (limited to 'src/cmd/internal') diff --git a/src/cmd/internal/objabi/funcid.go b/src/cmd/internal/objabi/funcid.go index 15a63ab8b3..92799107da 100644 --- a/src/cmd/internal/objabi/funcid.go +++ b/src/cmd/internal/objabi/funcid.go @@ -9,7 +9,7 @@ package objabi // Note that in some situations involving plugins, there may be multiple // copies of a particular special runtime function. // Note: this list must match the list in runtime/symtab.go. -type FuncID uint32 +type FuncID uint8 const ( FuncID_normal FuncID = iota // not a special function diff --git a/src/cmd/link/internal/ld/pcln.go b/src/cmd/link/internal/ld/pcln.go index 7b7f7068e7..24398fcc87 100644 --- a/src/cmd/link/internal/ld/pcln.go +++ b/src/cmd/link/internal/ld/pcln.go @@ -312,45 +312,19 @@ func (ctxt *Link) pclntab() { } off = int32(ftab.SetUint32(ctxt.Arch, int64(off), args)) - // funcID uint32 - funcID := objabi.FuncID_normal - switch s.Name { - case "runtime.main": - funcID = objabi.FuncID_runtime_main - case "runtime.goexit": - funcID = objabi.FuncID_goexit - case "runtime.jmpdefer": - funcID = objabi.FuncID_jmpdefer - case "runtime.mcall": - funcID = objabi.FuncID_mcall - case "runtime.morestack": - funcID = objabi.FuncID_morestack - case "runtime.mstart": - funcID = objabi.FuncID_mstart - case "runtime.rt0_go": - funcID = objabi.FuncID_rt0_go - case "runtime.asmcgocall": - funcID = objabi.FuncID_asmcgocall - case "runtime.sigpanic": - funcID = objabi.FuncID_sigpanic - case "runtime.runfinq": - funcID = objabi.FuncID_runfinq - case "runtime.gcBgMarkWorker": - funcID = objabi.FuncID_gcBgMarkWorker - case "runtime.systemstack_switch": - funcID = objabi.FuncID_systemstack_switch - case "runtime.systemstack": - funcID = objabi.FuncID_systemstack - case "runtime.cgocallback_gofunc": - funcID = objabi.FuncID_cgocallback_gofunc - case "runtime.gogo": - funcID = objabi.FuncID_gogo - case "runtime.externalthreadhandler": - funcID = objabi.FuncID_externalthreadhandler - case "runtime.debugCallV1": - funcID = objabi.FuncID_debugCallV1 + // deferreturn + deferreturn := uint32(0) + for _, r := range s.R { + if r.Sym != nil && r.Sym.Name == "runtime.deferreturn" && r.Add == 0 { + // Note: the relocation target is in the call instruction, but + // is not necessarily the whole instruction (for instance, on + // x86 the relocation applies to bytes [1:5] of the 5 byte call + // instruction). + deferreturn = uint32(r.Off) + break // only need one + } } - off = int32(ftab.SetUint32(ctxt.Arch, int64(off), uint32(funcID))) + off = int32(ftab.SetUint32(ctxt.Arch, int64(off), deferreturn)) if pcln != &pclntabZpcln { renumberfiles(ctxt, pcln.File, &pcln.Pcfile) @@ -396,7 +370,52 @@ func (ctxt *Link) pclntab() { off = addpctab(ctxt, ftab, off, &pcln.Pcfile) off = addpctab(ctxt, ftab, off, &pcln.Pcline) off = int32(ftab.SetUint32(ctxt.Arch, int64(off), uint32(len(pcln.Pcdata)))) - off = int32(ftab.SetUint32(ctxt.Arch, int64(off), uint32(len(pcln.Funcdata)))) + + // funcID uint8 + funcID := objabi.FuncID_normal + switch s.Name { + case "runtime.main": + funcID = objabi.FuncID_runtime_main + case "runtime.goexit": + funcID = objabi.FuncID_goexit + case "runtime.jmpdefer": + funcID = objabi.FuncID_jmpdefer + case "runtime.mcall": + funcID = objabi.FuncID_mcall + case "runtime.morestack": + funcID = objabi.FuncID_morestack + case "runtime.mstart": + funcID = objabi.FuncID_mstart + case "runtime.rt0_go": + funcID = objabi.FuncID_rt0_go + case "runtime.asmcgocall": + funcID = objabi.FuncID_asmcgocall + case "runtime.sigpanic": + funcID = objabi.FuncID_sigpanic + case "runtime.runfinq": + funcID = objabi.FuncID_runfinq + case "runtime.gcBgMarkWorker": + funcID = objabi.FuncID_gcBgMarkWorker + case "runtime.systemstack_switch": + funcID = objabi.FuncID_systemstack_switch + case "runtime.systemstack": + funcID = objabi.FuncID_systemstack + case "runtime.cgocallback_gofunc": + funcID = objabi.FuncID_cgocallback_gofunc + case "runtime.gogo": + funcID = objabi.FuncID_gogo + case "runtime.externalthreadhandler": + funcID = objabi.FuncID_externalthreadhandler + case "runtime.debugCallV1": + funcID = objabi.FuncID_debugCallV1 + } + off = int32(ftab.SetUint8(ctxt.Arch, int64(off), uint8(funcID))) + + // unused + off += 2 + + // nfuncdata must be the final entry. + off = int32(ftab.SetUint8(ctxt.Arch, int64(off), uint8(len(pcln.Funcdata)))) for i := range pcln.Pcdata { off = addpctab(ctxt, ftab, off, &pcln.Pcdata[i]) } diff --git a/src/runtime/runtime2.go b/src/runtime/runtime2.go index 2f009abdbb..bbb66bb8fa 100644 --- a/src/runtime/runtime2.go +++ b/src/runtime/runtime2.go @@ -650,14 +650,16 @@ type _func struct { entry uintptr // start pc nameoff int32 // function name - args int32 // in/out args size - funcID funcID // set for certain special runtime functions + args int32 // in/out args size + deferreturn uint32 // offset of a deferreturn block from entry, if any. pcsp int32 pcfile int32 pcln int32 npcdata int32 - nfuncdata int32 + funcID funcID // set for certain special runtime functions + _ [2]int8 // unused + nfuncdata uint8 // must be last } // layout of Itab known to compilers diff --git a/src/runtime/symtab.go b/src/runtime/symtab.go index 452e9d06ae..1dc7ab740e 100644 --- a/src/runtime/symtab.go +++ b/src/runtime/symtab.go @@ -357,7 +357,7 @@ const ( // Note that in some situations involving plugins, there may be multiple // copies of a particular special runtime function. // Note: this list must match the list in cmd/internal/objabi/funcid.go. -type funcID uint32 +type funcID uint8 const ( funcID_normal funcID = iota // not a special function @@ -856,7 +856,7 @@ func pcdatavalue(f funcInfo, table int32, targetpc uintptr, cache *pcvalueCache) return pcvalue(f, off, targetpc, cache, true) } -func funcdata(f funcInfo, i int32) unsafe.Pointer { +func funcdata(f funcInfo, i uint8) unsafe.Pointer { if i < 0 || i >= f.nfuncdata { return nil } diff --git a/src/runtime/traceback.go b/src/runtime/traceback.go index 69d5764c8f..d7265b2bb9 100644 --- a/src/runtime/traceback.go +++ b/src/runtime/traceback.go @@ -312,8 +312,7 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in // the function either doesn't return at all (if it has no defers or if the // defers do not recover) or it returns from one of the calls to // deferproc a second time (if the corresponding deferred func recovers). - // It suffices to assume that the most recent deferproc is the one that - // returns; everything live at earlier deferprocs is still live at that one. + // In the latter case, use a deferreturn call site as the continuation pc. frame.continpc = frame.pc if waspanic { // We match up defers with frames using the SP. @@ -324,7 +323,10 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in // can't push a defer, the defer can't belong // to that frame. if _defer != nil && _defer.sp == frame.sp && frame.sp != frame.fp { - frame.continpc = _defer.pc + frame.continpc = frame.fn.entry + uintptr(frame.fn.deferreturn) + 1 + // Note: the +1 is to offset the -1 that + // stack.go:getStackMap does to back up a return + // address make sure the pc is in the CALL instruction. } else { frame.continpc = 0 } diff --git a/test/fixedbugs/issue27518a.go b/test/fixedbugs/issue27518a.go new file mode 100644 index 0000000000..d6224df017 --- /dev/null +++ b/test/fixedbugs/issue27518a.go @@ -0,0 +1,45 @@ +// run + +// Copyright 2018 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import ( + "runtime" +) + +var nilp *int +var forceHeap interface{} + +func main() { + // x is a pointer on the stack to heap-allocated memory. + x := new([32]*int) + forceHeap = x + forceHeap = nil + + // Push a defer to be run when we panic below. + defer func() { + // Ignore the panic. + recover() + // Force a stack walk. Go 1.11 will fail because x is now + // considered live again. + runtime.GC() + }() + // Make x live at the defer's PC. + runtime.KeepAlive(x) + + // x is no longer live. Garbage collect the [32]*int on the + // heap. + runtime.GC() + // At this point x's dead stack slot points to dead memory. + + // Trigger a sigpanic. Since this is an implicit panic, we + // don't have an explicit liveness map here. + // Traceback used to use the liveness map of the most recent defer, + // but in that liveness map, x will be live again even though + // it points to dead memory. The fix is to use the liveness + // map of a deferreturn call instead. + *nilp = 0 +} diff --git a/test/fixedbugs/issue27518b.go b/test/fixedbugs/issue27518b.go new file mode 100644 index 0000000000..ea72a30885 --- /dev/null +++ b/test/fixedbugs/issue27518b.go @@ -0,0 +1,72 @@ +// run + +// Copyright 2018 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import "runtime" + +var finalized bool +var err string + +type HeapObj [8]int64 + +const filler int64 = 0x123456789abcdef0 + +func (h *HeapObj) init() { + for i := 0; i < len(*h); i++ { + h[i] = filler + } +} +func (h *HeapObj) check() { + for i := 0; i < len(*h); i++ { + if h[i] != filler { + err = "filler overwritten" + } + } +} + +type StackObj struct { + h *HeapObj +} + +func gc(shouldFinalize bool) { + runtime.GC() + runtime.GC() + runtime.GC() + if shouldFinalize != finalized { + err = "heap object finalized at the wrong time" + } +} + +func main() { + var s StackObj + s.h = new(HeapObj) + s.h.init() + runtime.SetFinalizer(s.h, func(h *HeapObj) { + finalized = true + }) + gc(false) + h := g(&s) + gc(false) + h.check() + gc(true) // finalize here, after return value's last use. (Go1.11 never runs the finalizer.) + if err != "" { + panic(err) + } +} + +func g(p *StackObj) (v *HeapObj) { + gc(false) + v = p.h // last use of the stack object. the only reference to the heap object is in the return slot. + gc(false) + defer func() { + gc(false) + recover() + gc(false) + }() + *(*int)(nil) = 0 + return +} -- cgit v1.3-5-g9baa