- 19 Apr, 2019 4 commits
-
-
Josh Bleecher Snyder authored
The phi tighten pass moves rematerializable phi args to the immediate predecessor of the phis. This reduces value lifetimes for regalloc. However, the critical edge removal pass can introduce new blocks, which can change what a block's immediate precedessor is. This can result in tightened phi args being spilled unnecessarily. This change moves the phi tighten pass after the critical edge pass, when the block structure is stable. This improves the code generated for func f(s string) bool { return s == "abcde" } Before this change: "".f STEXT nosplit size=44 args=0x18 locals=0x0 0x0000 00000 (x.go:3) MOVQ "".s+16(SP), AX 0x0005 00005 (x.go:3) CMPQ AX, $5 0x0009 00009 (x.go:3) JNE 40 0x000b 00011 (x.go:3) MOVQ "".s+8(SP), AX 0x0010 00016 (x.go:3) CMPL (AX), $1684234849 0x0016 00022 (x.go:3) JNE 36 0x0018 00024 (x.go:3) CMPB 4(AX), $101 0x001c 00028 (x.go:3) SETEQ AL 0x001f 00031 (x.go:3) MOVB AL, "".~r1+24(SP) 0x0023 00035 (x.go:3) RET 0x0024 00036 (x.go:3) XORL AX, AX 0x0026 00038 (x.go:3) JMP 31 0x0028 00040 (x.go:3) XORL AX, AX 0x002a 00042 (x.go:3) JMP 31 Observe the duplicated blocks at the end. After this change: "".f STEXT nosplit size=40 args=0x18 locals=0x0 0x0000 00000 (x.go:3) MOVQ "".s+16(SP), AX 0x0005 00005 (x.go:3) CMPQ AX, $5 0x0009 00009 (x.go:3) JNE 36 0x000b 00011 (x.go:3) MOVQ "".s+8(SP), AX 0x0010 00016 (x.go:3) CMPL (AX), $1684234849 0x0016 00022 (x.go:3) JNE 36 0x0018 00024 (x.go:3) CMPB 4(AX), $101 0x001c 00028 (x.go:3) SETEQ AL 0x001f 00031 (x.go:3) MOVB AL, "".~r1+24(SP) 0x0023 00035 (x.go:3) RET 0x0024 00036 (x.go:3) XORL AX, AX 0x0026 00038 (x.go:3) JMP 31 Change-Id: I12c81aa53b89456cb5809aa5396378245f3beda9 Reviewed-on: https://go-review.googlesource.com/c/go/+/172597 Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:Keith Randall <khr@golang.org>
-
Bryan C. Mills authored
Updates #31088 Change-Id: Ia126e4e83ac5cb12c2c4151d5e5c975497598f24 Reviewed-on: https://go-review.googlesource.com/c/go/+/172979Reviewed-by:
Brad Fitzpatrick <bradfitz@golang.org>
-
Bryan C. Mills authored
'go mod vendor' already drops test files and testdata directories, so users should not expect the vendored module to include unnecessary files in general. Files tagged "ignore" are typically only used to refresh or regenerate source files within the module to be vendored, so users of that module do not need them. Fixes #31088 Change-Id: I1ce9545e9b37c8e779a1826a9d494ac29d2cbfb8 Reviewed-on: https://go-review.googlesource.com/c/go/+/172978 Run-TryBot: Bryan C. Mills <bcmills@google.com> Reviewed-by:
-
LE Manh Cuong authored
Its only usage was removed in golang.org/cl/103860 Change-Id: I2a230b9475b0aadf3892b89f5e4ee6d4c5b70394 Reviewed-on: https://go-review.googlesource.com/c/go/+/172917Reviewed-by:
-
- 18 Apr, 2019 10 commits
-
-
Brad Fitzpatrick authored
Fixes #31554 Updates #12032 (also originally about plan9, but later openbsd/arm) Change-Id: Ib9f35d27a2304f38bf271c38c0b9153d210d8f95 Reviewed-on: https://go-review.googlesource.com/c/go/+/172837 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by:
David du Colombier <0intro@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
-
Rob Pike authored
CL 172698 documented this flag but the description was missing punctuation and could be clearer. Change-Id: I310d91ae8c6b947ce7d1ae7559882f49778f770a Reviewed-on: https://go-review.googlesource.com/c/go/+/172817Reviewed-by:
-
jfbus authored
There is a DNS resolution bug in Kubernetes (UDP response packets get dropped by conntrack, causing timeouts in DNS queries). The recommended workaround on Linux is to configure the resolver to use TCP for DNS queries, by setting the use-vc option in resolv.conf. With this PR, the pure Go resolver searches for "use-vc" in resolv.conf and switches to TCP when found. Fixes #29358 Change-Id: I26b935cae2c80e5bb9955da83299a8dea84591de GitHub-Last-Rev: 70bc00fe41f44f0b2b3cfebe67bbcc45701968cf GitHub-Pull-Request: golang/go#29594 Reviewed-on: https://go-review.googlesource.com/c/go/+/156366 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
-
Andrew Z Allen authored
Document that the default quickcheck configuration is to run 100 times and that there is a flag that configures it called "quickchecks". Change-Id: I46fdab9d572e132ccc23ef907f9cc6b2d06b37c4 Reviewed-on: https://go-review.googlesource.com/c/go/+/172698Reviewed-by:
-
Josh Bleecher Snyder authored
CL 172579 added field names in nodl. See that CL for an explanation. Cuong Manh Le pointed out that we should do the same in newnamel. This cuts 40k off the cmd/compile binary. Change-Id: I427b117531c59630dee36f1257aad8975626b2c2 Reviewed-on: https://go-review.googlesource.com/c/go/+/172604 Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com> Reviewed-by:
-
Rens Rikkerink authored
Running `go mod init` outside of GOPATH with `GO111MODULE=off` silently fails. This behavior was undocumented. This CL makes go mod fail with the error: go: modules disabled by GO111MODULE=off; see 'go help modules' Comparing with already erroring GO111MODULE=<value> conditions: * With GO111MODULE=auto, inside GOPATH: go modules disabled inside GOPATH/src by GO111MODULE=auto; see 'go help modules' * With GO111MODULE=auto outside of GOPATH: go: cannot determine module path for source directory /path/to/dir (outside GOPATH, no import comments) Fixes #31342 Change-Id: I749787d2a8640913c4ac263072d051314d76e778 GitHub-Last-Rev: b38447457d8cabed367ea4872cf7f238a49539c7 GitHub-Pull-Request: golang/go#31255 Reviewed-on: https://go-review.googlesource.com/c/go/+/170697Reviewed-by:Bryan C. Mills <bcmills@google.com> Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
-
Ian Lance Taylor authored
New versions of clang can generate multiple sections named ".text" when using vague C++ linkage. This is valid ELF, but would cause the Go linker to report an error when using internal linking: symbol PACKAGEPATH(.text) listed multiple times Avoid the problem by renaming section symbol names if there is a name collision. Change-Id: I41127e95003d5b4554aaf849177b3fe000382c02 Reviewed-on: https://go-review.googlesource.com/c/go/+/172697 Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:Cherry Zhang <cherryyz@google.com>
-
Rob Pike authored
The byte-at-a-time allocation done quoting strings in appendQuotedWith grows the output incrementally, which is poor behavior for very large strings. An easy fix is to make sure the buffer has enough room at least for an unquoted string. Add a benchmark with a megabyte of non-ASCII data. Before: 39 allocations. After: 7 allocations. We could do better by doing a lot more work but this seems like a big result for little effort. Fixes #31472. Change-Id: I852139e0a2bd13722c4dd329ded8ae1759abad5b Reviewed-on: https://go-review.googlesource.com/c/go/+/172677Reviewed-by:
Ian Lance Taylor <iant@golang.org> Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
-
Clément Chigot authored
As the stackguard was increased on AIX by CL 157117, every syscalls can now have libcall directly on the stack. This fixes some concurrency bugs which seems to occur when semasleep is interrupted by a SIGPROF signal. Change-Id: I905a9618d13ef227dad6f8328b0f958f2f917a5b Reviewed-on: https://go-review.googlesource.com/c/go/+/172359 Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
-
erifan01 authored
variable setg_gcc in runtime/cgo/*.c should be static, otherwise it will be mixed with the function of the same name in runtime/asm_*.s or tls_*.s, which causes an error when building PIE with internal linking mode. Fixes #31485 Change-Id: I79b311ffcaf450984328db65397840ae7d85e65d Reviewed-on: https://go-review.googlesource.com/c/go/+/172498Reviewed-by:
-
- 17 Apr, 2019 11 commits
-
-
erifan01 authored
Currently the offset values of ELF relocations and Macho relocations are 256 and 512 respectively, which means that the space reserved for ELF relocations is only 256. But AARCH64 has more than 256 ELF relocation types, in fact the maximum AARCH64 ELF relocation type recorded in file src/debug/elf/elf.go is 1032 currently. So this CL increases the offset of Macho relocations to 2048 to leave enough space for AARCH64 ELF relocations. Change-Id: I784ac38aeb3e102ac7825f6d621086849c8d3146 Reviewed-on: https://go-review.googlesource.com/c/go/+/172497 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
-
Matthew Dempsky authored
Casp1 is implemented in Go on js/wasm, so escape analysis correctly determines that the "old" parameter does not escape (which is good). Unfortunately, test/run.go doesn't have a way to indicate that ERROR messages are optional, and cmd/compile only emits diagnostics for "var x int" when it's moved to the heap; not when it stays on the stack. To accomodate that this test currently passes on some GOARCHes but not others, rewrite the Casp1 test to use "x := new(int)" and allow both "new(int) escapes to heap" or "new(int) does not escape". Updates #31525. Change-Id: I40150a7ff9042f184386ccdb2d4d428f63e8ba4f Reviewed-on: https://go-review.googlesource.com/c/go/+/172602 Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by:
-
Josh Bleecher Snyder authored
We use a struct to allocate two structs simultaneously. Because we embed structs rather than using named fields, the compiler generates forwarding method stubs for the anonymous type. In theory, the compiler could detect that these stubs are unnecessary: The value in question has a very limited scope, the methods are not called, and there are operations where an interface would need to be satisfied. This compiler optimization is unlikely to happen, though; the ROI is likely to be low. Instead, just give the fields names. Cuts 64k off the cmd/compile binary. Change-Id: Id10ec69c23cd2dd33306f4c1bc75724e3c571b56 Reviewed-on: https://go-review.googlesource.com/c/go/+/172579 Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com> Reviewed-by:
Matthew Dempsky <mdempsky@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
-
Matthew Dempsky authored
The //go:noescape directive says that arguments don't leak at all, which is too aggressive of a claim for functions that return pointers derived from their parameters. Remove the directive for now. Long term fix will require a new directive that allows more fine-grained control over escape analysis information supplied for functions implemented in assembly. Also, update the BAD comments in the test cases for Loadp: we really want that *ptr leaks to the result parameter, not that *ptr leaks to the heap. Updates #31525. Change-Id: Ibfa61f2b70daa7ed3223056b57eeee777eef2e31 Reviewed-on: https://go-review.googlesource.com/c/go/+/172578 Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
Austin Clements <austin@google.com>
-
Baokun Lee authored
Change-Id: I43d233739ce6a6fbc4ee281b569d6230dd552cb4 Reviewed-on: https://go-review.googlesource.com/c/go/+/160057Reviewed-by:
-
Michael Fraenkel authored
Fixes #30898 Change-Id: I903dd8ed2b10c49b2291ad0858774f3ca2f5b223 Reviewed-on: https://go-review.googlesource.com/c/go/+/172422Reviewed-by:
-
Matthew Dempsky authored
Cherry pointed out this case in review for CL 136496. That CL was slightly too aggressive, and I likely would have made the same mistake if I tried it myself. Updates #27772. Change-Id: I1fafabb9f8d9aba0494aa71333a4e17cf1bac5c8 Reviewed-on: https://go-review.googlesource.com/c/go/+/172421 Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
-
Matthew Dempsky authored
There weren't any tests to make sure these work correctly, and this led to escape analysis regressions in both linux/s390x and js/wasm. The underlying issue that cmd/compile is only getting some of these correct because escape analysis doesn't understand //go:linkname is still present, but at least this addresses the fragility aspect. Updates #15283. Change-Id: I546aee1899d098b2e3de45e9b33c3ca22de485f8 Reviewed-on: https://go-review.googlesource.com/c/go/+/172420 Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
-
Bryan C. Mills authored
This test fails frequently in the longtest builder, and the failures on the build dashboard have masked two other regressions so far. Let's skip it until it can be fixed. Updates #31263 Change-Id: I82bae216ebc3c5fd395c27c72c196334a130af7d Reviewed-on: https://go-review.googlesource.com/c/go/+/172423 Run-TryBot: Bryan C. Mills <bcmills@google.com> Reviewed-by:
David Chase <drchase@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
-
jfbus authored
There is a DNS resolution issue in Kubernetes (UDP response packets get dropped due to a race in conntrack between the parallel A and AAAA queries, causing timeouts in DNS queries). A workaround is to enable single-request / single-request-reopen in resolv.conf in order to use sequential A and AAAA queries instead of parallel queries. With this PR, the pure Go resolver searches for "single-request" and "single-request-reopen" in resolv.conf and send A and AAAA queries sequentially when found. Fixes #29644 Change-Id: I906b3484008c1b9adf2e3e9241ea23767e29df59 GitHub-Last-Rev: d481acfb4c49d82fd474078b31a1a4697b57dadf GitHub-Pull-Request: golang/go#29661 Reviewed-on: https://go-review.googlesource.com/c/go/+/157377 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
-
Matthew Dempsky authored
Writev was allocating a new []syscall.Iovec every call, rather than reusing the cached copy available at *fd.iovec. Fixes #26663. Change-Id: I5967b0d82dc671ce0eaf4ec36cc2a0e46eadde02 Reviewed-on: https://go-review.googlesource.com/c/go/+/172419 Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
-
- 16 Apr, 2019 15 commits
-
-
David Benjamin authored
The CBC mode ciphers in TLS are a disaster. By ordering authentication and encryption wrong, they are very subtly dependent on details and implementation of the padding check, admitting attacks such as POODLE and Lucky13. crypto/tls does not promise full countermeasures for Lucky13 and still contains some timing variations. This change fixes one of the easy ones: by checking the MAC, then the padding, rather than all at once, there is a very small timing variation between bad MAC and (good MAC, bad padding). The consequences depend on the effective padding value used in the MAC when the padding is bad. extractPadding simply uses the last byte's value, leaving the padding bytes effectively unchecked. This is the scenario in SSL 3.0 that led to POODLE. Specifically, the attacker can take an input record which uses 16 bytes of padding (a full block) and replace the final block with some interesting block. The MAC check will succeed with 1/256 probability due to the final byte being 16. This again means that after 256 queries, the attacker can decrypt one byte. To fix this, bitwise AND the two values so they may be checked with one branch. Additionally, zero the padding if the padding check failed, to make things more robust. Updates #27071 Change-Id: I332b14d215078928ffafe3cfeba1a68189f08db3 Reviewed-on: https://go-review.googlesource.com/c/go/+/170701Reviewed-by:
Filippo Valsorda <filippo@golang.org> Run-TryBot: Filippo Valsorda <filippo@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
-
Sergey Yanykin authored
Updates #20205 Change-Id: I44a7ee46a1cdc7fe6fd36c4db4c0dd87a19f7f5d Reviewed-on: https://go-review.googlesource.com/c/go/+/171733Reviewed-by:
Josh Bleecher Snyder <josharian@gmail.com> Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
-
Brad Fitzpatrick authored
Per https://golang.org/wiki/Spelling and CL 33017. Change-Id: Ia813a81d25603883114c4e4b6997eb560d6a3690 Reviewed-on: https://go-review.googlesource.com/c/go/+/172457Reviewed-by:
Dmitri Shuralyov <dmitshur@golang.org>
-
Ainar Garipov authored
Also gofmt test/escape5.go. Fixes #28369. Change-Id: I0a11748fd2b5cf01cb5437ae15827d9db91c0c0d Reviewed-on: https://go-review.googlesource.com/c/go/+/172358Reviewed-by:
-
Josh Bleecher Snyder authored
Though there is variation in the spelling of canceled, cancellation is always spelled with a double l. Reference: https://www.grammarly.com/blog/canceled-vs-cancelled/ Change-Id: I240f1a297776c8e27e74f3eca566d2bc4c856f2f Reviewed-on: https://go-review.googlesource.com/c/go/+/170060 Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
-
Bryan C. Mills authored
Fixes #31247 Change-Id: I85a760a5d36ae835c97a13f980804d06b658857e Reviewed-on: https://go-review.googlesource.com/c/go/+/172418 Run-TryBot: Bryan C. Mills <bcmills@google.com> Reviewed-by:
-
Matthew Dempsky authored
Same as CL 170323, but for s390x instead of wasm. Fixes #31495. Change-Id: Ie39f649f5e33690375a8bcb1bc3b92d912ca4398 Reviewed-on: https://go-review.googlesource.com/c/go/+/172417 Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by:
-
LE Manh Cuong authored
In typecheckclosure, a xfunc node will be put to xtop. But that node can be shared between multiple closures, like in a const declaration group: const ( x = unsafe.Sizeof(func() {}) y ) It makes a xfunc node appears multiple times in xtop, causing duplicate initLSym run. To fix this issue, we only do typecheck for xfunc one time, and setup closure node earlier in typecheckclosure process. Fixes #30709 Change-Id: Ic924a157ee9f3e5d776214bef5390849ddc8aab9 Reviewed-on: https://go-review.googlesource.com/c/go/+/172298Reviewed-by: -
Jay Conrod authored
When we construct the build list by loading packages (e.g., in "go build", "go list", or "go test"), we may load additional modules not mentioned in the original build list. If we encounter an error loading one of these modules, mvs.BuildList currently returns a BuildListError with a chain of requirments. Unfortunately, this is not helpful, since the graph is structured such that these missing modules are direct requirements of the main module. With this change, loader.load keeps track of the package that caused each "missing" module to be added. If an error occurs in a missing module, the chain of package imports is printed instead of the module requirements. Fixes #31475 Change-Id: Ie484814af42ceea3e85fedc38e705ba3a38cd495 Reviewed-on: https://go-review.googlesource.com/c/go/+/171859 Run-TryBot: Jay Conrod <jayconrod@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
-
Jay Conrod authored
mvs.BuildList reports errors with a chain of modules to make it clear why the module where the error occurred was part of the build. This is a little confusing with "go get -u" since there are edges in the module graph for requirements and for updates. With this change, we now print "requires" or "updates to" between each module version in the chain. Updates #30661 Change-Id: Ie689500ea86857e715b250b9e0cae0bc6686dc32 Reviewed-on: https://go-review.googlesource.com/c/go/+/171150 Run-TryBot: Jay Conrod <jayconrod@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
-
Jay Conrod authored
For example, "go get golang.org/x/tools/cmd/..." will add a requirement for "golang.org/x/tools" to go.mod and will install executables from the "cmd" subdirectory. Fixes #29363 Change-Id: Id53f051710708d7760ffe831d4274fd54533d2b7 Reviewed-on: https://go-review.googlesource.com/c/go/+/171138 Run-TryBot: Jay Conrod <jayconrod@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
-
Constantin Konstantinidis authored
This fix removes the special case of auto-deriving the module path only for VCS github.com. Error message now explicitly requests the module path. Documentation and its FAQ do not need an update as only the beginning of the message is mentioned and is not modified. Fixes #27951 Change-Id: Icaf87a38b5c58451edba9beaa12ae9a68e288ca1 Reviewed-on: https://go-review.googlesource.com/c/go/+/172019Reviewed-by:
Daniel Lublin <daniel@lublin.se> Reviewed-by:
-
Matthew Dempsky authored
RELNOTE=yes The new escape analysis pass is more precise, which for most Go code should be an improvement. However, it may also break code that happened to work before (e.g., code that violated the unsafe.Pointer safety rules). The old escape analysis pass can be re-enabled with "go build -gcflags=all=-newescape=false". N.B., it's NOT recommended to mix the old and new escape analysis passes such as by omitting "all=". While the old and new escape analysis passes use similar and mostly compatible metadata, there are cases (e.g., closure handling) where they semantically differ and could lead to memory corruption errors in compiled programs. Fixes #23109. Change-Id: I0b1b6a6de5e240cb30c87a165f47bb8795491158 Reviewed-on: https://go-review.googlesource.com/c/go/+/170448 Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
-
Gergely Brautigam authored
This change modifies cmd/go/list to format the error correctly in case -e flag is set. It also fixes a bug where the package loader was only ever checking the first pattern if it had the go extension. This caused and error when a file without .go extension was not the first argument. Fixes #29899 Change-Id: I029bf4465ad4ad054434b8337c1d2a59369783da Reviewed-on: https://go-review.googlesource.com/c/go/+/166398 Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
-
Bryan C. Mills authored
On some configurations of Windows, directories containing executable files may be locked for a while after the executable exits (perhaps due to antivirus scans?). It's probably worth a little extra latency on exit to avoid filling up the user's temporary directory with leaked files. Updates #30789 Change-Id: Iae7fcdd07fb9ecfb05967cfe0c8833db646d2f85 Reviewed-on: https://go-review.googlesource.com/c/go/+/172337 Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
-
