On most systems, a pointer is the worst case alignment, so adding
a pointer field at the end of a struct guarantees there will be no
padding added after that field (to satisfy overall struct alignment
due to some more-aligned field also present).
In the runtime, the map implementation needs a quick way to
get to the overflow pointer, which is last in the bucket struct,
so it uses size - sizeof(pointer) as the offset.
NaCl/amd64p32 is the exception, as always.
The worst case alignment is 64 bits but pointers are 32 bits.
There's a long history that is not worth going into, but when
we moved the overflow pointer to the end of the struct,
we didn't get the padding computation right.
The compiler computed the regular struct size and then
on amd64p32 added another 32-bit field.
And the runtime assumed it could step back two 32-bit fields
(one 64-bit register size) to get to the overflow pointer.
But in fact if the struct needed 64-bit alignment, the computation
of the regular struct size would have added a 32-bit pad already,
and then the code unconditionally added a second 32-bit pad.
This placed the overflow pointer three words from the end, not two.
The last two were padding, and since the runtime was consistent
about using the second-to-last word as the overflow pointer,
no harm done in the sense of overwriting useful memory.
But writing the overflow pointer to a non-pointer word of memory
means that the GC can't see the overflow blocks, so it will
collect them prematurely. Then bad things happen.
Correct all this in a few steps:
1. Add an explicit check at the end of the bucket layout in the
compiler that the overflow field is last in the struct, never
followed by padding.
2. When padding is needed on nacl (not always, just when needed),
insert it before the overflow pointer, to preserve the "last in the struct"
property.
3. Let the compiler have the final word on the width of the struct,
by inserting an explicit padding field instead of overwriting the
results of the width computation it does.
4. For the same reason (tell the truth to the compiler), set the type
of the overflow field when we're trying to pretend its not a pointer
(in this case the runtime maintains a list of the overflow blocks
elsewhere).
5. Make the runtime use "last in the struct" as its location algorithm.
This fixes TestTraceStress on nacl/amd64p32.
The 'bad map state' and 'invalid free list' failures no longer occur.
Fixes#11838.
Change-Id: If918887f8f252d988db0a35159944d2b36512f92
Reviewed-on: https://go-review.googlesource.com/12971
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
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>
Fixes arm64 builder crash.
The bug is possible on all architectures; you just have to get lucky
and hit a preemption or a stack growth on entry to assertE2I2.
The test stacks the deck.
Change-Id: I8419da909b06249b1ad15830cbb64e386b6aa5f6
Reviewed-on: https://go-review.googlesource.com/12890
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
If the compiler doesn't do it, cmd/internal/obj/arm64 will,
and that will break the zeroing of ambiguously live values
done in zerorange, which in turn produces uninitialized
pointer cells that the GC trips over.
For #9880.
Change-Id: Ice97c30bc8b36d06b7b88d778d87fab8e1827fdc
Reviewed-on: https://go-review.googlesource.com/12847
Reviewed-by: Austin Clements <austin@google.com>
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>
Some routines run without and m or g and cannot invoke the
race detector runtime. They must be opaque to the runtime.
That used to be true because they were written in C.
Now that they are written in Go, disable the race detector
annotations for those functions explicitly.
Add test.
Fixes#10874.
Change-Id: Ia8cc28d51e7051528f9f9594b75634e6bb66a785
Reviewed-on: https://go-review.googlesource.com/12534
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This is mostly Russ's https://golang.org/cl/12145 but with some extra fixes to
account for the fact that function declarations without implementations now
break shared libraries, and including my test case.
Fixes#11480.
Change-Id: Iabdc2934a0378e5025e4e7affadb535eaef2c8f1
Reviewed-on: https://go-review.googlesource.com/12340
Reviewed-by: Ian Lance Taylor <iant@golang.org>
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>
These used to be defined at use, but that breaks when shared libraries
are involved.
For #11480.
Change-Id: I416a848754fb615c0d75f9f0ccc00723d07f7f01
Reviewed-on: https://go-review.googlesource.com/12145
Reviewed-by: Rob Pike <r@golang.org>
The previous commit (git 2ae77376) just did golang.org. This one
includes golang.org subdomains like blog, play, and build.
Change-Id: I4469f7b307ae2a12ea89323422044e604c5133ae
Reviewed-on: https://go-review.googlesource.com/12071
Reviewed-by: Rob Pike <r@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>
This avoids both a write barrier and then dynamic initialization
globals of the form
var x something
var xp = unsafe.Pointer(&x)
Using static initialization avoids emitting a relocation for &x,
which helps cgo.
Fixes#9411.
Change-Id: I0dbf480859cce6ab57ab805d1b8609c45b48f156
Reviewed-on: https://go-review.googlesource.com/11693
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
The expansion of structure, array, slice, and map literals
does not use the right line number in its introduced assignments
to temporaries, which leads to incorrect line number attribution
for expressions in those literals.
Inlining also incorrectly replaced the line numbers of args to
inlined functions.
This was revealed in CL 9721 because a now-avoided temporary
assignment introduced the correct line number.
I.e. before CL 9721
"tmp_wrongline := expr"
was transformed to
"tmp_rightline := expr; tmp_wrongline := tmp_rightline"
Also includes a repair to CL 10334 involving line numbers
where a spurious -1 remained (should have been 0, now is 0).
Fixes#11400.
Change-Id: I3a4687efe463977fa1e2c996606f4d91aaf22722
Reviewed-on: https://go-review.googlesource.com/11730
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Sameer Ajmani <sameer@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Compiling a simple file containing a slice of 100,000 strings,
the size of the resulting binary dropped from 5,896,224 bytes
to 3,495,968 bytes, which is the expected 2,400,000 bytes,
give or take.
Fixes#7384.
Change-Id: I3e551b5a1395b523a41b33518d81a1bf28da0906
Reviewed-on: https://go-review.googlesource.com/11698
Reviewed-by: Austin Clements <austin@google.com>
The new inlined code for append assumed that it could pass the
desired new cap to growslice, not the number of new elements.
But growslice still interpreted the argument as the number of new elements,
making it always grow by >2x (more precisely, 2x+1 rounded up
to the next malloc block size). At the time, I had intended to change
the other callers to use the new cap as well, but it's too late for that.
Instead, introduce growslice_n for the old callers and keep growslice
for the inlined (common case) caller.
Fixes#11403.
Filed #11419 to merge them.
Change-Id: I1338b1e5b352f3be4e43641f44b652ef7195251b
Reviewed-on: https://go-review.googlesource.com/11541
Reviewed-by: Austin Clements <austin@google.com>
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>
Instrument operands of OKEY.
Also instrument OSLICESTR. Previously it was not needed
because of preceeding bounds checks (which were instrumented).
But the preceeding bounds checks have disappeared.
Change-Id: I3b0de213e23cbcf5b8ef800abeded5eeeb3f8287
Reviewed-on: https://go-review.googlesource.com/11417
Reviewed-by: Russ Cox <rsc@golang.org>
The -importmap option takes an argument of the form old=new
and specifies that import "old" should be interpreted as if it said
import "new". The option may be repeated to specify multiple mappings.
This option is here to support the go command's new -vendor flag.
Change-Id: I31b4ed4249b549982a720bf61bb230462b33c59b
Reviewed-on: https://go-review.googlesource.com/10922
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@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>
//go:systemstack means that the function must run on the system stack.
Add one use in runtime as a demonstration.
Fixes#9174.
Change-Id: I8d4a509cb313541426157da703f1c022e964ace4
Reviewed-on: https://go-review.googlesource.com/10840
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
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>
A send on an unbuffered channel to a blocked receiver is the only
case in the runtime where one goroutine writes directly to the stack
of another. The garbage collector assumes that if a goroutine is
blocked, its stack contains no new pointers since the last time it ran.
The send on an unbuffered channel violates this, so it needs an
explicit write barrier. It has an explicit write barrier, but not one that
can handle a write to another stack. Use one that can (based on type bitmap
instead of heap bitmap).
To make this work, raise the limit for type bitmaps so that they are
used for all types up to 64 kB in size (256 bytes of bitmap).
(The runtime already imposes a limit of 64 kB for a channel element size.)
I have been unable to reproduce this problem in a simple test program.
Could help #11035.
Change-Id: I06ad994032d8cff3438c9b3eaa8d853915128af5
Reviewed-on: https://go-review.googlesource.com/10815
Reviewed-by: Austin Clements <austin@google.com>
These were found by grepping the comments from the go code and feeding
the output to aspell.
Change-Id: Id734d6c8d1938ec3c36bd94a4dbbad577e3ad395
Reviewed-on: https://go-review.googlesource.com/10941
Reviewed-by: Aamir Khan <syst3m.w0rm@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This was a refactoring bug during
'go tool compile', CL 10289.
Change-Id: Ibfd333be39ec72bba331fdf352df619cc21851a9
Reviewed-on: https://go-review.googlesource.com/10849
Reviewed-by: Minux Ma <minux@golang.org>
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>
obj.ARET is the portable return mnemonic. ppc64.ARETURN is a legacy
alias.
This was done with
sed -i s/ppc64\.ARETURN/obj.ARET/ cmd/compile/**/*.go
sed -i s/ARETURN/obj.ARET/ cmd/internal/obj/ppc64/obj9.go
Change-Id: I4d8e83ff411cee764774a40ef4c7c34dcbca4e43
Reviewed-on: https://go-review.googlesource.com/10673
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Minux Ma <minux@golang.org>
These are the Go 1.4 docs but refreshed for Go 1.5.
The most sigificant change is that all references to the Plan 9 toolchain are gone.
The tools no longer bear any meaningful resemblance.
Change-Id: I44f5cadb832a982323d7fee0b77673e55d761b35
Reviewed-on: https://go-review.googlesource.com/10298
Reviewed-by: Rob Pike <r@golang.org>
Suggested during code reviews of last 15 CLs (or so).
Change-Id: If780f6eb47a7a31df133c64d5dcf0eaf04d8447b
Reviewed-on: https://go-review.googlesource.com/10675
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
The only unreviewed change is in mparith3.go.
Change-Id: Iec0885e7688981cbaed04c152dc9b1c7032677e6
Reviewed-on: https://go-review.googlesource.com/10665
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Griesemer <gri@golang.org>
The build ID is an opaque token supplied by the build system.
The compiler writes it out early in the Go export metadata
(the second line), in a way that does not bother existing readers.
The intent is that the go command can use this to store information
about the sources for the generated code, so that it can detect
stale packages even in cases (like removed files) where mtimes fail.
Change-Id: Ib5082515d6cde8a07a8d4b5c69d1e8e4190cb5e1
Reviewed-on: https://go-review.googlesource.com/9153
Reviewed-by: Ian Lance Taylor <iant@golang.org>
It is almost never set and Addr is large, so having the full struct
in the Prog wastes memory most of the time.
Before (on a 64-bit system):
$ sizeof -p cmd/internal/obj Addr Prog
Addr 80
Prog 376
$
After:
$ sizeof -p cmd/internal/obj Addr Prog
Addr 80
Prog 304
$
Change-Id: I491f201241f87543964a7d0f48b85830759be9d0
Reviewed-on: https://go-review.googlesource.com/10457
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
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>
This is an automated follow-up to CL 10210.
It was generated with a combination of eg and gofmt -r.
No functional changes. Passes toolstash -cmp.
Change-Id: I35f5897948a270b472d8cf80612071b4b29e9a2b
Reviewed-on: https://go-review.googlesource.com/10253
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently when the race detector is enabled, orderexpr always creates
a temporary for slice and append operations. This used to be necessary
because the race detector had a different code path for slice
assignment that required this temporary. Unfortunately, creating this
temporary inhibits the optimization that eliminates write barriers
when a slice is assigned only to change its length or cap. For most
code, this is bad for performance, and in go:nowritebarrier functions
in the runtime, this can mean the difference between compiling and not
compiling.
Now the race detector uses the regular slice assignment code, so
creating this temporary is no longer necessary.
Change-Id: I296042e1edc571b77c407f709c2ff9091c4aa795
Reviewed-on: https://go-review.googlesource.com/10456
Reviewed-by: Russ Cox <rsc@golang.org>
It shrinks Prog type from 448 bytes down to 376 bytes on amd64.
It also makes sense, because I don't know of any modern architecture
that have instructions which can write to two destinations, none of
which is a register (even x86 doesn't have such instructions).
Change-Id: I3061f1c9ac93d79ee2b92ecb9049641d0e0f6300
Reviewed-on: https://go-review.googlesource.com/10330
Reviewed-by: Aram Hăvărneanu <aram@mgk.ro>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
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>
The new lower-level barriers work fine and don't need special handling,
because they appear to the race detector as (visible) ordinary assignments.
Change-Id: I7477d73a3deecbebf68716580678c595cc4151e3
Reviewed-on: https://go-review.googlesource.com/10316
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Package-external tests must use the vendored math/big package, not
the original one, otherwise tests may fail if there are discrepancies
in the implementation.
Change-Id: Ic5f0489aa6420ffea1f488633453f871ce1f0f66
Reviewed-on: https://go-review.googlesource.com/10380
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This resolves the compiler part of issue #10321.
Change-Id: I44b9909f992b37dd34b1c5292decd12de3d3a65e
Reviewed-on: https://go-review.googlesource.com/10355
Reviewed-by: Alan Donovan <adonovan@google.com>
No manual code changes.
This will permit addressing the compiler aspect of issue #10321 in a
subsequent change.
Change-Id: I3376dc38cafa0ec98bf54de33293015d0183cc82
Reviewed-on: https://go-review.googlesource.com/10354
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Before this change, the check for too-large arrays (and other large
types) occurred after escape analysis. If the data moved off stack
and onto the heap contained any pointers, it would therefore escape,
but because the too-large check occurred after escape analysis this
would not be recorded and a stack pointer would leak to the heap
(see the modified escape_array.go for an example).
Some of these appear to remain, in calls to typecheck from within walk.
Also corrected a few comments in escape_array.go about "BAD"
analysis that is now done correctly.
Enhanced to move aditional EscNone-but-large-so-heap checks into esc.c.
Change-Id: I770c111baff28a9ed5f8beb601cf09dacc561b83
Reviewed-on: https://go-review.googlesource.com/10268
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>
Trivial merging of 5g, 6g, ... into go tool compile,
and similarlly 5l, 6l, ... into go tool link.
The files compile/main.go and link/main.go are new.
Everything else in those directories is a move followed by
change of imports and package name.
This CL breaks the build. Manual fixups are in the next CL.
See golang-dev thread titled "go tool compile, etc" for background.
Change-Id: Id35ff5a5859ad9037c61275d637b1bd51df6828b
Reviewed-on: https://go-review.googlesource.com/10287
Reviewed-by: Dave Cheney <dave@cheney.net>
Reviewed-by: Rob Pike <r@golang.org>
