Matthew Dempsky [Tue, 27 Mar 2018 20:50:08 +0000 (13:50 -0700)]
cmd/compile: insert instrumentation during SSA building
Insert appropriate race/msan calls before each memory operation during
SSA construction.
This is conceptually simple, but subtle because we need to be careful
that inserted instrumentation calls don't clobber arguments that are
currently being prepared for a user function call.
reorder1 already handles introducing temporary variables for arguments
in some cases. This CL changes it to use them for all arguments when
instrumenting.
Also, we can't SSA struct types with more than one field while
instrumenting. Otherwise, concurrent uses of disjoint fields within an
SSA-able struct can introduce false races.
This is both somewhat better and somewhat worse than the old racewalk
instrumentation pass. We're now able to easily recognize cases like
constructing non-escaping closures on the stack or accessing closure
variables don't need instrumentation calls. On the other hand,
spilling escaping parameters to the heap now results in an
instrumentation call.
Overall, this CL results in a small net reduction in the number of
instrumentation calls, but a small net increase in binary size for
instrumented executables. cmd/go ends up with 5.6% fewer calls, but a
2.4% larger binary.
This change replaces the vendored socks client implementation with the
bundle of golang.org/x/net/internal/socks package which contains fixes
for 19354 and 11682.
Kir Kolyshkin [Tue, 6 Feb 2018 21:16:02 +0000 (13:16 -0800)]
os/user: add a way to enforce pure Go implementation
This provides a way to enforce pure Go implementation of os/user
lookup functions on UNIX platforms by means of "osusergo" build tag,
in a manner similar to netgo/netcgo tags in the net package.
If "osusergo" build tag is set, Go implementation is selected.
If "osusergo" build tag is NOT set, the old behavior is retained,
that is to use cgo (libc-backed) implementation if both cgo and such
and such implementation are available.
The reason behind this change is to make it possible to build proper
static binaries on Linux. The problem is, glibc implementation of
getpw*, getgrp* and getgrouplist functions relies on presense of
libnss*.so libraries during runtime, making it impossible to build
a self-contained static binary which uses both cgo and os/user.
In such case, linker warnings like this are shown:
> warning: Using 'getgrouplist' in statically linked applications
> requires at runtime the shared libraries from the glibc version
> used for linking
While this can be solved by recompiling glibc with --enable-static-nss
flag or using a different libc implementation (like musl on Alpine Linux),
it is not always practical or even possible.
Matthew Dempsky [Fri, 6 Apr 2018 05:42:16 +0000 (22:42 -0700)]
cmd/compile: fix method expressions with anonymous receivers
Method expressions with anonymous receiver types like "struct { T }.m"
require wrapper functions, which we weren't always creating. This in
turn resulted in linker errors.
This CL ensures that we generate wrapper functions for any anonymous
receiver types used in a method expression.
Brian Kessler [Thu, 30 Nov 2017 16:32:07 +0000 (09:32 -0700)]
math/big: clean up z.div(z, x, y) calls
Updates #22830
Due to not checking if the output slices alias in divLarge,
calls of the form z.div(z, x, y) caused the slice z
to attempt to be used to store both the quotient and the
remainder of the division. CL 78995 applies an alias
check to correct that error. This CL cleans up the
additional div calls that attempt to supply the same slice
to hold both the quotient and remainder.
Note that the call in expNN was responsible for the reported
error in r.Exp(x, 1, m) when r was initialized to a non-zero value.
The second instance in expNNMontgomery did not result in an error
due to the size of the arguments.
// RR = 2**(2*_W*len(m)) mod m
RR := nat(nil).setWord(1)
zz := nat(nil).shl(RR, uint(2*numWords*_W))
_, RR = RR.div(RR, zz, m)
Specifically,
cap(RR) == 5 after setWord(1) due to const e = 4 in z.make(1)
len(zz) == 2*len(m) + 1 after shifting left, numWords = len(m)
Reusing the backing array for z and z2 in div was only triggered if
cap(RR) >= len(zz) + 1 and len(m) > 1 so that divLarge was called.
But, 5 < 2*len(m) + 2 if len(m) > 1, so new arrays were allocated
and the error was never triggered in this case.
Change-Id: Iedac80dbbde13216c94659e84d28f6f4be3aaf24
Reviewed-on: https://go-review.googlesource.com/81055
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
Matthew Dempsky [Thu, 5 Apr 2018 01:42:39 +0000 (18:42 -0700)]
cmd/compile: cleanup method symbol creation
There were multiple ad hoc ways to create method symbols, with subtle
and confusing differences between them. This CL unifies them into a
single well-documented encoding and implementation.
This introduces some inconsequential changes to symbol format for the
sake of simplicity and consistency. Two notable changes:
1) Symbol construction is now insensitive to the package currently
being compiled. Previously, non-exported methods on anonymous types
received different method symbols depending on whether the method was
local or imported.
2) Symbols for method values parenthesized non-pointer receiver types
and non-exported method names, and also always package-qualified
non-exported method names. Now they use the same rules as normal
method symbols.
The methodSym function is also now stricter about rejecting
non-sensical method/receiver combinations. Notably, this means that
typecheckfunc needs to call addmethod to validate the method before
calling declare, which also means we no longer emit errors about
redeclaring bogus methods.
Change-Id: I9501c7a53dd70ef60e5c74603974e5ecc06e2003
Reviewed-on: https://go-review.googlesource.com/104876 Reviewed-by: Robert Griesemer <gri@golang.org>
adjustpointers loops over a bitmap.
If the length of that bitmap is zero,
we can skip making the call entirely.
This speeds up stack copying when there are
no pointers present in either args or locals.
Richard Musiol [Wed, 28 Mar 2018 22:53:26 +0000 (00:53 +0200)]
cmd/compile/internal/gc: factor out beginning of SSAGenState.Call
This commit does not change the semantics of the Call method. Its
purpose is to avoid duplication of code by making PrepareCall available
for separate use by the wasm backend.
Joel Sing [Thu, 5 Apr 2018 19:00:25 +0000 (05:00 +1000)]
runtime: fix/improve exitThread on openbsd
OpenBSD's __threxit syscall takes a pointer to a 32-bit value that will be
zeroed immediately before the thread exits. Make use of this instead of
zeroing freeWait from the exitThread assembly and using hacks like switching
to a static stack, so this works on 386.
Matthew Dempsky [Thu, 5 Apr 2018 04:49:49 +0000 (21:49 -0700)]
cmd/compile: drop legacy code for generating iface wrappers
Originally, scalar values were directly stored within interface values
as long as they fit into a pointer-sized slot of memory. And since
interface method calls always pass the full pointer-sized value as the
receiver argument, value-narrowing wrappers were necessary to adapt to
the calling convention for methods with smaller receiver types.
However, for precise garbage collection, we now only store actual
pointers within interface values, so these wrappers are no longer
necessary.
VEX constants were used when instructions were added by hand.
Now all VEX-encoded instructions are auto-generated by x86avxgen,
so there is no need for those anymore.
Ben Shi [Thu, 22 Mar 2018 02:18:50 +0000 (02:18 +0000)]
cmd/compile: optimize 386 binary operations with a memory operand
Some integer/float binary operations of 386 can take a direct memory
operand, which is more efficient than loading to a register.
These CL does this optimization by copying the similar solution
of amd64. And the go1 benchmark shows some inprovements, especially
the test case Template. (excluding noise)
Alberto Donizetti [Wed, 4 Apr 2018 12:35:05 +0000 (14:35 +0200)]
cmd/compile: stack-allocate worklist in ReachableBlocks
Stack-allocate a local worklist in the deadcode pass. A size of 64 for
the pre-allocation is enough for >99% of the ReachableBlocks call in
a typical package.
Matthew Dempsky [Wed, 4 Apr 2018 22:53:27 +0000 (15:53 -0700)]
cmd/compile: extract inline related fields into separate Inline type
Inl, Inldcl, and InlCost are only applicable to functions with bodies
that can be inlined, so pull them out into a separate Inline type to
make understanding them easier.
A side benefit is that we can check if a function can be inlined by
just checking if n.Func.Inl is non-nil, which simplifies handling of
empty function bodies.
While here, remove some unnecessary Curfn twiddling, and make imported
functions use Inl.Dcl instead of Func.Dcl for consistency for local
functions.
Passes toolstash-check.
Change-Id: Ifd4a80349d85d9e8e4484952b38ec4a63182e81f
Reviewed-on: https://go-review.googlesource.com/104756
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
David Chase [Sat, 3 Mar 2018 01:33:15 +0000 (20:33 -0500)]
cmd/compile: adjust is-statement on Pos's to improve debugging
Stores to auto tmp variables can be hoisted to places
where the line numbers make debugging look "jumpy".
Turning those instructions into ones with is_stmt = 0 in
the DWARF (accomplished by marking ssa nodes with NotStmt)
makes debugging look better while still attributing the
instructions with the correct line number.
The same is true for certain register allocator spills and
reloads.
Change-Id: I97a394eb522d4911cc40b4bf5bf76d3d7221f6c0
Reviewed-on: https://go-review.googlesource.com/98415
Run-TryBot: David Chase <drchase@google.com> Reviewed-by: Cherry Zhang <cherryyz@google.com> Reviewed-by: Keith Randall <khr@golang.org>
David Chase [Tue, 13 Feb 2018 22:39:38 +0000 (17:39 -0500)]
cmd/link: process is_stmt data into dwarf line tables
To improve debugging, instructions should be annotated with
DWARF is_stmt. The DWARF default before was is_stmt=1, and
to remove "jumpy" stepping the optimizer was tagging
instructions with a no-position position, which interferes
with the accuracy of profiling information. This allows
that to be corrected, and also allows more "jumpy" positions
to be annotated with is_stmt=0 (these changes were not made
for 1.10 because of worries about further messing up
profiling).
The is_stmt values are placed in a pc-encoded table and
passed through a symbol derived from the name of the
function and processed in the linker alongside its
processing of each function's pc/line tables.
The only change in binary size is in the .debug_line tables
measured with "objdump -h --section=.debug_line go1.test"
For go1.test, these are 2614 bytes larger,
or 0.72% of the size of .debug_line,
or 0.025% of the file size.
This will increase in proportion to how much the is_stmt
flag is used (toggled).
David Chase [Wed, 3 Jan 2018 22:14:55 +0000 (17:14 -0500)]
cmd/compile: add IsStmt breakpoint info to src.lico
Add IsStmt information to src.lico so that suitable lines
for breakpoints (or not) can be noted, eventually for
communication to the debugger via the linker and DWARF.
The expectation is that the front end will apply statement
boundary marks because it has best information about the
input, and the optimizer will attempt to preserve these.
The exact method for placing these marks is still TBD;
ideally stopping "at" line N in unoptimized code will occur
at a point where none of the side effects of N have occurred
and all of the inputs for line N can still be observed.
The optimizer will work with the same markings supplied
for unoptimized code.
It is a goal that non-optimizing compilation should conserve
statement marks.
The optimizer will also use the not-a-statement annotation
to indicate instructions that have a line number (for
profiling purposes) but should not be the target of
debugger step, next, or breakpoints. Because instructions
marked as statements are sometimes removed, a third value
indicating that a position (instruction) can serve as a
statement if the optimizer removes the current instruction
marked as a statement for the same line. The optimizer
should attempt to conserve statement marks, but it is not
a bug if some are lost.
Includes changes to html output for GOSSAFUNC to indicate
not-default is-a-statement with bold and not-a-statement
with strikethrough.
Robert Griesemer [Wed, 4 Apr 2018 00:05:47 +0000 (17:05 -0700)]
go/printer, gofmt: tuned table alignment for better results
The go/printer (and thus gofmt) uses a heuristic to determine
whether to break alignment between elements of an expression
list which is spread across multiple lines. The heuristic only
kicked in if the entry sizes (character length) was above a
certain threshold (20) and the ratio between the previous and
current entry size was above a certain value (4).
This heuristic worked reasonably most of the time, but also
led to unfortunate breaks in many cases where a single entry
was suddenly much smaller (or larger) then the previous one.
The behavior of gofmt was sufficiently mysterious in some of
these situations that many issues were filed against it.
The simplest solution to address this problem is to remove
the heuristic altogether and have a programmer introduce
empty lines to force different alignments if it improves
readability. The problem with that approach is that the
places where it really matters, very long tables with many
(hundreds, or more) entries, may be machine-generated and
not "post-processed" by a human (e.g., unicode/utf8/tables.go).
If a single one of those entries is overlong, the result
would be that the alignment would force all comments or
values in key:value pairs to be adjusted to that overlong
value, making the table hard to read (e.g., that entry may
not even be visible on screen and all other entries seem
spaced out too wide).
Instead, we opted for a slightly improved heuristic that
behaves much better for "normal", human-written code.
1) The threshold is increased from 20 to 40. This disables
the heuristic for many common cases yet even if the alignment
is not "ideal", 40 is not that many characters per line with
todays screens, making it very likely that the entire line
remains "visible" in an editor.
2) Changed the heuristic to not simply look at the size ratio
between current and previous line, but instead considering the
geometric mean of the sizes of the previous (aligned) lines.
This emphasizes the "overall picture" of the previous lines,
rather than a single one (which might be an outlier).
3) Changed the ratio from 4 to 2.5. Now that we ignore sizes
below 40, a ratio of 4 would mean that a new entry would have
to be 4 times bigger (160) or smaller (10) before alignment
would be broken. A ratio of 2.5 seems more sensible.
Applied updated gofmt to all of src and misc. Also tested
against several former issues that complained about this
and verified that the output for the given examples is
satisfactory (added respective test cases).
Some of the files changed because they were not gofmt-ed
in the first place.
For #644.
For #7335.
For #10392.
(and probably more related issues)
Hana Kim [Wed, 4 Apr 2018 17:47:53 +0000 (13:47 -0400)]
cmd/trace: avoid emitting traceview slice with 0 duration
The trace viewer interprets the slice as a non-terminating
time interval which is quite opposit to what trace records indicate
(i.e., almostly immediately terminating time interval).
As observed in the issue #24663 this can result in quite misleading
visualization of the trace.
Work around the trace viewer's issue by setting a small value
(0.0001usec) as the duration if the time interval is not positive.
This mode is similar to the default traceview mode where the execution
trace is presented in P-oriented way. Each row represents a P, and each
slice represents the time interval of a goroutine's execution on the P.
The difference is that, in this mode, only the execution of goroutines
involved in the specified task is highlighted, and other goroutine
execution or events are greyed out. So, users can focus on how a task is
executed while considering other affecting conditions such as other
goroutines, network events, or process scheduling.
Here, for a while the program remained idle after the first burst of
activity related to the task because all other goroutines were also
being blocked or waiting for events, or no incoming network traffic
(indicated by the lack of any network activity). This is a bit hard to
discover when the usual task-oriented view (/trace?taskid=<taskid>)
mode.
Also, it simplifies the traceview generation mode logic.
/trace ---> 0
/trace?goid ---> modeGoroutineOriented
/trace?taskid ---> modeGoroutineOriented|modeTaskOriented
/trace?focustask ---> modeTaskOriented
Change-Id: Idcc0ae31b708ddfd19766f4e26ee7efdafecd3a5
Reviewed-on: https://go-review.googlesource.com/103555
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com> Reviewed-by: Heschi Kreinick <heschi@google.com>
runtime: stop when we run out of hints in race mode
Currently, the runtime falls back to asking for any address the OS can
offer for the heap when it runs out of hint addresses. However, the
race detector assumes the heap lives in [0x00c000000000,
0x00e000000000), and will fail in a non-obvious way if we go outside
this region.
Fix this by actively throwing a useful error if we run out of heap
hints in race mode.
This problem is currently being triggered by TestArenaCollision, which
intentionally triggers this fallback behavior. Fix the test to look
for the new panic message in race mode.
cmd/link: put runtime.framepointer_enabled in DATA instead of RODATA
On darwin, only writable symbol is exported
(cmd/link/internal/ld/macho.go:/machoShouldExport).
For plugin to work correctly, global variables, including
runtime.framepointer_enabled which is set by the linker, need
to be exported when dynamic linking. Put it in DATA so it is
exported. Also in Go it is defined as a var, which is not
read-only.
To avoid confusion, rename PipeNode.Decl to PipeNode.Vars, as the
variables may not always be declared after this change. Also change a
few other names to better reflect the added ambiguity of variables in
pipelines.
Modifying the text/template/parse package in a backwards incompatible
manner is acceptable, given that the package godoc clearly states that
it isn't intended for general use. It's the equivalent of an internal
package, back when internal packages didn't exist yet.
To make the changes to the parse package sit well with the cmd/api test,
update except.txt with the changes that we aren't worried about.
Fixes #10608.
Change-Id: I1f83a4297ee093fd45f9993cebb78fc9a9e81295
Reviewed-on: https://go-review.googlesource.com/84480
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Rob Pike <r@golang.org>
cmd/go: rebuild as needed when vetting test packages
If A's external test package imports B, which imports A, and A's
internal test code adds something to A that invalidates anything in A's
export data, then we need to build B against the test-augmented version
of A before using it to build A's external test package.
https://golang.org/cl/92215 taught 'go test' to do this rebuilding
properly, but 'go vet' was not taught the same trick when it learned to
vet test packages in https://golang.org/cl/87636. This commit moves the
necessary logic into the load.TestPackagesFor function so it can be
shared by 'go test' and 'go vet'.
Add the following helpers in lookup_windows.go:
1) lookupGroupName() is used to obtain the SID of a group based
on name.
2) listGroupsForUsernameAndDomain() uses NetUserGetLocalGroups()
as a WINAPI backend to obtain the list of local groups for this
user.
3) lookupUserPrimaryGroup() is now used to populate the User.Gid
field when looking up a user by name.
Implement listGroups(), lookupGroupId(), lookupGroup() and no longer
return unimplemented errors.
Do not skip Windows User.Gid tests in user_test.go.
Change-Id: I81fd41b406da51f9a4cb24e50d392a333df81141
GitHub-Last-Rev: d1448fd55d6eaa0f41bf347df18b40da06791df1
GitHub-Pull-Request: golang/go#24222
Reviewed-on: https://go-review.googlesource.com/98137 Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Giovanni Bajo [Tue, 3 Apr 2018 16:58:01 +0000 (18:58 +0200)]
cmd/compile: in prove, complete support for OpIsInBounds/OpIsSliceInBounds
The logic in addBranchRestrictions didn't allow to correctly
model OpIs(Slice)Bound for signed domain, and it was also partly
implemented within addRestrictions.
Thanks to the previous changes, it is now possible to handle
the negative conditions correctly, so that we can learn
both signed/LT + unsigned/LT on the positive side, and
signed/GE + unsigned/GE on the negative side (but only if
the index can be proved to be non-negative).
This is able to prove ~50 more slice accesses in std+cmd.
Change-Id: I9858080dc03b16f85993a55983dbc4b00f8491b0
Reviewed-on: https://go-review.googlesource.com/104037
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Austin Clements <austin@google.com>
Giovanni Bajo [Sun, 1 Apr 2018 23:45:53 +0000 (01:45 +0200)]
cmd/compile: in prove, make addRestrictions more generic
addRestrictions was taking a branch parameter, binding its logic
to that of addBranchRestrictions. Since we will need to use it
for updating the facts table for induction variables, refactor it
to remove the branch parameter.
Passes toolstash -cmp.
Change-Id: Iaaec350a8becd1919d03d8574ffd1bbbd906d068
Reviewed-on: https://go-review.googlesource.com/104036
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Austin Clements <austin@google.com>
math/big: remove "else" from if with block that ends with return
That "else" was needed due to gc DCE limitations.
Now it's not the case and we can avoid go lint complaints.
(See #23521 and https://golang.org/cl/91056.)
There is inlining test for bigEndianWord, so if test
is passing, no performance regression should occur.
Alberto Donizetti [Tue, 3 Apr 2018 11:46:36 +0000 (13:46 +0200)]
cmd/compile: stack-allocate values worklist in schedule
Compiler instrumentation shows that the cap of the stores slice in the
storeOrder function is almost always 64 or less. Since the slice does
not escape, pre-allocating on the stack a 64-elements one greatly
reduces the number of allocations performed by the function.
Filippo Valsorda [Fri, 2 Dec 2016 19:43:45 +0000 (19:43 +0000)]
crypto/tls: simplify the Handshake locking strategy
If in.Mutex is never locked by Handshake when c.handshakeComplete is
true, and since c.handshakeComplete is unset and then set back by
handleRenegotiation all under both in.Mutex and handshakeMutex, we can
significantly simplify the locking strategy by removing the sync.Cond.
See also https://groups.google.com/forum/#!topic/golang-dev/Xxiai-R_jH0
and a more complete analysis at https://go-review.googlesource.com/c/go/+/33776#message-223a3ccc819f7015cc773d214c65bad70de5dfd7
Change-Id: I6052695ece9aff9e3112c2fb176596fde8aa9cb2
Reviewed-on: https://go-review.googlesource.com/33776 Reviewed-by: Adam Langley <agl@golang.org>
Michael Munday [Tue, 3 Apr 2018 14:49:06 +0000 (15:49 +0100)]
cmd/asm, math: add s390x floating point test instructions
Floating point test instructions allow special cases (NaN, ±∞ and
a few other useful properties) to be checked directly.
This CL adds the following instructions to the assembler:
* LTEBR - load and test (float32)
* LTDBR - load and test (float64)
* TCEB - test data class (float32)
* TCDB - test data class (float64)
Note that I have only added immediate versions of the 'test data
class' instructions for now as that's the only case I think the
compiler will use.
Javier Kohen [Tue, 20 Mar 2018 18:35:37 +0000 (14:35 -0400)]
regexp: use sync.Pool to cache regexp.machine objects
Performance optimization for the internals of the Regexp type. This adds
no features and has no user-visible impact beyond performance. Copy now
shares the cache, so memory usage for programs that use Copy a lot
should go down; Copy has effectively become a no-op.
The before v. after benchmark results show a lot of noise from run to
run, but there's a clear improvement to the Shared case and no detriment
to the Copied case.
Macro benchmarks show that the lock contention in Regexp is gone, and my
server is now able to scale linearly 2.5x times more than before (and I
only stopped there because I ran out of CPU in my test machine).
isharipo [Thu, 1 Feb 2018 17:37:23 +0000 (20:37 +0300)]
cmd/compile: make DCE remove nodes after terminating if
This change makes compiler frontend dead code elimination of const expr if
statements introduced in https://golang.org/cl/38773 treat both
if constCondTrue { ...; returnStmt } toBeRemoved...
if constCondFalse { ...; } else { returnStmt } toBeRemoved...
identically to:
if constCondTrue { ...; returnStmt } else { toBeRemoved... }
Where "constCondTrue" is a an expression that can be evaluated
to "true" during compile time.
The additional checks are only triggered for const expr
if conditions that evaluate to true.
Daniel Martí [Mon, 23 Oct 2017 18:57:07 +0000 (19:57 +0100)]
cmd/compile: introduce gc.Node.copy method
When making a shallow copy of a node, various methods were used,
including calling nod(OXXX, nil, nil) and then overwriting it, or
"n1 := *n" and then using &n1.
Add a copy method instead, simplifying all of those and making them
consistent.
Passes toolstash -cmp on std cmd.
Change-Id: I3f3fc88bad708edc712bf6d87214cda4ddc43b01
Reviewed-on: https://go-review.googlesource.com/72710
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Giovanni Bajo [Sun, 1 Apr 2018 23:39:03 +0000 (01:39 +0200)]
cmd/compile: in prove, simplify logic of branch pushing
prove used a complex logic when trying to prove branch conditions:
tryPushBranch() was sometimes leaving a checkpoint on the factsTable,
sometimes not, and the caller was supposed to check the return value
to know what to do.
Since we're going to make the prove descend logic a little bit more
complex by adding also induction variables, simplify the tryPushBranch
logic, by removing any factsTable checkpoint handling from it.
Passes toolstash -cmp.
Change-Id: Idfb1703df8a455f612f93158328b36c461560781
Reviewed-on: https://go-review.googlesource.com/104035
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Austin Clements <austin@google.com>
Matthew Dempsky [Tue, 3 Apr 2018 00:33:38 +0000 (17:33 -0700)]
cmd/compile: improve declaration position precision
Previously, n.Pos was reassigned to lineno when declare was called,
which might not match where the identifier actually appeared in the
source. This caused a loss of position precision for function
parameters (which were all declared at the last parameter's position),
and required some clumsy workarounds in bimport.go.
This CL changes declare to leave n.Pos alone and also fixes a few
places where n.Pos was not being set correctly.
Change-Id: Ibe5b5fd30609c684367207df701f9a1bfa82867f
Reviewed-on: https://go-review.googlesource.com/104275 Reviewed-by: Robert Griesemer <gri@golang.org>
Robert Griesemer [Fri, 30 Mar 2018 01:22:23 +0000 (18:22 -0700)]
cmd/compile: better handling of incorrect type switches
Don't report errors if we don't have a correct type switch
guard; instead ignore it and leave it to the type-checker
to report the error. This leads to better error messages
concentrating on the type switch guard rather than errors
around (confusing) syntactic details.
Also clean up some code setting up AssertExpr (they never
have a nil Type field) and remove some incorrect TODOs.
Fixes #24470.
Change-Id: I69512f36e0417e3b5ea9c8856768e04b19d654a8
Reviewed-on: https://go-review.googlesource.com/103615
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Matthew Dempsky [Mon, 2 Apr 2018 22:38:57 +0000 (15:38 -0700)]
cmd/compile: simplify exportsym flags and logic
We used to have three Sym flags for dealing with export/reexport:
Export, Package, and Exported.
Export and Package were used to distinguish whether a symbol is
exported or package-scope (i.e., mutually exclusive), except that for
local declarations Export served double-duty as tracking whether the
symbol had been added to exportlist.
Meanwhile, imported declarations that needed reexporting could be
added to exportlist multiple times, necessitating a flag to track
whether they'd already been written out by exporter.
Simplify all of these into a single OnExportList flag so that we can
ensure symbols on exportlist are present exactly once. Merge
reexportsym into exportsym so there's a single place where we append
to exportlist.
Code that used to set Exported to prevent a symbol from being exported
can now just set OnExportList before calling declare to prevent it
from even appearing on exportlist.
Lastly, drop the IsAlias check in exportsym: we call exportsym too
early for local symbols to detect if they're an alias, and we never
reexport aliases.
Matthew Dempsky [Sat, 31 Mar 2018 01:58:03 +0000 (18:58 -0700)]
cmd/compile: simplify reexport logic
Currently, we reexport any package-scope constant, function, type, or
variable declarations needed by an inlineable function body. However,
now that we have an early pass to walk inlineable function bodies
(golang.org/cl/74110), we can simplify the logic for finding these
declarations.
The binary export format supports writing out type declarations
in-place at their first use. Also, it always writes out constants by
value, so their declarations never need to be reexported.
Notably, we attempted this before (golang.org/cl/36170) and had to
revert it (golang.org/cl/45911). However, this was because while
writing out inline bodies, we could discover variable/function
dependencies. By collecting variable/function dependencies during
inlineable function discovery, we avoid this problem.
While here, get rid of isInlineable. We already typecheck inlineable
function bodies during inlFlood, so it's become a no-op. Just move the
comment explaining parameter numbering to its caller.
Change-Id: Ibbfaafce793733675d3a2ad98791758583055666
Reviewed-on: https://go-review.googlesource.com/103864 Reviewed-by: Robert Griesemer <gri@golang.org>
Ilya Tocar [Mon, 26 Feb 2018 20:00:57 +0000 (14:00 -0600)]
strings: speed-up replace for byteStringReplacer case
Use Count instead of loop to determine a number of replacements.
Also increment index instead of advancing slices, to avoid some extra stores.
Shows very significant speed-up on html benchmarks:
Matthew Dempsky [Tue, 27 Mar 2018 22:35:51 +0000 (15:35 -0700)]
cmd/compile: disable instrumentation for no-race packages earlier
Rather than checking for each function whether the package supports
instrumentation, check once up front.
Relatedly, tweak the logic for preventing inlining calls to runtime
functions from instrumented packages. Previously, we simply disallowed
inlining runtime functions altogether when instrumenting. With this
CL, it's only disallowed from packages that are actually being
instrumented. That is, now intra-runtime calls can be inlined.
Giovanni Bajo [Mon, 2 Apr 2018 21:13:43 +0000 (23:13 +0200)]
go/types: fix column reporting of invalid selector names
Fixes #24645
Change-Id: I914674451b6667c3ebaf012893503d9de58991ee
Reviewed-on: https://go-review.googlesource.com/104155
Run-TryBot: Giovanni Bajo <rasky@develer.com> Reviewed-by: Robert Griesemer <gri@golang.org>
compress/gzip: do not count header bytes written in Write
Before, if an underlying writer errored within 10 bytes (plus any gzip
header metadata), a gzip.Write would erroneously report up to 10 bytes
written that were not actually written of the input slice. This is
especially problematic when the input slice is less than 10 bytes.
The error came from counting the 10 header byte write. If writing the
header is completely successful, the 10 bytes written is overridden by
the flate write with the input slice.
This removes counting the 10 required header bytes, and also changes the
return to use zero until the slice is used.
The old Write could return one byte written when it actually was not.
This is difficult to verify because the smallest input slice is one
byte; a test checking that the input slice was the byte written would be
quite involved. Thankfully, gzip's minimum header write is 10 bytes. If
we test that two bytes are not falsely written, we indirectly cover the
one byte case.
Fixes #24625
Change-Id: I1c1f8cd791e0c4cffc22aa8acd95186582c832ba
Reviewed-on: https://go-review.googlesource.com/103861 Reviewed-by: Joe Tsai <joetsai@google.com>
Run-TryBot: Joe Tsai <joetsai@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
When test/run script was removed, these two tests
were changed to be executed by test/run.go.
Because errchk does not exit with non-zero status on
errors, they were silently failing for a while.
This change makes 2 things:
1. Compile tested packages in GOROOT/test to match older runner script
behavior (strictly required only in bug345, optional in bug248)
2. Check command output with "(?m)^BUG" regexp.
It approximates older `grep -q '^BUG' that was used before.