- 17 Apr, 2014 3 commits
-
-
Russ Cox authored
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
-
Brad Fitzpatrick authored
Fixes #7225 LGTM=r R=golang-codereviews, r CC=golang-codereviews, rsc https://golang.org/cl/88710043
-
Rob Pike authored
Fixes #7752. LGTM=bradfitz, ruiu R=golang-codereviews, bradfitz, ruiu CC=golang-codereviews https://golang.org/cl/88690043
-
- 16 Apr, 2014 29 commits
-
-
Rui Ueyama authored
LGTM=bradfitz R=golang-codereviews, bradfitz CC=golang-codereviews https://golang.org/cl/88670043
-
Ian Lance Taylor authored
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
-
Russ Cox authored
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
-
Alan Donovan authored
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
-
Emil Hessman authored
LGTM=bradfitz R=golang-codereviews, bradfitz CC=golang-codereviews https://golang.org/cl/88000047
-
Alan Donovan authored
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
-
Brad Fitzpatrick authored
Fixes #7733 LGTM=minux.ma R=golang-codereviews, minux.ma CC=golang-codereviews, nigeltao, r, rsc https://golang.org/cl/88330044
-
Brad Fitzpatrick authored
Fixes #7264 LGTM=dvyukov R=dvyukov CC=golang-codereviews https://golang.org/cl/87970045
-
Russ Cox authored
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
-
Rob Pike authored
Update #7056 LGTM=bradfitz R=golang-codereviews, bradfitz CC=golang-codereviews https://golang.org/cl/88070045
-
Russ Cox authored
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/BoSBlLFzUTkJ https://groups.google.com/d/msg/golang-dev/KUojayEr20I/u4fp_Ej5PdUJ http://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
-
Russ Cox authored
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
-
Russ Cox authored
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
-
Russ Cox authored
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
-
Shenghou Ma authored
Didn't manage to find a way to write test cases. Fixes #7769. LGTM=iant R=iant CC=golang-codereviews https://golang.org/cl/88000045
-
Shenghou Ma authored
LGTM=iant R=golang-codereviews, iant CC=golang-codereviews https://golang.org/cl/88360044
-
Shenghou Ma authored
Don't advertise -s anymore. Fixes #7793. LGTM=iant R=golang-codereviews, iant CC=golang-codereviews https://golang.org/cl/88030045
-
Shenghou Ma authored
Fixes #7773. LGTM=bradfitz R=golang-codereviews, bradfitz CC=golang-codereviews https://golang.org/cl/87400043
-
Shenghou Ma authored
LGTM=bradfitz R=golang-codereviews, bradfitz, dave CC=golang-codereviews https://golang.org/cl/88000044
-
Billie Harold Cleek authored
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
-
Andrew Gerrand authored
Generated by addca. R=gobot CC=golang-codereviews https://golang.org/cl/87830046
-
Brad Fitzpatrick authored
Fixes #7775 LGTM=rsc R=agl, rsc CC=golang-codereviews https://golang.org/cl/88340043
-
Nigel Tao authored
so that the zlib.Reader returns nil error. Fixes #7762. LGTM=r R=r CC=golang-codereviews https://golang.org/cl/86750044
-
Russ Cox authored
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
-
Russ Cox authored
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
-
Brad Fitzpatrick authored
Trying to understand the linux-386-387 failures: http://build.golang.org/log/78a91da173c11e986b4e623527c2d0b746f4e814 Also modernize the closeOnce code with a method value, while I was looking. LGTM=adg R=golang-codereviews, adg CC=golang-codereviews, iant https://golang.org/cl/87950044
-
Brad Fitzpatrick authored
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
-
Russ Cox authored
LGTM=r R=r CC=golang-codereviews https://golang.org/cl/88050046
-
Russ Cox authored
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
-
- 15 Apr, 2014 8 commits
-
-
Rob Pike authored
Fixes #7188 LGTM=adg R=golang-codereviews, adg CC=golang-codereviews https://golang.org/cl/88280044
-
Rob Pike authored
Also make it clear this is not a complete description of all features. Fixes #7790. LGTM=bradfitz R=golang-codereviews, bradfitz CC=golang-codereviews https://golang.org/cl/88300044
-
David du Colombier authored
LGTM=rsc R=bradfitz, rsc CC=golang-codereviews https://golang.org/cl/87800044
-
Shenghou Ma authored
Fixes #7768. LGTM=iant, gri R=golang-codereviews, iant, gri CC=golang-codereviews https://golang.org/cl/87260043
-
Russ Cox authored
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
-
Russ Cox authored
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
-
Russ Cox authored
Fixes #7719. LGTM=iant R=iant CC=golang-codereviews https://golang.org/cl/87760050
-
Russ Cox authored
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
-