Andrew Gerrand [Sun, 1 Jun 2014 22:48:20 +0000 (08:48 +1000)]
cmd/dist: only use beta tag in version string for the exact revision
Right now, any revision on the default branch after go1.3beta2 is
described by "go verson" as go1.3beta2 plus some revision.
That's OK for now, but once go1.3 is released, that will seem wrong.
Andrew Gerrand [Sun, 1 Jun 2014 22:34:26 +0000 (08:34 +1000)]
misc/goplay: remove program
This program has barely been touched since it was first committed,
and in its current state it opens a code execution vector similar
to the one that was recently fixed in go.tools/playground/socket.
Russ Cox [Sun, 1 Jun 2014 17:57:46 +0000 (13:57 -0400)]
runtime: fix correctness test at end of traceback
We were requiring that the defer stack and the panic stack
be completely processed, thinking that if any were left over
the stack scan and the defer stack/panic stack must be out
of sync. It turns out that the panic stack may well have
leftover entries in some situations, and that's okay.
Fixes #8132.
LGTM=minux, r
R=golang-codereviews, minux, r
CC=golang-codereviews, iant, khr
https://golang.org/cl/100900044
Russ Cox [Sat, 31 May 2014 14:10:12 +0000 (10:10 -0400)]
runtime: make continuation pc available to stack walk
The 'continuation pc' is where the frame will continue
execution, if anywhere. For a frame that stopped execution
due to a CALL instruction, the continuation pc is immediately
after the CALL. But for a frame that stopped execution due to
a fault, the continuation pc is the pc after the most recent CALL
to deferproc in that frame, or else 0. That is where execution
will continue, if anywhere.
The liveness information is only recorded for CALL instructions.
This change makes sure that we never look for liveness information
except for CALL instructions.
Using a valid PC fixes crashes when a garbage collection or
stack copying tries to process a stack frame that has faulted.
Record continuation pc in heapdump (format change).
Fixes #8048.
LGTM=iant, khr
R=khr, iant, dvyukov
CC=golang-codereviews, r
https://golang.org/cl/100870044
Russ Cox [Sat, 31 May 2014 13:35:37 +0000 (09:35 -0400)]
runtime: fix error check in freebsd/386 i386_set_ldt
Update #2675
The code here was using the error check for Linux/386,
not the one for FreeBSD/386. Most of the time it worked.
Thanks to Neel Natu (FreeBSD developer) for finding this.
The s/JCC/JAE/ a few lines later is a no-op but makes the
test match the rest of the file. Why we write JAE instead of JCC
I don't know, but the two are equivalent and the file might
as well be consistent.
Russ Cox [Fri, 30 May 2014 20:41:58 +0000 (16:41 -0400)]
cmd/6g: treat vardef-initialized fat variables as live at calls
This CL forces the optimizer to preserve some memory stores
that would be redundant except that a stack scan due to garbage
collection or stack copying might look at them during a function call.
As such, it forces additional memory writes and therefore slows
down the execution of some programs, especially garbage-heavy
programs that are already limited by memory bandwidth.
The slowdown can be as much as 7% for end-to-end benchmarks.
These numbers are from running go1.test -test.benchtime=5s three times,
taking the best (lowest) ns/op for each benchmark. I am excluding
benchmarks with time/op < 10us to focus on macro effects.
All benchmarks are on amd64.
Comparing tip (a27f34c771cb) against this CL on an Intel Core i5 MacBook Pro:
Russ Cox [Thu, 29 May 2014 17:47:31 +0000 (13:47 -0400)]
cmd/gc: fix x=x crash
[Same as CL 102820043 except applied changes to 6g/gsubr.c
also to 5g/gsubr.c and 8g/gsubr.c. The problem I had last night
trying to do that was that 8g's copy of nodarg has different
(but equivalent) control flow and I was pasting the new code
into the wrong place.]
The 'nodarg' function is used to obtain a Node*
representing a function argument or result.
It returned a brand new Node*, but that violates
the guarantee in most places in the compiler that
two Node*s refer to the same variable if and only if
they are the same Node* pointer. Reestablish that
invariant by making nodarg return a preexisting
named variable if present.
Having fixed that, avoid any copy during x=x in
componentgen, because the VARDEF we emit
before the copy marks the lhs x as dead incorrectly.
The change in walk.c avoids modifying the result
of nodarg. This was the only place in the compiler
that did so.
Fixes #8097.
LGTM=khr
R=golang-codereviews, khr
CC=golang-codereviews, iant, khr, r
https://golang.org/cl/103750043
Breaks 386 and arm builds.
The obvious reason is that this CL only edited 6g/gsubr.c
and failed to edit 5g/gsubr.c and 8g/gsubr.c.
However, the obvious CL applying the same edit to those
files (CL 101900043) causes mysterious build failures
in various of the standard package tests, usually involving
reflect. Something deep and subtle is broken but only on
the 32-bit systems.
Undo this CL for now.
««« original CL description
cmd/gc: fix x=x crash
The 'nodarg' function is used to obtain a Node*
representing a function argument or result.
It returned a brand new Node*, but that violates
the guarantee in most places in the compiler that
two Node*s refer to the same variable if and only if
they are the same Node* pointer. Reestablish that
invariant by making nodarg return a preexisting
named variable if present.
Having fixed that, avoid any copy during x=x in
componentgen, because the VARDEF we emit
before the copy marks the lhs x as dead incorrectly.
The change in walk.c avoids modifying the result
of nodarg. This was the only place in the compiler
that did so.
Russ Cox [Wed, 28 May 2014 23:50:19 +0000 (19:50 -0400)]
cmd/gc: fix x=x crash
The 'nodarg' function is used to obtain a Node*
representing a function argument or result.
It returned a brand new Node*, but that violates
the guarantee in most places in the compiler that
two Node*s refer to the same variable if and only if
they are the same Node* pointer. Reestablish that
invariant by making nodarg return a preexisting
named variable if present.
Having fixed that, avoid any copy during x=x in
componentgen, because the VARDEF we emit
before the copy marks the lhs x as dead incorrectly.
The change in walk.c avoids modifying the result
of nodarg. This was the only place in the compiler
that did so.
Russ Cox [Wed, 28 May 2014 18:04:31 +0000 (14:04 -0400)]
cmd/cgo: given typedef struct S T, make C.T and C.struct_S interchangeable
For incomplete struct S, C.T and C.struct_S were interchangeable in Go 1.2
and earlier, because all incomplete types were interchangeable
(even C.struct_S1 and C.struct_S2).
CL 76450043, which fixed issue 7409, made different incomplete types
different from Go's point of view, so that they were no longer completely
interchangeable.
However, imprecision about C.T and C.struct_S - really the same
underlying C type - is the one behavior enabled by the bug that
is most likely to be depended on by existing cgo code.
Explicitly allow it, to keep that code working.
Fixes #7786.
LGTM=iant, r
R=golang-codereviews, iant, r
CC=golang-codereviews
https://golang.org/cl/98580046
Dmitriy Vyukov [Wed, 28 May 2014 04:00:01 +0000 (00:00 -0400)]
runtime: fix go of nil func value
Currently runtime derefences nil with m->locks>0,
which causes unrecoverable fatal error.
Panic instead.
Fixes #8045.
For historical reasons, temp was returning a copy
of the created Node*, not the original Node*.
This meant that if analysis recorded information in the
returned node (for example, n->addrtaken = 1), the
analysis would not show up on the original Node*, the
one kept in fn->dcl and consulted during liveness
bitmap creation.
Correct this, and watch for it when setting addrtaken.
This CL fixes the third. There are only three nod(OXXX, ...)
calls in sinit.c, so maybe we're done. Embarassing that it
took three CLs to find all three.
Russ Cox [Wed, 28 May 2014 03:59:06 +0000 (23:59 -0400)]
cmd/gc: fix defer copy(x, <-c)
In the first very rough draft of the reordering code
that was introduced in the Go 1.3 cycle, the pre-allocated
temporary for a ... argument was held in n->right.
It moved to n->alloc but the code avoiding n->right
was left behind in order.c. In copy(x, <-c), the receive
is in n->right and must be processed. Delete the special
case code, removing the bug.
Russ Cox [Wed, 28 May 2014 01:38:19 +0000 (21:38 -0400)]
cmd/gc: fix conversion of runtime constant
The code cannot have worked before, because it was
trying to use the old value in a range check for the new
type, which might have a different representation
(hence the 'internal compiler error').
Rob Pike [Tue, 27 May 2014 21:37:36 +0000 (14:37 -0700)]
cmd/go: improve error message when import path contains http://
Common mistake (at least for me) because hg etc. require the prefix
while the go command forbids it.
Before:
% go get http://code.google.com/p/go.text/unicode/norm
package http:/code.google.com/p/go.text/unicode/norm: unrecognized import path "http:/code.google.com/p/go.text/unicode/norm"
After:
% go get http://code.google.com/p/go.text/unicode/norm
package http:/code.google.com/p/go.text/unicode/norm: "http://" not allowed in import path
Robert Griesemer [Thu, 22 May 2014 19:23:25 +0000 (12:23 -0700)]
spec: explicitly disallow blank methods in interface types
The spec was unclear about whether blank methods should be
permitted in interface types. gccgo permits at most one, gc
crashes if there are more than one, go/types permits at most
one.
Discussion:
Since method sets of non-interface types never contain methods
with blank names (blank methods are never declared), it is impossible
to satisfy an interface with a blank method.
It is possible to declare variables of assignable interface types
(but not necessarily identical types) containing blank methods, and
assign those variables to each other, but the values of those
variables can only be nil.
There appear to be two "reasonable" alternatives:
1) Permit at most one blank method (since method names must be unique),
and consider it part of the interface. This is what appears to happen
now, with corner-case bugs. Such interfaces can never be implemented.
2) Permit arbitrary many blank methods but ignore them. This appears
to be closer to the handling of blank identifiers in declarations.
However, an interface type literal is not a declaration (it's a type
literal). Also, for struct types, blank identifiers are not ignored;
so the analogy with declarations is flawed.
Both these alternatives don't seem to add any benefit and are likely
(if only slightly) more complicated to explain and implement than
disallowing blank methods in interfaces altogether.
Russ Cox [Thu, 22 May 2014 15:45:03 +0000 (11:45 -0400)]
doc/go1.3.html: change uintptr to integer in unsafe.Pointer section
The key property here is what the bit pattern represents,
not what its type is. Storing 5 into a pointer is the problem.
Storing a uintptr that holds pointer bits back into a pointer
is not as much of a problem, and not what we are claiming
the runtime will detect.
Longer discussion at
https://groups.google.com/d/msg/golang-nuts/dIGISmr9hw0/0jO4ce85Eh0J
Pietro Gagliardi [Wed, 21 May 2014 23:01:54 +0000 (16:01 -0700)]
cmd/cgo: explicitly state that #cgo directives across multiple files are concatenated
This is a quick documentation change/clarification, as this
confused me before: in my own cgo-based projects, I currently have
identical #cgo directives in each relevant source file, and I notice
with go build -x that cgo is combining the directives, leading to
pkg-config invocations with the same package name (gtk+-3.0, in my
case) repeated several times, or on Mac OS X, LDFLAGS listing
-framework Foundation -framework AppKit multiple times. Since I am
about to add a CFLAGS as well, I checked the source to cmd/cgo and
go/build (where the work is actually done) to see if that still holds
true there. Hopefully other people who have made the same mistake I
have (I don't know if anyone has) can remove the excess declarations
now; this should make things slightly easier to manage as well.
Rob Pike [Wed, 21 May 2014 19:30:43 +0000 (12:30 -0700)]
fmt: fix floating-point padding once and for all
Rewrite formatFloat to be much simpler and clearer and
avoid the tricky interaction with padding.
The issue refers to complex but the problem is just floating-point.
The new tests added were incorrectly formatted before this fix.
Fixes #8064.
Robert Griesemer [Wed, 21 May 2014 00:46:08 +0000 (17:46 -0700)]
spec: specify order of init() calls
The spec did not specify the order in which
init() functions are called. Specify that
they are called in source order since we have
now also specified the initialization order
of independent variables.
While technically a language change, no
existing code could have relied on this,
so this should not break anything.
Per suggestion from rsc.
LGTM=r, iant
R=rsc, iant, r, ken
CC=golang-codereviews
https://golang.org/cl/98420046
Robert Griesemer [Tue, 20 May 2014 20:51:39 +0000 (13:51 -0700)]
spec: clarify section on package initialization
- split description of package initialization and
program execution
- better grouping of concerns in section on package
initialization
- more explicit definition of what constitues a
dependency
- removed language about constant dependencies -
they are computed at compile-time and not
initialized at run-time
- clarified that independent variables are initialized
in declaration order (rather than reference order)
Note that the last clarification is what distinguishes
gc and gccgo at the moment: gc uses reference order
(i.e., order in which variables are referenced in
initialization expressions), while gccgo uses declaration
order for independent variables.
Not a language change. But adopting this CL will
clarify what constitutes a dependency.
Russ Cox [Tue, 20 May 2014 16:10:19 +0000 (12:10 -0400)]
build: make nacl pass
Add nacl.bash, the NaCl version of all.bash.
It's a separate script because it builds a variant of package syscall
with a large zip file embedded in it, containing all the input files
needed for tests.
Disable various tests new since the last round, mostly the ones using os/exec.
Russ Cox [Tue, 20 May 2014 15:35:20 +0000 (11:35 -0400)]
cmd/ld: make lldb happy with Mach-O 6.out files
Apparently all the __DWARF sections need addresses
even though they are marked as "do not load from disk".
Continue the address numbering from the data segment.
With this change:
g% lldb helloworld
Current executable set to 'helloworld' (x86_64).
(lldb) b main.main
Breakpoint 1: where = helloworld`main.main + 25 at helloworld.go:12, address = 0x0000000000002019
(lldb) r
Process 68509 launched: '/Users/rsc/g/go/src/cmd/6l/helloworld' (x86_64)
1 location added to breakpoint 1
(lldb) \e[KProcess 68509 stopped
* thread #1: tid = 0x8b7a27, 0x0000000000002019 helloworld`main.main + 25 at helloworld.go:12, stop reason = breakpoint 1.2
frame #0: 0x0000000000002019 helloworld`main.main + 25 at helloworld.go:12
9 package main
10
11 func main() {
-> 12 print("hello, world\n")
13 }
(lldb) bt
* thread #1: tid = 0x8b7a27, 0x0000000000002019 helloworld`main.main + 25 at helloworld.go:12, stop reason = breakpoint 1.2
* frame #0: 0x0000000000002019 helloworld`main.main + 25 at helloworld.go:12
(lldb) disas
helloworld`main.main at helloworld.go:11:
0x2000: movq %gs:0x8a0, %rcx
0x2009: cmpq (%rcx), %rsp
0x200c: ja 0x2015 ; main.main + 21 at helloworld.go:11
0x200e: callq 0x20da0 ; runtime.morestack00_noctxt at atomic_amd64x.c:28
0x2013: jmp 0x2000 ; main.main at helloworld.go:11
0x2015: subq $0x10, %rsp
-> 0x2019: leaq 0x2c2e0, %rbx
0x2021: leaq (%rsp), %rbp
0x2025: movq %rbp, %rdi
0x2028: movq %rbx, %rsi
0x202b: movsq
0x202d: movsq
0x202f: callq 0x10300 ; runtime.printstring at compiler.go:1
0x2034: addq $0x10, %rsp
0x2038: ret
0x2039: addb %al, (%rax)
0x203b: addb %al, (%rax)
0x203d: addb %al, (%rax)
(lldb) quit
Quitting LLDB will kill one or more processes. Do you really want to proceed: [Y/n] y
g%
Russ Cox [Tue, 20 May 2014 04:30:58 +0000 (00:30 -0400)]
liblink: fix field tracking
The USEFIELD instructions no longer make it to the linker,
so we have to do something else to pin the references
they were pinning. Emit a 0-length relocation of type R_USEFIELD.
Fixes #7486.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews, r
https://golang.org/cl/95530043
Russ Cox [Tue, 20 May 2014 04:30:46 +0000 (00:30 -0400)]
runtime: switch default stack size back to 8kB
The move from 4kB to 8kB in Go 1.2 was to eliminate many stack split hot spots.
The move back to 4kB was predicated on copying stacks eliminating
the potential for hot spots.
Unfortunately, the fact that stacks do not copy 100% of the time means
that hot spots can still happen under the right conditions, and the slowdown
is worse now than it was in Go 1.2. There is a real program in issue 8030 that
sees about a 30x slowdown: it has a reflect call near the top of the stack
which inhibits any stack copying on that segment.
Go back to 8kB until stack copying can be used 100% of the time.
Fixes #8030.
LGTM=khr, dave, iant
R=iant, khr, r, bradfitz, dave
CC=golang-codereviews
https://golang.org/cl/92540043
Russ Cox [Tue, 20 May 2014 02:57:59 +0000 (22:57 -0400)]
cmd/gc: fix float32 const conversion and printing of big float consts
The float32 const conversion used to round to float64
and then use the hardware to round to float32.
Even though there was a range check before this
conversion, the double rounding introduced inaccuracy:
the round to float64 might round the value further away
from the float32 range, reaching a float64 value that
could not actually be rounded to float32. The hardware
appears to give us 0 in that case, but it is probably undefined.
Double rounding also meant that the wrong value might
be used for certain border cases.
Do the rounding the float32 ourselves, just as we already
did the rounding to float64. This makes the conversion
precise and also makes the conversion match the range check.
Finally, add some code to print very large (bigger than float64)
floating point constants in decimal floating point notation instead
of falling back to the precise but human-unreadable binary floating
point notation.
Fixes #8015.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews, r
https://golang.org/cl/100580044
Shenghou Ma [Tue, 20 May 2014 02:39:42 +0000 (22:39 -0400)]
cmd/ld: abort if (32-bit) address relocation is negative on amd64.
Update #7980
This CL make the linker abort for the example program. For Go 1.4,
we need to find a general way to handle large memory model programs.
Russ Cox [Mon, 19 May 2014 16:30:25 +0000 (12:30 -0400)]
math/rand: restore Go 1.2 value stream for Float32, Float64
CL 22730043 fixed a bug in these functions: they could
return 1.0 despite documentation saying otherwise.
But the fix changed the values returned in the non-buggy case too,
which might invalidate programs depending on a particular
stream when using rand.Seed(0) or when passing their own
Source to rand.New.
The example test says:
// These tests serve as an example but also make sure we don't change
// the output of the random number generator when given a fixed seed.
so I think there is some justification for thinking we have
promised not to change the values. In any case, there's no point in
changing the values gratuitously: we can easily fix this bug without
changing the values, and so we should.
That CL just changed the test values too, which defeats the
stated purpose, but it was just a comment.
Add an explicit regression test, which might be
a clearer signal next time that we don't want to change
the values.
Fixes #6721. (again)
Fixes #8013.
LGTM=r
R=iant, r
CC=golang-codereviews
https://golang.org/cl/95460049