[PATCH] mksysinfo: add support for musl libc

[PATCH] mksysinfo: add support for musl libc

[soeren]

From: Sören Tempel <soeren@soeren-tempel.net>

This patch addresses two minor compatibility issues with musl libc:

* On some architecture (e.g. PowerPC), musl has more than one field
  prefixed with st_{a,m,c}tim in struct stat. This causes the sed(1)
  invocation to not work correctly (since it will only replace the first
  occurrence) [1]. This can be fixed by passing the 'g' flag to replace
  all occurrences.
* Since version 1.2.3, musl defines SYS_SECCOMP in signal.h [2]. This
  conflicts with mksysinfo extraction of syscall numbers [3]. By
  restricting the grep expression to only match lower case characters we
  can avoid a redefinition of SYS_SECCOMP. This is GCC PR 105225.

This patch combines two Alpine Linux patches which have been written by
Ariadne Conill and Natanael Copa. I haven't tested this with glibc but I
strongly suspect that both changes should not introduce any issue with
glibc.

[1]: https://git.musl-libc.org/cgit/musl/tree/arch/powerpc/bits/stat.h
[2]: https://git.musl-libc.org/cgit/musl/commit/?id=3dcbd896907d9d474da811b7c6b769342abaf651
[3]: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105225

Signed-off-by: Sören Tempel <soeren@soeren-tempel.net>
---
 libgo/mksysinfo.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libgo/mksysinfo.sh b/libgo/mksysinfo.sh
index 5aa30915..72044624 100755
--- a/libgo/mksysinfo.sh
+++ b/libgo/mksysinfo.sh
@@ -126,7 +126,7 @@ if ! grep '^const SIGCLD ' ${OUT} >/dev/null 2>&1; then
 fi
 
 # The syscall numbers.  We force the names to upper case.
-grep '^const _SYS_' gen-sysinfo.go | \
+grep '^const _SYS_[a-z]' gen-sysinfo.go | \
   sed -e 's/const _\(SYS_[^= ]*\).*$/\1/' | \
   while read sys; do
     sup=`echo $sys | tr abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ`
@@ -506,7 +506,7 @@ fi
 
 # For historical reasons Go uses the suffix "timespec" instead of "tim" for
 # stat_t's time fields on NetBSD.
-st_times='-e s/st_atim/Atim/ -e s/st_mtim/Mtim/ -e s/st_ctim/Ctim/'
+st_times='-e s/st_atim/Atim/g -e s/st_mtim/Mtim/g -e s/st_ctim/Ctim/g'
 if test "${GOOS}" = "netbsd"; then
     st_times='-e s/st_atim/Atimespec/ -e s/st_mtim/Mtimespec/ -e s/st_ctim/Ctimespec/'
 fi

Re: [PATCH] mksysinfo: add support for musl libc

[Ian Lance Taylor]

On Mon, Jun 27, 2022 at 9:47 AM <soeren@soeren-tempel.net> wrote:
>
> From: Sören Tempel <soeren@soeren-tempel.net>
>
> This patch addresses two minor compatibility issues with musl libc:
>
> * On some architecture (e.g. PowerPC), musl has more than one field
>   prefixed with st_{a,m,c}tim in struct stat. This causes the sed(1)
>   invocation to not work correctly (since it will only replace the first
>   occurrence) [1]. This can be fixed by passing the 'g' flag to replace
>   all occurrences.
> * Since version 1.2.3, musl defines SYS_SECCOMP in signal.h [2]. This
>   conflicts with mksysinfo extraction of syscall numbers [3]. By
>   restricting the grep expression to only match lower case characters we
>   can avoid a redefinition of SYS_SECCOMP. This is GCC PR 105225.
>
> This patch combines two Alpine Linux patches which have been written by
> Ariadne Conill and Natanael Copa. I haven't tested this with glibc but I
> strongly suspect that both changes should not introduce any issue with
> glibc.

Given that pretty much every one of these musl patches has led to
problems on some glibc systems, it would be very nice if you could do
some testing with glibc.  Thanks.

Can you expand on the st_atim issue?  What does the musl type look
like in gen-sysinfo.go?

What is the value of SYS_SECCOMP in musl?  Is it a system call number?
 What does it look like in gen-sysinfo.go?

Thanks.

