Consider this code:
func f(*int)
func g() {
p := new(int)
f(p)
}
where f is an assembly function.
In general liveness analysis assumes that during the call to f, p is dead
in this frame. If f has retained p, p will be found alive in f's frame and keep
the new(int) from being garbage collected. This is all correct and works.
We use the Go func declaration for f to give the assembly function
liveness information (the arguments are assumed live for the entire call).
Now consider this code:
func h1() {
p := new(int)
syscall.Syscall(1, 2, 3, uintptr(unsafe.Pointer(p)))
}
Here syscall.Syscall is taking the place of f, but because its arguments
are uintptr, the liveness analysis and the garbage collector ignore them.
Since p is no longer live in h once the call starts, if the garbage collector
scans the stack while the system call is blocked, it will find no reference
to the new(int) and reclaim it. If the kernel is going to write to *p once
the call finishes, reclaiming the memory is a mistake.
We can't change the arguments or the liveness information for
syscall.Syscall itself, both for compatibility and because sometimes the
arguments really are integers, and the garbage collector will get quite upset
if it finds an integer where it expects a pointer. The problem is that
these arguments are fundamentally untyped.
The solution we have taken in the syscall package's wrappers in past
releases is to insert a call to a dummy function named "use", to make
it look like the argument is live during the call to syscall.Syscall:
func h2() {
p := new(int)
syscall.Syscall(1, 2, 3, uintptr(unsafe.Pointer(p)))
use(unsafe.Pointer(p))
}
Keeping p alive during the call means that if the garbage collector
scans the stack during the system call now, it will find the reference to p.
Unfortunately, this approach is not available to users outside syscall,
because 'use' is unexported, and people also have to realize they need
to use it and do so. There is much existing code using syscall.Syscall
without a 'use'-like function. That code will fail very occasionally in
mysterious ways (see #13372).
This CL fixes all that existing code by making the compiler do the right
thing automatically, without any code modifications. That is, it takes h1
above, which is incorrect code today, and makes it correct code.
Specifically, if the compiler sees a foreign func definition (one
without a body) that has uintptr arguments, it marks those arguments
as "unsafe uintptrs". If it later sees the function being called
with uintptr(unsafe.Pointer(x)) as an argument, it arranges to mark x
as having escaped, and it makes sure to hold x in a live temporary
variable until the call returns, so that the garbage collector cannot
reclaim whatever heap memory x points to.
For now I am leaving the explicit calls to use in package syscall,
but they can be removed early in a future cycle (likely Go 1.7).
The rule has no effect on escape analysis, only on liveness analysis.
Fixes#13372.
Change-Id: I2addb83f70d08db08c64d394f9d06ff0a063c500
Reviewed-on: https://go-review.googlesource.com/18584
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Brief background on "why heap allocate". Things can be
forced to the heap for the following reasons:
1) address published, hence lifetime unknown.
2) size unknown/too large, cannot be stack allocated
3) multiplicity unknown/too large, cannot be stack allocated
4) reachable from heap (not necessarily published)
The bug here is a case of failing to enforce 4) when an
object Y was reachable from a heap allocation X forced
because of 3). It was found in the case of a closure
allocated within a loop (X) and assigned to a variable
outside the loop (multiplicity unknown) where the closure
also captured a map (Y) declared outside the loop (reachable
from heap). Note the variable declared outside the loop (Y)
is not published, has known size, and known multiplicity
(one). The only reason for heap allocation is that it was
reached from a heap allocated item (X), but because that was
not forced by publication, it has to be tracked by loop
level, but escape-loop level was not tracked and thus a bug
results.
The fix is that when a heap allocation is newly discovered,
use its looplevel as the minimum loop level for downstream
escape flooding.
Every attempt to generalize this bug to X-in-loop-
references-Y-outside loop succeeded, so the fix was aimed
to be general. Anywhere that loop level forces heap
allocation, the loop level is tracked. This is not yet
tested for all possible X and Y, but it is correctness-
conservative and because it caused only one trivial
regression in the escape tests, it is probably also
performance-conservative.
The new test checks the following:
1) in the map case, that if fn escapes, so does the map.
2) in the map case, if fn does not escape, neither does the map.
3) in the &x case, that if fn escapes, so does &x.
4) in the &x case, if fn does not escape, neither does &x.
Fixes#13799.
Change-Id: Ie280bef2bb86ec869c7c206789d0b68f080c3fdb
Reviewed-on: https://go-review.googlesource.com/18234
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Adding the evconst(n) call for OANDAND and OOROR in
golang.org/cl/18262 was originally just to parallel the above iscmp
branch, but upon further inspection it seemed odd that removing it
caused test/fixedbugs/issue6671.go's
var b mybool
// ...
b = bool(true) && true // ERROR "cannot use"
to start failing (i.e., by not emitting the expected "cannot use"
error).
The problem is that evconst(n)'s settrue and setfalse paths always
reset n.Type to idealbool, even for logical operators where n.Type
should preserve the operand type. Adding the evconst(n) call for
OANDAND/OOROR inadvertantly worked around this by turning the later
evconst(n) call at line 2167 into a noop, so the "n.Type = t"
assignment at line 739 would preserve the operand type.
However, that means evconst(n) was still clobbering n.Type for ONOT,
so declarations like:
const _ bool = !mybool(true)
were erroneously accepted.
Update #13821.
Change-Id: I18e37287f05398fdaeecc0f0d23984e244f025da
Reviewed-on: https://go-review.googlesource.com/18362
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
We don't use these for benchmarking anymore.
Now we have the go1 dir and the benchmarks subrepo.
Some have problematic copyright notices, so move out of main repo.
Preserved in golang.org/x/exp/shootout.
Fixes#12688.
Fixes#13584.
Change-Id: Ic0b71191ca1a286d33d7813aca94bab1617a1c82
Reviewed-on: https://go-review.googlesource.com/18320
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Added a format option to inhibit output of .Note field in
printing, and enabled that option during export.
Added test.
Fixes#13777.
Change-Id: I739f9785eb040f2fecbeb96d5a9ceb8c1ca0f772
Reviewed-on: https://go-review.googlesource.com/18217
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: David Chase <drchase@google.com>
The test for non-package main top-level inputs is done while parsing
the export data. Issue #13468 happened because we were not parsing
the export data when using compiler-generated archives
(that is, when using go tool compile -pack).
Fix this by parsing the export data even for archives.
However, that turns up a different problem: the export data check
reports (one assumes spurious) skew errors now, because it has
not been run since Go 1.2.
(Go 1.3 was the first release to use go tool compile -pack.)
Since the code hasn't run since Go 1.2, it can't be that important.
Since it doesn't work today, just delete it.
Figuring out how to make this code work with Robert's export
format was one of the largest remaining TODOs for that format.
Now we don't have to.
Fixes#13468 and makes the world a better place.
Change-Id: I40a4b284cf140d49d48b714bd80762d6889acdb9
Reviewed-on: https://go-review.googlesource.com/17976
Reviewed-by: Robert Griesemer <gri@golang.org>
Fixes#12411.
Change-Id: I2202a754c7750e3b2119e3744362c98ca0d2433e
Reviewed-on: https://go-review.googlesource.com/17818
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Another (historic) artifact due to partially resolving symbols too early.
Fixes#13539.
Change-Id: Ie720c491cfa399599454f384b3a9735e75d4e8f1
Reviewed-on: https://go-review.googlesource.com/17600
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Following an empty import, a declaration involving a ? symbol
generates an internal compiler error when the name of the
symbol (in newname function).
package a
import""
var?
go.go:2: import path is empty
go.go:3: internal compiler error: newname nil
Make sure dclname is not called when the symbol is nil.
The error message is now:
go.go:2: import path is empty
go.go:3: invalid declaration
go.go:4: syntax error: unexpected EOF
This CL was initially meant to be applied to the old parser,
and has been updated to apply to the new parser.
Fixes#11610
Change-Id: I75e07622fb3af1d104e3a38c89d9e128e3b94522
Reviewed-on: https://go-review.googlesource.com/15268
Reviewed-by: Russ Cox <rsc@golang.org>
The following code:
func n() {(interface{int})}
generates:
3: interface contains embedded non-interface int
3: type %!v(PANIC=runtime error: invalid memory address or nil pointer dereference) is not an expression
It is because the corresponding symbol (Sym field in Type object)
is nil, resulting in a panic in typefmt.
Just skip the symbol if it is nil, so that the error message becomes:
3: interface contains embedded non-interface int
3: type interface { int } is not an expression
Fixes#11614
Change-Id: I219ae7eb01edca264fad1d4a1bd261d026294b00
Reviewed-on: https://go-review.googlesource.com/14015
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
The build tags are necessary to keep "go build" in that directory
building only stdio.go, but we have to arrange for test/run.go to
treat them as satisfied.
Fixes#12625.
Change-Id: Iec0cb2fdc2c9b24a4e0530be25e940aa0cc9552e
Reviewed-on: https://go-review.googlesource.com/17454
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Move test for isblank into addmethod so that most of the type checking
for methods is also performed for blank methods.
Fixes#11366.
Change-Id: I13d554723bf96d906d0b3ff390d7b7c87c1a5020
Reviewed-on: https://go-review.googlesource.com/16866
Reviewed-by: Robert Griesemer <gri@golang.org>
- use same local variable name (lno) for line number for LCOLAS everywhere
- remove now unneeded assignment of line number to yylval.i in lexer
Fix per suggestion of mdempsky.
Fixes#13415.
Change-Id: Ie3c7f5681615042a12b81b26724b3a5d8a979c25
Reviewed-on: https://go-review.googlesource.com/17248
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This is a bit ugly but it's a useful test. Run go install -buildmode=shared std
and then go run run.go -linkshared (it passes on linux/amd64).
Change-Id: I5684c79cd03817fa1fc399788b7320f8535c08da
Reviewed-on: https://go-review.googlesource.com/16343
Reviewed-by: Russ Cox <rsc@golang.org>
- fix/check location of popdcl calls where questioned
- remove unnecessary handling of ... (LDDD) in ntype (couldn't be reached)
- inlined and fnret_type and simplified fnres as a consequence
- leave handling of ... (LDDD) in arg_list alone (remove TODO)
- verify that parser requires a ';' after last statement in a case/default
(added test case)
Fixes#13243.
Change-Id: Iad94b498591a5e85f4cb15bbc01e8e101415560d
Reviewed-on: https://go-review.googlesource.com/17155
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Chris Manghane <cmang@golang.org>
Use a combination of follow- and stop-token lists and nesting levels
to better synchronize parser after a syntax error.
Fixes#13319.
Change-Id: I9592e0b5b3ba782fb9f9315fea16163328e204f7
Reviewed-on: https://go-review.googlesource.com/17080
Reviewed-by: Chris Manghane <cmang@golang.org>
Handling of &(T{}) assumed that the parser would not introduce ()'s.
Also: Better comments around handling of OPAREN syntax tree optimization.
Fixes#13261.
Change-Id: Ifc5047a0448f5e7d74cd42f6608b87dcc9c2f2fb
Reviewed-on: https://go-review.googlesource.com/17040
Reviewed-by: Chris Manghane <cmang@golang.org>
Also:
- better error messages in some cases
- factored out function to produce syntax error at given line number
Fixes#13273.
Change-Id: I0192a94731cc23444680a26bd0656ef663e6da0b
Reviewed-on: https://go-review.googlesource.com/16992
Reviewed-by: Chris Manghane <cmang@golang.org>
This is a translation of the yacc-based parser with adjustements
to make the grammar work for a recursive-descent parser followed
by cleanups and simplifications.
The yacc actions were mostly literally copied for correctness
with better temporary names.
A few of the syntax tests were adjusted for slightly different
error messages (it is very difficult to match the yacc-based
error messages in all cases, and sometimes the new parser could
produce better errors).
The new parser is enabled by default.
To switch back to the yacc-based parser, set -oldparser.
To hardwire the switch back, uncomment "oldparser = 1" in lex.go.
- passes all.bash
- ~18% reduced parse time per file on average for make.bash
- ~3% reduced compile time for building cmd/compile
Change-Id: Icb5651bb9d8b9f66261762d2c94a03793050d4ce
Reviewed-on: https://go-review.googlesource.com/16665
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Larger stack frames mean nosplit functions use more stack and so the limit
needs to increase.
The change to test/nosplit.go is a bit ugly but I can't really think of a
way to make it nicer.
Change-Id: I2616b58015f0b62abbd62951575fcd0d2d8643c2
Reviewed-on: https://go-review.googlesource.com/16504
Reviewed-by: Russ Cox <rsc@golang.org>
The heap profile is only guaranteed to be up-to-date after two GC
cycles, so force two GCs instead of just one.
Updates #13098.
Change-Id: I4fb9287b698f4a3b90b8af9fc6a2efb3b082bfe5
Reviewed-on: https://go-review.googlesource.com/16848
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
The heapsampling.go test occasionally fails on some architectures
because it finds zero heap samples in main.alloc. This happens because
the byte and object counts are only updated at a GC. Hence, if a GC
happens part way through allocInterleaved, but then doesn't happen
after we start calling main.alloc, checkAllocations will see buckets
for the lines in main.alloc (which are created eagerly), but the
object and byte counts will be zero.
Fix this by forcing a GC to update the profile before we collect it.
Fixes#13098.
Change-Id: Ia7a9918eea6399307f10499dd7abefd4f6d13cf6
Reviewed-on: https://go-review.googlesource.com/16846
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
Skip fixedbugs/issue10607.go because external linking is not supported
yet.
Skip nilptr3.go because of issue #9058 (same as ppc64).
Change-Id: Ib3dfbd9a03ee4052871cf57c74b3cc5e745e1f80
Reviewed-on: https://go-review.googlesource.com/14461
Reviewed-by: Minux Ma <minux@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Minux Ma <minux@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change is the same as CL #9345 which was reverted,
except for a small bug fix.
The only change is to the body of sendDirect and its callsite.
Also added a test.
The problem was during a channel send operation. The target
of the send was a sleeping goroutine waiting to receive. We
basically do:
1) Read the destination pointer out of the sudog structure
2) Copy the value we're sending to that destination pointer
Unfortunately, the previous change had a goroutine suspend
point between 1 & 2 (the call to sendDirect). At that point
the destination goroutine's stack could be copied (shrunk).
The pointer we read in step 1 is no longer valid for step 2.
Fixed by not allowing any suspension points between 1 & 2.
I suspect the old code worked correctly basically by accident.
Fixes#13169
The original 9345:
This change removes the retry mechanism we use for buffered channels.
Instead, any sender waking up a receiver or vice versa completes the
full protocol with its counterpart. This means the counterpart does
not need to relock the channel when it wakes up. (Currently
buffered channels need to relock on wakeup.)
For sends on a channel with waiting receivers, this change replaces
two copies (sender->queue, queue->receiver) with one (sender->receiver).
For receives on channels with a waiting sender, two copies are still required.
This change unifies to a large degree the algorithm for buffered
and unbuffered channels, simplifying the overall implementation.
Fixes#11506
Change-Id: I57dfa3fc219cffa4d48301ee15fe5479299efa09
Reviewed-on: https://go-review.googlesource.com/16740
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Make sure that we're moving or zeroing pointers atomically.
Anything that is a multiple of pointer size and at least
pointer aligned might have pointers in it. All the code looks
ok except for the 1-pointer-sized moves.
Fixes#13160
Update #12552
Change-Id: Ib97d9b918fa9f4cc5c56c67ed90255b7fdfb7b45
Reviewed-on: https://go-review.googlesource.com/16668
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Duffcopy now uses X0, as of 5cf281a. Teach the peephole
optimizer that duffcopy clobbers X0 so that it does not
rename registers use X0 across the duffcopy instruction.
Fixes#13171
Change-Id: I389cbf1982cb6eb2f51e6152ac96736a8589f085
Reviewed-on: https://go-review.googlesource.com/16715
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Minux Ma <minux@golang.org>
Reviewed-by: Ilya Tocar <ilya.tocar@intel.com>
sradi and sradi. hide the top bit of their immediate argument apart from the
rest of it, but the code only handled the sradi case.
I'm pretty sure this is the only instruction missing (a couple of the rotate
instructions encode their immediate the same way but their handling looks OK).
This fixes the failure of "GOARCH=amd64 ~/go/bin/go install -v runtime" as
reported in the bug.
Fixes#11987
Change-Id: I0cdefcd7a04e0e8fce45827e7054ffde9a83f589
Reviewed-on: https://go-review.googlesource.com/16710
Reviewed-by: Minux Ma <minux@golang.org>
Handling of special records for tiny allocations has two problems:
1. Once we queue a finalizer we mark the object. As the result any
subsequent finalizers for the same object will not be queued
during this GC cycle. If we have 16 finalizers setup (the worst case),
finalization will take 16 GC cycles. This is what caused misbehave
of tinyfin.go. The actual flakiness was caused by the fact that fing
is asynchronous and don't always run before the check.
2. If a tiny block has both finalizer and profile specials,
it is possible that we both queue finalizer, preserve the object live
and free the profile record. As the result heap profile can be skewed.
Fix both issues by analyzing all special records for a single object at once.
Also, make tinyfin test stricter and remove reliance on real time.
Also, add a test for the problem 2. Currently heap profile missed about
a half of live memory.
Fixes#13100
Change-Id: I9ae4dc1c44893724138a4565ca5cae29f2e97544
Reviewed-on: https://go-review.googlesource.com/16591
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
* use new(int32) to be pedantic about documented SetFinalizer rules:
"The argument x must be a pointer to an object allocated by calling
new or by taking the address of a composite literal"
* remove the amd64-only restriction. The GC is fully precise everywhere
now, even on 32-bit. (keep the gccgo restriction, though)
* remove a data race (perhaps the actual bug) and use atomic.LoadInt32
for the final check. The race detector is now happy, too.
Updates #13100
Change-Id: I8d05c0ac4f046af9ba05701ad709c57984b34893
Reviewed-on: https://go-review.googlesource.com/16535
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Some tests disabled, some bifurcated into _ssa and not,
with appropriate logging added to compiler.
"tests/live.go" in particular needs attention.
SSA-specific testing removed, since it's all SSA now.
Added "-run_skips" option to tests/run.go to simplify
checking whether a test still fails (or how it fails)
on a skipped platform.
The compiler now compiles with SSA by default.
If you don't want SSA, specify GOSSAHASH=n (or N) as
an environment variable. Function names ending in "_ssa"
are always SSA-compiled.
GOSSAFUNC=fname retains its "SSA for fname, log to ssa.html"
GOSSAPKG=pkg only has an effect when GOSSAHASH=n
GOSSAHASH=10101 etc retains its name-hash-matching behavior
for purposes of debugging.
See #13068
Change-Id: I8217bfeb34173533eaeb391b5f6935483c7d6b43
Reviewed-on: https://go-review.googlesource.com/16299
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: David Chase <drchase@google.com>
Some tests need to disable inlining of a function. It's currently done
in one of a few ways (adding a function call, an empty switch, or a
defer). Add support for a less fragile 'go:noinline' directive that
prevents inlining.
Fixes#12312
Change-Id: Ife444e13361b4a927709d81aa41e448f32eec8d4
Reviewed-on: https://go-review.googlesource.com/13911
Run-TryBot: Todd Neal <todd@tneal.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Modified GOSSA{HASH.PKG} environment variable filters to
make it easier to make/run with all SSA for testing.
Disable attempts at SSA for architectures that are not
amd64 (avoid spurious errors/unimplementeds.)
Removed easy out for unimplemented features.
Add convert op for proper liveness in presence of uintptr
to/from unsafe.Pointer conversions.
Tweaked stack sizes to get a pass on windows;
1024 instead 768, was observed to pass at least once.
Change-Id: Ida3800afcda67d529e3b1cf48ca4a3f0fa48b2c5
Reviewed-on: https://go-review.googlesource.com/16201
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: David Chase <drchase@google.com>
Small fix: looks like a short variable declaration with a type switch
checks to make sure the variable used had valid shape (ONAME, OTYPE, or
ONONAME) and rejects everything else. Then a new variable is declared.
If the symbol contained in the declaration was a named OLITERAL (still a
valid identifier obviously) it would be rejected, even though a new
variable would have been declared.
Fix adds this case to the check.
Added a test case from issue12413.
Fixes#12413
Change-Id: I150dadafa8ee5612c867d58031027f2dca8c6ebc
Reviewed-on: https://go-review.googlesource.com/15760
Reviewed-by: Minux Ma <minux@golang.org>
Run-TryBot: Minux Ma <minux@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Apply static bounds checking logic during type checking even to
zero-element arrays, but skip synthesized OINDEX nodes that the
compiler has asserted are within bounds (such as the ones generated
while desugaring ORANGE nodes). This matches the logic in walkexpr
that also skips static bounds checking when Bounded is true.
Passes toolstash/buildall.
Fixes#12944.
Change-Id: I14ba03d71c002bf969d69783bec8d1a8e10e7d75
Reviewed-on: https://go-review.googlesource.com/15902
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Following the C to Go translation, some useless variables
were left in the code. In fmt.go, this was harmless.
In lex.go, it broke the error message related to
non-canonical import paths.
Fix it, and remove the useless variables.
The added test case is ignored in the go/types tests, since
the behavior of the non-canonical import path check seems
to be different.
Fixes#11362
Change-Id: Ic9129139ede90357dc79ebf167af638cf44536fa
Reviewed-on: https://go-review.googlesource.com/15580
Reviewed-by: Marvin Stenger <marvin.stenger94@gmail.com>
Reviewed-by: Minux Ma <minux@golang.org>
Run-TryBot: Minux Ma <minux@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This lets us re-enable duffzero.
Fixes#12108
Change-Id: Iefd24d26eaa56067caa2c29ff99cd20a42d8714a
Reviewed-on: https://go-review.googlesource.com/14937
Reviewed-by: Keith Randall <khr@golang.org>
The current heap sampling introduces some bias that interferes
with unsampling, producing unexpected heap profiles.
The solution is to use a Poisson process to generate the
sampling points, using the formulas described at
https://en.wikipedia.org/wiki/Poisson_process
This fixes#12620
Change-Id: If2400809ed3c41de504dd6cff06be14e476ff96c
Reviewed-on: https://go-review.googlesource.com/14590
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Minux Ma <minux@golang.org>
Run-TryBot: Minux Ma <minux@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Turns out the summary information for the ... args was
already correctly computed, all that lacked was to make
use of it and correct tests that documented our prior
deficiencies.
Fixes#12006
Change-Id: Ie8adfab7547f179391d470679598f0904aabf9f7
Reviewed-on: https://go-review.googlesource.com/15200
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
Internal error arose from calling methodfunc on a invalid interface
field during the implements check. int obviously isn't a function,
and errors on getinarg...
for im := iface.Type; im != nil; im = im.Down {
imtype = methodfunc(im.Type, nil)
// ...
}
Fix handles the internal compiler error, but does not throw an
additional error, i.e. the following code will error on the I
interface, but type A will pass the implements check since
'Read(string) string' is implemented and 'int' is skipped
type I interface {
Read(string) string
int
}
type A struct {
}
func (a *A) Read(s string) string {
return s
}
func New() I {
return new(A)
}
Fixes#10975
Change-Id: I4b54013afb2814db3f315515f0c742d8631ca500
Reviewed-on: https://go-review.googlesource.com/13747
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The existing test did not take into account the implicit
dereference of &fixedArray and thus heap-escaped when it
was not necessary.
Also added a detailed test for this and related cases.
Fixes#12588
Change-Id: I951e9684a093082ccdca47710f69f4366bd6b3cf
Reviewed-on: https://go-review.googlesource.com/15130
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
As detailed in #11910, the current implementation attempts to execute an area
of memory with unknown content. If the memory is executable, the result is
unpredictable - instead, make the test deterministic by attempting to execute
an instruction that is known to trigger a trap on the given architecture.
The new implementation is written by iant@ and provided via #11910.
Update issue #11910
Change-Id: Ia698c36e0dd98a9d9d16a701f60f6748c6faf896
Reviewed-on: https://go-review.googlesource.com/15058
Run-TryBot: Joel Sing <jsing@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
For variables which get SSA'd, SSA keeps track of all the def/kill.
It is only for on-stack variables that we need them.
This reduces stack frame sizes significantly because often the
only use of a variable was a varkill, and without that last use
the variable doesn't get allocated in the frame at all.
Fixes#12602
Change-Id: I3f00a768aa5ddd8d7772f375b25f846086a3e689
Reviewed-on: https://go-review.googlesource.com/14758
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
recover4 allocates 16 pages of memory via mmap, makes a 4 page hole in it with
munmap, allocates another 16 pages of memory via normal allocation and then
tries to copy from one to the other. For some reason on arm64 (but no other
platform I have tested) the second allocation sometimes causes the runtime to
ask the kernel for 4 additional pages of memory -- which the kernel satisfies
by remapping the pages that were just unmapped!
Moving the second allocation before the munmap fixes this behaviour, I can run
recover4 tens of thousands of times without failure with this fix vs a failure
rate of ~0.5% before.
Fixes#12549
Change-Id: I490b895b606897e4f7f25b1b51f5d485a366fffb
Reviewed-on: https://go-review.googlesource.com/14632
Reviewed-by: Dave Cheney <dave@cheney.net>
For now, we only use typedmemmove. This can be optimized
in future CLs.
Also add a feature to help with binary searching bad compilations.
Together with GOSSAPKG, GOSSAHASH specifies the last few binary digits
of the hash of function names that should be compiled. So
GOSSAHASH=0110 means compile only those functions whose last 4 bits
of hash are 0110. By adding digits to the front we can binary search
for the function whose SSA-generated code is causing a test to fail.
Change-Id: I5a8b6b70c6f034f59e5753965234cd42ea36d524
Reviewed-on: https://go-review.googlesource.com/14530
Reviewed-by: Keith Randall <khr@golang.org>
Add line that was inadvertently removed.
Change-Id: I99ebc1041e984e408ae5825836c28b9891d6043b
Reviewed-on: https://go-review.googlesource.com/14470
Run-TryBot: Todd Neal <todd@tneal.org>
Reviewed-by: Keith Randall <khr@golang.org>
This matches existing behavior, see issue #2196
Change-Id: Ifa9359b7c821115389f337a57de355c5ec23be8f
Reviewed-on: https://go-review.googlesource.com/14261
Reviewed-by: Keith Randall <khr@golang.org>
Evaluating args can overwrite arg area, so we can't write argsize and func
until args are evaluated.
Fixes test/recover.go, test/recover1.go, and test/fixedbugs/issue4066.go
Change-Id: I862e4934ccdb8661431bcc3e1e93817ea834ea3f
Reviewed-on: https://go-review.googlesource.com/14405
Reviewed-by: David Chase <drchase@google.com>
Now that the standard library tests
are all passing, add the
test directory tests.
These contain a number of edge case tests
that are of particular interest for compilers.
Some kinds of tests are not well-suited
for a new backend, such as errorcheck tests.
To start, use SSA only for run and runoutput.
There are three failing tests now.
Just mark them as such for now,
so that we can prevent regressions.
This code will all be unwound once SSA
codegen matures and becomes the default.
Change-Id: Ic51e6d0cc1cd48ef1e2fe2c9a743bf0cce275200
Reviewed-on: https://go-review.googlesource.com/14344
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This one of a set of changes to make the transition away from NodeList
easier by removing cases in which NodeList doesn't act semi-trivially like a
[]*Node.
This CL was originally prepared by Josh Bleecher Snyder <josharian@gmail.com>.
This change passes go build -toolexec 'toolstash -cmp' -a std.
Change-Id: I4d041b343952f4a31f3150fd70669e08fcaa74f8
Reviewed-on: https://go-review.googlesource.com/14305
Run-TryBot: Dave Cheney <dave@cheney.net>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This is a first of a set of changes to make the transition away from NodeList
easier by removing cases in which NodeList doesn't act semi-trivially like a
[]*Node.
This CL was originally prepared by Josh Bleecher Snyder <josharian@gmail.com>.
This change passes go build -toolexec 'toolstash -cmp' -a std.
Change-Id: Iad10b75e42b5b24e1694407841282fa3bab2dc9f
Reviewed-on: https://go-review.googlesource.com/14232
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Instead of trying to delete dead code as soon as we find it, just
mark it as dead using a PlainAndDead block kind. The deadcode pass
will do the real removal.
This way is somewhat more efficient because we don't need to mess
with successor and predecessor lists of all the dead blocks.
Fixes#12347
Change-Id: Ia42d6b5f9cdb3215a51737b3eb117c00bd439b13
Reviewed-on: https://go-review.googlesource.com/14033
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
The issue 12226 has been caused by the allocation of the same register
for the equality check of two byte values. The code in cgen.go freed the
register for the second operand before the allocation of the register
for the first operand.
Fixes#12226
Change-Id: Ie4dc33a488bd48a17f8ae9b497fd63c1ae390555
Reviewed-on: https://go-review.googlesource.com/13771
Reviewed-by: Russ Cox <rsc@golang.org>
Currently an expression like
var v = 0 >> 1000
is rejected by gc with a "stupid shift" error, while gotype
compiles it successfully.
As suggested by gri on the issue tracker, allow an rsh right
operand to be any valid uint value.
Fixes#11328
Change-Id: I6ccb3b7f842338d91fd26ae37dd4fa279d7fc440
Reviewed-on: https://go-review.googlesource.com/13777
Reviewed-by: Robert Griesemer <gri@golang.org>
The reg[] array in .../gc is where truth lies. The copy in .../ARCH
is incorrect as it is mostly not updated to reflect regalloc decisions.
This bug was introduced in the rewrite
https://go-review.googlesource.com/#/c/7853/. The new reg[] array was
introduced in .../gc but not all of the uses were removed in the
.../ARCH directories.
Fixes#12133
Change-Id: I6364fc403cdab92d802d17f2913ba1607734037c
Reviewed-on: https://go-review.googlesource.com/13630
Reviewed-by: Russ Cox <rsc@golang.org>
ODOTTYPE should be treated a whole lot like ODOT,
but it was missing completely from the switch in
escwalk and thus escape status did not propagate
to fields.
Since interfaces are required to trigger this bug,
the test was added to escape_iface.go.
Fixes#11931.
Change-Id: Id0383981cc4b1a160f6ad447192a112eed084538
Reviewed-on: https://go-review.googlesource.com/12921
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
