This change allows us to give an hg tag such as "go1.3beta1" to
revisions in the main branch without breaking the build.
This is helpful for community members who want to build the beta
from source.
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/90190044
The backing memory for >1 word interfaces was being scanned
conservatively.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/94000043
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.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/95900044
Not only ChST but also MeST (America/Metlakatla) is a zone
name containing a lower case letter.
LGTM=robert.hencke, r
R=golang-codereviews, robert.hencke, bradfitz, r
CC=golang-codereviews
https://golang.org/cl/99910043
Some system doesn't have libc.a available.
While we're at here, also export GOROOT in run.bash, so that
one doesn't need to set GOROOT to run run.bash.
LGTM=iant
R=iant
CC=golang-codereviews
https://golang.org/cl/99870043
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.
LGTM=minux.ma, rsc
R=golang-codereviews, gobot, rsc, minux.ma
CC=golang-codereviews
https://golang.org/cl/91830046
This should make Go build without setting CC and CXX on newer FreeBSDs.
LGTM=iant
R=golang-codereviews, dave, gobot, iant
CC=golang-codereviews
https://golang.org/cl/89230045
This CL tries to fill the gap between Linux and other Unix-like systems
in the same way UDPConn and UnixConn already did.
Fixes#7887.
LGTM=iant
R=iant
CC=golang-codereviews
https://golang.org/cl/97810043
Use a real type for Gs instead of scanning them conservatively.
Zero the schedlink pointer when it is dead.
Update #7820
LGTM=rsc
R=rsc, dvyukov
CC=golang-codereviews
https://golang.org/cl/89360043
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.
LGTM=iant
R=iant
CC=golang-codereviews
https://golang.org/cl/96790047
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.
LGTM=r
R=r, adg
CC=golang-codereviews
https://golang.org/cl/92790043
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.
LGTM=iant
R=golang-codereviews, bradfitz, iant
CC=golang-codereviews
https://golang.org/cl/99800044
Previously Read wouldn't return once its internal input buffer
is filled with non-data bytes.
Fixes#7875.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/90820043
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.
Fixes#7856
LGTM=ruiu, iant
R=iant, ruiu
CC=golang-codereviews, gri
https://golang.org/cl/94740044
Godoc makes it look better this way; before, it all ran together into nonsense.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/90400045
Previously it would panic because of out-of-bound access
if s1 is longer than s2.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/90110043
TestVariousDeadlines1Proc was flaky on my system,
failing on about 5% of runs.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/89830045
This should have been part of 36eb4a62fbb6,
but I later discovered that addresses are all wrong.
Appropriate test added now.
LGTM=r
R=golang-codereviews, r
CC=golang-codereviews
https://golang.org/cl/89470043
`GOARCH=arm go tool 6c` used to give "<prog>: cannot use 6c with GOARCH=arm"
LGTM=r
R=golang-codereviews, r
CC=golang-codereviews
https://golang.org/cl/89330043
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).
LGTM=iant, matrixik
R=golang-codereviews, iant, matrixik
CC=golang-codereviews
https://golang.org/cl/89230044
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
changed (pwd string) to (dir string), as some think pwd means passwd.
Fixes#7811.
LGTM=iant
R=golang-codereviews, iant, bradfitz
CC=golang-codereviews
https://golang.org/cl/89100043
Patch from msolo. Just moving it to a CL.
The test fails before and passes with the fix.
Fixes#7098
LGTM=msolo, rsc
R=rsc, iant, msolo
CC=golang-codereviews
https://golang.org/cl/88900044
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.
LGTM=iant
R=iant
CC=golang-codereviews
https://golang.org/cl/89040043
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.
Fixes#7243
LGTM=nigeltao
R=nigeltao
CC=bradfitz, golang-codereviews
https://golang.org/cl/86050045
Variables declared with 'var' have no sym->def.
Fixes#7794.
LGTM=rsc
R=golang-codereviews, bradfitz, rsc
CC=golang-codereviews
https://golang.org/cl/88360043
Before the switch to liblink, the linkers accepted the -c flag
to print the call graph. This change restores the functionality.
This came in handy when I was trying to audit the use of SSE
instructions inside the Plan 9 note handler.
LGTM=rsc
R=golang-codereviews, bradfitz, rsc
CC=golang-codereviews
https://golang.org/cl/73990043
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.
Pointed out in the review of CL 68580043.
TBR=iant
CC=golang-codereviews
https://golang.org/cl/88760043
This information is required by cmd/nm
to calculate absolute symbol addresses.
Update #6936
Update #7738
LGTM=rsc
R=golang-codereviews, bradfitz, gobot, rsc
CC=golang-codereviews
https://golang.org/cl/87500043
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
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.
LGTM=rsc
R=rsc, minux.ma
CC=golang-codereviews
https://golang.org/cl/88210045
Without the leaf bit, the linker cannot record
the correct frame size in the symbol table, and
then stack traces get mangled. (Only for ARM.)
Fixes#7338.
Fixes#7347.
LGTM=iant
R=iant
CC=golang-codereviews
https://golang.org/cl/88550043
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).
Fixes#7765
LGTM=gri, rsc
R=rsc, gri
CC=golang-codereviews
https://golang.org/cl/88160050
A //line directive without a filename now denotes the same
filename as the previous line (as in C).
Previously it denoted the file's directory (!).
Fixes#7765
LGTM=gri
R=gri
CC=golang-codereviews
https://golang.org/cl/86990044
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
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).
Previous discussion at:
https://groups.google.com/d/msg/golang-nuts/uCT_7WxxopQ/BoSBlLFzUTkJhttps://groups.google.com/d/msg/golang-dev/KUojayEr20I/u4fp_Ej5PdUJhttp://golang.org/issue/7711
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.
[1] https://golang.org/cl/4383047/diff/5001/src/cmd/ld/lib.c
LGTM=minux.ma, iant, dave
R=golang-codereviews, minux.ma, bradfitz, iant, dave
CC=golang-codereviews, r
https://golang.org/cl/87910047
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.
Fixes#7748.
LGTM=bradfitz, iant
R=golang-codereviews, bradfitz, iant
CC=golang-codereviews, khr
https://golang.org/cl/88100048
Having the pointers means you can grub around in the
binary finding out more about them.
This helped with issue 7748.
LGTM=minux.ma, bradfitz
R=golang-codereviews, minux.ma, bradfitz
CC=golang-codereviews
https://golang.org/cl/88090045
Make it clear that types that wrap another reader or writer delegate to the wrapped type.
Fixes#7667
LGTM=adg
R=golang-codereviews, adg
CC=golang-codereviews
https://golang.org/cl/85720044
This code never got updated after the liblink shuffle.
Tested by hand that it works and respects GOROOT_FINAL.
The discussion in issue 6963 suggests that perhaps we should
just drop runtime-gdb.py entirely, but I am not convinced
that is true. It was in Go 1.2 and I don't see a reason not to
keep it in Go 1.3. The fact that binaries have not been emitting
the reference was just a missed detail in the liblink conversion,
not part of a grand plan.
Fixes#7506.
Fixes#6963.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews, iant, r
https://golang.org/cl/87870048
If we compile a generated file stored in a temporary
directory - let's say /tmp/12345/work/x.c - then by default
6c stores the full path and then the pcln table in the
final binary includes the full path. This makes repeated builds
(using different temporary directories) produce different
binaries, even if the inputs are the same.
In the old 'go tool pack', the P flag specified a prefix to remove
from all stored paths (if present), and cmd/go invoked
'go tool pack grcP $WORK' to remove references to the
temporary work directory.
We've changed the build to avoid pack as much as possible,
under the theory that instead of making pack convert from
.6 to .a, the tools should just write the .a directly and save a
round of I/O.
Instead of going back to invoking pack always, define a common
flag -trimpath in the assemblers, C compilers, and Go compilers,
implemented in liblink, and arrange for cmd/go to use the flag.
Then the object files being written out have the shortened paths
from the start.
While we are here, reimplement pcln support for GOROOT_FINAL.
A build in /tmp/go uses GOROOT=/tmp/go, but if GOROOT_FINAL=/usr/local/go
is set, then a source file named /tmp/go/x.go is recorded instead as
/usr/local/go/x.go. We use this so that we can prepare distributions
to be installed in /usr/local/go without actually working in that
directory. The conversion to liblink deleted all the old file name
handling code, including the GOROOT_FINAL translation.
Bring the GOROOT_FINAL translation back.
Before this CL, using GOROOT_FINAL=/goroot make.bash:
g% strings $(which go) | grep -c $TMPDIR
6
g% strings $(which go) | grep -c $GOROOT
793
g%
After this CL:
g% strings $(which go) | grep -c $TMPDIR
0
g% strings $(which go) | grep -c $GOROOT
0
g%
(The references to $TMPDIR tend to be cgo-generated source files.)
Adding the -trimpath flag to the assemblers required converting
them to the new Go-semantics flag parser. The text in go1.3.html
is copied and adjusted from go1.1.html, which is when we applied
that conversion to the compilers and linkers.
Fixes#6989.
LGTM=iant
R=r, iant
CC=golang-codereviews
https://golang.org/cl/88300045
Per golang-nuts question. Writing to p breaks
other writers (e.g. io.MultiWriter).
Make this explicit.
LGTM=gri, r, rsc
R=r, rsc, gri, joshlf13
CC=golang-codereviews
https://golang.org/cl/87780046
My cmd/go got in a weird state where it started invoking pack grcP.
Change pack to print a 1-line explanation of the usage problem
before the generic usage message.
LGTM=r
R=r
CC=golang-codereviews
https://golang.org/cl/87770047
OpenBSD is excluded from all the usual thread-local storage
code, not just emitting the tbss section in the external link .o
but emitting a PT_TLS section in an internally-linked executable.
I assume it just has no proper TLS support. Exclude it here too.
TBR=iant
CC=golang-codereviews
https://golang.org/cl/87900045
We get
/usr/lib/libc.a(stack_protector.o): In function `__stack_chk_fail_local':
stack_protector.c:(.text+0x158): multiple definition of `__stack_chk_fail_local'
/var/tmp/go-link-04838a/000001.o:/tmp/gobuilder/netbsd-386-minux-c7a9e9243878/go/src/pkg/runtime/cgo/gcc_386.S:41: first defined here
I am assuming this has never worked and possibly is not intended to work.
(Some systems are vehemently against static linking.)
TBR=iant
CC=golang-codereviews
https://golang.org/cl/88130046
When I did the original 386 ports on Linux and OS X, I chose to
define GS-relative expressions like 4(GS) as relative to the actual
thread-local storage base, which was usually GS but might not be
(it might be FS, or it might be a different constant offset from GS or FS).
The original scope was limited but since then the rewrites have
gotten out of control. Sometimes GS is rewritten, sometimes FS.
Some ports do other rewrites to enable shared libraries and
other linking. At no point in the code is it clear whether you are
looking at the real GS/FS or some synthesized thing that will be
rewritten. The code manipulating all these is duplicated in many
places.
The first step to fixing issue 7719 is to make the code intelligible
again.
This CL adds an explicit TLS pseudo-register to the 386 and amd64.
As a register, TLS refers to the thread-local storage base, and it
can only be loaded into another register:
MOVQ TLS, AX
An offset from the thread-local storage base is written off(reg)(TLS*1).
Semantically it is off(reg), but the (TLS*1) annotation marks this as
indexing from the loaded TLS base. This emits a relocation so that
if the linker needs to adjust the offset, it can. For example:
MOVQ TLS, AX
MOVQ 8(AX)(TLS*1), CX // load m into CX
On systems that support direct access to the TLS memory, this
pair of instructions can be reduced to a direct TLS memory reference:
MOVQ 8(TLS), CX // load m into CX
The 2-instruction and 1-instruction forms correspond roughly to
ELF TLS initial exec mode and ELF TLS local exec mode, respectively.
Liblink applies this rewrite on systems that support the 1-instruction form.
The decision is made using only the operating system (and probably
the -shared flag, eventually), not the link mode. If some link modes
on a particular operating system require the 2-instruction form,
then all builds for that operating system will use the 2-instruction
form, so that the link mode decision can be delayed to link time.
Obviously it is late to be making changes like this, but I despair
of correcting issue 7719 and issue 7164 without it. To make sure
I am not changing existing behavior, I built a "hello world" program
for every GOOS/GOARCH combination we have and then worked
to make sure that the rewrite generates exactly the same binaries,
byte for byte. There are a handful of TODOs in the code marking
kludges to get the byte-for-byte property, but at least now I can
explain exactly how each binary is handled.
The targets I tested this way are:
darwin-386
darwin-amd64
dragonfly-386
dragonfly-amd64
freebsd-386
freebsd-amd64
freebsd-arm
linux-386
linux-amd64
linux-arm
nacl-386
nacl-amd64p32
netbsd-386
netbsd-amd64
openbsd-386
openbsd-amd64
plan9-386
plan9-amd64
solaris-amd64
windows-386
windows-amd64
There were four exceptions to the byte-for-byte goal:
windows-386 and windows-amd64 have a time stamp
at bytes 137 and 138 of the header.
darwin-386 and plan9-386 have five or six modified
bytes in the middle of the Go symbol table, caused by
editing comments in runtime/sys_{darwin,plan9}_386.s.
Fixes#7164.
LGTM=iant
R=iant, aram, minux.ma, dave
CC=golang-codereviews
https://golang.org/cl/87920043
It was said already but apparently not enough times.
Fixes#6985.
LGTM=crawshaw
R=golang-codereviews, crawshaw
CC=golang-codereviews
https://golang.org/cl/86300043