Message ID | 20230911125341.3231008-1-josimmon@redhat.com |
---|---|
State | New |
Headers | show |
Series | [v2] stdio: Remove __libc_message alloca usage | expand |
On 11/09/23 09:53, Joe Simmons-Talbott wrote: > Use a fixed size array instead. The maximum number of arguments > is set by macro tricks. > > Co-authored-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> LGTM, thanks. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > --- > Changes to v1: > * Add a printf attribute to help catch mismatched arguments as well. > > include/stdio.h | 29 ++++++++++++++++++++++- > sysdeps/posix/libc_fatal.c | 47 +++++++++++--------------------------- > 2 files changed, 41 insertions(+), 35 deletions(-) > > diff --git a/include/stdio.h b/include/stdio.h > index 6755877911..7e70f95c6d 100644 > --- a/include/stdio.h > +++ b/include/stdio.h > @@ -172,10 +172,37 @@ extern int __gen_tempname (char *__tmpl, int __suffixlen, int __flags, > and abort. */ > extern void __libc_fatal (const char *__message) > __attribute__ ((__noreturn__)); > -_Noreturn void __libc_message (const char *__fnt, ...) attribute_hidden; > extern void __fortify_fail (const char *msg) __attribute__ ((__noreturn__)); > libc_hidden_proto (__fortify_fail) > > +/* The maximum number of varargs allowed in a __libc_message format string */ > +#define LIBC_MESSAGE_MAX_ARGS 4 > + > +_Noreturn void __libc_message_impl (const char *__fnt, ...) attribute_hidden > + __attribute__ ((__format__ (__printf__, 1, 2))); > + > +#define __libc_message0(fmt) \ > + __libc_message_impl (fmt) > +#define __libc_message1(fmt, a1) \ > + __libc_message_impl (fmt, a1) > +#define __libc_message2(fmt, a1, a2) \ > + __libc_message_impl (fmt, a1, a2) > +#define __libc_message3(fmt, a1, a2, a3) \ > + __libc_message_impl (fmt, a1, a2, a3) > +#define __libc_message4(fmt, a1, a2, a3, a4) \ > + __libc_message_impl (fmt, a1, a2, a3, a4) > + > +#define __libc_message_concat_x(a,b) a##b > +#define __libc_message_concat(a,b) __libc_message_concat_x (a, b) > + > +#define __libc_message_nargs_x(a0,a1,a2,a3,a4,a5,a6,...) a6 > +#define __libc_message_nargs(b, ...) \ > + __libc_message_nargs_x (__VA_ARGS__,6,5,4,3,2,1,0,) > +#define __libc_message_disp(b, ...) \ > + __libc_message_concat (b, __libc_message_nargs (__VA_ARGS__))(__VA_ARGS__) > +#define __libc_message(...) \ > + __libc_message_disp (__libc_message, __VA_ARGS__) > + > /* Acquire ownership of STREAM. */ > extern void __flockfile (FILE *__stream) attribute_hidden; > > diff --git a/sysdeps/posix/libc_fatal.c b/sysdeps/posix/libc_fatal.c > index 70edcc10c1..cf28387ee6 100644 > --- a/sysdeps/posix/libc_fatal.c > +++ b/sysdeps/posix/libc_fatal.c > @@ -45,22 +45,13 @@ writev_for_fatal (int fd, const struct iovec *iov, size_t niov, size_t total) > } > #endif > > -struct str_list > -{ > - const char *str; > - size_t len; > - struct str_list *next; > -}; > - > /* Abort with an error message. */ > void > -__libc_message (const char *fmt, ...) > +__libc_message_impl (const char *fmt, ...) > { > va_list ap; > int fd = -1; > > - va_start (ap, fmt); > - > #ifdef FATAL_PREPARE > FATAL_PREPARE; > #endif > @@ -68,9 +59,11 @@ __libc_message (const char *fmt, ...) > if (fd == -1) > fd = STDERR_FILENO; > > - struct str_list *list = NULL; > - int nlist = 0; > + struct iovec iov[LIBC_MESSAGE_MAX_ARGS * 2 - 1]; > + int iovcnt = 0; > + ssize_t total = 0; > > + va_start (ap, fmt); > const char *cp = fmt; > while (*cp != '\0') > { > @@ -100,28 +93,16 @@ __libc_message (const char *fmt, ...) > cp = next; > } > > - struct str_list *newp = alloca (sizeof (struct str_list)); > - newp->str = str; > - newp->len = len; > - newp->next = list; > - list = newp; > - ++nlist; > + iov[iovcnt].iov_base = (char *) str; > + iov[iovcnt].iov_len = len; > + total += len; > + iovcnt++; > } > + va_end (ap); > > - if (nlist > 0) > + if (iovcnt > 0) > { > - struct iovec *iov = alloca (nlist * sizeof (struct iovec)); > - ssize_t total = 0; > - > - for (int cnt = nlist - 1; cnt >= 0; --cnt) > - { > - iov[cnt].iov_base = (char *) list->str; > - iov[cnt].iov_len = list->len; > - total += list->len; > - list = list->next; > - } > - > - WRITEV_FOR_FATAL (fd, iov, nlist, total); > + WRITEV_FOR_FATAL (fd, iov, iovcnt, total); > > total = (total + 1 + GLRO(dl_pagesize) - 1) & ~(GLRO(dl_pagesize) - 1); > struct abort_msg_s *buf = __mmap (NULL, total, > @@ -131,7 +112,7 @@ __libc_message (const char *fmt, ...) > { > buf->size = total; > char *wp = buf->msg; > - for (int cnt = 0; cnt < nlist; ++cnt) > + for (int cnt = 0; cnt < iovcnt; ++cnt) > wp = mempcpy (wp, iov[cnt].iov_base, iov[cnt].iov_len); > *wp = '\0'; > > @@ -144,8 +125,6 @@ __libc_message (const char *fmt, ...) > } > } > > - va_end (ap); > - > /* Kill the application. */ > abort (); > }
diff --git a/include/stdio.h b/include/stdio.h index 6755877911..7e70f95c6d 100644 --- a/include/stdio.h +++ b/include/stdio.h @@ -172,10 +172,37 @@ extern int __gen_tempname (char *__tmpl, int __suffixlen, int __flags, and abort. */ extern void __libc_fatal (const char *__message) __attribute__ ((__noreturn__)); -_Noreturn void __libc_message (const char *__fnt, ...) attribute_hidden; extern void __fortify_fail (const char *msg) __attribute__ ((__noreturn__)); libc_hidden_proto (__fortify_fail) +/* The maximum number of varargs allowed in a __libc_message format string */ +#define LIBC_MESSAGE_MAX_ARGS 4 + +_Noreturn void __libc_message_impl (const char *__fnt, ...) attribute_hidden + __attribute__ ((__format__ (__printf__, 1, 2))); + +#define __libc_message0(fmt) \ + __libc_message_impl (fmt) +#define __libc_message1(fmt, a1) \ + __libc_message_impl (fmt, a1) +#define __libc_message2(fmt, a1, a2) \ + __libc_message_impl (fmt, a1, a2) +#define __libc_message3(fmt, a1, a2, a3) \ + __libc_message_impl (fmt, a1, a2, a3) +#define __libc_message4(fmt, a1, a2, a3, a4) \ + __libc_message_impl (fmt, a1, a2, a3, a4) + +#define __libc_message_concat_x(a,b) a##b +#define __libc_message_concat(a,b) __libc_message_concat_x (a, b) + +#define __libc_message_nargs_x(a0,a1,a2,a3,a4,a5,a6,...) a6 +#define __libc_message_nargs(b, ...) \ + __libc_message_nargs_x (__VA_ARGS__,6,5,4,3,2,1,0,) +#define __libc_message_disp(b, ...) \ + __libc_message_concat (b, __libc_message_nargs (__VA_ARGS__))(__VA_ARGS__) +#define __libc_message(...) \ + __libc_message_disp (__libc_message, __VA_ARGS__) + /* Acquire ownership of STREAM. */ extern void __flockfile (FILE *__stream) attribute_hidden; diff --git a/sysdeps/posix/libc_fatal.c b/sysdeps/posix/libc_fatal.c index 70edcc10c1..cf28387ee6 100644 --- a/sysdeps/posix/libc_fatal.c +++ b/sysdeps/posix/libc_fatal.c @@ -45,22 +45,13 @@ writev_for_fatal (int fd, const struct iovec *iov, size_t niov, size_t total) } #endif -struct str_list -{ - const char *str; - size_t len; - struct str_list *next; -}; - /* Abort with an error message. */ void -__libc_message (const char *fmt, ...) +__libc_message_impl (const char *fmt, ...) { va_list ap; int fd = -1; - va_start (ap, fmt); - #ifdef FATAL_PREPARE FATAL_PREPARE; #endif @@ -68,9 +59,11 @@ __libc_message (const char *fmt, ...) if (fd == -1) fd = STDERR_FILENO; - struct str_list *list = NULL; - int nlist = 0; + struct iovec iov[LIBC_MESSAGE_MAX_ARGS * 2 - 1]; + int iovcnt = 0; + ssize_t total = 0; + va_start (ap, fmt); const char *cp = fmt; while (*cp != '\0') { @@ -100,28 +93,16 @@ __libc_message (const char *fmt, ...) cp = next; } - struct str_list *newp = alloca (sizeof (struct str_list)); - newp->str = str; - newp->len = len; - newp->next = list; - list = newp; - ++nlist; + iov[iovcnt].iov_base = (char *) str; + iov[iovcnt].iov_len = len; + total += len; + iovcnt++; } + va_end (ap); - if (nlist > 0) + if (iovcnt > 0) { - struct iovec *iov = alloca (nlist * sizeof (struct iovec)); - ssize_t total = 0; - - for (int cnt = nlist - 1; cnt >= 0; --cnt) - { - iov[cnt].iov_base = (char *) list->str; - iov[cnt].iov_len = list->len; - total += list->len; - list = list->next; - } - - WRITEV_FOR_FATAL (fd, iov, nlist, total); + WRITEV_FOR_FATAL (fd, iov, iovcnt, total); total = (total + 1 + GLRO(dl_pagesize) - 1) & ~(GLRO(dl_pagesize) - 1); struct abort_msg_s *buf = __mmap (NULL, total, @@ -131,7 +112,7 @@ __libc_message (const char *fmt, ...) { buf->size = total; char *wp = buf->msg; - for (int cnt = 0; cnt < nlist; ++cnt) + for (int cnt = 0; cnt < iovcnt; ++cnt) wp = mempcpy (wp, iov[cnt].iov_base, iov[cnt].iov_len); *wp = '\0'; @@ -144,8 +125,6 @@ __libc_message (const char *fmt, ...) } } - va_end (ap); - /* Kill the application. */ abort (); }