cmd/go/internal/work: fix TestRespectGroupSticky on FreeBSD
FreeBSD doesn't allow non-root users to enable the SetGID bit on
files or directories in /tmp, however it does allow this in
subdirectories, so create the test directory one level deeper.
Followup to golang/go#19596.
Change-Id: I30e71c6d6a156badc863e8068df10ef6ed817e26
Reviewed-on: https://go-review.googlesource.com/41216 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Keith Randall [Wed, 19 Apr 2017 18:19:53 +0000 (11:19 -0700)]
cmd/compile: zero ambiguously live variables at VARKILLs
At VARKILLs, zero a variable if it is ambiguously live.
After the VARKILL anything this variable references
might be collected. If it were to become live again later,
the GC will see references to already-collected objects.
We don't know a variable is ambiguously live until very
late in compilation (after lowering, register allocation, ...),
so it is hard to generate the code in an arch-independent way.
We also have to be careful not to clobber any registers.
Fortunately, this almost never happens so performance is ~irrelevant.
There are only 2 instances where this triggers in the stdlib.
Fixes #20029
Change-Id: Ia9585a91d7b823fad4a9d141d954464cc7af31f4
Reviewed-on: https://go-review.googlesource.com/41076
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com>
Make poolLocal size multiple of 128, so it aligns to CPU cache line
on the most common architectures.
This also has the following benefits:
- It may help compiler substituting integer multiplication
by bit shift inside indexLocal.
- It shrinks poolLocal size from 176 bytes to 128 bytes on amd64,
so now it fits two cache lines (or a single cache line on certain
Intel CPUs - see https://software.intel.com/en-us/articles/optimizing-application-performance-on-intel-coret-microarchitecture-using-hardware-implemented-prefetchers).
No measurable performance changes on linux/amd64 and linux/386.
Robert Griesemer [Thu, 20 Apr 2017 19:50:41 +0000 (12:50 -0700)]
go/gcimporter: fix importing of anonymous interfaces
Imported interfaces must be completed, whether they are named or not.
The original code was collecting all types (including anonymous ones)
in the importer's typList. That list was used in the end to complete
interface types. When we introduced tracking of named types only, we
lost anonymous interfaces. Use an independent list of interface types
so the completion code is independent of which types are tracked.
Added test and factored some of the existing tests.
Fixes #20046.
Change-Id: Icd1329032aec33f96890380dd5042de3bef8cdc7
Reviewed-on: https://go-review.googlesource.com/41198 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
David Lazar [Wed, 19 Apr 2017 16:04:18 +0000 (12:04 -0400)]
runtime: make test independent of inlining
TestBreakpoint expects to see "runtime.Breakpoint()" in the stack trace.
If runtime.Breakpoint() is inlined, then the stack trace prints
"runtime.Breakpoint(...)" since the runtime does not have information
about arguments (or lack thereof) to inlined functions. This change
makes the test independent of inlining by looking for the string
"runtime.Breakpoint(". Now TestBreakpoint passes with -l=4.
TestBlockProfile matches samples against a regexp that accepts "," in
profile PCs. I suspect this was just a syntax mistake. Remove "," from
the character class.
TestBlockProfile currently requires exactly five PCs in each sample.
With more aggressive inlining there may be fewer, so change this test
to use the same pattern as TestMutexProfile, which accepts one or more
PCs. With this change, this test passes when compiled with -l=4.
CL 40876 changed ExampleFrames so that the output
was stable with and without mid-stack inlining.
However, that change lost some of the
pedagogical and copy/paste value of the example.
It was unclear why both more and i were being tracked,
and whether the 5 in i < 5 is related to len(pc),
and if so, why and how.
This CL rewrites the example with lots more comments,
and such that the core structure more closely matches
normal usage, and such that it is obvious
which lines of code should be deleted when copying.
As a bonus, it also now illustrates Frame.File.
The period recorded in CPU profiles is in nanoseconds, but was being
computed incorrectly as hz * 1000. As a result, many absolute times
displayed by pprof were incorrect.
Samuel Tan [Thu, 13 Apr 2017 17:57:04 +0000 (10:57 -0700)]
html/template: ignore case when handling type attribute in script element
Convert the parsed attribute name to lowercase before checking its value in
the HTML parser state machine. This ensures that the type attribute in
the script element is handled in a case-sensitive manner, just like all
other attribute names.
Instead of populating the aux symbol
of CALLudiv during rewrite rules,
populate it during genssa.
This simplifies the rewrite rules.
It also removes all remaining calls
to ctxt.Lookup from any rewrite rules.
This is a first step towards removing
ctxt from ssa.Cache entirely,
and also a first step towards converting
the obj.LSym.Version field into a boolean.
It should also speed up compilation.
Also, move func udiv into package runtime.
That's where it is anyway,
and it lets udiv look and act like the rest of
the runtime support functions.
cmd/internal/obj: split Link.hash into version 0 and 1
Though LSym.Version is an int, it can only have the value 0 or 1.
Using that, split Link.hash into two maps, one for version 0
(which is far more common) and one for version 1.
This lets use just the name for lookups,
which is both faster and more compact.
This matters because Link.hash map lookups are frequent,
and will be contended once the backend is concurrent.
Dave Cheney [Thu, 20 Apr 2017 00:45:01 +0000 (10:45 +1000)]
cmd/link/internal/ld: remove C style gotos from ldelf
ld.ldelf contained a mixture of normal and C style, goto bad, error
handling. The use of goto requires many variables to be declared well
before their use which inhibited further refactoring to this method.
This CL removes the gotos in this function. Future CLs will address
remainder of the C style function scoped declarations in this function.
Change-Id: Ib9def495209a2f8deb11dcf30ee954bca95390c6
Reviewed-on: https://go-review.googlesource.com/41172
Run-TryBot: Dave Cheney <dave@cheney.net>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
- PkgMap was only needed to test import/export in a "cleanroom"
environment, with debugFormat set. Provided helper function
instead.
- PkgList was only used to identify directly imported packages.
Instead, compute that list explicitly from the package map.
It happens only once, the list is small, and it's more robust
than keeping two data structures in sync.
Change-Id: I82dce3c0b5cb816faae58708e877799359c20fcb
Reviewed-on: https://go-review.googlesource.com/41078 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
runtime: record swept and reclaimed bytes in sweep trace
This extends the GCSweepDone event with counts of swept and reclaimed
bytes. These are useful for understanding the duration and
effectiveness of sweep events.
Change-Id: I3c97a4f0f3aad3adbd188adb264859775f54e2df
Reviewed-on: https://go-review.googlesource.com/40811
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
runtime: make sweep trace events encompass entire sweep loop
Currently, each individual span sweep emits a span to the trace. But
sweeps are generally done in loops until some condition is satisfied,
so this tracing is lower-level than anyone really wants any hides the
fact that no other work is being accomplished between adjacent sweep
events. This is also high overhead: enabling tracing significantly
impacts sweep latency.
Replace this with instead tracing around the sweep loops used for
allocation. This is slightly tricky because sweep loops don't
generally know if any sweeping will happen in them. Hence, we make the
tracing lazy by recording in the P that we would like to start tracing
the sweep *if* one happens, and then only closing the sweep event if
we started it.
This does mean we don't get tracing on every sweep path, which are
legion. However, we get much more informative tracing on the paths
that block allocation, which are the paths that matter.
Change-Id: I73e14fbb250acb0c9d92e3648bddaa5e7d7e271c
Reviewed-on: https://go-review.googlesource.com/40810
Run-TryBot: Austin Clements <austin@google.com> Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Michael Munday [Wed, 19 Apr 2017 17:48:33 +0000 (13:48 -0400)]
runtime: avoid restricting GOARCH values in documentation
Changes the text to match GOOS which appends 'and so on' at the
end to avoid restricting the set of possible values.
Change-Id: I54bcde71334202cf701662cdc2582c974ba8bf53
Reviewed-on: https://go-review.googlesource.com/41074 Reviewed-by: Ian Lance Taylor <iant@golang.org>
When allocating a non-small array of buckets for a map,
also preallocate some overflow buckets.
The estimate of the number of overflow buckets
is based on a simulation of putting mid=(low+high)/2 elements
into a map, where low is the minimum number of elements
needed to reach this value of b (according to overLoadFactor),
and high is the maximum number of elements possible
to put in this value of b (according to overLoadFactor).
This estimate is surprisingly reliable and accurate.
The number of overflow buckets needed is quadratic,
for a fixed value of b.
Using this mid estimate means that we will overallocate a few
too many overflow buckets when the actual number of elements is near low,
and underallocate significantly too few overflow buckets
when the actual number of elements is near high.
The mechanism introduced in this CL can be re-used for
other overflow bucket optimizations.
For example, given an initial size hint,
we could estimate quite precisely the number of overflow buckets.
This is #19931.
We could also change from "non-nil means end-of-list"
to "pointer-to-hmap.buckets means end-of-list",
and then create a linked list of reusable overflow buckets
when they are freed by map growth.
That is #19992.
We could also use a similar mechanism to do bulk allocation
of overflow buckets.
All these uses can co-exist with only the one additional pointer
in mapextra, given a little care.
Any change to how we allocate overflow buckets
will require some extra hmap storage,
but we don't want hmap to grow,
particular as small maps usually don't need overflow buckets.
This CL converts the existing hmap overflow field,
which is usually used for pointer-free maps,
into a generic extra field.
This extra field can be used to hold data that is optional.
If it is valuable enough to do have special
handling of overflow buckets, which are medium-sized,
it is valuable enough to pay an extra alloc and two extra words for.
Adding fields to extra would entail adding overhead to pointer-free maps;
any mapextra fields added would need to be weighed against that.
This CL is just rearrangement, though.
cmd/internal: remove duplicate pathToPrefix function
goobj.importPathToPrefix is 3x faster than gc.pathToPrefix so rename and
move it to cmd/internal/objabi which is already imported by both goobj and
gc.
Robert Griesemer [Wed, 19 Apr 2017 00:16:54 +0000 (17:16 -0700)]
cmd/compile: report block start for gotos jumping into blocks
Follow-up on https://go-review.googlesource.com/#/c/39998/
which dropped this information.
The reported blocks are the innermost blocks containing a
label jumped to from outside, not the outermost block as
reported originally by cmd/compile.
We could report the outermost block with a slighly more
involved algorithm (need to track containing blocks for
all unresolved forward gotos), but since gccgo also reports
the innermost blocks, the current approach seems good enough.
Change-Id: Ic0235b8fafe8d5f99dc9872b58e90e8d9e72c5db
Reviewed-on: https://go-review.googlesource.com/40980
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Marvin Stenger <marvin.stenger94@gmail.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Robert Griesemer [Fri, 7 Apr 2017 22:41:45 +0000 (15:41 -0700)]
cmd/compile: check labels and branches during parse time
Instead of a separate check control flow pass (checkcfg.go)
operating on nodes, perform this check at parse time on the
new syntax tree. Permits this check to be done concurrently,
and doesn't depend on the specifics of the symbol's dclstack
implementation anymore. The remaining dclstack uses will be
removed in a follow-up change.
- added CheckBranches Mode flag (so we can turn off the check
if we only care about syntactic correctness, e.g. for tests)
- adjusted test/goto.go error messages: the new branches
checker only reports if a goto jumps into a block, but not
which block (we may want to improve this again, eventually)
- also, the new branches checker reports one variable that
is being jumped over by a goto, but it may not be the first
one declared (this is fine either way)
- the new branches checker reports additional errors for
fixedbugs/issue14006.go (not crucial to avoid those errors)
- the new branches checker now correctly reports only
variable declarations being jumped over, rather than
all declarations (issue 8042). Added respective tests.
Fixes #8042.
Change-Id: I53b6e1bda189748e1e1fb5b765a8a64337c27d40
Reviewed-on: https://go-review.googlesource.com/39998 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Matthew Dempsky [Tue, 18 Apr 2017 19:53:25 +0000 (12:53 -0700)]
cmd/internal/objabi: extract shared functionality from obj
Now only cmd/asm and cmd/compile depend on cmd/internal/obj. Changing
the assembler backends no longer requires reinstalling cmd/link or
cmd/addr2line.
There's also now one canonical definition of the object file format in
cmd/internal/objabi/doc.go, with a warning to update all three
implementations.
objabi is still something of a grab bag of unrelated code (e.g., flag
and environment variable handling probably belong in a separate "tool"
package), but this is still progress.
Alexander Menzhinsky [Tue, 18 Apr 2017 03:06:46 +0000 (22:06 -0500)]
cmd/cgo: read source files once
Now cgo reads source files twice: for c prefix generation and parsing
go code to an ast node. It can be narrowed down to single loop.
Change-Id: Ie05452a3a12106aaab863244727390037e69e8e6
Reviewed-on: https://go-review.googlesource.com/40939 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Alexander Menzhinsky [Mon, 17 Apr 2017 21:13:59 +0000 (16:13 -0500)]
cmd/cgo: reject references to builtin functions other than calls
Here we restrict using cgo builtin references because internally they're go functions
as opposed to C usafe.Pointer values.
Fixes #18889
Change-Id: I1e4332e4884063ccbaf9772c172d4462ec8f3d13
Reviewed-on: https://go-review.googlesource.com/40934 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
David Lazar [Tue, 18 Apr 2017 19:12:54 +0000 (15:12 -0400)]
io: correctly compute call depth in test
TestMultiReaderFlatten determines the call depth by counting PCs
returned by runtime.Callers. With inlining, this is incorrect because
a PC can represent multiple calls. Furthermore, runtime.Callers might
return an additional "skip" PC, which does not represent a real call.
This modifies the test to use CallersFrames to determine the call depth.
Now the test passes with -l=4.
David Lazar [Mon, 10 Apr 2017 18:33:07 +0000 (14:33 -0400)]
runtime: skip logical frames in runtime.Caller
This rewrites runtime.Caller in terms of stackExpander, which already
handles inlined frames and partially skipped frames. This also has the
effect of making runtime.Caller understand cgo frames if there is a cgo
symbolizer.
CL 27254 changed hextable to a byte array for performance.
CL 28219 fixed the compiler so that that is no longer necessary.
As Kirill notes in #15808, a string is preferable
as the linker can easily de-dup it.
So go back. No performance changes.
Matthew Dempsky [Tue, 18 Apr 2017 17:18:34 +0000 (10:18 -0700)]
cmd/internal/obj: un-embed FuncInfo field in LSym
Automated refactoring using github.com/mdempsky/unbed (to rewrite
s.Foo to s.FuncInfo.Foo) and then gorename (to rename the FuncInfo
field to just Func).
cmd/compile: eliminate dead code in if statements after typechecking
This is a more thorough and cleaner fix
than doing dead code elimination separately
during inlining, escape analysis, and export.
Unfortunately, it does add another full walk of the AST.
The performance impact is very small, but not non-zero.
If a label or goto is present in the dead code, it is not eliminated.
This restriction can be removed once label/goto checking occurs
much earlier in the compiler. In practice, it probably doesn't
matter much.
This CL adds a simple explanation about what means the ptrdata field of
the reflect.rtype type.
Also document that rtype needs to be kept in sync with the runtime._type
type that rtype mirrors.
Daniel Martí [Tue, 18 Apr 2017 11:32:05 +0000 (12:32 +0100)]
all: remove unnecessary ", _" from map reads
If the bool value isn't used, there is no need to assign to underscore -
there is a shorter form that only returns the value and behaves in the
exact same way.
name old time/op new time/op delta
Pool-4 19.1ns ± 2% 10.1ns ± 1% -47.15% (p=0.000 n=10+8)
PoolOverflow-4 3.11µs ± 1% 2.10µs ± 2% -32.66% (p=0.000 n=10+10)
Performance results on linux/386:
name old time/op new time/op delta
Pool-4 20.0ns ± 2% 13.1ns ± 1% -34.59% (p=0.000 n=10+9)
PoolOverflow-4 3.51µs ± 1% 2.49µs ± 0% -28.99% (p=0.000 n=10+8)
Change-Id: I7d57a2d4cd47ec43d09ca1267bde2e3f05a9faa9
Reviewed-on: https://go-review.googlesource.com/40913 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
syscall: add Accept4 and SOCK_{CLOEXEC,NONBLOCK} on dragonfly
This change just picks a few constants from DragonfFly BSD 4.6 kernel
and doesn't synchronize all the existing constants with the latest
DragonFly BSD kernels.
The compiler handled gcargs and gclocals LSyms unusually.
It generated placeholder symbols (makefuncdatasym),
filled them in, and then renamed them for content-addressability.
This is an important binary size optimization;
the same locals information occurs over and over.
This CL continues to treat these LSyms unusually,
but in a slightly more explicit way,
and importantly for concurrent compilation,
in a way that does not require concurrent
modification of Ctxt.Hash.
Instead of creating gcargs and gclocals in the usual way,
by creating a types.Sym and then an obj.LSym,
we add them directly to obj.FuncInfo,
initialize them in obj.InitTextSym,
and deduplicate and add them to ctxt.Data at the end.
Then the backend's job is simply to fill them in
and rename them appropriately.
runtime: make internal CallersFrames-equivalent that doesn't escape PC slice
The Frames API forces the PC slice to escape to the heap because it
stores it in the Frames object. However, we'd like to use this API for
call stack expansion internally in the runtime in places where it
would be very good to avoid heap allocation.
This commit makes this possible by pulling the bulk of the Frames
implementation into an internal frameExpander API. The key difference
between these APIs is that the frameExpander does not hold the PC
slice; instead, the caller is responsible for threading the PC slice
through the frameExpander API calls. This makes it possible to keep
the PC slice on the stack. The Frames API then becomes a thin shim
around the frameExpander that keeps the PC slice in the Frames object.
Andy Balholm [Mon, 17 Apr 2017 18:58:30 +0000 (11:58 -0700)]
net/http: ignore extra space between response version and status code
Reading a response with a status line like "HTTP/1.0 401 Unauthorized"
(with two spaces after the version) has been returning an error. Now the
extra space will be ignored.
Michael Hudson-Doyle [Wed, 12 Apr 2017 04:08:46 +0000 (16:08 +1200)]
cmd/link: only include the version info and export data in ABI hash
Previously the "ABI hash" for a package (used to determine if a loaded shared
library has the ABI expected by its loader) was the hash of the entire
__.PKGDEF file. But that means it depends on the build ID generated by the go
tool for the package, which means that if a file is added (even a .c or .h
file!) to the package, the ABI changes, perhaps uncessarily.
Fixes #19920
Change-Id: If919481e1a03afb350c8a9c7a0666bb90ee90270
Reviewed-on: https://go-review.googlesource.com/40401
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Robert Griesemer [Tue, 11 Apr 2017 20:39:24 +0000 (13:39 -0700)]
spec: clarify use of fused-floating point operations
Added a paragraph and examples explaining when an implementation
may use fused floating-point operations (such as FMA) and how to
prevent operation fusion.
For #17895.
Change-Id: I64c9559fc1097e597525caca420cfa7032d67014
Reviewed-on: https://go-review.googlesource.com/40391 Reviewed-by: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Rob Pike <r@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
cmd/asm: detect invalid DS form offsets for ppc64x
While debugging a recent regression it was discovered that
the assembler for ppc64x was not always generating the correct
instruction for DS form loads and stores. When an instruction
is DS form then the offset must be a multiple of 4, and if it
isn't then bits outside the offset field were being incorrectly
set resulting in unexpected and incorrect instructions.
This change adds a check to determine when the opcode is DS form
and then verifies that the offset is a multiple of 4 before
generating the instruction, otherwise logs an error.
This also changes a few asm files that were using unaligned offsets
for DS form loads and stores. In the runtime package these were
instructions intended to cause a crash so using aligned or unaligned
offsets doesn't change that behavior.
Change-Id: Ie3a7e1e65dcc9933b54de7a46a054da8459cb56f
Reviewed-on: https://go-review.googlesource.com/40476 Reviewed-by: Michael Hudson-Doyle <michael.hudson@canonical.com>
The first part of this test tries to confirm that we can't create
a TempFile in a non-existent directory, but does not ensure that
the non-existent directory really does not exist. Instead, let's
create an empty temp directory, and use a non-existent subdir of
that.
cmd/go/internal/work: clean up after TestRespectGroupSticky
Use our own tempdir, to avoid having to Init (and somehow teardown)
Builder. This way we don't leave behind any temp files.
Also, don't create a hardcoded path inside a testcase.
Followup to golang/go#18878.
Fixes golang/go#19449.
Change-Id: Ieb1ebeab24ae8a74a6fa058d9c23f72b3fc1c444
Reviewed-on: https://go-review.googlesource.com/40912 Reviewed-by: Ian Lance Taylor <iant@golang.org>