From 9abbe277105f9b9a3b4c5905e7faf9a02827ce18 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Sat, 19 Dec 2020 13:46:49 -0800 Subject: [PATCH 01/11] test: skip issue11656.go on mips/mips64/ppc64 For #11656 For #43283 Change-Id: I1fcf2b24800f421e36201af43130b487abe605b1 Reviewed-on: https://go-review.googlesource.com/c/go/+/279312 Trust: Ian Lance Taylor Run-TryBot: Ian Lance Taylor Run-TryBot: Emmanuel Odeke Reviewed-by: Emmanuel Odeke --- test/fixedbugs/issue11656.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/fixedbugs/issue11656.go b/test/fixedbugs/issue11656.go index 62b36cf790f..50182633645 100644 --- a/test/fixedbugs/issue11656.go +++ b/test/fixedbugs/issue11656.go @@ -27,6 +27,13 @@ import ( ) func main() { + // This test is currently failing on some architectures. + // See issue #43283. + switch runtime.GOARCH { + case "ppc64", "mips", "mipsle", "mips64", "mips64le": + return + } + debug.SetPanicOnFault(true) defer func() { if err := recover(); err == nil { From 53c984d976f49b5671b11ff17f5d622572d4cf58 Mon Sep 17 00:00:00 2001 From: Richard Miller Date: Sat, 5 Dec 2020 19:53:08 +0000 Subject: [PATCH 02/11] runtime: skip wakep call in wakeNetPoller on Plan 9 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This was part of a performance improvement made by CL 232298 to reduce timer latency. On multiprocessor Plan 9 machines, it triggers memory faults often enough that the builder test suite never completes successfully. See issue #42303 for discussion. As shown by the benchmark result below, worst case latency on plan9_arm is very bad even with the wakep call in place - in the tickers-per-P=1 case, a 3ms timer is 270ms late. Skipping the wakep call and running the benchmark again shows some cases worse, some better. The performance cost doesn't seem excessive for this temporary workaround which makes the plan9_arm builders usable again. With wakep call: cpu% go test -bench Latency time goos: plan9 goarch: arm pkg: time BenchmarkParallelTimerLatency-4 100 10985859 avg-late-ns 18630963 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=1-4 195 270294688 avg-late-ns 542057670 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=2-4 234 182452000 avg-late-ns 423933688 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=3-4 280 193003004 avg-late-ns 408034405 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=4-4 282 132819086 avg-late-ns 313624570 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=5-4 339 71152187 avg-late-ns 189014519 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=6-4 315 26860484 avg-late-ns 101759844 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=7-4 357 19106739 avg-late-ns 59435620 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=8-4 376 7246933 avg-late-ns 38888461 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=9-4 267 40476892 avg-late-ns 205851926 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=10-4 294 87836303 avg-late-ns 252059695 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=2ms/tickers-per-P=1-4 379 4127144 avg-late-ns 10494927 max-late-ns Without wakep call: BenchmarkParallelTimerLatency-4 61 10775151 avg-late-ns 18668517 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=1-4 199 299587535 avg-late-ns 597182307 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=2-4 272 184561831 avg-late-ns 449739837 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=3-4 235 154983257 avg-late-ns 370940553 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=4-4 290 150034689 avg-late-ns 332399843 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=5-4 298 47540764 avg-late-ns 133709031 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=6-4 350 20379394 avg-late-ns 81742809 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=7-4 363 14403223 avg-late-ns 98901212 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=8-4 375 12293090 avg-late-ns 50266552 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=9-4 336 40628820 avg-late-ns 150946099 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=10-4 289 88265539 avg-late-ns 280770418 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=2ms/tickers-per-P=1-4 375 8364937 avg-late-ns 22598421 max-late-ns Fixes #42303 Change-Id: I70c63cb2a2bad46950a7cd9dfc7bb32943710d32 Reviewed-on: https://go-review.googlesource.com/c/go/+/275672 Reviewed-by: David du Colombier <0intro@gmail.com> Trust: Michael Pratt --- src/runtime/proc.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/runtime/proc.go b/src/runtime/proc.go index 64e102fb0aa..418e06932e6 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -2882,7 +2882,9 @@ func wakeNetPoller(when int64) { } else { // There are no threads in the network poller, try to get // one there so it can handle new timers. - wakep() + if GOOS != "plan9" { // Temporary workaround - see issue #42303. + wakep() + } } } From cb95819cf6e969dc7dcc64ec7820d3995379c9f4 Mon Sep 17 00:00:00 2001 From: Michael Pratt Date: Fri, 11 Dec 2020 14:14:30 -0500 Subject: [PATCH 03/11] runtime: detect netbsd netpoll overrun in sysmon The netbsd kernel has a bug [1] that occassionally prevents netpoll from waking with netpollBreak, which could result in missing timers for an unbounded amount of time, as netpoll can't restart with a shorter delay when an earlier timer is added. Prior to CL 232298, sysmon could detect these overrun timers and manually start an M to run them. With this fallback gone, the bug actually prevents timer execution indefinitely. As a workaround, we add back sysmon detection only for netbsd. [1] https://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=50094 Updates #42515 Change-Id: I8391f5b9dabef03dd1d94c50b3b4b3bd4f889e66 Reviewed-on: https://go-review.googlesource.com/c/go/+/277332 Run-TryBot: Michael Pratt TryBot-Result: Go Bot Reviewed-by: Michael Knyszek Reviewed-by: Austin Clements Trust: Michael Pratt --- src/runtime/proc.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/runtime/proc.go b/src/runtime/proc.go index 418e06932e6..5adcbf07dcb 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -5130,6 +5130,26 @@ func sysmon() { } } mDoFixup() + if GOOS == "netbsd" { + // netpoll is responsible for waiting for timer + // expiration, so we typically don't have to worry + // about starting an M to service timers. (Note that + // sleep for timeSleepUntil above simply ensures sysmon + // starts running again when that timer expiration may + // cause Go code to run again). + // + // However, netbsd has a kernel bug that sometimes + // misses netpollBreak wake-ups, which can lead to + // unbounded delays servicing timers. If we detect this + // overrun, then startm to get something to handle the + // timer. + // + // See issue 42515 and + // https://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=50094. + if next, _ := timeSleepUntil(); next < now { + startm(nil, false) + } + } if atomic.Load(&scavenge.sysmonWake) != 0 { // Kick the scavenger awake if someone requested it. wakeScavenger() From 8438a5779b76620237d608282a99d17467b91f4c Mon Sep 17 00:00:00 2001 From: Cherry Zhang Date: Wed, 11 Nov 2020 21:27:56 -0500 Subject: [PATCH 04/11] runtime: use _exit on darwin On darwin, where we use libc for syscalls, when the runtime exits, it calls libc exit function, which may call back into user code, e.g. invoking functions registered with atexit. In particular, it may call back into Go. But at this point, the Go runtime is already exiting, so this wouldn't work. On non-libc platforms we use exit syscall directly, which doesn't invoke any callbacks. Use _exit on darwin to achieve the same behavior. No test for now, as it doesn't pass on all platforms (see trybot run of PS2). May fix #42465. May fix #43294. Change-Id: Ia1ada22b5da8cb64fdd598d0541eb90e195367eb Reviewed-on: https://go-review.googlesource.com/c/go/+/269378 Trust: Cherry Zhang Run-TryBot: Cherry Zhang TryBot-Result: Go Bot Reviewed-by: Ian Lance Taylor --- src/runtime/sys_darwin.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/runtime/sys_darwin.go b/src/runtime/sys_darwin.go index c89ce780124..55845bf2e51 100644 --- a/src/runtime/sys_darwin.go +++ b/src/runtime/sys_darwin.go @@ -467,7 +467,7 @@ func setNonblock(fd int32) { //go:cgo_import_dynamic libc_pthread_create pthread_create "/usr/lib/libSystem.B.dylib" //go:cgo_import_dynamic libc_pthread_self pthread_self "/usr/lib/libSystem.B.dylib" //go:cgo_import_dynamic libc_pthread_kill pthread_kill "/usr/lib/libSystem.B.dylib" -//go:cgo_import_dynamic libc_exit exit "/usr/lib/libSystem.B.dylib" +//go:cgo_import_dynamic libc_exit _exit "/usr/lib/libSystem.B.dylib" //go:cgo_import_dynamic libc_raise raise "/usr/lib/libSystem.B.dylib" //go:cgo_import_dynamic libc_open open "/usr/lib/libSystem.B.dylib" From 6cff874c47bdb4567f5c84bc59d93311493caefe Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Mon, 7 Dec 2020 15:11:46 +0000 Subject: [PATCH 05/11] runtime/metrics: add Read examples This change adds two examples of using the Read function: one that reads one metric and one that reads all metrics. Change-Id: I4940a44c9b1d65f3f7a1554e3145ff07e6492fc1 Reviewed-on: https://go-review.googlesource.com/c/go/+/275855 Run-TryBot: Michael Knyszek TryBot-Result: Go Bot Reviewed-by: Michael Pratt Trust: Michael Knyszek --- src/runtime/metrics/example_test.go | 96 +++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100644 src/runtime/metrics/example_test.go diff --git a/src/runtime/metrics/example_test.go b/src/runtime/metrics/example_test.go new file mode 100644 index 00000000000..cade0c38bfd --- /dev/null +++ b/src/runtime/metrics/example_test.go @@ -0,0 +1,96 @@ +// Copyright 2020 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 metrics_test + +import ( + "fmt" + "runtime/metrics" +) + +func ExampleRead_readingOneMetric() { + // Name of the metric we want to read. + const myMetric = "/memory/classes/heap/free:bytes" + + // Create a sample for the metric. + sample := make([]metrics.Sample, 1) + sample[0].Name = myMetric + + // Sample the metric. + metrics.Read(sample) + + // Check if the metric is actually supported. + // If it's not, the resulting value will always have + // kind KindBad. + if sample[0].Value.Kind() == metrics.KindBad { + panic(fmt.Sprintf("metric %q no longer supported", myMetric)) + } + + // Handle the result. + // + // It's OK to assume a particular Kind for a metric; + // they're guaranteed not to change. + freeBytes := sample[0].Value.Uint64() + + fmt.Printf("free but not released memory: %d\n", freeBytes) +} + +func ExampleRead_readingAllMetrics() { + // Get descriptions for all supported metrics. + descs := metrics.All() + + // Create a sample for each metric. + samples := make([]metrics.Sample, len(descs)) + for i := range samples { + samples[i].Name = descs[i].Name + } + + // Sample the metrics. Re-use the samples slice if you can! + metrics.Read(samples) + + // Iterate over all results. + for _, sample := range samples { + // Pull out the name and value. + name, value := sample.Name, sample.Value + + // Handle each sample. + switch value.Kind() { + case metrics.KindUint64: + fmt.Printf("%s: %d\n", name, value.Uint64()) + case metrics.KindFloat64: + fmt.Printf("%s: %f\n", name, value.Float64()) + case metrics.KindFloat64Histogram: + // The histogram may be quite large, so let's just pull out + // a crude estimate for the median for the sake of this example. + fmt.Printf("%s: %f\n", name, medianBucket(value.Float64Histogram())) + case metrics.KindBad: + // This should never happen because all metrics are supported + // by construction. + panic("bug in runtime/metrics package!") + default: + // This may happen as new metrics get added. + // + // The safest thing to do here is to simply log it somewhere + // as something to look into, but ignore it for now. + // In the worst case, you might temporarily miss out on a new metric. + fmt.Printf("%s: unexpected metric Kind: %v\n", name, value.Kind()) + } + } +} + +func medianBucket(h *metrics.Float64Histogram) float64 { + total := uint64(0) + for _, count := range h.Counts { + total += count + } + thresh := total / 2 + total = 0 + for i, count := range h.Counts { + total += count + if total > thresh { + return h.Buckets[i] + } + } + panic("should not happen") +} From bc7e4d9257693413d57ad467814ab71f1585a155 Mon Sep 17 00:00:00 2001 From: Cherry Zhang Date: Mon, 21 Dec 2020 13:14:41 -0500 Subject: [PATCH 06/11] syscall: don't generate ptrace on iOS May fix #43302. Change-Id: I6b7ddf94495c4fa80cf8a50a38eef5f8b2872669 Reviewed-on: https://go-review.googlesource.com/c/go/+/279481 Trust: Cherry Zhang Reviewed-by: Ian Lance Taylor --- src/syscall/mksyscall.pl | 2 +- src/syscall/ptrace_darwin.go | 14 ++++++++++++++ src/syscall/ptrace_ios.go | 12 ++++++++++++ src/syscall/syscall_darwin_amd64.go | 2 +- src/syscall/syscall_darwin_arm64.go | 2 +- src/syscall/zsyscall_darwin_amd64.go | 2 +- src/syscall/zsyscall_darwin_arm64.go | 2 +- 7 files changed, 31 insertions(+), 5 deletions(-) create mode 100644 src/syscall/ptrace_darwin.go create mode 100644 src/syscall/ptrace_ios.go diff --git a/src/syscall/mksyscall.pl b/src/syscall/mksyscall.pl index 25b40d7ba23..7e2cedfb6c1 100755 --- a/src/syscall/mksyscall.pl +++ b/src/syscall/mksyscall.pl @@ -125,7 +125,7 @@ while(<>) { # without reading the header. $text .= "// THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT\n\n"; - if ($darwin && $func eq "ptrace") { + if ($darwin && $func eq "ptrace1") { # The ptrace function is called from forkAndExecInChild where stack # growth is forbidden. $text .= "//go:nosplit\n" diff --git a/src/syscall/ptrace_darwin.go b/src/syscall/ptrace_darwin.go new file mode 100644 index 00000000000..a873d826b8a --- /dev/null +++ b/src/syscall/ptrace_darwin.go @@ -0,0 +1,14 @@ +// Copyright 2020 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. + +// +build !ios + +package syscall + +// Nosplit because it is called from forkAndExecInChild. +// +//go:nosplit +func ptrace(request int, pid int, addr uintptr, data uintptr) error { + return ptrace1(request, pid, addr, data) +} diff --git a/src/syscall/ptrace_ios.go b/src/syscall/ptrace_ios.go new file mode 100644 index 00000000000..2f61a88a089 --- /dev/null +++ b/src/syscall/ptrace_ios.go @@ -0,0 +1,12 @@ +// Copyright 2020 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 syscall + +// Nosplit because it is called from forkAndExecInChild. +// +//go:nosplit +func ptrace(request int, pid int, addr uintptr, data uintptr) (err error) { + panic("unimplemented") +} diff --git a/src/syscall/syscall_darwin_amd64.go b/src/syscall/syscall_darwin_amd64.go index 23a4e5f9962..22ddb78ae5b 100644 --- a/src/syscall/syscall_darwin_amd64.go +++ b/src/syscall/syscall_darwin_amd64.go @@ -21,7 +21,7 @@ func setTimeval(sec, usec int64) Timeval { //sys Stat(path string, stat *Stat_t) (err error) = SYS_stat64 //sys Statfs(path string, stat *Statfs_t) (err error) = SYS_statfs64 //sys fstatat(fd int, path string, stat *Stat_t, flags int) (err error) = SYS_fstatat64 -//sys ptrace(request int, pid int, addr uintptr, data uintptr) (err error) +//sys ptrace1(request int, pid int, addr uintptr, data uintptr) (err error) = SYS_ptrace func SetKevent(k *Kevent_t, fd, mode, flags int) { k.Ident = uint64(fd) diff --git a/src/syscall/syscall_darwin_arm64.go b/src/syscall/syscall_darwin_arm64.go index c824f6d89d8..ecb9ffff492 100644 --- a/src/syscall/syscall_darwin_arm64.go +++ b/src/syscall/syscall_darwin_arm64.go @@ -21,7 +21,7 @@ func setTimeval(sec, usec int64) Timeval { //sys Stat(path string, stat *Stat_t) (err error) //sys Statfs(path string, stat *Statfs_t) (err error) //sys fstatat(fd int, path string, stat *Stat_t, flags int) (err error) -//sys ptrace(request int, pid int, addr uintptr, data uintptr) (err error) +//sys ptrace1(request int, pid int, addr uintptr, data uintptr) (err error) = SYS_ptrace func SetKevent(k *Kevent_t, fd, mode, flags int) { k.Ident = uint64(fd) diff --git a/src/syscall/zsyscall_darwin_amd64.go b/src/syscall/zsyscall_darwin_amd64.go index 093739ebc77..c246c3a2675 100644 --- a/src/syscall/zsyscall_darwin_amd64.go +++ b/src/syscall/zsyscall_darwin_amd64.go @@ -2091,7 +2091,7 @@ func libc_fstatat64_trampoline() // THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT //go:nosplit -func ptrace(request int, pid int, addr uintptr, data uintptr) (err error) { +func ptrace1(request int, pid int, addr uintptr, data uintptr) (err error) { _, _, e1 := syscall6(funcPC(libc_ptrace_trampoline), uintptr(request), uintptr(pid), uintptr(addr), uintptr(data), 0, 0) if e1 != 0 { err = errnoErr(e1) diff --git a/src/syscall/zsyscall_darwin_arm64.go b/src/syscall/zsyscall_darwin_arm64.go index 7698b2503e8..ede0091de24 100644 --- a/src/syscall/zsyscall_darwin_arm64.go +++ b/src/syscall/zsyscall_darwin_arm64.go @@ -2091,7 +2091,7 @@ func libc_fstatat_trampoline() // THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT //go:nosplit -func ptrace(request int, pid int, addr uintptr, data uintptr) (err error) { +func ptrace1(request int, pid int, addr uintptr, data uintptr) (err error) { _, _, e1 := syscall6(funcPC(libc_ptrace_trampoline), uintptr(request), uintptr(pid), uintptr(addr), uintptr(data), 0, 0) if e1 != 0 { err = errnoErr(e1) From 9b6147120a30a8bc30a41c1651f369e8bcb80948 Mon Sep 17 00:00:00 2001 From: Cherry Zhang Date: Mon, 21 Dec 2020 14:11:02 -0500 Subject: [PATCH 07/11] cmd/pack: treat compiler's -linkobj output as "compiler object" Treat the compiler's -linkobj output as "compiler object, which means "pack c" will "see through" the file and add individual entry to the new archive, instead of the object as a whole. This is somewhat peculiar. But Go 1.15's cmd/pack does this, although seemingly accidental. We just do the same. FWIW, it does make things more consistent with/without -linkobj flag. Fixes #43271. Change-Id: I6b2d99256db7ebf0fa430f85afa7464e334f6bcb Reviewed-on: https://go-review.googlesource.com/c/go/+/279483 Trust: Cherry Zhang Run-TryBot: Cherry Zhang Reviewed-by: Jeremy Faller Reviewed-by: Than McIntosh --- src/cmd/pack/pack.go | 31 ++++++++++-------- src/cmd/pack/pack_test.go | 66 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 13 deletions(-) diff --git a/src/cmd/pack/pack.go b/src/cmd/pack/pack.go index 82546ea7dcb..3dffabe5ec8 100644 --- a/src/cmd/pack/pack.go +++ b/src/cmd/pack/pack.go @@ -315,20 +315,25 @@ func (ar *Archive) extractContents1(e *archive.Entry, out io.Writer) { } // isGoCompilerObjFile reports whether file is an object file created -// by the Go compiler, which is an archive file with exactly two entries: -// __.PKGDEF and _go_.o. +// by the Go compiler, which is an archive file with exactly one entry +// of __.PKGDEF, or _go_.o, or both entries. func isGoCompilerObjFile(a *archive.Archive) bool { - if len(a.Entries) != 2 { + switch len(a.Entries) { + case 1: + return (a.Entries[0].Type == archive.EntryGoObj && a.Entries[0].Name == "_go_.o") || + (a.Entries[0].Type == archive.EntryPkgDef && a.Entries[0].Name == "__.PKGDEF") + case 2: + var foundPkgDef, foundGo bool + for _, e := range a.Entries { + if e.Type == archive.EntryPkgDef && e.Name == "__.PKGDEF" { + foundPkgDef = true + } + if e.Type == archive.EntryGoObj && e.Name == "_go_.o" { + foundGo = true + } + } + return foundPkgDef && foundGo + default: return false } - var foundPkgDef, foundGo bool - for _, e := range a.Entries { - if e.Type == archive.EntryPkgDef && e.Name == "__.PKGDEF" { - foundPkgDef = true - } - if e.Type == archive.EntryGoObj && e.Name == "_go_.o" { - foundGo = true - } - } - return foundPkgDef && foundGo } diff --git a/src/cmd/pack/pack_test.go b/src/cmd/pack/pack_test.go index 218c7acda6d..16a51358009 100644 --- a/src/cmd/pack/pack_test.go +++ b/src/cmd/pack/pack_test.go @@ -302,6 +302,72 @@ func TestIssue21703(t *testing.T) { run(goBin, "tool", "compile", "-I", ".", "b.go") } +// Test the "c" command can "see through" the archive generated by the compiler. +// This is peculiar. (See issue ) +func TestCreateWithCompilerObj(t *testing.T) { + testenv.MustHaveGoBuild(t) + + dir := tmpDir(t) + defer os.RemoveAll(dir) + src := filepath.Join(dir, "p.go") + prog := "package p; var X = 42\n" + err := os.WriteFile(src, []byte(prog), 0666) + if err != nil { + t.Fatal(err) + } + + run := func(args ...string) string { + return doRun(t, dir, args...) + } + + goBin := testenv.GoToolPath(t) + run(goBin, "build", "cmd/pack") // writes pack binary to dir + run(goBin, "tool", "compile", "-pack", "-o", "p.a", "p.go") + run("./pack", "c", "packed.a", "p.a") + fi, err := os.Stat(filepath.Join(dir, "p.a")) + if err != nil { + t.Fatalf("stat p.a failed: %v", err) + } + fi2, err := os.Stat(filepath.Join(dir, "packed.a")) + if err != nil { + t.Fatalf("stat packed.a failed: %v", err) + } + // For compiler-generated object file, the "c" command is + // expected to get (essentially) the same file back, instead + // of packing it into a new archive with a single entry. + if want, got := fi.Size(), fi2.Size(); want != got { + t.Errorf("packed file with different size: want %d, got %d", want, got) + } + + // Test -linkobj flag as well. + run(goBin, "tool", "compile", "-linkobj", "p2.a", "-o", "p.x", "p.go") + run("./pack", "c", "packed2.a", "p2.a") + fi, err = os.Stat(filepath.Join(dir, "p2.a")) + if err != nil { + t.Fatalf("stat p2.a failed: %v", err) + } + fi2, err = os.Stat(filepath.Join(dir, "packed2.a")) + if err != nil { + t.Fatalf("stat packed2.a failed: %v", err) + } + if want, got := fi.Size(), fi2.Size(); want != got { + t.Errorf("packed file with different size: want %d, got %d", want, got) + } + + run("./pack", "c", "packed3.a", "p.x") + fi, err = os.Stat(filepath.Join(dir, "p.x")) + if err != nil { + t.Fatalf("stat p.x failed: %v", err) + } + fi2, err = os.Stat(filepath.Join(dir, "packed3.a")) + if err != nil { + t.Fatalf("stat packed3.a failed: %v", err) + } + if want, got := fi.Size(), fi2.Size(); want != got { + t.Errorf("packed file with different size: want %d, got %d", want, got) + } +} + // doRun runs a program in a directory and returns the output. func doRun(t *testing.T, dir string, args ...string) string { cmd := exec.Command(args[0], args[1:]...) From 4d27c4c223ccb7de3876abbac79b58ad9579be1a Mon Sep 17 00:00:00 2001 From: Nikhil Benesch Date: Wed, 9 Dec 2020 15:14:59 -0500 Subject: [PATCH 08/11] runtime: correct error handling in several FreeBSD syscall wrappers The FreeBSD syscall convention uses the carry flag to indicate whether an error has occured. The sys_umtx_op, thr_new, and pipe2 syscall wrappers were failing to account for this convention and silently suppressing errors as a result. This commit corrects these wrappers by copying the pattern used by the other fallible syscall wrappers. Note that futexsleep1 must now explicitly ignore the ETIMEDOUT error from sys_umtx_op. Previously ETIMEDOUT was implicitly ignored because sys_umtx_op never returned an error. Fixes #43106. Change-Id: I9c422b87cf4c6d308003bf42c3b419f785578b5d Reviewed-on: https://go-review.googlesource.com/c/go/+/276892 Run-TryBot: Ian Lance Taylor Reviewed-by: Austin Clements Trust: Than McIntosh --- src/runtime/defs_freebsd_386.go | 9 +++++---- src/runtime/defs_freebsd_amd64.go | 9 +++++---- src/runtime/defs_freebsd_arm.go | 9 +++++---- src/runtime/defs_freebsd_arm64.go | 9 +++++---- src/runtime/os_freebsd.go | 3 +-- src/runtime/sys_freebsd_386.s | 6 ++++++ src/runtime/sys_freebsd_amd64.s | 6 ++++++ src/runtime/sys_freebsd_arm.s | 3 +++ src/runtime/sys_freebsd_arm64.s | 6 ++++++ 9 files changed, 42 insertions(+), 18 deletions(-) diff --git a/src/runtime/defs_freebsd_386.go b/src/runtime/defs_freebsd_386.go index 767755425c1..f822934d582 100644 --- a/src/runtime/defs_freebsd_386.go +++ b/src/runtime/defs_freebsd_386.go @@ -13,10 +13,11 @@ const ( ) const ( - _EINTR = 0x4 - _EFAULT = 0xe - _EAGAIN = 0x23 - _ENOSYS = 0x4e + _EINTR = 0x4 + _EFAULT = 0xe + _EAGAIN = 0x23 + _ENOSYS = 0x4e + _ETIMEDOUT = 0x3c _O_NONBLOCK = 0x4 _O_CLOEXEC = 0x100000 diff --git a/src/runtime/defs_freebsd_amd64.go b/src/runtime/defs_freebsd_amd64.go index 5a833426fd6..0b696cf2270 100644 --- a/src/runtime/defs_freebsd_amd64.go +++ b/src/runtime/defs_freebsd_amd64.go @@ -13,10 +13,11 @@ const ( ) const ( - _EINTR = 0x4 - _EFAULT = 0xe - _EAGAIN = 0x23 - _ENOSYS = 0x4e + _EINTR = 0x4 + _EFAULT = 0xe + _EAGAIN = 0x23 + _ENOSYS = 0x4e + _ETIMEDOUT = 0x3c _O_NONBLOCK = 0x4 _O_CLOEXEC = 0x100000 diff --git a/src/runtime/defs_freebsd_arm.go b/src/runtime/defs_freebsd_arm.go index b55dfd88cf2..b6f3e790cff 100644 --- a/src/runtime/defs_freebsd_arm.go +++ b/src/runtime/defs_freebsd_arm.go @@ -13,10 +13,11 @@ const ( ) const ( - _EINTR = 0x4 - _EFAULT = 0xe - _EAGAIN = 0x23 - _ENOSYS = 0x4e + _EINTR = 0x4 + _EFAULT = 0xe + _EAGAIN = 0x23 + _ENOSYS = 0x4e + _ETIMEDOUT = 0x3c _O_NONBLOCK = 0x4 _O_CLOEXEC = 0x100000 diff --git a/src/runtime/defs_freebsd_arm64.go b/src/runtime/defs_freebsd_arm64.go index 5b9d504ba6f..0759a1238f9 100644 --- a/src/runtime/defs_freebsd_arm64.go +++ b/src/runtime/defs_freebsd_arm64.go @@ -13,10 +13,11 @@ const ( ) const ( - _EINTR = 0x4 - _EFAULT = 0xe - _EAGAIN = 0x23 - _ENOSYS = 0x4e + _EINTR = 0x4 + _EFAULT = 0xe + _EAGAIN = 0x23 + _ENOSYS = 0x4e + _ETIMEDOUT = 0x3c _O_NONBLOCK = 0x4 _O_CLOEXEC = 0x100000 diff --git a/src/runtime/os_freebsd.go b/src/runtime/os_freebsd.go index 730973a2029..1c60ee2a57f 100644 --- a/src/runtime/os_freebsd.go +++ b/src/runtime/os_freebsd.go @@ -166,7 +166,7 @@ func futexsleep1(addr *uint32, val uint32, ns int64) { utp = &ut } ret := sys_umtx_op(addr, _UMTX_OP_WAIT_UINT_PRIVATE, val, unsafe.Sizeof(*utp), utp) - if ret >= 0 || ret == -_EINTR { + if ret >= 0 || ret == -_EINTR || ret == -_ETIMEDOUT { return } print("umtx_wait addr=", addr, " val=", val, " ret=", ret, "\n") @@ -208,7 +208,6 @@ func newosproc(mp *m) { var oset sigset sigprocmask(_SIG_SETMASK, &sigset_all, &oset) - // TODO: Check for error. ret := thr_new(¶m, int32(unsafe.Sizeof(param))) sigprocmask(_SIG_SETMASK, &oset, nil) if ret < 0 { diff --git a/src/runtime/sys_freebsd_386.s b/src/runtime/sys_freebsd_386.s index c346e719e14..97e6d9ab366 100644 --- a/src/runtime/sys_freebsd_386.s +++ b/src/runtime/sys_freebsd_386.s @@ -13,12 +13,16 @@ TEXT runtime·sys_umtx_op(SB),NOSPLIT,$-4 MOVL $454, AX INT $0x80 + JAE 2(PC) + NEGL AX MOVL AX, ret+20(FP) RET TEXT runtime·thr_new(SB),NOSPLIT,$-4 MOVL $455, AX INT $0x80 + JAE 2(PC) + NEGL AX MOVL AX, ret+8(FP) RET @@ -120,6 +124,8 @@ TEXT runtime·pipe2(SB),NOSPLIT,$12-16 MOVL flags+0(FP), BX MOVL BX, 8(SP) INT $0x80 + JAE 2(PC) + NEGL AX MOVL AX, errno+12(FP) RET diff --git a/src/runtime/sys_freebsd_amd64.s b/src/runtime/sys_freebsd_amd64.s index 010b2ec4d4d..07734b0d7d7 100644 --- a/src/runtime/sys_freebsd_amd64.s +++ b/src/runtime/sys_freebsd_amd64.s @@ -18,6 +18,8 @@ TEXT runtime·sys_umtx_op(SB),NOSPLIT,$0 MOVQ ut+24(FP), R8 MOVL $454, AX SYSCALL + JCC 2(PC) + NEGQ AX MOVL AX, ret+32(FP) RET @@ -26,6 +28,8 @@ TEXT runtime·thr_new(SB),NOSPLIT,$0 MOVL size+8(FP), SI MOVL $455, AX SYSCALL + JCC 2(PC) + NEGQ AX MOVL AX, ret+16(FP) RET @@ -118,6 +122,8 @@ TEXT runtime·pipe2(SB),NOSPLIT,$0-20 MOVL flags+0(FP), SI MOVL $542, AX SYSCALL + JCC 2(PC) + NEGQ AX MOVL AX, errno+16(FP) RET diff --git a/src/runtime/sys_freebsd_arm.s b/src/runtime/sys_freebsd_arm.s index 1e12f9cfcb0..b12e47c576d 100644 --- a/src/runtime/sys_freebsd_arm.s +++ b/src/runtime/sys_freebsd_arm.s @@ -51,6 +51,7 @@ TEXT runtime·sys_umtx_op(SB),NOSPLIT,$0 ADD $20, R13 // arg 5 is passed on stack MOVW $SYS__umtx_op, R7 SWI $0 + RSB.CS $0, R0 SUB $20, R13 // BCS error MOVW R0, ret+20(FP) @@ -61,6 +62,7 @@ TEXT runtime·thr_new(SB),NOSPLIT,$0 MOVW size+4(FP), R1 MOVW $SYS_thr_new, R7 SWI $0 + RSB.CS $0, R0 MOVW R0, ret+8(FP) RET @@ -144,6 +146,7 @@ TEXT runtime·pipe2(SB),NOSPLIT,$0-16 MOVW flags+0(FP), R1 MOVW $SYS_pipe2, R7 SWI $0 + RSB.CS $0, R0 MOVW R0, errno+12(FP) RET diff --git a/src/runtime/sys_freebsd_arm64.s b/src/runtime/sys_freebsd_arm64.s index 8a4f9b7fa14..1aa09e87ca1 100644 --- a/src/runtime/sys_freebsd_arm64.s +++ b/src/runtime/sys_freebsd_arm64.s @@ -60,6 +60,9 @@ TEXT runtime·sys_umtx_op(SB),NOSPLIT,$0 MOVD ut+24(FP), R4 MOVD $SYS__umtx_op, R8 SVC + BCC ok + NEG R0, R0 +ok: MOVW R0, ret+32(FP) RET @@ -69,6 +72,9 @@ TEXT runtime·thr_new(SB),NOSPLIT,$0 MOVW size+8(FP), R1 MOVD $SYS_thr_new, R8 SVC + BCC ok + NEG R0, R0 +ok: MOVW R0, ret+16(FP) RET From 0aa9b4709acd609c1a3e9cb028e7f4c4da3f0357 Mon Sep 17 00:00:00 2001 From: Cherry Zhang Date: Tue, 22 Dec 2020 12:40:32 -0500 Subject: [PATCH 09/11] cmd/pack: r command create output file if not exist Go 1.15 pack's r command creates the output file if it does not exist. The system "ar" command does this as well. Do the same. For bazelbuild/rules_go#2762. Change-Id: Icd88396b5c714b735c859a29ab29851e4301f4d2 Reviewed-on: https://go-review.googlesource.com/c/go/+/279516 Trust: Cherry Zhang Run-TryBot: Cherry Zhang Reviewed-by: Than McIntosh TryBot-Result: Go Bot --- src/cmd/pack/pack.go | 7 +++++-- src/cmd/pack/pack_test.go | 25 ++++++++++++++++++++++++- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/src/cmd/pack/pack.go b/src/cmd/pack/pack.go index 3dffabe5ec8..412ea36d60f 100644 --- a/src/cmd/pack/pack.go +++ b/src/cmd/pack/pack.go @@ -43,7 +43,7 @@ func main() { ar = openArchive(os.Args[2], os.O_RDONLY, os.Args[3:]) ar.scan(ar.printContents) case 'r': - ar = openArchive(os.Args[2], os.O_RDWR, os.Args[3:]) + ar = openArchive(os.Args[2], os.O_RDWR|os.O_CREATE, os.Args[3:]) ar.addFiles() case 'c': ar = openArchive(os.Args[2], os.O_RDWR|os.O_TRUNC|os.O_CREATE, os.Args[3:]) @@ -124,10 +124,13 @@ func openArchive(name string, mode int, files []string) *Archive { log.Fatal(err) } var a *archive.Archive - if mode&os.O_CREATE != 0 { // the c command + if mode&os.O_TRUNC != 0 { // the c command a, err = archive.New(f) } else { a, err = archive.Parse(f, verbose) + if err != nil && mode&os.O_CREATE != 0 { // the r command + a, err = archive.New(f) + } } if err != nil { log.Fatal(err) diff --git a/src/cmd/pack/pack_test.go b/src/cmd/pack/pack_test.go index 16a51358009..118376f9df6 100644 --- a/src/cmd/pack/pack_test.go +++ b/src/cmd/pack/pack_test.go @@ -303,7 +303,7 @@ func TestIssue21703(t *testing.T) { } // Test the "c" command can "see through" the archive generated by the compiler. -// This is peculiar. (See issue ) +// This is peculiar. (See issue #43271) func TestCreateWithCompilerObj(t *testing.T) { testenv.MustHaveGoBuild(t) @@ -368,6 +368,29 @@ func TestCreateWithCompilerObj(t *testing.T) { } } +// Test the "r" command creates the output file if it does not exist. +func TestRWithNonexistentFile(t *testing.T) { + testenv.MustHaveGoBuild(t) + + dir := tmpDir(t) + defer os.RemoveAll(dir) + src := filepath.Join(dir, "p.go") + prog := "package p; var X = 42\n" + err := os.WriteFile(src, []byte(prog), 0666) + if err != nil { + t.Fatal(err) + } + + run := func(args ...string) string { + return doRun(t, dir, args...) + } + + goBin := testenv.GoToolPath(t) + run(goBin, "build", "cmd/pack") // writes pack binary to dir + run(goBin, "tool", "compile", "-o", "p.o", "p.go") + run("./pack", "r", "p.a", "p.o") // should succeed +} + // doRun runs a program in a directory and returns the output. func doRun(t *testing.T, dir string, args ...string) string { cmd := exec.Command(args[0], args[1:]...) From c06a354bccf60ea32ed74238be409a00aac292c5 Mon Sep 17 00:00:00 2001 From: Cherry Zhang Date: Mon, 21 Dec 2020 18:41:16 -0500 Subject: [PATCH 10/11] test: trigger SIGSEGV instead of SIGTRAP in issue11656.go In issue11656.go, it tests that if the runtime can get a reasonable traceback when it faults at a non-function PC. It does it by jumping to an address that contains an illegal or trap instruction. When it traps, the SIGTRAP crashes the runtime. This CL changes it to use an instruction that triggers SIGSEGV. This is due to two reasons: - currently, the handling of bad PC is done by preparePanic, which is only used for a panicking signal (SIGSEGV, SIGBUS, SIGFPE), not a fatal signal (e.g. SIGTRAP). - the test uses defer+recover to get a traceback, which only works for panicking signals, not fatal signals. Ideally, we should handle all kinds of faults (SIGSEGV, SIGBUS, SIGILL, SIGTRAP, etc.) with a nice traceback. I'll leave this for the future. This CL also adds RISCV64 support. Fixes #43283. Change-Id: I5e0fbf8530cc89d16e05c3257d282bc1d4d03405 Reviewed-on: https://go-review.googlesource.com/c/go/+/279423 Trust: Cherry Zhang Reviewed-by: Ian Lance Taylor --- test/fixedbugs/issue11656.go | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/test/fixedbugs/issue11656.go b/test/fixedbugs/issue11656.go index 50182633645..acd3f4f3e5d 100644 --- a/test/fixedbugs/issue11656.go +++ b/test/fixedbugs/issue11656.go @@ -27,13 +27,6 @@ import ( ) func main() { - // This test is currently failing on some architectures. - // See issue #43283. - switch runtime.GOARCH { - case "ppc64", "mips", "mipsle", "mips64", "mips64le": - return - } - debug.SetPanicOnFault(true) defer func() { if err := recover(); err == nil { @@ -61,27 +54,30 @@ func f(n int) { x uintptr } - // We want to force an illegal instruction, to get a crash - // at a PC value != 0. + // We want to force a seg fault, to get a crash at a PC value != 0. // Not all systems make the data section non-executable. ill := make([]byte, 64) switch runtime.GOARCH { case "386", "amd64": - binary.LittleEndian.PutUint16(ill, 0x0b0f) // ud2 + ill = append(ill, 0x89, 0x04, 0x25, 0x00, 0x00, 0x00, 0x00) // MOVL AX, 0 case "arm": - binary.LittleEndian.PutUint32(ill, 0xe7f000f0) // no name, but permanently undefined + binary.LittleEndian.PutUint32(ill, 0xe3a00000) // MOVW $0, R0 + binary.LittleEndian.PutUint32(ill, 0xe5800000) // MOVW R0, (R0) case "arm64": - binary.LittleEndian.PutUint32(ill, 0xd4207d00) // brk #1000 + binary.LittleEndian.PutUint32(ill, 0xf90003ff) // MOVD ZR, (ZR) case "ppc64": - binary.BigEndian.PutUint32(ill, 0x7fe00008) // trap + binary.BigEndian.PutUint32(ill, 0xf8000000) // MOVD R0, (R0) case "ppc64le": - binary.LittleEndian.PutUint32(ill, 0x7fe00008) // trap + binary.LittleEndian.PutUint32(ill, 0xf8000000) // MOVD R0, (R0) case "mips", "mips64": - binary.BigEndian.PutUint32(ill, 0x00000034) // trap + binary.BigEndian.PutUint32(ill, 0xfc000000) // MOVV R0, (R0) case "mipsle", "mips64le": - binary.LittleEndian.PutUint32(ill, 0x00000034) // trap + binary.LittleEndian.PutUint32(ill, 0xfc000000) // MOVV R0, (R0) case "s390x": - binary.BigEndian.PutUint32(ill, 0) // undefined instruction + ill = append(ill, 0xa7, 0x09, 0x00, 0x00) // MOVD $0, R0 + ill = append(ill, 0xe3, 0x00, 0x00, 0x00, 0x00, 0x24) // MOVD R0, (R0) + case "riscv64": + binary.LittleEndian.PutUint32(ill, 0x00003023) // MOV X0, (X0) default: // Just leave it as 0 and hope for the best. } From c9fb4eb0a22131cc9922fa96afba01d4e21d4fd4 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Sun, 8 Nov 2020 11:57:42 +0100 Subject: [PATCH 11/11] cmd/link: handle grouped resource sections The Go PE linker does not support enough generalized PE logic to properly handle .rsrc sections gracefully. Instead a few things are special cased for these. The linker also does not support PE's "grouped sections" features, in which input objects have several named sections that are sorted, merged, and renamed in the output file. In the past, more sophisticated support for resources or for PE features like grouped sections have not been necessary, as Go's own object formats are pretty vanilla, and GNU binutils also produces pretty vanilla objects where all sections are already merged. However, GNU binutils is lagging with arm support, and here LLVM has picked up the slack. In particular, LLVM has its own rc/cvtres combo, which are glued together in mingw LLVM distributions as windres, a command line compatible tool with binutils' windres, which supports arm and arm64. But there's a key difference between binutils' windres and LLVM's windres: the LLVM one uses proper grouped sections. So, this commit adds grouped sections support for resource sections to the linker. We don't attempt to plumb generic support for grouped sections, just as there isn't generic support already for what resources require. Instead we augment the resource handling logic to deal with standard two-section resource objects. We also add a test for this, akin to the current test for more vanilla binutils resource objects, and make sure that the rsrc tests are always performed. Fixes #42866. Fixes #43182. Change-Id: I059450021405cdf2ef1c195ddbab3960764ad711 Reviewed-on: https://go-review.googlesource.com/c/go/+/268337 Run-TryBot: Jason A. Donenfeld TryBot-Result: Go Bot Reviewed-by: Cherry Zhang Trust: Alex Brainman Trust: Jason A. Donenfeld --- src/cmd/link/internal/ld/lib.go | 2 +- src/cmd/link/internal/ld/pe.go | 60 ++++++++++-------- src/cmd/link/internal/loadpe/ldpe.go | 49 +++++++------- src/cmd/link/link_test.go | 19 ++++++ .../link/testdata/testPErsrc-complex/main.go | 43 +++++++++++++ .../testdata/testPErsrc-complex/rsrc.syso | Bin 0 -> 352 bytes 6 files changed, 124 insertions(+), 49 deletions(-) create mode 100644 src/cmd/link/testdata/testPErsrc-complex/main.go create mode 100644 src/cmd/link/testdata/testPErsrc-complex/rsrc.syso diff --git a/src/cmd/link/internal/ld/lib.go b/src/cmd/link/internal/ld/lib.go index 833b3eb9db6..bf95745d8d9 100644 --- a/src/cmd/link/internal/ld/lib.go +++ b/src/cmd/link/internal/ld/lib.go @@ -1820,7 +1820,7 @@ func ldobj(ctxt *Link, f *bio.Reader, lib *sym.Library, length int64, pn string, Errorf(nil, "%v", err) return } - if rsrc != 0 { + if len(rsrc) != 0 { setpersrc(ctxt, rsrc) } ctxt.Textp = append(ctxt.Textp, textp...) diff --git a/src/cmd/link/internal/ld/pe.go b/src/cmd/link/internal/ld/pe.go index adbf516d5c5..5edaf54dd2f 100644 --- a/src/cmd/link/internal/ld/pe.go +++ b/src/cmd/link/internal/ld/pe.go @@ -253,7 +253,7 @@ type Dll struct { } var ( - rsrcsym loader.Sym + rsrcsyms []loader.Sym PESECTHEADR int32 PEFILEHEADR int32 pe64 int @@ -1508,46 +1508,56 @@ func (ctxt *Link) dope() { initdynexport(ctxt) } -func setpersrc(ctxt *Link, sym loader.Sym) { - if rsrcsym != 0 { +func setpersrc(ctxt *Link, syms []loader.Sym) { + if len(rsrcsyms) != 0 { Errorf(nil, "too many .rsrc sections") } - - rsrcsym = sym + rsrcsyms = syms } func addpersrc(ctxt *Link) { - if rsrcsym == 0 { + if len(rsrcsyms) == 0 { return } - data := ctxt.loader.Data(rsrcsym) - size := len(data) - h := pefile.addSection(".rsrc", size, size) + var size int64 + for _, rsrcsym := range rsrcsyms { + size += ctxt.loader.SymSize(rsrcsym) + } + h := pefile.addSection(".rsrc", int(size), int(size)) h.characteristics = IMAGE_SCN_MEM_READ | IMAGE_SCN_CNT_INITIALIZED_DATA h.checkOffset(ctxt.Out.Offset()) - // relocation - relocs := ctxt.loader.Relocs(rsrcsym) - for i := 0; i < relocs.Count(); i++ { - r := relocs.At(i) - p := data[r.Off():] - val := uint32(int64(h.virtualAddress) + r.Add()) - - // 32-bit little-endian - p[0] = byte(val) - - p[1] = byte(val >> 8) - p[2] = byte(val >> 16) - p[3] = byte(val >> 24) + for _, rsrcsym := range rsrcsyms { + // A split resource happens when the actual resource data and its relocations are + // split across multiple sections, denoted by a $01 or $02 at the end of the .rsrc + // section name. + splitResources := strings.Contains(ctxt.loader.SymName(rsrcsym), ".rsrc$") + relocs := ctxt.loader.Relocs(rsrcsym) + data := ctxt.loader.Data(rsrcsym) + for ri := 0; ri < relocs.Count(); ri++ { + r := relocs.At(ri) + p := data[r.Off():] + val := uint32(int64(h.virtualAddress) + r.Add()) + if splitResources { + // If we're a split resource section, and that section has relocation + // symbols, then the data that it points to doesn't actually begin at + // the virtual address listed in this current section, but rather + // begins at the section immediately after this one. So, in order to + // calculate the proper virtual address of the data it's pointing to, + // we have to add the length of this section to the virtual address. + // This works because .rsrc sections are divided into two (but not more) + // of these sections. + val += uint32(len(data)) + } + binary.LittleEndian.PutUint32(p, val) + } + ctxt.Out.Write(data) } - - ctxt.Out.Write(data) h.pad(ctxt.Out, uint32(size)) // update data directory pefile.dataDirectory[pe.IMAGE_DIRECTORY_ENTRY_RESOURCE].VirtualAddress = h.virtualAddress - pefile.dataDirectory[pe.IMAGE_DIRECTORY_ENTRY_RESOURCE].Size = h.virtualSize } diff --git a/src/cmd/link/internal/loadpe/ldpe.go b/src/cmd/link/internal/loadpe/ldpe.go index 1e6f978531d..a5c025de8ff 100644 --- a/src/cmd/link/internal/loadpe/ldpe.go +++ b/src/cmd/link/internal/loadpe/ldpe.go @@ -157,8 +157,9 @@ func makeUpdater(l *loader.Loader, bld *loader.SymbolBuilder, s loader.Sym) *loa // Load loads the PE file pn from input. // Symbols are written into syms, and a slice of the text symbols is returned. -// If an .rsrc section is found, its symbol is returned as rsrc. -func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, input *bio.Reader, pkg string, length int64, pn string) (textp []loader.Sym, rsrc loader.Sym, err error) { +// If an .rsrc section or set of .rsrc$xx sections is found, its symbols are +// returned as rsrc. +func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, input *bio.Reader, pkg string, length int64, pn string) (textp []loader.Sym, rsrc []loader.Sym, err error) { lookup := func(name string, version int) (*loader.SymbolBuilder, loader.Sym) { s := l.LookupOrCreateSym(name, version) sb := l.MakeSymbolUpdater(s) @@ -176,7 +177,7 @@ func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, input *bio.Read // TODO: replace pe.NewFile with pe.Load (grep for "add Load function" in debug/pe for details) f, err := pe.NewFile(sr) if err != nil { - return nil, 0, err + return nil, nil, err } defer f.Close() @@ -211,21 +212,21 @@ func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, input *bio.Read bld.SetType(sym.STEXT) default: - return nil, 0, fmt.Errorf("unexpected flags %#06x for PE section %s", sect.Characteristics, sect.Name) + return nil, nil, fmt.Errorf("unexpected flags %#06x for PE section %s", sect.Characteristics, sect.Name) } if bld.Type() != sym.SNOPTRBSS { data, err := sect.Data() if err != nil { - return nil, 0, err + return nil, nil, err } sectdata[sect] = data bld.SetData(data) } bld.SetSize(int64(sect.Size)) sectsyms[sect] = s - if sect.Name == ".rsrc" { - rsrc = s + if sect.Name == ".rsrc" || strings.HasPrefix(sect.Name, ".rsrc$") { + rsrc = append(rsrc, s) } } @@ -246,22 +247,23 @@ func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, input *bio.Read continue } + splitResources := strings.HasPrefix(rsect.Name, ".rsrc$") sb := l.MakeSymbolUpdater(sectsyms[rsect]) for j, r := range rsect.Relocs { if int(r.SymbolTableIndex) >= len(f.COFFSymbols) { - return nil, 0, fmt.Errorf("relocation number %d symbol index idx=%d cannot be large then number of symbols %d", j, r.SymbolTableIndex, len(f.COFFSymbols)) + return nil, nil, fmt.Errorf("relocation number %d symbol index idx=%d cannot be large then number of symbols %d", j, r.SymbolTableIndex, len(f.COFFSymbols)) } pesym := &f.COFFSymbols[r.SymbolTableIndex] _, gosym, err := readpesym(l, arch, l.LookupOrCreateSym, f, pesym, sectsyms, localSymVersion) if err != nil { - return nil, 0, err + return nil, nil, err } if gosym == 0 { name, err := pesym.FullName(f.StringTable) if err != nil { name = string(pesym.Name[:]) } - return nil, 0, fmt.Errorf("reloc of invalid sym %s idx=%d type=%d", name, r.SymbolTableIndex, pesym.Type) + return nil, nil, fmt.Errorf("reloc of invalid sym %s idx=%d type=%d", name, r.SymbolTableIndex, pesym.Type) } rSym := gosym @@ -271,11 +273,11 @@ func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, input *bio.Read var rType objabi.RelocType switch arch.Family { default: - return nil, 0, fmt.Errorf("%s: unsupported arch %v", pn, arch.Family) + return nil, nil, fmt.Errorf("%s: unsupported arch %v", pn, arch.Family) case sys.I386, sys.AMD64: switch r.Type { default: - return nil, 0, fmt.Errorf("%s: %v: unknown relocation type %v", pn, sectsyms[rsect], r.Type) + return nil, nil, fmt.Errorf("%s: %v: unknown relocation type %v", pn, sectsyms[rsect], r.Type) case IMAGE_REL_I386_REL32, IMAGE_REL_AMD64_REL32, IMAGE_REL_AMD64_ADDR32, // R_X86_64_PC32 @@ -302,7 +304,7 @@ func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, input *bio.Read case sys.ARM: switch r.Type { default: - return nil, 0, fmt.Errorf("%s: %v: unknown ARM relocation type %v", pn, sectsyms[rsect], r.Type) + return nil, nil, fmt.Errorf("%s: %v: unknown ARM relocation type %v", pn, sectsyms[rsect], r.Type) case IMAGE_REL_ARM_SECREL: rType = objabi.R_PCREL @@ -323,8 +325,9 @@ func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, input *bio.Read // ld -r could generate multiple section symbols for the // same section but with different values, we have to take - // that into account - if issect(pesym) { + // that into account, or in the case of split resources, + // the section and its symbols are split into two sections. + if issect(pesym) || splitResources { rAdd += int64(pesym.Value) } @@ -346,7 +349,7 @@ func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, input *bio.Read name, err := pesym.FullName(f.StringTable) if err != nil { - return nil, 0, err + return nil, nil, err } if name == "" { continue @@ -384,7 +387,7 @@ func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, input *bio.Read bld, s, err := readpesym(l, arch, l.LookupOrCreateSym, f, pesym, sectsyms, localSymVersion) if err != nil { - return nil, 0, err + return nil, nil, err } if pesym.SectionNumber == 0 { // extern @@ -402,14 +405,14 @@ func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, input *bio.Read } else if pesym.SectionNumber > 0 && int(pesym.SectionNumber) <= len(f.Sections) { sect = f.Sections[pesym.SectionNumber-1] if _, found := sectsyms[sect]; !found { - return nil, 0, fmt.Errorf("%s: %v: missing sect.sym", pn, s) + return nil, nil, fmt.Errorf("%s: %v: missing sect.sym", pn, s) } } else { - return nil, 0, fmt.Errorf("%s: %v: sectnum < 0!", pn, s) + return nil, nil, fmt.Errorf("%s: %v: sectnum < 0!", pn, s) } if sect == nil { - return nil, 0, nil + return nil, nil, nil } if l.OuterSym(s) != 0 { @@ -418,7 +421,7 @@ func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, input *bio.Read } outerName := l.SymName(l.OuterSym(s)) sectName := l.SymName(sectsyms[sect]) - return nil, 0, fmt.Errorf("%s: duplicate symbol reference: %s in both %s and %s", pn, l.SymName(s), outerName, sectName) + return nil, nil, fmt.Errorf("%s: duplicate symbol reference: %s in both %s and %s", pn, l.SymName(s), outerName, sectName) } bld = makeUpdater(l, bld, s) @@ -429,7 +432,7 @@ func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, input *bio.Read bld.SetSize(4) if l.SymType(sectsym) == sym.STEXT { if bld.External() && !bld.DuplicateOK() { - return nil, 0, fmt.Errorf("%s: duplicate symbol definition", l.SymName(s)) + return nil, nil, fmt.Errorf("%s: duplicate symbol definition", l.SymName(s)) } bld.SetExternal(true) } @@ -446,7 +449,7 @@ func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, input *bio.Read if l.SymType(s) == sym.STEXT { for ; s != 0; s = l.SubSym(s) { if l.AttrOnList(s) { - return nil, 0, fmt.Errorf("symbol %s listed multiple times", l.SymName(s)) + return nil, nil, fmt.Errorf("symbol %s listed multiple times", l.SymName(s)) } l.SetAttrOnList(s, true) textp = append(textp, s) diff --git a/src/cmd/link/link_test.go b/src/cmd/link/link_test.go index 4eb02c9e8a1..7eeb7ef568f 100644 --- a/src/cmd/link/link_test.go +++ b/src/cmd/link/link_test.go @@ -786,6 +786,25 @@ func TestPErsrc(t *testing.T) { if !bytes.Contains(b, []byte("Hello Gophers!")) { t.Fatalf("binary does not contain expected content") } + + pkgdir = filepath.Join("testdata", "testPErsrc-complex") + exe = filepath.Join(tmpdir, "a.exe") + cmd = exec.Command(testenv.GoToolPath(t), "build", "-o", exe) + cmd.Dir = pkgdir + // cmd.Env = append(os.Environ(), "GOOS=windows", "GOARCH=amd64") // uncomment if debugging in a cross-compiling environment + out, err = cmd.CombinedOutput() + if err != nil { + t.Fatalf("building failed: %v, output:\n%s", err, out) + } + + // Check that the binary contains the rsrc data + b, err = ioutil.ReadFile(exe) + if err != nil { + t.Fatalf("reading output failed: %v", err) + } + if !bytes.Contains(b, []byte("resname RCDATA a.rc")) { + t.Fatalf("binary does not contain expected content") + } } func TestContentAddressableSymbols(t *testing.T) { diff --git a/src/cmd/link/testdata/testPErsrc-complex/main.go b/src/cmd/link/testdata/testPErsrc-complex/main.go new file mode 100644 index 00000000000..affd6eada2e --- /dev/null +++ b/src/cmd/link/testdata/testPErsrc-complex/main.go @@ -0,0 +1,43 @@ +// Copyright 2020 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. + +// Test that a PE rsrc section is handled correctly, when the object files +// have been created by llvm-rc or msvc's rc.exe, which means there's the +// @feat.00 symbol as well as split .rsrc$00 and .rsrc$01 section to deal with. +// +// rsrc.syso is created with: +// windres -i a.rc -o rsrc.syso -O coff +// where this windres calls into llvm-rc and llvm-cvtres. The source file, +// a.rc, simply contains a reference to its own bytes: +// +// resname RCDATA a.rc +// +// Object dumping the resultant rsrc.syso, we can see the split sections and +// the @feat.00 SEH symbol: +// +// rsrc.syso: file format coff-x86-64 +// +// architecture: x86_64 +// start address: 0x0000000000000000 +// +// Export Table: +// Sections: +// Idx Name Size VMA Type +// 0 .rsrc$01 00000068 0000000000000000 DATA +// 1 .rsrc$02 00000018 0000000000000000 DATA +// +// SYMBOL TABLE: +// [ 0](sec -1)(fl 0x00)(ty 0)(scl 3) (nx 0) 0x00000011 @feat.00 +// [ 1](sec 1)(fl 0x00)(ty 0)(scl 3) (nx 1) 0x00000000 .rsrc$01 +// AUX scnlen 0x68 nreloc 1 nlnno 0 checksum 0x0 assoc 0 comdat 0 +// [ 3](sec 2)(fl 0x00)(ty 0)(scl 3) (nx 1) 0x00000000 .rsrc$02 +// AUX scnlen 0x18 nreloc 0 nlnno 0 checksum 0x0 assoc 0 comdat 0 +// [ 5](sec 2)(fl 0x00)(ty 0)(scl 3) (nx 0) 0x00000000 $R000000 +// RELOCATION RECORDS FOR [.rsrc$01]: +// OFFSET TYPE VALUE +// 0000000000000048 IMAGE_REL_AMD64_ADDR32NB $R000000 + +package main + +func main() {} diff --git a/src/cmd/link/testdata/testPErsrc-complex/rsrc.syso b/src/cmd/link/testdata/testPErsrc-complex/rsrc.syso new file mode 100644 index 0000000000000000000000000000000000000000..eff630b8a23de71fa27da5bf0b49965bf4cea69a GIT binary patch literal 352 zcmYdkV`8vbwl4ky0|Nsa5CZ|DUQuyTvWkHrgv-E?0c4~A@foN9Ban6gBB(kes4#;B zkZ}XXfzl3OrO1Me3|v4iP;CRMC<8+TP|N^L9OxVdP8J3R52#`hBu(rLK@6@8!3=&3 zjtss)7Dx>%5HkaDQEG8sVs5HJkh6