- 25 Oct, 2019 19 commits
-
-
Austin Clements authored
Currently, gcscanvalid is used to resolve a race between attempts to scan a stack. Now that there's a clear owner of the stack scan operation, there's no longer any danger of racing or attempting to scan a stack more than once, so this CL eliminates gcscanvalid. I double-checked my reasoning by first adding a throw if gcscanvalid was set in scanstack and verifying that all.bash still passed. For #10958, #24543. Fixes #24363. Change-Id: I76794a5fcda325ed7cfc2b545e2a839b8b3bc713 Reviewed-on: https://go-review.googlesource.com/c/go/+/201139 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
Cherry Zhang <cherryyz@google.com>
-
Austin Clements authored
This removes scang and preemptscan, since the stack scanning code now uses suspendG. For #10958, #24543. Change-Id: Ic868bf5d6dcce40662a82cb27bb996cb74d0720e Reviewed-on: https://go-review.googlesource.com/c/go/+/201138 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
-
Austin Clements authored
Currently, the process of suspending a goroutine is tied to stack scanning. In preparation for non-cooperative preemption, this CL abstracts this into general purpose suspendG/resumeG functions. suspendG and resumeG closely follow the existing scang and restartg functions with one exception: the addition of a _Gpreempted status. Currently, preemption tasks (stack scanning) are carried out by the target goroutine if it's in _Grunning. In this new approach, the task is always carried out by the goroutine that called suspendG. Thus, we need a reliable way to drive the target goroutine out of _Grunning until the requesting goroutine is ready to resume it. The new _Gpreempted state provides the handshake: when a runnable goroutine responds to a preemption request, it now parks itself and enters _Gpreempted. The requesting goroutine races to put it in _Gwaiting, which gives it ownership, but also the responsibility to start it again. This CL adds several TODOs about improving the synchronization on the G status. The existing code already has these problems; we're just taking note of them. The next CL will remove the now-dead scang and preemptscan. For #10958, #24543. Change-Id: I16dbf87bea9d50399cc86719c156f48e67198f16 Reviewed-on: https://go-review.googlesource.com/c/go/+/201137 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
-
pokutuna authored
Fixes #35161 Updates #34439 Change-Id: I978534cbb8b9fb32c115dba0066cf099c61d8ee9 GitHub-Last-Rev: d60581635e8cefb7cfc4b571057542395034c575 GitHub-Pull-Request: golang/go#35162 Reviewed-on: https://go-review.googlesource.com/c/go/+/203478Reviewed-by:
Bryan C. Mills <bcmills@google.com> Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
-
Bryan C. Mills authored
mod_get_svn passes, and I also tested this manually on a real-world svn-hosted package: example.com$ go mod init example.com go: creating new go.mod: module example.com example.com$ GOPROXY=direct GONOSUMDB=llvm.org go get -d llvm.org/llvm/bindings/go/llvm go: finding llvm.org/llvm latest go: finding llvm.org/llvm/bindings/go/llvm latest go: downloading llvm.org/llvm v0.0.0-20191022153947-000000375505 go: extracting llvm.org/llvm v0.0.0-20191022153947-000000375505 example.com$ go list llvm.org/llvm/bindings/... llvm.org/llvm/bindings/go llvm.org/llvm/bindings/go/llvm Fixes #26092 Change-Id: Iefe2151b82a0225c73bb6f8dd7cd8a352897d4c0 Reviewed-on: https://go-review.googlesource.com/c/go/+/203497 Run-TryBot: Bryan C. Mills <bcmills@google.com> Reviewed-by:
Jay Conrod <jayconrod@google.com>
-
Tobias Klauser authored
CL 198544 broke the linux/arm64 build because it declares emptyfunc for GOARCH=arm64, but only freebsd/arm64 defines it. Make it a static assembly function specific for freebsd/arm64 and remove the stub. Fixes #35160 Change-Id: I5fd94249b60c6fd259c251407b6eccc8fa512934 Reviewed-on: https://go-review.googlesource.com/c/go/+/203418Reviewed-by:
-
Bryan C. Mills authored
bradfitz is actively thinking about a proper fix. In the meantime, skip the test to suss out any other failures in the builder. Updates #35122 Change-Id: I9bf0640222e3d385c1a3e2be5ab52b80d3e8c21a Reviewed-on: https://go-review.googlesource.com/c/go/+/203500 Run-TryBot: Bryan C. Mills <bcmills@google.com> Reviewed-by:
Brad Fitzpatrick <bradfitz@golang.org>
-
Bryan C. Mills authored
Previously, TestAtomicStop used a hard-coded 2-second timeout. That empirically is not long enough on certain builders. Rather than adjusting it to a different arbitrary value, use a slice of the overall timeout for the test binary. If everything is working, we won't block nearly that long anyway. Updates #35085 Change-Id: I7b789388e3152413395088088fc497419976cf5c Reviewed-on: https://go-review.googlesource.com/c/go/+/203499 Run-TryBot: Bryan C. Mills <bcmills@google.com> Reviewed-by:
-
Tobias Klauser authored
Updates #24715 Change-Id: I110a10a5d5ed4a471f67f35cbcdcbea296c5dcaf Reviewed-on: https://go-review.googlesource.com/c/go/+/198542 Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
Ian Lance Taylor <iant@golang.org>
-
Tobias Klauser authored
Based on work by Mikaël Urankar (@MikaelUrankar), Shigeru YAMAMOTO (@bsd-hacker) and @myfreeweb. Updates #24715 Change-Id: If3189a693ca0aa627029e22b0f91534bcf322bc0 Reviewed-on: https://go-review.googlesource.com/c/go/+/198544 Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
-
Austin Clements authored
We already claim on the documentation for _Grunning that this is case, but execute transitions to _Grunning before assigning g.m. Fix this and make the documentation even more explicit. For #10958, #24543, but also a good cleanup. Change-Id: I1eb0108e7762f55cfb0282aca624af1c0a15fe56 Reviewed-on: https://go-review.googlesource.com/c/go/+/201440 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
-
Austin Clements authored
Currently, shrinkstack will not shrink a stack on Windows if gp.m.libcallsp != 0. In general, we can't shrink stacks in syscalls because the syscall may hold pointers into the stack, and in principle this is supposed to be preventing that for libcall-based syscalls (which are direct syscalls from the runtime). But this test is actually broken and has been for a long time. That turns out to be okay because it also appears it's not necessary. This test is racy. g.m points to whatever M the G was last running on, even if the G is in a blocked state, and that M could be doing anything, including making libcalls. Hence, observing that libcallsp == 0 at one moment in shrinkstack is no guarantee that it won't become non-zero while we're shrinking the stack, and vice-versa. It's also weird that this check is only performed on Windows, given that we now use libcalls on macOS, Solaris, and AIX. This check was added when stack shrinking was first implemented in CL 69580044. The history of that CL (though not the final version) suggests this was necessary for libcalls that happened on Go user stacks, which we never do now because of the limited stack space. It could also be defending against user stack pointers passed to libcall system calls from blocked Gs. But the runtime isn't allowed to keep pointers into the user stack for blocked Gs on any OS, so it's not clear this would be of any value. Hence, this checks seems to be simply unnecessary. Rather than simply remove it, this CL makes it defensive. We can't do anything about blocked Gs, since it doesn't even make sense to look at their M, but if a G tries to shrink its own stack while in a libcall, that indicates a bug in the libcall code. This CL makes shrinkstack panic in this case. For #10958, #24543, since those are going to rearrange how we decide that it's safe to shrink a stack. Change-Id: Ia865e1f6340cff26637f8d513970f9ebb4735c6d Reviewed-on: https://go-review.googlesource.com/c/go/+/173724 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
-
Austin Clements authored
The x86 assembler supports an "ADJSP" pseudo-op that compiles to an ADD/SUB from SP. Unfortunately, while this seems perfect for an instruction that would allow obj to continue to track the SP/FP delta, obj currently doesn't do that. As a result, FP-relative references won't work and, perhaps worse, the pcsp table will have the wrong frame size. We don't currently use this instruction in any assembly or generate it in the compiler, but this is a perfect instruction for solving a problem in #24543. This CL makes ADJSP useful by incorporating it into the SP delta logic. One subtlety is that we do generate ADJSP in obj itself to open a function's stack frame. Currently, when preprocess enters the loop to compute the SP delta, it may or may not start at this ADJSP instruction depending on various factors. We clean this up by instead always starting the SP delta at 0 and always starting this loop at the entry to the function. Why not just recognize ADD/SUB of SP? The danger is that could change the meaning of existing code. For example, walltime1 in sys_linux_amd64.s saves SP, SUBs from it, and aligns it. Later, it restores the saved copy and then does a few FP-relative references. Currently obj doesn't know any of this is happening, but that's fine once it gets to the FP-relative references. If we taught obj to recognize the SUB, it would start to miscompile this code. An alternative would be to recognize unknown instructions that write to SP and refuse subsequent FP-relative references, but that's kind of annoying. This passes toolstash -cmp for std on both amd64 and 386. Change-Id: Ic6c6a7cbf980bca904576676c07b44c0aaa9c82d Reviewed-on: https://go-review.googlesource.com/c/go/+/200877 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
David Chase <drchase@google.com>
-
ZYunH authored
Error string should not be capitalized. Change-Id: I8e1d148c6b999450bcd702f420c2a240f82aadc7 GitHub-Last-Rev: 6ca1b3edb4a61723fa6472a0f54cc6329898edbc GitHub-Pull-Request: golang/go#35147 Reviewed-on: https://go-review.googlesource.com/c/go/+/203339Reviewed-by:
-
Meng Zhuo authored
Change-Id: I625f0bc533a7d14010c0344f36e8f157a19c13f2 Reviewed-on: https://go-review.googlesource.com/c/go/+/203437 Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com> Reviewed-by:
Tobias Klauser <tobias.klauser@gmail.com>
-
Meng Zhuo authored
Change-Id: I60a1ca606fe7492c05697c4d58afc7f19fcc63fe Reviewed-on: https://go-review.googlesource.com/c/go/+/203340Reviewed-by:
-
Jason A. Donenfeld authored
According to MSDN, "If the data has the REG_SZ, REG_MULTI_SZ or REG_EXPAND_SZ type, this size includes any terminating null character or characters unless the data was stored without them. [...] If the data has the REG_SZ, REG_MULTI_SZ or REG_EXPAND_SZ type, the string may not have been stored with the proper terminating null characters. Therefore, even if the function returns ERROR_SUCCESS, the application should ensure that the string is properly terminated before using it; otherwise, it may overwrite a buffer." It's therefore dangerous to pass it off unbounded as we do, and in fact this led to crashes on real systems. Change-Id: I6d786211814656f036b87fd78631466634cd764a Reviewed-on: https://go-review.googlesource.com/c/go/+/202937 Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
Alex Brainman <alex.brainman@gmail.com>
-
Rob Pike authored
There were a couple of bugs, including not requiring a percent and returning the wrong error for a bad format containing %%. Both are addressed by fixing the first. Fixes #34180. Change-Id: If96c0c0258bcb95eec49871437d719cb9d399d9b Reviewed-on: https://go-review.googlesource.com/c/go/+/202879 Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
Robert Griesemer <gri@golang.org>
-
Rémy Oudompheng authored
This notably allows to reuse temporaries across the karatsubaSqr recursion. benchmark old ns/op new ns/op delta BenchmarkNatMul/10-4 227 228 +0.44% BenchmarkNatMul/100-4 8339 8589 +3.00% BenchmarkNatMul/1000-4 313796 312272 -0.49% BenchmarkNatMul/10000-4 11924720 11873589 -0.43% BenchmarkNatMul/100000-4 503813354 503839058 +0.01% BenchmarkNatSqr/20-4 549 513 -6.56% BenchmarkNatSqr/30-4 945 874 -7.51% BenchmarkNatSqr/50-4 1993 1832 -8.08% BenchmarkNatSqr/80-4 4096 3874 -5.42% BenchmarkNatSqr/100-4 6192 5712 -7.75% BenchmarkNatSqr/200-4 20388 19543 -4.14% BenchmarkNatSqr/300-4 38735 36715 -5.21% BenchmarkNatSqr/500-4 99562 93542 -6.05% BenchmarkNatSqr/800-4 195554 184907 -5.44% BenchmarkNatSqr/1000-4 286302 275053 -3.93% BenchmarkNatSqr/10000-4 9817057 9441641 -3.82% BenchmarkNatSqr/100000-4 390713416 379696789 -2.82% benchmark old allocs new allocs delta BenchmarkNatMul/10-4 1 1 +0.00% BenchmarkNatMul/100-4 1 1 +0.00% BenchmarkNatMul/1000-4 2 1 -50.00% BenchmarkNatMul/10000-4 2 1 -50.00% BenchmarkNatMul/100000-4 9 11 +22.22% BenchmarkNatSqr/20-4 2 1 -50.00% BenchmarkNatSqr/30-4 2 1 -50.00% BenchmarkNatSqr/50-4 2 1 -50.00% BenchmarkNatSqr/80-4 2 1 -50.00% BenchmarkNatSqr/100-4 2 1 -50.00% BenchmarkNatSqr/200-4 2 1 -50.00% BenchmarkNatSqr/300-4 4 1 -75.00% BenchmarkNatSqr/500-4 4 1 -75.00% BenchmarkNatSqr/800-4 10 1 -90.00% BenchmarkNatSqr/1000-4 10 1 -90.00% BenchmarkNatSqr/10000-4 731 1 -99.86% BenchmarkNatSqr/100000-4 19687 6 -99.97% benchmark old bytes new bytes delta BenchmarkNatMul/10-4 192 192 +0.00% BenchmarkNatMul/100-4 4864 4864 +0.00% BenchmarkNatMul/1000-4 57344 49224 -14.16% BenchmarkNatMul/10000-4 565248 498772 -11.76% BenchmarkNatMul/100000-4 5749504 7263720 +26.34% BenchmarkNatSqr/20-4 672 352 -47.62% BenchmarkNatSqr/30-4 992 512 -48.39% BenchmarkNatSqr/50-4 1792 896 -50.00% BenchmarkNatSqr/80-4 2688 1408 -47.62% BenchmarkNatSqr/100-4 3584 1792 -50.00% BenchmarkNatSqr/200-4 6656 3456 -48.08% BenchmarkNatSqr/300-4 24448 16387 -32.97% BenchmarkNatSqr/500-4 36864 24591 -33.29% BenchmarkNatSqr/800-4 69760 40981 -41.25% BenchmarkNatSqr/1000-4 86016 49180 -42.82% BenchmarkNatSqr/10000-4 2524800 487368 -80.70% BenchmarkNatSqr/100000-4 68599808 5876581 -91.43% Change-Id: I8e6e409ae1cb48be9d5aa9b5f428d6cbe487673a Reviewed-on: https://go-review.googlesource.com/c/go/+/172017 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
-
- 24 Oct, 2019 15 commits
-
-
Bryan C. Mills authored
Previously, codehost.Repo.ReadZip returned an 'actualSubdir' value that was the empty string in all current implementations. Updates #26092 Change-Id: I6708dd0f13ba88bcf1a1fb405e9d818fd6f9197e Reviewed-on: https://go-review.googlesource.com/c/go/+/203277 Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
-
Jay Conrod authored
This change adds the -modfile flag to module aware build commands and to 'go mod' subcommands. -modfile may be set to a path to an alternate go.mod file to be read and written. A real go.mod file must still exist and is used to set the module root directory. However, it is not opened. When -modfile is set, the effective location of the go.sum file is also changed to the -modfile with the ".mod" suffix trimmed (if present) and ".sum" added. Updates #34506 Change-Id: I2d1e044e18af55505a4f24bbff09b73bb9c908b4 Reviewed-on: https://go-review.googlesource.com/c/go/+/202564 Run-TryBot: Jay Conrod <jayconrod@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
-
Jay Conrod authored
base.Cwd should be used instead. Change-Id: I3dbdecf745b0823160984cc942c883dc04c91d7b Reviewed-on: https://go-review.googlesource.com/c/go/+/203037 Run-TryBot: Jay Conrod <jayconrod@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
-
Bryan C. Mills authored
Change-Id: I241a3bbaf9c4b779b74146232d2f144bb46a0dc7 Reviewed-on: https://go-review.googlesource.com/c/go/+/203178 Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
-
Jay Conrod authored
Sections will be filled in with individual CLs before Go 1.14. NOTE: This document is currently in Markdown for ease of writing / reviewing. Before Go 1.14, we will either ensure that x/website can render Markdown (flavor TBD) or check in a rendered HTML file that can be displayed directly. Updates #33637 Change-Id: Icd43fa2bdb7d256b28a56b93214b70343f43492e Reviewed-on: https://go-review.googlesource.com/c/go/+/202081Reviewed-by:
-
Bryan C. Mills authored
I had prohibited 'go list -m' with -mod=vendor because the module graph is incomplete, but I've realized that many queries do not actually require the full graph — and may, in fact, be driven using modules previously reported by 'go list' for specific, vendored packages. Queries for those modules should succeed. Updates #33848 Change-Id: I1000b4cf586a830bb78faf620ebf62d73a3cb300 Reviewed-on: https://go-review.googlesource.com/c/go/+/203138 Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
-
Bryan C. Mills authored
Updates #33848 Change-Id: I10b4c79faef8bc3dee2ceba14d496fa049e84fb2 Reviewed-on: https://go-review.googlesource.com/c/go/+/202977 Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
-
Cuong Manh Le authored
Correct comment about allocating big enough slice to copy result of Getdirentries. While at it, also convert from Dirent directly to slice of byte. Updates #35092 Change-Id: I892de7953120622882e1561728e1e56b009a2351 Reviewed-on: https://go-review.googlesource.com/c/go/+/202880 Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
-
Dan Scales authored
Generate inline code at defer time to save the args of defer calls to unique (autotmp) stack slots, and generate inline code at exit time to check which defer calls were made and make the associated function/method/interface calls. We remember that a particular defer statement was reached by storing in the deferBits variable (always stored on the stack). At exit time, we check the bits of the deferBits variable to determine which defer function calls to make (in reverse order). These low-cost defers are only used for functions where no defers appear in loops. In addition, we don't do these low-cost defers if there are too many defer statements or too many exits in a function (to limit code increase). When a function uses open-coded defers, we produce extra FUNCDATA_OpenCodedDeferInfo information that specifies the number of defers, and for each defer, the stack slots where the closure and associated args have been stored. The funcdata also includes the location of the deferBits variable. Therefore, for panics, we can use this funcdata to determine exactly which defers are active, and call the appropriate functions/methods/closures with the correct arguments for each active defer. In order to unwind the stack correctly after a recover(), we need to add an extra code segment to functions with open-coded defers that simply calls deferreturn() and returns. This segment is not reachable by the normal function, but is returned to by the runtime during recovery. We set the liveness information of this deferreturn() to be the same as the liveness at the first function call during the last defer exit code (so all return values and all stack slots needed by the defer calls will be live). I needed to increase the stackguard constant from 880 to 896, because of a small amount of new code in deferreturn(). The -N flag disables open-coded defers. '-d defer' prints out the kind of defer being used at each defer statement (heap-allocated, stack-allocated, or open-coded). Cost of defer statement [ go test -run NONE -bench BenchmarkDefer$ runtime ] With normal (stack-allocated) defers only: 35.4 ns/op With open-coded defers: 5.6 ns/op Cost of function call alone (remove defer keyword): 4.4 ns/op Text size increase (including funcdata) for go binary without/with open-coded defers: 0.09% The average size increase (including funcdata) for only the functions that use open-coded defers is 1.1%. The cost of a panic followed by a recover got noticeably slower, since panic processing now requires a scan of the stack for open-coded defer frames. This scan is required, even if no frames are using open-coded defers: Cost of panic and recover [ go test -run NONE -bench BenchmarkPanicRecover runtime ] Without open-coded defers: 62.0 ns/op With open-coded defers: 255 ns/op A CGO Go-to-C-to-Go benchmark got noticeably faster because of open-coded defers: CGO Go-to-C-to-Go benchmark [cd misc/cgo/test; go test -run NONE -bench BenchmarkCGoCallback ] Without open-coded defers: 443 ns/op With open-coded defers: 347 ns/op Updates #14939 (defer performance) Updates #34481 (design doc) Change-Id: I63b1a60d1ebf28126f55ee9fd7ecffe9cb23d1ff Reviewed-on: https://go-review.googlesource.com/c/go/+/202340Reviewed-by:
Austin Clements <austin@google.com>
-
Bryan C. Mills authored
The default value of cfg.BuildMod depends on the 'go' version in the go.mod file. The go.mod file is read and parsed, and its settings are applied, in modload.InitMod. As it turns out, modload.Enabled does not invoke InitMod, so cfg.BuildMod is not necessarily set even if modload.Enabled returns true. Updates #33848 Change-Id: I13a4dd80730528e6f1a5acc492fcfe07cb59d94e Reviewed-on: https://go-review.googlesource.com/c/go/+/202917 Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
-
Cuong Manh Le authored
Change-Id: Ic809a533f9c4042373bdad3ba1cd237d203bacff Reviewed-on: https://go-review.googlesource.com/c/go/+/202881Reviewed-by:
-
Cuong Manh Le authored
Fixes #35092 Change-Id: I8f1ee2b79d42b2291548fd5645940a61f6d67582 Reviewed-on: https://go-review.googlesource.com/c/go/+/202878 Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
Matthew Dempsky <mdempsky@google.com>
-
Brad Fitzpatrick authored
Fixes Darwin. Updates #35092 Change-Id: I045f070c8549d00610b459e3a82cac870d9ddb54 Reviewed-on: https://go-review.googlesource.com/c/go/+/203077 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by:
Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by:
-
Robert Griesemer authored
A Rat is represented via a quotient a/b where a and b are Int values. To make it possible to use an uninitialized Rat value (with a and b uninitialized and thus == 0), the implementation treats a 0 denominator as 1. Rat.Num and Rat.Denom return pointers to these values a and b. Because b may be 0, Rat.Denom used to first initialize it to 1 and thus produce an undesirable side-effect (by changing the Rat's denominator). This CL changes Denom to return a new (not shared) *Int with value 1 in the rare case where the Rat was not initialized. This eliminates the side effect and returns the correct denominator value. While this is changing behavior of the API, the impact should now be minor because together with (prior) CL https://golang.org/cl/202997, which initializes Rats ASAP, Denom is unlikely used to access the denominator of an uninitialized (and thus 0) Rat. Any operation that will somehow set a Rat value will ensure that the denominator is not 0. Fixes #33792. Updates #3521. Change-Id: I0bf15ac60513cf52162bfb62440817ba36f0c3fc Reviewed-on: https://go-review.googlesource.com/c/go/+/203059 Run-TryBot: Robert Griesemer <gri@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
-
Robert Griesemer authored
A Rat is represented via a quotient a/b where a and b are Int values. To make it possible to use an uninitialized Rat value (with a and b uninitialized and thus == 0), the implementation treats a 0 denominator as 1. For each operation we check if the denominator is 0, and then treat it as 1 (if necessary). Operations that create a new Rat result, normalize that value such that a result denominator 1 is represened as 0 again. This CL changes this behavior slightly: 0 denominators are still interpreted as 1, but whenever we (safely) can, we set an uninitialized 0 denominator to 1. This simplifies the code overall. Also: Improved some doc strings. Preparation for addressing issue #33792. Updates #33792. Change-Id: I3040587c8d0dad2e840022f96ca027d8470878a0 Reviewed-on: https://go-review.googlesource.com/c/go/+/202997 Run-TryBot: Robert Griesemer <gri@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
-
- 23 Oct, 2019 6 commits
-
-
Cherry Zhang authored
On ARM and ARM64, during a VDSO call, the g register may be temporarily clobbered by the VDSO code. If a signal is received during the execution of VDSO code, we may not find a valid g reading the g register. In CL 192937, we conservatively assume g is nil. But this approach has a problem: we cannot handle the signal in this case. Further, if the signal is not a profiling signal, we'll call badsignal, which calls needm, which wants to get an extra m, but we don't have one in a non-cgo binary, which cuases the program to hang. This is even more of a problem with async preemption, where we will receive more signals than before. I ran into this problem while working on async preemption support on ARM64. In this CL, before making a VDSO call, we save the g on the gsignal stack. When we receive a signal, we will be running on the gsignal stack, so we can fetch the g from there and move on. We probably want to do the same for PPC64. Currently we rely on that the VDSO code doesn't actually clobber the g register, but this is not guaranteed and we don't have control with. Idea from discussion with Dan Cross and Austin. Should fix #34391. Change-Id: Idbefc5e4c2f4373192c2be797be0140ae08b26e3 Reviewed-on: https://go-review.googlesource.com/c/go/+/202759 Run-TryBot: Cherry Zhang <cherryyz@google.com> Reviewed-by:
-
Rohan Verma authored
Fixes #35099 Change-Id: Ieaf3174540087bd20443b38703ef88d9f9638052 GitHub-Last-Rev: 12973bc66f39caebb4156396de3b9df670bf12b5 GitHub-Pull-Request: golang/go#35123 Reviewed-on: https://go-review.googlesource.com/c/go/+/202998Reviewed-by:
Dmitri Shuralyov <dmitshur@golang.org> Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
-
Dmitri Shuralyov authored
benchcmp was moved out of misc into x/tools in CL 60100043 in 2014, and then replaced by a forwarding script in CL 82710043. Five years have since passed, and the forwarding script has outlived its usefulness. It's now more confusing than helpful. Delete it. Change-Id: I8c7d65b97e0b3fe367df69a86ae10c7960c05be3 Reviewed-on: https://go-review.googlesource.com/c/go/+/202762Reviewed-by:
-
Jason A. Donenfeld authored
It turns out that Windows has "legitimate" keys that have bogus type values or bogus lengths that don't correspond with their type. On up to date Windows 10 systems, this test always fails for this reason. These keys exist because of bugs in Microsoft's code. This commit works around the problem by simply blacklisting known instances. It also expands the error message a bit so that we can make adjustments should the problem ever happen again, and reformats the messages so that it makes copy and pasting into the blacklist easier. Updates #35084 Change-Id: I50322828c0eb0ccecbb62d6bf4f9c726fa0b3c27 Reviewed-on: https://go-review.googlesource.com/c/go/+/202897 Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com> Reviewed-by:
-
Fazlul Shahriar authored
Update #25234 Fixes #35083 Change-Id: Ida39516ab1c14a34a62c2232476a75e83f4e3f75 Reviewed-on: https://go-review.googlesource.com/c/go/+/202657Reviewed-by:
-
Jay Conrod authored
Since CL 194600, search.CleanPaths preserves characters after '@' in each argument. This was done so that paths could be cleaned while version queries were preserved. However, local and absolute file paths may contain '@' characters. With this change, '@' is treated as a normal character by search.CleanPaths in local and absolute paths. Fixes #35115 Change-Id: Ia7d37e0a2737442d4f1796cc2fc3a59237a8ddfe Reviewed-on: https://go-review.googlesource.com/c/go/+/202761 Run-TryBot: Jay Conrod <jayconrod@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by:
-
