Adam Langley [Thu, 22 Feb 2018 20:05:29 +0000 (12:05 -0800)]
crypto/x509: parse invalid DNS names and email addresses.
Go 1.10 requires that SANs in certificates are valid. However, a
non-trivial number of (generally non-WebPKI) certificates have invalid
strings in dnsName fields and some have even put those dnsName SANs in
CA certificates.
This change defers validity checking until name constraints are checked.
The tests checking for empty interfaces so that they can be fast-
tracked in the code actually didn't test the right field and the
fast track code never executed. Doing it now.
Change-Id: I58b2951efb3fb40b3366874c79fd653591ae0e99
Reviewed-on: https://go-review.googlesource.com/97519 Reviewed-by: Alan Donovan <adonovan@google.com>
Robert Griesemer [Tue, 27 Feb 2018 22:02:09 +0000 (14:02 -0800)]
go/types: fix incorrect context when type-checking interfaces
Regression, introduced by https://go-review.googlesource.com/c/go/+/79575
which meant to be more conservative but ended up destroying an important
context.
Fixes #24140.
Change-Id: Id428dbb295ce9f11ab7cd54ec5ab51ef4291ac3f
Reviewed-on: https://go-review.googlesource.com/97535 Reviewed-by: Alan Donovan <adonovan@google.com>
Richard Miller [Wed, 28 Feb 2018 14:29:27 +0000 (14:29 +0000)]
syscall: reduce redundant getwd tracking in Plan 9
In Plan 9, each M is implemented as a separate OS process with
its own working directory. To keep the wd consistent across
goroutines (or rescheduling of the same goroutine), CL 6350
introduced a Fixwd procedure which checks using getwd and calls
chdir if necessary before any syscall operating on a pathname.
This wd checking will not be necessary if the pathname is absolute
(starts with '/' or '#'). Getwd is a fairly expensive operation
in Plan 9 (implemented by opening "." and calling Fd2path on the
file descriptor). Eliminating the redundant getwd calls can
significantly reduce overhead for common operations like
"dist test --list" which perform many syscalls on absolute pathnames.
Updates #9428.
Change-Id: I13fd9380779de27b0ac2f2b488229778d6839255
Reviewed-on: https://go-review.googlesource.com/97675 Reviewed-by: David du Colombier <0intro@gmail.com> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: David du Colombier <0intro@gmail.com>
Richard Miller [Wed, 28 Feb 2018 09:35:55 +0000 (09:35 +0000)]
runtime: don't try to shrink address space with brk in Plan 9
Plan 9 won't let brk shrink the data segment if it's shared with
other processes (which it is in the go runtime). So we keep track
of the notional end of the segment as it moves up and down, and
call brk only when it grows.
Corrects CL 94776.
Updates #23860.
Fixes #24013.
Change-Id: I754232decab81dfd71d690f77ee6097a17d9be11
Reviewed-on: https://go-review.googlesource.com/97595 Reviewed-by: David du Colombier <0intro@gmail.com> Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: David du Colombier <0intro@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Robert Griesemer [Tue, 27 Feb 2018 01:35:29 +0000 (17:35 -0800)]
cmd/compile, cmd/compile/internal/syntax: print relative column info
This change enables printing of relative column information if a
prior line directive specified a valid column. If there was no
line directive, or the line directive didn't specify a column
(or the -C flag is specified), no column information is shown in
file positions.
Implementation: Column values (and line values, for that matter)
that are zero are interpreted as "unknown". A line directive that
doesn't specify a column records that as a zero column in the
respective PosBase data structure. When computing relative columns,
a relative value is zero of the base's column value is zero.
When formatting a position, a zero column value is not printed.
To make this work without special cases, the PosBase for a file
is given a concrete (non-0:0) position 1:1 with the PosBase's
line and column also being 1:1. In other words, at the position
1:1 of a file, it's relative positions are starting with 1:1 as
one would expect.
In the package syntax, this requires self-recursive PosBases for
file bases, matching what cmd/internal/src.PosBase was already
doing. In src.PosBase, file and inlining bases also need to be
based at 1:1 to indicate "known" positions.
This change completes the cmd/compiler part of the issue below.
Fixes #22662.
Change-Id: I6c3d2dee26709581fba0d0261b1d12e93f1cba1a
Reviewed-on: https://go-review.googlesource.com/97375 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Keith Randall [Tue, 27 Feb 2018 21:46:03 +0000 (13:46 -0800)]
cmd/compile: mark the first word of an interface as a uintptr
The first word of an interface is a pointer, but for the purposes
of GC we don't need to treat it as such.
1. If it is a non-empty interface, the pointer points to an itab
which is always in persistentalloc space.
2. If it is an empty interface, the pointer points to a _type.
a. If it is a compile-time-allocated type, it points into
the read-only data section.
b. If it is a reflect-allocated type, it points into the Go heap.
Reflect is responsible for keeping a reference to
the underlying type so it won't be GCd.
If we ever have a moving GC, we need to change this for 2b (as
well as scan itabs to update their itab._type fields).
Write barriers on the first word of interfaces have already been removed.
Change-Id: I643e91d7ac4de980ac2717436eff94097c65d959
Reviewed-on: https://go-review.googlesource.com/97518
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com>
Explicitly whitelist args of OpSelect{1|2} that zero upper 32 bits.
Use better values in corresponding test.
This should have been a part of CL 96815, but it was submitted, before
relevant comments.
Ilya Tocar [Tue, 20 Feb 2018 16:59:19 +0000 (10:59 -0600)]
cmd/compile/internal/ssa: combine byte stores on amd64
On amd64 we optimize encoding/binary.BigEndian.PutUint{16,32,64}
into bswap + single store, but strangely enough not LittleEndian.PutUint{16,32}.
We have similar rules, but they use 64-bit shifts everywhere,
and fail for 16/32-bit case. Add rules that matchLittleEndian.PutUint,
and relevant tests. Performance results:
Heschi Kreinick [Wed, 24 Jan 2018 18:26:15 +0000 (13:26 -0500)]
cmd/link: fix up location lists for dsymutil
LLVM tools, particularly lldb and dsymutil, don't support base address
selection entries in location lists. When targeting GOOS=darwin,
mode, have the linker translate location lists to CU-relative form
instead.
Technically, this isn't necessary when linking internally, as long as
nobody plans to use anything other than Delve to look at the DWARF. But
someone might want to use lldb, and it's really confusing when dwarfdump
shows gibberish for the location entries. The performance cost isn't
noticeable, so enable it even for internal linking.
Doing this in the linker is a little weird, but it was more expensive in
the compiler, probably because the compiler is much more stressful to
the GC. Also, if we decide to only do it for external linking, the
compiler can't see the link mode.
Benchmark before and after this commit on Mac with -dwarflocationlists=1:
name old time/op new time/op delta
StdCmd 21.3s ± 1% 21.3s ± 1% ~ (p=0.310 n=27+27)
Only StdCmd is relevant, because only StdCmd runs the linker. Whatever
the cost is here, it's not very large.
Change-Id: I200246dedaee4f824966f7551ac95f8d7123d3b1
Reviewed-on: https://go-review.googlesource.com/89535 Reviewed-by: David Chase <drchase@google.com>
Philip Hofer [Wed, 21 Feb 2018 21:35:08 +0000 (13:35 -0800)]
cmd/compile/internal/ssa: clear branch likeliness in clobberBlock
The branchelim pass makes some blocks unreachable, but does not
remove them from Func.Values. Consequently, ssacheck complains
when it finds a block with a non-zero likeliness value but no
successors.
Joe Tsai [Fri, 16 Feb 2018 23:35:35 +0000 (15:35 -0800)]
go/doc: replace unexported values with underscore if necessary
When a var or const declaration contains a mixture of exported and unexported
identifiers, replace the unexported identifiers with underscore.
Otherwise, the LHS and the RHS may mismatch or the declaration may mismatch
with an iota from above.
Fixes #22426
Change-Id: Icd5fb81b4ece647232a9f7d05cb140227091e9cb
Reviewed-on: https://go-review.googlesource.com/94877
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
Giovanni Bajo [Tue, 20 Feb 2018 08:39:09 +0000 (09:39 +0100)]
cmd/compile: fix bit-test rules for highest bit
Bit-test rules failed to match when matching the highest bit
of a word because operands in SSA are signed int64. Fix
them by treating them as unsigned (and correctly handling
32-bit operands as well).
Tests will be added in next CL.
Change-Id: I491c4e88e7e2f87e9bb72bd0d9fa5d4025b90736
Reviewed-on: https://go-review.googlesource.com/94765 Reviewed-by: Keith Randall <khr@golang.org>
Moving tighten after lowering benefits from the removal of values by
lowering and lowered CSE. It lets us make better decisions about
which values are rematerializable and which generate flags.
Empirically, it lowers stack usage (by avoiding spills)
and generates slightly smaller and faster binaries.
Keith Randall [Wed, 3 Jan 2018 22:38:55 +0000 (14:38 -0800)]
cmd/compile: implement comparisons directly with memory
Allow the compiler to generate code like CMPQ 16(AX), $7
It's tricky because it's difficult to spill such a comparison during
flagalloc, because the same memory state might not be available at
the restore locations.
Solve this problem by decomposing the compare+load back into its parts
if it needs to be spilled.
The big win is that the write barrier test goes from:
MOVL runtime.writeBarrier(SB), CX
TESTL CX, CX
JNE 60
Kunpei Sakai [Sun, 25 Feb 2018 09:14:20 +0000 (18:14 +0900)]
cmd/compile: fix typechecking in finishcompare
Previously, finishcompare just used SetTypecheck, but this didn't
recursively update any untyped bool typed subexpressions. This CL
changes it to call typecheck, which correctly handles this.
Also cleaned up outdated code for simplifying logic.
Updates #23834
Change-Id: Ic7f92d2a77c2eb74024ee97815205371761c1c90
Reviewed-on: https://go-review.googlesource.com/97035 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Ilya Tocar [Fri, 23 Feb 2018 19:46:44 +0000 (13:46 -0600)]
cmd/compile/internal/amd64: use appropriate NEG for div
Currently we generate NEGQ for DIV{Q,L,W}. By generating NEGL and NEGW,
we will reduce code size, because NEGL doesn't require rex prefix.
This also guarantees that upper 32 bits are zeroed, so we can revert CL 85736,
and remove zero-extensions of DIVL results.
Also adds test for redundant zero extend elimination.
motemen [Mon, 26 Feb 2018 18:22:46 +0000 (18:22 +0000)]
cmd/go: fix formatting of file paths under cwd
The output of go with -x flag is formatted in a manner that file paths
under current directory are modified to start with a dot (.), but when
the directory path ends with a slash (/), the formatting goes wrong.
Fixes #23982
Change-Id: I8f8d15dd52bee882a9c6357eb9eabdc3eaa887c3
GitHub-Last-Rev: 1493f38bafdf2c40f16392b794fd1a12eb12a151
GitHub-Pull-Request: golang/go#23985
Reviewed-on: https://go-review.googlesource.com/95755
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Robert Griesemer [Thu, 22 Feb 2018 18:43:35 +0000 (10:43 -0800)]
go/types: type-check embedded methods in correct scope (regression)
Change https://go-review.googlesource.com/79575 fixed the computation
of recursive method sets by separating the method set computation from
type computation. However, it didn't track an embedded method's scope
and as a result, some methods' signatures were typed in the wrong
context.
This change tracks embedded methods together with their scope and
uses that scope for the correct context setup when typing those
method signatures.
Fixes #23914.
Change-Id: If3677dceddb43e9db2f9fb3c7a4a87d2531fbc2a
Reviewed-on: https://go-review.googlesource.com/96376 Reviewed-by: Alan Donovan <adonovan@google.com>
Rens Rikkerink [Mon, 26 Feb 2018 18:23:00 +0000 (18:23 +0000)]
cmd/cgo: clarify implicit "cgo" build constraint
When using the special import "C", the "cgo" build constraint is implied for the go file,
potentially triggering unclear "undefined" error messages.
Explicitly explain this in the documentation.
Updates #24068
Change-Id: Ib656ceccd52c749ffe7fb2d3db9ac144f17abb32
GitHub-Last-Rev: 5a13f00a9b917e51246a5fbb642c4e9ed55aa21d
GitHub-Pull-Request: golang/go#24072
Reviewed-on: https://go-review.googlesource.com/96655 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Robert Griesemer [Fri, 23 Feb 2018 01:24:19 +0000 (17:24 -0800)]
cmd/compile: track line directives w/ column information
Extend cmd/internal/src.PosBase to track column information,
and adjust the meaning of the PosBase position to mean the
position at which the PosBase's relative (line, col) position
starts (rather than indicating the position of the //line
directive). Because this semantic change is made in the
compiler's noder, it doesn't affect the logic of src.PosBase,
only its test setup (where PosBases are constructed with
corrected incomming positions). In short, src.PosBase now
matches syntax.PosBase with respect to the semantics of
src.PosBase.pos.
For #22662.
Change-Id: I5b1451cb88fff3f149920c2eec08b6167955ce27
Reviewed-on: https://go-review.googlesource.com/96535 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Robert Griesemer [Wed, 3 Jan 2018 00:58:37 +0000 (16:58 -0800)]
cmd/compile/internal/syntax: remove dependency on cmd/internal/src
For dependency reasons, the data structure implementing source
positions in the compiler is in cmd/internal/src. It contains
highly compiler specific details (e.g. inlining index).
This change introduces a parallel but simpler position
representation, defined in the syntax package, which removes
that package's dependency on cmd/internal/src, and also removes
the need to deal with certain filename-specific operations
(defined by the needs of the compiler) in the syntax package.
As a result, the syntax package becomes again a compiler-
independent, stand-alone package that at some point might
replace (or augment) the existing top-level go/* syntax-related
packages.
Additionally, line directives that update column numbers
are now correctly tracked through the syntax package, with
additional tests added. (The respective changes also need to
be made in cmd/internal/src; i.e., the compiler accepts but
still ignores column numbers in line directives.)
This change comes at the cost of a new position translation
step, but that step is cheap because it only needs to do real
work if the position base changed (i.e., if there is a new file,
or new line directive).
There is no noticeable impact on overall compiler performance
measured with `compilebench -count 5 -alloc`:
I don't think these allocate-as-fast-as-you-can benchmarks are very
interesting because they're effectively just GC benchmarks, but sure.
If one wants to see that there's 1 fewer allocation, there it is. The
ns/op and B/op numbers will change as the built string size changes.
Tobias Klauser [Mon, 26 Feb 2018 13:06:01 +0000 (14:06 +0100)]
os: unify supportsCloseOnExec definition
On Darwin and FreeBSD, supportsCloseOnExec is defined in its own file,
even though it is set to true as on other Unices. Drop the separate
definitions but keep the accompanying comments.
Alex Brainman [Fri, 15 Dec 2017 23:04:03 +0000 (10:04 +1100)]
os: do not forget to set ModeDevice when using ModeCharDevice
Fixes #23123
Change-Id: Ia4ac947cc49ef3d150ef60a095b86552dcef397d
Reviewed-on: https://go-review.googlesource.com/84435 Reviewed-by: Giovanni Bajo <rasky@develer.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Giovanni Bajo <rasky@develer.com>
Agniva De Sarker [Sat, 24 Feb 2018 05:22:04 +0000 (10:52 +0530)]
archive/tar: remove loop label from reader
CL 14624 introduced this label. At that time,
the switch-case had a break to label statement which made this necessary.
But now, the code no longer has a break statement and it directly returns.
Hence, it is no longer necessary to have a label.
Change-Id: Idde0fcc4d2db2d76424679f5acfe33ab8573bce4
Reviewed-on: https://go-review.googlesource.com/96935 Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Lubomir I. Ivanov (VMware) [Sat, 24 Feb 2018 12:06:06 +0000 (12:06 +0000)]
os/user: obtain a user home path on Windows
newUserFromSid() is extended so that the retriaval of the user home
path based on a user SID becomes possible.
(1) The primary method it uses is to lookup the Windows registry for
the following key:
HKLM\SOFTWARE\Microsoft\Windows NT\CurrentVersion\ProfileList\[SID]
If the key does not exist the user might not have logged in yet.
If (1) fails it falls back to (2)
(2) The second method the function uses is to look at the default home
path for users (e.g. WINAPI's GetProfilesDirectory()) and append
the username to that. The procedure is in the lines of:
c:\Users + \ + <username>
The function newUser() now requires the following arguments:
uid, gid, dir, username, domain
This is done to avoid multiple calls to usid.String() and
usid.LookupAccount("") in the case of a newUserFromSid()
call stack.
The functions current() and newUserFromSid() both call newUser()
supplying the arguments in question. The helpers
lookupUsernameAndDomain() and findHomeDirInRegistry() are
added.
This commit also updates:
- go/build/deps_test.go, so that the test now includes the
"internal/syscall/windows/registry" import.
- os/user/user_test.go, so that User.HomeDir is tested on Windows.
GitHub-Last-Rev: 25423e2a3820121f4c42321e7a77a3977f409724
GitHub-Pull-Request: golang/go#23822
Change-Id: I6c3ad1c4ce3e7bc0d1add024951711f615b84ee5
Reviewed-on: https://go-review.googlesource.com/93935 Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Daniel Martí [Tue, 20 Feb 2018 10:21:41 +0000 (10:21 +0000)]
cmd/compile/internal/syntax: use stringer for operators and tokens
With its new -linecomment flag, it is now possible to use stringer on
values whose strings aren't valid identifiers. This is the case with
tokens and operators in Go.
Operator alredy had inline comments with each operator's string
representation; only minor modifications were needed. The inline
comments were added to each of the token names, using the same strategy.
Comments that were previously inline or part of the string arrays were
moved to the line immediately before the name they correspond to.
Finally, declare tokStrFast as a function that uses the generated arrays
directly. Avoiding the branch and strconv call means that we avoid a
performance regression in the scanner, perhaps due to the lack of
mid-stack inlining.
Performance is not affected. Measured with 'go test -run StdLib -fast'
on an X1 Carbon Gen2 (i5-4300U @ 1.90GHz, 8GB RAM, SSD), the best of 5
runs before and after the changes are:
Ilya Tocar [Tue, 31 Oct 2017 20:49:21 +0000 (15:49 -0500)]
math/big: speed-up addMulVVW on amd64
Use MULX/ADOX/ADCX instructions to speed-up addMulVVW,
when they are available. addMulVVW is a hotspot in rsa.
This is faster than ADD/ADC/IMUL version, because ADOX/ADCX only
modify carry/overflow flag, so they can be interleaved with each other
and with MULX, which doesn't modify flags at all.
Increasing unroll factor to e. g. 16 makes rsa 1% faster, but 3PrimeRSA2048Decrypt
performance falls back to baseline.
Joe Tsai [Fri, 23 Feb 2018 23:08:11 +0000 (15:08 -0800)]
archive/zip: fix handling of Info-ZIP Unix extended timestamps
The Info-ZIP Unix1 extra field is specified as such:
>>>
Value Size Description
----- ---- -----------
0x5855 Short tag for this extra block type ("UX")
TSize Short total data size for this block
AcTime Long time of last access (GMT/UTC)
ModTime Long time of last modification (GMT/UTC)
<<<
The previous handling was incorrect in that it read the AcTime field
instead of the ModTime field.
The test-osx.zip test unfortunately locked in the wrong behavior.
Manually parsing that ZIP file shows that the encoded MS-DOS
date and time are 0x4b5f and 0xa97d, which corresponds with a
date of 2017-10-31 21:11:58, which matches the correct mod time
(off by 1 second due to MS-DOS timestamp resolution).
Fixes #23901
Change-Id: I567824c66e8316b9acd103dbecde366874a4b7ef
Reviewed-on: https://go-review.googlesource.com/96895
Run-TryBot: Joe Tsai <joetsai@google.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Ian Lance Taylor [Fri, 23 Feb 2018 18:34:01 +0000 (10:34 -0800)]
runtime: don't check for String/Error methods in printany
They have either already been called by preprintpanics, or they can
not be called safely because of the various conditions checked at the
start of gopanic.
Yuval Pavel Zholkover [Sat, 16 Dec 2017 17:06:10 +0000 (19:06 +0200)]
os: respect umask in Mkdir and OpenFile on BSD systems when perm has ModeSticky set
Instead of calling Chmod directly on perm, stat the created file/dir to extract the
actual permission bits which can be different from perm due to umask.
Fixes #23120.
Change-Id: I3e70032451fc254bf48ce9627e98988f84af8d91
Reviewed-on: https://go-review.googlesource.com/84477
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Austin Clements [Fri, 23 Feb 2018 17:03:00 +0000 (12:03 -0500)]
runtime: reduce arena size to 4MB on 64-bit Windows
Currently, we use 64MB heap arenas on 64-bit platforms. This works
well on UNIX-like OSes because they treat untouched pages as
essentially free. However, on Windows, committed memory is charged
against a process whether or not it has demand-faulted physical pages
in. Hence, on Windows, even a process with a tiny heap will commit
64MB for one heap arena, plus another 32MB for the arena map. Things
are much worse under the race detector, which increases the heap
commitment by a factor of 5.5X, leading to 384MB of committed memory
at runtime init.
Fix this by reducing the heap arena size to 4MB on Windows.
To counterbalance the effect of increasing the arena map size by a
factor of 16, and to further reduce the impact of the commitment for
the arena map, we switch from a single entry L1 arena map to a 64
entry L1 arena map.
Compared to the original arena design, this slows down the
x/benchmarks garbage benchmark by 0.49% (the slow down of this commit
alone is 1.59%, but the previous commit bought us a 1% speed-up):
name old time/op new time/op delta
Garbage/benchmem-MB=64-12 2.28ms ± 1% 2.29ms ± 1% +0.49% (p=0.000 n=17+18)
Austin Clements [Fri, 23 Feb 2018 01:38:09 +0000 (20:38 -0500)]
runtime: support a two-level arena map
Currently, the heap arena map is a single, large array that covers
every possible arena frame in the entire address space. This is
practical up to about 48 bits of address space with 64 MB arenas.
However, there are two problems with this:
1. mips64, ppc64, and s390x support full 64-bit address spaces (though
on Linux only s390x has kernel support for 64-bit address spaces).
On these platforms, it would be good to support these larger
address spaces.
2. On Windows, processes are charged for untouched memory, so for
processes with small heaps, the mostly-untouched 32 MB arena map
plus a 64 MB arena are significant overhead. Hence, it would be
good to reduce both the arena map size and the arena size, but with
a single-level arena, these are inversely proportional.
This CL adds support for a two-level arena map. Arena frame numbers
are now divided into arenaL1Bits of L1 index and arenaL2Bits of L2
index.
At the moment, arenaL1Bits is always 0, so we effectively have a
single level map. We do a few things so that this has no cost beyond
the current single-level map:
1. We embed the L2 array directly in mheap, so if there's a single
entry in the L2 array, the representation is identical to the
current representation and there's no extra level of indirection.
2. Hot code that accesses the arena map is structured so that it
optimizes to nearly the same machine code as it does currently.
3. We make some small tweaks to hot code paths and to the inliner
itself to keep some important functions inlined despite their
now-larger ASTs. In particular, this is necessary for
heapBitsForAddr and heapBits.next.
Possibly as a result of some of the tweaks, this actually slightly
improves the performance of the x/benchmarks garbage benchmark:
name old time/op new time/op delta
Garbage/benchmem-MB=64-12 2.28ms ± 1% 2.26ms ± 1% -1.07% (p=0.000 n=17+19)
Austin Clements [Fri, 23 Feb 2018 00:58:59 +0000 (19:58 -0500)]
cmd/compile: teach front-end deadcode about && and ||
The front-end dead code elimination is very simple. Currently, it just
looks for if statements with constant boolean conditions. Its main
purpose is to reduce load on the compiler and shrink code before
inlining computes hairiness.
This CL teaches front-end dead code elimination about short-circuiting
boolean expressions && and ||, since they're essentially the same as
if statements.
This also teaches the inliner that the constant 'if' form left behind
by deadcode is free.
These changes will help with runtime modifications in the next CL that
would otherwise inhibit inlining in some hot code paths. Currently,
however, they have no significant impact on benchmarks.
Austin Clements [Thu, 22 Feb 2018 17:35:30 +0000 (12:35 -0500)]
runtime: rename "arena index" to "arena map"
There are too many places where I want to talk about "indexing into
the arena index". Make this less awkward and ambiguous by calling it
the "arena map" instead.
Ian Lance Taylor [Mon, 19 Feb 2018 21:26:01 +0000 (13:26 -0800)]
path: use OS-specific function in MkdirAll, don't always keep trailing slash
CL 86295 changed MkdirAll to always pass a trailing path separator to
support extended-length paths on Windows.
However, when Stat is called on an existing file followed by trailing
slash, it will return a "not a directory" error, skipping the fast
path at the beginning of MkdirAll.
This change fixes MkdirAll to only pass the trailing path separator
where required on Windows, by using an OS-specific function fixRootDirectory.
Updates #23918
Change-Id: I23f84a20e65ccce556efa743d026d352b4812c34
Reviewed-on: https://go-review.googlesource.com/95255
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David du Colombier <0intro@gmail.com> Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Daniel Martí [Sat, 3 Feb 2018 15:36:38 +0000 (15:36 +0000)]
cmd/vet: use type info to detect the atomic funcs
Simply checking if a name is "atomic" isn't enough, as that might be a
var or another imported package. Now that vet requires type information,
we can do better. And add a simple regression test.
Change-Id: Ibd2004428374e3628cd3cd0ffb5f37cedaf448ea
Reviewed-on: https://go-review.googlesource.com/91795
Run-TryBot: Daniel Martí <mvdan@mvdan.cc> Reviewed-by: Robert Griesemer <gri@golang.org>
Adam Langley [Thu, 22 Feb 2018 20:30:44 +0000 (12:30 -0800)]
crypto/x509: tighten EKU checking for requested EKUs.
There are, sadly, many exceptions to EKU checking to reflect mistakes
that CAs have made in practice. However, the requirements for checking
requested EKUs against the leaf should be tighter than for checking leaf
EKUs against a CA.
Oleg Bulatov [Fri, 23 Feb 2018 15:55:19 +0000 (16:55 +0100)]
regexp: Regexp shouldn't keep references to inputs
If you try to find something in a slice of bytes using a Regexp object,
the byte array will not be released by GC until you use the Regexp object
on another slice of bytes. It happens because the Regexp object keep
references to the input data in its cache.
Alberto Donizetti [Fri, 23 Feb 2018 10:42:49 +0000 (11:42 +0100)]
cmd/compile: add code generation tests for sqrt intrinsics
Add "sqrt-intrisified" code generation tests for mips64 and 386, where
we weren't intrisifying math.Sqrt (see CL 96615 and CL 95916), and for
mips and amd64, which lacked sqrt intrinsics tests.
Alberto Donizetti [Thu, 22 Feb 2018 13:32:57 +0000 (14:32 +0100)]
cmd/compile: use | in the last repetitive generic rules
This change or-ifies the last low-hanging rules in generic. Again,
this is limited at short and repetitive rules, where the use or ors
does not impact readability.
Ran rulegen, no change in the actual compiler code.
Change-Id: I972b523bc08532f173a3645b47d6936b6e1218c8
Reviewed-on: https://go-review.googlesource.com/96335 Reviewed-by: Giovanni Bajo <rasky@develer.com>
Tobias Klauser [Thu, 22 Feb 2018 12:51:24 +0000 (13:51 +0100)]
syscall: remove list of unimplemented syscalls
The syscall package is frozen and we don't want to encourage anyone to
implement these syscalls.
Change-Id: I6b6e33e32a4b097da6012226aa15300735e50e9f
Reviewed-on: https://go-review.googlesource.com/96315 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Robert Griesemer [Thu, 22 Feb 2018 04:27:29 +0000 (20:27 -0800)]
go/types: fix regression with short variable declarations
The variables on the lhs of a short variable declaration are
only in scope after the variable declaration. Specifically,
function literals on the rhs of a short variable declaration
must not see newly declared variables on the lhs.
This used to work and this bug was likely introduced with
https://go-review.googlesource.com/c/go/+/83397 for go1.11.
Luckily this is just an oversight and the fix is trivial:
Simply use the mechanism for delayed type-checkin of function
literals introduced in the before-mentioned change here as well.
Fixes #24026.
Change-Id: I74ce3a0d05c5a2a42ce4b27601645964f906e82d
Reviewed-on: https://go-review.googlesource.com/96177 Reviewed-by: Alan Donovan <adonovan@google.com>
Alberto Donizetti [Wed, 21 Feb 2018 18:00:21 +0000 (19:00 +0100)]
cmd/compile: use | in the most repetitive s390x rules
For now, limited to the most repetitive rules that are also short and
simple, so that we can have a substantial conciseness win without
compromising rules readability.
Ran rulegen, no changes in the rewrite files.
Change-Id: I8447784895a218c5c1b4dfa1cdb355bd73dabfd1
Reviewed-on: https://go-review.googlesource.com/95955 Reviewed-by: Giovanni Bajo <rasky@develer.com>
Martin Möhrmann [Wed, 21 Feb 2018 21:27:12 +0000 (22:27 +0100)]
reflect: avoid calling common if type is known to be *rtype
If the type of Type is known to be *rtype than the common
function is a no-op and does not need to be called.
name old time/op new time/op delta
New 31.0ns ± 5% 30.2ns ± 4% -2.74% (p=0.008 n=20+20)
Change-Id: I5d00346dbc782e34c530166d1ee0499b24068b51
Reviewed-on: https://go-review.googlesource.com/96115 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Ben Shi [Sat, 17 Feb 2018 12:57:44 +0000 (12:57 +0000)]
cmd/compile: improve FP performance on ARM64
FMADD/FMSUB/FNMADD/FNMSUB are efficient FP instructions, which can
be used by the comiler to improve FP performance. This CL implements
this optimization.
David Chase [Fri, 30 Jun 2017 20:20:10 +0000 (16:20 -0400)]
cmd/compile: decouple emitted block order from regalloc block order
While tinkering with different block orders for the preemptible
loop experiment, crashed the register allocator with a "bad"
one (these exist). Realized that one knob was controlling
two things (register allocation and branch patterns) and
decided that life would be simpler if the two orders were
independent.
Ran some experiments and determined that we have probably,
mostly, been optimizing for register allocation effects, not
branch effects. Bad block orders for register allocation are
somewhat costly.
This will also allow separate experimentation with perhaps-
better block orders for register allocation.
Hana Kim [Tue, 6 Feb 2018 18:21:39 +0000 (13:21 -0500)]
cmd/trace: add memory usage reporting
Enabled when the tool runs with DEBUG_MEMORY_USAGE=1 env var.
After reporting the usage, it waits until user enters input
(helpful when checking top or other memory monitor)
Also adds net/http/pprof to export debug endpoints.
From the trace included in #21870
$ DEBUG_MEMORY_USAGE=1 go tool trace trace.out
2018/02/21 16:04:49 Parsing trace...
after parsing trace
Alloc: 3385747848 Bytes
Sys: 3661654648 Bytes
HeapReleased: 0 Bytes
HeapSys: 3488907264 Bytes
HeapInUse: 3426377728 Bytes
HeapAlloc: 3385747848 Bytes
Enter to continue...
2018/02/21 16:05:09 Serializing trace...
after generating trace
Alloc: 4908929616 Bytes
Sys: 5319063640 Bytes
HeapReleased: 0 Bytes
HeapSys: 5032411136 Bytes
HeapInUse: 4982865920 Bytes
HeapAlloc: 4908929616 Bytes
Enter to continue...
2018/02/21 16:05:18 Splitting trace...
after spliting trace
Alloc: 4909026200 Bytes
Sys: 5319063640 Bytes
HeapReleased: 0 Bytes
HeapSys: 5032411136 Bytes
HeapInUse: 4983046144 Bytes
HeapAlloc: 4909026200 Bytes
Enter to continue...
2018/02/21 16:05:39 Opening browser. Trace viewer is listening on http://127.0.0.1:33661
after httpJsonTrace
Alloc: 5288336048 Bytes
Sys: 7790245896 Bytes
HeapReleased: 0 Bytes
HeapSys: 7381123072 Bytes
HeapInUse: 5324120064 Bytes
HeapAlloc: 5288336048 Bytes
Enter to continue...
Change-Id: I88bb3cb1af3cb62e4643a8cbafd5823672b2e464
Reviewed-on: https://go-review.googlesource.com/92355 Reviewed-by: Peter Weinberger <pjw@google.com>
Robert Griesemer [Wed, 21 Feb 2018 04:14:51 +0000 (20:14 -0800)]
cmd/compile/internal/syntax: simpler position base update for line directives (cleanup)
The existing code was somewhat convoluted and made several assumptions
about the encoding of position bases:
1) The position's base for a file contained a position whose base
pointed to itself (which is true but an implementation detail
of src.Pos).
2) Updating the position base for a line directive required finding
the base of the most recent's base position.
This change simply stores the file's position base and keeps using it
directly for each line directive (instead of getting it from the most
recently updated base).
Change-Id: I4d80da513bededb636eab0ce53257fda73f0dbc0
Reviewed-on: https://go-review.googlesource.com/95736 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Austin Clements [Tue, 20 Feb 2018 16:59:02 +0000 (11:59 -0500)]
runtime: clarify address space limit constants and comments
Now that we support the full non-contiguous virtual address space of
amd64 hardware, some of the comments and constants related to this are
out of date.
This renames memLimitBits to heapAddrBits because 1<<memLimitBits is
no longer the limit of the address space and rewrites the comment to
focus first on hardware limits (which span OSes) and then discuss
kernel limits.
Second, this eliminates the memLimit constant because there's no
longer a meaningful "highest possible heap pointer value" on amd64.
Austin Clements [Mon, 19 Feb 2018 21:10:58 +0000 (16:10 -0500)]
runtime: offset the heap arena index by 2^47 on amd64
On amd64, the virtual address space, when interpreted as signed
values, is [-2^47, 2^47). Currently, we only support heap addresses in
the "positive" half of this, [0, 2^47). This suffices for linux/amd64
and windows/amd64, but solaris/amd64 can map user addresses in the
negative part of this range. Specifically, addresses
0xFFFF8000'00000000 to 0xFFFFFD80'00000000 are part of user space.
This leads to "memory allocated by OS not in usable address space"
panic, since we don't map heap arena index space for these addresses.
Fix this by offsetting addresses when computing arena indexes so that
arena entry 0 corresponds to address -2^47 on amd64. We already map
enough arena space for 2^48 heap addresses on 64-bit (because arm64's
virtual address space is [0, 2^48)), so we don't need to grow any
structures to support this.
A different approach would be to simply mask out the top 16 bits.
However, there are two advantages to the offset approach: 1) invalid
heap addresses continue to naturally map to invalid arena indexes so
we don't need extra checks and 2) it perturbs the mapping of addresses
to arena indexes more, which helps check that we don't accidentally
compute incorrect arena indexes somewhere that happen to be right most
of the time.
Several comments and constant names are now somewhat misleading. We'll
fix that in the next CL. This CL is the core change the arena
indexing.
Austin Clements [Fri, 16 Feb 2018 22:53:16 +0000 (17:53 -0500)]
runtime: abstract indexing of arena index
Accessing the arena index is about to get slightly more complicated.
Abstract this away into a set of functions for going back and forth
between addresses and arena slice indexes.
Austin Clements [Fri, 16 Feb 2018 22:45:21 +0000 (17:45 -0500)]
runtime: simplify bulkBarrierPreWrite
Currently, bulkBarrierPreWrite uses inheap to decide whether the
destination is in the heap or whether to check for stack or global
data. However, this isn't the best question to ask.
Instead, get the span directly and query its state. This lets us
directly determine whether this might be a global, or is stack memory,
or is heap memory.
At this point, inheap is no longer used in the hot path, so drop it
from the must-be-inlined list and substitute spanOf.
This will help in a circuitous way with #23862, since fixing that is
going to push inheap very slightly over the inline-able threshold on a
few platforms.
Hana Kim [Fri, 26 Jan 2018 15:28:10 +0000 (10:28 -0500)]
cmd/trace: include P info in goroutine slices
The task-oriented trace view presents the execution trace organized
based on goroutines. Often, which P a goroutine was running on is
useful, so this CL includes the P ids in the goroutine execution slices.
Heschi Kreinick [Fri, 2 Feb 2018 21:26:58 +0000 (16:26 -0500)]
cmd/compile/internal: use sparseSet, optimize isSynthetic
changedVars was functionally a set, but couldn't be iterated over
efficiently. In functions with many variables, the wasted iteration was
costly. Use a sparseSet instead.
(*gc.Node).String() is very expensive: it calls Sprintf, which does
reflection, etc, etc. Instead, just look at .Sym.Name, which is all we
care about.
Change-Id: Ib61cd7b5c796e1813b8859135e85da5bfe2ac686
Reviewed-on: https://go-review.googlesource.com/92402 Reviewed-by: David Chase <drchase@google.com>