It's not safe to do p+x with unsafe if that would point past the
end of the object. (Valid in C, not safe in Go.)
Pass a "whySafe" reason (compiled away) to explain at each
call site why it's safe.
Fixes#21733.
Change-Id: I5da8c25bde66f5c9beac232f2135dcab8e8bf3b1
Reviewed-on: https://go-review.googlesource.com/80738
Reviewed-by: Austin Clements <austin@google.com>
The number of threads in syscall presented by execution tracer's
trace view includes not only the threads calling system calls on behalf
of user created goroutines, but also those running on behalf of system
goroutines.
When the number of such system goroutines was small, the graph was
useful when examining where a program was saturating the CPU.
But as more and more system goroutines are invloved the graph became
less useful for the purpose - for example, after golang.org/cl/34784,
the timer goroutines dominate in the graph with large P
because the runtime creates per-P timer goroutines.
This change excludes the threads in syscall on behalf of runtime (system
goroutines) from the visualization. Alternatively, I could visualize the
count of such threads in a separate counter but in the same graph.
Given that many other debug endpoints (e.g. /debug/pprof/goroutine) hide
the system goroutines, including them in the same graph can confuse users.
Update #22574
Change-Id: If758cd6b9ed0596fde9a471e846b93246580b9d5
Reviewed-on: https://go-review.googlesource.com/81315
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Minor.
Makes reading failing runtime test stacktraces easier (by having fewer
goroutines to read) on machines where these gdb tests wouldn't have
ever run anyway.
Change-Id: I3fab0667e017f20ef3bf96a8cc4cfcc614d25b5c
Reviewed-on: https://go-review.googlesource.com/81575
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This adds logging for the expected duration of a growStack, plus
progress information on the growStack that timed out.
Updates #19381.
Change-Id: Ic358f8350f499ff22dd213b658aece7d1aa62675
Reviewed-on: https://go-review.googlesource.com/81556
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Per the decision for #14844, index expressions that are non-constant
shifts where the LHS operand is representable as an int are now valid.
Fixes#21693.
Change-Id: Ifafad2c0c65975e0200ce7e28d1db210e0eacd9d
Reviewed-on: https://go-review.googlesource.com/81277
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
I think of "sending" a signal as calling kill, but sigsend is involved
in handling a signal and, specifically delivering it to the internal
signal queue. The term "delivery" is already used in
signalWaitUntilIdle, so this CL also uses it in the documentation for
sigsend.
Change-Id: I86e171f247f525ece884a680bace616fa9a3c7bd
Reviewed-on: https://go-review.googlesource.com/81235
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently, when we minit on a thread that already has an alternate
signal stack (e.g., because the M was an extram being used for a cgo
callback, or to handle a signal on a C thread, or because the
platform's libc always allocates a signal stack like on Android), we
simply drop the Go-allocated gsignal stack on the floor.
This is a problem for Ms on the extram list because those Ms may later
be reused for a different thread that may not have its own alternate
signal stack. On tip, this manifests as a crash in sigaltstack because
we clear the gsignal stack bounds in unminit and later try to use
those cleared bounds when we re-minit that M. On 1.9 and earlier, we
didn't clear the bounds, so this manifests as running more than one
signal handler on the same signal stack, which could lead to arbitrary
memory corruption.
This CL fixes this problem by saving the Go-allocated gsignal stack in
a new field in the m struct when overwriting it with a system-provided
signal stack, and then restoring the original gsignal stack in
unminit.
This CL is designed to be easy to back-port to 1.9. It won't quite
cherry-pick cleanly, but it should be sufficient to simply ignore the
change in mexit (which didn't exist in 1.9).
Now that we always have a place to stash the original signal stack in
the m struct, there are some simplifications we can make to the signal
stack handling. We'll do those in a later CL.
Fixes#22930.
Change-Id: I55c5a6dd9d97532f131146afdef0b216e1433054
Reviewed-on: https://go-review.googlesource.com/81476
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
If package strings has a particular set of gcflags, then the strings_test
pseudo-package built as part of the test binary should inherit the same flags.
Fixes#22831.
Change-Id: I0e896b6c0f1063454300b7323f577feffbd6650b
Reviewed-on: https://go-review.googlesource.com/81496
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
If the build of the test binary failed, the go command correctly
avoided running the binary, but the -x output indicated otherwise.
Fixes#22659.
Change-Id: Ib4d262bf1735f057c994a45fc23c499d4ebe3246
Reviewed-on: https://go-review.googlesource.com/81495
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The CL introducing merged handling of cover profiles
did not correctly account for the fact that the file name argument
to -coverprofile is required to be interpreted relative to
the -outputdir argument.
Fixes#22804.
Change-Id: I804774013c12187313b8fd2044302978bdbb6697
Reviewed-on: https://go-review.googlesource.com/81455
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The code that generates the list of DWARF variables for a function
(params and autos) will emit a "no-location" entry in the DWARF for a
user var that appears in the original pre-optimization version of the
function but is no longer around when optimization is complete. The
intent is that if a GDB user types "print foo" (where foo has been
optimized out), the response will be "<optimized out>" as opposed to
"there is no such variable 'foo'). This change fixes said code to
include vars on the autom list for the function, to insure that the
type symbol for the variable makes it to the linker.
Fixes#22941.
Change-Id: Id29f1f39d68fbb798602dfd6728603040624fc41
Reviewed-on: https://go-review.googlesource.com/81415
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Values must not be copied after the first use.
Using noCopy makes vet complain about copies
even before the first use, which is incorrect
and very frustrating.
Drop it.
Fixes#21504.
Change-Id: Icd3a5ac3fe11e84525b998e848ed18a5d996f45a
Reviewed-on: https://go-review.googlesource.com/80836
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Call is meant to mirror the language semantics, which allow:
var r io.ReadWriter
f := func(io.Reader){}
f(r)
even though the conversion from io.ReadWriter to io.Reader is
being applied to a nil interface. This is different from an explicit
conversion:
_ = r.(io.Reader)
f(r.(io.Reader))
Both of those lines panic, but the implicit conversion does not.
By using E2I, which is the implementation of the explicit conversion,
the reflect.Call equivalent of f(r) was inadvertently panicking.
Avoid the panic.
Fixes#22143.
Change-Id: I6b2f5b808e0cd3b89ae8bc75881e307bf1c25558
Reviewed-on: https://go-review.googlesource.com/80736
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The ssa backend is aggressive about placing constants and
certain other values in the Entry block. It's implausible
that the original line numbers for these constants makes
any sort of sense when it appears to a user stepping in a
debugger, and they're also not that useful in dumps since
entry-block instructions tend to be constants (i.e.,
unlikely to be the cause of a crash).
Therefore, use src.NoXPos for any values that are explicitly
inserted into a function's entry block.
Passes all tests, including ssa/debug_test.go with both
gdb and a fairly recent dlv. Hand-verified that it solves
the reported problem; constructed a test that reproduced
a problem, and fixed it.
Modified test harness to allow injection of slightly more
interesting inputs.
Fixes#22558.
Change-Id: I4476927067846bc4366da7793d2375c111694c55
Reviewed-on: https://go-review.googlesource.com/81215
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Fixes#19293
Change-Id: I35f2f786e2e3972eda21ba5a948433bfcd621269
Reviewed-on: https://go-review.googlesource.com/81355
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This reverts commit 08f19bbde1.
Reason for revert:
The changed transformation takes effect on a larger set
of code snippets than expected.
For example, this:
func foo() {
// Comment
bar()
}
becomes:
func foo() {
// Comment
bar()
}
This is an unintended consequence.
Change-Id: Ifca88d6267dab8a8170791f7205124712bf8ace8
Reviewed-on: https://go-review.googlesource.com/81335
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Joe Tsai <joetsai@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
GOEXPERIMENT is only set during make.bash, so checking the environment
variable isn't effectual. Instead, check the values exposed by objabi.
These experiments look potentially safe, but it seems too late in the
release cycle to try to assuage that. The one exception is frame
pointer experiment, which is trivially safe: it just amounts to
incrementing some stack offsets by PtrSize.
Fixes#22223.
Change-Id: I46dc7c54b1347143d02d6b9635038230cda6d164
Reviewed-on: https://go-review.googlesource.com/80760
Reviewed-by: Russ Cox <rsc@golang.org>
It's not safe (it crashes), and it's also useless: if you run
multiple benchmarks in parallel you will not get reliable
timing results from any of them.
Fixes#18603.
Change-Id: I00e5a72f7c98151543cf7d5573c38383276e391a
Reviewed-on: https://go-review.googlesource.com/80841
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The build tags already prevent the tests from being run on windows or
plan9, so there is no need to check GOOS again.
Change-Id: I74d3c3b7756d9c50f6e5fd4c3e8b0db618fdebbb
Reviewed-on: https://go-review.googlesource.com/81295
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Otherwise we may delay the delivery of these signals for an arbitrary
length of time. We are already careful to not block signals that the
program has asked to see.
Also make sure that we don't miss a signal delivery if a thread
decides to stop for a while while executing the signal handler.
Also clean up the TestAtomicStop output a little bit.
Fixes#21433
Change-Id: Ic0c1a4eaf7eba80d1abc1e9537570bf4687c2434
Reviewed-on: https://go-review.googlesource.com/79581
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Dim performance has regressed by 14% vs 1.9 on amd64.
Current pure go version of Dim is faster and,
what is even more important for performance, is inlinable, so
instead of tweaking asm implementation, just remove it.
I had to update BenchmarkDim, because it was simply reloading
constant(answer) in a loop.
Perf data below:
name old time/op new time/op delta
Dim-6 6.79ns ± 0% 1.60ns ± 1% -76.39% (p=0.000 n=7+10)
If I modify benchmark to be the same as in this CL results are even better:
name old time/op new time/op delta
Dim-6 10.2ns ± 0% 1.6ns ± 1% -84.27% (p=0.000 n=8+10)
Updates #21913
Change-Id: I00e23c8affc293531e1d9f0e0e49f3a525634f53
Reviewed-on: https://go-review.googlesource.com/80695
Run-TryBot: Ilya Tocar <ilya.tocar@intel.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
In nat.divLarge (having signature (z nat).divLarge(u, uIn, v nat)),
we check whether z aliases uIn or v, but aliasing is currently not
checked for the u parameter.
Unfortunately, z and u aliasing each other can in some cases cause
errors in the computation.
The q return parameter (which will hold the result's quotient), is
unconditionally initialized as
q = z.make(m + 1)
When cap(z) ≥ m+1, z.make() will reuse z's backing array, causing q
and z to share the same backing array. If then z aliases u, setting q
during the quotient computation will then corrupt u, which at that
point already holds computation state.
To fix this, we add an alias(z, u) check at the beginning of the
function, taking care of aliasing the same way we already do for uIn
and v.
Fixes#22830
Change-Id: I3ab81120d5af6db7772a062bb1dfc011de91f7ad
Reviewed-on: https://go-review.googlesource.com/78995
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
The trace command computes IO, Schedule, Block, and Syscall profiles
by following the unblocking links in the execution trace and summing
up the duration. This change offers variations of those profiles
that include only selected goroutine types. The id parameter takes the
goroutine type - i.e. pc of the goroutine.
The output is available from the /goroutine view. So, users can see
where the goroutines of interest typically block.
Also, these profiles are available for download so users can use
pprof or other tools to interpret the output. This change adds links
for download of global profile in the main page.
Change-Id: I35699252056d164e60de282b0406caf96d629c85
Reviewed-on: https://go-review.googlesource.com/75710
Reviewed-by: Sameer Ajmani <sameer@golang.org>
I've been doing these tweaks by hand. I was going to write a tool in
Go for it, but it's not much additional shell here.
Fixes#22912
Updates #9797 (already closed)
Change-Id: Ia15bd9b6876e6f6a76aa9ca86b10f113095e96a3
Reviewed-on: https://go-review.googlesource.com/80895
Reviewed-by: Ian Lance Taylor <iant@golang.org>
GOMIPS is a GOARCH=mips{,le} specific option, for a choice between
hard-float and soft-float. Valid values are 'hardfloat' (default) and
'softfloat'. It is passed to the assembler as
'GOMIPS_{hardfloat,softfloat}'.
Note: GOMIPS will later also be used for a choice of MIPS instruction
set (mips32/mips32r2).
Updates #18162
Change-Id: I35417db8625695f09d6ccc3042431dd2eaa756a6
Reviewed-on: https://go-review.googlesource.com/37954
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Fix a typo of "packageshlib" used for generating the link action ID.
Change-Id: Id6d39830908b03de658a58661030c32c592a1da9
Reviewed-on: https://go-review.googlesource.com/80935
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alberto Donizetti <alb.donizetti@gmail.com>
Compiler and linker changes to support DWARF inlined instances,
see https://go.googlesource.com/proposal/+/HEAD/design/22080-dwarf-inlining.md
for design details.
This functionality is gated via the cmd/compile option -gendwarfinl=N,
where N={0,1,2}, where a value of 0 disables dwarf inline generation,
a value of 1 turns on dwarf generation without tracking of formal/local
vars from inlined routines, and a value of 2 enables inlines with
variable tracking.
Updates #22080
Change-Id: I69309b3b815d9fed04aebddc0b8d33d0dbbfad6e
Reviewed-on: https://go-review.googlesource.com/75550
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
ORANGE node's Right node is the expression it is ranging over,
which is evaluated before the loop. In the escape analysis,
we should walk this node without loop depth incremented.
Fixes#21709.
Change-Id: Idc1e4c76e39afb5a344d85f6b497930a488ce5cf
Reviewed-on: https://go-review.googlesource.com/80740
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
This reverts commit a631daba5f.
Reason for revert: I was wrong. It still fails on the builders.
I don't know what's different about my VMWare VM, but on GCE it fails.
Change-Id: Ic6bee494b69235768bf08ba0bf59026bca41ad12
Reviewed-on: https://go-review.googlesource.com/80915
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
It must've been fixed since NetBSD 7.0. I can no longer reproduce it
with NetBSD 8-BETA (our new minimum NetBSD requirement).
Fixes#19293
Change-Id: I28f970ca41a53a037e1c6cddf1b7f286bda2d725
Reviewed-on: https://go-review.googlesource.com/80875
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The fix (CL 79575) for #18395 is too risky at this stage of the Go 1.10
release process.
Since issue #18395 is easily recognized (but not easily fixed), report
an error instead of silently continuing. This avoids inscrutable follow
on errors.
Also, make sure all empty interfaces are "completed", and adjust
printing code to report incomplete interfaces.
For #18395.
Change-Id: I7fa5f97ff31ac9775c9a6d318fce9f526b0350cd
Reviewed-on: https://go-review.googlesource.com/80455
Reviewed-by: Alan Donovan <adonovan@google.com>
In the x/arch repo, CL 45098 introduced SymLookup type, replacing
the unnamed function type for lookup functions. This affects the
signature of x86asm.GoSyntax. In particular, it cannot convert
one named type, namely lookupFunc, to the other without an
explicit cast. Make lookupFunc unnamed to fix.
Change-Id: I973300d29ef1dbfdbd7fc2429e89c5849e6a7329
Reviewed-on: https://go-review.googlesource.com/80842
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
For "type T = U" we were accidentally emitting a #define for "U__size"
instead of "T__size".
Fixes#22877.
Change-Id: I5ed6757d697753ed6d944077c16150759f6e1285
Reviewed-on: https://go-review.googlesource.com/80759
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The note about the single change workflow is included in the
git-codereview installation instructions, but it has nothing to do with
installing git-codereview. This note is more relevant for when a change
is actually being made.
Change-Id: Iccb90f3b7da87fab863fa4808438cd69a21a2fce
Reviewed-on: https://go-review.googlesource.com/76317
Reviewed-by: Steve Francia <spf@golang.org>
