libgo: use _off_t for mmap offset argument

Message ID 20220929144912.21826-1-soeren@soeren-tempel.net
State New
Headers
Series libgo: use _off_t for mmap offset argument |

Commit Message

Li, Pan2 via Gcc-patches Sept. 29, 2022, 2:49 p.m. UTC
  From: Sören Tempel <soeren@soeren-tempel.net>

On glibc-based systems, off_t is a 32-bit type on 32-bit systems and a
64-bit type on 64-bit systems by default. However, on systems using musl
libc off_t is unconditionally a 64-bit type. As such, it is insufficient
to use a uintptr type for the mmap offset parameter.

Presently, the (incorrect) mmap declaration causes a libgo run-time
failure on 32-bit musl systems (fatal error: runtime: cannot allocate
memory). This commit fixes this run-time error.

Signed-off-by: Sören Tempel <soeren@soeren-tempel.net>
---
This implements what has been proposed by Ian in a GitHub comment
https://github.com/golang/go/issues/51280#issuecomment-1046322011

I don't have access to a 32-bit glibc system to test this on but
this does seem to work fine on 32-bit and 64-bit musl systems.

 libgo/go/runtime/mem_gccgo.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
  

Comments

Sören Tempel Oct. 22, 2022, 1:45 p.m. UTC | #1
PING.

soeren@soeren-tempel.net wrote:
> From: Sören Tempel <soeren@soeren-tempel.net>
> 
> On glibc-based systems, off_t is a 32-bit type on 32-bit systems and a
> 64-bit type on 64-bit systems by default. However, on systems using musl
> libc off_t is unconditionally a 64-bit type. As such, it is insufficient
> to use a uintptr type for the mmap offset parameter.
> 
> Presently, the (incorrect) mmap declaration causes a libgo run-time
> failure on 32-bit musl systems (fatal error: runtime: cannot allocate
> memory). This commit fixes this run-time error.
> 
> Signed-off-by: Sören Tempel <soeren@soeren-tempel.net>
> ---
> This implements what has been proposed by Ian in a GitHub comment
> https://github.com/golang/go/issues/51280#issuecomment-1046322011
> 
> I don't have access to a 32-bit glibc system to test this on but
> this does seem to work fine on 32-bit and 64-bit musl systems.
> 
>  libgo/go/runtime/mem_gccgo.go | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libgo/go/runtime/mem_gccgo.go b/libgo/go/runtime/mem_gccgo.go
> index fa3389d8..07bf325a 100644
> --- a/libgo/go/runtime/mem_gccgo.go
> +++ b/libgo/go/runtime/mem_gccgo.go
> @@ -15,7 +15,7 @@ import (
>  //go:linkname sysFree
>  
>  //extern mmap
> -func sysMmap(addr unsafe.Pointer, n uintptr, prot, flags, fd int32, off uintptr) unsafe.Pointer
> +func sysMmap(addr unsafe.Pointer, n uintptr, prot, flags, fd int32, off _off_t) 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, off)
> +	p := sysMmap(addr, n, prot, flags, fd, _off_t(off))
>  	if uintptr(p) == _MAP_FAILED {
>  		return nil, errno()
>  	}
  
Ian Lance Taylor Oct. 28, 2022, 12:13 a.m. UTC | #2
On Sat, Oct 22, 2022 at 6:45 AM Sören Tempel <soeren@soeren-tempel.net> wrote:
>
> PING.
>
> soeren@soeren-tempel.net wrote:
> > From: Sören Tempel <soeren@soeren-tempel.net>
> >
> > On glibc-based systems, off_t is a 32-bit type on 32-bit systems and a
> > 64-bit type on 64-bit systems by default. However, on systems using musl
> > libc off_t is unconditionally a 64-bit type. As such, it is insufficient
> > to use a uintptr type for the mmap offset parameter.
> >
> > Presently, the (incorrect) mmap declaration causes a libgo run-time
> > failure on 32-bit musl systems (fatal error: runtime: cannot allocate
> > memory). This commit fixes this run-time error.
> >
> > Signed-off-by: Sören Tempel <soeren@soeren-tempel.net>
> > ---
> > This implements what has been proposed by Ian in a GitHub comment
> > https://github.com/golang/go/issues/51280#issuecomment-1046322011
> >
> > I don't have access to a 32-bit glibc system to test this on but
> > this does seem to work fine on 32-bit and 64-bit musl systems.

Thanks.  Committed as follows using _libgo_off_t_type to avoid the
confusion between off_t and off64_t.

Sorry for the delay.

Ian
11a5fc0c76aedb100b5d7ecc7dd4bed33d850bb8
diff --git a/gcc/go/gofrontend/MERGE b/gcc/go/gofrontend/MERGE
index 5b95b38a541..7e531c3f90b 100644
--- a/gcc/go/gofrontend/MERGE
+++ b/gcc/go/gofrontend/MERGE
@@ -1,4 +1,4 @@
-6c188108858e3ae8c8ea8e4cc55427d8cf01bbc8
+5e658f4659c551330ea68f5667e4f951b218f32d
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
diff --git a/libgo/go/runtime/mem_gccgo.go b/libgo/go/runtime/mem_gccgo.go
index fa3389d857e..1e84f4f5c56 100644
--- a/libgo/go/runtime/mem_gccgo.go
+++ b/libgo/go/runtime/mem_gccgo.go
@@ -15,7 +15,7 @@ import (
 //go:linkname sysFree
 
 //extern mmap
-func sysMmap(addr unsafe.Pointer, n uintptr, prot, flags, fd int32, off uintptr) unsafe.Pointer
+func sysMmap(addr unsafe.Pointer, n uintptr, prot, flags, fd int32, off _libgo_off_t_type) 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, off)
+	p := sysMmap(addr, n, prot, flags, fd, _libgo_off_t_type(off))
 	if uintptr(p) == _MAP_FAILED {
 		return nil, errno()
 	}
@@ -47,6 +47,7 @@ func mmap(addr unsafe.Pointer, n uintptr, prot, flags, fd int32, off uintptr) (u
 
 // Don't split the stack as this method may be invoked without a valid G, which
 // prevents us from allocating more stack.
+//
 //go:nosplit
 func sysAlloc(n uintptr, sysStat *sysMemStat) unsafe.Pointer {
 	p, err := mmap(nil, n, _PROT_READ|_PROT_WRITE, _MAP_ANON|_MAP_PRIVATE, mmapFD, 0)
@@ -165,6 +166,7 @@ func sysHugePage(v unsafe.Pointer, n uintptr) {
 
 // Don't split the stack as this function may be invoked without a valid G,
 // which prevents us from allocating more stack.
+//
 //go:nosplit
 func sysFree(v unsafe.Pointer, n uintptr, sysStat *sysMemStat) {
 	sysStat.add(-int64(n))
  

Patch

diff --git a/libgo/go/runtime/mem_gccgo.go b/libgo/go/runtime/mem_gccgo.go
index fa3389d8..07bf325a 100644
--- a/libgo/go/runtime/mem_gccgo.go
+++ b/libgo/go/runtime/mem_gccgo.go
@@ -15,7 +15,7 @@  import (
 //go:linkname sysFree
 
 //extern mmap
-func sysMmap(addr unsafe.Pointer, n uintptr, prot, flags, fd int32, off uintptr) unsafe.Pointer
+func sysMmap(addr unsafe.Pointer, n uintptr, prot, flags, fd int32, off _off_t) 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, off)
+	p := sysMmap(addr, n, prot, flags, fd, _off_t(off))
 	if uintptr(p) == _MAP_FAILED {
 		return nil, errno()
 	}