Re: Correct but unhelpful VLA warning vs. gnulib's gettext.h; can we eliminate the false positive?

[Pádraig Brady]

On 09/07/11 15:32, James Youngman wrote:
> To be clear before we start, gnulib is doing the right thing here.  It
> contains this code in lib/gettext.h:-
> 
> static const char *
> dcpgettext_expr (const char *domain,
>                  const char *msgctxt, const char *msgid,
>                  int category)
> {
>   size_t msgctxt_len = strlen (msgctxt) + 1;
>   size_t msgid_len = strlen (msgid) + 1;
>   const char *translation;
> #if _LIBGETTEXT_HAVE_VARIABLE_SIZE_ARRAYS
>   char msg_ctxt_id[msgctxt_len + msgid_len];
> #else
>   char buf[1024];
>   char *msg_ctxt_id =
>     (msgctxt_len + msgid_len <= sizeof (buf)
>      ? buf
>      : (char *) malloc (msgctxt_len + msgid_len));
>   if (msg_ctxt_id != NULL)
> #endif
> 
> 
> tl;dr: it uses a variable-length array if we determined that the
> compiler supports those.   All well and good.   But, if we compile the
> code with more GCC warnings turned on via the manywarnings module, we
> get this result:

> In function 'dcpgettext_expr':
> /home/james/source/GNU/findutils/git/gnu/findutils/gl/lib/gettext.h:216:
> warning: variable length array 'msg_ctxt_id' is used

> In other words, "gcc -Wvla" is issuing a warning for a construct we
> know is safe.   However, I can't be sure I won't accidentally write
> code in the future which is not protected by something similar to
> _LIBGETTEXT_HAVE_VARIABLE_SIZE_ARRAYS.   So I think that -Wvla is a
> useful warning flag.
> 
> Is there a way of eliminating this false positive which doesn't force
> me to give up -Wvla?   I mean, apart from giving up the use of VLAs in
> gnulib even when it's safe to use them.

We might want to disable use of VLAs even if the compiler supports it,
for security reasons (like the Linux kernel now does), or if you
didn't want to consider VLA portability in gnulib using projects,
as you've suggested.

Attached allows one to define GNULIB_NO_VLA to support that,
which I've tested in coreutils with:

  AC_DEFINE([GNULIB_NO_VLA], [1], [Define to 1 to disable use of VLAs])

Note -Wvla is implicitly added by gl_MANYWARN_ALL_GCC,
so we don't need any special handling of this option once GNULIB_NO_VLA is 
defined.

cheers,
Pádraig

gnulib-no-vla.diff
Description: Text Data