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: TestArenaCollision failing on Solaris #23862

Closed
aclements opened this issue Feb 15, 2018 · 6 comments

Comments

Projects
None yet
3 participants
@aclements
Copy link
Member

commented Feb 15, 2018

2b41554 broke Solaris. For example, https://build.golang.org/log/d32bb2fc9ace29dcdddd34f0df6361a4a2978fbb:

--- FAIL: TestArenaCollision (0.06s)
	malloc_test.go:175: === RUN   TestArenaCollision
		runtime: memory allocated by OS [0xfffffd7ff0000000, 0xfffffd7ff4000000) exceeds address space limit (0x1000000000000)
		fatal error: memory reservation exceeds address space limit

Apparently Solaris can allocate user address space in the upper portion of the amd64 virtual address space.

@aclements aclements self-assigned this Feb 15, 2018

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Feb 16, 2018

There is a diagram of the Solaris memory map at http://www.oracle.com/technetwork/server-storage/solaris/solaris-memory-135224.html . I haven't looked at this test, but Solaris will put the stack and shared objects above 0xFFFF8000 00000000, and mmap can return address up there too. Solaris doesn't get around the 48-bit limit, it just sign extends.

@aclements

This comment has been minimized.

Copy link
Member Author

commented Feb 16, 2018

Thanks for the pointer; that's consistent with the builder failure. I'm already mapping enough metadata for 48 bit addresses because of arm64. I'll just have to close the hole either by masking off the sign extension or adding to rotate the hole around.

@gopherbot

This comment has been minimized.

Copy link

commented Feb 20, 2018

Change https://golang.org/cl/95497 mentions this issue: runtime: offset the heap arena index by 2^47 on amd64

@gopherbot

This comment has been minimized.

Copy link

commented Feb 20, 2018

Change https://golang.org/cl/95496 mentions this issue: runtime: abstract indexing of arena index

@gopherbot

This comment has been minimized.

Copy link

commented Feb 20, 2018

Change https://golang.org/cl/95495 mentions this issue: runtime: simplify bulkBarrierPreWrite

@gopherbot

This comment has been minimized.

Copy link

commented Feb 20, 2018

Change https://golang.org/cl/95498 mentions this issue: runtime: clarify address space limit constants and comments

gopherbot pushed a commit that referenced this issue Feb 21, 2018

runtime: simplify bulkBarrierPreWrite
Currently, bulkBarrierPreWrite uses inheap to decide whether the
destination is in the heap or whether to check for stack or global
data. However, this isn't the best question to ask.

Instead, get the span directly and query its state. This lets us
directly determine whether this might be a global, or is stack memory,
or is heap memory.

At this point, inheap is no longer used in the hot path, so drop it
from the must-be-inlined list and substitute spanOf.

This will help in a circuitous way with #23862, since fixing that is
going to push inheap very slightly over the inline-able threshold on a
few platforms.

Change-Id: I5360fc1181183598502409f12979899e1e4d45f7
Reviewed-on: https://go-review.googlesource.com/95495
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>

gopherbot pushed a commit that referenced this issue Feb 21, 2018

runtime: abstract indexing of arena index
Accessing the arena index is about to get slightly more complicated.
Abstract this away into a set of functions for going back and forth
between addresses and arena slice indexes.

For #23862.

Change-Id: I0b20e74ef47a07b78ed0cf0a6128afe6f6e40f4b
Reviewed-on: https://go-review.googlesource.com/95496
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>

@gopherbot gopherbot closed this in ed1959c Feb 21, 2018

gopherbot pushed a commit that referenced this issue Feb 21, 2018

runtime: clarify address space limit constants and comments
Now that we support the full non-contiguous virtual address space of
amd64 hardware, some of the comments and constants related to this are
out of date.

This renames memLimitBits to heapAddrBits because 1<<memLimitBits is
no longer the limit of the address space and rewrites the comment to
focus first on hardware limits (which span OSes) and then discuss
kernel limits.

Second, this eliminates the memLimit constant because there's no
longer a meaningful "highest possible heap pointer value" on amd64.

Updates #23862.

Change-Id: I44b32033d2deb6b69248fb8dda14fc0e65c47f11
Reviewed-on: https://go-review.googlesource.com/95498
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>

@golang golang locked and limited conversation to collaborators Feb 21, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.