Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runtime: nanotime1 is not reentrant #40697

Closed
cherrymui opened this issue Aug 11, 2020 · 2 comments
Closed

runtime: nanotime1 is not reentrant #40697

cherrymui opened this issue Aug 11, 2020 · 2 comments

Comments

@cherrymui
Copy link
Contributor

@cherrymui cherrymui commented Aug 11, 2020

What version of Go are you using (go version)?

tip (up to ba9e108)

Does this issue reproduce with the latest release?

Not sure.

What operating system and processor architecture are you using (go env)?

linux/amd64

What did you do?

In the runtime, on platforms that uses VDSO, nanotime1 (and walltime1) is not reentrant. At entry it saves the current PC and SP to m.vdsoPC and m.vdsoSP, and clears m.vdsoSP at exit. If a signal lands in between, and nanotime1 is called in the signal handler (e.g. sigprof calling cpuProfile.add calling nanotime1), it will clear m.vdsoSP at exit. Now, if it receives another signal, it will observe m.vdsoSP is zero and assumed it is not in VDSO and so okay to unwind the stack, which is not true, and the stack unwinding code will crash. This could happen in the rare situation where two (or more) profiling signals land in one nanotime1 (or walltime1) execution.

CL https://golang.org/cl/246763 addresses this, by making nanotime1 (and walltime1) reentrant.

The risk of that CL: if some register is accidentally clobbered, it will crash the runtime, and essentially any Go program won't run. Or all times will go completely off. Neither of them seems happening.

@cherrymui
Copy link
Contributor Author

@cherrymui cherrymui commented Aug 11, 2020

Closing as it is fixed via CL 246763 (commit a93a4c1).

@cherrymui cherrymui closed this Aug 11, 2020
@andybons andybons added this to the Go1.15 milestone Aug 11, 2020
@gopherbot
Copy link

@gopherbot gopherbot commented Aug 11, 2020

Change https://golang.org/cl/247900 mentions this issue: [release-branch.go1.15] all: merge master into release-branch.go1.15

gopherbot pushed a commit that referenced this issue Aug 11, 2020
5c7748d doc/go1.15: encoding/json's CL 191783 was reverted
5ff5b3c doc/go1.15: remove draft notice
5ae1d62 CONTRIBUTORS: update for the Go 1.15 release
7ad776d doc/go1.15: document crypto/tls permanent error
a93a4c1 runtime: make nanotime1 reentrant

Updates #40697

Change-Id: Ie39896ee6304544cc9e9c1938bdf176f1dcf8766
Reviewed-on: https://go-review.googlesource.com/c/go/+/247900
Run-TryBot: Andrew Bonventre <andybons@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.