Re: coreutils and GCC -fanalyzer
On 7/10/20 2:21 PM, Pádraig Brady wrote: I'd be inclined to not enable -fanalyzer by default. At least not until it matures more. -fanalyzer didn't find any actual issues in coreutils, Yes, the only bug I found related to coreutils: https://gmplib.org/list-archives/gmp-bugs/2020-July/004844.html is a bug in library code that coreutils never triggers. I'd be inclined to not enable -fanalyzer by default. I installed the attached patch into coreutils; will that do? The basic idea is that you get -fanalyzer only if you configure with the new --enable-gcc-warnings=expensive option. >From 6b6f0f54c0eca06345b7741e5fd37b4500675286 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Fri, 10 Jul 2020 15:54:51 -0700 Subject: [PATCH] build: be less aggressive about -fanalyzer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * configure.ac: Don’t enable -fanalyzer unless configured with the new --enable-gcc-warnings=expensive option. See thread at: https://lists.gnu.org/r/coreutils/2020-07/msg00011.html --- configure.ac | 22 -- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/configure.ac b/configure.ac index 0afbff627..69d4e7d7f 100644 --- a/configure.ac +++ b/configure.ac @@ -89,10 +89,14 @@ AC_DEFUN([gl_GCC_VERSION_IFELSE], ) AC_ARG_ENABLE([gcc-warnings], - [AS_HELP_STRING([--enable-gcc-warnings], - [turn on many GCC warnings (for developers; best with GNU make)])], + [AS_HELP_STRING([--enable-gcc-warnings@<:@=TYPE@:>@], +[control generation of GCC warnings. The TYPE 'no' disables + warnings (default for non-developer builds); 'yes' generates + cheap warnings if available (default for developer builds); + 'expensive' in addition generates expensive-to-compute warnings + if available.])], [case $enableval in - yes|no) ;; + no|yes|expensive) ;; *) AC_MSG_ERROR([bad value $enableval for gcc-warnings option]) ;; esac gl_gcc_warnings=$enableval], @@ -108,12 +112,18 @@ AC_ARG_ENABLE([gcc-warnings], && gl_gcc_warnings=yes])] ) -if test "$gl_gcc_warnings" = yes; then +if test $gl_gcc_warnings != no; then gl_WARN_ADD([-Werror], [WERROR_CFLAGS]) AC_SUBST([WERROR_CFLAGS]) - nw= + ew= + AS_IF([test $gl_gcc_warnings != expensive], +[# -fanalyzer and related options slow GCC considerably. + ew="$ew -fanalyzer -Wno-analyzer-double-free -Wno-analyzer-malloc-leak" + ew="$ew -Wno-analyzer-null-dereference -Wno-analyzer-use-after-free"]) + # This, $nw, is the list of warnings we disable. + nw=$ew nw="$nw -Wdeclaration-after-statement" # too useful to forbid nw="$nw -Waggregate-return" # anachronistic nw="$nw -Wlong-long" # C90 is anachronistic (lib/gethrxtime.h) @@ -190,7 +200,7 @@ if test "$gl_gcc_warnings" = yes; then # We use a slightly smaller set of warning options for lib/. # Remove the following and save the result in GNULIB_WARN_CFLAGS. - nw= + nw=$ew nw="$nw -Wduplicated-branches"# Too many false alarms nw="$nw -Wformat-truncation=2" nw="$nw -Wstrict-overflow" -- 2.17.1
Re: coreutils and GCC -fanalyzer
On Fri, Jul 10, 2020 at 5:22 PM Pádraig Brady wrote: > > On 02/07/2020 01:26, Paul Eggert wrote: > > On 5/23/20 9:08 AM, Paul Eggert wrote: > > > >> So I am thinking of killing two > >> stones by doing the following. > >> > >> 1. Test for -fanalyzer, -Wall, -Wextra. > >> > >> 2. Test for flags not automatically enabled by -fanalyzer, -Wall, -Wextra > >> but > >> flags that we want anyway. > >> > >> 3. Test for flags automatically enabled by -fanalyzer, -Wall, -Wextra that > >> are > >> also flags that we don't want. > > > > I did that in Gnulib by installing the attached patch. This could greatly > > increase compile times due to the -fanalyzer option, so let's keep an eye > > out > > for that. > > s/could/does/. > compile time has gone from seconds to minutes for coreutils at least. > > I'd be inclined to not enable -fanalyzer by default. > At least not until it matures more. > -fanalyzer didn't find any actual issues in coreutils, > and currently bails on most files (after taking lots of time about it), > as indicated with the -Wanalyzer-too-complex option. Just my 2 cents, but leave analyzer options to testing and QA folks. Make the option available for convenience, but leave it off by default. The testers and QA folks will perform the enhanced testing on occasion, like during a CI build or manually before a release. The remainder of the time, regular users get a regular build. Jeff
Re: coreutils and GCC -fanalyzer
On 02/07/2020 01:26, Paul Eggert wrote: On 5/23/20 9:08 AM, Paul Eggert wrote: So I am thinking of killing two stones by doing the following. 1. Test for -fanalyzer, -Wall, -Wextra. 2. Test for flags not automatically enabled by -fanalyzer, -Wall, -Wextra but flags that we want anyway. 3. Test for flags automatically enabled by -fanalyzer, -Wall, -Wextra that are also flags that we don't want. I did that in Gnulib by installing the attached patch. This could greatly increase compile times due to the -fanalyzer option, so let's keep an eye out for that. s/could/does/. compile time has gone from seconds to minutes for coreutils at least. I'd be inclined to not enable -fanalyzer by default. At least not until it matures more. -fanalyzer didn't find any actual issues in coreutils, and currently bails on most files (after taking lots of time about it), as indicated with the -Wanalyzer-too-complex option. cheers, Pádraig
Re: Callbacks in the abstact data types and extra contextual data
Marc Nieper-Wißkirchen wrote: > For GCC one can use nested functions, but how can > 'partial_function_last' be implemented in ISO C? It can be implemented for each CPU type and ABI. But this is OK since the number of CPU types are hardly increasing nowadays. It cannot be implemented in ISO C. > For GCC one can use nested functions GCC handles each CPU type and ABI separately as well. Bruno
Re: Callbacks in the abstact data types and extra contextual data
Hi Bruno, Am Fr., 10. Juli 2020 um 20:38 Uhr schrieb Bruno Haible : > OK. Then let's take the problem seriously. If your solution can be implemented portable, that will be the best solution by far. For GCC one can use nested functions, but how can 'partial_function_last' be implemented in ISO C? > I think it's time to solve the problem once and for all. I propose to add > a module that defines a function 'partial_function_last' such that, when > you have a function pointer > >int (*cmp3) (void *arg1, void *arg2, void *context) > > then > >cmp2 = partial_function_last (cmp3, context); partial_function_last has to return the address of an existing function. But this function has no context accept for global (thread local) variables. How can this function know about 'cmp3'? Marc
Re: Callbacks in the abstact data types and extra contextual data
Hi Marc, > > > a number of modules (like the hash module or the list module) allow > > > the user to specify callbacks (e.g. a comparison function). > > > Unfortunately, these procedures do not take a context parameter, which > > > can be a problem because C lacks closures. > > > > Is this a practical, actual problem, or only a theoretical one? > > It is a practical one; in a computer algebra application, I have lots > of small entities (two words each), whose sort order is determined by > the extra data. OK. Then let's take the problem seriously. I think it's time to solve the problem once and for all. I propose to add a module that defines a function 'partial_function_last' such that, when you have a function pointer int (*cmp3) (void *arg1, void *arg2, void *context) then cmp2 = partial_function_last (cmp3, context); produces a function pointer int (*cmp2) (void *arg1, void *arg2) that invokes cmp3 with the given context. Wikipedia calls it "partial function application". The module will also have a function 'partial_function_free' that frees a function that was constructed in this way. Give me a couple of days to implement that. Bruno
Re: portability issues with unicodeio
> * tests/test-unicodeio.c (main): In the "C" locale, expect either the > UTF-8 output or the specified fallback. Now that we have a unit test, I see that the test fails on NetBSD 9.0. This patch fixes it. 2020-07-10 Bruno Haible unicodeio: Fix wrong result on NetBSD. * lib/unicodeio.c (unicode_to_mb): Handle question mark fallback characters also on NetBSD. diff --git a/lib/unicodeio.c b/lib/unicodeio.c index 81fe0dd..b616e3d 100644 --- a/lib/unicodeio.c +++ b/lib/unicodeio.c @@ -134,9 +134,10 @@ unicode_to_mb (unsigned int code, # if !defined _LIBICONV_VERSION && (defined sgi || defined __sgi) || (res > 0 && code != 0 && outptr - outbuf == 1 && *outbuf == '\0') # endif - /* Solaris 11 iconv() inserts a '?' if it cannot convert. */ -# if !defined _LIBICONV_VERSION && defined __sun - || (res > 0 && code != 0 && outptr - outbuf == 1 && *outbuf == '?') + /* NetBSD iconv() and Solaris 11 iconv() insert a '?' if they cannot + convert. */ +# if !defined _LIBICONV_VERSION && (defined __NetBSD__ || defined __sun) + || (res > 0 && outptr - outbuf == 1 && *outbuf == '?') # endif ) return failure (code, NULL, callback_arg);
