Skip to content

runtime: fix panic if newstack at runtime.acquireLockRank #40844

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

runtime: fix panic if newstack at runtime.acquireLockRank #40844

wants to merge 1 commit into from
+10 −0

Conversation

chainhelen
Copy link
Contributor

@chainhelen chainhelen commented Aug 17, 2020 •
edited
Loading

Process may crash becaues acquireLockRank and releaseLockRank may
be called in nosplit context. With optimizations and inlining
disabled, these functions won't get inlined or have their morestack
calls eliminated.
Nosplit is not strictly required for lockWithRank, unlockWithRank
and lockWithRankMayAcquire, just keep consistency with lockrank_on.go
here.

Fixes #40843

@googlebot googlebot added the cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change. label Aug 17, 2020
@chainhelen chainhelen mentioned this pull request Aug 17, 2020
Closed
@gopherbot
Copy link
Contributor

This PR (HEAD: 3cad25e) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/248878 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@chainhelen chainhelen mentioned this pull request Aug 17, 2020
Closed
@gopherbot
Copy link
Contributor

Message from Michael Pratt:

Patch Set 1: Code-Review-1

(7 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/248878.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Michael Pratt:

Patch Set 1: Run-TryBot+1

Please don’t reply on this GitHub thread. Visit golang.org/cl/248878.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 1:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=ecebfc00

Please don’t reply on this GitHub thread. Visit golang.org/cl/248878.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 1: TryBot-Result+1

TryBots are happy.

Please don’t reply on this GitHub thread. Visit golang.org/cl/248878.
After addressing review feedback, remember to publish your drafts!

@chainhelen chainhelen force-pushed the acquireLockRank_panic branch from 3cad25e to e37bf92 Compare August 18, 2020 06:14
@gopherbot
Copy link
Contributor

This PR (HEAD: e37bf92) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/248878 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@chainhelen chainhelen force-pushed the acquireLockRank_panic branch from e37bf92 to 19948db Compare August 18, 2020 06:37
@gopherbot
Copy link
Contributor

This PR (HEAD: 19948db) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/248878 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@chainhelen chainhelen changed the title runtime: Fix panic if newstack at runtime.acquireLockRank runtime: fix panic if newstack at runtime.acquireLockRank Aug 18, 2020
@gopherbot
Copy link
Contributor

Message from chainhelen:

Patch Set 4:

(7 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/248878.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Michael Pratt:

Patch Set 4: Run-TryBot+1

Please don’t reply on this GitHub thread. Visit golang.org/cl/248878.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 4:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=afcf1f79

Please don’t reply on this GitHub thread. Visit golang.org/cl/248878.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 4: TryBot-Result+1

TryBots are happy.

Please don’t reply on this GitHub thread. Visit golang.org/cl/248878.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Austin Clements:

Patch Set 4:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/248878.
After addressing review feedback, remember to publish your drafts!

@chainhelen chainhelen force-pushed the acquireLockRank_panic branch from 19948db to 9f29912 Compare August 21, 2020 07:47
@gopherbot
Copy link
Contributor

This PR (HEAD: 9f29912) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/248878 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from chainhelen:

Patch Set 5:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/248878.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Austin Clements:

Patch Set 5:

(3 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/248878.
After addressing review feedback, remember to publish your drafts!

@chainhelen
runtime: fix panic if newstack at runtime.acquireLockRank
38fd3cc 
Process may crash becaues acquireLockRank and releaseLockRank may
be called in nosplit context. With optimizations and inlining
disabled, these functions won't get inlined or have their morestack
calls eliminated.
Nosplit is not strictly required for lockWithRank, unlockWithRank
and lockWithRankMayAcquire, just keep consistency with lockrank_on.go
here.

Fixes golang#40843
@chainhelen chainhelen force-pushed the acquireLockRank_panic branch from 9f29912 to 38fd3cc Compare August 21, 2020 16:43
@gopherbot
Copy link
Contributor

This PR (HEAD: 38fd3cc) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/248878 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from chainhelen:

Patch Set 6:

(3 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/248878.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Dan Scales:

Patch Set 6: Code-Review+2

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/248878.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Emmanuel Odeke:

Patch Set 6: Run-TryBot+1

Please don’t reply on this GitHub thread. Visit golang.org/cl/248878.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 6:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=10846ade

Please don’t reply on this GitHub thread. Visit golang.org/cl/248878.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 6: TryBot-Result+1

TryBots are happy.

Please don’t reply on this GitHub thread. Visit golang.org/cl/248878.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from chainhelen:

Patch Set 6:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/248878.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Dmitri Shuralyov:

Patch Set 6:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/248878.
After addressing review feedback, remember to publish your drafts!

gopherbot pushed a commit that referenced this pull request Sep 1, 2020
@chainhelen @danscales
runtime: fix panic if newstack at runtime.acquireLockRank
b246c0e 
Process may crash becaues acquireLockRank and releaseLockRank may
be called in nosplit context. With optimizations and inlining
disabled, these functions won't get inlined or have their morestack
calls eliminated.
Nosplit is not strictly required for lockWithRank, unlockWithRank
and lockWithRankMayAcquire, just keep consistency with lockrank_on.go
here.

Fixes #40843

Change-Id: I5824119f98a1da66d767cdb9a60dffe768f13c81
GitHub-Last-Rev: 38fd3cc
GitHub-Pull-Request: #40844
Reviewed-on: https://go-review.googlesource.com/c/go/+/248878
Reviewed-by: Dan Scales <[email protected]>
Run-TryBot: Emmanuel Odeke <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
@gopherbot
Copy link
Contributor

This PR is being closed because golang.org/cl/248878 has been merged.

@gopherbot gopherbot closed this Sep 1, 2020
gopherbot pushed a commit that referenced this pull request Sep 2, 2020
@chainhelen @dmitshur
[release-branch.go1.15] runtime: fix panic if newstack at runtime.acq…
a269e5f 
…uireLockRank

Process may crash becaues acquireLockRank and releaseLockRank may
be called in nosplit context. With optimizations and inlining
disabled, these functions won't get inlined or have their morestack
calls eliminated.
Nosplit is not strictly required for lockWithRank, unlockWithRank
and lockWithRankMayAcquire, just keep consistency with lockrank_on.go
here.

Updates #40843.
Fixes #40845.

Change-Id: I5824119f98a1da66d767cdb9a60dffe768f13c81
GitHub-Last-Rev: 38fd3cc
GitHub-Pull-Request: #40844
Reviewed-on: https://go-review.googlesource.com/c/go/+/248878
Reviewed-by: Dan Scales <[email protected]>
Run-TryBot: Emmanuel Odeke <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
(cherry picked from commit b246c0e)
Reviewed-on: https://go-review.googlesource.com/c/go/+/252339
Run-TryBot: Dmitri Shuralyov <[email protected]>
claucece pushed a commit to claucece/go that referenced this pull request Oct 22, 2020
@chainhelen @claucece
[release-branch.go1.15] runtime: fix panic if newstack at runtime.acq…
5900a2e 
…uireLockRank

Process may crash becaues acquireLockRank and releaseLockRank may
be called in nosplit context. With optimizations and inlining
disabled, these functions won't get inlined or have their morestack
calls eliminated.
Nosplit is not strictly required for lockWithRank, unlockWithRank
and lockWithRankMayAcquire, just keep consistency with lockrank_on.go
here.

Updates golang#40843.
Fixes golang#40845.

Change-Id: I5824119f98a1da66d767cdb9a60dffe768f13c81
GitHub-Last-Rev: 38fd3cc
GitHub-Pull-Request: golang#40844
Reviewed-on: https://go-review.googlesource.com/c/go/+/248878
Reviewed-by: Dan Scales <[email protected]>
Run-TryBot: Emmanuel Odeke <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
(cherry picked from commit b246c0e)
Reviewed-on: https://go-review.googlesource.com/c/go/+/252339
Run-TryBot: Dmitri Shuralyov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

runtime: Panic if newstack at runtime.acquireLockRank
3 participants
@chainhelen @gopherbot @googlebot