Proportional concurrent sweep is currently based on a ratio of spans
to be swept per bytes of object allocation. However, proportional
sweeping is performed during span allocation, not object allocation,
in order to minimize contention and overhead. Since objects are
allocated from spans after those spans are allocated, the system tends
to operate in debt, which means when the next GC cycle starts, there
is often sweep debt remaining, so GC has to finish the sweep, which
delays the start of the cycle and delays enabling mutator assists.

For example, it's quite likely that many Ps will simultaneously refill
their span caches immediately after a GC cycle (because GC flushes the
span caches), but at this point, there has been very little object
allocation since the end of GC, so very little sweeping is done. The
Ps then allocate objects from these cached spans, which drives up the
bytes of object allocation, but since these allocations are coming
from cached spans, nothing considers whether more sweeping has to
happen. If the sweep ratio is high enough (which can happen if the
next GC trigger is very close to the retained heap size), this can
easily represent a sweep debt of thousands of pages.

Fix this by making proportional sweep proportional to the number of
bytes of spans allocated, rather than the number of bytes of objects
allocated. Prior to allocating a span, both the small object path and
the large object path ensure credit for allocating that span, so the
system operates in the black, rather than in the red.

Combined with the previous commit, this should eliminate all sweeping
from GC start up. On the stress test in issue #11911, this reduces the
time spent sweeping during GC (and delaying start up) by several
orders of magnitude:

                mean    99%ile     max
    pre fix      1 ms    11 ms   144 ms
    post fix   270 ns   735 ns   916 ns

Updates #11911.

Change-Id: I89223712883954c9d6ec2a7a51ecb97172097df3
Reviewed-on: https://go-review.googlesource.com/13044Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>

Currently it's possible for the next_gc heap size trigger computed for
the next GC cycle to be less than the current allocated heap size.
This means the next cycle will start immediately, which means there's
no time to perform the concurrent sweep between GC cycles. This places
responsibility for finishing the sweep on GC itself, which delays GC
start-up and hence delays mutator assist.

Fix this by ensuring that next_gc is always at least a little higher
than the allocated heap size, so we won't trigger the next cycle
instantly.

Updates #11911.

Change-Id: I74f0b887bf187518d5fedffc7989817cbcf30592
Reviewed-on: https://go-review.googlesource.com/13043Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>

Currently there are two sensitive periods during which a mutator can
allocate past the heap goal but mutator assists can't be enabled: 1)
at the beginning of GC between when the heap first passes the heap
trigger and sweep termination and 2) at the end of GC between mark
termination and when the background GC goroutine parks. During these
periods there's no back-pressure or safety net, so a rapidly
allocating mutator can allocate past the heap goal. This is
exacerbated if there are many goroutines because the GC coordinator is
scheduled as any other goroutine, so if it gets preempted during one
of these periods, it may stay preempted for a long period (10s or 100s
of milliseconds).

Normally the mutator does scan work to create back-pressure against
allocation, but there is no scan work during these periods. Hence, as
a fall back, if a mutator would assist but can't yet, simply yield the
CPU. This delays the mutator somewhat, but more importantly gives more
CPU time to the GC coordinator for it to complete the transition.

This is obviously a workaround. Issue #11970 suggests a far better but
far more invasive way to fix this.

Updates #11911. (This very nearly fixes the issue, but about once
every 15 minutes I get a GC cycle where the assists are enabled but
don't do enough work.)

Change-Id: I9768b79e3778abd3e06d306596c3bd77f65bf3f1
Reviewed-on: https://go-review.googlesource.com/13026Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>

Currently allocation checks the GC trigger speculatively during
allocation and then triggers the GC without rechecking. As a result,
it's possible for G 1 and G 2 to detect the trigger simultaneously,
both enter startGC, G 1 actually starts GC while G 2 gets preempted
until after the whole GC cycle, then G 2 immediately starts another GC
cycle even though the heap is now well under the trigger.

Fix this by re-checking the GC trigger non-speculatively just before
actually kicking off a new GC cycle.

This contributes to #11911 because when this happens, we definitely
don't finish the background sweep before starting the next GC cycle,
which can significantly delay the start of concurrent scan.

Change-Id: I560ab79ba5684ba435084410a9765d28f5745976
Reviewed-on: https://go-review.googlesource.com/13025Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>

