The revised test now checks that unsafe-uintptr correctly works for
variadic uintptr parameters too, and the CL corrects the code so this
code compiles again.
The pointers are still not kept alive properly. That will be fixed by
a followup CL. But this CL at least allows programs not affected by
that to build again.
Updates #24991.
Updates #41460.
Change-Id: If4c39167b6055e602213fb7522c4f527c43ebda9
Reviewed-on: https://go-review.googlesource.com/c/go/+/255877
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
They can't reasonably be allocated on the heap. Not a huge deal, but
it has an interesting and useful side effect.
After CL 249917, the compiler and runtime treat pointers to
go:notinheap types as uintptrs instead of real pointers (no write
barrier, not processed during stack scanning, ...). That feature is
exactly what we want for cgo to fix#40954. All the cases we have of
pointers declared in C, but which might actually be filled with
non-pointer data, are of this form (JNI's jobject heirarch, Darwin's
CFType heirarchy, ...).
Fixes#40954
Change-Id: I44a3b9bc2513d4287107e39d0cbbd0efd46a3aae
Reviewed-on: https://go-review.googlesource.com/c/go/+/250940
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
runtime.GC() doesn't guarantee the finalizer has run, so use a channel
instead to make sure finalizer was run in call to "after()".
Fixes#41361
Change-Id: I69c801e29aea49757ea72c52e8db13239de19ddc
Reviewed-on: https://go-review.googlesource.com/c/go/+/254401
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
So we can insert theses OVARLIVE nodes right after OpStaticCall in SSA.
This helps fixing issue that unsafe-uintptr arguments are not kept alive
during return statement, or can be kept alive longer than expected.
Fixes#24491
Change-Id: Ic04a5d1bbb5c90dcfae65bd95cdd1da393a66800
Reviewed-on: https://go-review.googlesource.com/c/go/+/254397
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This CL changes "T literal.M" error message to "T{...}.M". It's clearer
expression and focusing user on actual issue.
Updates #38745
Change-Id: I84b455a86742f37e0bde5bf390aa02984eecc3c9
Reviewed-on: https://go-review.googlesource.com/c/go/+/253677
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Currently, the statement:
go g(uintptr(f()))
gets rewritten into:
tmp := f()
newproc(8, g, uintptr(tmp))
runtime.KeepAlive(tmp)
which doesn't guarantee that tmp is still alive by time the g call is
scheduled to run.
This CL fixes the issue, by wrapping g call in a closure:
go func(p unsafe.Pointer) {
g(uintptr(p))
}(f())
then this will be rewritten into:
tmp := f()
go func(p unsafe.Pointer) {
g(uintptr(p))
runtime.KeepAlive(p)
}(tmp)
runtime.KeepAlive(tmp) // superfluous, but harmless
So the unsafe.Pointer p will be kept alive at the time g call runs.
Updates #24491
Change-Id: Ic10821251cbb1b0073daec92b82a866c6ebaf567
Reviewed-on: https://go-review.googlesource.com/c/go/+/253457
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Flag constant Ops on arm and arm64 are under refactoring, this change adds
a couple of testcases that verify the behavior of 'noov' branches.
Updates #39505
Updates #38740
Updates #39303
Change-Id: I493344b52276900cd296c32da494d72932dfc9be
Reviewed-on: https://go-review.googlesource.com/c/go/+/238677
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We're using sort.SliceStable, so no need to keep track of indexes as well.
Use a more robust test for whether a node is a call.
Add a test that we're actually reordering comparisons. This test fails
without the alg.go changes in this CL because eqstring uses OCALLFUNC
instead of OCALL for its data comparisons.
Update #8606
Change-Id: Ieeec33434c72e3aa328deb11cc415cfda05632e2
Reviewed-on: https://go-review.googlesource.com/c/go/+/237921
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
go1.14 drop nacl support, as go1.15 was released, go1.13 is not
supported anymore, nacl is absolutely gone.
Change-Id: I05efb46891ec875b08da8f2996751a8e9cb57d0c
Reviewed-on: https://go-review.googlesource.com/c/go/+/249977
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
checkptr has code to recognize &^ expressions, but it didn't take into
account that "p &^ x" gets rewritten to "p & ^x" during walk, which
resulted in false positive diagnostics.
This CL changes walkexpr to mark OANDNOT expressions with Implicit
when they're rewritten to OAND, so that walkCheckPtrArithmetic can
still recognize them later.
It would be slightly more idiomatic to instead mark the OBITNOT
expression as Implicit (as it's a compiler-generated Node), but the
OBITNOT expression might get constant folded. It's not worth the extra
complexity/subtlety of relying on n.Right.Orig, so we set Implicit on
the OAND node instead.
To atone for this transgression, I add documentation for nodeImplicit.
Fixes#40917.
Change-Id: I386304171ad299c530e151e5924f179e9a5fd5b8
Reviewed-on: https://go-review.googlesource.com/c/go/+/249477
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Makes sure the copyright notice is not interpreted as the package level
godoc.
Change-Id: I2afce7c9d620f19d51ec1438b1d0db1774b57146
Reviewed-on: https://go-review.googlesource.com/c/go/+/248760
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dave Cheney <dave@cheney.net>
The previous array length was large enough to exceed
maxImplicitStackSize on 64-bit architectures, but not on 32-bit
architectures.
Fixes#40808.
Change-Id: I69e9abb447454b2e7875ba503a0cb772e965ae31
Reviewed-on: https://go-review.googlesource.com/c/go/+/248680
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently, generated struct wrapper for closure is not handled in
mustHeapAlloc. That causes compiler crashes when the wrapper struct
is too large for stack, and must be heap allocated instead.
Fixes#39292
Change-Id: I14c1e591681d9d92317bb2396d6cf5207aa93e08
Reviewed-on: https://go-review.googlesource.com/c/go/+/244917
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
CL 230121 fixed the bug that struct literal blank fields type array/struct
can not be initialized. But it still misses some cases when an expression
causes "candiscard(value)" return false. When these happen, we recursively
call fixedlit with "var_" set to "_", and hit the bug again.
To fix it, just making splitnode return "nblank" whenever "var_" is "nblank".
Fixes#38905
Change-Id: I281941b388acbd551a4d8ca1a235477f8d26fb6e
Reviewed-on: https://go-review.googlesource.com/c/go/+/232617
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Taking the live variable set from the last return point is problematic.
See #40629 for details, but there may not be a return point, or it may
be before the final defer.
Additionally, keeping track of the last call as a *Value doesn't quite
work. If it is dead-code eliminated, the storage for the Value is reused
for some other random instruction. Its live variable information,
if it is available at all, is wrong.
Instead, just mark all the open-defer argument slots as live
throughout the function. (They are already zero-initialized.)
Fixes#40629
Change-Id: Ie456c7db3082d0de57eaa5234a0f32525a1cce13
Reviewed-on: https://go-review.googlesource.com/c/go/+/247522
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>
Currently in addLocalInductiveFacts, we only check whether
direct edge from if block to phi block exists. If not, the
following logic will treat the phi block as the first successor,
which is wrong.
This patch makes prove pass more conservative, so we disable
some cases in test/prove.go. We will do some optimization in
the following CL and enable these cases then.
Fixes#40367.
Change-Id: I27cf0248f3a82312a6f7dabe11c79a1a34cf5412
Reviewed-on: https://go-review.googlesource.com/c/go/+/244579
Reviewed-by: Zach Jones <zachj1@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The ICE reported as #33308 was fixed by a related CL; this change adds
a regression test with the crasher.
Fixes#33308
Change-Id: I3260075dbe3823b56b8825e6269e57a0fad185a6
Reviewed-on: https://go-review.googlesource.com/c/go/+/243458
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
It doesn't have to. The type in the aux field is authoritative.
There are cases involving casting from interface{} where pointers
have a placeholder pointer type (because the type is not known when
the IData op is generated).
The check was introduced in CL 13447.
Fixes#39459
Change-Id: Id77a57577806a271aeebd20bea5d92d08ee7aa6b
Reviewed-on: https://go-review.googlesource.com/c/go/+/239817
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
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>
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>
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>
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>
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>
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>
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#38093Fixes#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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
Missed as part of CL 221790. It isn't just * and / that can make NaNs.
Update #36400Fixes#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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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 #35652Fixes#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>
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>
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 #28387Fixes#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>
This CL detects infinite loops due to negative dereference cycles
during escape analysis, and terminates the loop gracefully. We still
fail to print a complete explanation of the escape path, but esc.go
didn't print *any* explanation for these test cases, so the release
blocking issue here is simply that we don't infinite loop.
Updates #35518.
Change-Id: I39beed036e5a685706248852f1fa619af3b7abbc
Reviewed-on: https://go-review.googlesource.com/c/go/+/206619
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Unlike function calls, when processing instructions that directly
fault we must not subtract 1 from the pc before looking up the
file/line information.
Since the file/line lookup unconditionally subtracts 1, add 1 to
the faulting instruction PCs to compensate.
Fixes#34123
Change-Id: Ie7361e3d2f84a0d4f48d97e5a9e74f6291ba7a8b
Reviewed-on: https://go-review.googlesource.com/c/go/+/196962
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
When we do a successful recover of a panic, we resume normal execution by
returning from the frame that had the deferred call that did the recover (after
executing any remaining deferred calls in that frame).
However, suppose we have called runtime.Goexit and there is a panic during one of the
deferred calls run by the Goexit. Further assume that there is a deferred call in
the frame of the Goexit or a parent frame that does a recover. Then the recovery
process will actually resume normal execution above the Goexit frame and hence
abort the Goexit. We will not terminate the thread as expected, but continue
running in the frame above the Goexit.
To fix this, we explicitly create a _panic object for a Goexit call. We then
change the "abort" behavior for Goexits, but not panics. After a recovery, if the
top-level panic is actually a Goexit that is marked to be aborted, then we return
to the Goexit defer-processing loop, so that the Goexit is not actually aborted.
Actual code changes are just panic.go, runtime2.go, and funcid.go. Adjusted the
test related to the new Goexit behavior (TestRecoverBeforePanicAfterGoexit) and
added several new tests of aborted panics (whose behavior has not changed).
Fixes#29226
Change-Id: Ib13cb0074f5acc2567a28db7ca6912cfc47eecb5
Reviewed-on: https://go-review.googlesource.com/c/go/+/200081
Run-TryBot: Dan Scales <danscales@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
"declared and not used" is technically correct, but might confuse
the user. Switching "and" to "but" will hopefully create the
contrast for the users: they did one thing (declaration), but
not the other --- actually using the variable.
This new message is still not ideal (specifically, declared is not
entirely precise here), but at least it matches the other parsers
and is one step in the right direction.
Change-Id: I725c7c663535f9ab9725c4b0bf35b4fa74b0eb20
Reviewed-on: https://go-review.googlesource.com/c/go/+/203282
Run-TryBot: Minux Ma <minux@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Updates #35157 (the bug there was fixed by CL200861)
Change-Id: I67069207b4cdc2ad4a475dd0bbc8555ecc5f534f
Reviewed-on: https://go-review.googlesource.com/c/go/+/203598
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alberto Donizetti <alb.donizetti@gmail.com>
We need to explicitly convert pointers to unsafe.Pointer before
passing to the runtime checkptr instrumentation in case the user
declared their own type with underlying type unsafe.Pointer.
Updates #22218.
Fixes#34966.
Change-Id: I3baa2809d77f8257167cd78f57156f819130baa8
Reviewed-on: https://go-review.googlesource.com/c/go/+/201782
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
When a subsequent load/store of a ptr makes the nil check of that pointer
unnecessary, if their lines differ, change the line of the load/store
to that of the nilcheck, and attempt to rehome the load/store position
instead.
This fix makes profiling less accurate in order to make panics more
informative.
Fixes#33724
Change-Id: Ib9afaac12fe0d0320aea1bf493617facc34034b3
Reviewed-on: https://go-review.googlesource.com/c/go/+/200197
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Add to the testcase originally created for issue 34577 so
as to also trigger the error condition for issue 34852 (the
two bugs are closely related).
Updates #34577.
Updates #34852.
Change-Id: I2347369652ce500184347606b2bb3e76d802b204
Reviewed-on: https://go-review.googlesource.com/c/go/+/201017
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Increases the exec timeout from 5sec to 1min, but
also print out the error value on any test failure.
Fixes#34836
Change-Id: Ida2b8bd460243491ef0f90dfe0f978dfe02a0703
Reviewed-on: https://go-review.googlesource.com/c/go/+/200519
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Test case with code that caused a gccgo error while emitting export
data for an inlinable function.
Updates #34577.
Change-Id: I28b598c4c893c77f4a76bb4f2d27e5b42f702992
Reviewed-on: https://go-review.googlesource.com/c/go/+/198057
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 170950 had a regression that makes the compiler produce
an invalid wasm binary if the data section is too large.
Loading such a binary gives the following error:
"LinkError: WebAssembly.instantiate(): data segment is out of bounds"
This change fixes the issue by ensuring that the minimum size of the
linear memory is larger than the end of the data section.
Fixes#34395.
Change-Id: I0c8629de7ffd0d85895ad31bf8c9d45fef197a57
Reviewed-on: https://go-review.googlesource.com/c/go/+/199358
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We're allowed to remove a write barrier when both the old
value in memory and the new value we're writing are not heap pointers.
Improve both those checks a little bit.
A pointer is known to not be a heap pointer if it is read from
read-only memory. This sometimes happens for loads of pointers
from string constants in read-only memory.
Do a better job of tracking which parts of memory are known to be
zero. Before we just kept track of a range of offsets in the most
recently allocated object. For code that initializes the new object's
fields in a nonstandard order, that tracking is imprecise. Instead,
keep a bit map of the first 64 words of that object, so we can track
precisely what we know to be zeroed.
The new scheme is only precise up to the first 512 bytes of the object.
After that, we'll use write barriers unnecessarily. Hopefully most
initializers of large objects will use typedmemmove, which does only one
write barrier check for the whole initialization.
Fixes#34723
Update #21561
Change-Id: Idf6e1b7d525042fb67961302d4fc6f941393cac8
Reviewed-on: https://go-review.googlesource.com/c/go/+/199558
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
CL 188317 introduced a compiler crash during dwarf generation which
was reported as Issue #34520. After CL 188217, the issue appears to be
fixed. Add a testcase to avoid future regressions.
Fixes#34520
Change-Id: I73544a9e9baf8dbfb85c19eb6d202beea05affb6
Reviewed-on: https://go-review.googlesource.com/c/go/+/198546
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
