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>
By lazily starting the signal watch loop only on Notify,
we are able to have deadlock detection even when
"os/signal" is imported.
Thanks to Ian Lance Taylor for the solution and discussion.
With this change in, fix a runtime gorountine count test that
assumed that os/signal.init would unconditionally start the
signal watching goroutine, but alas no more.
Fixes#21576.
Change-Id: I6eecf82a887f59f2ec8897f1bcd67ca311ca42ff
Reviewed-on: https://go-review.googlesource.com/c/go/+/101036
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This commit just adds a regress test for a few of the important corner
cases that I identified in #27557, which turn out to not be tested
anywhere.
While here, annotate a few of the existing test cases where we could
improve escape analysis.
Updates #27557.
Change-Id: Ie57792a538f7899bb17915485fabc86100f469a3
Reviewed-on: https://go-review.googlesource.com/c/go/+/197137
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
golang.org/cl/109517 optimized the compiler to avoid the allocation for make in
append(x, make([]T, y)...). This was only implemented for the case that y has type int.
This change extends the optimization to trigger for all integer types where the value
is known at compile time to fit into an int.
name old time/op new time/op delta
ExtendInt-12 106ns ± 4% 106ns ± 0% ~ (p=0.351 n=10+6)
ExtendUint64-12 1.03µs ± 5% 0.10µs ± 4% -90.01% (p=0.000 n=9+10)
name old alloc/op new alloc/op delta
ExtendInt-12 0.00B 0.00B ~ (all equal)
ExtendUint64-12 13.6kB ± 0% 0.0kB -100.00% (p=0.000 n=10+10)
name old allocs/op new allocs/op delta
ExtendInt-12 0.00 0.00 ~ (all equal)
ExtendUint64-12 1.00 ± 0% 0.00 -100.00% (p=0.000 n=10+10)
Updates #29785
Change-Id: Ief7760097c285abd591712da98c5b02bc3961fcd
Reviewed-on: https://go-review.googlesource.com/c/go/+/182559
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This CL expands the test for #29612 to check that type switches also
work correctly when type hashes collide.
Change-Id: Ia153743e6ea0736c1a33191acfe4d8ba890be527
Reviewed-on: https://go-review.googlesource.com/c/go/+/195782
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
