Adam Langley [Sun, 9 Nov 2014 01:12:23 +0000 (17:12 -0800)]
lib/codereview: fix with more recent hg revisions.
I've Mercurial version 3.2 and hg submit fails with:
File "/home/agl/devel/go/lib/codereview/codereview.py", line 3567, in get_hg_status
ret = hg_commands.status(fui, self.repo, *[], **{'rev': [rev], 'copies': True})
File "/usr/lib/python2.7/site-packages/mercurial/commands.py", line 5714, in status
fm = ui.formatter('status', opts)
File "/home/agl/devel/go/lib/codereview/codereview.py", line 3464, in formatter
return plainformatter(self, topic, opts)
File "/usr/lib/python2.7/site-packages/mercurial/formatter.py", line 57, in __init__
if ui.debugflag:
AttributeError: 'FakeMercurialUI' object has no attribute 'debugflag'
This change dumbly adds a boolean debugflag and that seems to work.
Russ Cox [Fri, 7 Nov 2014 00:56:55 +0000 (19:56 -0500)]
cmd/objdump, cmd/pprof: factor disassembly into cmd/internal/objfile
Moving so that new Go 1.4 pprof can use it.
The old 'GNU objdump workalike' mode for 'go tool objdump'
is now gone, as are the tests for that mode. It was used only
by pre-Go 1.4 pprof. You can still specify an address range on
the command line; you just get the same output format as
you do when dumping the entire binary (without an address
limitation).
Keith Randall [Thu, 6 Nov 2014 17:30:41 +0000 (09:30 -0800)]
runtime: don't stop bitmap dump at BitsDead
Stack bitmaps need to be scanned past any BitsDead entries.
Object bitmaps will not have any BitsDead in them (bitmap extraction stops at
the first BitsDead entry in makeheapobjbv). data/bss bitmaps also have no BitsDead entries.
Keith Randall [Thu, 6 Nov 2014 04:25:20 +0000 (20:25 -0800)]
os/exec: tell lsof not to block
For some reason lsof is now hanging on my workstation
without the -b (avoid blocking in the kernel) option.
Adding -b makes the test pass and shouldn't hurt.
I don't know how recent the -b option is. If the builders
are ok with it, it's probably ok.
Russ Cox [Thu, 6 Nov 2014 04:01:48 +0000 (23:01 -0500)]
runtime: avoid gentraceback of self on user goroutine stack
Gentraceback may grow the stack.
One of the gentraceback wrappers may grow the stack.
One of the gentraceback callback calls may grow the stack.
Various stack pointers are stored in various stack locations
as type uintptr during the execution of these calls.
If the stack does grow, these stack pointers will not be
updated and will start trying to decode stack memory that
is no longer valid.
It may be possible to change the type of the stack pointer
variables to be unsafe.Pointer, but that's pretty subtle and
may still have problems, even if we catch every last one.
An easier, more obviously correct fix is to require that
gentraceback of the currently running goroutine must run
on the g0 stack, not on the goroutine's own stack.
Not doing this causes faults when you set
StackFromSystem = 1
StackFaultOnFree = 1
The new check in gentraceback will catch future lapses.
The more general problem is calling getcallersp but then
calling a function that might relocate the stack, which would
invalidate the result of getcallersp. Add note to stubs.go
declaration of getcallersp explaining the problem, and
check all existing calls to getcallersp. Most needed fixes.
This affects Callers, Stack, and nearly all the runtime
profiling routines. It does not affect stack copying directly
nor garbage collection.
LGTM=khr
R=khr, bradfitz
CC=golang-codereviews, r
https://golang.org/cl/167060043
Rob Pike [Wed, 5 Nov 2014 22:57:46 +0000 (09:57 +1100)]
bufio: don't loop generating empty tokens
The new rules for split functions mean that we are exposed
to the common bug of a function that loops forever at EOF.
Pick these off by shutting down the scanner if too many
consecutive empty tokens are delivered.
Austin Clements [Wed, 5 Nov 2014 20:14:47 +0000 (15:14 -0500)]
5g: don't generate reg variables for direct-called functions
The test intended to skip direct calls when creating
registerization variables was testing p->to.type instead of
p->to.name, so it always failed, causing regopt to create
unnecessary variables for these names.
Ian Lance Taylor [Tue, 4 Nov 2014 18:20:35 +0000 (10:20 -0800)]
test: comment out failing cases from sinit.go
One failing case this removes is:
var bytes = []byte("hello, world")
var copy_bytes = bytes
We could handle this in the compiler, but it requires special
case for a variable that is initialized to the value of a
variable that is initialized to a string literal converted to
[]byte. This seems an unlikely case--it never occurs in the
standrd library--and it seems unnecessary to write the code to
handle it.
If we do want to support this case, one approach is
https://golang.org/cl/171840043.
The other failing cases are of the form
var bx bool
var copy_bx = bx
The compiler used to initialize copy_bx to false. However,
that led to issue 7665, since bx may be initialized in non-Go
code. The compiler no longer assumes that bx must be false,
so copy_bx can not be statically initialized.
We can fix these with https://golang.org/cl/169040043
if we also pass -complete to the compiler as part of this
test. This is OK but it's too late in the release cycle.
Austin Clements [Tue, 4 Nov 2014 14:43:37 +0000 (09:43 -0500)]
gc: abort if given an unknown debug (-d) flag
The check for unknown command line debug flags in gc was
incorrect: the loop over debugtab terminates when it reaches a
nil entry, but it was only reporting an error if the parser
had passed the last entry of debugtab (which it never did).
Fix this by reporting the usage error if the loop reaches a
nil entry.
Austin Clements [Mon, 3 Nov 2014 18:26:46 +0000 (13:26 -0500)]
runtime: make Go and C mallocgc signatures match
Previously, the flags argument to mallocgc was an int in Go,
but a uint32 in C. Change the Go type to use uint32 so these
agree. The largest flag value is 2 (and of course no flag
values are negative), so this won't change anything on little
endian architectures, but it matters on big endian.
Russ Cox [Thu, 30 Oct 2014 00:37:44 +0000 (20:37 -0400)]
runtime: change top-most return PC from goexit to goexit+PCQuantum
If you get a stack of PCs from Callers, it would be expected
that every PC is immediately after a call instruction, so to find
the line of the call, you look up the line for PC-1.
CL 163550043 now explicitly documents that.
The most common exception to this is the top-most return PC
on the stack, which is the entry address of the runtime.goexit
function. Subtracting 1 from that PC will end up in a different
function entirely.
To remove this special case, make the top-most return PC
goexit+PCQuantum and then implement goexit in assembly
so that the first instruction can be skipped.
Russ Cox [Wed, 29 Oct 2014 22:07:24 +0000 (18:07 -0400)]
cmd/objdump: use cmd/internal/objfile
This removes a bunch of ugly duplicate code.
The end goal is to factor the disassembly code
into cmd/internal/objfile too, so that pprof can use it,
but one step at a time.
Russ Cox [Wed, 29 Oct 2014 19:14:24 +0000 (15:14 -0400)]
runtime: fix line number in first stack frame in printed stack trace
Originally traceback was only used for printing the stack
when an unexpected signal came in. In that case, the
initial PC is taken from the signal and should be used
unaltered. For the callers, the PC is the return address,
which might be on the line after the call; we subtract 1
to get to the CALL instruction.
Traceback is now used for a variety of things, and for
almost all of those the initial PC is a return address,
whether from getcallerpc, or gp->sched.pc, or gp->syscallpc.
In those cases, we need to subtract 1 from this initial PC,
but the traceback code had a hard rule "never subtract 1
from the initial PC", left over from the signal handling days.
Change gentraceback to take a flag that specifies whether
we are tracing a trap.
Change traceback to default to "starting with a return PC",
which is the overwhelmingly common case.
Add tracebacktrap, like traceback but starting with a trap PC.
Use tracebacktrap in signal handlers.
Fixes #7690.
LGTM=iant, r
R=r, iant
CC=golang-codereviews
https://golang.org/cl/167810044
David du Colombier [Tue, 28 Oct 2014 21:44:59 +0000 (22:44 +0100)]
os: fix write on Plan 9
In CL 160670043 the write function was changed
so a zero-length write is now allowed. This leads
the ExampleWriter_Init test to fail.
The reason is that Plan 9 preserves message
boundaries, while the os library expects systems
that don't preserve them. We have to ignore
zero-length writes so they will never turn into EOF.
This issue was previously discussed in CL 7406046.
Russ Cox [Tue, 28 Oct 2014 15:35:00 +0000 (11:35 -0400)]
syscall: fix ParseRoutingSockaddr with unexpected submessages
No easy way to test (would have to actually trigger some routing
events from kernel) but the code is clearly wrong as written.
If the header says there is a submessage, we need to at least
skip over its bytes, not just continue to the next iteration.
Fixes #8203.
LGTM=r
R=r
CC=golang-codereviews, mikioh.mikioh, p
https://golang.org/cl/164140044
Russ Cox [Tue, 28 Oct 2014 15:14:25 +0000 (11:14 -0400)]
cmd/go: add get -f flag
get -u now checks that remote repo paths match the
ones predicted by the import paths: if you are get -u'ing
rsc.io/pdf, it has to be checked out from the right location.
This is important in case the rsc.io/pdf redirect changes.
In some cases, people have good reasons to use
non-standard remote repos. Add -f flag to allow that.
The f can stand for force or fork, as you see fit.
Mikio Hara [Tue, 28 Oct 2014 07:20:49 +0000 (16:20 +0900)]
net: add test for lookupIPDeadline
Just to confirm the fix, by typing the follwing:
go test -run=TestLookupIPDeadline -dnsflood or
go test -run=TestLookupIPDeadline -dnsflood -tags netgo
Rob Pike [Tue, 28 Oct 2014 00:08:50 +0000 (17:08 -0700)]
doc/go_mem.html: don't be clever
Add a short introductory section saying what most Go
programmers really need to know, which is that you
shouldn't have to read this document to understand
the behavior of your program.
Robert Griesemer [Mon, 27 Oct 2014 23:31:15 +0000 (16:31 -0700)]
spec: permit parentheses around builtin function names
Not a language change.
This is simply documenting the status quo which permits
builtin function names to be parenthesized in calls; e.g.,
both
len(s)
and
(((len)))(s)
are accepted by all compilers and go/types.
Changed the grammar by merging the details of BuiltinCall
with ordinary Calls. Also renamed the Call production to
Arguments which more clearly identifies that part of the
grammar and also matches better with its counterpart on
the declaration side (Parameters).
The fact that the first argument can be a type (for builtins)
or cannot be a type (for regular function calls) is expressed
in the prose, no need to make the grammar more complicated.
Fixes #9001.
LGTM=iant, r, rsc
R=r, rsc, iant, ken, dave
CC=golang-codereviews
https://golang.org/cl/160570043
Russ Cox [Mon, 27 Oct 2014 22:59:02 +0000 (18:59 -0400)]
test: make maplinear more robust
The test just doubled a certain number of times
and then gave up. On a mostly fast but occasionally
slow machine this may never make the test run
long enough to see the linear growth.
Change test to keep doubling until the first round
takes at least a full second, to reduce the effect of
occasional scheduling or other jitter.
The failure we saw had a time for the first round
of around 100ms.
Note that this test still passes once it sees a linear
effect, even with a very small total time.
The timeout here only applies to how long the execution
must be to support a reported failure.
Austin Clements [Mon, 27 Oct 2014 21:12:48 +0000 (17:12 -0400)]
runtime: fix endianness assumption when decoding ftab
The ftab ends with a half functab record consisting only of
the 'entry' field followed by a uint32 giving the offset of
the next table. Previously, symtabinit assumed it could read
this uint32 as a uintptr. Since this is unsafe on big endian,
explicitly read the offset as a uint32.
Ian Lance Taylor [Mon, 27 Oct 2014 15:46:18 +0000 (08:46 -0700)]
net: if a DNS lookup times out, forget that it is in flight
Before this CL, if the system resolver does a very slow DNS
lookup for a particular host, all subsequent requests for that
host will hang waiting for that lookup to complete. That is
more or less expected when Dial is called with no deadline.
When Dial has a deadline, though, we can accumulate a large
number of goroutines waiting for that slow DNS lookup. Try to
avoid this problem by restarting the DNS lookup when it is
redone after a deadline is passed.
This CL also avoids creating an extra goroutine purely to
handle the deadline.
No test because we would have to simulate a slow DNS lookup
followed by a fast DNS lookup.
Emil Hessman [Mon, 27 Oct 2014 01:43:14 +0000 (12:43 +1100)]
misc/makerelease/windows: fix 404 help URL in installer
ARPHELPLINK yields 404; update the URL.
While here, also prefix the ARPREADME and ARPURLINFOABOUT URL's with the HTTP scheme to make 'em clickable links in the Add or Remove Programs listing.
Rob Pike [Fri, 24 Oct 2014 16:37:25 +0000 (09:37 -0700)]
unsafe: document that unsafe programs are not protected
The compatibility guideline needs to be clear about this even
though it means adding a clause that was not there from the
beginning. It has always been understood, so this isn't really
a change in policy, just in its expression.
Russ Cox [Fri, 24 Oct 2014 14:27:39 +0000 (10:27 -0400)]
cmd/gc: synthesize zeroed value for non-assignment context
CL 157910047 introduced code to turn a node representing
a zeroed composite literal into N, the nil Node* pointer
(which represents any zero, not the Go literal nil).
That's great for assignments like x = T{}, but it doesn't work
when T{} is used in a value context like T{}.v or x == T{}.
Fix those.
Should have no effect on performance; confirmed.
The deltas below are noise (compare ns/op):
Russ Cox [Fri, 24 Oct 2014 03:44:47 +0000 (23:44 -0400)]
encoding/csv: for Postgres, unquote empty strings, quote \.
In theory both of these lines encode the same three fields:
a,,c
a,"",c
However, Postgres defines that when importing CSV, the unquoted
version is treated as NULL (missing), while the quoted version is
treated as a string value (empty string). If the middle field is supposed to
be an integer value, the first line can be imported (NULL is okay), but
the second line cannot (empty string is not).
Postgres's import command (COPY FROM) has an option to force
the unquoted empty to be interpreted as a string but it does not
have an option to force the quoted empty to be interpreted as a NULL.
From http://www.postgresql.org/docs/9.0/static/sql-copy.html:
The CSV format has no standard way to distinguish a NULL
value from an empty string. PostgreSQL's COPY handles this
by quoting. A NULL is output as the NULL parameter string
and is not quoted, while a non-NULL value matching the NULL
parameter string is quoted. For example, with the default
settings, a NULL is written as an unquoted empty string,
while an empty string data value is written with double
quotes (""). Reading values follows similar rules. You can
use FORCE_NOT_NULL to prevent NULL input comparisons for
specific columns.
Therefore printing the unquoted empty is more flexible for
imports into Postgres than printing the quoted empty.
In addition to making the output more useful with Postgres, not
quoting empty strings makes the output smaller and easier to read.
It also matches the behavior of Microsoft Excel and Google Drive.
Since we are here and making concessions for Postgres, handle this
case too (again quoting the Postgres docs):
Because backslash is not a special character in the CSV
format, \., the end-of-data marker, could also appear as a
data value. To avoid any misinterpretation, a \. data value
appearing as a lone entry on a line is automatically quoted
on output, and on input, if quoted, is not interpreted as
the end-of-data marker. If you are loading a file created by
another application that has a single unquoted column and
might have a value of \., you might need to quote that value
in the input file.
Dmitriy Vyukov [Wed, 22 Oct 2014 16:23:49 +0000 (20:23 +0400)]
sync: release Pool memory during second and later GCs
Pool memory was only being released during the first GC after the first Put.
Put assumes that p.local != nil means p is on the allPools list.
poolCleanup (called during each GC) removed each pool from allPools
but did not clear p.local, so each pool was cleared by exactly one GC
and then never cleared again.
This bug was introduced late in the Go 1.3 release cycle.
Dave Cheney [Wed, 22 Oct 2014 01:30:15 +0000 (12:30 +1100)]
runtime/cgo: encode BLX directly, fixes one clang build error on arm
Fixes #8348.
Trying to work around clang's dodgy support for .arch by reverting to the external assembler didn't work out so well. Minux had a much better solution to encode the instructions we need as .word directives which avoids .arch altogether.
I've confirmed with gdb that this form produces the expected machine code
Jens Frederich [Tue, 21 Oct 2014 23:56:26 +0000 (10:56 +1100)]
misc/makerelease: set version number in Windows installer
Set correct version number at Windows installer based on
Go's Mercurial tag.
Name | Version
------------------------------------------------
Go Programming Language amd64 go1.3.3 | 1.3.3
Go Programming Language amd64 go1.2rc3 | 1.2
Go Programming Language amd64 go1.2beta1 | 1.2
Some platforms are still not working even with the integrated assembler disabled, will have to find another solution.
««« original CL description
cmd/cgo: disable clang's integrated assembler
Fixes #8348.
Clang's internal assembler (introduced by default in clang 3.4) understands the .arch directive, but doesn't change the default value of -march. This causes the build to fail when we use BLX (armv5 and above) when clang is compiled for the default armv4t architecture (which appears to be the default on all the distros I've used).
This is probably a clang bug, so work around it for the time being by disabling the integrated assembler when compiling the cgo assembly shim.
This CL also includes a small change to ldelf.c which was required as clang 3.4 and above generate more weird symtab entries.