Brad Fitzpatrick [Tue, 30 Jun 2015 16:22:41 +0000 (09:22 -0700)]
[release-branch.go1.4] net/textproto: don't treat spaces as hyphens in header keys
This was originally done in https://codereview.appspot.com/5690059
(Feb 2012) to deal with bad response headers coming back from webcams,
but it presents a potential security problem with HTTP request
smuggling for request headers containing "Content Length" instead of
"Content-Length".
Part of overall HTTP hardening for request smuggling. See RFC 7230.
Shenghou Ma [Wed, 11 Mar 2015 16:07:50 +0000 (12:07 -0400)]
[release-branch.go1.4] runtime: don't return a slice with nil ptr but non-zero len from growslice
Fixes #10135.
Change-Id: Ic4c5ab15bcb7b9c3fcc685a788d3b59c60c26e1e Signed-off-by: Shenghou Ma <minux@golang.org>
Reviewed-on: https://go-review.googlesource.com/7400 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-on: https://go-review.googlesource.com/14248 Reviewed-by: Andrew Gerrand <adg@golang.org>
Keith Randall [Sat, 14 Feb 2015 23:46:36 +0000 (15:46 -0800)]
[release-branch.go1.4] runtime: don't fail if we find a pointer to an invalid span on 32 bit
The 32-bit heap may have holes in it. Pointers to (non-heap) objects
in those holes shouldn't cause the GC to throw.
This change is somewhat of a band-aid fix for 1.4.2. We should do
a more thorough fix for tip (keep track of the holes in the heap
with special MSpans, say).
Ian Lance Taylor [Wed, 21 Jan 2015 05:32:22 +0000 (21:32 -0800)]
[release-branch.go1.4] cmd/gc: treat non-local vars inlined into wrapper as escaping
The compiler has a phase ordering problem. Escape analysis runs
before wrapper generation. When a generated wrapper calls a method
defined in a different package, if that call is inlined, there will be
no escape information for the variables defined in the inlined call.
Those variables will be placed on the stack, which fails if they
actually do escape.
There are probably various complex ways to fix this. This is a simple
way to avoid it: when a generated wrapper calls a method defined in a
different package, treat all local variables as escaping.
Fixes #9537.
Change-Id: I530f39346de16ad173371c6c3f69cc189351a4e9
Reviewed-on: https://go-review.googlesource.com/3092 Reviewed-by: Russ Cox <rsc@golang.org>
(cherry picked from commit ec0ebc2281f79294c299ece35c5a690a6415e0e0)
Reviewed-on: https://go-review.googlesource.com/5003
Run-TryBot: Andrew Gerrand <adg@golang.org> Reviewed-by: Andrew Gerrand <adg@golang.org>
Shenghou Ma [Tue, 17 Feb 2015 04:43:40 +0000 (23:43 -0500)]
[release-branch.go1.4] runtime: don't panic when given a callback with no input params on windows
Fixes #9871 for Go 1.4.
Change-Id: I550a5bdb29e9a872652e0dd468a434227d7d9502
Reviewed-on: https://go-review.googlesource.com/4937
Run-TryBot: Minux Ma <minux@golang.org> Reviewed-by: Rob Pike <r@golang.org> Reviewed-by: Alex Brainman <alex.brainman@gmail.com> Reviewed-by: Andrew Gerrand <adg@golang.org>
Keith Randall [Thu, 15 Jan 2015 22:39:58 +0000 (14:39 -0800)]
[release-branch.go1.4] cmd/5g: make sure we normalize after unary ops on small types
We were failing ^uint16(0xffff) == 0, as we computed 0xffff0000 instead.
I could only trigger a failure for the above case, the other two tests
^uint16(0xfffe) == 1 and -uint16(0xffff) == 1 didn't seem to fail
previously. Somehow they get MOVHUs inserted for other reasons (used
by CMP instead of TST?). I fixed OMINUS anyway, better safe than
sorry.
Fixes #9604
Change-Id: I4c2d5bdc667742873ac029fdbe3db0cf12893c27
Reviewed-on: https://go-review.googlesource.com/2940 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Minux Ma <minux@golang.org>
(cherry picked from commit daa64ddfe64dda368e80cf224dc485fa63386f81)
Reviewed-on: https://go-review.googlesource.com/5002
Russ Cox [Wed, 14 Jan 2015 06:23:26 +0000 (01:23 -0500)]
[release-branch.go1.4] cmd/go: adjust error for custom import checkout mismatch
Before:
...
imports golang.org/x/net/context: /Users/rsc/g/src/golang.org/x/net is from https://code.google.com/p/go.net, should be from https://go.googlesource.com/net
After:
...
imports golang.org/x/net/context: golang.org/x/net is a custom import path for https://go.googlesource.com/net, but /Users/rsc/g/src/golang.org/x/net is checked out from https://code.google.com/p/go.net
Change-Id: I93c35b85f955c7de684f71fbd4baecc717405318
Reviewed-on: https://go-review.googlesource.com/2808 Reviewed-by: Andrew Gerrand <adg@golang.org> Reviewed-by: Rob Pike <r@golang.org>
(cherry picked from commit b8d67596f67ea13525e752a02f45c9d9f346472d)
Reviewed-on: https://go-review.googlesource.com/2813 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Keith Randall [Mon, 15 Dec 2014 22:39:28 +0000 (14:39 -0800)]
[release-branch.go1.4] runtime: fix deadlock in runtime.Stack
It shouldn't semacquire() inside an acquirem(), the runtime
thinks that means deadlock. It actually isn't a deadlock, but it
looks like it because acquirem() does m.locks++.
Candidate for inclusion in 1.4.1. runtime.Stack with all=true
is pretty unuseable in GOMAXPROCS>1 environment.
fixes #9321
Change-Id: Iac6b664217d24763b9878c20e49229a1ecffc805
Reviewed-on: https://go-review.googlesource.com/1600 Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
(cherry picked from commit 50bc3d5bbc6710663c082aa72c8ba4f9ee515ab3)
Reviewed-on: https://go-review.googlesource.com/2807 Reviewed-by: Andrew Gerrand <adg@golang.org>
CL 2789 backported a change that required a barrage of followup CLs.
This CL backports all the followup CLs together.
There are manual edits to os_plan9.go and syscall_windows.go to take
the place of edits to defs_windows_{amd64,386}.go and os2_plan9.go
in the original. Those files do not exist in the release branch, but the
definition being added must go somewhere.
Original change descriptions below.
---
runtime/cgo: initialize our pthread_create wrapper earlier on openbsd
This is a genuine bug exposed by our test for issue 9456: our wrapper
for pthread_create is not initialized until we initialize cgo itself,
but it is possible that a static constructor could call pthread_create,
and in that case, it will be calling a nil function pointer.
Fix that by also initializing the sys_pthread_create function pointer
inside our pthread_create wrapper function, and use a pthread_once to
make sure it is only initialized once.
Fix build for openbsd.
Change-Id: Ica4da2c21fcaec186fdd3379128ef46f0e767ed7
Reviewed-on: https://go-review.googlesource.com/2232 Reviewed-by: David Crawshaw <crawshaw@golang.org>
(cherry picked from commit 77cd6197d7561ab7ccbf5d892efb6f97d929546a)
---
runtime: provide a dummy value of _SIGPROF on plan9 and windows
Fixes build on plan9 and windows.
Change-Id: Ic9b02c641ab84e4f6d8149de71b9eb495e3343b2
Reviewed-on: https://go-review.googlesource.com/2233 Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
(cherry picked from commit 1f282385579fc404f1246fd7ffa8b4e517401d19)
---
runtime/cgo: remove unused variable
I missed this one in golang.org/cl/2232 and only tested the patch
on openbsd/amd64.
Change-Id: I4ff437ae0bfc61c989896c01904b6d33f9bdf0ec
Reviewed-on: https://go-review.googlesource.com/2234 Reviewed-by: Minux Ma <minux@golang.org>
(cherry picked from commit 0b2a74e89cf940e1c4cd91785ff3d744684edc49)
---
runtime: skip TestCgoExternalThreadSIGPROF on OS X 10.6
The test program requires static constructor, which in turn needs
external linking to work, but external linking never works on 10.6.
Keith Randall [Sat, 20 Dec 2014 04:44:18 +0000 (20:44 -0800)]
[release-branch.go1.4] runtime: hashmap: move overflow pointer to end of bucket
Pointers to zero-sized values may end up pointing to the next
object in memory, and possibly off the end of a span. This
can cause memory leaks and/or confuse the garbage collector.
By putting the overflow pointer at the end of the bucket, we
make sure that pointers to any zero-sized keys or values don't
accidentally point to the next object in memory.
fixes #9384
Change-Id: I5d434df176984cb0210b4d0195dd106d6eb28f73
Reviewed-on: https://go-review.googlesource.com/1869 Reviewed-by: Russ Cox <rsc@golang.org>
(cherry picked from commit fbc56cf05015899aba236d5a68096a770de3ad0a)
Reviewed-on: https://go-review.googlesource.com/2801 Reviewed-by: Andrew Gerrand <adg@golang.org>
Keith Randall [Tue, 23 Dec 2014 03:07:05 +0000 (19:07 -0800)]
[release-branch.go1.4] reflect: add kindNoPointers if a function layout has no pointers.
malloc checks kindNoPointers and if it is not set and the object
is one pointer in size, it assumes it contains a pointer. So we
must set kindNoPointers correctly; it isn't just a hint.
Fixes #9425
Change-Id: Ia43da23cc3298d6e3d6dbdf66d32e9678f0aedcf
Reviewed-on: https://go-review.googlesource.com/2055 Reviewed-by: Russ Cox <rsc@golang.org>
(cherry picked from commit d11f41118116e0b5c2fb3b3296323d888dff2d6e)
Reviewed-on: https://go-review.googlesource.com/2800 Reviewed-by: Andrew Gerrand <adg@golang.org>
Shenghou Ma [Sun, 28 Dec 2014 00:15:38 +0000 (19:15 -0500)]
[release-branch.go1.4] runtime: ignore SIGPROF to foreign threads before cgocallback is fully initialized
Some libraries, for example, OpenBLAS, create work threads in a global constructor.
If we're doing cpu profiling, it's possible that SIGPROF might come to some of the
worker threads before we make our first cgo call. Cgocallback used to terminate the
process when that happens, but it's better to miss a couple profiling signals than
to abort in this case.
Fixes #9456.
Change-Id: I112b8e1a6e10e6cc8ac695a4b518c0f577309b6b
Reviewed-on: https://go-review.googlesource.com/2141 Reviewed-by: Ian Lance Taylor <iant@golang.org>
(cherry picked from commit 5da9c8cd0a0427d1771b3a9a6d8d931430ce50dd)
Reviewed-on: https://go-review.googlesource.com/2789 Reviewed-by: Andrew Gerrand <adg@golang.org>
Russ Cox [Wed, 14 Jan 2015 04:57:27 +0000 (23:57 -0500)]
[release-branch.go1.4] doc: copy contribute.html and install-source.html from master
This incorporates the various git-related updates that have
happened since the Go 1.4 release. Since Go 1.4.1 will be issued
from Git, it is appropriate to replace the Mercurial instructions
with Git instructions.
Change-Id: Idec041002c7f325c4eee6f25c50423b088b11468
Reviewed-on: https://go-review.googlesource.com/2788 Reviewed-by: Andrew Gerrand <adg@golang.org>
Russ Cox [Fri, 5 Dec 2014 19:04:17 +0000 (14:04 -0500)]
[release-branch.go1.4] cmd/api: make API check fail for undeclared API in release branch
We forgot to do the usual API review.
Make that not possible in the future.
I'll pull this change over to the main
branch too, but it's more important
(and only testable) here.
Scanner can't handle stupid long lines and there are
reports of stupid long lines in production.
Note the issue isn't long "//go:generate" lines, but
any long line in any Go source file.
To be fair, if you're going to have a stupid long line
it's not a bad bet you'll want to run it through go
generate, because it's some embeddable asset that
has been machine generated. (One could ask why
that generation process didn't add a newline or two,
but we should cope anyway.)
Rewrite the file scanner in "go generate" so it can
handle arbitrarily long lines, and only stores in memory
those lines that start "//go:generate".
Also: Adjust the documentation to make clear that it
does not parse the file.
Russ Cox [Mon, 1 Dec 2014 21:42:41 +0000 (16:42 -0500)]
[release-branch.go1.4] runtime: fix hang in GC due to shrinkstack vs netpoll race
««« CL 179680043 / 752cd9199639
runtime: fix hang in GC due to shrinkstack vs netpoll race
During garbage collection, after scanning a stack, we think about
shrinking it to reclaim some memory. The shrinking code (called
while the world is stopped) checked that the status was Gwaiting
or Grunnable and then changed the state to Gcopystack, to essentially
lock the stack so that no other GC thread is scanning it.
The same locking happens for stack growth (and is more necessary there).
oldstatus = runtime·readgstatus(gp);
oldstatus &= ~Gscan;
if(oldstatus == Gwaiting || oldstatus == Grunnable)
runtime·casgstatus(gp, oldstatus, Gcopystack); // oldstatus is Gwaiting or Grunnable
else
runtime·throw("copystack: bad status, not Gwaiting or Grunnable");
Unfortunately, "stop the world" doesn't stop everything. It stops all
normal goroutine execution, but the network polling thread is still
blocked in epoll and may wake up. If it does, and it chooses a goroutine
to mark runnable, and that goroutine is the one whose stack is shrinking,
then it can happen that between readgstatus and casgstatus, the status
changes from Gwaiting to Grunnable.
casgstatus assumes that if the status is not what is expected, it is a
transient change (like from Gwaiting to Gscanwaiting and back, or like
from Gwaiting to Gcopystack and back), and it loops until the status
has been restored to the expected value. In this case, the status has
changed semi-permanently from Gwaiting to Grunnable - it won't
change again until the GC is done and the world can continue, but the
GC is waiting for the status to change back. This wedges the program.
To fix, call a special variant of casgstatus that accepts either Gwaiting
or Grunnable as valid statuses.
Without the fix bug with the extra check+throw in casgstatus, the
program below dies in a few seconds (2-10) with GOMAXPROCS=8
on a 2012 Retina MacBook Pro. With the fix, it runs for minutes
and minutes.
package main
import (
"io"
"log"
"net"
"runtime"
)
func main() {
const N = 100
for i := 0; i < N; i++ {
l, err := net.Listen("tcp", "127.0.0.1:0")
if err != nil {
log.Fatal(err)
}
ch := make(chan net.Conn, 1)
go func() {
var err error
c1, err := net.Dial("tcp", l.Addr().String())
if err != nil {
log.Fatal(err)
}
ch <- c1
}()
c2, err := l.Accept()
if err != nil {
log.Fatal(err)
}
c1 := <-ch
l.Close()
go netguy(c1, c2)
go netguy(c2, c1)
c1.Write(make([]byte, 100))
}
for {
runtime.GC()
}
}
func netguy(r, w net.Conn) {
buf := make([]byte, 100)
for {
bigstack(1000)
_, err := io.ReadFull(r, buf)
if err != nil {
log.Fatal(err)
}
w.Write(buf)
}
}
var g int
func bigstack(n int) {
var buf [100]byte
if n > 0 {
bigstack(n - 1)
}
g = int(buf[0]) + int(buf[99])
}
Fixes #9186.
LGTM=rlh
R=austin, rlh
CC=dvyukov, golang-codereviews, iant, khr, r
https://golang.org/cl/179680043
»»»
Russ Cox [Thu, 20 Nov 2014 15:14:49 +0000 (10:14 -0500)]
[release-branch.go1.4] runtime: fix atomic operations on non-heap addresses
««« CL 179030043 / e4ab8f908aac
runtime: fix atomic operations on non-heap addresses
Race detector runtime does not tolerate operations on addresses
that was not previously declared with __tsan_map_shadow
(namely, data, bss and heap). The corresponding address
checks for atomic operations were removed in
https://golang.org/cl/111310044
Restore these checks.
It's tricker than just not calling into race runtime,
because it is the race runtime that makes the atomic
operations themselves (if we do not call into race runtime
we skip the atomic operation itself as well). So instead we call
__tsan_go_ignore_sync_start/end around the atomic operation.
This forces race runtime to skip all other processing
except than doing the atomic operation itself.
Fixes #9136.
Russ Cox [Wed, 19 Nov 2014 20:31:31 +0000 (15:31 -0500)]
[release-branch.go1.4] runtime: remove assumption that noptrdata data bss noptrbss are ordered and contiguous
««« CL 179980043 / d71cc7e8a0e0
runtime: remove assumption that noptrdata data bss noptrbss are ordered and contiguous
The assumption can be violated by external linkers reordering them or
inserting non-Go sections in between them. I looked briefly at trying
to write out the _go_.o in external linking mode in a way that forced
the ordering, but no matter what there's no way to force Go's data
and Go's bss to be next to each other. If there is any data or bss from
non-Go objects, it's very likely to get stuck in between them.
Instead, rewrite the two places we know about that make the assumption.
I grepped for noptrdata to look for more and didn't find any.
The added race test (os/exec in external linking mode) fails without
the changes in the runtime. It crashes with an invalid pointer dereference.
In go1.3, I type enter at the prompt and the program exits.
With the CL being rolled back, the program wedges at the
prompt.
««« original CL description
syscall: SysProcAttr job control changes
Making the child's process group the foreground process group and
placing the child in a specific process group involves co-ordination
between the parent and child that must be done post-fork but pre-exec.
Andrew Gerrand [Tue, 18 Nov 2014 22:47:56 +0000 (09:47 +1100)]
[release-branch.go1.4] doc/go1.4.html: rewrite first sentence to make it clearer
««« CL 178910043 / 3916b070c5f3
doc/go1.4.html: rewrite first sentence to make it clearer
The grammar was atrocious, probably the victim of an editing error.
debug/goobj is not ready to be published but it is
needed for the various binary-reading commands.
Move to cmd/internal/goobj.
(The Go 1.3 release branch deleted it, but that's not
an option anymore due to the command dependencies.
The API is still not vetted nor terribly well designed.)
Russ Cox [Sun, 16 Nov 2014 21:44:45 +0000 (16:44 -0500)]
runtime: fix sudog leak
The SudoG used to sit on the stack, so it was cheap to allocated
and didn't need to be cleaned up when finished.
For the conversion to Go, we had to move sudog off the stack
for a few reasons, so we added a cache of recently used sudogs
to keep allocation cheap. But we didn't add any of the necessary
cleanup before adding a SudoG to the new cache, and so the cached
SudoGs had stale pointers inside them that have caused all sorts
of awful, hard to debug problems.
CL 155760043 made sure SudoG.elem is cleaned up.
CL 150520043 made sure SudoG.selectdone is cleaned up.
This CL makes sure SudoG.next, SudoG.prev, and SudoG.waitlink
are cleaned up. I should have done this when I did the other two
fields; instead I wasted a week tracking down a leak they caused.
A dangling SudoG.waitlink can point into a sudogcache list that
has been "forgotten" in order to let the GC collect it, but that
dangling .waitlink keeps the list from being collected.
And then the list holding the SudoG with the dangling waitlink
can find itself in the same situation, and so on. We end up
with lists of lists of unusable SudoGs that are still linked into
the object graph and never collected (given the right mix of
non-trivial selects and non-channel synchronization).
Robert Griesemer [Tue, 11 Nov 2014 21:19:47 +0000 (13:19 -0800)]
spec: method selectors don't auto-deref named pointer types
Language clarification.
The existing rules for selector expressions imply
automatic dereferencing of pointers to struct fields.
They also implied automatic dereferencing of selectors
denoting methods. In almost all cases, such automatic
dereferencing does indeed take place for methods but the
reason is not the selector rules but the fact that method
sets include both methods with T and *T receivers; so for
a *T actual receiver, a method expecting a formal T
receiver, also accepts a *T (and the invocation or method
value expression is the reason for the auto-derefering).
However, the rules as stated so far implied that even in
case of a variable p of named pointer type P, a selector
expression p.f would always be shorthand for (*p).f. This
is true for field selectors f, but cannot be true for
method selectors since a named pointer type always has an
empty method set.
Named pointer types may never appear as anonymous field
types (and method receivers, for that matter), so this
only applies to variables declared of a named pointer
type. This is exceedingly rare and perhaps shouldn't be
permitted in the first place (but we cannot change that).
Amended the selector rules to make auto-deref of values
of named pointer types an exception to the general rules
and added corresponding examples with explanations.
Both gc and gccgo have a bug where they do auto-deref
pointers of named types in method selectors where they
should not:
See http://play.golang.org/p/c6VhjcIVdM , line 45.
Fixes #5769.
Fixes #8989.
LGTM=r, rsc
R=r, rsc, iant, ken
CC=golang-codereviews
https://golang.org/cl/168790043
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.