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

[Ian Lance Taylor <iant@golang.org>]

[-- 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