[go] internal/runtime/maps: document hashing of stack pointers
4 views
Skip to first unread message
Keith Randall (Gerrit)
May 18, 2026, 12:06:54 PMMay 18
to Arseny Samoylov, Michael Pratt, goph...@pubsubhelper.golang.org, Keith Randall, golang-co...@googlegroups.com
Attention needed from Arseny Samoylov and Michael Pratt
Keith Randall has uploaded the change for review![]( "Open in Gerrit")
Keith Randall would like Arseny Samoylov and Michael Pratt to review this change.
Commit message
internal/runtime/maps: document hashing of stack pointers
CL 776281 makes it a bit clearer that we're doing pointer to uintptr
conversion on pointer keys. That sent me down something of a rabbit
hole trying to figure out how that actually could work, given stack
copies could happen after hashing but before the core map operation.
Turns out it does work, for subtle reasons. Documenting here.
Change-Id: I49158c44254f57ca56c7e1ec23c337d80e46ed6a
Change diff
diff --git a/src/cmd/compile/internal/escape/assign.go b/src/cmd/compile/internal/escape/assign.go
index 6af5388..27f13ca 100644
--- a/src/cmd/compile/internal/escape/assign.go
+++ b/src/cmd/compile/internal/escape/assign.go
@@ -50,6 +50,8 @@
case ir.OINDEXMAP:
n := n.(*ir.IndexExpr)
e.discard(n.X)
+ // Keys used in map assignments must escape.
+ // See "Hashing Pointers" doc in internal/runtime/maps/map.go.
e.assignHeap(n.Index, "key of map put", n)
}
diff --git a/src/cmd/compile/internal/escape/call.go b/src/cmd/compile/internal/escape/call.go
index 074edd9..9fa7fbc 100644
--- a/src/cmd/compile/internal/escape/call.go
+++ b/src/cmd/compile/internal/escape/call.go
@@ -209,6 +209,8 @@
e.discard(arg)
}
e.discard(call.RType)
+ // Note: keys used in map deletes do not need to escape.
+ // See "Hashing Pointers" doc in internal/runtime/maps/map.go.
case ir.OMIN, ir.OMAX:
call := call.(*ir.CallExpr)
diff --git a/src/cmd/compile/internal/escape/expr.go b/src/cmd/compile/internal/escape/expr.go
index 1521c2e..468c471 100644
--- a/src/cmd/compile/internal/escape/expr.go
+++ b/src/cmd/compile/internal/escape/expr.go
@@ -90,6 +90,8 @@
case ir.OINDEXMAP:
n := n.(*ir.IndexExpr)
e.discard(n.X)
+ // Keys used in map lookups do not need to escape.
+ // See "Hashing Pointers" doc in internal/runtime/maps/map.go.
e.discard(n.Index)
case ir.OSLICE, ir.OSLICEARR, ir.OSLICE3, ir.OSLICE3ARR, ir.OSLICESTR:
n := n.(*ir.SliceExpr)
diff --git a/src/internal/runtime/maps/map.go b/src/internal/runtime/maps/map.go
index 0a0dd359..e6f7a83 100644
--- a/src/internal/runtime/maps/map.go
+++ b/src/internal/runtime/maps/map.go
@@ -177,6 +177,24 @@
// For (b), we must adjust the current directory index when the directory
// grows. This is more straightforward, as the directory orders remains the
// same after grow, so we just double the index if the directory size doubles.
+//
+// Hashing Pointers
+//
+// Keys in Go maps can be pointers, or contain pointers. The hash of
+// a pointer is a somewhat tricky concept, as pointers to stack
+// objects can change during a stack copy. Because we hash a pointer
+// by just hashing its uintptr-converted value, the hash of a key can
+// potentially become stale across any stack copy.
+//
+// For keys that are stored into maps, we must avoid this. All key
+// arguments to map assignments must be marked as escaping so that the
+// hash of the key in the map is stable. This is true even when the
+// map itself does not escape and can live on the stack.
+//
+// For keys that are used for lookup (or delete), it turns out that
+// escaping is not required. If we are looking up a pointer which
+// points to the stack, the hash value is ~irrelevant, as the key is
+// guaranteed to not be in the map (due to the previous paragraph).
// Extracts the H1 portion of a hash: the 57 upper bits.
// TODO(prattmic): what about 32-bit systems?
Change information
Files:
- M src/cmd/compile/internal/escape/assign.go
- M src/cmd/compile/internal/escape/call.go
- M src/cmd/compile/internal/escape/expr.go
- M src/internal/runtime/maps/map.go
Change size: S
Delta: 4 files changed, 24 insertions(+), 0 deletions(-)
Related details
Attention is currently required from:
- Arseny Samoylov
- Michael Pratt
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Arseny Samoylov (Gerrit)
May 18, 2026, 12:48:35 PMMay 18
to Keith Randall, goph...@pubsubhelper.golang.org, golang...@luci-project-accounts.iam.gserviceaccount.com, Michael Pratt, golang-co...@googlegroups.com
Attention needed from Keith Randall and Michael Pratt
Arseny Samoylov voted and added 2 comments![]( "Open in Gerrit")
Votes added by Arseny Samoylov
| Code-Review | +1 |
2 comments
Patchset-level comments
File-level comment, Patchset 1 (Latest):
Arseny Samoylov . resolved
This looks great! This very non-trivial detail that definitely worth documenting.
File src/internal/runtime/maps/map.go
Line 189, Patchset 1 (Latest):
// For keys that are stored into maps, we must avoid this. All key
// arguments to map assignments must be marked as escaping so that the
// hash of the key in the map is stable. This is true even when the
// map itself does not escape and can live on the stack.Arseny Samoylov . unresolved
I think this could be rephrased slightly.
On first read, I interpreted this as saying that the key itself must escape to the heap, even if the map lives on the stack. But actually it the object that the key references must escape to the heap. The key itself doesn't need to escape.
And with that it makes sense that map[ptr] can be on stack and store keys (pointers to heap allocated objects) on the stack.
Related details
Attention is currently required from:
- Keith Randall
- Michael Pratt
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Keith Randall (Gerrit)
May 18, 2026, 8:43:05 PMMay 18
to Keith Randall, goph...@pubsubhelper.golang.org, Arseny Samoylov, golang...@luci-project-accounts.iam.gserviceaccount.com, Michael Pratt, golang-co...@googlegroups.com
Attention needed from Arseny Samoylov and Michael Pratt
Keith Randall added 1 comment![]( "Open in Gerrit")
File src/internal/runtime/maps/map.go
Line 189, Patchset 1 (Latest):
// For keys that are stored into maps, we must avoid this. All key
// arguments to map assignments must be marked as escaping so that the
// hash of the key in the map is stable. This is true even when the
// map itself does not escape and can live on the stack.Arseny Samoylov . resolved
I think this could be rephrased slightly.
On first read, I interpreted this as saying that the key itself must escape to the heap, even if the map lives on the stack. But actually it the object that the key references must escape to the heap. The key itself doesn't need to escape.
And with that it makes sense that map[ptr] can be on stack and store keys (pointers to heap allocated objects) on the stack.
Keith Randall
I always confuse myself about whether escaping applies to the *allocation*, or to the *pointer* to the allocation.
Wikipedia generally talks like it is the *pointer* that escapes, although parts of that page contradict itself: https://en.wikipedia.org/wiki/Escape_analysis
I think our code generally applies it to the allocation itself.
I'll reword to make it clearer.
Related details
Attention is currently required from:
- Arseny Samoylov
- Michael Pratt
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Keith Randall (Gerrit)
May 18, 2026, 8:44:45 PMMay 18
to Keith Randall, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Arseny Samoylov and Michael Pratt
Keith Randall uploaded new patchset![]( "Open in Gerrit")
Keith Randall uploaded patch set #2 to this change.
Following approvals got outdated and were removed:
- TryBots-Pass: LUCI-TryBot-Result+1 by golang...@luci-project-accounts.iam.gserviceaccount.com
Related details
Attention is currently required from:
- Arseny Samoylov
- Michael Pratt
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Arseny Samoylov (Gerrit)
Jun 24, 2026, 10:32:18 AM (7 days ago) Jun 24
to Keith Randall, goph...@pubsubhelper.golang.org, golang...@luci-project-accounts.iam.gserviceaccount.com, Michael Pratt, golang-co...@googlegroups.com
Attention needed from Keith Randall and Michael Pratt
Arseny Samoylov voted and added 1 comment![]( "Open in Gerrit")
Votes added by Arseny Samoylov
| Code-Review | +1 |
1 comment
Patchset-level comments
File-level comment, Patchset 2 (Latest):
Arseny Samoylov . resolved
Gentle ping. This CL seems to have slipped out of sight a bit.
Related details
Attention is currently required from:
- Keith Randall
- Michael Pratt
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Pratt (Gerrit)
Jun 30, 2026, 3:12:56 PM (9 hours ago) Jun 30
to Keith Randall, goph...@pubsubhelper.golang.org, Michael Pratt, golang...@luci-project-accounts.iam.gserviceaccount.com, Arseny Samoylov, golang-co...@googlegroups.com
Attention needed from Keith Randall
Michael Pratt voted Code-Review+2![]( "Open in Gerrit")
| Code-Review | +2 |
Related details
Attention is currently required from:
- Keith Randall
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Keith Randall (Gerrit)
Jun 30, 2026, 5:13:27 PM (7 hours ago) Jun 30
to Keith Randall, goph...@pubsubhelper.golang.org, Michael Pratt, golang...@luci-project-accounts.iam.gserviceaccount.com, Arseny Samoylov, golang-co...@googlegroups.com
Attention needed from Keith Randall
Keith Randall voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
Attention is currently required from:
- Keith Randall
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Keith Randall (Gerrit)
Jun 30, 2026, 5:13:34 PM (7 hours ago) Jun 30
to Keith Randall, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Michael Pratt, golang...@luci-project-accounts.iam.gserviceaccount.com, Arseny Samoylov, golang-co...@googlegroups.com
Keith Randall submitted the change![]( "Open in Gerrit")
Change information
Commit message:
internal/runtime/maps: document hashing of stack pointers
CL 776281 makes it a bit clearer that we're doing pointer to uintptr
conversion on pointer keys. That sent me down something of a rabbit
hole trying to figure out how that actually could work, given stack
copies could happen after hashing but before the core map operation.
Turns out it does work, for subtle reasons. Documenting here.
Change-Id: I49158c44254f57ca56c7e1ec23c337d80e46ed6a
Reviewed-on: https://go-review.googlesource.com/c/go/+/779240
LUCI-TryBot-Result: golang...@luci-project-accounts.iam.gserviceaccount.com <golang...@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Arseny Samoylov <samoylo...@gmail.com>
Reviewed-by: Michael Pratt <mpr...@google.com>
Reviewed-by: Keith Randall <k...@google.com>
Files:
- M src/cmd/compile/internal/escape/assign.go
- M src/cmd/compile/internal/escape/call.go
- M src/cmd/compile/internal/escape/expr.go
- M src/internal/runtime/maps/map.go
Change size: S
Delta: 4 files changed, 25 insertions(+), 0 deletions(-)
Branch: refs/heads/master
Submit Requirements:
Code-Review: +1 by Arseny Samoylov, +2 by Michael Pratt, +1 by Keith Randall
TryBots-Pass: LUCI-TryBot-Result+1 by golang...@luci-project-accounts.iam.gserviceaccount.com
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages