Background: when compiling a function, it's possible that a local
variable will be optimized away, which could potentially degrade the
debugging experience if the compiler fails to emit DWARF information
for the variable's type. To mitigate this situation, the compiler
emits R_USETYPE relocations for the function's auto/param variables as
a signal to the linker to generate DWARF for the types in question,
even if the type is not specifically attached to a DWARF param or var.
This patch change the logic in the compiler to avoid emitting a
R_USETYPE relocation if the type in question is already referenced by
a concrete DWARF param or auto record. This cuts down on the amount of
work the linker has to do, also makes object files a bit smaller on
average (about 1% for the runtime package).
Than McIntosh [Wed, 20 May 2020 19:06:22 +0000 (15:06 -0400)]
[dev.link] cmd/link: refactor DWARF fn die processing to remove name lookups
Rework the code in the linker that visits DWARF subprorgam DIEs to
reduce number of symbol name instantiations and name lookups, by
making better use of relocation target symbol types.
Change-Id: Ifb2a4e24874b8c891d7fdf17dd749c3f9139157a
Reviewed-on: https://go-review.googlesource.com/c/go/+/234685
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jeremy Faller <jeremy@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>
Than McIntosh [Wed, 20 May 2020 17:51:59 +0000 (13:51 -0400)]
[dev.link] cmd/{compile,link}: split SDWARFINFO symtype into sub-types
This change splits the SDWARFINFO symbol type (a generic container of
DWARF content) into separate sub-classes. The new symbol types are
SDWARFCUINFO comp unit DIE, also CU info and CU packagename syms
SDWARFCONST constant DIE
SDWARFFCN subprogram DIE (default and concrete)
SDWARFABSFCN abstract function DIE
SDWARFTYPE type DIE
SDWARFVAR global variable DIE
Advantage of doing this: in the linker there are several places where
we have to iterate over a symbol's relocations to pick out references
to specific classes of DWARF sub-symbols (for example, looking for all
abstract function DIEs referenced by a subprogram DIE, or looking at
all the type DIEs used in a subprogram DIE). By splitting SDWARFINFO
into parts clients can now look only at the relocation target's sym
type as opposed to having to materialize the target sym name, or do a
lookup.
Michael Anthony Knyszek [Tue, 19 May 2020 16:33:17 +0000 (16:33 +0000)]
runtime: synchronize StartTrace and StopTrace with sysmon
Currently sysmon is not stopped when the world is stopped, which is
in general a difficult thing to do. The result of this is that when
tracing starts and the value of trace.enabled changes, it's possible
for sysmon to fail to emit an event when it really should. This leads to
traces which the execution trace parser deems inconsistent.
Fix this by putting all of sysmon's work behind a new lock sysmonlock.
StartTrace and StopTrace both acquire this lock after stopping the world
but before performing any work in order to ensure sysmon sees the
required state change in tracing. This change is expected to slow down
StartTrace and StopTrace, but will help ensure consistent traces are
generated.
Updates #29707.
Fixes #38794.
Change-Id: I64c58e7c3fd173cd5281ffc208d6db24ff6c0284
Reviewed-on: https://go-review.googlesource.com/c/go/+/234617
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Cherry Zhang [Mon, 18 May 2020 19:23:46 +0000 (15:23 -0400)]
cmd/link: fix size calculation for file space preallocation on darwin
On darwin, we preallocate file storage space with fcntl
F_ALLOCATEALL in F_PEOFPOSMODE mode. This is specified as
allocating from the physical end of the file. So the size we give
it should be the increment, instead of the total size.
Than McIntosh [Fri, 15 May 2020 16:19:07 +0000 (12:19 -0400)]
cmd/compile: delay inlinable method compilation for -c=1
When the concurrent back end is not enabled, it is possible to have a
scenario where: we compile a specific inlinable non-pointer-receiver
method T.M, then at some point later on in the compilation we visit a
type that triggers generation of a pointer-receiver wrapper (*T).M,
which then results in an inline of T.M into (*T).M. This introduces
subtle differences in the DWARF as compared with when the concurrent
back end is enabled (in the concurrent case, by the time we run the
SSA back end on T.M is is marked as being inlined, whereas in the
non-current case it is not marked inlined).
As a fix, at the point where we would normally compile a given
function in the xtop list right away, if the function is a method AND
is inlinable AND hasn't been inlined, then delay its compilation until
compileFunctions (so as to make sure that when we do compile it, all
possible inlining has been complete). In addition, make sure that
the abstract function symbol for the inlined function gets recorded
correctly.
Fixes #38068.
Change-Id: I57410ab5658bd4ee5b4b80750518e9b20fd6ba52
Reviewed-on: https://go-review.googlesource.com/c/go/+/234178
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com> Reviewed-by: Keith Randall <khr@golang.org>
Jay Conrod [Wed, 20 May 2020 21:54:55 +0000 (17:54 -0400)]
cmd/go: rank errUseProxy lower when handling proxy errors
modfetch.TryProxies ranks errors returned by GOPROXY entries by
usefulness. It returns the error of the highest rank from the last
proxy. Errors from "direct" and "noproxy" are most useful, followed by
errors other than ErrNotExist, followed by ErrNotExist.
This change ranks errUseProxy with ErrNotExist even though it's
reported by "noproxy". There is almost always a more useful message
than "path does not match GOPRIVATE/GONOPROXY".
Fixes #39180
Change-Id: Ifa5b96462d7bf411e6d2d951888465c839d42471
Reviewed-on: https://go-review.googlesource.com/c/go/+/234687
Run-TryBot: Jay Conrod <jayconrod@google.com> Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Cherry Zhang [Mon, 18 May 2020 18:14:11 +0000 (14:14 -0400)]
runtime: add a barrier after a new span is allocated
When copying a stack, we
1. allocate a new stack,
2. adjust pointers pointing to the old stack to pointing to the
new stack.
If the GC is running on another thread concurrently, on a machine
with weak memory model, the GC could observe the adjusted pointer
(e.g. through gp._defer which could be a special heap-to-stack
pointer), but not observe the publish of the new stack span. In
this case, the GC will see the adjusted pointer pointing to an
unallocated span, and throw. Fixing this by adding a publication
barrier between the allocation of the span and adjusting pointers.
One testcase for this is TestDeferHeapAndStack in long mode. It
fails reliably on linux-mips64le-mengzhuo builder without the fix,
and passes reliably after the fix.
Ian Lance Taylor [Wed, 20 May 2020 03:23:58 +0000 (20:23 -0700)]
syscall: preserve Windows file permissions for O_CREAT|O_TRUNC
On Windows, calling syscall.Open(file, O_CREAT|O_TRUNC, 0) for a file
that already exists would change the file to be read-only.
That is not how the Unix syscall.Open behaves, so avoid it on
Windows by calling CreateFile twice if necessary.
Fixes #38225
Change-Id: I70097fca8863df427cc8a97b9376a9ffc69c6318
Reviewed-on: https://go-review.googlesource.com/c/go/+/234534
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Yasuhiro Matsumoto [Thu, 14 May 2020 09:17:49 +0000 (18:17 +0900)]
cmd/go: use temporary file for output of gcc command on Windows
On Windows, some of gcc command (like msys2 native) output NUL as a file.
Fixes #36000
Change-Id: I02c632fa2d710a011d79f24d5beee4bc57ad994e
Reviewed-on: https://go-review.googlesource.com/c/go/+/233977
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
David Chase [Fri, 1 May 2020 21:30:33 +0000 (17:30 -0400)]
cmd/go: remove GOAMD64 environment variable
This removes the GOAMD64 environment variable and its documentation.
The value is instead supplied by a compiled-in constant.
Note that function alignment is also dependent on the value of
the (removed) flag; it is 32 for aligned jumps, 16 if not.
When the flag-dependent logic is removed, it will be 32.
Bryan C. Mills [Tue, 19 May 2020 05:29:11 +0000 (01:29 -0400)]
runtime: allocate fewer bytes during TestEINTR
This will hopefully address the occasional "runtime: out of memory"
failures observed on the openbsd-arm-jsing builder:
https://build.golang.org/log/c296d866e5d99ba401b18c1a2ff3e4d480e5238c
Also make the "spin" and "winch" loops concurrent instead of
sequential to cut down the test's running time.
Finally, change Block to coordinate by closing stdin instead of
sending SIGINT. The SIGINT handler wasn't necessarily registered by
the time the signal was sent.
Updates #20400
Updates #39043
Change-Id: Ie12fc75b87e33847dc25a12edb4126db27492da6
Reviewed-on: https://go-review.googlesource.com/c/go/+/234538
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Michael Anthony Knyszek [Mon, 18 May 2020 16:06:11 +0000 (16:06 +0000)]
runtime: don't use (addrRange).subtract in removeGreaterEqual
Currently in (*addrRanges).removeGreaterEqual we use
(addrRange).subtract with a range from specified address to "infinity"
which is supposed to be maxOffAddr. However, maxOffAddr is necessarily
an inclusive bound on the address space, because on many platforms an
exclusive bound would overflow back to 0.
On some platforms like mips and mipsle, the address space is smaller
than what's representable in a pointer, so if there's a range which hits
the top of the address space (such as in the pageAlloc tests), the limit
doesn't overflow, but maxOffAddr is inclusive, so any attempt to prune
this range with (*addrRange).removeGreaterEqual causes a failure, since
the range passed to subtract is contained within the address range which
touches the top of the address space.
Another problem with using subtract here is that addr and
maxOffAddr.addr() may not be in the same segment which could cause
makeAddrRange to panic. While this unlikely to happen, on some platforms
such as Solaris it is possible.
Fix these issues by not using subtract at all. Create a specific
implementation of (addrRange).removeGreaterEqual which side-steps all of
this by not having to worry about the top of the address space at all.
Michael Hudson-Doyle [Wed, 20 May 2020 02:39:13 +0000 (14:39 +1200)]
cmd/go: accept smart quotes when checking for missing gold in TestNoteReading
Fixes #39157
Change-Id: Ia983f5e66698519cd19c1565cfb80e86d08fdfc6
Reviewed-on: https://go-review.googlesource.com/c/go/+/234380 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Bryan C. Mills [Tue, 19 May 2020 18:17:05 +0000 (14:17 -0400)]
crypto/x509: save the temp dir in TestReadUniqueDirectoryEntries
In CL 231958, TempDir was changed to create a new temp directory on
each allocation, on the theory that it is easy to save in a variable
for callers that want the same directory repeatedly. Apply that
transformation here.
Cherry Zhang [Mon, 18 May 2020 22:27:10 +0000 (18:27 -0400)]
[dev.link] cmd/asm: expand package path if known
If the package path is known, pass it to the object file writer
so the symbol names are pre-expanded. (We already expand the
package path in debug info.)
Change-Id: I2b2b71edbb98924cbf3c4f9142b7e109e5b7501a
Reviewed-on: https://go-review.googlesource.com/c/go/+/234491
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jeremy Faller <jeremy@golang.org> Reviewed-by: Than McIntosh <thanm@google.com>
Cherry Zhang [Mon, 18 May 2020 22:20:18 +0000 (18:20 -0400)]
[dev.link] cmd/link: only do name expansion when needed
Most Go objects are compiled with known package path, so the
symbol name is already fully expanded. Nevertheless, currently
in the linker strings.Replace is called unconditionally, and most
of the time it doesn't do anything.
This CL records a per-object flag in the object file, and do the
name expansion only when the name is not expanded at compile time.
This gives small speedups for the linker. Linking cmd/compile:
Katie Hockman [Mon, 18 May 2020 20:49:04 +0000 (16:49 -0400)]
crypto/tls: remove version check when unmarshaling sessionState
This was causing issues when fuzzing with
TestMarshalUnmarshal since the test would
occassionally set the version to VersionTLS13,
which would fail when unmarshaling. The check
doesn't add much in practice, and there is no
harm in removing it to de-flake the test.
Jay Conrod [Thu, 14 May 2020 18:52:17 +0000 (14:52 -0400)]
cmd: update golang.org/x/mod to v0.3.0 (same commit)
v0.3.0 is a tag on 859b3ef565e2, the version that was already being
used. This change is a no-op, except for letting us use a release
version instead of a pseudo-version.
For #36905
Change-Id: I70b8ce2a3f1451f5602c469501362d7a6a673b12
Reviewed-on: https://go-review.googlesource.com/c/go/+/234002
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Michael Matloob <matloob@golang.org>
Richard Miller [Mon, 18 May 2020 08:34:17 +0000 (09:34 +0100)]
runtime: don't enable notes (=signals) too early in Plan 9
The Plan 9 runtime startup was enabling notes (like Unix signals)
before the gsignal stack was allocated. This left a small window
of time where an interrupt (eg by the parent killing a subprocess
quickly after exec) would cause a null pointer dereference in
sigtramp. This would leave the interrupted process suspended in
'broken' state instead of exiting. We've observed this on the
builders, where it can make a test time out waiting for the broken
process to terminate.
Updates #38772
Change-Id: I54584069fd3109595f06c78724c1f6419e028aab
Reviewed-on: https://go-review.googlesource.com/c/go/+/234397
Run-TryBot: David du Colombier <0intro@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David du Colombier <0intro@gmail.com>
Cherry Zhang [Thu, 14 May 2020 23:22:59 +0000 (19:22 -0400)]
cmd/link: detect trampoline of deferreturn call
The runtime needs to find the PC of the deferreturn call in a few
places. So for functions that have defer, we record the PC of
deferreturn call in its funcdata.
For very large binaries, the deferreturn call could be made
through a trampoline. The current code of finding deferreturn PC
fails in this case. This CL handles the trampoline as well.
Fixes #39049.
Change-Id: I929be54d6ae436f5294013793217dc2a35f080d4
Reviewed-on: https://go-review.googlesource.com/c/go/+/234105
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jeremy Faller <jeremy@golang.org> Reviewed-by: Than McIntosh <thanm@google.com>
Cherry Zhang [Thu, 14 May 2020 20:05:29 +0000 (16:05 -0400)]
[dev.link] cmd/link: delete sym.Symbol and sym.Reloc
This deletes all sym.Symbol and sym.Reloc references. This is
certainly not complete, and there are more cleanups to do. But I
feel this makes a good first round.
Cherry Zhang [Thu, 14 May 2020 17:15:53 +0000 (13:15 -0400)]
[dev.link] cmd/link: simplify field tracking support
Currently, for the special field tracking symbol go.track.XXX,
when they are reachable, we set its type to SCONST. There is no
need to do that. Just leave it unset (as Sxxx). The symbol is
done after this point.
Cherry Zhang [Thu, 14 May 2020 16:16:10 +0000 (12:16 -0400)]
cmd/link: fix SCONST symbol handling on darwin
Don't include SCONST symbols in the symbol table when
NotInSymbolTable is set. This is what the old code (genasmsym)
does.
In fact, SCONST symbol is only emitted by the field tracking
code, and is always NotInSymbolTable. So we should just not
include them at all, or not generate SCONST symbols at all. But
at this late stage I'll just restore the old behavior.
Michael Anthony Knyszek [Tue, 12 May 2020 16:08:50 +0000 (16:08 +0000)]
runtime: make maxOffAddr reflect the actual address space upper bound
Currently maxOffAddr is defined in terms of the whole 64-bit address
space, assuming that it's all supported, by using ^uintptr(0) as the
maximal address in the offset space. In reality, the maximal address in
the offset space is (1<<heapAddrBits)-1 because we don't have more than
that actually available to us on a given platform.
On most platforms this is fine, because arenaBaseOffset is just
connecting two segments of address space, but on AIX we use it as an
actual offset for the starting address of the available address space,
which is limited. This means using ^uintptr(0) as the maximal address in
the offset address space causes wrap-around, especially when we just
want to represent a range approximately like [addr, infinity), which
today we do by using maxOffAddr.
To fix this, we define maxOffAddr more appropriately, in terms of
(1<<heapAddrBits)-1.
This change also redefines arenaBaseOffset to not be the negation of the
virtual address corresponding to address zero in the virtual address
space, but instead directly as the virtual address corresponding to
zero. This matches the existing documentation more closely and makes the
logic around arenaBaseOffset decidedly simpler, especially when trying
to reason about its use on AIX.
Fixes #38966.
Change-Id: I1336e5036a39de846f64cc2d253e8536dee57611
Reviewed-on: https://go-review.googlesource.com/c/go/+/233497
Run-TryBot: Michael Knyszek <mknyszek@google.com> Reviewed-by: Austin Clements <austin@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Michael Munday [Wed, 13 May 2020 15:46:16 +0000 (16:46 +0100)]
cmd/compile: fix tuple selector bug in CSE pass
When tuple generators and selectors are eliminated as part of the
CSE pass we may end up with tuple selectors that are in different
blocks to the tuple generators that they correspond to. This breaks
the invariant that tuple generators and their corresponding
selectors must be in the same block. Therefore after CSE this
situation must be corrected.
Unfortunately the fixup code did not take into account that selectors
could be eliminated by CSE. It assumed that only the tuple generators
could be eliminated. In some situations this meant that it got into
a state where it was replacing references to selectors with references
to dead selectors in the wrong block.
To fix this we move the fixup code after the CSE rewrites have been
applied. This removes any difficult-to-reason-about interactions
with the CSE rewriter.
Fixes #38916.
Change-Id: I2211982dcdba399d03299f0a819945b3eb93b291
Reviewed-on: https://go-review.googlesource.com/c/go/+/233857
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>
Bryan C. Mills [Wed, 13 May 2020 03:06:14 +0000 (23:06 -0400)]
cmd/go: terminate TestScript commands more aggressively when the test times out
- Avoid starting subprocesses when the test is already very close to
timing out. The overhead of starting and stopping processes may
cause the test to exceed its deadline even if each individual
process is signaled soon after it is started.
- If a command does not shut down quickly enough after receiving
os.Interrupt, send it os.Kill using the same style of grace period
as in CL 228438.
- Fail the test if a background command whose exit status is not
ignored is left running at the end of the test. We have no reliable
way to distinguish a failure due to the termination signal from an
unexpected failure, and the termination signal varies across
platforms (so may cause failure on one platform but success on
another).
For #38797
Change-Id: I767898cf551dca45579bf01a9d1bb312e12d6193
Reviewed-on: https://go-review.googlesource.com/c/go/+/233526
Run-TryBot: Bryan C. Mills <bcmills@google.com> Reviewed-by: Jay Conrod <jayconrod@google.com>
Bryan C. Mills [Wed, 6 May 2020 20:22:15 +0000 (16:22 -0400)]
cmd/go: do not ignore permission errors when matching patterns
While reviewing CL 228784, I noticed that various filepath.WalkFunc
implementations within cmd/go were dropping non-nil errors.
Those errors turn out to be significant, at least in some cases: for
example, they can cause packages to appear to be missing when any
parent of the directory had the wrong permissions set.
(This also turned up a bug in the existing list_dedup_packages test,
which was accidentally passing a nonexistent directory instead of the
intended duplicate path.)
Change-Id: Ia09a0a33aa7a966d9f132d3747d6c674a5370b2d
Reviewed-on: https://go-review.googlesource.com/c/go/+/232579
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>
Bryan C. Mills [Wed, 13 May 2020 16:43:46 +0000 (12:43 -0400)]
runtime: reduce timing sensitivity in TestEINTR
- Don't assume that a process interrupted at 100μs intervals will have
enough remaining time to make progress. (Stop sending signals
in between signal storms to allow the process to quiesce.)
- Don't assume that a child process that spins for 1ms will block long
enough for the parent process to receive signals or make meaningful
progress. (Instead, have the child block indefinitely, and unblock
it explicitly after the signal storm.)
For #39043
Updates #22838
Updates #20400
Change-Id: I85cba23498c346a637e6cfe8684ca0c478562a93
Reviewed-on: https://go-review.googlesource.com/c/go/+/233877
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Remove the loader's PropagateSymbolChangesBackToLoader and
PropagateLoaderChangesToSymbols shim functions. These were used at one
point to enable conversion of phases in the linker that were
"downstream" of loadlibfull -- given the current wavefront position
there's not much point keeping them around.
Than McIntosh [Tue, 12 May 2020 21:18:05 +0000 (17:18 -0400)]
[dev.link] cmd/link: rename adddynrel2 back to adddynrel
Minor renaming cleanup to get rid of a couple of old sym.Symbol
adddynrel helpers and rename the current crop of adddynrel2
methods/functions back to adddynrel.
Ian Lance Taylor [Fri, 8 May 2020 04:34:54 +0000 (21:34 -0700)]
internal/poll, os: loop on EINTR
Historically we've assumed that we can install all signal handlers
with the SA_RESTART flag set, and let the system restart slow functions
if a signal is received. Therefore, we don't have to worry about EINTR.
This is only partially true, and we've added EINTR checks already for
connect, and open/read on Darwin, and sendfile on Solaris.
Other cases have turned up in #36644, #38033, and #38836.
Also, #20400 points out that when Go code is included in a C program,
the C program may install its own signal handlers without SA_RESTART.
In that case, Go code will see EINTR no matter what it does.
So, go ahead and check for EINTR. We don't check in the syscall package;
people using syscalls directly may want to check for EINTR themselves.
But we do check for EINTR in the higher level APIs in os and net,
and retry the system call if we see it.
This change looks safe, but of course we may be missing some cases
where we need to check for EINTR. As such cases turn up, we can add
tests to runtime/testdata/testprogcgo/eintr.go, and fix the code.
If there are any such cases, their handling after this change will be
no worse than it is today.
For #22838
Fixes #20400
Fixes #36644
Fixes #38033
Fixes #38836
Change-Id: I7e46ca8cafed0429c7a2386cc9edc9d9d47a6896
Reviewed-on: https://go-review.googlesource.com/c/go/+/232862
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Keith Randall [Thu, 7 May 2020 20:44:51 +0000 (13:44 -0700)]
cmd/compile: in prove, zero right shifts of positive int by #bits - 1
Taking over Zach's CL 212277. Just cleaned up and added a test.
For a positive, signed integer, an arithmetic right shift of count
(bit-width - 1) equals zero. e.g. int64(22) >> 63 -> 0. This CL makes
prove replace these right shifts with a zero-valued constant.
These shifts may arise in source code explicitly, but can also be
created by the generic rewrite of signed division by a power of 2.
// Signed divide by power of 2.
// n / c = n >> log(c) if n >= 0
// = (n+c-1) >> log(c) if n < 0
// We conditionally add c-1 by adding n>>63>>(64-log(c))
(first shift signed, second shift unsigned).
(Div64 <t> n (Const64 [c])) && isPowerOfTwo(c) ->
(Rsh64x64
(Add64 <t> n (Rsh64Ux64 <t>
(Rsh64x64 <t> n (Const64 <typ.UInt64> [63]))
(Const64 <typ.UInt64> [64-log2(c)])))
(Const64 <typ.UInt64> [log2(c)]))
If n is known to be positive, this rewrite includes an extra Add and 2
extra Rsh. This CL will allow prove to replace one of the extra Rsh with
a 0. That replacement then allows lateopt to remove all the unneccesary
fixups from the generic rewrite.
There is a rewrite rule to handle this case directly:
(Div64 n (Const64 [c])) && isNonNegative(n) && isPowerOfTwo(c) ->
(Rsh64Ux64 n (Const64 <typ.UInt64> [log2(c)]))
But this implementation of isNonNegative really only handles constants
and a few special operations like len/cap. The division could be
handled if the factsTable version of isNonNegative were available.
Unfortunately, the first opt pass happens before prove even has a
chance to deduce the numerator is non-negative, so the generic rewrite
has already fired and created the extra Ops discussed above.
Fixes #36159
By Printf count, this zeroes 137 right shifts when building std and cmd.
Alberto Donizetti [Sat, 9 May 2020 15:10:32 +0000 (17:10 +0200)]
runtime: fix dead link in gcc_androd.c file
Old url 404s because the file no longer exists on master; change it to
point to the android 10 release branch.
Change-Id: If0f8b645f2c746f9fc8bbd68f4d1fe41868493ba
Reviewed-on: https://go-review.googlesource.com/c/go/+/232809 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Steven Hartland [Fri, 8 May 2020 14:17:55 +0000 (14:17 +0000)]
net: only enable broadcast on sockets which support it
Only enable broadcast on SOCK_DGRAM and SOCK_RAW sockets, SOCK_STREAM
and others don't support it.
Don't enable SO_BROADCAST on UNIX domain sockets as they don't support it.
This caused failures on WSL which strictly checks setsockopt calls
unlike other OSes which often silently ignore bad options.
Also return error for setsockopt call for SO_BROADCAST on Windows
matching all other platforms but for IPv4 only as it's not supported
on IPv6 as per:
https://docs.microsoft.com/en-us/windows/win32/winsock/socket-options
Fixes #38954
Change-Id: I0503fd1ce96102b17121af548b66b3e9c2bb80d3
Reviewed-on: https://go-review.googlesource.com/c/go/+/232807
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Ayan George [Fri, 8 May 2020 04:13:44 +0000 (00:13 -0400)]
image/png: remove too early declaration of "n"
Before this commit, the code declares and assigns "n" with the result of
io.ReadFull() -- but the value is not used. The variable is then reused
later in the function.
This commit removes the first declaration of "n" and declares it closer
to where it is used.
Change-Id: I7ffe19a10f2a563c306bb6fe6562493435b9dc5a
Reviewed-on: https://go-review.googlesource.com/c/go/+/232917 Reviewed-by: Rob Pike <r@golang.org>
Daniel Martí [Fri, 8 May 2020 13:14:15 +0000 (14:14 +0100)]
testing: tests and benchmarks can assume flag.Parsed
testing.M.Run has this bit of code:
if !flag.Parsed() {
flag.Parse()
}
It makes sense, and it's common knowledge for many Go developers that
test flags are automatically parsed by the time tests and benchmarks are
run. However, the docs didn't clarify that. The previous wording only
mentioned that flag.Parse isn't run before TestMain, which doesn't
necessarily mean that it's run afterwards.
Fixes #38952.
Change-Id: I85f7a9dce637a23c5cb9abc485d47415c1a1ca27
Reviewed-on: https://go-review.googlesource.com/c/go/+/232806 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Daniel Martí [Wed, 29 May 2019 10:09:50 +0000 (11:09 +0100)]
encoding/json: reuse values when decoding map elements
When we decode into a struct, each input key-value may be decoded into
one of the struct's fields. Particularly, existing data isn't dropped,
so that some sub-fields can be decoded into without zeroing all other
data.
However, decoding into a map behaved in the opposite way. Whenever a
key-value was decoded, it completely replaced the previous map element.
If the map contained any non-zero data in that key, it's dropped.
Instead, try to reuse the existing element value if possible. If the map
element type is a pointer, and the value is non-nil, we can decode
directly into it. If it's not a pointer, make a copy and decode into
that copy, as map element values aren't addressable.
This means we have to parse and convert the map element key before the
value, to be able to obtain the existing element value. This is fine,
though. Moreover, reporting errors on the key before the value follows
the input order more closely.
Finally, add a test to explore the four combinations, involving pointer
and non-pointer, and non-zero and zero values. A table-driven test
wasn't used, as each case required different checks, such as checking
that the non-nil pointer case doesn't end up with a different pointer.
Fixes #31924.
Change-Id: I5ca40c9963a98aaf92f26f0b35843c021028dfca
Reviewed-on: https://go-review.googlesource.com/c/go/+/179337
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Cherry Zhang [Fri, 8 May 2020 19:08:55 +0000 (15:08 -0400)]
cmd/internal/obj/arm64: fix 32-bit BITCON test
The BITCON test, isbitcon, assumes 32-bit constants are expanded
repeatedly, i.e. by copying the low 32 bits to high 32 bits,
instead of zero extending. We already do such expansion in
progedit. In con32class when classifying 32-bit constants, we
should use the expanded constant, instead of zero-extending it.
TODO: we could have better encoding for things like ANDW $-1, Rx.
Robert Griesemer [Fri, 8 May 2020 19:13:11 +0000 (12:13 -0700)]
strconv: fix ParseComplex for strings with separators
The recently added function parseFloatPrefix tested the entire
string for correct placement of separators rather than just the
consumed part. The 4-char fix is in readFloat (atof.go:303).
Added more tests. Also added some white space for nicer
grouping of the test cases.
While at it, removed the need for calling testing.Run.
Fixes #38962.
Change-Id: Ifce84f362bb4ede559103f8d535556d3de9325f1
Reviewed-on: https://go-review.googlesource.com/c/go/+/233017 Reviewed-by: Ian Lance Taylor <iant@golang.org>
cmd/compile: omit file:pos for non-existent errors
Omits printing the file:line:column when trying to
open non-existent files
Given:
go tool compile x.go
* Before:
x.go:0: open x.go: no such file or directory
* After:
open x.go: no such file or directory
Reverts the revert in CL 231043 by only fixing the case
of non-existent errors which is what the original bug
was about. The fix for "permission errors" will come later
on when I have bandwidth to investigate the differences
between running with root and why os.Open works for some
builders and not others.