Before this change, when DeepEqual checks values with cycle, it may
panic due to stack overflow.
Here is a sample to reproduce the issue.
makeCycleMap := func() interface{} {
cycleMap := map[string]interface{}{}
cycleMap["foo"] = cycleMap
return cycleMap
}
m1 := makeCycleMap()
m2 := makeCycleMap()
reflect.DeepEqual(m1, m2) // stack overflow
The root cause is that DeepEqual fails to cache interface values
in visited map, which is used to detect cycle. DeepEqual calls
CanAddr to check whether a value should be cached or not. However,
all values referenced by interface don't have flagAddr thus all these
values are not cached.
THe fix is to remove CanAddr calls and use underlying ptr in value
directly. As ptr is only read-only in DeepEqual for caching, it's
safe to do so. We don't use UnsafeAddr this time, because this method
panics when CanAddr returns false.
Fixes#33907
Change-Id: I2aa88cc060a2c2192b1d34c129c0aad4bd5597e7
Reviewed-on: https://go-review.googlesource.com/c/go/+/191940
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Make it clear that IsValid checks that we have valid
reflect.Value and not the value of `v`
fixes#34152
Change-Id: Ib3d359eeb3a82bf733b9ed17c777fc4c143bc29c
Reviewed-on: https://go-review.googlesource.com/c/go/+/193841
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This CL moves parameter tagging to before escape analysis is complete,
so we still have access to EscLocation. This will be useful once
EscLocation starts tracking higher-fidelity escape details.
Notably, this CL stops using n.Esc to record parameter escape analysis
details. Now escape analysis only ever sets n.Esc to EscNone or
EscHeap. (It still defaults to EscUnknown, and is set to EscNever in
some places though.)
Passes toolstash-check.
Updates #33981.
Change-Id: I50a91ea1e38c442092de6cd14e20b211f8f818c9
Reviewed-on: https://go-review.googlesource.com/c/go/+/193178
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Remove unnecessary conditional guard for a couple of assertions in the
type parser's update() method (inspired by comment from Robert). No
change in functionality.
Change-Id: I706a54569e75c6960768247889b7dec3f267dde9
Reviewed-on: https://go-review.googlesource.com/c/go/+/194565
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
If an embedded field refers to a type via a pointer, the parser needs
to know the name of the embedded field. It is possible that the
pointer type is not yet resolved. This CL fixes the parser to handle
that case by setting the pointer element type to the unresolved named
type while the pointer is being resolved.
Fixes#34182
Change-Id: I48435e0404362a85effd7463685c502290fa3c57
Reviewed-on: https://go-review.googlesource.com/c/go/+/194440
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
No behavior change; just inverting the loop ordering so the
per-parameter behavior is a bit clearer.
Passes toolstash-check.
Updates #33981.
Change-Id: I9bfcd7d0a4aff65a27ced157767ca2ba8038319a
Reviewed-on: https://go-review.googlesource.com/c/go/+/193177
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reset is already performed when retrieving from pool
Change-Id: Ia810dd18d3e55a1565a5ad435a00d1e46724576c
GitHub-Last-Rev: d9df74a4ae
GitHub-Pull-Request: golang/go#34195
Reviewed-on: https://go-review.googlesource.com/c/go/+/194338
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Also fix the key used to store the ID.
This is a significant speedup in cmd/go run time when using an
unreleased toolchain. For example, the TestGoBuildTestOnly cmd/go test
goes from 15 seconds to 1 second.
Change-Id: Ibfd697d55084db059c6b563f70f71f635e935391
Reviewed-on: https://go-review.googlesource.com/c/go/+/194441
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
This CL gets rid of the MOVDreg and MOVDnop SSA operations on
s390x. They were originally inserted to help avoid situations
where a sign/zero extension was elided but a spill invalidated
the optimization. It's not really clear we need to do this though
(amd64 doesn't have these ops for example) so long as we are
careful when removing sign/zero extensions. Also, the MOVDreg
technique doesn't work if the register is spilled before the
MOVDreg op (I haven't seen that in practice).
Removing these ops reduces the complexity of the rules and also
allows us to unblock optimizations. For example, the compiler can
now merge the loads in binary.{Big,Little}Endian.PutUint16 which
it wasn't able to do before. This CL reduces the size of the .text
section in the go tool by about 4.7KB (0.09%).
Change-Id: Icaddae7f2e4f9b2debb6fabae845adb3f73b41db
Reviewed-on: https://go-review.googlesource.com/c/go/+/173897
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This removes the unnecessary code to check whether the shift
is within limits or not when the shift amount is a constant.
The rules hit 23034 times when building std cmd.
grep -E "Wasm.rules:(106|107|121|122|139|140)" rulelog | wc -l
23034
Reduces the size of pkg/js_wasm by 132 bytes.
Change-Id: I64a2b8faca08c3b5039d6a027d4676130d2db18d
Reviewed-on: https://go-review.googlesource.com/c/go/+/194239
Run-TryBot: Agniva De Sarker <agniva.quicksilver@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Richard Musiol <neelance@gmail.com>
The indirect method checked the type of the child when indirecting a
pointer. If the current value is a pointer and we are decoding null, we
can skip this entirely and return early, avoiding the whole descent.
Fixes#31776
Change-Id: Ib8b2a2357572c41f56fceac59b5a858980f3f65e
Reviewed-on: https://go-review.googlesource.com/c/go/+/174699
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Leave reporting of multiple errors for strings alone for now;
we probably want to see all incorrect escape sequences in
runes/strings independent of other errors.
Fixes#33961.
Change-Id: Id722e95f802687963eec647d1d1841bd6ed17d35
Reviewed-on: https://go-review.googlesource.com/c/go/+/192499
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This code enables handling of ASN1's string type BMPString, used in some digital signatures.
Parsing code taken from golang.org/x/crypto/pkcs12.
Change-Id: Ibeae9cf4d8ae7c18f8b5420ad9244a16e117ff6b
GitHub-Last-Rev: 6945253514
GitHub-Pull-Request: golang/go#26690
Reviewed-on: https://go-review.googlesource.com/c/go/+/126624
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Rewording the comments for Join to do a better job of calling out
when Clean is called. Also clarifing other portions of the comment.
Fixes#29875
Change-Id: Ied43983bb10a97922898d28af133de0930224496
Reviewed-on: https://go-review.googlesource.com/c/go/+/194339
Reviewed-by: Rob Pike <r@golang.org>
Previously, we used a single "untyped number" type for all untyped
numeric constants. This led to vague error messages like "string(1.0)"
reporting that "1 (type untyped number)" can't be converted to string,
even though "string(1)" is valid.
This CL makes cmd/compile more like go/types by utilizing
types.Ideal{int,rune,float,complex} instead of types.Types[TIDEAL],
and keeping n.Type in sync with n.Val().Ctype() during constant
folding.
Thanks to K Heller for looking into this issue, and for the included
test case.
Fixes#21979.
Change-Id: Ibfea88c05704bc3c0a502a455d018a375589754d
Reviewed-on: https://go-review.googlesource.com/c/go/+/194019
Reviewed-by: Robert Griesemer <gri@golang.org>
This is a documentation-only change
Fixes#33298
Change-Id: I816058a872b57dc868dff11887214d9de92d9342
Reviewed-on: https://go-review.googlesource.com/c/go/+/188821
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
On modern 64bit CPUs a SHR, SHL or AND instruction take 1 cycle to execute.
A pair of shifts that operate on the same register will take 2 cycles
and needs to wait for the input register value to be available.
Large constants used to mask the high bits of a register with an AND
instruction can not be encoded as an immediate in the AND instruction
on amd64 and therefore need to be loaded into a register with a MOV
instruction.
However that MOV instruction is not dependent on the output register and
on many CPUs does not compete with the AND or shift instructions for
execution ports.
Using a pair of shifts to mask high bits instead of an AND to mask high
bits of a register has a shorter encoding and uses one less general
purpose register but is slower due to taking one clock cycle longer
if there is no register pressure that would make the AND variant need to
generate a spill.
For example the instructions emitted for (x & 1 << 63) before this CL are:
48c1ea3f SHRQ $0x3f, DX
48c1e23f SHLQ $0x3f, DX
after this CL the instructions are the same as GCC and LLVM use:
48b80000000000000080 MOVQ $0x8000000000000000, AX
4821d0 ANDQ DX, AX
Some platforms such as arm64 already have SSA optimization rules to fuse
two shift instructions back into an AND.
Removing the general rule to rewrite AND to SHR+SHL speeds up this benchmark:
var GlobalU uint
func BenchmarkAndHighBits(b *testing.B) {
x := uint(0)
for i := 0; i < b.N; i++ {
x &= 1 << 63
}
GlobalU = x
}
amd64/darwin on Intel(R) Core(TM) i7-3520M CPU @ 2.90GHz:
name old time/op new time/op delta
AndHighBits-4 0.61ns ± 6% 0.42ns ± 6% -31.42% (p=0.000 n=25+25):
Updates #33826
Updates #32781
Change-Id: I862d3587446410c447b9a7265196b57f85358633
Reviewed-on: https://go-review.googlesource.com/c/go/+/191780
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Call the Nano methods of Timespec and Timeval in TimespecToNsec and
TimevalToNsec respectively, instead of duplicating the implementation.
Change-Id: I17551ea54c59c1e45ce472e029c625093a67251a
GitHub-Last-Rev: fecf43d163
GitHub-Pull-Request: golang/go#33390
Reviewed-on: https://go-review.googlesource.com/c/go/+/188397
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Use the following (suboptimal) script to obtain a list of possible
typos:
#!/usr/bin/env sh
set -x
git ls-files |\
grep -e '\.\(c\|cc\|go\)$' |\
xargs -n 1\
awk\
'/\/\// { gsub(/.*\/\//, ""); print; } /\/\*/, /\*\// { gsub(/.*\/\*/, ""); gsub(/\*\/.*/, ""); }' |\
hunspell -d en_US -l |\
grep '^[[:upper:]]\{0,1\}[[:lower:]]\{1,\}$' |\
grep -v -e '^.\{1,4\}$' -e '^.\{16,\}$' |\
sort -f |\
uniq -c |\
awk '$1 == 1 { print $2; }'
Then, go through the results manually and fix the most obvious typos in
the non-vendored code.
Change-Id: I3cb5830a176850e1a0584b8a40b47bde7b260eae
Reviewed-on: https://go-review.googlesource.com/c/go/+/193848
Reviewed-by: Robert Griesemer <gri@golang.org>
CL 193843 disabled sysctl on iOS. This change disables two tests that
rely on sysctl.
Updates #34133
Change-Id: I7c569a1992a50ad6027a294c1fd535cccddcfc4e
Reviewed-on: https://go-review.googlesource.com/c/go/+/193844
Run-TryBot: Elias Naur <mail@eliasnaur.com>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
Start implementing an assembler for RISC-V - this provides register
definitions and instruction mnemonics as defined in the RISC-V
Instruction Set Manual, along with instruction encoding.
The instruction encoding is generated by the parse_opcodes script with
the "opcodes" and "opcodes-pseudo" files from (`make inst.go`):
https://github.com/riscv/riscv-opcodes
This is based on the riscv-go port:
https://github.com/riscv/riscv-go
Contributors to the riscv-go port are:
Amol Bhave <ammubhave@gmail.com>
Benjamin Barenblat <bbaren@google.com>
Josh Bleecher Snyder <josharian@gmail.com>
Michael Pratt <michael@pratt.im>
Michael Yenik <myenik@google.com>
Ronald G. Minnich <rminnich@gmail.com>
Stefan O'Rear <sorear2@gmail.com>
This port has been updated to Go 1.13:
https://github.com/4a6f656c/riscv-go
Updates #27532
Change-Id: I257b6de87e9864df61a2b0ce9be15968c1227b49
Reviewed-on: https://go-review.googlesource.com/c/go/+/193677
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Rather than manually setting the LinkArch for each case, pass the correct
*obj.LinkArch to the arch* function, as is already done for archX86().
Change-Id: I4cf950780aa30a1385e785fb1d26edacb99bda79
Reviewed-on: https://go-review.googlesource.com/c/go/+/193818
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 187037 applied a fix to handle the case where
files larger than 2GiB were not being sendfile-d,
in one shot, rejecting any files whose size was
larger than the 2GiB.
This CL allows files that are larger than limit
by SendFile-ing in chunks of upto 2GiB per chunk.
The test has been excluded as testing with 3GB
requires creating a local file, flushing it
and then doing sendfile which takes a while
and could cause flakes on computers without capacity,
but the test can be retroactively accessed at:
https://go-review.googlesource.com/c/go/+/192518/8/src/net/sendfile_windows_test.goFixes#33193.
Change-Id: If57c25bc289aec82b748890ac1ac4f55798d6a5e
Reviewed-on: https://go-review.googlesource.com/c/go/+/192518
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
This CL detangles the hairy mess that was convlit+defaultlit. In
particular, it makes the following changes:
1. convlit1 now follows the standard typecheck behavior of setting
"n.Type = nil" if there's an error. Notably, this means for a lot of
test cases, we now avoid reporting useless follow-on error messages.
For example, after reporting that "1 << s + 1.0" has an invalid shift,
we no longer also report that it can't be assigned to string.
2. Previously, assignconvfn had some extra logic for trying to
suppress errors from convlit/defaultlit so that it could provide its
own errors with better context information. Instead, this extra
context information is now passed down into convlit1 directly.
3. Relatedly, this CL also removes redundant calls to defaultlit prior
to assignconv. As a consequence, when an expression doesn't make sense
for a particular assignment (e.g., assigning an untyped string to an
integer), the error messages now say "untyped string" instead of just
"string". This is more consistent with go/types behavior.
4. defaultlit2 is now smarter about only trying to convert pairs of
untyped constants when it's likely to succeed. This allows us to
report better error messages for things like 3+"x"; instead of "cannot
convert 3 to string" we now report "mismatched types untyped number
and untyped string".
Passes toolstash-check.
Change-Id: I26822a02dc35855bd0ac774907b1cf5737e91882
Reviewed-on: https://go-review.googlesource.com/c/go/+/187657
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
We used to be more aggressive about constant folding in the frontend,
handling expressions that the Go spec does not consider constant;
e.g., "(*int)(unsafe.Pointer(uintptr(200)))". However, that led to a
lot of subtle Go spec conformance issues, so we've since abandoned
that effort (CL 151320), leaving SSA to handle these cases instead.
As such, the only time we now end up with pointer-typed OLITERALs is
when "nil" is implicitly converted to a pointer-typed variable.
Instead of representing these OLITERALs with an CTINT of 0, we can
just use CTNIL.
Saves a few bytes of memory and lines of code.
Change-Id: Ibc5c756b992fdc89c3bdaf4fda3aa352e8e2b101
Reviewed-on: https://go-review.googlesource.com/c/go/+/193437
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
This reverts commit 2da9c3e0f9.
Reason for revert: while the new error messages are more informative,
they're not strictly correct. This CL also conflicts with CL 187657.
Change-Id: I1c36cf7e86c2f35ee83a4f98918ee38aa1f59965
Reviewed-on: https://go-review.googlesource.com/c/go/+/193977
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Consistent with the spec's definition of "ordered" and "comparable".
Fixes#34147
Change-Id: Id13186df5343588d80eaebfeb23092596a846d51
Reviewed-on: https://go-review.googlesource.com/c/go/+/193840
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Follow-up to Change-Id: If6e52c59eab438599d641ecf6f110ebafca740a9
This addresses the remaining tech debt on issue 21979.
The aforementioned previous CL silenced one of two mostly redundant
compiler errors. However, the silenced error was the more expressive
error. This CL now imbues the surviving error with the same level
of expressiveness as the old semi-redundant error.
Fixes#21979
Change-Id: I3273d48c88bbab073fabe53421d801df621ce321
Reviewed-on: https://go-review.googlesource.com/c/go/+/191079
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
GCC has supported the __atomic intrinsics since 4.7, and clang
supports them as well. They are better than the __sync intrinsics in
that they specify a memory model and, more importantly for our purposes,
they are reliably implemented either in the compiler or in libatomic.
Change-Id: I5e0036ea3300f65c28b1c3d1f3b93fb61c1cd646
Reviewed-on: https://go-review.googlesource.com/c/go/+/193603
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
It was already covered by a regex pattern, but it didn't have a test.
To fix the issue for good, added regression test.
Fixes#28832.
Change-Id: I861e3bed92d3b9484fd8671270dbd2e264b10d2d
Reviewed-on: https://go-review.googlesource.com/c/go/+/191311
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 59610 merged tempname to tempAt, but some of comments and error
message still refer to tempname. So changing to tempAt instead.
Change-Id: I032f3bedc135d17124b0daaf22c97d0d5ada0a6f
Reviewed-on: https://go-review.googlesource.com/c/go/+/193817
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Perform a single lookup of "plugin.Open" at the point where we set the
loaded flag for the context, then cache whether the result is nil, so
that we can consult this cached value later on (instead of having to
look up the symbol each time). This helps speed up the DynLinkingGo()
context method, which is called from within some very hot loops in the
linker (when linking 'hyperkube' from kubernetes, reduces total calls
to "sym.(*Symbols).ROLookup" from 6.5M to 4.3M)
Change-Id: I92a2ea2b21d24f67aec0a7afeef4acc77c095adf
Reviewed-on: https://go-review.googlesource.com/c/go/+/193260
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This test contains a very tight loop with locking/unlocking that can
wind up as an unpreemptible when compiled with gccgo, depending on
inlining. Tweak the test slightly to avoid this problem.
Change-Id: I155fd2b4bfea961244eb6c6594c24ab03d32d41c
Reviewed-on: https://go-review.googlesource.com/c/go/+/193619
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Test with some code that triggered a compilation error bug in gccgo.
Updates #33866.
Change-Id: Ib2f226bbbebbfae33b41037438fe34dc5f2ad034
Reviewed-on: https://go-review.googlesource.com/c/go/+/193261
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This affects the position information that's associated with the
implicit OCONV nodes created in assignconvfn.
Not super important and the followup rewrite CL fixes this too, but
fixing it separately is easy and makes toolstash-check happier.
Change-Id: Ifd65dc524b367812d14a4d996647a5d40665fb38
Reviewed-on: https://go-review.googlesource.com/c/go/+/193606
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
