[go] runtime: on android/arm64, don't read outside 16-byte regions
3 views
Skip to first unread message
Keith Randall (Gerrit)
Feb 25, 2026, 7:07:22 PM (2 days ago) Feb 25
to Michael Pratt, goph...@pubsubhelper.golang.org, Keith Randall, golang-co...@googlegroups.com
Attention needed from Michael Pratt
Keith Randall has uploaded the change for review![]( "Open in Gerrit")
Keith Randall would like Michael Pratt to review this change.
Commit message
runtime: on android/arm64, don't read outside 16-byte regions
Because MTE might be enforced.
Update #59090
Update #27610
Change-Id: Idfaecbf3b7a93c5e371abcace666febfc303de9a
Change diff
diff --git a/src/internal/bytealg/indexbyte_arm64.s b/src/internal/bytealg/indexbyte_arm64.s
index 92a61a4..1101122ec 100644
--- a/src/internal/bytealg/indexbyte_arm64.s
+++ b/src/internal/bytealg/indexbyte_arm64.s
@@ -34,6 +34,9 @@
// identify exactly which byte has matched.
CBZ R1, fail
+#ifdef GOOS_android
+ ADD R0, R1, R20 // R20 = end of data
+#endif
MOVD R0, R11
// Magic constant 0x40100401 allows us to identify
// which lane matches the requested byte.
@@ -51,7 +54,17 @@
// Input string is not 32-byte aligned. We calculate the
// syndrome value for the aligned 32 bytes block containing
// the first bytes and mask off the irrelevant part.
+#ifdef GOOS_android
+ // Android requires us to not read outside an aligned 16-byte
+ // region because MTE might be enforced.
+ VLD1.P (R3), [V1.B16]
+ CMP R3, R20
+ BLS 2(PC)
+ VLD1 (R3), [V2.B16]
+ ADD $0x10, R3
+#else
VLD1.P (R3), [V1.B16, V2.B16]
+#endif
SUB $0x20, R9, R4
ADDS R4, R1, R1
VCMEQ V0.B16, V1.B16, V3.B16
@@ -71,7 +84,15 @@
CBNZ R6, tail
loop:
+#ifdef GOOS_android
+ VLD1.P (R3), [V1.B16]
+ CMP R3, R20
+ BLS 2(PC)
+ VLD1 (R3), [V2.B16]
+ ADD $0x10, R3
+#else
VLD1.P (R3), [V1.B16, V2.B16]
+#endif
SUBS $0x20, R1, R1
VCMEQ V0.B16, V1.B16, V3.B16
VCMEQ V0.B16, V2.B16, V4.B16
diff --git a/src/runtime/string.go b/src/runtime/string.go
index 15b3868..b911c0c 100644
--- a/src/runtime/string.go
+++ b/src/runtime/string.go
@@ -8,6 +8,7 @@
"internal/abi"
"internal/bytealg"
"internal/goarch"
+ "internal/goos"
"internal/runtime/math"
"internal/runtime/sys"
"internal/strconv"
@@ -505,7 +506,9 @@
// It must be the minimum page size for any architecture Go
// runs on. It's okay (just a minor performance loss) if the
// actual system page size is larger than this value.
- const pageSize = 4096
+ // For Android, we set the page size to the MTE size, as MTE
+ // might be enforced. See issue 59090.
+ const pageSize = 4096 - 4080*goos.IsAndroid
offset := 0
ptr := unsafe.Pointer(s)
Change information
Files:
- M src/internal/bytealg/indexbyte_arm64.s
- M src/runtime/string.go
Change size: S
Delta: 2 files changed, 25 insertions(+), 1 deletion(-)
Related details
Attention is currently required from:
- 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)
Feb 25, 2026, 7:29:25 PM (2 days ago) Feb 25
to Keith Randall, goph...@pubsubhelper.golang.org, Go LUCI, Michael Pratt, golang-co...@googlegroups.com
Attention needed from Michael Pratt
Keith Randall voted and added 1 comment![]( "Open in Gerrit")
Votes added by Keith Randall
| Commit-Queue | +1 |
1 comment
Patchset-level comments
File-level comment, Patchset 1 (Latest):
Keith Randall . resolved
I looked through internal/bytealg/*_arm64.s and runtime/*_arm64.s and I think this is the only case where we read outside the passed-in ranges. So this CL might be a reasonably complete fix for MTE compatibility.
Related details
Attention is currently required from:
- 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)
Feb 25, 2026, 8:15:43 PM (2 days ago) Feb 25
to Keith Randall, goph...@pubsubhelper.golang.org, Go LUCI, Michael Pratt, golang-co...@googlegroups.com
Attention needed from Michael Pratt
Keith Randall voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Attention is currently required from:
- 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)
Feb 26, 2026, 6:15:40 PM (yesterday) Feb 26
to Keith Randall, goph...@pubsubhelper.golang.org, Go LUCI, Michael Pratt, golang-co...@googlegroups.com
Attention needed from Keith Randall
Michael Pratt added 3 comments![]( "Open in Gerrit")
Commit Message
Line 12, Patchset 1 (Latest):
Update #27610Michael Pratt . resolved
I assume that this patch won't make valgrind happy, since it still reads beyond the end of the object, just limited to MTE's minimum resolution.
File src/internal/bytealg/indexbyte_arm64.s
Line 60, Patchset 1 (Latest):
VLD1.P (R3), [V1.B16]Michael Pratt . resolved
I'm not too familiar with arm64 vector instructions or our arm64 assembly syntax, so just to check, this is the post-index variant, which writes R3+16 back to R3?
Line 64, Patchset 1 (Latest):
ADD $0x10, R3Michael Pratt . unresolved
Why add manually instead of using VLD1.P?
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)
Feb 26, 2026, 7:47:29 PM (23 hours ago) Feb 26
to Keith Randall, goph...@pubsubhelper.golang.org, Go LUCI, Michael Pratt, golang-co...@googlegroups.com
Attention needed from Michael Pratt
Keith Randall added 1 comment![]( "Open in Gerrit")
File src/internal/bytealg/indexbyte_arm64.s
Line 64, Patchset 1 (Latest):
ADD $0x10, R3Michael Pratt . resolved
Why add manually instead of using VLD1.P?
Keith Randall
We still need to do the ADD $16 even when we don't do the load, so they have to be separate. (We could use VLD1.P, but then we need a separate ADD $16 somewhere else.)
Related details
Attention is currently required from:
- 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)
4:02 PM (2 hours ago) 4:02 PM
to Keith Randall, goph...@pubsubhelper.golang.org, Michael Pratt, Go LUCI, golang-co...@googlegroups.com
Attention needed from Keith Randall
Michael Pratt voted and added 2 comments![]( "Open in Gerrit")
Votes added by Michael Pratt
| Code-Review | +2 |
2 comments
File src/internal/bytealg/indexbyte_arm64.s
Line 64, Patchset 1 (Latest):
ADD $0x10, R3Michael Pratt . resolved
Keith RandallWhy add manually instead of using VLD1.P?
We still need to do the ADD $16 even when we don't do the load, so they have to be separate. (We could use VLD1.P, but then we need a separate ADD $16 somewhere else.)
Michael Pratt
Ah, right, thanks.
File src/runtime/string.go
Line 511, Patchset 1 (Latest):
const pageSize = 4096 - 4080*goos.IsAndroidMichael Pratt . unresolved
This is pretty confusing. IMO it would be clearer as
```suggestion
const pageSize = 4096*(1-goos.IsAndroid) + 16*goos.IsAndroid
```
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)
5:23 PM (1 hour ago) 5:23 PM
to Keith Randall, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Keith Randall
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 Go LUCI
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)
5:23 PM (1 hour ago) 5:23 PM
to Keith Randall, goph...@pubsubhelper.golang.org, Michael Pratt, Go LUCI, golang-co...@googlegroups.com
Keith Randall voted and added 1 comment![]( "Open in Gerrit")
Votes added by Keith Randall
| Auto-Submit | +1 |
1 comment
File src/runtime/string.go
This is pretty confusing. IMO it would be clearer as
```suggestion
const pageSize = 4096*(1-goos.IsAndroid) + 16*goos.IsAndroid
```
Keith Randall
Sure, done.
Related details
Attention set is empty
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)
5:23 PM (1 hour ago) 5:23 PM
to Keith Randall, goph...@pubsubhelper.golang.org, Michael Pratt, Go LUCI, golang-co...@googlegroups.com
Attention needed from Keith Randall
Keith Randall voted Code-Review+1![]( "Open in Gerrit")
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. |
Gopher Robot (Gerrit)
5:43 PM (1 hour ago) 5:43 PM
to Keith Randall, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Go LUCI, Keith Randall, Michael Pratt, golang-co...@googlegroups.com
Gopher Robot submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
1 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: src/runtime/string.go
Insertions: 1, Deletions: 1.
@@ -508,7 +508,7 @@
// actual system page size is larger than this value.
// For Android, we set the page size to the MTE size, as MTE
// might be enforced. See issue 59090.
- const pageSize = 4096 - 4080*goos.IsAndroid
+ const pageSize = 4096*(1-goos.IsAndroid) + 16*goos.IsAndroid
offset := 0
ptr := unsafe.Pointer(s)
```
Change information
Commit message:
runtime: on android/arm64, don't read outside 16-byte regions
Because MTE might be enforced.
Update #59090
Update #27610
Change-Id: Idfaecbf3b7a93c5e371abcace666febfc303de9a
Reviewed-on: https://go-review.googlesource.com/c/go/+/749062
Reviewed-by: Michael Pratt <mpr...@google.com>
Auto-Submit: Keith Randall <k...@golang.org>
LUCI-TryBot-Result: Go LUCI <golang...@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <k...@google.com>
Files:
- M src/internal/bytealg/indexbyte_arm64.s
- M src/runtime/string.go
Change size: S
Delta: 2 files changed, 25 insertions(+), 1 deletion(-)
Branch: refs/heads/master
Submit Requirements:
Code-Review: +1 by Keith Randall, +2 by Michael Pratt
TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages