From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.3 required=5.0 tests=MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 16976 invoked from network); 3 Sep 2020 21:17:31 -0000 Received: from mother.openwall.net (195.42.179.200) by inbox.vuxu.org with ESMTPUTF8; 3 Sep 2020 21:17:31 -0000 Received: (qmail 19963 invoked by uid 550); 3 Sep 2020 21:17:28 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 19942 invoked from network); 3 Sep 2020 21:17:27 -0000 Date: Thu, 3 Sep 2020 17:17:15 -0400 From: Rich Felker To: musl@lists.openwall.com Message-ID: <20200903211715.GG3265@brightrain.aerifal.cx> References: <20200903112309.102601-1-sorear@fastmail.com> <20200903112309.102601-3-sorear@fastmail.com> <20200903155615.GC3265@brightrain.aerifal.cx> <05292d85-1bcf-4967-8c48-276d5d82e91b@www.fastmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <05292d85-1bcf-4967-8c48-276d5d82e91b@www.fastmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Re: [musl] [PATCH 02/14] time64: Don't make aliases to nonexistent syscalls On Thu, Sep 03, 2020 at 03:36:00PM -0400, Stefan O'Rear wrote: > On Thu, Sep 3, 2020, at 11:56 AM, Rich Felker wrote: > > On Thu, Sep 03, 2020 at 07:22:57AM -0400, Stefan O'Rear wrote: > > > riscv32 and future architectures lack the _time32 variants entirely, so > > > don't try to use their numbers. > > > --- > > > src/internal/syscall.h | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/src/internal/syscall.h b/src/internal/syscall.h > > > index d5f294d4..66fc4e5c 100644 > > > --- a/src/internal/syscall.h > > > +++ b/src/internal/syscall.h > > > @@ -201,6 +201,7 @@ static inline long __alt_socketcall(int sys, int sock, int cp, long a, long b, l > > > #define SYS_sendfile SYS_sendfile64 > > > #endif > > > > > > +#ifdef SYS_timer_settime32 > > > #ifndef SYS_timer_settime > > > #define SYS_timer_settime SYS_timer_settime32 > > > #endif > > > @@ -240,6 +241,7 @@ static inline long __alt_socketcall(int sys, int sock, int cp, long a, long b, l > > > #ifndef SYS_settimeofday > > > #define SYS_settimeofday SYS_settimeofday_time32 > > > #endif > > > +#endif > > > > The existing expectation internally in musl is that archs that lack > > legacy time32 syscalls have both the unadorned and _time64 macros > > defined to the same value. The public headers don't have to be like > > that but the internal ones do. See arch/x32/syscall_arch.h which does > > it but in the opposite direction. This logic is all tested for x32 and > > a big part of the point of that was preparing for rv32 and future > > 32-bit archs. > > > > I'm actually missing how this patch worked as-written, since for > > example timer_settime.c unconditionally assumes SYS_timer_settime is > > defined, and if it's defined to anything other than the same value as > > the time64 version, it will use the value as a fallback. > > src/internal/syscall.h has two groups of conditional definitions of unadorned > syscall numbers, the first of which sets undefined syscalls to the _time32 > version, the second of which sets to the _time64 version. So for instance I > see no way for the #define SYS_clock_gettime SYS_clock_gettime64 > (https://git.musl-libc.org/cgit/musl/tree/src/internal/syscall.h#n250) to execute. OK, I think I see what happened. Originally 4bbd7baea7 was done with the intent that riscv32 and future 32-bit archs would not need to do their own definitions of the unadorned syscall names in terms of the time64 ones, and when committed it would have worked. However, later 5a105f19b5 and 2cae9f59da hid the old syscalls that were unable to be used safely in applictions, but at risk of being used as such, behind _time32 renamings, and broke the intended mechanism. As such, your patch does get around the problem and make the right definitions exposed, but I don't really like that it conditions them all on a single #ifdef SYS_timer_settime32. If we want to keep the property that rv32 and future archs aren't required to define the unadorned names themselves (maybe they'd want to for the sake of public headers anyway, but I'm not sure about this still; I think we need it at least for futex), a solution I like somewhat better would be along the lines of: -#ifndef SYS_timer_settime +#ifdef SYS_timer_settime32 for each of the affected syscalls. This would also have the nice effect of producing an error if there already was a conflicting definition where the unadorned and time32 names both exist but don't match. Rich