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>
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>
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>
Instead of pushing the denominator argument on the stack,
the denominator is now passed in m.
This fixes a variety of bugs related to trying to take stack traces
backwards from the middle of the software div/mod routines.
Some of those bugs have been kludged around in the past,
but others have not. Instead of trying to patch up after breaking
the stack, this CL stops breaking the stack.
This is an update of https://golang.org/cl/19810043,
which was rolled back in https://golang.org/cl/20350043.
The problem in the original CL was that there were divisions
at bad times, when m was not available. These were divisions
by constant denominators, either in C code or in assembly.
The Go compiler knows how to generate division by multiplication
for constant denominators, but the C compiler did not.
There is no longer any C code, so that's taken care of.
There was one problematic DIV in runtime.usleep (assembly)
but https://golang.org/cl/12898 took care of that one.
So now this approach is safe.
Reject DIV/MOD in NOSPLIT functions to keep them from
coming back.
Fixes#6681.
Fixes#6699.
Fixes#10486.
Change-Id: I09a13c76ad08ba75b3bd5d46a3eb78e66a84ab38
Reviewed-on: https://go-review.googlesource.com/12899
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Russ Cox fixed this issue for other systems
in CL 12026, but the Plan 9 part was forgotten.
Fixes#11656.
Change-Id: I91c033687987ba43d13ad8f42e3fe4c7a78e6075
Reviewed-on: https://go-review.googlesource.com/12762
Reviewed-by: Russ Cox <rsc@golang.org>
This is a reprise of https://golang.org/cl/12623. In that a CL I made
a suggestion which forgot that the +build constraints in the test
directory are not the same as those supported by the go tool: in the
test directory, if a single +build line fails, the test is skipped.
(In my defense, the code I was commenting on was also wrong.)
Change-Id: I8f29392a80b1983027f9a33043c803578409d678
Reviewed-on: https://go-review.googlesource.com/12776
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Old code appended, did not play well with a closure
with a ... param.
Fixes#11075.
Change-Id: Ib7c8590c5c4e576e798837e7499e00f3494efb4a
Reviewed-on: https://go-review.googlesource.com/12580
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: David Chase <drchase@google.com>
There was already special code to recognize "?" in hidden_structdcl,
which is used for inlined types and variables. This recognizes "?" in
structdcl as well, a case that arises when a struct type appears
within an inlined function body.
Fixes#10219.
Change-Id: Ic5257ae54f817e0d4a189c2294dcd633c9f2101a
Reviewed-on: https://go-review.googlesource.com/12241
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
The one in misc/makerelease/makerelease.go is particularly bad and
probably warrants rotating our keys.
I didn't update old weekly notes, and reverted some changes involving
test code for now, since we're late in the Go 1.5 freeze. Otherwise,
the rest are all auto-generated changes, and all manually reviewed.
Change-Id: Ia2753576ab5d64826a167d259f48a2f50508792d
Reviewed-on: https://go-review.googlesource.com/12048
Reviewed-by: Rob Pike <r@golang.org>
We can't address more than this on amd64 anyway.
Fixes#9862.
Change-Id: Ifb1abae558e2e1ee2dc953a76995f3f08c60b1df
Reviewed-on: https://go-review.googlesource.com/11715
Reviewed-by: Austin Clements <austin@google.com>
Currently the runtime fails to clear a G's stack barriers in gfput if
the G's stack allocation is _FixedStack bytes. This causes the runtime
to panic if the following sequence of events happens:
1) The runtime installs stack barriers on a G.
2) The G exits by calling runtime.Goexit. Since this does not
necessarily return through the stack barriers installed on the G,
there may still be untriggered stack barriers left on the G's stack
in recorded in g.stkbar.
3) The runtime calls gfput to add the exiting G to the free pool. If
the G's stack allocation is _FixedStack bytes, we fail to clear
g.stkbar.
4) A new G starts and allocates the G that was just added to the free
pool.
5) The new G begins to execute and overwrites the stack slots that had
stack barriers in them.
6) The garbage collector enters mark termination, attempts to remove
stack barriers from the new G, and finds that they've been
overwritten.
Fix this by clearing the stack barriers in gfput in the case where it
reuses the stack.
Fixes#11256.
Change-Id: I377c44258900e6bcc2d4b3451845814a8eeb2bcf
Reviewed-on: https://go-review.googlesource.com/11461
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Russ Cox <rsc@golang.org>
In walkdiv, an OMUL node was created and passed to typecheck,
before the op was changed back to OHMUL. In some instances,
the node that came back was an evaluated literal constant that
occurred with a full multiply. The end result was a literal node
with a non-shifted value and an OHMUL op. This change causes code
to be generated for the OHMUL.
Fixes#11358Fixes#11369
Change-Id: If42a98c6830d07fe065d5ca57717704fb8cfbd33
Reviewed-on: https://go-review.googlesource.com/11400
Reviewed-by: Russ Cox <rsc@golang.org>
When heapBitsSetType repeats a source bitmap with a scalar tail
(typ.ptrdata < typ.size), it lays out the tail upon reaching the end
of the source bitmap by simply increasing the number of bits claimed
to be in the incoming bit buffer. This causes later iterations to read
the appropriate number of zeros out of the bit buffer before starting
on the next repeat of the source bitmap.
Currently, however, later iterations of the loop continue to read bits
from the source bitmap *regardless of the number of bits currently in
the bit buffer*. The bit buffer can only hold 32 or 64 bits, so if the
scalar tail is large and the padding bits exceed the size of the bit
buffer, the read from the source bitmap on the next iteration will
shift the incoming bits into oblivion when it attempts to put them in
the bit buffer. When the buffer does eventually shift down to where
these bits were supposed to be, it will contain zeros. As a result,
words that should be marked as pointers on later repetitions are
marked as scalars, so the garbage collector does not trace them. If
this is the only reference to an object, it will be incorrectly freed.
Fix this by adding logic to drain the bit buffer down if it is large
instead of reading more bits from the source bitmap.
Fixes#11286.
Change-Id: I964432c4b9f1cec334fc8c3da0ff16460203feb6
Reviewed-on: https://go-review.googlesource.com/11360
Reviewed-by: Russ Cox <rsc@golang.org>
When a method is called using the Type.Method(receiver, args...) syntax
without the receiver, or enough arguments, provide the more helpful
error message "not enough arguments in call to method expression
Type.Method" instead of the old message "not enough arguments in call
to Type.Method".
Fixes#8385
Change-Id: Id5037eb1ee5fa93687d4a6557b4a8233b29e9df2
Reviewed-on: https://go-review.googlesource.com/2193
Reviewed-by: Russ Cox <rsc@golang.org>
Also modified test/run.go to ignore messages prefixed <autogenerated>
because those cannot be described with "// ERROR ...", and backed out
patch from issue #9537 because it is no longer necessary. The reasons
described in the 9537 discussion for why escape analysis cannot run
late no longer hold, happily.
Fixes#11053.
Change-Id: Icb14eccdf2e8cde3d0f8fb8a216b765400a96385
Reviewed-on: https://go-review.googlesource.com/11088
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: David Chase <drchase@google.com>
Bool codegen was generating a temp for function calls
and other complex expressions, but was not using it.
This was a refactoring bug introduced by CL 7853.
The cmp code used to do (in short):
l, r := &n1, &n2
It was changed to:
l, r := nl, nr
But the requisite assignments:
nl, nr = &n1, &n2
were only introduced on one of two code paths.
Fixes#10654.
Change-Id: Ie8de0b3a333842a048d4308e02911bb10c6915ce
Reviewed-on: https://go-review.googlesource.com/10844
Reviewed-by: Minux Ma <minux@golang.org>
Run-TryBot: Minux Ma <minux@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Use pkgimport == nil (or not) to distinguish between
parsing .go source files where "p" exponent specifier
is not allowed and parsing .a or .o export data where
it is. Use that to control error when p-exponent is
seen.
Fixes#9036
Change-Id: I8924f09c91d4945ef3f20e80a6e544008a94a7e4
Reviewed-on: https://go-review.googlesource.com/10450
Reviewed-by: Russ Cox <rsc@golang.org>
Added a lineno parameter to treecopy and listtreecopy
(ignored if = 0). When nodes are copied the copy is
assigned the non-zero lineno (normally this would be
the destination).
Fixes#8183
Change-Id: Iffb767a745093fb89aa08bf8a7692c2f0122be98
Reviewed-on: https://go-review.googlesource.com/10334
Reviewed-by: Russ Cox <rsc@golang.org>
Indirect function and method calls should leak everything,
but they didn't.
This fix had no particular effect on the cost of running the
compiler on html/template/*.go and added a single new "escape"
to the standard library:
syscall/syscall_unix.go:85: &b[0] escapes to heap
in
if errno := m.munmap(uintptr(unsafe.Pointer(&b[0])),
uintptr(len(b))); errno != nil {
Added specific escape testing to escape_calls.go
(and verified that it fails without this patch)
I also did a little code cleanup around the changes in esc.c.
Fixes#10925
Change-Id: I9984b701621ad4c49caed35b01e359295c210033
Reviewed-on: https://go-review.googlesource.com/10295
Reviewed-by: Russ Cox <rsc@golang.org>
This CL removes the remaining visible uses of the "architecture letter" concept.
(They are no longer in tool names nor in source directory names.)
Because the architecture letter concept is now gone, delete GOCHAR
from "go env" output, and change go/build.ArchChar to return an
error always.
The architecture letter is still used in the compiler and linker sources
as a clumsy architecture enumeration, but that use is not visible to
Go users and can be cleaned up separately.
Change-Id: I4d97a38f372003fb610c9c5241bea440d9dbeb8d
Reviewed-on: https://go-review.googlesource.com/10289
Reviewed-by: Rob Pike <r@golang.org>
This CL fixes the build to use the newly created go tool compile
and go tool link in place of go tool 5g, go tool 5l, and so on.
See golang-dev thread titled "go tool compile, etc" for background.
Although it was not a primary motivation, this conversion does
reduce the wall clock time and cpu time required for make.bash
by about 10%.
Change-Id: I79cbbdb676cab029db8aeefb99a53178ff55f98d
Reviewed-on: https://go-review.googlesource.com/10288
Reviewed-by: Rob Pike <r@golang.org>
Set overflowing integer constants to 1 rather than 0 to avoid
spurious div-zero errors in subsequent constant expressions.
Also: Exclude new test case from go/types test since it's
running too long (go/types doesn't have an upper constant
size limit at the moment).
Fixes#7746.
Change-Id: I3768488ad9909a3cf995247b81ee78a8eb5a1e41
Reviewed-on: https://go-review.googlesource.com/9165
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Today's earlier fix can stay, but it's a band-aid over the real problem,
which is that bad code was slipping through the type checker
into the back end (and luckily causing a type error there).
I discovered this because my new append does not use the same
temporaries and failed the test as written.
Fixes#9521.
Change-Id: I7e33e2ea15743406e15c6f3fdf73e1edecda69bd
Reviewed-on: https://go-review.googlesource.com/9921
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Instead of errors like:
./blank2.go:15: cannot use ~b1 (type []int) as type int in assignment
we now have:
./blank2.go:15: cannot use _ (type []int) as type int in assignment
Less confusing for users.
Fixes#9521
Change-Id: Ieab9859040e8e0df95deeaee7eeb408d3be61c0f
Reviewed-on: https://go-review.googlesource.com/9902
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Try to provide hints for common areas, either *interface
were interface would have been better, and note incorrect
capitalization (but don't be more ambitious than that, at
least not today).
Added code and test for cases
ptrInterface.ExistingMethod
ptrInterface.unexportedMethod
ptrInterface.MissingMethod
ptrInterface.withwRongcASEdMethod
interface.withwRongcASEdMethod
ptrStruct.withwRongcASEdMethod
struct.withwRongcASEdMethod
also included tests for related errors to check for
unintentional changes and consistent wording.
Somewhat simplified from previous versions to avoid second-
guessing user errors, yet also biased to point out most-likely
root cause.
Fixes#10700
Change-Id: I16693e93cc8d8ca195e7742a222d640c262105b4
Reviewed-on: https://go-review.googlesource.com/9731
Reviewed-by: Russ Cox <rsc@golang.org>
This includes the following information in the per-function summary:
outK = paramJ encoded in outK bits for paramJ
outK = *paramJ encoded in outK bits for paramJ
heap = paramJ EscHeap
heap = *paramJ EscContentEscapes
Note that (currently) if the address of a parameter is taken and
returned, necessarily a heap allocation occurred to contain that
reference, and the heap can never refer to stack, therefore the
parameter and everything downstream from it escapes to the heap.
The per-function summary information now has a tuneable number of bits
(2 is probably noticeably better than 1, 3 is likely overkill, but it
is now easy to check and the -m debugging output includes information
that allows you to figure out if more would be better.)
A new test was added to check pointer flow through struct-typed and
*struct-typed parameters and returns; some of these are sensitive to
the number of summary bits, and ought to yield better results with a
more competent escape analysis algorithm. Another new test checks
(some) correctness with array parameters, results, and operations.
The old analysis inferred a piece of plan9 runtime was non-escaping by
counteracting overconservative analysis with buggy analysis; with the
bug fixed, the result was too conservative (and it's not easy to fix
in this framework) so the source code was tweaked to get the desired
result. A test was added against the discovered bug.
The escape analysis was further improved splitting the "level" into
3 parts, one tracking the conventional "level" and the other two
computing the highest-level-suffix-from-copy, which is used to
generally model the cancelling effect of indirection applied to
address-of.
With the improved escape analysis enabled, it was necessary to
modify one of the runtime tests because it now attempts to allocate
too much on the (small, fixed-size) G0 (system) stack and this
failed the test.
Compiling src/std after touching src/runtime/*.go with -m logging
turned on shows 420 fewer heap allocation sites (10538 vs 10968).
Profiling allocations in src/html/template with
for i in {1..5} ;
do go tool 6g -memprofile=mastx.${i}.prof -memprofilerate=1 *.go;
go tool pprof -alloc_objects -text mastx.${i}.prof ;
done
showed a 15% reduction in allocations performed by the compiler.
Update #3753
Update #4720Fixes#10466
Change-Id: I0fd97d5f5ac527b45f49e2218d158a6e89951432
Reviewed-on: https://go-review.googlesource.com/8202
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
With this fix,
GOMAXPROCS=8 ./all.bash
passes, at least on my machine.
Fixes#10216.
Change-Id: Ib5991950892a1399ec81aced0a52b435e6f83fdf
Reviewed-on: https://go-review.googlesource.com/9392
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
In https://golang.org/cl/7797 I attempted to use myimportpath to set the value
of the go.importpath.$foo. symbol for the module being compiled, but I messed
it up and only set the name (which the linker rewrites anyway). This lead to
the importpath for the module being compiled being "". This was hard to notice,
because all modules that import another define the importpath for their
imported modules correctly -- but main is not imported, and this meant that the
reflect module saw all fields of all types defined in the main module as
exported.
The fix is to do what I meant to do the first time, add a test and change the
go tool to compile main packages with -p main and not -p
command-line-arguments.
Fixes#10332
Change-Id: I5fc6e9b1dc2b26f058641e382f9a56a526eca291
Reviewed-on: https://go-review.googlesource.com/8481
Reviewed-by: Russ Cox <rsc@golang.org>
This CL extends cmd/yacc to expose a yyErrorVerbose variable that
changes the error messages from just "syntax error" to "syntax error:
unexpected ${tokname}".
It also moves the yyToknames table generation to after rules have been
processed so that entries can be generated for tokens that aren't
mentioned in the preamble (e.g., '.' in the case of go.y).
Lastly, it restores gc's old code for applying yytfix to yyToknames,
except that substituting "LLITERAL" with litbuf happens in Yyerror.
Fixes#9968.
Change-Id: Icec188d11fdabc1dae31b8a471c35b5c7f6deec7
Reviewed-on: https://go-review.googlesource.com/8432
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