Change-Id: Ifac10621fece766f3a0e8551e98d1f8d7072852f
Reviewed-on: https://go-review.googlesource.com/13068Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Rob Pike <r@golang.org>

I accidentally submitted https://golang.org/cl/13080 too early.

Update #11955.

Change-Id: I1a5a6860bb46bc4bc6fd278f8a867d2dd9e411e1
Reviewed-on: https://go-review.googlesource.com/13096Reviewed-by: Andrew Gerrand <adg@golang.org>

Do not assume that if stat shows multiple links that we should mark the
file as a hardlink in the tar format.  If the hardlink link was not
referenced, this caused a link to "/".  On an overlay file system, all
files have multiple links.

The caller must keep the inode references and set TypeLink, Size = 0,
and LinkName themselves.

Change-Id: I873b8a235bc8f8fbb271db74ee54232da36ca013
Reviewed-on: https://go-review.googlesource.com/13045Reviewed-by: Ian Lance Taylor <iant@golang.org>

The buildmode docs mention exported functions, but don't say anything
about how to export them.  Mention the cgo tool to make this somewhat
clearer.

Fixes #11955.

Change-Id: Ie5420445daa87f5aceec6ad743465d5d32d0a786
Reviewed-on: https://go-review.googlesource.com/13080Reviewed-by: Russ Cox <rsc@golang.org>

For niceness, when go/exact was moved from x/tools, it
was renamed go/constant.

For simplicity, when go/types was moved from x/tools, its
imports of (now) go/constant were done with a rename:

    import exact "go/constant"

This kept the code just as it was before and avoided the issue
of what to call the internal constant called, um, constant.

But not all was hidden, as the text of some fields of structs and
the like leaked the old name, so things like "exact.Value" appeared
in type definitions and function signatures in the documentation.
This is unacceptable.

Fix the documentation issue by fixing the code. Rename the constant
constant constant_, and remove the renaming import.

This should go into 1.5. It's mostly a mechanical change, is
internal to the package, and fixes the documentation. It contains
no semantic changes except to fix a benchmark that was broken
in the original transition.

Fixes #11949.

Change-Id: Ieb94b6558535b504180b1378f19e8f5a96f92d3c
Reviewed-on: https://go-review.googlesource.com/13051Reviewed-by: Russ Cox <rsc@golang.org>

Change-Id: Ia5d41b66006682084fcbfac3da020946ea3dd116
Reviewed-on: https://go-review.googlesource.com/13093Reviewed-by: Andrew Gerrand <adg@golang.org>

Change-Id: Ia21501df23a91c065d9f2acc6f043019a1419b22
Reviewed-on: https://go-review.googlesource.com/13092Reviewed-by: Andrew Gerrand <adg@golang.org>

I walked through the steps for a contribution but ended up
with an error when doing "git mail" because I didn't have a
signed agreement.

Added a section to check for or create one through Gerrit right
after the user has created the account and logged in.

Moved some info from copyright section to the new section.

Change-Id: I79bbd3e18fc3a742fa59a242085da14be9e19ba0
Reviewed-on: https://go-review.googlesource.com/13062Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>

Change-Id: I4dc75065038a9cfd06f61c0deca1c86c70713d3a
Reviewed-on: https://go-review.googlesource.com/13091Reviewed-by: Andrew Gerrand <adg@golang.org>

This was confusing when I was trying to fix go build -o.
Perhaps due to that fix, this can now be simplified from
three functions to one.

Change-Id: I878a6d243b14132a631e7c62a3bb6d101bc243ea
Reviewed-on: https://go-review.googlesource.com/13027Reviewed-by: Ian Lance Taylor <iant@golang.org>

Quoting the new docs:

«
If the arguments to build are a list of .go files, build treats
them as a list of source files specifying a single package.

When compiling a single main package, build writes
the resulting executable to an output file named after
the first source file ('go build ed.go rx.go' writes 'ed' or 'ed.exe')
or the source code directory ('go build unix/sam' writes 'sam' or 'sam.exe').
The '.exe' suffix is added when writing a Windows executable.

When compiling multiple packages or a single non-main package,
build compiles the packages but discards the resulting object,
serving only as a check that the packages can be built.

The -o flag, only allowed when compiling a single package,
forces build to write the resulting executable or object
to the named output file, instead of the default behavior described
in the last two paragraphs.
»

