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=-1.7 required=5.0 tests=MAILING_LIST_MULTI, RCVD_IN_DNSWL_LOW,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 31976 invoked from network); 26 May 2023 21:04:17 -0000 Received: from second.openwall.net (193.110.157.125) by inbox.vuxu.org with ESMTPUTF8; 26 May 2023 21:04:17 -0000 Received: (qmail 16360 invoked by uid 550); 26 May 2023 21:04:14 -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 16262 invoked from network); 26 May 2023 21:04:10 -0000 Date: Fri, 26 May 2023 17:03:58 -0400 From: Rich Felker To: =?utf-8?B?SuKCkeKCmeKCmw==?= Gustedt Cc: musl@lists.openwall.com Message-ID: <20230526210358.GQ4163@brightrain.aerifal.cx> References: <20230526203107.GN4163@brightrain.aerifal.cx> <20230526225119.4daa2815@inria.fr> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20230526225119.4daa2815@inria.fr> User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Re: [musl] [C23 printf 2/3] C23: implement the wN length specifiers for printf On Fri, May 26, 2023 at 10:51:19PM +0200, Jā‚‘ā‚™ā‚› Gustedt wrote: > Rich, > > on Fri, 26 May 2023 16:31:07 -0400 you (Rich Felker ) > wrote: > > > On Fri, May 26, 2023 at 09:41:03PM +0200, Jens Gustedt wrote: > > > These are mandatory for C23 and concern all types for which the > > > platform has `int_leastN_t` and `uint_leastN_t`. For musl these > > > types always coincide with `intN_t` and `uintN_t` and are always > > > present for N equal 8, 16, 32 and 64. > > > > > > They can be added for general use since all lowercase letters were > > > previously reserved. > > > > > > Nevertheless, users that use these modifiers will see a lot of > > > warnings from compilers in the beginning. This is because the > > > compilers have not yet integrated this form of a specifier into > > > their correponding extensions (gcc attributes). So unfortunately > > > also testing this feature may be a bit noisy for the moment. > > > > > > The only architecture dependend choice is the type for N == 64, > > > which may be `long` or `long long`. We just mimick the test that is > > > done in other places to compare `UINTPTR_MAX` and `UINT64_MAX` to > > > determine that. > > > --- > > > src/stdio/vfprintf.c | 18 ++++++++++++++++-- > > > src/stdio/vfwprintf.c | 18 ++++++++++++++++-- > > > 2 files changed, 32 insertions(+), 4 deletions(-) > > > > > > diff --git a/src/stdio/vfprintf.c b/src/stdio/vfprintf.c > > > index cbc79783..1a516663 100644 > > > --- a/src/stdio/vfprintf.c > > > +++ b/src/stdio/vfprintf.c > > > @@ -33,7 +33,7 @@ > > > > > > enum { > > > BARE, LPRE, LLPRE, HPRE, HHPRE, BIGLPRE, > > > - ZTPRE, JPRE, > > > + ZTPRE, JPRE, WPRE, > > > STOP, > > > PTR, INT, UINT, ULLONG, > > > LONG, ULONG, > > > @@ -57,7 +57,7 @@ static const unsigned char states[]['z'-'A'+1] = { > > > S('s') = PTR, S('S') = PTR, S('p') = UIPTR, S('n') > > > = PTR, S('m') = NOARG, > > > S('l') = LPRE, S('h') = HPRE, S('L') = BIGLPRE, > > > - S('z') = ZTPRE, S('j') = JPRE, S('t') = ZTPRE, > > > + S('z') = ZTPRE, S('j') = JPRE, S('t') = ZTPRE, > > > S('w') = WPRE, }, { /* 1: l-prefixed */ > > > S('b') = ULONG, S('B') = ULONG, > > > S('d') = LONG, S('i') = LONG, > > > @@ -525,8 +525,22 @@ static int printf_core(FILE *f, const char > > > *fmt, va_list *ap, union arg *nl_arg, st=0; > > > do { > > > if (OOB(*s)) goto inval; > > > + wpre: > > > ps=st; > > > st=states[st]S(*s++); > > > + if (st == WPRE) { > > > + switch (getint(&s)) { > > > + case 8: st = HHPRE; goto wpre; > > > + case 16: st = HPRE; goto wpre; > > > + case 32: st = BARE; goto wpre; > > > +#if UINTPTR_MAX >= UINT64_MAX > > > + case 64: st = LPRE; goto wpre; > > > +#else > > > + case 64: st = LLPRE; goto wpre; > > > +#endif > > > + default: goto inval; > > > + } > > > + } > > > } while (st-1 > > if (!st) goto inval; > > > > I don't see how this works. While you're in this new WPRE state, > > you're accesing an element of the states[] array with a potentially > > out-of-bounds index, because you skipped over the bounds check to > > ensure that the index is valid. > > ah, ok, the `wpre` should probably move up a line. OK, I was thinking you just want a break, but you're trying to avoid a transition to BARE state getting treated as invalid by the loop condition. However, the underlying problem here is that you can't reuse the BARE state for something that's not BARE. If you do, formats like %w32w32w32w32w32w32d or %w32lld or even %w32f will be allowed. I think you need an extra state that's "plain but not bare" that duplicates only the integer transitions out of it, like the l, ll, etc. prefix states do. > > I'm not clear why you're doing that > > instead of just continuing the loop. > > > > My preference would be not adding any code at all here and using the > > existing state machine, adding state transitions for the new prefixes > > to it, but that would require expanding the states stable to start at > > '1' instead of 'A', and to have a couple more intermediate states. I'm > > not sure how large that would get. There's a good chance it's > > comparable to the size of any added code, though. > > I am not sure that I understand the alternative that you are proposing. > > The difficulty that lead to this, is the "BARE" state which now > becomes accessible with a "w32" prefix. Then later, the "BARE" state > assumes (for the stop condition of the loop) that there had not been a > prefix, I think. Yes, but I don't think you've solved that. It should be solvable as above. Rich