Ian

Re: [PATCH] mksysinfo: add support for musl libc

[Sören Tempel]

Ian Lance Taylor <iant@golang.org> wrote:
> Given that pretty much every one of these musl patches has led to
> problems on some glibc systems, it would be very nice if you could do
> some testing with glibc.  Thanks.

Sorry, my bad.

I just tested this on Arch Linux and it compiles fine with the patch.

> Can you expand on the st_atim issue?

The st_atim issue is simply that struct stat contains additional struct
fields on 32-bit musl architectures (__st_{a,m,c}tim32) in addition to
st_{a,m,c}tim [1]. The sed expression currently only replaces the first
occurrence (i.e. __st_{a,m,c}tim32) from gen-sysinfo.go:

	$ grep 'st_[acm]tim' sysinfo.go
	type _stat struct { st_dev uint64; __st_dev_padding int32; __st_ino_truncated int32; st_mode uint32; st_nlink uint32; st_uid uint32; st_gid uint32; st_rdev uint64; __st_rdev_padding int32; st_size int64; st_blksize int32; st_blocks int64; __st_atim32 struct { tv_sec int32; tv_nsec int32; }; __st_mtim32 struct { tv_sec int32; tv_nsec int32; }; __st_ctim32 struct { tv_sec int32; tv_nsec int32; }; st_ino uint64; st_atim Timespec; st_mtim Timespec; st_ctim Timespec; }
	type Stat_t struct { Dev uint64; __st_dev_padding int32; __Ino_truncated int32; Mode uint32; Nlink uint32; Uid uint32; Gid uint32; Rdev uint64; __st_rdev_padding int32; Size int64; Blksize int32; Blocks int64; __Atim32 struct { tv_sec int32; tv_nsec int32; }; __Mtim32 struct { tv_sec int32; tv_nsec int32; }; __Ctim32 struct { tv_sec int32; tv_nsec int32; }; Ino uint64; st_atim Timespec; st_mtim Timespec; st_ctim Timespec; }

This causes the following build error on 32-bit musl architectures:

	stat_atim.go: error: reference to undefined field or method 'Mtim'
	   17 |         fs.modTime = timespecToTime(fs.sys.Mtim)
	      |                                           ^
	stat_atim.go: error: reference to undefined field or method 'Atim'
	   52 |         return timespecToTime(fi.Sys().(*syscall.Stat_t).Atim)
	      |                                                         ^

This is fixed by the proposed patch by replacing both members in struct stat.

> What does the musl type look like in gen-sysinfo.go?

	$ grep 'st_[acm]tim' gen-sysinfo.go
	// unknowndefine st_atime st_atim.tv_sec
	// unknowndefine st_mtime st_mtim.tv_sec
	// unknowndefine st_ctime st_ctim.tv_sec
	type _stat struct { st_dev uint64; __st_dev_padding int32; __st_ino_truncated int32; st_mode uint32; st_nlink uint32; st_uid uint32; st_gid uint32; st_rdev uint64; __st_rdev_padding int32; st_size int64; st_blksize int32; st_blocks int64; __st_atim32 struct { tv_sec int32; tv_nsec int32; }; __st_mtim32 struct { tv_sec int32; tv_nsec int32; }; __st_ctim32 struct { tv_sec int32; tv_nsec int32; }; st_ino uint64; st_atim _timespec; st_mtim _timespec; st_ctim _timespec; }

> What is the value of SYS_SECCOMP in musl?  Is it a system call number?

SYS_SECCOMP is a macro defined in signal.h which can be used to check
the si_code member of siginfo_t for SIGSYS, see the SECCOMP_RET_TRAP
description in seccomp(2). As such, it is not a system call number.
The value of SYS_SECCOMP is simply 1 [2].

> What does it look like in gen-sysinfo.go?

Defined in gen-sysinfo.go as follows:

	4688:const _SYS_seccomp = 317
	4775:const _SYS_SECCOMP = 1

where the former is the syscall and the latter is the expanded
SYS_SECCOMP macro constant. When generating sysinfo.go the name seems to
be uppercased, thus resulting in a compilation failure. The generated
sysinfo.go contains the following lines:

	3067:const _SYS_seccomp = 317
	3154:const _SYS_SECCOMP = 1
	6600:const SYS_SECCOMP = _SYS_seccomp
	6606:const SYS_SECCOMP = _SYS_SECCOMP

