1
0
mirror of https://github.com/golang/go synced 2024-11-05 17:46:16 -07:00
Commit Graph

1674 Commits

Author SHA1 Message Date
Keith Randall
3dec253783 reflect: zero stack slots before writing to them with write barriers
reflect.assignTo writes to the target using write barriers. Make sure
that the memory it is writing to is zeroed, so the write barrier does
not read pointers from uninitialized memory.

Fixes #39541

Change-Id: Ia64b2cacc193bffd0c1396bbce1dfb8182d4905b
Reviewed-on: https://go-review.googlesource.com/c/go/+/238760
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2020-06-19 02:11:35 +00:00
Michael Munday
377c1536f5 cmd/compile: mark s390x int <-> float conversions as clobbering flags
These conversion instructions set the condition code and so should
be marked as clobbering flags.

Fixes #39651.

Change-Id: I91cc9687ea70ef0551bb3139c1875071c349d43e
Reviewed-on: https://go-review.googlesource.com/c/go/+/238628
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2020-06-18 16:36:16 +00:00
Keith Randall
8c8045fd38 cmd/compile: fix ordering problems in struct equality
Make sure that if a field comparison might panic, we evaluate
(and short circuit if not equal) all previous fields, and don't
evaluate any subsequent fields.

Add a bunch more tests to the equality+panic checker.

Update #8606

Change-Id: I6a159bbc8da5b2b7ee835c0cd1fc565575b58c46
Reviewed-on: https://go-review.googlesource.com/c/go/+/237919
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-06-15 18:03:08 +00:00
Michael Munday
ac743dea8e cmd/compile: always tighten and de-duplicate tuple selectors
The scheduler assumes two special invariants that apply to tuple
selectors (Select0 and Select1 ops):

  1. There is only one tuple selector of each type per generator.
  2. Tuple selectors and generators reside in the same block.

Prior to this CL the assumption was that these invariants would
only be broken by the CSE pass. The CSE pass therefore contained
code to move and de-duplicate selectors to fix these invariants.

However it is also possible to write relatively basic optimization
rules that cause these invariants to be broken. For example:

  (A (Select0 (B))) -> (Select1 (B))

