[gofrontend] runtime: avoid libc memmove and memclr

1 view

Skip to first unread message

Ian Lance Taylor (Gerrit)

unread,

Jul 8, 2025, 6:47:51 PMJul 8

to Ian Lance Taylor, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Than McIntosh, Dmitri Shuralyov, Dmitri Shuralyov, Cherry Mui, golang-co...@googlegroups.com

Ian Lance Taylor submitted the change with unreviewed changes

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: libgo/runtime/go-memmove.c
Insertions: 30, Deletions: 45.

@@ -16,71 +16,56 @@
 // moves.
 
 void
-gomemmove (void *dst, void *src, uintptr len)
+gomemmove(void *dst, void *src, uintptr len)
 {
   const uintptr ptr_size = sizeof(dst);
-  uintptr offset, tail;
+  uintptr tail;
   uintptr rem;
   uintptr dwords;
   uintptr i;
   char *bdst, *bsrc;
 
-  rem = len;
-
   if (len == 0) {
-	return;
+    return;
   }
 
-  // If src and dst don't have the same pointer alignment then
-  // there is no issue with copying pointer atomicity. Use the
-  // builtin.
-  if (((uintptr)dst % ptr_size) != ((uintptr)src % ptr_size) || len < ptr_size) {
-	__builtin_memmove(dst, src, len);
-	return;
+  // We expect pointer-containing values to be pointer-aligned.
+  // If these pointers are not aligned, they don't contain pointers.
+  if ((uintptr)dst % ptr_size != 0 || (uintptr)src % ptr_size != 0 || len < ptr_size) {
+    __builtin_memmove(dst, src, len);
+    return;
   }
 
-  // Length >= ptr_size && same ptr alignment
-  offset = (uintptr)dst % ptr_size;
+  bdst = (char*)dst;
+  bsrc = (char*)src;
 
-  // If not pointer alignment, move the intial bytes.
-  if (offset > 0) {
-	__builtin_memmove(dst, src, ptr_size-offset);
-	dst += (ptr_size-offset);
-	src += (ptr_size-offset);
-	rem -= (ptr_size-offset);
-  }
-
-  // Move the tail bytes to make the backward move
-  // easier.
+  // Move the tail bytes to make the backward move easier.
+  rem = len;
   tail = rem % ptr_size;
   if (tail > 0) {
-	__builtin_memmove(dst+rem-tail, src+rem-tail, tail);
-	rem -= tail;
-  }
-
-  if (rem == 0) {
-	return;
+    __builtin_memmove(bdst+rem-tail, bsrc+rem-tail, tail);
+    rem -= tail;
   }
 
   // Must now be pointer alignment and rem is multiple of ptr_size.
-  dwords = len / ptr_size;
+  dwords = rem / ptr_size;
 
-  // Determine if a backwards move is needed
-  // Forward or backward, move all doublewords
+  // Determine if a backwards move is needed.
+  // Forward or backward, move all words.
 
-  if ((uintptr)(dst - src) < rem) {
-	bdst = dst+rem-ptr_size;
-	bsrc = src+rem-ptr_size;
-	for (i = 0; i<dwords; i++) {
-		*(uintptr*)bdst = *(uintptr*)bsrc;
-		bdst -= ptr_size;
-		bsrc -= ptr_size;
-	}
+  if ((uintptr)(bdst - bsrc) < rem) {
+    bdst += rem - ptr_size;
+    bsrc += rem - ptr_size;
+    for (i = 0; i<dwords; i++) {
+      *(uintptr*)bdst = *(uintptr*)bsrc;
+      bdst -= ptr_size;
+      bsrc -= ptr_size;
+    }
   } else {
-	for (i = 0; i<dwords; i++) {
-		*(uintptr*)dst = *(uintptr*)src;
-		dst += ptr_size;
-		src += ptr_size;
-	}
+    for (i = 0; i<dwords; i++) {
+      *(uintptr*)bdst = *(uintptr*)bsrc;
+      bdst += ptr_size;
+      bsrc += ptr_size;
+    }
   }
 }
```
```
The name of the file: libgo/runtime/go-memclr.c
Insertions: 7, Deletions: 14.

@@ -11,7 +11,7 @@
   __attribute__ ((no_split_stack));
 
 void
-memclrNoHeapPointers (void *p1, uintptr len)
+memclrNoHeapPointers(void *p1, uintptr len)
 {
   const uintptr ptr_size = sizeof(p1);
   uintptr rem,drem,i;
@@ -24,20 +24,13 @@
   rem = len;
 
   offset = (uintptr)p1 % ptr_size;
-  // This memset is OK since it can't contain
-  // an pointer aligned pointer.
-  if ((rem < ptr_size) || (offset > 0 && offset+rem <= 2*ptr_size)) {
+  if (rem < ptr_size || offset > 0) {
+    // This memset is OK since it can't contain
+    // an pointer aligned pointer.
     __builtin_memset(p1, 0, rem);
     return;
   }
-  // Move initial bytes to get to pointer boundary
-  if (offset > 0) {
-    __builtin_memset(p1, 0, ptr_size-offset);
-    p1 = (void*)((char*)p1+ptr_size-offset);
-    rem -= ptr_size-offset;
-  }
 
-  // If at least pointer-sized bytes left, clear
   drem = rem / ptr_size;
 
   vp = (volatile uintptr*)(p1);
@@ -48,9 +41,9 @@
     vp++;
     rem -= ptr_size;
   }
-  p1 = (void*)((char*)p1 + ptr_size*drem);
-  // Clear any remaining
+  // Clear any remaining bytes.
   if (rem > 0) {
-    __builtin_memset (p1, 0, rem);
+    p1 = (void*)((char*)p1 + ptr_size*drem);
+    __builtin_memset(p1, 0, rem);
   }
 }
```

Change information

Commit message:

runtime: avoid libc memmove and memclr

The libc memmove and memclr don't reliably operate on full memory words.
We already avoided them on PPC64, but the same problem can occur even
on x86, where some processors use "rep movsb" and "rep stosb".
Always use C code that stores full memory words.

While we're here, clean up the C code. We don't need special handling
if the memmove/memclr pointers are not pointer-aligned.

Unfortunately, this will likely be slower. Perhaps some day we can
have our own assembly code that operates a word at a time,
or we can use different operations when we know there are no pointers.

Change-Id: I87f66331cc04aa07cb28f59cbc16810dc903c049

Reviewed-on: https://go-review.googlesource.com/c/gofrontend/+/685178

Reviewed-by: Dmitri Shuralyov <dmit...@google.com>

Reviewed-by: Than McIntosh <th...@golang.org>

Reviewed-by: Cherry Mui <cher...@google.com>

Files:

M libgo/runtime/go-memclr.c
M libgo/runtime/go-memmove.c

Change size: M

Delta: 2 files changed, 53 insertions(+), 82 deletions(-)

Branch: refs/heads/master

Submit Requirements:

Code-Review: +1 by Than McIntosh, +2 by Cherry Mui, +1 by Dmitri Shuralyov

Open in Gerrit

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

open

diffy

satisfied_requirement

Reply all

Reply to author

Forward

0 new messages