Modify skip rule for TestRaiseException to trigger on both the base
builder (windows-amd64-2012) and the newcc canary builder
(windows-amd64-2012-newcc).
Updates #49681.
Change-Id: I132f9ddd102666b68ad04cc661fdcc2cd841051a
Reviewed-on: https://go-review.googlesource.com/c/go/+/451294
Auto-Submit: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
This will let us reuse it in crypto/ecdsa for the NIST scalar fields.
The main change in API is around encoding and decoding. The SetBytes +
ExpandFor sequence was hacky: SetBytes could produce a bigger size than
the modulus if leading zeroes in the top byte overflowed the limb
boundary, so ExpandFor had to check for and tolerate that. Also, the
caller was responsible for checking that the overflow was actually all
zeroes (which we weren't doing, exposing a crasher in decryption and
signature verification) and then for checking that the result was less
than the modulus. Instead, make SetBytes take a modulus and return an
error if the value overflows. Same with Bytes: we were always allocating
based on Size before FillBytes anyway, so now Bytes takes a modulus.
Finally, SetBig was almost only used for moduli, so replaced
NewModulusFromNat and SetBig with NewModulusFromBig.
Moved the constant-time bitLen to math/big.Int.BitLen. It's slower, but
BitLen is primarily used in cryptographic code, so it's safer this way.
Change-Id: Ibaf7f36d80695578cb80484167d82ce1aa83832f
Reviewed-on: https://go-review.googlesource.com/c/go/+/450055
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
This is missing a test for Ed25519ph with context, since the RFC doesn't
provide one.
Fixes#31804
Change-Id: I20947374c51c6b22fb2835317d00edf816c9a2d2
Reviewed-on: https://go-review.googlesource.com/c/go/+/404274
Auto-Submit: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Several operations emulate instructions available on power9. Use
the GOPPC64_power9 macro provided by the compiler to select the
native instructions if the minimum cpu requirements are met.
Likewise rework the LXSDX_BE to simplify usage when overriding
it. It is only used in one place.
All three configurations are tested via CI.
On POWER9:
pkg:crypto/cipher goos:linux goarch:ppc64le
AESCBCEncrypt1K 949MB/s ± 0% 957MB/s ± 0% +0.83%
AESCBCDecrypt1K 1.82GB/s ± 0% 1.99GB/s ± 0% +8.93%
pkg:crypto/aes goos:linux goarch:ppc64le
Encrypt 1.01GB/s ± 0% 1.05GB/s ± 0% +4.36%
Decrypt 987MB/s ± 0% 1024MB/s ± 0% +3.77%
Change-Id: I56d0eb845647dd3c43bcad71eb281b499e1d1789
Reviewed-on: https://go-review.googlesource.com/c/go/+/449116
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Auto-Submit: Paul Murphy <murp@ibm.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Joedian Reid <joedian@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Paul Murphy <murp@ibm.com>
This patch reworks the handling of DLL import symbols in the PE host
object loader to ensure that the Go linker can deal with them properly
during internal linking.
Prior to this point the strategy was to immediately treat an import
symbol reference of the form "__imp__XXX" as if it were a reference to
the corresponding DYNIMPORT symbol XXX, except for certain special
cases. This worked for the most part, but ran into problems in
situations where the target ("XXX") wasn't a previously created
DYNIMPORT symbol (and when these problems happened, the root cause was
not always easy to see).
The new strategy is to not do any renaming or forwarding immediately,
but to delay handling until host object loading is complete. At that
point we make a scan through the newly introduced text+data sections
looking at the relocations that target import symbols, forwarding
the references to the corresponding DYNIMPORT sym where appropriate
and where there are direct refs to the DYNIMPORT syms, tagging them
for stub generation later on.
Updates #35006.
Updates #53540.
Change-Id: I2d42b39141ae150a9f82ecc334001749ae8a3b4a
Reviewed-on: https://go-review.googlesource.com/c/go/+/451738
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
Adapt the testcshared tests to handle the case where the path output
by invoking
gcc -print-prog-name=dlltool
is a path lacking the final ".exe" suffix (this seems to be what clang
is doing); tack it on before using if this is the case.
Updates #35006.
Updates #53540.
Change-Id: I04fb7b9fc90677880b1ced4a4ad2a8867a3f5f86
Reviewed-on: https://go-review.googlesource.com/c/go/+/451816
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Add a new debugging flag "-capturehostobjs" that instructs the linker
to capture copies of all object files loaded in during the host object
loading portion of CGO internal linking. The intent is to make it
easier to analyze the objects after the fact (as opposed to having to
dig around inside archives, which can be a "find needle in haystack"
exercise).
Change-Id: I7023a5b72b1b899ea9b3bd6501f069d1f21bbaf0
Reviewed-on: https://go-review.googlesource.com/c/go/+/451737
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
When ctxt.Debugvlog > 1, produce additional trace output to describe
which object files are being pulled out of host archive libraries and
why they were pulled (e.g. which symbol had a reference to something
in a library). Intended to make it easier to debug problems with cgo
internal linking.
Change-Id: Icd64aff244b9145162a00cb51642ef32f26adfba
Reviewed-on: https://go-review.googlesource.com/c/go/+/451736
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This is faster than the current code because computing RR involves
one more shiftIn and using it involves an extra multiplication, but each
exponentiation was doing montgomeryRepresentation twice, once for x and
once for 1, and now they share the RR precomputation.
More importantly, it allows precomputing the value and attaching it to
the private key in a future CL.
name old time/op new time/op delta
DecryptPKCS1v15/2048-8 1.46ms ± 0% 1.40ms ± 7% -3.69% (p=0.003 n=10+9)
DecryptPKCS1v15/3072-8 4.23ms ± 0% 4.13ms ± 4% -2.36% (p=0.004 n=9+9)
DecryptPKCS1v15/4096-8 9.42ms ± 0% 9.08ms ± 3% -3.69% (p=0.000 n=9+10)
EncryptPKCS1v15/2048-8 221µs ± 0% 137µs ± 1% -37.91% (p=0.000 n=9+10)
DecryptOAEP/2048-8 1.46ms ± 0% 1.39ms ± 1% -4.97% (p=0.000 n=9+10)
EncryptOAEP/2048-8 221µs ± 0% 138µs ± 0% -37.71% (p=0.000 n=8+10)
SignPKCS1v15/2048-8 1.68ms ± 0% 1.53ms ± 1% -8.85% (p=0.000 n=9+10)
VerifyPKCS1v15/2048-8 220µs ± 0% 137µs ± 1% -37.84% (p=0.000 n=9+10)
SignPSS/2048-8 1.68ms ± 0% 1.52ms ± 1% -9.16% (p=0.000 n=8+8)
VerifyPSS/2048-8 234µs ±12% 138µs ± 1% -40.87% (p=0.000 n=10+9)
Change-Id: I6c650bad9019765d793fd37a529ca186cf1eeef7
Reviewed-on: https://go-review.googlesource.com/c/go/+/445019
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
I have never encountered multi-prime RSA in the wild. A GitHub-wide
search reveals exactly two explicit uses of it (and a couple of tools
that leave the number configurable but defaulting to two).
https://github.com/decred/tumblebit/blob/31898baea/puzzle/puzzlekey.go#L38https://github.com/carl-mastrangelo/pixur/blob/95d4a4208/tools/genkeys/genkeys.go#L13
Multi-prime RSA has a slight performance advantage, but has limited
compatibility and the number of primes must be chosen carefully based on
the key size to avoid security issues. It also requires a completely
separate and rarely used private key operation code path, which if buggy
or incorrect would leak the private key.
Mark it as deprecated, and remove the dedicated CRT optimization,
falling back instead to the slower but safer non-CRT fallback.
Change-Id: Iba95edc044fcf9b37bc1f4bb59c6ea273975837f
Reviewed-on: https://go-review.googlesource.com/c/go/+/445017
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Infamously, big.Int does not provide constant-time arithmetic, making
its use in cryptographic code quite tricky. RSA uses big.Int
pervasively, in its public API, for key generation, precomputation, and
for encryption and decryption. This is a known problem. One mitigation,
blinding, is already in place during decryption. This helps mitigate the
very leaky exponentiation operation. Because big.Int is fundamentally
not constant-time, it's unfortunately difficult to guarantee that
mitigations like these are completely effective.
This patch removes the use of big.Int for encryption and decryption,
replacing it with an internal nat type instead. Signing and verification
are also affected, because they depend on encryption and decryption.
Overall, this patch degrades performance by 55% for private key
operations, and 4-5x for (much faster) public key operations.
(Signatures do both, so the slowdown is worse than decryption.)
name old time/op new time/op delta
DecryptPKCS1v15/2048-8 1.50ms ± 0% 2.34ms ± 0% +56.44% (p=0.000 n=8+10)
DecryptPKCS1v15/3072-8 4.40ms ± 0% 6.79ms ± 0% +54.33% (p=0.000 n=10+9)
DecryptPKCS1v15/4096-8 9.31ms ± 0% 15.14ms ± 0% +62.60% (p=0.000 n=10+10)
EncryptPKCS1v15/2048-8 8.16µs ± 0% 355.58µs ± 0% +4258.90% (p=0.000 n=10+9)
DecryptOAEP/2048-8 1.50ms ± 0% 2.34ms ± 0% +55.68% (p=0.000 n=10+9)
EncryptOAEP/2048-8 8.51µs ± 0% 355.95µs ± 0% +4082.75% (p=0.000 n=10+9)
SignPKCS1v15/2048-8 1.51ms ± 0% 2.69ms ± 0% +77.94% (p=0.000 n=10+10)
VerifyPKCS1v15/2048-8 7.25µs ± 0% 354.34µs ± 0% +4789.52% (p=0.000 n=9+9)
SignPSS/2048-8 1.51ms ± 0% 2.70ms ± 0% +78.80% (p=0.000 n=9+10)
VerifyPSS/2048-8 8.27µs ± 1% 355.65µs ± 0% +4199.39% (p=0.000 n=10+10)
Keep in mind that this is without any assembly at all, and that further
improvements are likely possible. I think having a review of the logic
and the cryptography would be a good idea at this stage, before we
complicate the code too much through optimization.
The bulk of the work is in nat.go. This introduces two new types: nat,
representing natural numbers, and modulus, representing moduli used in
modular arithmetic.
A nat has an "announced size", which may be larger than its "true size",
the number of bits needed to represent this number. Operations on a nat
will only ever leak its announced size, never its true size, or other
information about its value. The size of a nat is always clear based on
how its value is set. For example, x.mod(y, m) will make the announced
size of x match that of m, since x is reduced modulo m.
Operations assume that the announced size of the operands match what's
expected (with a few exceptions). For example, x.modAdd(y, m) assumes
that x and y have the same announced size as m, and that they're reduced
modulo m.
Nats are represented over unsatured bits.UintSize - 1 bit limbs. This
means that we can't reuse the assembly routines for big.Int, which use
saturated bits.UintSize limbs. The advantage of unsaturated limbs is
that it makes Montgomery multiplication faster, by needing fewer
registers in a hot loop. This makes exponentiation faster, which
consists of many Montgomery multiplications.
Moduli use nat internally. Unlike nat, the true size of a modulus always
matches its announced size. When creating a modulus, any zero padding is
removed. Moduli will also precompute constants when created, which is
another reason why having a separate type is desirable.
Updates #20654
Co-authored-by: Filippo Valsorda <filippo@golang.org>
Change-Id: I73b61f87d58ab912e80a9644e255d552cbadcced
Reviewed-on: https://go-review.googlesource.com/c/go/+/326012
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Joedian Reid <joedian@golang.org>
Currently, tfork on openbsd/mips64 returns the thread ID on success and
a negative error number on error. In CL#447175, newosproc was changed
to assume that a non-zero value is an error - return zero on success to
match this expectation.
Change-Id: I955efad49b149146165eba3d05fe40ba75caa098
Reviewed-on: https://go-review.googlesource.com/c/go/+/451257
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Joedian Reid <joedian@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Joel Sing <joel@sing.id.au>
This test exercises the case where a net.Conn error occurs while
writing a response body. It injects an error by setting a timeout
on the Conn. If this timeout expires before response headers are
written, the test fails. The test attempts to recover from this
failure by extending the timeout and retrying.
Set the timeout after the response headers are removed, and
remove the retry loop.
Fixes#56274.
Change-Id: I293f8bedb7b20a21d14f43ea9bb48fc56b59441c
Reviewed-on: https://go-review.googlesource.com/c/go/+/452175
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
Set a logger in newClientServerTest that directs the server
log output to the testing.T's log, so log output gets properly
associated with the test that caused it.
Change-Id: I13686ca35c3e21adae16b2fc37ce36daea3df9d5
Reviewed-on: https://go-review.googlesource.com/c/go/+/452075
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Adds a method which allows users to set a fallback certificate pool for
usage during verification if the system certificate pool is empty.
Updates #43958
Change-Id: I279dd2f753743bce19790f2ae29f063c89c9359d
Reviewed-on: https://go-review.googlesource.com/c/go/+/449235
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Roland Shoemaker <roland@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
To pick up CL 451515.
This CL also updates x/net because x/crypto's dependency was bumped
while tagging v0.3.0.
Done by
go get -d golang.org/x/crypto@2c476679df9a
go mod tidy
go mod vendor
Change-Id: I432a04586be3784b1027aa9b62d86c0df6d4a97e
Reviewed-on: https://go-review.googlesource.com/c/go/+/452096
Reviewed-by: Roland Shoemaker <roland@golang.org>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Before emitting a "wanted Foo but got Bar" message for an interface
type match failure, check that Foo and Bar are different. If they
are not, add package paths to first unexported struct field seen,
because that is the cause (a cause, there could be more than one).
Replicated in go/types.
Added tests to go/types and cmd/compile/internal/types2
Fixes#54258.
Change-Id: Ifc2b2067d62fe2138996972cdf3b6cb7ca0ed456
Reviewed-on: https://go-review.googlesource.com/c/go/+/422914
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
We save non-go files in the cached srcfiles file because we want the
non-go files for vet, but we shouldn't report them in CompiledGoFiles.
Filter them out before adding them to CompiledGoFiles.
Fixes#28749
Change-Id: I889d4bbf8c4ec1348584a62ef5e4f8b3f05e97da
Reviewed-on: https://go-review.googlesource.com/c/go/+/451285
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
In the script engine, if a command does not return a Wait function and
it succeeds, we won't call checkStatus. That means that commands that
don't have a wait function, have a "!" indicating that they are
supposed to fail, and then succeed will spuriously not fail the script
engine test even they were supposed to fail but didn't.
Change-Id: Ic88c3cdd628064d48f14a8a4a2e97cded48890fb
Reviewed-on: https://go-review.googlesource.com/c/go/+/451284
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
For #50101
For #51152
For #53482
For #55301
For #56515
Change-Id: I11edeb4be0a7f80fb72fd7680a3407d081f83b8b
Reviewed-on: https://go-review.googlesource.com/c/go/+/451420
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Change-Id: I69065f8adf101fdb28682c55997f503013a50e29
Reviewed-on: https://go-review.googlesource.com/c/go/+/449757
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Joedian Reid <joedian@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Joedian Reid <joedian@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
More stuff to do = more stack needed. Bump up the guard space when
building with the race detector.
Fixes#54291
Change-Id: I701bc8800507921bed568047d35b8f49c26e7df7
Reviewed-on: https://go-review.googlesource.com/c/go/+/451217
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Inline the only use of checkMarks which also allows to drop the
always-true report argument. This also ensures the correct line gets
reported in case of an error.
Also remove the unused markTree function and drop the unused testing.T
argument from makeTree.
Change-Id: I4033d3e5ecd929d08ce03c563aa99444e102d931
Reviewed-on: https://go-review.googlesource.com/c/go/+/451615
Reviewed-by: Joedian Reid <joedian@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Tobias Klauser <tobias.klauser@gmail.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
If a compatible trampoline has been inserted by a previously laid
function in the same section, and is known to be sufficiently close,
it can be reused.
When testing if the trampoline can be reused, the addend of the direct
call should be ignored. It is already encoded in the trampoline. If the
addend is non-zero, and the target sufficiently far away, and just
beyond direct call reach, this may cause the trampoline to be
incorrectly reused.
This was observed on go1.17.13 and openshift-installer commit f3c53b382
building in release mode with the following error:
github.com/aliyun/alibaba-cloud-sdk-go/services/cms.(*Client).DescribeMonitoringAgentAccessKeyWithChan.func1: direct call too far: runtime.duffzero+1f0-tramp0-1 -2000078
Fixes#56775
Change-Id: I54af957302506d4e3cd5d3121542c83fe980e912
Reviewed-on: https://go-review.googlesource.com/c/go/+/451415
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Paul Murphy <murp@ibm.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Reviewed-by: Than McIntosh <thanm@google.com>
The previous rule may move the phi value into a wrong block.
This CL make it only rewrite the phi value not the If block,
so that the phi value will stay in old block.
Fixes#56777
Change-Id: I9479a5c7f28529786968413d35b82a16181bb1f1
Reviewed-on: https://go-review.googlesource.com/c/go/+/451496
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Wayne Zuo <wdvxdr@golangcn.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: David Chase <drchase@google.com>
This CL cleans up the literal pool implementation and inserts an UNDEF
instruction before the literal pool if the last instruction of the
function is not an unconditional jump instruction, RET or ERET
instruction.
Change-Id: Ifecb9e3372478362dde246c1bc9bc8d527a469d5
Reviewed-on: https://go-review.googlesource.com/c/go/+/424134
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Joedian Reid <joedian@golang.org>
Run-TryBot: Eric Fang <eric.fang@arm.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This change adds a new GODEBUG flag called pagetrace that writes a
low-overhead trace of how pages of memory are managed by the Go runtime.
The page tracer is kept behind a GOEXPERIMENT flag due to a potential
security risk for setuid binaries.
Change-Id: I6f4a2447d02693c25214400846a5d2832ad6e5c0
Reviewed-on: https://go-review.googlesource.com/c/go/+/444157
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Currently, we judge whether we need to fix up the branch instruction
based on Optab.type_ field, but the type_ field in optab may change.
This CL marks the branch instruction in optab, and checks whether to
do fixing up according to the mark. Depending on the constant parameter
range of the branch instruction, there are two labels, BRANCH14BITS,
BRANCH19BITS. For the 26-bit branch, linker will handle it.
Besides this CL removes the unnecessary alignment of the DWORD
instruction. Because the ISA doesn't require it and no 64-bit load
assume it. The only effect is that there is some performance penalty
for loading from DWORDs if the 8-byte DWORD instruction crosses the
cache line, but this is very rare.
Change-Id: I993902b3fb5ad8e081dd6c441e86bcf581031835
Reviewed-on: https://go-review.googlesource.com/c/go/+/424135
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Eric Fang <eric.fang@arm.com>
Reviewed-by: Joedian Reid <joedian@golang.org>
are provided
Improve the accuracy of recorded types and instances for function calls,
by instantiating their signature before checking arguments if all type
arguments are provided. This avoids a problem where fully instantiated
function signatures are are not recorded as such following an error
checking their arguments.
Fixesgolang/go#51803
Change-Id: Iec4cbd219a2cd19bb1bcf2a5c4019f556e4304b1
Reviewed-on: https://go-review.googlesource.com/c/go/+/451436
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This makes it easier to use the package string prefix in some cases
(cleanup).
Change-Id: I0ae74bf8770999110e7d6e49eac4e42962e78596
Reviewed-on: https://go-review.googlesource.com/c/go/+/451795
Auto-Submit: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
lfstack does very unsafe things. In particular, it will not
work with nodes that live on the heap. In normal use by the runtime,
that is the case (it is only used for gc work bufs). But the lfstack
test does use heap objects. It goes through some hoops to prevent
premature deallocation, but those hoops are not enough to convince
-d=checkptr that everything is ok.
Instead, allocate the test objects outside the heap, like the runtime
does for all of its lfstack usage. Remove the lifetime workaround
from the test.
Reported in https://groups.google.com/g/golang-nuts/c/psjrUV2ZKyI
Change-Id: If611105eab6c823a4d6c105938ce145ed731781d
Reviewed-on: https://go-review.googlesource.com/c/go/+/448899
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
When /etc/nsswitch.conf lists: "hosts: files" then LookupHost returns two nils when no entry inside /etc/hosts is found.
Change-Id: I96d68a079dfe009655c84cf0e697ce19a5bb6698
GitHub-Last-Rev: 894f066bbc
GitHub-Pull-Request: golang/go#56747
Reviewed-on: https://go-review.googlesource.com/c/go/+/450875
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Joedian Reid <joedian@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
For a while now, we've had intermittent reports about problems with
os/exec on macOS, but no clear way to reproduce them. Recent changes
in the os/exec package test seem to have aligned the stars just right,
at least on my two x86 and ARM MacBook Pro laptops, to make the
package test hang with roughly 50% probability. When it does hang, the
stacks I see in the hung process match the ones reported for the
Go-based hangs in #33565. (They do not match the ones reported in the
so-called C reproducer in that issue, but I think that reproducer is
actually reproducing a different race, between fork and exit.)
The stacks obtained from the hung child processes are in
libSystem_atfork_child, which is supposed to reinitialize various
parts of the C library in the new process.
One common stack dies in _notify_fork_child calling _notify_globals
(inlined) calling _os_alloc_once, because _os_alloc_once detects that
the once lock is held by the parent process and then calls
_os_once_gate_corruption_abort. The allocation is setting up the
globals for the notification subsystem. See the source code at [1].
To work around this, we can allocate the globals earlier in the Go
program's lifetime, before any execs are involved, by calling any
notify routine that is exported, calls _notify_globals, and doesn't do
anything too expensive otherwise. notify_is_valid_token(0) fits the bill.
The other common stack dies in xpc_atfork_child calling
_objc_msgSend_uncached which ends up in
WAITING_FOR_ANOTHER_THREAD_TO_FINISH_CALLING_+initialize. Of course,
whatever thread the child is waiting for is in the parent process and
is not going to finish anything in the child process. There is no
public source code for these routines, so it is unclear exactly what
the problem is. However, xpc_atfork_child turns out to be exported
(for use by libSystem_atfork_child, which is in a different library,
so xpc_atfork_child is unlikely to be unexported any time soon).
It also stands to reason that since xpc_atfork_child is called at the
start of any forked child process, it can't be too harmful to call at
the start of an ordinary Go process. And whatever caches it needs for
a non-deadlocking fast path during exec empirically do get initialized
by calling it at startup.
This CL introduces a function osinit_hack, called at osinit time,
which calls notify_is_valid_token(0) and xpc_atfork_child().
Doing so makes the os/exec test pass reliably on both my laptops -
I can run it successfully hundreds of times in a row when my previous
record was twice in a row.
Fixes#33565.
Fixes#56784.
[1] https://opensource.apple.com/source/Libnotify/Libnotify-241/notify_client.c.auto.html
Change-Id: I16a14a800893c40244678203532a3e8d6214b6bd
Reviewed-on: https://go-review.googlesource.com/c/go/+/451735
Run-TryBot: Russ Cox <rsc@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Currently for every CPU profile sample, we apply its weight to all
call edges of the entire call stack. Frames higher up the stack
are unlikely to be repeated calls (e.g. runtime.main calling
main.main). So adding weights to call edges higher up the stack
may be not reflecting the actual call edge weights in the program.
This CL changes it to add weights to only the edge between the
last two frames.
Without a branch profile (e.g. LBR records) this is not perfect,
but seems more reasonable.
Change-Id: I0aee75cc608a152adad41c51120b661a6c542283
Reviewed-on: https://go-review.googlesource.com/c/go/+/450915
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Currently, we use CDF to compute a weight threshold and then use
the weight threshold to determine whether a call site is hot. As
when we compute the CDF we already have a list of hot call sites
that make up the given percentage of the CDF, just use that list.
Also, when computing the CDF threshold, include the very last node
that makes it to go over the threshold. (I.e. if the CDF threshold
is 50% and one hot node takes 60% of weight, we should include that
node instead of excluding it. In practice it rarely matters,
probably only for testing and micro-benchmarks.)
Change-Id: I535ae9cd6b679609e247c3d0d9ee572c1a1187cc
Reviewed-on: https://go-review.googlesource.com/c/go/+/450737
Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com>
For implementing interface to empty interface conversion, the compiler
generate code like:
var res *uint8
res = itab
if res != nil {
res = res.type
}
However, itab has type *uintptr, so the assignment is broken. The
problem is not shown up, until CL 450215, which call typecheck on this
broken assignment.
To fix this, just cast itab to *uint8 when doing the conversion.
Fixes#56768
Change-Id: Id42792d18e7f382578b40854d46eecd49673792c
Reviewed-on: https://go-review.googlesource.com/c/go/+/451256
Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