Build error:

	sysinfo.go:6606:7: error: redefinition of 'SYS_SECCOMP'
	 6606 | const SYS_SECCOMP = _SYS_SECCOMP
	      |       ^
	sysinfo.go:6600:7: note: previous definition of 'SYS_SECCOMP' was here
	 6600 | const SYS_SECCOMP = _SYS_seccomp
	      |       ^

This is fixed by the patch by only extracting _SYS_seccomp, not _SYS_SECCOMP.

If you need more information, just let me know :)

Greetings,
Sören

[1]: https://git.musl-libc.org/cgit/musl/tree/arch/i386/bits/stat.h
[2]: https://git.musl-libc.org/cgit/musl/commit/?id=3dcbd896907d9d474da811b7c6b769342abaf651

Re: [PATCH] mksysinfo: add support for musl libc

[Ian Lance Taylor]

[-- Attachment #1: Type: text/plain, Size: 4626 bytes --]

On Tue, Jun 28, 2022 at 7:32 AM Sören Tempel <soeren@soeren-tempel.net> wrote:
>
> Ian Lance Taylor <iant@golang.org> wrote:
> > Given that pretty much every one of these musl patches has led to
> > problems on some glibc systems, it would be very nice if you could do
> > some testing with glibc.  Thanks.
>
> Sorry, my bad.
>
> I just tested this on Arch Linux and it compiles fine with the patch.
>
> > Can you expand on the st_atim issue?
>
> The st_atim issue is simply that struct stat contains additional struct
> fields on 32-bit musl architectures (__st_{a,m,c}tim32) in addition to
> st_{a,m,c}tim [1]. The sed expression currently only replaces the first
> occurrence (i.e. __st_{a,m,c}tim32) from gen-sysinfo.go:
>
>         $ grep 'st_[acm]tim' sysinfo.go
>         type _stat struct { st_dev uint64; __st_dev_padding int32; __st_ino_truncated int32; st_mode uint32; st_nlink uint32; st_uid uint32; st_gid uint32; st_rdev uint64; __st_rdev_padding int32; st_size int64; st_blksize int32; st_blocks int64; __st_atim32 struct { tv_sec int32; tv_nsec int32; }; __st_mtim32 struct { tv_sec int32; tv_nsec int32; }; __st_ctim32 struct { tv_sec int32; tv_nsec int32; }; st_ino uint64; st_atim Timespec; st_mtim Timespec; st_ctim Timespec; }
>         type Stat_t struct { Dev uint64; __st_dev_padding int32; __Ino_truncated int32; Mode uint32; Nlink uint32; Uid uint32; Gid uint32; Rdev uint64; __st_rdev_padding int32; Size int64; Blksize int32; Blocks int64; __Atim32 struct { tv_sec int32; tv_nsec int32; }; __Mtim32 struct { tv_sec int32; tv_nsec int32; }; __Ctim32 struct { tv_sec int32; tv_nsec int32; }; Ino uint64; st_atim Timespec; st_mtim Timespec; st_ctim Timespec; }
>
> This causes the following build error on 32-bit musl architectures:
>
>         stat_atim.go: error: reference to undefined field or method 'Mtim'
>            17 |         fs.modTime = timespecToTime(fs.sys.Mtim)
>               |                                           ^
>         stat_atim.go: error: reference to undefined field or method 'Atim'
>            52 |         return timespecToTime(fi.Sys().(*syscall.Stat_t).Atim)
>               |                                                         ^
>
> This is fixed by the proposed patch by replacing both members in struct stat.
>
> > What does the musl type look like in gen-sysinfo.go?
>
>         $ grep 'st_[acm]tim' gen-sysinfo.go
>         // unknowndefine st_atime st_atim.tv_sec
>         // unknowndefine st_mtime st_mtim.tv_sec
>         // unknowndefine st_ctime st_ctim.tv_sec
>         type _stat struct { st_dev uint64; __st_dev_padding int32; __st_ino_truncated int32; st_mode uint32; st_nlink uint32; st_uid uint32; st_gid uint32; st_rdev uint64; __st_rdev_padding int32; st_size int64; st_blksize int32; st_blocks int64; __st_atim32 struct { tv_sec int32; tv_nsec int32; }; __st_mtim32 struct { tv_sec int32; tv_nsec int32; }; __st_ctim32 struct { tv_sec int32; tv_nsec int32; }; st_ino uint64; st_atim _timespec; st_mtim _timespec; st_ctim _timespec; }
>
> > What is the value of SYS_SECCOMP in musl?  Is it a system call number?
>
> SYS_SECCOMP is a macro defined in signal.h which can be used to check
> the si_code member of siginfo_t for SIGSYS, see the SECCOMP_RET_TRAP
> description in seccomp(2). As such, it is not a system call number.
> The value of SYS_SECCOMP is simply 1 [2].
>
> > What does it look like in gen-sysinfo.go?
>
> Defined in gen-sysinfo.go as follows:
>
>         4688:const _SYS_seccomp = 317
>         4775:const _SYS_SECCOMP = 1
>
> where the former is the syscall and the latter is the expanded
> SYS_SECCOMP macro constant. When generating sysinfo.go the name seems to
> be uppercased, thus resulting in a compilation failure. The generated
> sysinfo.go contains the following lines:
>
>         3067:const _SYS_seccomp = 317
>         3154:const _SYS_SECCOMP = 1
>         6600:const SYS_SECCOMP = _SYS_seccomp
>         6606:const SYS_SECCOMP = _SYS_SECCOMP
>
> Build error:
>
>         sysinfo.go:6606:7: error: redefinition of 'SYS_SECCOMP'
>          6606 | const SYS_SECCOMP = _SYS_SECCOMP
>               |       ^
>         sysinfo.go:6600:7: note: previous definition of 'SYS_SECCOMP' was here
>          6600 | const SYS_SECCOMP = _SYS_seccomp
>               |       ^
>
> This is fixed by the patch by only extracting _SYS_seccomp, not _SYS_SECCOMP.
>
> If you need more information, just let me know :)


