Michael Fraenkel [Wed, 30 Apr 2014 17:03:38 +0000 (13:03 -0400)]
cmd/go: test: clean up all temporary directories
go test may call builder.init() multiple times which will create a new work directory. The cleanup needs to hoist the current work directory.
Fixes #7904.
Ian Lance Taylor [Tue, 29 Apr 2014 12:53:38 +0000 (08:53 -0400)]
cmd/cgo: for gccgo add #define to cgo_export.h for expected name
For gccgo we rename exported functions so that the compiler
will make them visible. This CL adds a #define so that C
functions that #include "cgo_export.h" can use the expected
names of the function.
The test for this is the existing issue6833 test in
misc/cgo/test. Without this CL it fails when using
-compiler=gccgo.
Shenghou Ma [Mon, 28 Apr 2014 18:24:14 +0000 (14:24 -0400)]
misc/vim/readme.txt: workaround weird OS X vim bug.
The vi bundled with OS X has a weird bug in that if you turn off
filetype in .vimrc when it's not turned on, even a clean exit of
vi will return 1 which breaks almost everything.
While we're at it, add hint to change $GOROOT to its actual value
in .vimrc.
Ian Lance Taylor [Sun, 27 Apr 2014 05:31:32 +0000 (22:31 -0700)]
misc/cgo/test/backdoor: add gccgo version of backdoor function
For the gc compiler the Go function Issue7695 is defined in
runtime.c, but there is no way to do that for gccgo, because
there is no way to get the correct pkgpath. The test is not
important for gccgo in any case.
Peter Collingbourne [Sun, 27 Apr 2014 05:16:38 +0000 (22:16 -0700)]
cmd/cgo: fix C.CString for strings containing null terminators under gccgo
Previously we used strndup(3) to implement C.CString for gccgo. This
is wrong because strndup assumes the string to be null terminated,
and stops at the first null terminator. Instead, use malloc
and memmove to create a copy of the string, as we do in the
gc implementation.
net/http/httputil: tell people not to use ClientConn and ServerConn
A very smart developer at Gophercon just asked me to help debug
a problem and I was horrified to learn that he was using httputil's
ClientConn. I forgot ClientConn and ServerConn were even included
in Go 1! They should've been deleted.
Scare people away from using them. The net/http package does
not use them and they're unused, unmaintained and untouched in
4+ years.
Robert Obryk [Sun, 27 Apr 2014 02:59:00 +0000 (19:59 -0700)]
net: Remove an unmatched unlock of ForkLock
Remove an RUnlock of syscall.ForkLock with no matching RLock.
Holding ForkLock in netFD.dup is unnecessary: dupCloseOnExecOld
locks and unlocks the lock on its own and dupCloseOnExec doesn't
need the ForkLock to be held.
net/http, strings, bytes: fix http race, revert part of Reader behavior change
I fixed this data race regression in two ways: in net/http itself, and also
partially reverting the change from https://golang.org/cl/77580046 .
Previously a Read from a strings.Reader or bytes.Reader returning 0 bytes
would not be a memory write. After 77580046 it was. This reverts that back
in case others depended on that. Also adds tests.
Shenghou Ma [Mon, 21 Apr 2014 04:02:21 +0000 (00:02 -0400)]
cmd/go: handle -ccflags in 'go test'
CL 89050043 only allows -ccflags for 'go test', this
CL really handles the flag like the other -??flags.
Many thanks to Dobrosław Żybort for pointing this out.
Fixes #7810 (again).
David du Colombier [Sat, 19 Apr 2014 07:55:09 +0000 (09:55 +0200)]
include/plan9: make mklibc.rc compatible with 9atom
Since there is no Runemask constant in UTF-8 enum in
9atom, removing the other constants leads to an empty
enum, which is illegal. Thus, we remove enum entirely.
LGTM=r
R=golang-codereviews, r
CC=golang-codereviews
https://golang.org/cl/82660044
Mike Andrews [Fri, 18 Apr 2014 22:37:55 +0000 (15:37 -0700)]
cmd/ld: don't delete output binary if not "ordinary" file (redux).
following on CL https://golang.org/cl/76810045 and
issue 7563, i now see there's another "remove(outfile)" a few
dozen lines down that also needs fixing.
LGTM=iant
R=golang-codereviews, iant
CC=0intro, golang-codereviews, r
https://golang.org/cl/89030043
cmd/pack: avoid ./ import in test (fix Windows build)
It is possible to use ./ imports on Windows but it
requires some extra command-line work
('go build' does this automatically, but we can't use 'go build' here).
Instead, use an ordinary import and -I/-L, which are easier to use.
net/http: allow commas and spaces in cookie values
According to RFC 6265 a cookie value may contain neither
commas nor spaces but such values are very common in the
wild and browsers handle them very well so we'll allow
both commas and spaces.
Values starting or ending in a comma or a space are
sent in the quoted form to prevent missinterpetations.
RFC 6265 conforming values are handled as before and
semicolons, backslashes and double-quotes are still
disallowed.
Cgo writes C function declarations pretending every arg is a pointer.
If the C function is deferred, it does not inhibit stack copying on split.
The stack copying code believes the C declaration, possibly misinterpreting
integers as pointers.
Probably the right fix for Go 1.3 is to make deferred C functions inhibit
stack copying.
For Go 1.4 and beyond we probably need to make cgo generate Go code
for 6g here, not C code for 6c.
https://golang.org/cl/60590044 edited
doc.go without editing the file it is generated from.
The edit was lost at the next mkdoc.sh.
Make the change in help.go and rerun mkdoc.sh.
liblink, cmd/ld: reenable nosplit checking and test
The new code is adapted from the Go 1.2 nosplit code,
but it does not have the bug reported in issue 7623:
g% go run nosplit.go
g% go1.2 run nosplit.go
BUG
rejected incorrectly:
main 0 call f; f 120
linker output:
# _/tmp/go-test-nosplit021064539
main.main: nosplit stack overflow
120 guaranteed after split check in main.main
112 on entry to main.f
-8 after main.f uses 120
g%
Fixes #6931.
Fixes #7623.
LGTM=iant
R=golang-codereviews, iant, ality
CC=golang-codereviews, r
https://golang.org/cl/88190043
Ian Lance Taylor [Wed, 16 Apr 2014 21:36:44 +0000 (14:36 -0700)]
liblink, cmd/gc, cmd/{5,6,8}{a,c}: rename linkwriteobj to writeobj
The name linkwriteobj is misleading because it implies that
the function has something to do with the linker, which it
does not. The name is historical: the function performs an
operation that was previously performed by the linker, but no
longer is.
Alan Donovan [Wed, 16 Apr 2014 20:17:50 +0000 (16:17 -0400)]
go/scanner: interpret //line directives sans filename sensibly, second try.
A //line directive without a filename now denotes the empty
filename, not the current directory (the Go 1.2 behaviour) nor
the previous //line's filename (the behaviour since CL 86990044).
They should never appear (but they do, e.g. due to a bug in godoc).
cmd/5g, cmd/6g, cmd/8g: preserve wide values in large functions
In large functions with many variables, the register optimizer
may give up and choose not to track certain variables at all.
In this case, the "nextinnode" information linking together
all the words from a given variable will be incomplete, and
the result may be that only some of a multiword value is
preserved across a call. That confuses the garbage collector,
so don't do that. Instead, mark those variables as having
their address taken, so that they will be preserved at all
calls. It's overkill, but correct.
Tested by hand using the 6g -S output to see that it does fix
the buggy generated code leading to the issue 7726 failure.
There is no automated test because I managed to break the
compiler while writing a test (see issue 7727). I will check
in a test along with the fix to issue 7727.
Fixes #7726.
LGTM=khr
R=khr, bradfitz, dave
CC=golang-codereviews
https://golang.org/cl/85200043
runtime: crash when func main calls Goexit and all other goroutines exit
This has typically crashed in the past, although usually with
an 'all goroutines are asleep - deadlock!' message that shows
no goroutines (because there aren't any).
There is general agreement that runtime.Goexit terminates the
main goroutine, so that main cannot return, so the program does
not exit.
The interpretation that all other goroutines exiting causes an
exit(0) is relatively new and was not part of those discussions.
That is what this CL changes.
Thankfully, even though the exit(0) has been there for a while,
some other accounting bugs made it very difficult to trigger,
so it is reasonable to replace. In particular, see golang.org/issue/7711#c10
for an examination of the behavior across past releases.
Fixes #7711.
LGTM=iant, r
R=golang-codereviews, iant, dvyukov, r
CC=golang-codereviews
https://golang.org/cl/88210044
linklookup uses hash(name, v) as the hash table index but then
only compares name to find a symbol to return.
If hash(name, v1) == hash(name, v2) for v1 != v2, the lookup
for v2 will return the symbol with v1.
The input routines assume that each symbol is found only once,
and then each symbol is added to a linked list, with the list header
in the symbol. Adding a symbol to such a list multiple times
short-circuits the list the second time it is added, causing symbols
to be dropped.
The liblink rewrite introduced an elegant, if inefficient, handling
of duplicated symbols by creating a dummy symbol to read the
duplicate into. The dummy symbols are named .dup with
sequential version numbers. With many .dup symbols, eventually
there will be a conflict, causing a duplicate list add, causing elided
symbols, causing a crash when calling one of the elided symbols.
The bug is old (2011) but could not have manifested until the
liblink rewrite introduced this heavily duplicated symbol .dup.
(See History section below.)
1. Correct the lookup function.
2. Since we want all the .dup symbols to be different, there's no
point in inserting them into the table. Call linknewsym directly,
avoiding the lookup function entirely.
3. Since nothing can refer to the .dup symbols, do not bother
adding them to the list of functions (textp) at all.
4. In lieu of a unit test, introduce additional consistency checks to
detect adding a symbol to a list multiple times. This would have
caught the short-circuit more directly, and it will detect a variety
of double-use bugs, including the one arising from the bad lookup.
Fixes #7749.
History
On April 9, 2011, I submitted CL 4383047, making ld 25% faster.
Much of the focus was on the hash table lookup function, and
one of the changes was to remove the s->version == v comparison [1].
I don't know if this was a simple editing error or if I reasoned that
same name but different v would yield a different hash slot and
so the name test alone sufficed. It is tempting to claim the former,
but it was probably the latter.
Because the hash is an iterated multiply+add, the version ends up
adding v*3ⁿ to the hash, where n is the length of the name.
A collision would need x*3ⁿ ≡ y*3ⁿ (mod 2²⁴ mod 100003),
or equivalently x*3ⁿ ≡ x*3ⁿ + (y-x)*3ⁿ (mod 2²⁴ mod 100003),
so collisions will actually be periodic: versions x and y collide
when d = y-x satisfies d*3ⁿ ≡ 0 (mod 2²⁴ mod 100003).
Since we allocate version numbers sequentially, this is actually
about the best case one could imagine: the collision rate is
much lower than if the hash were more random.
http://play.golang.org/p/TScD41c_hA computes the collision
period for various name lengths.
The most common symbol in the new linker is .dup, and for n=4
the period is maximized: the 100004th symbol is the first collision.
Unfortunately, there are programs with more duplicated symbols
than that.
In Go 1.2 and before, duplicate symbols were handled without
creating a dummy symbol, so this particular case for generating
many duplicate symbols could not happen. Go does not use
versioned symbols. Only C does; each input file gives a different
version to its static declarations. There just aren't enough C files
for this to come up in that context.
So the bug is old but the realization of the bug is new.
reflect: correct type descriptor for call of interface method
When preparing a call with an interface method, the argument
frame holds the receiver "iword", but funcLayout was being
asked to write a descriptor as if the receiver were a complete
interface value. This was originally caught by running a large
program with Debug=3 in runtime/mgc0.c, but the new panic
in funcLayout suffices to catch the mistake with the existing
tests.