stdio: Remove __libc_message alloca usage

  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>
---
 include/stdio.h            | 28 ++++++++++++++++++++++-
 sysdeps/posix/libc_fatal.c | 47 +++++++++++---------------------------
 2 files changed, 40 insertions(+), 35 deletions(-)
  

On 06/09/23 16:33, 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>

Patch looks ok, just a small comment below.

> ---
>  include/stdio.h            | 28 ++++++++++++++++++++++-
>  sysdeps/posix/libc_fatal.c | 47 +++++++++++---------------------------
>  2 files changed, 40 insertions(+), 35 deletions(-)
> 
> diff --git a/include/stdio.h b/include/stdio.h
> index 6755877911..e2100066ed 100644
> --- a/include/stdio.h
> +++ b/include/stdio.h
> @@ -172,10 +172,36 @@ 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;

Add a printf attribute to help catch issues with mismatch arguments as well:

__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];

Ok, it requires to add the string itself not only the arguments.

> +  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 ();
>  }
  

On Mon, Sep 11, 2023 at 09:00:23AM -0300, Adhemerval Zanella Netto wrote:
> 
> 
> On 06/09/23 16:33, 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>
> 
> Patch looks ok, just a small comment below.
> 
> > ---
> >  include/stdio.h            | 28 ++++++++++++++++++++++-
> >  sysdeps/posix/libc_fatal.c | 47 +++++++++++---------------------------
> >  2 files changed, 40 insertions(+), 35 deletions(-)
> > 
> > diff --git a/include/stdio.h b/include/stdio.h
> > index 6755877911..e2100066ed 100644
> > --- a/include/stdio.h
> > +++ b/include/stdio.h
> > @@ -172,10 +172,36 @@ 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;
> 
> Add a printf attribute to help catch issues with mismatch arguments as well:
> 
> __attribute__ ((__format__ (__printf__, 1, 2)))
> 

Ack.  Posted v2 with this change.

Thanks,
Joe