public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Sören Tempel" <soeren@soeren-tempel.net>
To: Ian Lance Taylor <iant@golang.org>
Cc: gofrontend-dev@googlegroups.com, gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] mksysinfo: add support for musl libc
Date: Tue, 28 Jun 2022 16:28:03 +0200	[thread overview]
Message-ID: <29VGBCEYSJXVZ.36Y9MIET2J505@8pit.net> (raw)
In-Reply-To: <CAOyqgcWn_arrE5Gu=A2ncsFr2EpxzOv8rCfU5BojNgQsem7ZnA@mail.gmail.com>

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

  reply	other threads:[~2022-06-28 14:32 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-27 16:44 soeren
2022-06-27 17:01 ` Ian Lance Taylor
2022-06-28 14:28   ` Sören Tempel [this message]
2022-06-29 22:24     ` Ian Lance Taylor
2022-06-30 16:59       ` Sören Tempel
2022-06-30 19:35         ` Ian Lance Taylor

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=29VGBCEYSJXVZ.36Y9MIET2J505@8pit.net \
    --to=soeren@soeren-tempel.net \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=gofrontend-dev@googlegroups.com \
    --cc=iant@golang.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).