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/race: simplify meta shadow mapping #24133

Open
aclements opened this issue Feb 26, 2018 · 3 comments

Comments

@aclements
Copy link
Member

commented Feb 26, 2018

Currently, the race detector runtime for Go assumes that the heap is contiguous and grows up. In particular, MapShadow always maps the meta shadow contiguously. According to @dvyukov's comment, this was done to deal with the fact that the Go heap grew irregularly and the meta shadow mapping has large alignment constraints.

As of 2b41554, the Go heap is no longer contiguous and doesn't necessarily grow up. However, it also now grows in large, aligned blocks (as of 7884647, 4MB on Windows, 64MB elsewhere). This easily satisfies the alignment constraints on the meta shadow. Hence, the meta shadow mapping code can be simplified to take advantage of this while at the same time adding support for Go's new sparse heap layout.

@dgryski

This comment has been minimized.

Copy link
Contributor

commented Feb 26, 2018

This bug description sounds like the race detector is broken with tip since the sparse heap switch. Is it actually broken or just sub-optimal?

@aclements

This comment has been minimized.

Copy link
Member Author

commented Feb 26, 2018

It's mostly not broken. :) In the unlikely event that the runtime encounters another memory mapping when it's attempting to grow the heap linearly, it will skip forward by 4GB in the virtual address space, which right now will cause the race detector to consume an extra 2GB of memory (which it will never touch and doesn't matter on Unix-like OSes, but which Windows charges the process for). And this could happen multiple times (though that's even more unlikely). If this happens 32 times, then it could genuinely break because the heap could start to grow outside of the area required by the race detector, though if it happens 32 times something is probably terribly wrong already.

@gopherbot

This comment has been minimized.

Copy link

commented Apr 4, 2018

Change https://golang.org/cl/104717 mentions this issue: runtime: stop when we run out of hints in race mode

gopherbot pushed a commit that referenced this issue Apr 4, 2018
runtime: stop when we run out of hints in race mode
Currently, the runtime falls back to asking for any address the OS can
offer for the heap when it runs out of hint addresses. However, the
race detector assumes the heap lives in [0x00c000000000,
0x00e000000000), and will fail in a non-obvious way if we go outside
this region.

Fix this by actively throwing a useful error if we run out of heap
hints in race mode.

This problem is currently being triggered by TestArenaCollision, which
intentionally triggers this fallback behavior. Fix the test to look
for the new panic message in race mode.

Fixes #24670.
Updates #24133.

Change-Id: I57de6d17a3495dc1f1f84afc382cd18a6efc2bf7
Reviewed-on: https://go-review.googlesource.com/104717
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

@ianlancetaylor ianlancetaylor modified the milestones: Go1.11, Unplanned Jun 28, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.