* [PATCH] libgo: use _off_t for mmap offset argument @ 2022-09-29 14:49 soeren 2022-10-22 13:45 ` Sören Tempel 0 siblings, 1 reply; 3+ messages in thread From: soeren @ 2022-09-29 14:49 UTC (permalink / raw) To: iant; +Cc: gcc-patches, gofrontend-dev 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() } ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] libgo: use _off_t for mmap offset argument 2022-09-29 14:49 [PATCH] libgo: use _off_t for mmap offset argument soeren @ 2022-10-22 13:45 ` Sören Tempel 2022-10-28 0:13 ` Ian Lance Taylor 0 siblings, 1 reply; 3+ messages in thread From: Sören Tempel @ 2022-10-22 13:45 UTC (permalink / raw) To: iant; +Cc: gcc-patches, gofrontend-dev 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() > } ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] libgo: use _off_t for mmap offset argument 2022-10-22 13:45 ` Sören Tempel @ 2022-10-28 0:13 ` Ian Lance Taylor 0 siblings, 0 replies; 3+ messages in thread From: Ian Lance Taylor @ 2022-10-28 0:13 UTC (permalink / raw) To: Sören Tempel; +Cc: gcc-patches, gofrontend-dev [-- Attachment #1: Type: text/plain, Size: 1183 bytes --] 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 [-- Attachment #2: patch.txt --] [-- Type: text/plain, Size: 1959 bytes --] 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)) ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-10-28 0:13 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-09-29 14:49 [PATCH] libgo: use _off_t for mmap offset argument soeren 2022-10-22 13:45 ` Sören Tempel 2022-10-28 0:13 ` Ian Lance Taylor
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).