libgo patch committed: Use a C function to call mmap
Checks
Commit Message
This libgo patches changes the runtime pacakge to use a C function to call mmap.
The final argument to mmap, of type off_t, varies. In
https://go.dev/cl/445375
(https://gcc.gnu.org/pipermail/gcc-patches/2022-October/604158.html)
we changed it to always use the C off_t type, but that broke 32-bit
big-endian Linux systems. On those systems, using the C off_t type
requires calling the mmap64 function. In C this is automatically
handled by the <sys/mman.h> file. In Go, we would have to change the
magic //extern comment to call mmap64 when appropriate. Rather than
try to get that right, we instead go through a C function that uses C
implicit type conversions to pick the right type.
This fixes https://gcc.gnu.org/PR110297.
Bootstrapped and tested on x86_64-pc-linux-gnu and
powerpc-pc-linux-gnu (32-bit and 64-bit). Committed to trunk and GCC
13 branch.
Ian
55557f5a6c8a27190daf9daadf5e9f14ef5f4ece
Comments
On Jun 20 2023, Ian Lance Taylor via Gcc-patches wrote:
> This libgo patches changes the runtime pacakge to use a C function to call mmap.
>
> The final argument to mmap, of type off_t, varies. In
> https://go.dev/cl/445375
> (https://gcc.gnu.org/pipermail/gcc-patches/2022-October/604158.html)
> we changed it to always use the C off_t type, but that broke 32-bit
> big-endian Linux systems.
This has nothing to do with big-endian, armv7 isn't big-endian.
On Tue, Jun 20, 2023 at 11:35 AM Andreas Schwab <schwab@linux-m68k.org> wrote:
>
> On Jun 20 2023, Ian Lance Taylor via Gcc-patches wrote:
>
> > This libgo patches changes the runtime pacakge to use a C function to call mmap.
> >
> > The final argument to mmap, of type off_t, varies. In
> > https://go.dev/cl/445375
> > (https://gcc.gnu.org/pipermail/gcc-patches/2022-October/604158.html)
> > we changed it to always use the C off_t type, but that broke 32-bit
> > big-endian Linux systems.
>
> This has nothing to do with big-endian, armv7 isn't big-endian.
OK, but I think that it does have something to do with big-endian.
The bug was that on some 32-bit systems it was passing a 64-bit value
to a function that expected a 32-bit value. The problem didn't show
up on 32-bit x86 because it is little-endian, and did show up on
32-bit PPC because it is big-endian. I guess the armv7 case was
failing for a different reason.
Ian
On Jun 20 2023, Ian Lance Taylor wrote:
> OK, but I think that it does have something to do with big-endian.
> The bug was that on some 32-bit systems it was passing a 64-bit value
> to a function that expected a 32-bit value. The problem didn't show
> up on 32-bit x86 because it is little-endian, and did show up on
> 32-bit PPC because it is big-endian. I guess the armv7 case was
> failing for a different reason.
Not failing is no proof for correctness. It fails everywhere for the
same reason.
On Tue, Jun 20, 2023 at 3:37 PM Ian Lance Taylor <iant@golang.org> wrote:
> On Tue, Jun 20, 2023 at 11:35 AM Andreas Schwab <schwab@linux-m68k.org>
> wrote:
> >
> > On Jun 20 2023, Ian Lance Taylor via Gcc-patches wrote:
> >
> > > This libgo patches changes the runtime pacakge to use a C function to
> call mmap.
> > >
> > > The final argument to mmap, of type off_t, varies. In
> > > https://go.dev/cl/445375
> > > (https://gcc.gnu.org/pipermail/gcc-patches/2022-October/604158.html)
> > > we changed it to always use the C off_t type, but that broke 32-bit
> > > big-endian Linux systems.
> >
> > This has nothing to do with big-endian, armv7 isn't big-endian.
>
> OK, but I think that it does have something to do with big-endian.
> The bug was that on some 32-bit systems it was passing a 64-bit value
> to a function that expected a 32-bit value. The problem didn't show
> up on 32-bit x86 because it is little-endian, and did show up on
> 32-bit PPC because it is big-endian. I guess the armv7 case was
> failing for a different reason.
I think there is a calling convention issue. On 32-bit ARM, for the case of
mmap, if the last argument is 32-bit, it is passed 4 bytes at sp+4. If it
is 64-bit, the offset is aligned and it is stored as 8 bytes at sp+8. So if
the callee tries to read at sp+4, it gets the wrong value, even for little
endian. On 32-bit x86 it doesn't seem to have that alignment padding.
@@ -1,4 +1,4 @@
-a3a3c3a2d1bc6a8ca51b302d08c94ef27cdd8f0f
+6a1d165c2218cd127ee937a1f45599075762f716
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
@@ -462,6 +462,7 @@ runtime_files = \
runtime/go-memclr.c \
runtime/go-memmove.c \
runtime/go-memequal.c \
+ runtime/go-mmap.c \
runtime/go-nanotime.c \
runtime/go-now.c \
runtime/go-nosys.c \
@@ -14,8 +14,8 @@ import (
//go:linkname sysAlloc
//go:linkname sysFree
-//extern mmap
-func sysMmap(addr unsafe.Pointer, n uintptr, prot, flags, fd int32, off _libgo_off_t_type) unsafe.Pointer
+//extern __go_mmap
+func sysMmap(addr unsafe.Pointer, n uintptr, prot, flags, fd int32, off uintptr) unsafe.Pointer
//extern munmap
func munmap(addr unsafe.Pointer, length uintptr) int32
@@ -38,7 +38,7 @@ func init() {
}
func mmap(addr unsafe.Pointer, n uintptr, prot, flags, fd int32, off uintptr) (unsafe.Pointer, int) {
- p := sysMmap(addr, n, prot, flags, fd, _libgo_off_t_type(off))
+ p := sysMmap(addr, n, prot, flags, fd, off)
if uintptr(p) == _MAP_FAILED {
return nil, errno()
}
new file mode 100644
@@ -0,0 +1,21 @@
+/* go-mmap.c -- functions for calling C mmap functions.
+
+ Copyright 2023 The Go Authors. All rights reserved.
+ Use of this source code is governed by a BSD-style
+ license that can be found in the LICENSE file. */
+
+#include "config.h"
+
+#include <stdint.h>
+#include <sys/mman.h>
+
+/* The exact C function to call varies between mmap and mmap64, and
+ the size of the off_t argument also varies. Here we provide a
+ function that Go code can call with consistent types. */
+
+void *
+__go_mmap(void *addr, uintptr_t length, int32_t prot, int32_t flags,
+ int32_t fd, uintptr_t offset)
+{
+ return mmap(addr, length, prot, flags, fd, offset);
+}
@@ -355,9 +355,6 @@ bool runtime_notetsleepg(Note*, int64) // false - timeout
/*
* low level C-called
*/
-#define runtime_mmap mmap
-#define runtime_munmap munmap
-#define runtime_madvise madvise
#define runtime_memclr(buf, size) __builtin_memset((buf), 0, (size))
#define runtime_getcallerpc() __builtin_return_address(0)