Reduce the scope of some. Also remove vars that were simply the index or
the value in a range statement. While at it, remove a var that was
exactly the length of a slice.
Also replaced 'bad' with a more clear 'errored' of type bool, and
renamed a single-char name with a comment to a name that is
self-explanatory.
And removed a few unnecessary Index calls within loops.
Passes toolstash -cmp on std cmd.
Change-Id: I26eee5f04e8f7e5418e43e25dca34f89cca5c80a
Reviewed-on: https://go-review.googlesource.com/70930
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The complainant is confused by the ambiguity of 'next' in the
phrase 'next operand'. It seems clear enough to me that things
are always read left to right when formatting, but to calm the
waters we add a clarifying parenthetical.
Fixes#22275
Change-Id: I82418c1e987db736f4bee0faa53fe715c9cde8f5
Reviewed-on: https://go-review.googlesource.com/71010
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
ANSI X9.62 specifies that Unmarshal should fail if the a given coordinate is
not smaller than the prime of the elliptic curve. This change makes Unmarshal
ANSI X9.62 compliant and explicitly documents that the Marshal/Unmarshal only
supports uncompressed points.
Fixes#20482
Change-Id: I161a73da8279cae505c9ba0b3022021709fe8145
Reviewed-on: https://go-review.googlesource.com/44312
Reviewed-by: Adam Langley <agl@golang.org>
Reviewed-by: Filippo Valsorda <hi@filippo.io>
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The indexed bitmap of a frame is encoded into a GIF by first LZW
compression, and then packaged by a simple block mechanism. Each block
of up-to-256 bytes starts with one byte, which indicates the size of the
block (0x01-0xff). The sequence of blocks is terminated by a 0x00.
While the format supports it, there is no good reason why any particular
image should be anything but a sequence of 255-byte blocks with one last
block less than 255-bytes.
The old blockWriter implementation would not buffer between Write()s,
meaning if the lzw Writer needs to flush more than one chunk of data via
a Write, multiple short blocks might exist in the middle of a stream.
Separate but related, the old implementation also forces lzw.NewWriter
to allocate a bufio.Writer because the blockWriter is not an
io.ByteWriter itself. But, even though it doesn't effectively buffer
data between Writes, it does make extra copies of sub-blocks during the
course of writing them to the GIF's writer.
Now, the blockWriter shall continue to use the encoder's [256]byte buf,
but use it to effectively buffer a series of WriteByte calls from the
lzw Writer. Once a WriteByte fills the buffer, the staged block is
Write()n to the underlying GIF writer. After the lzw Writer is Closed,
the blockWriter should also be closed, which will flush any remaining
block along with the block terminator.
BenchmarkEncode indicates slight improvements:
name old time/op new time/op delta
Encode-8 7.71ms ± 0% 7.38ms ± 0% -4.27% (p=0.008 n=5+5)
name old speed new speed delta
Encode-8 159MB/s ± 0% 167MB/s ± 0% +4.46% (p=0.008 n=5+5)
name old alloc/op new alloc/op delta
Encode-8 84.1kB ± 0% 80.0kB ± 0% -4.94% (p=0.008 n=5+5)
name old allocs/op new allocs/op delta
Encode-8 9.00 ± 0% 7.00 ± 0% -22.22% (p=0.008 n=5+5)
Change-Id: I9eb9367d41d7c3d4d7f0adc9b720fc24fb50006a
Reviewed-on: https://go-review.googlesource.com/68351
Reviewed-by: Nigel Tao <nigeltao@golang.org>
After we detect errors, the AST is in a precarious state and more
likely to trip useless ICE failures. Instead let the user fix any
existing errors and see if the ICE persists. This makes Fatalf more
consistent with how panics are handled by hidePanic.
While here, also fix detection for release versions: release version
strings begin with "go" ("go1.8", "go1.9.1", etc), not "release".
Fixes#22252.
Change-Id: I1c400af62fb49dd979b96e1bf0fb295a81c8b336
Reviewed-on: https://go-review.googlesource.com/70850
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
The caller's PC is always available in the frame. We can just
load it when needed, no need to spill.
Change-Id: I9c0a525903e574bb4eec9fe53cbeb8c64321166a
Reviewed-on: https://go-review.googlesource.com/70710
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Consolidate the signature and hash fields (SignatureAndHashAlgorithm in
TLS 1.2) into a single uint16 (SignatureScheme in TLS 1.3 draft 21).
This makes it easier to add RSASSA-PSS for TLS 1.2 in the future.
Fields were named like "signatureAlgorithm" rather than
"signatureScheme" since that name is also used throughout the 1.3 draft.
The only new public symbol is ECDSAWithSHA1, other than that this is an
internal change with no new functionality.
Change-Id: Iba63d262ab1af895420583ac9e302d9705a7e0f0
Reviewed-on: https://go-review.googlesource.com/62210
Reviewed-by: Adam Langley <agl@golang.org>
Currently only a single P can run a fractional mark worker at a time.
This doesn't let us spread out the load, so it gets concentrated on
whatever unlucky P picks up the token to run a fractional worker. This
can significantly delay goroutines on that P.
This commit changes this scheduling rule so each P separately
schedules fractional workers. This can significantly reduce the load
on any individual P and allows workers to self-preempt earlier. It
does have the downside that it's possible for all Ps to be in
fractional workers simultaneously (an effect STW).
Updates #21698.
Change-Id: Ia1e300c422043fa62bb4e3dd23c6232d81e4419c
Reviewed-on: https://go-review.googlesource.com/68574
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Currently fractional workers run until preempted by the scheduler,
which means they typically run for 20ms. During this time, all other
goroutines on that P are blocked, which can introduce significant
latency variance.
This modifies fractional workers to self-preempt shortly after
achieving the fractional utilization goal. In practice this means they
preempt much sooner, and the scale of their preemption is on the order
of how often the user goroutine block (so, if the application is
compute-bound, the fractional workers will also run for long times,
but if the application blocks frequently, the fractional workers will
also preempt quickly).
Fixes#21698.
Updates #18534.
Change-Id: I03a5ab195dae93154a46c32083c4bb52415d2017
Reviewed-on: https://go-review.googlesource.com/68573
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
We haven't used non-zero gcForcePreemptNS for ages. Remove it and
declutter the code.
Change-Id: Id5cc62f526d21ca394d2b6ca17d34a72959535da
Reviewed-on: https://go-review.googlesource.com/68572
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
When GOMAXPROCS is not small, fractional workers don't add much to
throughput, but they do add to the latency of individual goroutines.
In this case, it makes sense to just use dedicated workers, even if we
can't exactly hit the 25% CPU goal with dedicated workers.
This implements this logic by computing the number of dedicated mark
workers that will us closest to the 25% target. We only fall back to
fractional workers if that would be more than 30% off of the target
(less than 17.5% or more than 32.5%, which in practice happens for
GOMAXPROCS <= 3 and GOMAXPROCS == 6).
Updates #21698.
Change-Id: I484063adeeaa1190200e4ef210193a20e635d552
Reviewed-on: https://go-review.googlesource.com/68571
Reviewed-by: Rick Hudson <rlh@golang.org>
Currently these are the same constant, but are separate concepts.
Split them into two constants for easier experimentation and better
documentation.
Change-Id: I121854d4fd1a4a827f727c8e5153160c24aacda7
Reviewed-on: https://go-review.googlesource.com/68570
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
https://golang.org/cl/67270 wasn't `go fmt`ed correctly, according to
the current `go fmt`. However, what `go fmt` did looked odd, so this
change tweaks the test to use a more standard layout.
Whitespace-only; no semantic change.
Change-Id: Id820352e7c9e68189ee485c8a9bfece75ca4f9cb
Reviewed-on: https://go-review.googlesource.com/69031
Run-TryBot: Adam Langley <agl@golang.org>
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: Adam Langley <agl@golang.org>
Database drivers should be called from a single goroutine to ease
driver's design. If a driver chooses to handle context
cancels internally it may do so.
The sql package violated this agreement when calling Next or
NextResultSet. It was possible for a concurrent rollback
triggered from a context cancel to call a Tx.Rollback (which
takes a driver connection lock) while a Rows.Next is in progress
(which does not tack the driver connection lock).
The current internal design of the sql package is each call takes
roughly two locks: a closemu lock which prevents an disposing of
internal resources (assigning nil or removing from lists)
and a driver connection lock that prevents calling driver code from
multiple goroutines.
Fixes#21117
Change-Id: Ie340dc752a503089c27f57ffd43e191534829360
Reviewed-on: https://go-review.googlesource.com/65731
Reviewed-by: Ian Lance Taylor <iant@golang.org>
CL 69370 introduced a hasmain field to moduledata after the
modulehashes slice. However that code was relying on the zeroing
code after it to cover modulehashes if len(Shlibs) == 0. The
hasmain field gets in the way of that. So clear modulehashes
explicitly in that case.
Found when looking at #22250. Not sure if it's related.
Change-Id: I81050cb4554cd49e9f245d261ef422f97d026df4
Reviewed-on: https://go-review.googlesource.com/70730
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
In https://golang.org/cl/66334, the test was changed so that the second
Listen would also be closed. However, it shouldn't have reused the same
ln variable, as that can lead to a data race with the background loop
that accepts connections.
Simply define a new Listener, since we don't need to overwrite the first
variable.
I was able to reproduce the data race report locally about 10% of the
time by reducing the sleep from a millisecond to a nanosecond. After the
fix, it's entirely gone after 1000 runs.
Fixes#22226.
Change-Id: I7c639f9f2ee5098eac951a45f42f97758654eacd
Reviewed-on: https://go-review.googlesource.com/70230
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This change adds support for accelerating time.Now by using
the __vdso_clock_gettime fast-path via the vDSO on linux/386
if it is available.
When the vDSO path to the clocks is available, it is typically
5x-10x faster than the syscall path (see benchmark extract
below). Two such calls are made for each time.Now() call
on most platforms as of go 1.9.
- Add vdso_linux_386.go, containing the ELF32 definitions
for use by vdso_linux.go, the maximum array size, and
the symbols to be located in the vDSO.
- Modify runtime.walltime and runtime.nanotime to check for
and use the vDSO fast-path if available, or fall back to
the existing syscall path.
- Reduce the stack reservations for runtime.walltime and
runtime.monotime from 32 to 16 bytes. It appears the syscall
path actually only needed 8 bytes, but 16 is now needed to
cover the syscall and vDSO paths.
- Remove clearing DX from the syscall paths as clock_gettime
only takes 2 args (BX, CX in syscall calling convention),
so there should be no need to clear DX.
The included BenchmarkTimeNow was run with -cpu=1 -count=20
on an "Intel(R) Celeron(R) CPU J1900 @ 1.99GHz", comparing
released go 1.9.1 vs this change. This shows a gain in
performance on linux/386 (6.89x), and that no regression
occurred on linux/amd64 due to this change.
Kernel: linux/i686, GOOS=linux GOARCH=386
name old time/op new time/op delta
TimeNow 978ns ± 0% 142ns ± 0% -85.48% (p=0.000 n=16+20)
Kernel: linux/x86_64, GOOS=linux GOARCH=amd64
name old time/op new time/op delta
TimeNow 125ns ± 0% 125ns ± 0% ~ (all equal)
Gains are more dramatic in virtualized environments,
presumably due to the overhead of virtualizing the syscall.
Fixes#22190
Change-Id: I2f83ce60cb1b8b310c9ced0706bb463c1b3aedf8
Reviewed-on: https://go-review.googlesource.com/69390
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
syscall.Select uses SYS_PSELECT6 on arm64 and mipx64x, however this
syscall expects its 5th argument to be of type Timespec (with seconds
and nanoseconds) instead of type Timeval (with seconds and microseconds)
This leads to the timeout being too short by a factor of 1000.
This CL fixes this by adjusting the timeout argument accordingly,
similarly to how glibc does it for architectures where neither
SYS_SELECT nor SYS__NEWSELECT are available.
Fixes#22246
Change-Id: I33a183b0b87c2dae4a77a2d00f8615169fad48dd
Reviewed-on: https://go-review.googlesource.com/70590
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Some ARM64-specific instructions (such as SIMD instructions) are not supported.
This patch adds support for the following:
1. Extended register, e.g.:
ADD Rm.<ext>[<<amount], Rn, Rd
<ext> can have the following values:
UXTB, UXTH, UXTW, UXTX, SXTB, SXTH, SXTW and SXTX
2. Arrangement for SIMD instructions, e.g.:
VADDP Vm.<T>, Vn.<T>, Vd.<T>
<T> can have the following values:
B8, B16, H4, H8, S2, S4 and D2
3. Width specifier and element index for SIMD instructions, e.g.:
VMOV Vn.<T>[index], Rd // MOV(to general register)
<T> can have the following values:
S and D
4. Register List, e.g.:
VLD1 (Rn), [Vt1.<T>, Vt2.<T>, Vt3.<T>]
5. Register offset variant, e.g.:
VLD1.P (Rn)(Rm), [Vt1.<T>, Vt2.<T>] // Rm is the post-index register
6. Go assembly for ARM64 reference manual
new added instructions are required to have according explanation items in
the manual and items for existed instructions will be added incrementally
For more information about the refinement background, please refer to the
discussion (https://groups.google.com/forum/#!topic/golang-dev/rWgDxCrL4GU)
This patch only adds syntax and doesn't break any assembly that already exists.
Change-Id: I34e90b7faae032820593a0e417022c354a882008
Reviewed-on: https://go-review.googlesource.com/41654
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The GIF format allows for an image to contain a global color table which
might be used for some or every frame in an animated GIF. This palette
contains 24-bit opaque RGB values. An individual frame may use the
global palette and enable transparency by picking one number to be
transparent, instead of the color value in the palette.
image/gif decodes a GIF, which contains an []*image.Paletted that holds
each frame. When decoded, if a frame has a transparent color and uses
the global palette, a copy of the global []color.Color is made, and the
transparency color index is replaced with color.RGBA{}.
When encoding a GIF, each frame's palette is encoded to the form it
might exist in a GIF, up to 768 bytes "RGBRGBRGBRGB...". If a frame's
encoded palette is equal to the encoded global color table, the frame
will be encoded with the flag set to use the global color table,
otherwise the frame's palette will be included.
So, if the color in the global color table that matches the transparent
index of one frame wasn't black (and it frequently is not), reencoding a
GIF will likely result in a larger file because each frame's palette
will have to be encoded inline.
This commit takes a frame's transparent color index into account when
comparing an individual image.Paletted's encoded color table to the
global color table.
Fixes#22137
Change-Id: I5460021da6e4d7ce19198d5f94a8ce714815bc08
Reviewed-on: https://go-review.googlesource.com/68313
Reviewed-by: Nigel Tao <nigeltao@golang.org>
Excluded when -short because it still runs relatively long,
but deflaked.
Removed timeouts from normal path and ensured that they were
not needed and that reference files did not change.
Use "tbreak" instead of "break" with gdb to reduce chance
of multiple hits on main.main. (Seems not enough, but a
move in the right direction).
By default, testing ignores repeated lines that occur when
nexting. This appears to sometimes be timing-dependent and
is the observed source of flakiness in testing so far.
Note that these can also be signs of a bug in the generated
debugging output, but it is one of the less-confusing bugs
that can occur.
By default, testing with gdb uses compilation with
inlining disabled to prevent dependence on library code
(it's a bug that library code is seen while Nexting, but
the bug is current behavior).
Also by default exclude all source files outside /testdata
to prevent accidental dependence on library code. Note that
this is currently only applicable to dlv because (for the
debugging information we produce) gdb does not indicate a
change in the source file for inlined code.
Added flags -i and -r to make gdb testing compile with
inlining and be sensitive to repeats in the next stream.
This is for developer-testing and so we can describe these
problems in bug reports.
Updates #22206.
Change-Id: I9a30ebbc65aa0153fe77b1858cf19743bdc985e4
Reviewed-on: https://go-review.googlesource.com/69930
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This somewhat mirrors the special case behavior of the copy built-in.
Fixes#22215
Change-Id: Ic353003ad3de659d3a6b4e9d97295b42510f3bf7
Reviewed-on: https://go-review.googlesource.com/70431
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This is a preparation step for adding vDSO support on linux/386.
This change relocates the elf64 and amd64 specifics from
vdso_linux.go to a new vdso_linux_amd64.go.
This should enable vdso_linux.go to be used for vDSO
support on linux architectures other than amd64.
- Relocate the elf64X structure definitions appropriate to amd64,
and change their names to elfX so that the code in vdso_linux.go
is ELFnn-agnostic.
- Relocate the sym_keys and corresponding __vdso_* variables
appropriate to amd64.
- Provide an amd64-specific constant for the maximum byte size of
an array, and use this in vdso_linux.go to compute constants for
sizing the elf structure arrays traversed in the loaded vDSO.
Change-Id: I1edb4e4ec9f2d79b7533aa95fbd09f771fa4edef
Reviewed-on: https://go-review.googlesource.com/69391
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently we look to see if the main.main symbol address is in the
module data text range. This requires access to the main.main
symbol, which usually the runtime has, but does not when building
a plugin.
To avoid a dynamic relocation to main.main (which I haven't worked
out how to have the linker generate on darwin), stop using the
symbol. Instead record a boolean in the moduledata if the module
has the main function.
Fixes#22175
Change-Id: If313a118f17ab499d0a760bbc2519771ed654530
Reviewed-on: https://go-review.googlesource.com/69370
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
For #22095
Change-Id: I8f48fce571b69a7e8edf2ad7733ffdfd38676e63
Reviewed-on: https://go-review.googlesource.com/70310
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Follow CL 41477 and add two more line position tests for yyerror calls
in the typechecker which are currently not tested.
Update #19683
Change-Id: Iacd865195a3bfba87d8c22655382af267aba47a9
Reviewed-on: https://go-review.googlesource.com/70251
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
These functions are identical to math/bits.TrailingZeros{32,64}, which
are already intrinsified on ppc64.
Change-Id: If7ee57e7afe53154874f4b66bacdb6237806128a
Reviewed-on: https://go-review.googlesource.com/70350
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
When we split separate packages into separate compilation units, we
lost PC range information because it was no longer contiguous. This
brings it back by constructing proper per-package PC range tables.
Change-Id: Id0ab5187e08ac5d13b3d3794977bfc857a56224f
Reviewed-on: https://go-review.googlesource.com/69974
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Currently, the linker generates one huge DWARF compilation unit for
the entire Go binary. This commit creates a separate compilation unit
and line table per Go package.
We temporarily lose compilation unit PC range information, since it's
now discontiguous, so harder to emit. We'll bring it back in the next
commit.
Beyond being "more traditional", this has various technical
advantages:
* It should speed up line table lookup, since that requires a
sequential scan of the line table. With this change, a debugger can
first locate the per-package line table and then scan only that line
table.
* Once we emit compilation unit PC ranges again, this should also
speed up various other debugger reverse PC lookups.
* It puts us in a good position to move more DWARF generation into the
compiler, which could produce at least the CU header, per-function
line table fragments, and per-function frame unwinding info that the
linker could simply paste together.
* It will let us record a per-package compiler command-line flags
(#22168).
Change-Id: Ibac642890984636b3ef1d4b37fe97f4453c2cc84
Reviewed-on: https://go-review.googlesource.com/69973
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The DWARF code currently clears all section relocations every time it
creates a section. This is unnecessary and confusing, so don't do it.
This dates back to
https://codereview.appspot.com/7891044/diff/26001/src/cmd/ld/dwarf.c.
At the time, this was only done for one symbol and that symbol was
used solely for collecting relocations (which is why it made sense to
clear the relocations but not the actual data). Furthermore, DWARF
generation potentially required two passes, so it was important to
clear the state from the first pass. None of this is true now, but
this pattern had been cargo-culted all over the dwarf.go.
Change-Id: I87d4ff8ccd5c807796241559be46168ce3ccb49a
Reviewed-on: https://go-review.googlesource.com/70312
Run-TryBot: Austin Clements <austin@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The .debug_aranges section is an odd vestige of DWARF, since its
contents are easy and efficient for a debugger to reconstruct from the
attributes of the top-level compilation unit DIEs. Neither GCC nor
clang emit it by default these days. GDB and Delve ignore it entirely.
LLDB will use it if present, but is happy to construct the index from
the compilation unit attributes (and, indeed, a remarkable variety of
other ways if those aren't available either).
We're about to split up the compilation units by package, which means
they'll have discontiguous PC ranges, which is going to make
.debug_aranges harder to construct (and larger).
Rather than try to maintain this essentially unused code, let's
simplify things and remove it.
Change-Id: I8e0ccc033b583b5b8908cbb2c879b2f2d5f9a50b
Reviewed-on: https://go-review.googlesource.com/69972
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
During Mach-O linking, dsymutil takes the DWARF from individual object
files and combines it into a debug archive. Because it's content-aware,
it doesn't need our help to do its job. Nonetheless, it does try to
honor relocations that are present in its input.
When dsymutil encounters a relocation, it uses the value of that
relocation as an index into the debug map to find its final location.
When it does that, it's assuming that the value is an address in the
object file. But DWARF references are section-relative. So when it
processes a relocation for a DWARF reference, it gets confused,
and if the value happens to match the address of a function or
data symbol, it will rewrite it incorrectly.
Since the relocations don't help, and can hurt, drop them when
externally linking a Mach-O binary.
Fixes#22068
Change-Id: I8ec36da626575d9f6c8d0e7a0b76eab8ba22d62c
Reviewed-on: https://go-review.googlesource.com/68330
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
The alternative signal stack doesn't work on ios, so the setup of
the alternative stack was skipped. The corresponding unminitSignals
was effectively a no-op on ios until CL 70130. Skip unminitSignals
on ios to restore the previous behaviour.
For the ios builders.
Change-Id: I5692ca7f5997e6b9d10cc5f2383a5a37c42b133c
Reviewed-on: https://go-review.googlesource.com/70270
Run-TryBot: Elias Naur <elias.naur@gmail.com>
Reviewed-by: Austin Clements <austin@google.com>
The two are not meaningfully different, and it is confusing to have two.
Change-Id: Ie6a355ea4d79fb4bb79bf5124071a866038b19ba
Reviewed-on: https://go-review.googlesource.com/70211
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
CL 36799 made SetFileCompletionNotificationModes to be called for
file handles. I don't think it is correct. Revert that change.
Fixes#22024Fixes#22207
Change-Id: I26260e8a727131cffbf60958d79eca2457495554
Reviewed-on: https://go-review.googlesource.com/69871
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Current code assumes that SetFileCompletionNotificationModes
is safe to call even if we know that it is not safe to use
FILE_SKIP_COMPLETION_PORT_ON_SUCCESS flag. It appears (see issue #22149),
SetFileCompletionNotificationModes crashes when we call it without
FILE_SKIP_COMPLETION_PORT_ON_SUCCESS flag.
Do not call SetFileCompletionNotificationModes in that situation.
We are allowed to do that, because SetFileCompletionNotificationModes
is just an optimisation.
Fixes#22149
Change-Id: I0ad3aff4eabd8c27739417a62c286b1819ae166a
Reviewed-on: https://go-review.googlesource.com/69870
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Decode decodes entire GIF image and returns the first frame as an
image.Image. There's no need for it to keep every decoded frame in
memory except for the one it returns.
Fixes#22199
Change-Id: I76b4bd31608ebc76a1a3df02e85c20eb80df7877
Reviewed-on: https://go-review.googlesource.com/69890
Reviewed-by: Nigel Tao <nigeltao@golang.org>
CL 69292 unified the amd64 entry-points, but Dragonfly doesn't follow
the same entry-point argument conventions as most other amd64
platforms. Fix the Dragonfly entry point.
Change-Id: I0f84e2e4101ce68217af185ee9baaf455b8b6dad
Reviewed-on: https://go-review.googlesource.com/70212
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Fixes#21054
Change-Id: I016486dc62c04a80727f8da7d1dcec52f2c7f344
Reviewed-on: https://go-review.googlesource.com/62291
Reviewed-by: Giovanni Bajo <rasky@develer.com>
Reviewed-by: Chris Broadfoot <cbro@golang.org>
Since CL 46037, the runtime is crashing after calling
exitThread on Plan 9.
The exitThread function shouldn't be called on
Plan 9, because the system manages thread stacks.
Fixes#22221.
Change-Id: I5d61c9660a87dc27e4cfcb3ca3ddcb4b752f2397
Reviewed-on: https://go-review.googlesource.com/70190
Run-TryBot: David du Colombier <0intro@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Previously, we were treating cross-package function calls as free for
inlining budgeting.
In theory, we should be able to recompute InlCost from the
exported/reimported function bodies. However, that process mutates the
structure of the Node AST enough that it doesn't preserve InlCost. To
avoid unexpected issues, just record and restore InlCost in the export
data.
Fixes#19261.
Change-Id: Iac2bc0d32d4f948b64524aca657051f9fc96d92d
Reviewed-on: https://go-review.googlesource.com/70151
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
Noticed while reading some code that the two branches in this loop body
shared the last statements. Rewrite it in a way that they are not
duplicated.
Passes toolstash -cmp on std.
Change-Id: I3356ca9fa37c32eee496e221d7830bfc581dade1
Reviewed-on: https://go-review.googlesource.com/66470
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Calls to a closure held in a local, non-escaping,
variable can be inlined, provided the closure body
can be inlined and the variable is never written to.
The current implementation has the following limitations:
- closures with captured variables are not inlined because
doing so naively triggers invariant violation in the SSA
phase
- re-assignment check is currently approximated by checking
the Addrtaken property of the variable which should be safe
but may miss optimization opportunities if the address is
not used for a write before the invocation
Updates #15561
Change-Id: I508cad5d28f027bd7e933b1f793c14dcfef8b5a1
Reviewed-on: https://go-review.googlesource.com/65071
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Hugues Bruant <hugues.bruant@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
Android's libc creates a signal stack for every thread it creates. In
Go, minitSignalStack picks up this existing signal stack and puts it
in m.gsignal.stack. However, if we later try to exit a thread (because
a locked goroutine is exiting), we'll attempt to stackfree this
libc-allocated signal stack and panic.
Fix this by clearing gsignal.stack when we unminitSignals in such a
situation.
This should fix the Android build, which is currently broken.
Change-Id: Ieea8d72ef063d22741c54c9daddd8bb84926a488
Reviewed-on: https://go-review.googlesource.com/70130
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In #10909, it was decided that "Deprecated:" is a magic string for
tools (e.g., #17056 for godoc) to detect deprecated identifiers.
Use those convention instead of custom written prose.
Change-Id: Ia514fc3c88fc502e86c6e3de361c435f4cb80b22
Reviewed-on: https://go-review.googlesource.com/70110
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
In order to improve the line numbering for debuggers,
it's necessary to trace lines through compilation.
This makes it (much) easier to follow.
The format of the last column of the ssa.html output was
also changed to reduce the spamminess of the file name,
which is usually the same and makes it far harder to read
instructions and line numbers, and to make it wider and also
able to break words when wrapping (long path names still
can push off the end otherwise; side-to-side scrolling was
tried but was more annoying than the occasional wrapped
line).
Sample output now, where [...] is elision for sake of making
the CL character-counter happy -- and the (##) line numbers
are rendered in italics and a smaller font (11 point) under
control of a CSS class "line-number".
genssa
# /Users/drchase/[...]/ssa/testdata/hist.go
00000 (35) TEXT "".main(SB)
00001 (35) FUNCDATA $0, gclocals·7be4bb[...]1e8b(SB)
00002 (35) FUNCDATA $1, gclocals·9ab98a[...]4568(SB)
v920 00003 (36) LEAQ ""..autotmp_31-640(SP), DI
v858 00004 (36) XORPS X0, X0
v6 00005 (36) LEAQ -48(DI), DI
v6 00006 (36) DUFFZERO $277
v576 00007 (36) LEAQ ""..autotmp_31-640(SP), AX
v10 00008 (36) TESTB AX, (AX)
b1 00009 (36) JMP 10
and from an earlier phase:
b18: ← b17
v242 (47) = Copy <mem> v238
v243 (47) = VarKill <mem> {.autotmp_16} v242
v244 (48) = Addr <**bufio.Scanner> {scanner} v2
v245 (48) = Load <*bufio.Scanner> v244 v243
[...]
v279 (49) = Store <mem> {int64} v277 v276 v278
v280 (49) = Addr <*error> {.autotmp_18} v2
v281 (49) = Load <error> v280 v279
v282 (49) = Addr <*error> {err} v2
v283 (49) = VarDef <mem> {err} v279
v284 (49) = Store <mem> {error} v282 v281 v283
v285 (47) = VarKill <mem> {.autotmp_18} v284
v286 (47) = VarKill <mem> {.autotmp_17} v285
v287 (50) = Addr <*error> {err} v2
v288 (50) = Load <error> v287 v286
v289 (50) = NeqInter <bool> v288 v51
If v289 → b21 b22 (line 50)
Change-Id: I3f46310918f965761f59e6f03ea53067237c28a8
Reviewed-on: https://go-review.googlesource.com/69591
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