This rule could result in the newly added selector (Select1) being
in a different block to the tuple generator (see issue #38356). It
could also result in duplicate selectors if this rule matches
multiple times for the same tuple generator (see issue #39472).

The CSE pass will 'fix' these invariants. However it will only do
so when optimizations are enabled (since disabling optimizations
disables the CSE pass).

This CL moves the CSE tuple selector fixup code into its own pass
and makes it mandatory even when optimizations are disabled. This
allows tuple selectors to be treated like normal ops for most of
the compilation pipeline until after the new pass has run, at which
point we need to be careful to maintain the invariant again.

Fixes #39472.

Change-Id: Ia3f79e09d9c65ac95f897ce37e967ee1258a080b
Reviewed-on: https://go-review.googlesource.com/c/go/+/237118
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
2020-06-10 14:55:29 +00:00
Keith Randall
f72d7cfc8f cmd/compile: add interface equality tests
Add interfaces which differ in type. Those used so far only
differ in value, not type.

These additional tests are needed to generate a failure
before CL 236278 went in.

Update #8606

Change-Id: Icdb7647b1973c2fff7e5afe2bd8b8c1b384f583e
Reviewed-on: https://go-review.googlesource.com/c/go/+/236418
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2020-06-04 06:21:55 +00:00
Keith Randall
9984ef824c cmd/compile: test that equality is evaluated in order
Make sure that we compare fields of structs and elements of arrays in order,
with proper short-circuiting.

Update #8606

Change-Id: I0a66ad92ea0af7bcc56dfdb275dec2b8d7e8b4fe
Reviewed-on: https://go-review.googlesource.com/c/go/+/236147
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-06-03 19:07:55 +00:00
Richard Musiol
0452f9460f runtime: fix race condition between timer and event handler
This change fixes a race condition between beforeIdle waking up the
innermost event handler and a timer causing a different goroutine to
wake up at the exact same moment. This messes up the wasm event handling
and leads to memory corruption. The solution is to make beforeIdle
return the goroutine that must run next and have findrunnable pick
this goroutine without considering timers again.

Fixes #38093
Fixes #38574

Change-Id: Iffbe99411d25c2730953d1c8b0741fd892f8e540
Reviewed-on: https://go-review.googlesource.com/c/go/+/230178
Run-TryBot: Richard Musiol <neelance@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2020-05-31 18:35:04 +00:00
Josh Bleecher Snyder
364a05e2fe cmd/compile: add test for issue 37246
CL 233857 fixed the underlying issue for #37246,
which had arisen again as #38916.

Add the test case from #37246 to ensure it stays fixed.

Fixes #37246

Change-Id: If7fd75a096d2ce4364dc15509253c3882838161d
Reviewed-on: https://go-review.googlesource.com/c/go/+/233941
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Munday <mike.munday@ibm.com>
2020-05-14 15:18:29 +00:00
Michael Munday
f073395b73 cmd/compile: fix tuple selector bug in CSE pass
When tuple generators and selectors are eliminated as part of the
CSE pass we may end up with tuple selectors that are in different
blocks to the tuple generators that they correspond to. This breaks
the invariant that tuple generators and their corresponding
selectors must be in the same block. Therefore after CSE this
situation must be corrected.

Unfortunately the fixup code did not take into account that selectors
could be eliminated by CSE. It assumed that only the tuple generators
could be eliminated. In some situations this meant that it got into
a state where it was replacing references to selectors with references
to dead selectors in the wrong block.

To fix this we move the fixup code after the CSE rewrites have been
applied. This removes any difficult-to-reason-about interactions
with the CSE rewriter.

Fixes #38916.

Change-Id: I2211982dcdba399d03299f0a819945b3eb93b291
Reviewed-on: https://go-review.googlesource.com/c/go/+/233857
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2020-05-14 08:07:52 +00:00
Emmanuel T Odeke
7cbee12444 cmd/compile: improve error when setting unexported fields
Improve the error user experience when users try to set/refer
to unexported fields and methods of struct literals, by directly saying

    "cannot refer to unexported field or method"

Fixes #31053

Change-Id: I6fd3caf64b7ca9f9d8ea60b7756875e340792d59
Reviewed-on: https://go-review.googlesource.com/c/go/+/201657
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-05-08 20:44:01 +00:00
Emmanuel T Odeke
26de581a70 cmd/compile: omit file:pos for non-existent errors
Omits printing the file:line:column when trying to
open non-existent files

Given:
    go tool compile x.go

* Before:
    x.go:0: open x.go: no such file or directory

* After:
    open x.go: no such file or directory

Reverts the revert in CL 231043 by only fixing the case
of non-existent errors which is what the original bug
was about. The fix for "permission errors" will come later
on when I have bandwidth to investigate the differences
between running with root and why os.Open works for some
builders and not others.

Fixes #36437

Change-Id: I9c8a0981ad708b504bb43990a4105b42266fa41f
Reviewed-on: https://go-review.googlesource.com/c/go/+/230941
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
2020-05-08 20:28:57 +00:00
Cuong Manh Le
0f47c12a29 cmd/compile: do not emit code for discardable blank fields
Fixes #38690

Change-Id: I3544daf617fddc0f89636265c113001178d16b0c
Reviewed-on: https://go-review.googlesource.com/c/go/+/230121
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2020-05-06 04:34:54 +00:00
Keith Randall
b4ecafc986 cmd/compile: restrict bit test rewrite rules
The {AND,OR,XOR}const ops can only take an int32 as an argument.
Make sure that when rewriting a BTx op to one of these, the result
has no high-order bits.

Fixes #38746

Change-Id: Ia7c5f76952329f60974bc033c29a5433610f3b28
Reviewed-on: https://go-review.googlesource.com/c/go/+/231977
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
2020-05-05 15:41:37 +00:00
Austin Clements
eda6fe3572 Revert "cmd/compile: omit file:pos for non-existent, permission errors"
This reverts commit 4f7053c87f.

Reason for revert: Newly added test is failing on several builders.

Change-Id: I22dcbfebf2f57735b2f479886bbeb623f95b132f
Reviewed-on: https://go-review.googlesource.com/c/go/+/231043
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-04-30 02:29:55 +00:00
Emmanuel T Odeke
4f7053c87f cmd/compile: omit file:pos for non-existent, permission errors
Omits printing the file:line:column when trying to open either
* non-existent files
* files without permission

Given:
    go tool compile x.go

For either of x.go not existing, or if no read permissions:

* Before:
    x.go:0: open x.go: no such file or directory
    x.go:0: open x.go: permission denied

* After:
    open x.go: no such file or directory
    open x.go: permission denied

While here, noticed an oddity with the Linux builders, that appear
to always be running under root, hence the test for permission errors
with 0222 -W-*-W-*-W- can't pass on linux-amd64 builders.
The filed bug is #38608.

Fixes #36437

Change-Id: I9645ef73177c286c99547e3a0f3719fa07b35cb5
Reviewed-on: https://go-review.googlesource.com/c/go/+/229357
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2020-04-30 01:17:47 +00:00
Matthew Dempsky
a44d06d3b4 cmd/compile: use fixVariadicCall in escape analysis
This CL uses fixVariadicCall before escape analyzing function calls.
This has a number of benefits, though also some minor obstacles:

Most notably, it allows us to remove ODDDARG along with the logic
involved in setting it up, manipulating EscHoles, and later copying
its escape analysis flags to the actual slice argument. Instead, we
uniformly handle all variadic calls the same way. (E.g., issue31573.go
is updated because now f() and f(nil...) are handled identically.)

It also allows us to simplify handling of builtins and generic
function calls. Previously handling of calls was hairy enough to
require multiple dispatches on n.Op, whereas now the logic is uniform
enough that we can easily handle it with a single dispatch.

The downside is handling //go:uintptrescapes is now somewhat clumsy.
(It used to be clumsy, but it still is, too.) The proper fix here is
probably to stop using escape analysis tags for //go:uintptrescapes
and unsafe-uintptr, and have an earlier pass responsible for them.

Finally, note that while we now call fixVariadicCall in Escape, we
still have to call it in Order, because we don't (yet) run Escape on
all compiler-generated functions. In particular, the generated "init"
function for initializing package-level variables can contain calls to
variadic functions and isn't escape analyzed.

Passes toolstash-check -race.

Change-Id: I4cdb92a393ac487910aeee58a5cb8c1500eef881
Reviewed-on: https://go-review.googlesource.com/c/go/+/229759
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
2020-04-23 22:02:12 +00:00
Matthew Dempsky
2a2423bd05 cmd/compile: more precise analysis of method values
Previously for a method value "x.M", we always flowed x directly to
the heap, which led to the receiver argument generally needing to be
heap allocated.

This CL changes it to flow x to the closure and M's receiver
parameter. This allows receiver arguments to be stack allocated as
long as (1) the closure never escapes, *and* (2) method doesn't leak
its receiver parameter.

Within the standard library, this allows a handful of objects to be
stack allocated instead. Listed here are diagnostics that were
previously emitted by "go build -gcflags=-m std cmd" that are no
longer emitted:

archive/tar/writer.go:118:6: moved to heap: f
archive/tar/writer.go:208:6: moved to heap: f
archive/tar/writer.go:248:6: moved to heap: f
cmd/compile/internal/gc/initorder.go:252:2: moved to heap: d
cmd/compile/internal/gc/initorder.go:75:2: moved to heap: s
cmd/go/internal/generate/generate.go:206:7: &Generator literal escapes to heap
cmd/internal/obj/arm64/asm7.go:910:2: moved to heap: c
cmd/internal/obj/mips/asm0.go:415:2: moved to heap: c
cmd/internal/obj/pcln.go:294:22: new(pcinlineState) escapes to heap
cmd/internal/obj/s390x/asmz.go:459:2: moved to heap: c
crypto/tls/handshake_server.go:56:2: moved to heap: hs

Thanks to Cuong Manh Le for help coming up with this solution.

Fixes #27557.

Change-Id: I8c85d671d07fb9b53e11d2dd05949a34dbbd7e17
Reviewed-on: https://go-review.googlesource.com/c/go/+/228263
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-04-21 20:49:34 +00:00
Josh Bleecher Snyder
29d925dfcf test: add test for nil check / bounds check compiler confusion
This test started failing at CL 228106 and was fixed by CL 228677.

Fixes #38496

Change-Id: I2dadcd99227347e8d28179039f5f345e728c4595
Reviewed-on: https://go-review.googlesource.com/c/go/+/228698
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
2020-04-17 04:26:54 +00:00
Michael Munday
334d410ae3 cmd/compile: fix incorrect block for s390x Select1 op
When inserting Select0 and Select1 ops we need to ensure that they
live in the same block as their argument. This is because they need
to be scheduled immediately after their argument for register and
flag allocation to work correctly.

Fixes #38356.

Change-Id: Iba384dbe87010f1c7c4ce909f08011e5f1de7fd5
Reviewed-on: https://go-review.googlesource.com/c/go/+/227879
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
2020-04-14 19:01:47 +00:00
Keith Randall
83e288f3db cmd/compile: prevent constant folding of +/- when result is NaN
Missed as part of CL 221790. It isn't just * and / that can make NaNs.

Update #36400
Fixes #38359

Change-Id: I3fa562f772fe03b510793a6dc0cf6189c0c3e652
Reviewed-on: https://go-review.googlesource.com/c/go/+/227860
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alberto Donizetti <alb.donizetti@gmail.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
2020-04-10 19:32:41 +00:00
Ian Lance Taylor
162f1bf7c2 test: add test case that gccgo failed to compile
Change-Id: I08ca5f77b7352fe3ced1fbe3d027d6f5b4828e35
Reviewed-on: https://go-review.googlesource.com/c/go/+/227783
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
2020-04-10 18:42:07 +00:00
Austin Clements
da8591b61c all: remove darwin/arm build-tags and files
This removes all files that are only used on darwin/arm and cleans up
build tags in files that are still used on other platforms.

Updates #37611.

Change-Id: Ic9490cf0edfc157c6276a7ca950c1768b34a998f
Reviewed-on: https://go-review.googlesource.com/c/go/+/227197
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2020-04-08 18:35:43 +00:00
Cuong Manh Le
e9850462aa cmd/compile: don't inline reflect.Value.UnsafeAddr/Pointer if enable checkptr
Fixes #35073

Change-Id: I4b555bbc33d39a97544e6dd9c61d95ae212f472b
Reviewed-on: https://go-review.googlesource.com/c/go/+/222878
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2020-04-01 06:12:05 +00:00
Cuong Manh Le
7b30a2d268 cmd/compile: make isSmallMakeSlice checks slice cap only
If slice cap is not set, it will be equal to slice len. So
isSmallMakeSlice only needs to check whether slice cap is constant.

While at it, also add test to make sure panicmakeslicecap is called
when make slice contains invalid non-constant len.

For this benchmark:

func BenchmarkMakeSliceNonConstantLen(b *testing.B) {
	len := 1
	for i := 0; i < b.N; i++ {
		s := make([]int, len, 2)
		_ = s

	}
}

Result compare with parent:

name                        old time/op  new time/op  delta
MakeSliceNonConstantLen-12  18.4ns ± 1%   0.2ns ± 2%  -98.66%  (p=0.008 n=5+5)

Fixes #37975

Change-Id: I4bc926361bc2ffeab4cfaa888ef0a30cbc3b80e8
Reviewed-on: https://go-review.googlesource.com/c/go/+/226278
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
2020-03-31 21:51:51 +00:00
Matthew Dempsky
34314280e4 cmd/compile: fix constant conversion involving complex types
In CL 187657, I refactored constant conversion logic without realizing
that conversions between int/float and complex types are allowed for
constants (assuming the constant values are representable by the
destination type), but are never allowed for non-constant expressions.

This CL expands convertop to take an extra srcConstant parameter to
indicate whether the source expression is a constant; and if so, to
allow any numeric-to-numeric conversion. (Conversions of values that
cannot be represented in the destination type are rejected by
evconst.)

Fixes #38117.

Change-Id: Id7077d749a14c8fd910be38da170fa5254819f2b
Reviewed-on: https://go-review.googlesource.com/c/go/+/226197
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2020-03-31 20:59:14 +00:00
alex-semenyuk
82047a080f test, test/fixedbugs, crypto/x509, go/internal/gccgoimporter: fix typos
Change-Id: Ie2d605ca8cc3bde2e26c6865642ff4e6412cd075
GitHub-Last-Rev: ce5c3ba369
GitHub-Pull-Request: golang/go#38137
Reviewed-on: https://go-review.googlesource.com/c/go/+/226201
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2020-03-29 17:12:56 +00:00
Bryan C. Mills
91b8b130dd test: make runindir tests pass regardless of whether module mode is in use
The "runindir" tests used "go run", but relied on relative imports
(which are not supported by "go run" in module mode). Instead, such
tests must use fully-qualified imports, which require either a go.mod
file (in module mode) or that the package be in an appropriate
subdirectory of GOPATH/src (in GOPATH mode).

To set up such a directory, we use yet another copy of the same
overlayDir function currently found in the misc subdirectory of this
repository.

Fixes #33912
Updates #30228

Change-Id: If3d7ea2f7942ba496d98aaaf24a90bcdcf4df9f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/225205
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2020-03-25 14:19:25 +00:00
Cuong Manh Le
6bed304244 test: fix issue 15992 test wrong function call
Change-Id: I623ae5faffce456b34d97a2a0aa277ecbf1990f0
Reviewed-on: https://go-review.googlesource.com/c/go/+/217699
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2020-03-11 06:06:20 +00:00
Keith Randall
2b8e60d464 runtime: make typehash match compiler generated hashes exactly
If typehash (used by reflect) does not match the built-in map's hash,
then problems occur. If a map is built using reflect, and then
assigned to a variable of map type, the hash function can change. That
causes very bad things.

This issue is rare. MapOf consults a cache of all types that occur in
the binary before making a new one. To make a true new map type (with
a hash function derived from typehash) that map type must not occur in
the binary anywhere. But to cause the bug, we need a variable of that
type in order to assign to it. The only way to make that work is to
use a named map type for the variable, so it is distinct from the
unnamed version that MapOf looks for.

Fixes #37716

Change-Id: I3537bfceca8cbfa1af84202f432f3c06953fe0ed
Reviewed-on: https://go-review.googlesource.com/c/go/+/222357
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2020-03-10 16:26:59 +00:00
Josh Bleecher Snyder
5fac45a320 cmd/compile: use only bit patterns in isNonNegative
CL 212777 added a check to isNonNegative
to return true for unsigned values.
However, the SSA backend isn't type safe
enough for that to be sound.
The other checks in isNonNegative
look only at the pattern of bits.
Remove the type-based check.

Updates #37753

Change-Id: I059d0e86353453133f2a160dce53af299f42e533
Reviewed-on: https://go-review.googlesource.com/c/go/+/222620
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
2020-03-09 20:19:25 +00:00
Robert Griesemer
bfb903f252 cmd/compile/internal/syntax: better scanner error messages
This is one of several changes that were part of a larger rewrite
which I made in early 2019 after switching to the new number literal
syntax implementation. The purpose of the rewrite was to simplify
reading of source code (Unicode character by character) and speed up
the scanner but was never submitted for review due to other priorities.

Part 2 of 3:

This change contains improvements to the scanner error messages:

- Use "rune literal" rather than "character literal" to match the
  spec nomenclature.

- Shorter, more to the point error messages.
  (For instance, "more than one character in rune literal" rather
  than "invalid character literal (more than one character)", etc.)

Change-Id: I1aaf79003374a68dbb05926437ed305cf2a8ec96
Reviewed-on: https://go-review.googlesource.com/c/go/+/221602
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2020-03-05 00:40:38 +00:00
Keith Randall
63f1bc5992 runtime: print instruction bytes when reporting a SIGILL
Print the bytes of the instruction that generated a SIGILL.
This should help us respond to bug reports without having to
go back-and-forth with the reporter to get the instruction involved.
Might also help with SIGILL problems that are difficult to reproduce.

Update #37513

Change-Id: I33059b1dbfc97bce16142a843f32a88a6547e280
Reviewed-on: https://go-review.googlesource.com/c/go/+/221431
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2020-03-02 18:46:06 +00:00
TennyZhuang
ee46f135a9 cmd/compile: output cost while inlining function with Debug['m'] > 1
The existing implementation outputs inline cost iff function cannot be inlined with Debug['m'] > 1, the cost info is also useful if the function is inlineable.

Fixes #36780

Change-Id: Ic96f6baf96aee25fb4b33d31d4d644dc2310e536
Reviewed-on: https://go-review.googlesource.com/c/go/+/216778
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
2020-02-26 14:44:24 +00:00
Keith Randall
4f074b58d2 runtime/cgo: fix unsetenv wrapper
The wrapper takes a pointer to the argument, not the argument itself.

Fixes #36705

Change-Id: I566d4457d00bf5b84e4a8315a26516975f0d7e10
Reviewed-on: https://go-review.googlesource.com/c/go/+/215942
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2020-02-24 21:32:48 +00:00
Keith Randall
bc98e35b53 cmd/compile: avoid memmove -> SSA move rewrite when size is negative
We should panic in this situation. Rewriting to a SSA op just leads
to a compiler panic.

Fixes #36259

Change-Id: I6e0bccbed7dd0fdac7ebae76b98a211947947386
Reviewed-on: https://go-review.googlesource.com/c/go/+/212405
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
2020-02-24 20:23:14 +00:00
Ian Lance Taylor
059a5ac3eb test: add test that gccgo fails to compile
Change-Id: I14d31bb62fd0e26a84f8781644c40918404f5f3e
Reviewed-on: https://go-review.googlesource.com/c/go/+/213899
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2020-02-22 04:31:41 +00:00
Cuong Manh Le
af686da46f cmd/compile: allow print/println(f()) where f() is multi-value
Fixes #35576

Change-Id: Ia128f412120069e9f8813e9e910e62644f950bfb
Reviewed-on: https://go-review.googlesource.com/c/go/+/207118
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2020-02-22 03:34:22 +00:00
Robert Griesemer
ffc0573b85 cmd/compile/internal/syntax: better error when an assignment is used in value context
The error message is now positioned at the statement position (which is
an identifing token, such as the '=' for assignments); and in case of
assignments it emphasizes the assignment by putting the Lhs and Rhs
in parentheses. Finally, the wording is changed from "use of * as value"
to the stronger "cannot use * as value" (for which there is precedent
elsewhere in the parser).

Fixes #36858.

Change-Id: Ic3f101bba50f58e3a1d9b29645066634631f2d61
Reviewed-on: https://go-review.googlesource.com/c/go/+/218337
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2020-02-21 22:57:52 +00:00
Cherry Zhang
6fbdfe4804 cmd/compile: on PPC64, fold offset into some loads/stores only when offset is 4-aligned
On PPC64, MOVWload, MOVDload, and MOVDstore are assembled to a
"DS from" instruction which requiers the offset is a multiple of
4. Only fold offset to such instructions if it is a multiple of 4.

Fixes #36723.

"GOARCH=ppc64 GOOS=linux go build -gcflags=all=-d=ssa/check/on std cmd"
passes now.

Change-Id: I67f2a6ac02f0d33d470f68ff54936c289a4c765b
Reviewed-on: https://go-review.googlesource.com/c/go/+/216379
Reviewed-by: Carlos Eduardo Seo <cseo@linux.vnet.ibm.com>
2020-01-27 16:13:58 +00:00
Joel Sing
7f331e0e17 test: adjust tests for riscv64
This disables some tests that are unsupported on riscv64 and adds support
for risc64 to test/nosplit.

Updates #27532, #36739 and #36765

Change-Id: I0a57797a05bc80236709fc240c0a0efb0ee0d16b
Reviewed-on: https://go-review.googlesource.com/c/go/+/216263
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2020-01-25 16:30:26 +00:00
Tobias Klauser
df2999ef43 test: disable test for #36516 when cgo is not enabled
CL 214679 added a -race test which shouldn't be run when cgo is not
enabled.

Fixes the nocgo builder.

Change-Id: Iceddf802c4ef6c0de2c3a968e86342303d2d27d8
Reviewed-on: https://go-review.googlesource.com/c/go/+/215477
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-19 20:36:10 +00:00
Keith Randall
316fd8cc4a cmd/compile: mark ... argument to checkptrArithmetic as not escaping
Fixes #36516

Change-Id: Ibf4f86fb3a25fa30e0cd54e2dd2e12c60ee75ddb
Reviewed-on: https://go-review.googlesource.com/c/go/+/214679
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2020-01-17 17:38:40 +00:00
Keith Randall
2248fc63ab cmd/compile: give every really deep type a unique name
This avoids the security problem in #29312 where two very deep, but
distinct, types are given the same name. They both make it to the
linker which chooses one, and the use of the other is now type unsafe.

Instead, give every very deep type its own name. This errs on the
other side, in that very deep types that should be convertible to each
other might now not be. But at least that's not a security hole.

Update #29312.

Change-Id: Iac0ebe73fdc50594fd6fbf7432eef65f9a053126
Reviewed-on: https://go-review.googlesource.com/c/go/+/213517
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
2020-01-08 18:43:55 +00:00
Cherry Zhang
6f2b8347b1 test: add a test for gccgo compiler bug of missing type descriptor
The gccgo compiler did not generate type descriptor for a pointer
to a type alias defined in another package, causing linking error.
The fix is CL 210787. This CL adds a test.

Updates #36085.

Change-Id: I3237c7fedb4d92fb2dc610ee2b88087f96dc2a1a
Reviewed-on: https://go-review.googlesource.com/c/go/+/210858
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2019-12-11 19:48:39 +00:00
Brad Fitzpatrick
a6c8fac781 os: skip a new failing test on Windows
This test was recently added in CL 209961.

Apparently Windows can't seek a directory filehandle?

And move the test from test/fixedbugs (which is mostly for compiler bugs) to
an os package test.

Updates #36019

Change-Id: I626b69b0294471014901d0ccfeefe5e2c7651788
Reviewed-on: https://go-review.googlesource.com/c/go/+/210283
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
2019-12-06 18:04:42 +00:00
Keith Randall
e3c7ffcd95 os: reset dirinfo when seeking on Darwin
The first Readdirnames calls opendir and caches the result.
The behavior of that cached opendir result isn't specified on a seek
of the underlying fd. Free the opendir result on a seek so that
we'll allocate a new one the next time around.

Also fix wasm behavior in this regard, so that a seek to the
file start resets the Readdirnames position, regardless of platform.

p.s. I hate the Readdirnames API.

Fixes #35767.

Change-Id: Ieffb61b3c5cdd42591f69ab13f932003966f2297
Reviewed-on: https://go-review.googlesource.com/c/go/+/209961
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-12-05 20:40:38 +00:00
David Chase
0e02cfb369 cmd/compile: try harder to not use an empty src.XPos for a bogus line
The fix for #35652 did not guarantee that it was using a non-empty
src position to replace an empty one.  The new code checks again
and falls back to a more certain position.  (The input in question
compiles to a single empty infinite loop, and none of the actual instructions
had any source position at all.  That is a bug, but given the pathology
of this input, not one worth dealing with this late in the release cycle,
if ever.)

Literally:

00000 (5) TEXT "".f(SB), ABIInternal
00001 (5) PCDATA $0, $-2
00002 (5) PCDATA $1, $-2
00003 (5) FUNCDATA $0, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
00004 (5) FUNCDATA $1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
00005 (5) FUNCDATA $2, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
b2
00006 (?) XCHGL AX, AX
b6
00007 (+1048575) JMP 6
00008 (?) END

TODO: Add runtime.InfiniteLoop(), replace infinite loops with a call to
that, and use an eco-friendly runtime.gopark instead.  (This was Cherry's
excellent idea.)

Updates #35652
Fixes #35695

Change-Id: I4b9a841142ee4df0f6b10863cfa0721a7e13b437
Reviewed-on: https://go-review.googlesource.com/c/go/+/207964
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2019-11-22 03:06:22 +00:00
David Chase
9bba63bbbe cmd/compile: make a better bogus line for empty infinite loops
The old recipe for making an infinite loop not be infinite
in the debugger could create an instruction (Prog) with a
line number not tied to any file (index == 0).  This caused
downstream failures in DWARF processing.

So don't do that.  Also adds a test, also adds a check+panic
to ensure that the next time this happens the error is less
mystifying.

Fixes #35652

Change-Id: I04f30bc94fdc4aef20dd9130561303ff84fd945e
Reviewed-on: https://go-review.googlesource.com/c/go/+/207613
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2019-11-19 00:38:53 +00:00
Than McIntosh
a174a3aea9 test: new test for gollvm compiler crash bug
Reduced test case for gollvm compiler crash building docker-ce.

Updates #35586.

Change-Id: Ib805dc9ab7b63cc61f207f1f000bef9809cfd428
Reviewed-on: https://go-review.googlesource.com/c/go/+/207258
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2019-11-18 21:19:53 +00:00
Bryan C. Mills
afac2c0508 test: avoid writing temporary files to GOROOT
This reverts CL 207477, restoring CL 207352 with a fix for the
regression observed in the Windows builders.

cmd/compile evidently does not fully support NUL as an output on
Windows, so this time we write ignored 'compile' outputs
to temporary files (instead of os.DevNull as in CL 207352).

Updates #28387
Fixes #35619

Change-Id: I2edc5727c3738fa1bccb4b74e50d114cf2a7fcff
Reviewed-on: https://go-review.googlesource.com/c/go/+/207602
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-11-18 14:40:07 +00:00