On 04/01/19 14:09, Bernhard Voelker wrote: > On 1/4/19 10:36 PM, Pádraig Brady wrote: >> On 04/01/19 13:07, Bernhard Voelker wrote: >>> On 1/4/19 6:38 PM, Pádraig Brady wrote: >>>> I wonder should we avoid VLAs in coreutils altogether? >>>> I.E. add -Werror=vla. The kernel has done this for security reaons. >>> >>> Well, why not? The only problem is still the one mentioned in >>> 'configure.ac': >>> >>> nw="$nw -Wvla" # warnings in gettext.h >>> >>> With -Werror=vla, we get a lot of warnings, but all originate from >>> these 2 places in gettext.h: >>> >>> ./lib/gettext.h: In function 'dcpgettext_expr': >>> ./lib/gettext.h:220:3: error: ISO C90 forbids variable length array >>> 'msg_ctxt_id' [-Werror=vla] >>> char msg_ctxt_id[msgctxt_len + msgid_len]; >>> ^~~~ >>> ./lib/gettext.h: In function 'dcnpgettext_expr': >>> ./lib/gettext.h:268:3: error: ISO C90 forbids variable length array >>> 'msg_ctxt_id' [-Werror=vla] >>> char msg_ctxt_id[msgctxt_len + msgid_len]; >>> ^~~~ >> >> It should be sufficient to also >> >> #define __STDC_NO_VLA__ 1 >> > > No, that is inactive in gettext.h; > > > #if (((__GNUC__ >= 3 || __GNUG__ >= 2) && !defined __STRICT_ANSI__) \ > /* || (__STDC_VERSION__ == 199901L && !defined __HP_cc) > || (__STDC_VERSION__ >= 201112L && !defined __STDC_NO_VLA__) */ ) > # define _LIBGETTEXT_HAVE_VARIABLE_SIZE_ARRAYS 1 > #else > # define _LIBGETTEXT_HAVE_VARIABLE_SIZE_ARRAYS 0 > #endif > > See Bruno's comment in gnulib commit a750b78e69: > > * lib/gettext.h (_LIBGETTEXT_HAVE_VARIABLE_SIZE_ARRAYS): Extend comment > to include the theoretical condition for availability of variable size > arrays, if we could trust the value of __STDC_VERSION__.
Ok I've sent a gnulib patch to support disbling VLA use with GNULIB_NO_VLA, which is enabled in coreutils with the attached. cheers, Pádraig
>From 2ce9531f47ad88f0a8f7d6b956dd18397a5fd99b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email protected]> Date: Sun, 13 Jan 2019 22:11:11 -0800 Subject: [PATCH] build: ensure VLAs are not used Fail developer builds if VLAs are used, as there are portability concerns to consider with them. * configure.ac: Enable -Wvla which is implicit in the full list added. * m4/jm-macros.m4: Define GNULIB_NO_VLA which disables use of VLAs within gnulib code. --- configure.ac | 1 - m4/jm-macros.m4 | 4 ++++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 13bb167..23086cd 100644 --- a/configure.ac +++ b/configure.ac @@ -130,7 +130,6 @@ if test "$gl_gcc_warnings" = yes; then nw="$nw -Wredundant-decls" # openat.h declares e.g., mkdirat nw="$nw -Wlogical-op" # Too many warnings until GCC 4.8.0 nw="$nw -Wformat-nonliteral" # who.c and pinky.c strftime uses - nw="$nw -Wvla" # warnings in gettext.h nw="$nw -Wnested-externs" # use of XARGMATCH/verify_function__ nw="$nw -Wswitch-enum" # Too many warnings for now nw="$nw -Wswitch-default" # Too many warnings for now diff --git a/m4/jm-macros.m4 b/m4/jm-macros.m4 index 39191de..3374137 100644 --- a/m4/jm-macros.m4 +++ b/m4/jm-macros.m4 @@ -41,6 +41,10 @@ AC_DEFUN([coreutils_MACROS], AC_DEFINE([ARGMATCH_DIE_DECL], [void usage (int _e)], [Define to the declaration of the xargmatch failure function.]) + # Ensure VLAs are not used. + # Note -Wvla is implicitly added by gl_MANYWARN_ALL_GCC + AC_DEFINE([GNULIB_NO_VLA], [1], [Define to 1 to disable use of VLAs]) + # used by shred AC_CHECK_FUNCS_ONCE([directio]) -- 2.9.3