There is a change in behavior here, namely that 'go build -o x.a x.go'
where x.go is not a command (not package main) did not write any
output files (back to at least Go 1.2) but now writes x.a.
This seems more reasonable than trying to explain that -o is
sometimes silently ignored.

Otherwise the behavior is unchanged.

The lines being deleted in goFilesPackage look like they are
setting up 'go build x.o' to write 'x.a', but they were overridden
by the p.target = "" in runBuild. Again back to at least Go 1.2,
'go build x.go' for a non-main package has never produced
output. It seems better to keep it that way than to change it,
both for historical consistency and for consistency with
'go build strings' and 'go build std'.

All of this behavior is now tested.

Fixes #10865.

Change-Id: Iccdf21f366fbc8b5ae600a1e50dfe7fc3bff8b1c
Reviewed-on: https://go-review.googlesource.com/13024Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Dave Day <djd@golang.org>

It was failing with multiple goroutines a few out of every thousand
runs (with errRequestCanceled) because it was using the same
*http.Request for all 5 RoundTrips, but the RoundTrips' goroutines
(notably the readLoop method) were all still running, sharing that
same pointer. Because the response has no body (which is what
TestZeroLengthPostAndResponse tests), the readLoop was marking the
connection as reusable early (before the caller read until the body's
EOF), but the Transport code was clearing the Request's cancelation
func *AFTER* the caller had already received it from RoundTrip. This
let the test continue looping and do the next request with the same
pointer, fetch a connection, and then between getConn and roundTrip
have an invariant violated: the Request's cancelation func was nil,
tripping this check:

        if !pc.t.replaceReqCanceler(req.Request, pc.cancelRequest) {
                pc.t.putIdleConn(pc)
                return nil, errRequestCanceled
        }

The solution is to clear the request cancelation func in the readLoop
goroutine in the no-body case before it's returned to the caller.

This now passes reliably:

$ go test -race -run=TestZeroLengthPostAndResponse -count=3000

