Andrew Gerrand [Mon, 29 Sep 2014 03:46:12 +0000 (13:46 +1000)]
[release-branch.go1.3] time: removed from tests now obsolete assumption about Australian tz abbreviations
««« CL 130920043 / 7dba9475ee72
time: removed from tests now obsolete assumption about Australian tz abbreviations
Australian timezones abbreviation for standard and daylight saving time were recently
changed from EST for both to AEST and AEDT in the icann tz database (see changelog
on www.iana.org/time-zones).
A test in the time package was written to check that the ParseInLocation function
understand that Feb EST and Aug EST are different time zones, even though they are
both called EST. This is no longer the case, and the Date function now returns
AEST or AEDT for australian tz on every Linux system with an up to date tz database
(and this makes the test fail).
Since I wasn't able to find another country that 1) uses daylight saving and 2) has
the same abbreviation for both on tzdata, I changed the test to make sure that
ParseInLocation does not get confused when it parses, in different locations, two
dates with the same abbreviation (this was suggested in the mailing list).
Andrew Gerrand [Thu, 25 Sep 2014 22:32:20 +0000 (22:32 +0000)]
[release-branch.go1.3] crypto/tls: ensure that we don't resume when tickets are disabled
A security bug affects programs that use crypto/tls to implement a TLS server
from Go 1.1 onwards. If the server enables TLS client authentication using
certificates (this is rare) and explicitly sets SessionTicketsDisabled to true
in the tls.Config, then a malicious client can falsely assert ownership of any
client certificate it wishes.
This issue was discovered internally and there is no evidence of exploitation.
Change authored by Adam Langley <agl@golang.org>
https://golang.org/cl/148080043/
Andrew Gerrand [Wed, 13 Aug 2014 03:18:02 +0000 (13:18 +1000)]
[release-branch.go1.3] net: prevent spurious on-connect events via epoll on linux
««« CL 120820043 / 06a4b59c1393
net: prevent spurious on-connect events via epoll on linux
On Linux, adding a socket descriptor to epoll instance before getting
the EINPROGRESS return value from connect system call could be a root
cause of spurious on-connect events.
See golang.org/issue/8276, golang.org/issue/8426 for further information.
Andrew Gerrand [Mon, 11 Aug 2014 23:45:11 +0000 (09:45 +1000)]
[release-branch.go1.3] crypto/rsa: fix out-of-bound access with short session keys.
««« CL 102670044 / c5f72a685e25
crypto/rsa: fix out-of-bound access with short session keys.
Thanks to Cedric Staub for noting that a short session key would lead
to an out-of-bounds access when conditionally copying the too short
buffer over the random session key.
Instead of immediately completing pointer type mappings, add them to
a queue to allow them to be completed later. This fixes issues caused
by Type() returning arbitrary in-progress type mappings.
glibc devs have apparently decided _BSD_SOURCE will be
deprecated on Linux, and issue a preprocessor warning if
declaring _BSD_SOURCE without _DEFAULT_SOURCE.
Andrew Gerrand [Fri, 25 Jul 2014 00:37:47 +0000 (10:37 +1000)]
[release-branch.go1.3] doc: drop scheme from links that are known to support HTTPS
««« CL 112650043 / bdac8d858e74
doc: drop scheme from links that are known to support HTTPS
golang.org now serves HTTPS with a valid cert, so it's reasonable
that users should click through to the HTTPS versions of *.golang.org
and other known sites.
Andrew Gerrand [Wed, 18 Jun 2014 23:54:17 +0000 (09:54 +1000)]
[release-branch.go1.3] fmt: fix signs when zero padding.
««« CL 103480043 / 777dd5a434db
fmt: fix signs when zero padding.
Bug was introduced recently. Add more tests, fix the bugs.
Suppress + sign when not required in zero padding.
Do not zero pad infinities.
All old tests still pass.
This time for sure!
Fixes #8217.
It appears that something about Go on Windows
cannot handle the fault cause by a jump to address 0.
The way Go represents and calls functions, this
never happened at all, until CL 105140044.
This CL changes the code added in CL 105140044
to make jump to 0 impossible once again.
Fixes #8047. (again, on Windows)
TBR=bradfitz
R=golang-codereviews, dave
CC=adg, golang-codereviews, iant, r
https://golang.org/cl/105120044
»»»
Russ Cox [Thu, 12 Jun 2014 20:55:36 +0000 (16:55 -0400)]
[release-branch.go1.3] runtime: do not trace past jmpdefer during pprof traceback on arm
««« CL 107970043 / b336da131a84
runtime: do not trace past jmpdefer during pprof traceback on arm
jmpdefer modifies PC, SP, and LR, and not atomically,
so walking past jmpdefer will often end up in a state
where the three are not a consistent execution snapshot.
This was causing warning messages a few frames later
when the traceback realized it was confused, but given
the right memory it could easily crash instead.
Update #8153
LGTM=minux, iant
R=golang-codereviews, minux, iant
CC=golang-codereviews, r
https://golang.org/cl/107970043
»»»
LGTM=r
R=golang-codereviews, r
CC=adg, golang-codereviews, iant
https://golang.org/cl/101260043
Russ Cox [Wed, 11 Jun 2014 21:00:17 +0000 (17:00 -0400)]
[release-branch.go1.3] cmd/gc: two escape analysis fixes
««« CL 108860043 / f153208c0a0e
cmd/gc: fix escape analysis for &x inside switch x := v.(type)
The analysis for &x was using the loop depth on x set
during x's declaration. A type switch creates a list of
implicit declarations that were not getting initialized
with loop depths.
There is a hierarchy of location defined by loop depth:
-1 = the heap
0 = function results
1 = local variables (and parameters)
2 = local variable declared inside a loop
3 = local variable declared inside a loop inside a loop
etc
In general if an address from loopdepth n is assigned to
something in loop depth m < n, that indicates an extended
lifetime of some form that requires a heap allocation.
Function results can be local variables too, though, and so
they don't actually fit into the hierarchy very well.
Treat the address of a function result as level 1 so that
if it is written back into a result, the address is treated
as escaping.
The putpclcdelta function set the DWARF line number PC to
s->value + pcline->pc, which is correct, but the code then set
the local variable pc to epc, which can be a different value.
This caused the next delta in the DWARF table to be wrong.
Russ Cox [Sat, 7 Jun 2014 02:07:32 +0000 (22:07 -0400)]
[release-branch.go1.3] runtime: fix panic stack during runtime.Goexit during panic
««« CL 102220043 / 00224712f89e
runtime: fix panic stack during runtime.Goexit during panic
A runtime.Goexit during a panic-invoked deferred call
left the panic stack intact even though all the stack frames
are gone when the goroutine is torn down.
The next goroutine to reuse that struct will have a
bogus panic stack and can cause the traceback routines
to walk into garbage.
Most likely to happen during tests, because t.Fatal might
be called during a deferred func and uses runtime.Goexit.
This "not enough cleared in Goexit" failure mode has
happened to us multiple times now. Clear all the pointers
that don't make sense to keep, not just gp->panic.
I am not sure what the rounding here was
trying to do, but it was skipping the first
pointer on native client.
The code above the rounding already checks
that xoffset is widthptr-aligned, so the rnd
was a no-op everywhere but on Native Client.
And on Native Client it was wrong.
Perhaps it was supposed to be rounding down,
not up, but zerorange handles the extra 32 bits
correctly, so the rnd does not seem to be necessary
at all.
This wouldn't be worth doing for Go 1.3 except
that it can affect code on the playground.
Russ Cox [Sat, 7 Jun 2014 02:07:11 +0000 (22:07 -0400)]
[release-branch.go1.3] doc: fix happens-before rules for buffered channels
««« CL 101980047 / 12c9a9ff50d8
doc: fix happens-before rules for buffered channels
The current wording is reversed in 2 places.
Not sure how it got 4 LGTMs (mine was there as well).
Update #6242.
Russ Cox [Tue, 3 Jun 2014 18:42:14 +0000 (14:42 -0400)]
[release-branch.go1.3] cmd/gc: fix escape analysis of func returning indirect of parameter
««« CL 102040046 / a078b2056ebc
cmd/gc: fix escape analysis of func returning indirect of parameter
I introduced this bug when I changed the escape
analysis to run in phases based on call graph
dependency order, in order to be more precise about
inputs escaping back to outputs (functions returning
their arguments).
Given
func f(z **int) *int { return *z }
we were tagging the function as 'z does not escape
and is not returned', which is all true, but not
enough information.
If used as:
var x int
p := &x
q := &p
leak(f(q))
then the compiler might try to keep x, p, and q all
on the stack, since (according to the recorded
information) nothing interesting ends up being
passed to leak.
In fact since f returns *q = p, &x is passed to leak
and x needs to be heap allocated.
To trigger the bug, you need a chain that the
compiler wants to keep on the stack (like x, p, q
above), and you need a function that returns an
indirect of its argument, and you need to pass the
head of the chain to that function. This doesn't
come up very often: this bug has been present since
June 2012 (between Go 1 and Go 1.1) and we haven't
seen it until now. It helps that most functions that
return indirects are getters that are simple enough
to be inlined, avoiding the bug.
Earlier versions of Go also had the benefit that if
&x really wasn't used beyond x's lifetime, nothing
broke if you put &x in a heap-allocated structure
accidentally. With the new stack copying, though,
heap-allocated structures containing &x are not
updated when the stack is copied and x moves,
leading to crashes in Go 1.3 that were not crashes
in Go 1.2 or Go 1.1.
The fix is in two parts.
First, in the analysis of a function, recognize when
a value obtained via indirect of a parameter ends up
being returned. Mark those parameters as having
content escape back to the return results (but we
don't bother to write down which result).
Second, when using the analysis to analyze, say,
f(q), mark parameters with content escaping as
having any indirections escape to the heap. (We
don't bother trying to match the content to the
return value.)
The fix could be less precise (simpler).
In the first part we might mark all content-escaping
parameters as plain escaping, and then the second
part could be dropped. Or we might assume that when
calling f(q) all the things pointed at by q escape
always (for any f and q).
The fix could also be more precise (more complex).
We might record the specific mapping from parameter
to result along with the number of indirects from the
parameter to the thing being returned as the result,
and then at the call sites we could set up exactly the
right graph for the called function. That would make
notleaks(f(q)) be able to keep x on the stack, because
the reuslt of f(q) isn't passed to anything that leaks it.
The less precise the fix, the more stack allocations
become heap allocations.
This fix is exactly as precise as it needs to be so that
none of the current stack allocations in the standard
library turn into heap allocations.
Fixes #8120.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews, khr, r
https://golang.org/cl/102040046
»»»
David Symonds [Tue, 3 Jun 2014 01:20:32 +0000 (11:20 +1000)]
[release-branch.go1.3] runtime: fix 1-byte return during x.(T) for 0-byte T
««« CL 100940043 / 93baf7bea171
runtime: fix 1-byte return during x.(T) for 0-byte T
The 1-byte write was silently clearing a byte on the stack.
If there was another function call with more arguments
in the same stack frame, no harm done.
Otherwise, if the variable at that location was already zero,
no harm done.
Otherwise, problems.
Fixes #8139.
LGTM=dsymonds
R=golang-codereviews, dsymonds
CC=golang-codereviews, iant, r
https://golang.org/cl/100940043
»»»
David Symonds [Tue, 3 Jun 2014 01:19:11 +0000 (11:19 +1000)]
[release-branch.go1.3] cmd/gc: don't generate zillions of linehists for wrapper functions
««« CL 104840043 / 876107512a67
cmd/gc: don't generate zillions of linehists for wrapper functions
This is a workaround - the code should be better than this - but the
fix avoids generating large numbers of linehist entries for the wrapper
functions that enable interface conversions. There can be many of
them, they all happen at the end of compilation, and they can all
share a linehist entry.
Avoids bad n^2 behavior in liblink.
Test case in issue 8135 goes from 64 seconds to 2.5 seconds (still bad
but not intolerable).
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.