Thanks for the info.  Does this patch work?  It tweaks the handling of
SYS_SECCOMP to be specific to that constant.

Ian

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 856 bytes --]

diff --git a/libgo/mksysinfo.sh b/libgo/mksysinfo.sh
index 5aa309155..ea1fa17d4 100755
--- a/libgo/mksysinfo.sh
+++ b/libgo/mksysinfo.sh
@@ -127,6 +127,7 @@ fi
 
 # The syscall numbers.  We force the names to upper case.
 grep '^const _SYS_' gen-sysinfo.go | \
+  grep -v '^const _SYS_SECCOMP = ' | \
   sed -e 's/const _\(SYS_[^= ]*\).*$/\1/' | \
   while read sys; do
     sup=`echo $sys | tr abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ`
@@ -506,7 +507,7 @@ fi
 
 # For historical reasons Go uses the suffix "timespec" instead of "tim" for
 # stat_t's time fields on NetBSD.
-st_times='-e s/st_atim/Atim/ -e s/st_mtim/Mtim/ -e s/st_ctim/Ctim/'
+st_times='-e s/st_atim/Atim/g -e s/st_mtim/Mtim/g -e s/st_ctim/Ctim/g'
 if test "${GOOS}" = "netbsd"; then
     st_times='-e s/st_atim/Atimespec/ -e s/st_mtim/Mtimespec/ -e s/st_ctim/Ctimespec/'
 fi

Re: [PATCH] mksysinfo: add support for musl libc

[Sören Tempel]

Ian Lance Taylor <iant@golang.org> wrote:
> Thanks for the info.  Does this patch work?  It tweaks the handling of
> SYS_SECCOMP to be specific to that constant.

Yes, your patch works for me too on Alpine Linux Edge.

Thanks!

Greetings,
Sören

Re: [PATCH] mksysinfo: add support for musl libc

[Ian Lance Taylor]

On Thu, Jun 30, 2022 at 9:59 AM Sören Tempel <soeren@soeren-tempel.net> wrote:
>
> Ian Lance Taylor <iant@golang.org> wrote:
> > Thanks for the info.  Does this patch work?  It tweaks the handling of
> > SYS_SECCOMP to be specific to that constant.
>
> Yes, your patch works for me too on Alpine Linux Edge.

Thanks.  Committed to mainline.

Ian