I think we've only seen this recently because we now randomize scheduling
of goroutines in race mode (https://golang.org/cl/11795). This race
has existed for a long time but the window was hard to hit.

Change-Id: Idb91c582919f85aef5b9e5ef23706f1ba9126e9a
Reviewed-on: https://go-review.googlesource.com/13070
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

Introduced in https://go-review.googlesource.com/12865 (git rev c2db5f4c).

This fix doesn't add any new lock acquistions: it just moves the
existing one taken by the unreadDataSize method and moves it out
wider.

It became flaky at rev c2db5f4c, but now reliably passes again:
$ go test -v -race -run=TestTransportAndServerSharedBodyRace -count=100 net/http

Fixes #11985

Change-Id: I6956d62839fd7c37e2f7441b1d425793f4a0db30
Reviewed-on: https://go-review.googlesource.com/12909
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

Updates #11990.

Change-Id: I6c58923a1b5a3805acfb6e333e3c9e87f4edf4ba
Reviewed-on: https://go-review.googlesource.com/13050Reviewed-by: Ian Lance Taylor <iant@golang.org>

Fixes #11973.

Change-Id: Icffa3213246663982b7cc795982e0923e272f405
Reviewed-on: https://go-review.googlesource.com/12919Reviewed-by: Ian Lance Taylor <iant@golang.org>

Change-Id: I992cb1afeef498353d529238e508fa438d6c069c
Reviewed-on: https://go-review.googlesource.com/12912Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Rob Pike <r@golang.org>

Fixes #11983

Change-Id: I5ee930314a43356f5be31d758d90d7ddcafc7b37
Reviewed-on: https://go-review.googlesource.com/12908Reviewed-by: Andrew Gerrand <adg@golang.org>

HTTP servers attempt to entirely consume a request body before sending a
response.  However, when doing so, it previously would ignore any errors
encountered.

Unfortunately, the errors triggered at this stage are indicative of at
least a couple problems: read timeouts and chunked encoding errors.
This means properly crafted and/or timed requests could lead to a
"smuggled" request.

The fix is to inspect the errors created by the response body Reader,
and treat anything other than io.EOF or ErrBodyReadAfterClose as
fatal to the connection.

Fixes #11930

Change-Id: I0bf18006d7d8f6537529823fc450f2e2bdb7c18e
Reviewed-on: https://go-review.googlesource.com/12865Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

This makes the receiver name consistent with the rest of the methods on
type Server.

Change-Id: Ic2a007d3b5eb50bd87030e15405e9856109cf590
Reviewed-on: https://go-review.googlesource.com/13035Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>

Per suggestions by Peter Olsen (https://github.com/pto).

Fixes #11964.

Change-Id: Iae261ac14f75abf848f5601f59d7fe6e95b6805a
Reviewed-on: https://go-review.googlesource.com/13006Reviewed-by: Ian Lance Taylor <iant@golang.org>

The test uses external linking mode, which is probably not available
if cgo does not work.

Fixes #11969.

Change-Id: Id1c2828cd2540391e16b422bf51674ba6ff084b0
Reviewed-on: https://go-review.googlesource.com/13005Reviewed-by: Russ Cox <rsc@golang.org>

Fixes #11919

Issue #11918 suggested that os.File.Chown and os.Lchown were under tested.

Change-Id: Ib41f7cb2d2fe0066d2ccb4d1bdabe1795efe80fc
Reviewed-on: https://go-review.googlesource.com/12834Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

Fixes #11960.

Change-Id: I9361a9f17f4eaf8e4f54b4ba380fd50a4b9cf003
Reviewed-on: https://go-review.googlesource.com/13023Reviewed-by: Ian Lance Taylor <iant@golang.org>

The percolation of errors upward in the load process could
drop errors, meaning that a build tree could, depending on the
processing order, import the same directory as both "p/vendor/x"
and as "x". That's not supposed to be allowed. But then, worse,
the build would generate two jobs for building that directory,
which would use the same work space and overwrite each other's files,
leading to very strange failures.

Two fixes:

1. Fix the propagation of errors upward (prefer errors over success).
2. Check explicitly for duplicated packages before starting a build.

New test for #1.
Since #2 can't happen, tested #2 by hand after reverting fix for #1.

Fixes #11913.

Change-Id: I6d2fc65f93b8fb5f3b263ace8d5f68d803a2ae5c
Reviewed-on: https://go-review.googlesource.com/13022Reviewed-by: Ian Lance Taylor <iant@golang.org>

On most systems, a pointer is the worst case alignment, so adding
a pointer field at the end of a struct guarantees there will be no
padding added after that field (to satisfy overall struct alignment
due to some more-aligned field also present).

In the runtime, the map implementation needs a quick way to
get to the overflow pointer, which is last in the bucket struct,
so it uses size - sizeof(pointer) as the offset.

NaCl/amd64p32 is the exception, as always.
The worst case alignment is 64 bits but pointers are 32 bits.
There's a long history that is not worth going into, but when
we moved the overflow pointer to the end of the struct,
we didn't get the padding computation right.
The compiler computed the regular struct size and then
on amd64p32 added another 32-bit field.
And the runtime assumed it could step back two 32-bit fields
(one 64-bit register size) to get to the overflow pointer.
But in fact if the struct needed 64-bit alignment, the computation
of the regular struct size would have added a 32-bit pad already,
and then the code unconditionally added a second 32-bit pad.
This placed the overflow pointer three words from the end, not two.
The last two were padding, and since the runtime was consistent
about using the second-to-last word as the overflow pointer,
no harm done in the sense of overwriting useful memory.
But writing the overflow pointer to a non-pointer word of memory
means that the GC can't see the overflow blocks, so it will
collect them prematurely. Then bad things happen.

Correct all this in a few steps:

1. Add an explicit check at the end of the bucket layout in the
compiler that the overflow field is last in the struct, never
followed by padding.

2. When padding is needed on nacl (not always, just when needed),
insert it before the overflow pointer, to preserve the "last in the struct"
property.

3. Let the compiler have the final word on the width of the struct,
by inserting an explicit padding field instead of overwriting the
results of the width computation it does.

4. For the same reason (tell the truth to the compiler), set the type
of the overflow field when we're trying to pretend its not a pointer
(in this case the runtime maintains a list of the overflow blocks
elsewhere).

5. Make the runtime use "last in the struct" as its location algorithm.

This fixes TestTraceStress on nacl/amd64p32.
The 'bad map state' and 'invalid free list' failures no longer occur.

Fixes #11838.

Change-Id: If918887f8f252d988db0a35159944d2b36512f92
Reviewed-on: https://go-review.googlesource.com/12971Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Austin Clements <austin@google.com>

Change-Id: Ic8ff6c28ec899cf5e01553b83110eb6262870995
Reviewed-on: https://go-review.googlesource.com/12918Reviewed-by: Russ Cox <rsc@golang.org>

Fixes some minor issues regarding quoted-string when parsing
the local-part.

Those strings should return an error:
- quoted-string without any content: `""@test.com`
- quoted-string containing tab: "\"\t\"@test.com"

Fixes #11293

Change-Id: Ied93eb6831915c9b1f8e727cea14168af21f8d3b
Reviewed-on: https://go-review.googlesource.com/12905Reviewed-by: Russ Cox <rsc@golang.org>

The code already fixed large non-stack offsets
but explicitly excluded stack references.
Perhaps you could get away with that before,
but current versions of nacl reject such stack
references. Rewrite them the same as the others.

For #11956 but probably not the last problem.

Change-Id: I0db4e3a1ed4f88ccddf0d30228982960091d9fb7
Reviewed-on: https://go-review.googlesource.com/13010Reviewed-by: Dave Cheney <dave@cheney.net>

For #11956.

Change-Id: Ic9b57cafa197953cc7f435941e44d42b60b3ddf0
Reviewed-on: https://go-review.googlesource.com/13011Reviewed-by: Dave Cheney <dave@cheney.net>

Update #11943

Change-Id: I3e6592876bf16d2f9129995b723ecf69c069653d
Reviewed-on: https://go-review.googlesource.com/12913Reviewed-by: Russ Cox <rsc@golang.org>

Dangling pointer error. Unlikely to trigger in practice, but still.
Found by running GODEBUG=efence=1 GOGC=1 trace.test.

Change-Id: Ice474dedcf62dd33ab77526287a023ba3b166db9
Reviewed-on: https://go-review.googlesource.com/12991Reviewed-by: Austin Clements <austin@google.com>

Etcd and kubernetes have hit this.
See  https://bugzilla.redhat.com/show_bug.cgi?id=1248071

Change-Id: I6231013efa0a19ee74f7ebacd1024adb368af83a
Reviewed-on: https://go-review.googlesource.com/12951Reviewed-by: Ian Lance Taylor <iant@golang.org>

In https://golang.org/cl/12080 we forbade installing cross-compiled
binaries into a subdirectory of $GOBIN, in order to fix
https://golang.org/issue/9769.  However, that fix was too aggressive,
in that it also forbade installing into a subdirectory of $GOPATH/bin.

This patch permits installing cross-compiled binaries into a
subdirectory $GOPATH/bin while continuing to forbid installing into a
subdirectory of $GOBIN.

Fixes #11778.

Change-Id: Ibc9919554e8c275beff54ec8bf919cfaa03b11ba
Reviewed-on: https://go-review.googlesource.com/12938
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>

Fixes #11952.

Change-Id: I548f9d75c6223bf79bdf654ef733f1568e3d5804
Reviewed-on: https://go-review.googlesource.com/12990Reviewed-by: Ian Lance Taylor <iant@golang.org>

The spec didn't specify several aspects of expression switches:

- The switch expression is evaluated exactly once.

- Switch expressions evaluating to an untyped value are converted
  to the respective default type before use.

- An (untyped) nil value is not permitted as expression switch
  value. (We could permit it relatively easily, but gc doesn't,
  and disallowing it is in symmetry with the rules for var decls
  without explicit type and untyped initializer expressions.)

- The comparison x == t between each case expression x and
  switch expression value t must be valid.

- (Some) duplicate constant case expressions are not permitted.

This change also clarifies the following issues:

 4524: mult. equal int const switch case values should be illegal
                                         -> spec issue fixed
 6398: switch w/ no value uses bool rather than untyped bool
                                         -> spec issue fixed
11578: allows duplicate switch cases     -> go/types bug
11667: int overflow in switch expression -> go/types bug
11668: use of untyped nil in switch      -> not a gc bug

Fixes #4524.
Fixes #6398.
Fixes #11668.

Change-Id: Iae4ab3e714575a5d11c92c9b8fbf027aa706b370
Reviewed-on: https://go-review.googlesource.com/12711Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Rob Pike <r@golang.org>

Change-Id: If61c2063a8b63f0e3e498a5e86803b5ddba9fa3c
Reviewed-on: https://go-review.googlesource.com/12886Reviewed-by: Austin Clements <austin@google.com>