Re: stack module

[Paul Eggert]

On 5/23/20 3:06 PM, Bruno Haible wrote:
> How about this instead?

Thanks, good point about the danger. Also, I forgot to include verify.h.

I tightened up the commentary, folded in Marc's suggestion, and installed the
attached.
>From 2df56dc44200074077ebace04079ac4b0a34e4fc Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Sat, 23 May 2020 19:06:16 -0700
Subject: [PATCH] =?UTF-8?q?assure:=20new=20macro=20=E2=80=98affirm?=
 =?UTF-8?q?=E2=80=99?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* lib/assure.h: Include verify.h.
(affirm): New macro, after a suggestion by Marc Nieper-Wißkirchen in:
https://lists.gnu.org/r/bug-gnulib/2020-05/msg00263.html
and commentary by Bruno Haible in:
https://lists.gnu.org/r/bug-gnulib/2020-05/msg00278.html
* modules/assure (Depends-on:): Add verify.
---
 ChangeLog  | 10 ++
 lib/assure.h   | 24 ++--
 modules/assure |  1 +
 3 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index a7101db80..c05ef910d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2020-05-23  Paul Eggert  
+
+	assure: new macro ‘affirm’
+	* lib/assure.h: Include verify.h.
+	(affirm): New macro, after a suggestion by Marc Nieper-Wißkirchen in:
+	https://lists.gnu.org/r/bug-gnulib/2020-05/msg00263.html
+	and commentary by Bruno Haible in:
+	https://lists.gnu.org/r/bug-gnulib/2020-05/msg00278.html
+	* modules/assure (Depends-on:): Add verify.
+
 2020-05-23  Bruno Haible  
 
 	calloc-gnu: Make test work in non-flat address spaces.
diff --git a/lib/assure.h b/lib/assure.h
index 8ea2f6e48..09a4edfa5 100644
--- a/lib/assure.h
+++ b/lib/assure.h
@@ -21,12 +21,32 @@
 #define _GL_ASSURE_H
 
 #include 
+#include "verify.h"
+
+/* Evaluate an assertion E that is guaranteed to be true.
+   If NDEBUG is not defined, abort the program if E is false.
+   If NDEBUG is defined, the compiler can assume E and behavior is
+   undefined if E is false, fails to evaluate, or has side effects.
+
+   Unlike standard 'assert', this macro evaluates E even when NDEBUG
+   is defined, so as to catch typos, avoid some GCC warnings, and
+   improve performance when E is simple enough.
+
+   Also see the documentation for 'assume' in verify.h.  */
+
+#ifdef NDEBUG
+# define affirm(E) assume (E)
+#else
+# define affirm(E) assert (E)
+#endif
 
 /* Check E's value at runtime, and report an error and abort if not.
However, do nothing if NDEBUG is defined.
 
-   Unlike standard 'assert', this macro always compiles E even when NDEBUG
-   is defined, so as to catch typos and avoid some GCC warnings.  */
+   Unlike standard 'assert', this macro compiles E even when NDEBUG
+   is defined, so as to catch typos and avoid some GCC warnings.
+   Unlike 'affirm', it is OK for E to use hard-to-optimize features,
+   since E is not executed if NDEBUG is defined.  */
 
 #ifdef NDEBUG
 # define assure(E) ((void) (0 && (E)))
diff --git a/modules/assure b/modules/assure
index 3cfe1f874..c37191717 100644
--- a/modules/assure
+++ b/modules/assure
@@ -5,6 +5,7 @@ Files:
 lib/assure.h
 
 Depends-on:
+verify
 
 configure.ac:
 
-- 
2.17.1

Re: valgrind-tests and suppression files

[Jeffrey Walton]

On Sat, May 23, 2020 at 5:40 PM Marc Nieper-Wißkirchen
 wrote:
>
> Is there any built-in support in the valgrind-tests module for
> suppression files? For example, if I want to run my tests under
> valgrind (if available), I write
>
> LOG_COMPILER = $(VALGRIND)
>
> or, in case I use libtool,
>
> LOG_COMPILER = $(LIBTOOL) --mode=execute $(VALGRIND)
>
> But how to add valgrind suppression files in this context?
>
> LOG_COMPILER = $(VALGRIND) --suppressions=...
>
> is not a good solution because it will fail when VALGRIND is empty
> because valgrind-tests couldn't find a valgrind.
>
> What's the canonical solution of this problem?

Placing Valgrind annotations in the source file so the annotations
cannot be disgorged from the source file. It also benefits users so
things "just work" for them out of the box. No reading manuals, no
crafting suppression files.

I don't know if Valgrind support has matured that far, though.

Jeff

Re: Fix calloc.m4 test

[Paul Eggert]

On 5/23/20 2:53 PM, Bruno Haible wrote:
> Does this look like a reportable bug? :-)

Absolutely!

Re: stack module

[Bruno Haible]

Hi Paul,

> Something like the attached?

This documentation is quite misleading:

  +/* Check E's value at runtime, and report an error and abort if not.
  +   However, do nothing if NDEBUG is defined; in this case behavior is

because the "do nothing" is an understatement. 'assume' is a dangerous macro,
because it allows the compiler to do optimizations based on putative
assumptions. Since 'affirm' invokes 'assume', the documentation should
emphasize this danger.

How about this instead?

/* States an assertion that the programmer believes to be true.
   When NDEBUG is not defined, this macro is like assert: it verifies the
   assertion at run time and aborts the program if the assertion is not true.
   When NDEBUG is defined, the programmer guarantees that the assertion is
   true, and the macro informs the compiler about it, so that the compiler
   may produce optimized code based on it.  */

Bruno

Re: Fix calloc.m4 test

[Bruno Haible]

Hi Paul,

> Unfortunately Vincent Lefevre is correct that the C Standard allows calloc
> (SIZE_MAX, 2) to succeed on a hypothetical machine that actually can allocate
> that amount of memory. This could in theory occur on a machine that doesn't 
> have
> a flat address space.
> 
> A program like the following should defeat Clang's optimization, though (at
> least, if Clang is not buggy). And it would also detect the hypothetical 
> machine
> that Vincent alluded to, which would make it more-accurate than the 'volatile'
> workaround. How about if we switch calloc.m4 to use this test instead?
> 
>   #include 
>   int main ()
>   {
> struct { char c[8]; } *s;
> size_t sn = (size_t) -1 / sizeof *s + 2;
> int result = 0;
> char *p = calloc (0, 0);
> if (!p)
>   result |= 1;
> free (p);
> s = calloc (sn, sizeof *s);
> if (s)
>   {
>   s[0].c[0] = 1;
>   if (s[sn - 1].c[0])
> result |= 2;
>   }
> free (s);
> return result;
>   }

OK, I'm adding this to calloc.m4. Patch below. However, the 'volatile' is still
needed. And even if it wasn't, it would be good to keep two arrows in our
quiver.

Now, this looks really like a bug that you could report: With clang 10.0.0
on x86_64 (Ubuntu 18.04), the following program

= foo.c 
#include 
int main ()
{
  int result;
  typedef struct { char c[8]; } S8;
  size_t n = (size_t) -1 / sizeof (S8) + 2;
  S8 *s = calloc (n, sizeof (S8));
  if (s)
{
  s[0].c[0] = 1;
  if (s[n - 1].c[0])
result = 0;
  else
result = 2;
}
  else
result = 3;
  free (s);
  return result;
}

returns with exit code 0, when optimization is enabled:

$ clang -O2 foo.c
$ ./a.out; echo $?
0

When no optimization is enabled, it returns with exit code 3, as
expected.

Does this look like a reportable bug? :-)


2020-05-23  Bruno Haible  

calloc-gnu: Make test work in non-flat address spaces.
Uses code by Paul Eggert.
* m4/calloc.m4 (_AC_FUNC_CALLOC_IF): Allow a calloc() implementation
to return more than SIZE_MAX bytes, but only without wrap-around bugs.

diff --git a/m4/calloc.m4 b/m4/calloc.m4
index a93439e..f179a8a 100644
--- a/m4/calloc.m4
+++ b/m4/calloc.m4
@@ -1,4 +1,4 @@
-# calloc.m4 serial 22
+# calloc.m4 serial 23
 
 # Copyright (C) 2004-2020 Free Software Foundation, Inc.
 # This file is free software; the Free Software Foundation
@@ -38,16 +38,27 @@ AC_DEFUN([_AC_FUNC_CALLOC_IF],
AC_RUN_IFELSE(
  [AC_LANG_PROGRAM(
 [AC_INCLUDES_DEFAULT],
-[[int result = 0;
-  char * volatile p = calloc ((size_t) -1 / 8 + 1, 8);
-  if (!p)
-result |= 2;
-  free (p);
+[[int result;
+  typedef struct { char c[8]; } S8;
+  size_t n = (size_t) -1 / sizeof (S8) + 2;
+  S8 * volatile s = calloc (n, sizeof (S8));
+  if (s)
+{
+  s[0].c[0] = 1;
+  if (s[n - 1].c[0])
+result = 0;
+  else
+result = 2;
+}
+  else
+result = 3;
+  free (s);
   return result;
 ]])],
- dnl The exit code of this program is 0 if calloc() succeeded (which
- dnl it shouldn't), 2 if calloc() failed, or 1 if some leak sanitizer
- dnl terminated the program as a result of the calloc() call.
+ dnl The exit code of this program is 0 if calloc() succeeded with a
+ dnl wrap-around bug (which it shouldn't), 2 if calloc() succeeded in
+ dnl a non-flat address space, 3 if calloc() failed, or 1 if some leak
+ dnl sanitizer terminated the program as a result of the calloc() call.
  [ac_cv_func_calloc_0_nonnull=no],
  [])
  else

valgrind-tests and suppression files

[Marc Nieper-Wißkirchen]

Is there any built-in support in the valgrind-tests module for
suppression files? For example, if I want to run my tests under
valgrind (if available), I write

LOG_COMPILER = $(VALGRIND)

or, in case I use libtool,

LOG_COMPILER = $(LIBTOOL) --mode=execute $(VALGRIND)

But how to add valgrind suppression files in this context?

LOG_COMPILER = $(VALGRIND) --suppressions=...

is not a good solution because it will fail when VALGRIND is empty
because valgrind-tests couldn't find a valgrind.

What's the canonical solution of this problem?

Re: stack module

[Marc Nieper-Wißkirchen]

Am Sa., 23. Mai 2020 um 22:51 Uhr schrieb Paul Eggert :
>
> On 5/23/20 10:38 AM, Marc Nieper-Wißkirchen wrote:
> > But if "affirm" is fine with you, I would love to see it in a module.
> > Either in verify or assure or in a new module named affirm.
>
> Something like the attached?

That would be a good addition to Gnulib, indeed, I think. (And may
even already be used by a number of modules, which currently call
abort explicitly.)

I find the comments in your code very valuable. I may change only one
thing. Above the definition of affirm, you write "Unlike standard
'assert', this macro always compiles E even when NDEBUG is defined
...". I think it is better to change it to "... always evaluates E
even when ...". In fact, when the assumption is fulfilled, E must have
been evaluated. If the assumption fails we are in the realm of
undefined behavior, so we may as well claim that E has been evaluated.
This is important not only so that the side effects of the evaluation
of E are documented, but also to remind the user of a potentially
costly evaluation.

Re: stack module

[Paul Eggert]

On 5/23/20 10:38 AM, Marc Nieper-Wißkirchen wrote:
> But if "affirm" is fine with you, I would love to see it in a module.
> Either in verify or assure or in a new module named affirm.

Something like the attached?
diff --git a/lib/assure.h b/lib/assure.h
index 8ea2f6e48..2f1326027 100644
--- a/lib/assure.h
+++ b/lib/assure.h
@@ -22,11 +22,29 @@
 
 #include 
 
+/* Check E's value at runtime, and report an error and abort if not.
+   However, do nothing if NDEBUG is defined; in this case behavior is
+   undefined if E is false, fails to evaluate, or has side effects.
+
+   Unlike standard 'assert', this macro always compiles E even when
+   NDEBUG is defined, so as to catch typos, avoid some GCC warnings,
+   and improve performance when E is simple enough.
+
+   Also see the documentation for 'assume' in verify.h.  */
+
+#ifdef NDEBUG
+# define affirm(E) assume (E)
+#else
+# define affirm(E) assert (E)
+#endif
+
 /* Check E's value at runtime, and report an error and abort if not.
However, do nothing if NDEBUG is defined.
 
Unlike standard 'assert', this macro always compiles E even when NDEBUG
-   is defined, so as to catch typos and avoid some GCC warnings.  */
+   is defined, so as to catch typos and avoid some GCC warnings.
+   Unlike 'affirm', it is OK for E to use hard-to-optimize features,
+   since E is not executed if NDEBUG is defined.  */
 
 #ifdef NDEBUG
 # define assure(E) ((void) (0 && (E)))
diff --git a/modules/assure b/modules/assure
index 3cfe1f874..c37191717 100644
--- a/modules/assure
+++ b/modules/assure
@@ -5,6 +5,7 @@ Files:
 lib/assure.h
 
 Depends-on:
+verify
 
 configure.ac:
 

Re: Fix memleak in getdelim.m4

[Bruno Haible]

Tim Rühsen wrote:
> - clang-10 with unset CFLAGS (*.clang-10)
> 
> And the same set with -fsanitize... (*.sanitize).

The other differences between the configure results without and with sanitizers
can be explained by the fact that these -fsanitize options have the effect that
the resulting executables are linked with
  - libpthread,
  - librt
  - libm
  - libdl
by default.

Consequences of linking with libpthread:

-ac_cv_func_pthread_sigmask=${ac_cv_func_pthread_sigmask=no}
+ac_cv_func_pthread_sigmask=${ac_cv_func_pthread_sigmask=yes}
-ac_cv_func_thrd_create=${ac_cv_func_thrd_create=no}
+ac_cv_func_thrd_create=${ac_cv_func_thrd_create=yes}
-ac_cv_lib_stdthreads_thrd_create=${ac_cv_lib_stdthreads_thrd_create=no}
+ac_cv_lib_pthread_pthread_kill=${ac_cv_lib_pthread_pthread_kill=yes}
-gl_cv_func_pthread_sigmask_in_LIBMULTITHREAD=${gl_cv_func_pthread_sigmask_in_LIBMULTITHREAD=yes}
+gl_cv_func_pthread_sigmask_in_libc_works=${gl_cv_func_pthread_sigmask_in_libc_works=yes}

Consequences of linking with librt:

-ac_cv_search_timer_settime=${ac_cv_search_timer_settime=-lrt}
+ac_cv_search_timer_settime=${ac_cv_search_timer_settime='none required'}

Consequences of linking with libm:

-gl_cv_func_acosl_in_libm=${gl_cv_func_acosl_in_libm=yes}
-gl_cv_func_acosl_no_libm=${gl_cv_func_acosl_no_libm=no}
-gl_cv_func_asinl_in_libm=${gl_cv_func_asinl_in_libm=yes}
-gl_cv_func_asinl_no_libm=${gl_cv_func_asinl_no_libm=no}
-gl_cv_func_atanl_in_libm=${gl_cv_func_atanl_in_libm=yes}
-gl_cv_func_atanl_no_libm=${gl_cv_func_atanl_no_libm=no}
+gl_cv_func_acosl_no_libm=${gl_cv_func_acosl_no_libm=yes}
+gl_cv_func_asinl_no_libm=${gl_cv_func_asinl_no_libm=yes}
+gl_cv_func_atanl_no_libm=${gl_cv_func_atanl_no_libm=yes}
-gl_cv_func_ceil_libm=${gl_cv_func_ceil_libm=-lm}
+gl_cv_func_ceil_libm=${gl_cv_func_ceil_libm=}
-gl_cv_func_ceilf_libm=${gl_cv_func_ceilf_libm=-lm}
+gl_cv_func_ceilf_libm=${gl_cv_func_ceilf_libm=}
-gl_cv_func_ceill_libm=${gl_cv_func_ceill_libm=-lm}
+gl_cv_func_ceill_libm=${gl_cv_func_ceill_libm=}
-gl_cv_func_cosl_in_libm=${gl_cv_func_cosl_in_libm=yes}
-gl_cv_func_cosl_no_libm=${gl_cv_func_cosl_no_libm=no}
+gl_cv_func_cosl_no_libm=${gl_cv_func_cosl_no_libm=yes}
-gl_cv_func_exp2_in_libm=${gl_cv_func_exp2_in_libm=yes}
-gl_cv_func_exp2_no_libm=${gl_cv_func_exp2_no_libm=no}
+gl_cv_func_exp2_no_libm=${gl_cv_func_exp2_no_libm=yes}
-gl_cv_func_expl_in_libm=${gl_cv_func_expl_in_libm=yes}
-gl_cv_func_expl_no_libm=${gl_cv_func_expl_no_libm=no}
+gl_cv_func_expl_no_libm=${gl_cv_func_expl_no_libm=yes}
-gl_cv_func_expm1_in_libm=${gl_cv_func_expm1_in_libm=yes}
-gl_cv_func_expm1_no_libm=${gl_cv_func_expm1_no_libm=no}
+gl_cv_func_expm1_no_libm=${gl_cv_func_expm1_no_libm=yes}
-gl_cv_func_expm1l_in_libm=${gl_cv_func_expm1l_in_libm=yes}
-gl_cv_func_expm1l_no_libm=${gl_cv_func_expm1l_no_libm=no}
+gl_cv_func_expm1l_no_libm=${gl_cv_func_expm1l_no_libm=yes}
-gl_cv_func_fabs_in_libm=${gl_cv_func_fabs_in_libm=yes}
-gl_cv_func_fabs_no_libm=${gl_cv_func_fabs_no_libm=no}
-gl_cv_func_fabsf_in_libm=${gl_cv_func_fabsf_in_libm=yes}
-gl_cv_func_fabsf_no_libm=${gl_cv_func_fabsf_no_libm=no}
-gl_cv_func_fabsl_in_libm=${gl_cv_func_fabsl_in_libm=yes}
-gl_cv_func_fabsl_no_libm=${gl_cv_func_fabsl_no_libm=no}
+gl_cv_func_fabs_no_libm=${gl_cv_func_fabs_no_libm=yes}
+gl_cv_func_fabsf_no_libm=${gl_cv_func_fabsf_no_libm=yes}
+gl_cv_func_fabsl_no_libm=${gl_cv_func_fabsl_no_libm=yes}
-gl_cv_func_fegetround_in_libm=${gl_cv_func_fegetround_in_libm=yes}
-gl_cv_func_fegetround_no_libm=${gl_cv_func_fegetround_no_libm=no}
+gl_cv_func_fegetround_no_libm=${gl_cv_func_fegetround_no_libm=yes}
-gl_cv_func_floor_libm=${gl_cv_func_floor_libm=-lm}
+gl_cv_func_floor_libm=${gl_cv_func_floor_libm=}
-gl_cv_func_floorf_libm=${gl_cv_func_floorf_libm=-lm}
-gl_cv_func_floorl_libm=${gl_cv_func_floorl_libm=-lm}
-gl_cv_func_fma_in_libm=${gl_cv_func_fma_in_libm=yes}
-gl_cv_func_fma_no_libm=${gl_cv_func_fma_no_libm=no}
+gl_cv_func_floorf_libm=${gl_cv_func_floorf_libm=}
+gl_cv_func_floorl_libm=${gl_cv_func_floorl_libm=}
+gl_cv_func_fma_no_libm=${gl_cv_func_fma_no_libm=yes}
-gl_cv_func_fmaf_in_libm=${gl_cv_func_fmaf_in_libm=yes}
-gl_cv_func_fmaf_no_libm=${gl_cv_func_fmaf_no_libm=no}
+gl_cv_func_fmaf_no_libm=${gl_cv_func_fmaf_no_libm=yes}
-gl_cv_func_fmal_in_libm=${gl_cv_func_fmal_in_libm=yes}
-gl_cv_func_fmal_no_libm=${gl_cv_func_fmal_no_libm=no}
+gl_cv_func_fmal_no_libm=${gl_cv_func_fmal_no_libm=yes}
-gl_cv_func_ilogb_in_libm=${gl_cv_func_ilogb_in_libm=yes}
-gl_cv_func_ilogb_no_libm=${gl_cv_func_ilogb_no_libm=no}
+gl_cv_func_ilogb_no_libm=${gl_cv_func_ilogb_no_libm=yes}
-gl_cv_func_ilogbf_in_libm=${gl_cv_func_ilogbf_in_libm=yes}
-gl_cv_func_ilogbf_no_libm=${gl_cv_func_ilogbf_no_libm=no}
+gl_cv_func_ilogbf_no_libm=${gl_cv_func_ilogbf_no_libm=yes}
-gl_cv_func_logbf_in_libm=${gl_cv_func_logbf_in_libm=yes}
-gl_cv_func_logbf_no_libm=${gl_cv_func_logbf_no_libm=no}
+gl_cv_func_logbf_no_libm=${gl_cv_func_logbf_no_libm=yes}
-gl_cv_func_logbl_in_libm=${gl_cv_func_logbl_in_libm=yes}

Re: Fix calloc.m4 test

[Paul Eggert]

On 5/23/20 11:48 AM, Bruno Haible wrote:
>   2) It has estimated that the second call would return a non-NULL pointer,
>  although the address space does not allow this.
>  Reported at . But some
>  people claim it is not a bug. Paul, can you please help with ISO C
>  citations?

Unfortunately Vincent Lefevre is correct that the C Standard allows calloc
(SIZE_MAX, 2) to succeed on a hypothetical machine that actually can allocate
that amount of memory. This could in theory occur on a machine that doesn't have
a flat address space.

A program like the following should defeat Clang's optimization, though (at
least, if Clang is not buggy). And it would also detect the hypothetical machine
that Vincent alluded to, which would make it more-accurate than the 'volatile'
workaround. How about if we switch calloc.m4 to use this test instead?

  #include 
  int main ()
  {
struct { char c[8]; } *s;
size_t sn = (size_t) -1 / sizeof *s + 2;
int result = 0;
char *p = calloc (0, 0);
if (!p)
  result |= 1;
free (p);
s = calloc (sn, sizeof *s);
if (s)
  {
s[0].c[0] = 1;
if (s[sn - 1].c[0])
  result |= 2;
  }
free (s);
return result;
  }

Fix calloc-gnu configure results

[Bruno Haible]

Tim Rühsen wrote:
> - gcc-10 with unset CFLAGS (*.gcc-10)
> And the same set with -fsanitize... (*.sanitize).

Looking at the differences between config.cache for the gcc-10 without and with
sanitizers, there are some changes that are due to present vs. missing -O2
options (gl_cv_c_inline_effective and possibly gl_cv_func_printf_directive_n).

Other than that:

-ac_cv_func_calloc_0_nonnull=${ac_cv_func_calloc_0_nonnull=yes}
+ac_cv_func_calloc_0_nonnull=${ac_cv_func_calloc_0_nonnull=no}

Here the problem is that in the test program, we expect
  calloc ((size_t) -1 / 8 + 1, 8)
to return NULL, but the AddressSanitizer instead makes the program
exit with code 1:

  configure:45855: ./conftest
  =
  ==462953==ERROR: AddressSanitizer: calloc parameters overflow: count * size 
(2305843009213693952 * 8) cannot be represented in type size_t (thread T0)
  #0 0x7f01a2073037 in calloc 
(/usr/lib/x86_64-linux-gnu/libasan.so.6+0xaa037)
  #1 0x5653669731b1 in main (/home/tim/src/testdir-all/conftest+0x11b1)
  #2 0x7f01a14c2e0a in __libc_start_main ../csu/libc-start.c:308

  ==462953==HINT: if you don't care about these errors you may set 
allocator_may_return_null=1
  SUMMARY: AddressSanitizer: calloc-overflow 
(/usr/lib/x86_64-linux-gnu/libasan.so.6+0xaa037) in calloc
  ==462953==ABORTING
  configure:45855: $? = 1
  configure: program exited with status 1

This patch fixed it.


2020-05-23  Bruno Haible  

calloc-gnu: Avoid wrong configure results with GCC's AddressSanitizer.
* m4/calloc.m4 (_AC_FUNC_CALLOC_IF): Split the AC_RUN_IFELSE into two
AC_RUN_IFELSE invocations.

diff --git a/m4/calloc.m4 b/m4/calloc.m4
index 3361cba..a93439e 100644
--- a/m4/calloc.m4
+++ b/m4/calloc.m4
@@ -1,4 +1,4 @@
-# calloc.m4 serial 21
+# calloc.m4 serial 22
 
 # Copyright (C) 2004-2020 Free Software Foundation, Inc.
 # This file is free software; the Free Software Foundation
@@ -21,33 +21,48 @@ AC_DEFUN([_AC_FUNC_CALLOC_IF],
   AC_REQUIRE([AC_CANONICAL_HOST]) dnl for cross-compiles
   AC_CACHE_CHECK([for GNU libc compatible calloc],
 [ac_cv_func_calloc_0_nonnull],
-[AC_RUN_IFELSE(
-   [AC_LANG_PROGRAM(
-  [AC_INCLUDES_DEFAULT],
-  [[int result = 0;
-char * volatile p = calloc (0, 0);
-if (!p)
-  result |= 1;
-free (p);
-p = calloc ((size_t) -1 / 8 + 1, 8);
-if (p)
-  result |= 2;
-free (p);
-return result;
-  ]])],
-   [ac_cv_func_calloc_0_nonnull=yes],
-   [ac_cv_func_calloc_0_nonnull=no],
-   [case "$host_os" in
- # Guess yes on glibc systems.
-  *-gnu* | gnu*) ac_cv_func_calloc_0_nonnull="guessing yes" ;;
- # Guess yes on musl systems.
-  *-musl*)   ac_cv_func_calloc_0_nonnull="guessing yes" ;;
- # Guess yes on native Windows.
-  mingw*)ac_cv_func_calloc_0_nonnull="guessing yes" ;;
- # If we don't know, obey --enable-cross-guesses.
-  *) ac_cv_func_calloc_0_nonnull="$gl_cross_guess_normal" 
;;
-esac
-   ])])
+[if test $cross_compiling != yes; then
+   ac_cv_func_calloc_0_nonnull=yes
+   AC_RUN_IFELSE(
+ [AC_LANG_PROGRAM(
+[AC_INCLUDES_DEFAULT],
+[[int result = 0;
+  char * volatile p = calloc (0, 0);
+  if (!p)
+result |= 1;
+  free (p);
+  return result;
+]])],
+ [],
+ [ac_cv_func_calloc_0_nonnull=no])
+   AC_RUN_IFELSE(
+ [AC_LANG_PROGRAM(
+[AC_INCLUDES_DEFAULT],
+[[int result = 0;
+  char * volatile p = calloc ((size_t) -1 / 8 + 1, 8);
+  if (!p)
+result |= 2;
+  free (p);
+  return result;
+]])],
+ dnl The exit code of this program is 0 if calloc() succeeded (which
+ dnl it shouldn't), 2 if calloc() failed, or 1 if some leak sanitizer
+ dnl terminated the program as a result of the calloc() call.
+ [ac_cv_func_calloc_0_nonnull=no],
+ [])
+ else
+   case "$host_os" in
+# Guess yes on glibc systems.
+ *-gnu* | gnu*) ac_cv_func_calloc_0_nonnull="guessing yes" ;;
+# Guess yes on musl systems.
+ *-musl*)   ac_cv_func_calloc_0_nonnull="guessing yes" ;;
+# Guess yes on native Windows.
+ mingw*)ac_cv_func_calloc_0_nonnull="guessing yes" ;;
+# If we don't know, obey --enable-cross-guesses.
+ *) ac_cv_func_calloc_0_nonnull="$gl_cross_guess_normal" ;;
+   esac
+ fi
+])
   case "$ac_cv_func_calloc_0_nonnull" in
 *yes)
   $1

Fix invalid use of __builtin_isnanf and __builtin_isnanl

[Bruno Haible]

Tim Rühsen wrote:
> - gcc-10 with unset CFLAGS (*.gcc-10)
> - clang-10 with unset CFLAGS (*.clang-10)

Another difference between these test results is this:

-gl_cv_func_isnanf_no_libm=${gl_cv_func_isnanf_no_libm=yes}
-gl_cv_func_isnanf_works=${gl_cv_func_isnanf_works=yes}
-gl_cv_func_isnanl_no_libm=${gl_cv_func_isnanl_no_libm=yes}
-gl_cv_func_isnanl_works=${gl_cv_func_isnanl_works=yes}
+gl_cv_func_isnanf_in_libm=${gl_cv_func_isnanf_in_libm=no}
+gl_cv_func_isnanf_no_libm=${gl_cv_func_isnanf_no_libm=no}
+gl_cv_func_isnanl_in_libm=${gl_cv_func_isnanl_in_libm=no}
+gl_cv_func_isnanl_no_libm=${gl_cv_func_isnanl_no_libm=no}

config.log shows that clang has no __builtin_isnanf and __builtin_isnanl,
although it defines __GNUC__ to a value >= 4. This patch fixes it.


2020-05-23  Bruno Haible  

isnanf, isnanl, isnan: Don't use nonexistent builtins with clang.
* m4/isnanf.m4 (gl_HAVE_ISNANF_NO_LIBM, gl_HAVE_ISNANF_IN_LIBM,
gl_ISNANF_WORKS): Don't use __builtin_isnanf on clang versions that
don't have it.
* m4/isnanl.m4 (gl_HAVE_ISNANL_NO_LIBM, gl_HAVE_ISNANL_IN_LIBM,
gl_FUNC_ISNANL_WORKS): Don't use __builtin_isnanl on clang versions that
don't have it.
* lib/isnanf-nolibm.h (__has_builtin): New macro.
(isnanf): Don't use __builtin_isnanf on clang versions that don't have
it.
* lib/isnanl-nolibm.h (__has_builtin): New macro.
(isnanl): Don't use __builtin_isnanl on clang versions that don't have
it.
* lib/math.in.h (__has_builtin): New macro.
(isnanf): Don't use __builtin_isnanf on clang versions that don't have
it.
(isnanl): Don't use __builtin_isnanl on clang versions that don't have
it.
(isnan): Don't use the builtins on clang versions that don't have
__builtin_isnanf and __builtin_isnanl.

diff --git a/lib/isnanf-nolibm.h b/lib/isnanf-nolibm.h
index 647ffed..17a2f5e 100644
--- a/lib/isnanf-nolibm.h
+++ b/lib/isnanf-nolibm.h
@@ -17,7 +17,10 @@
 #if HAVE_ISNANF_IN_LIBC
 /* Get declaration of isnan macro or (older) isnanf function.  */
 # include 
-# if __GNUC__ >= 4
+# ifndef __has_builtin
+#  define __has_builtin(name) 0
+# endif
+# if __GNUC__ >= 4 && (!defined __clang__ || __has_builtin (__builtin_isnanf))
/* GCC 4.0 and newer provides three built-ins for isnan.  */
 #  undef isnanf
 #  define isnanf(x) __builtin_isnanf ((float)(x))
diff --git a/lib/isnanl-nolibm.h b/lib/isnanl-nolibm.h
index c45e3ab..103d31a 100644
--- a/lib/isnanl-nolibm.h
+++ b/lib/isnanl-nolibm.h
@@ -17,7 +17,10 @@
 #if HAVE_ISNANL_IN_LIBC
 /* Get declaration of isnan macro or (older) isnanl function.  */
 # include 
-# if __GNUC__ >= 4
+# ifndef __has_builtin
+#  define __has_builtin(name) 0
+# endif
+# if __GNUC__ >= 4 && (!defined __clang__ || __has_builtin (__builtin_isnanl))
/* GCC 4.0 and newer provides three built-ins for isnan.  */
 #  undef isnanl
 #  define isnanl(x) __builtin_isnanl ((long double)(x))
diff --git a/lib/math.in.h b/lib/math.in.h
index e5e37d6..30465f8 100644
--- a/lib/math.in.h
+++ b/lib/math.in.h
@@ -127,6 +127,12 @@ static void (*_gl_math_fix_itold) (long double *, int) = 
_Qp_itoq;
 #endif
 
 
+/* For clang: Use __has_builtin to determine whether a builtin is available.  
*/
+#ifndef __has_builtin
+# define __has_builtin(name) 0
+#endif
+
+
 /* POSIX allows platforms that don't support NAN.  But all major
machines in the past 15 years have supported something close to
IEEE NaN, so we define this unconditionally.  We also must define
@@ -2318,7 +2324,7 @@ _GL_WARN_REAL_FLOATING_DECL (isinf);
 # if @HAVE_ISNANF@
 /* The original  included above provides a declaration of isnan macro
or (older) isnanf function.  */
-#  if __GNUC__ >= 4
+#  if __GNUC__ >= 4 && (!defined __clang__ || __has_builtin (__builtin_isnanf))
 /* GCC 4.0 and newer provides three built-ins for isnan.  */
 #   undef isnanf
 #   define isnanf(x) __builtin_isnanf ((float)(x))
@@ -2362,7 +2368,7 @@ _GL_EXTERN_C int isnand (double x);
 # if @HAVE_ISNANL@
 /* The original  included above provides a declaration of isnan
macro or (older) isnanl function.  */
-#  if __GNUC__ >= 4
+#  if __GNUC__ >= 4 && (!defined __clang__ || __has_builtin (__builtin_isnanl))
 /* GCC 4.0 and newer provides three built-ins for isnan.  */
 #   undef isnanl
 #   define isnanl(x) __builtin_isnanl ((long double)(x))
@@ -2385,7 +2391,7 @@ _GL_EXTERN_C int isnanl (long double x) 
_GL_ATTRIBUTE_CONST;
isnanf.h (e.g.) here, because those may end up being macros
that recursively expand back to isnan.  So use the gnulib
replacements for them directly. */
-#  if @HAVE_ISNANF@ && __GNUC__ >= 4
+#  if @HAVE_ISNANF@ && __GNUC__ >= 4 && (!defined __clang__ || __has_builtin 
(__builtin_isnanf))
 #   define gl_isnan_f(x) __builtin_isnanf ((float)(x))
 #  else
 _GL_EXTERN_C int rpl_isnanf (float x);
@@ -2397,7 +2403,7 @@ _GL_EXTERN_C int rpl_isnanf (float x);
 _GL_EXTERN_C int 

Fix calloc.m4 test

[Bruno Haible]

Tim Rühsen wrote:
> - gcc-10 with unset CFLAGS (*.gcc-10)
> - clang-10 with unset CFLAGS (*.clang-10)

Comparing these two results, there is a difference:

-ac_cv_func_calloc_0_nonnull=${ac_cv_func_calloc_0_nonnull=yes}
+ac_cv_func_calloc_0_nonnull=${ac_cv_func_calloc_0_nonnull=no}

Why this test result? Let's see:

$ cat foo.c
#include 
int main ()
{
  int result = 0;
  char *p = calloc (0, 0);
  if (!p)
result |= 1;
  free (p);
  p = calloc ((size_t) -1 / 8 + 1, 8);
  if (p)
result |= 2;
  free (p);
  return result;
}

$ clang -O2 -S foo.c
$ cat foo.s
.text
.file   "foo.c"
.globl  main# -- Begin function main
.p2align4, 0x90
.type   main,@function
main:   # @main
.cfi_startproc
# %bb.0:
movl$2, %eax
retq
.Lfunc_end0:
.size   main, .Lfunc_end0-main
.cfi_endproc
# -- End function
.ident  "clang version 10.0.0 "
.section".note.GNU-stack","",@progbits
.addrsig

As you can see:

  1) clang has eliminated the calloc() and free() calls from the program.
 Apparently it "knows" how to optimize matching calloc - free pairs.

  2) It has estimated that the second call would return a non-NULL pointer,
 although the address space does not allow this.
 Reported at . But some
 people claim it is not a bug. Paul, can you please help with ISO C
 citations?

This patch provides a workaround.


2020-05-23  Bruno Haible  

calloc-gnu: Avoid wrong configure results with clang.
* m4/calloc.m4 (_AC_FUNC_CALLOC_IF): Mark the pointer variable as
'volatile', to defeat compiler optimizations.

diff --git a/m4/calloc.m4 b/m4/calloc.m4
index 2e0d8ff..3361cba 100644
--- a/m4/calloc.m4
+++ b/m4/calloc.m4
@@ -1,4 +1,4 @@
-# calloc.m4 serial 20
+# calloc.m4 serial 21
 
 # Copyright (C) 2004-2020 Free Software Foundation, Inc.
 # This file is free software; the Free Software Foundation
@@ -25,7 +25,7 @@ AC_DEFUN([_AC_FUNC_CALLOC_IF],
[AC_LANG_PROGRAM(
   [AC_INCLUDES_DEFAULT],
   [[int result = 0;
-char *p = calloc (0, 0);
+char * volatile p = calloc (0, 0);
 if (!p)
   result |= 1;
 free (p);

Fix exponentl.m4 test

[Bruno Haible]

Tim Rühsen wrote:
> Attached is the config.log and config.cache for
> - gcc-9 with unset CFLAGS (*.gcc-9)
> - clang-9 with unset CFLAGS (*.clang-9)
> - gcc-10 with unset CFLAGS (*.gcc-10)
> - clang-10 with unset CFLAGS (*.clang-10)
> 
> And the same set with -fsanitize... (*.sanitize).

Thanks, that's much more useful.

The first interesting finding is this difference between config.cache.gcc-9
and config.cache.gcc-10:

-gl_cv_cc_long_double_expbit0=${gl_cv_cc_long_double_expbit0='word 2 bit 0'}
+gl_cv_cc_long_double_expbit0=${gl_cv_cc_long_double_expbit0=unknown}

Indeed, with GCC 10 I see:

  checking where to find the exponent in a 'long double'... unknown

I see it also with older versions of GCC, with "-O2".

What happens is that on x86_64, 'long double' is the 80-bit "extended double"
format, which needs 3 'unsigned int' values. But sizeof (long double) = 16
[whereas on x86, sizeof (long double) = 12]. So, in the test, NWORDS = 4.
But when optimizing, the compiler apparently copies only 3 words, not 4 words,
when reading or writing a parameter of type 'long double'.

I would like to avoid hard coding the particular representation (since
possible some other compilers will use 'float128' as 'long double').

This patch brings back

  checking where to find the exponent in a 'long double'... word 2 bit 0


2020-05-23  Bruno Haible  

isnanl, isnanl-nolibm: Make a test work better with "gcc -O2" on x86_64.
* m4/exponentl.m4 (gl_LONG_DOUBLE_EXPONENT_LOCATION): Pass the
'long double' values by reference, with values taken from a statically
allocated array.

diff --git a/m4/exponentl.m4 b/m4/exponentl.m4
index b33b3bf..0a35c11 100644
--- a/m4/exponentl.m4
+++ b/m4/exponentl.m4
@@ -1,4 +1,4 @@
-# exponentl.m4 serial 4
+# exponentl.m4 serial 5
 dnl Copyright (C) 2007-2020 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -22,14 +22,14 @@ typedef union { long double value; unsigned int 
word[NWORDS]; }
 memory_long_double;
 static unsigned int ored_words[NWORDS];
 static unsigned int anded_words[NWORDS];
-static void add_to_ored_words (long double x)
+static void add_to_ored_words (long double *x)
 {
   memory_long_double m;
   size_t i;
   /* Clear it first, in case
  sizeof (long double) < sizeof (memory_long_double).  */
   memset (, 0, sizeof (memory_long_double));
-  m.value = x;
+  m.value = *x;
   for (i = 0; i < NWORDS; i++)
 {
   ored_words[i] |= m.word[i];
@@ -38,17 +38,15 @@ static void add_to_ored_words (long double x)
 }
 int main ()
 {
+  static long double samples[5] = { 0.25L, 0.5L, 1.0L, 2.0L, 4.0L };
   size_t j;
   FILE *fp = fopen ("conftest.out", "w");
   if (fp == NULL)
 return 1;
   for (j = 0; j < NWORDS; j++)
 anded_words[j] = ~ (unsigned int) 0;
-  add_to_ored_words (0.25L);
-  add_to_ored_words (0.5L);
-  add_to_ored_words (1.0L);
-  add_to_ored_words (2.0L);
-  add_to_ored_words (4.0L);
+  for (j = 0; j < 5; j++)
+add_to_ored_words ([j]);
   /* Remove bits that are common (e.g. if representation of the first mantissa
  bit is explicit).  */
   for (j = 0; j < NWORDS; j++)

Re: stack module

[Marc Nieper-Wißkirchen]

Hi Paul,

Am Sa., 23. Mai 2020 um 19:33 Uhr schrieb Paul Eggert :

> Probably not for -O0. I'm not so sure for -Og. Either way, we shouldn't rely 
> on
> GCC's current behavior in this area as it is neither documented nor guaranteed
> to stay the same.

Agreed.

> > #include 
> > #include "verify.h"
> >
> > #ifdef NDEBUG
> > # define checked_assume(E) assume (E)
> > #else
> > # define checked_assume(E) assert (E)
> > #endif
>
> Something like that would work, though the name "checked_assume" is misleading
> since the assumption is not always checked.
>
> "affirm (E)" would be a better name, since the name's not being used anymore 
> by
> the old software verification project[1] and it slides in well next to 
> "assume"
> and "assert". (Some day we're going to run out of synonyms. :-)

Believe it or not, but when I first proposed the (initial version of
the) macro, I wanted to name it "affirm" after I had looked for
synonyms. Only eventually, I switched to the name "checked_assume".

But if "affirm" is fine with you, I would love to see it in a module.
Either in verify or assure or in a new module named affirm.

Re: stack module

[Paul Eggert]

On 5/23/20 9:54 AM, Marc Nieper-Wißkirchen wrote:

> Sure, but not in "-O0" or "-Og" builds, I think. Or am I mistaken?

Probably not for -O0. I'm not so sure for -Og. Either way, we shouldn't rely on
GCC's current behavior in this area as it is neither documented nor guaranteed
to stay the same.

> #include 
> #include "verify.h"
> 
> #ifdef NDEBUG
> # define checked_assume(E) assume (E)
> #else
> # define checked_assume(E) assert (E)
> #endif

Something like that would work, though the name "checked_assume" is misleading
since the assumption is not always checked.

"affirm (E)" would be a better name, since the name's not being used anymore by
the old software verification project[1] and it slides in well next to "assume"
and "assert". (Some day we're going to run out of synonyms. :-)

[1] http://www.softwarepreservation.org/projects/verification/affirm/

Re: [gnu.org #1534539] Re: Licensing issues for gendocs_template_min

[Asher Gordon]

Bruno Haible  writes:

> Thanks. Pushed.

Great, thanks!

Asher

-- 
: The following (relative to AutoSplit 1.03) attempts to please everyone
: and perhaps pleases no one:

I think that's way cool.
-- Larry Wall in <199709292015.naa09...@wall.org>

GPG fingerprint: 38F3 975C D173 4037 B397  8095 D4C9 C4FC 5460 8E68


signature.asc
Description: PGP signature

Re: stack module

[Bruno Haible]

Marc Nieper-Wißkirchen wrote:
> > I was expecting that you write
> >
> >   struct
> >   {
> > void *base; ...
> >   }
> 
> This removes type safety. The benefit of the current approach is that
> stack types of different types are not compatible.

Indeed. Yes, it's a difficult trade-off between debuggability, binary code size,
and type safety...

Bruno

Re: stack module

[Marc Nieper-Wißkirchen]

Am Sa., 23. Mai 2020 um 18:44 Uhr schrieb Paul Eggert :
>
> On 5/23/20 8:55 AM, Marc Nieper-Wißkirchen wrote:
> > A combination of assure and assume would be helpful:
> >
> > #define checked_assume(X) do { assure (X); assume (X); } while (0)
>
> No, because the compiler is entitled to optimize away the 'assure (X)' in this
> case. I installed the attached to try to explain this better.

Sure, but not in "-O0" or "-Og" builds, I think. Or am I mistaken?

But it is definitely better to make it more robust and not rely on
optimization levels:

#include 
#include "verify.h"

#ifdef NDEBUG
# define checked_assume(E) assume (E)
#else
# define checked_assume(E) assert (E)
#endif

Re: stack module

[Paul Eggert]

On 5/23/20 8:55 AM, Marc Nieper-Wißkirchen wrote:
> A combination of assure and assume would be helpful:
> 
> #define checked_assume(X) do { assure (X); assume (X); } while (0)

No, because the compiler is entitled to optimize away the 'assure (X)' in this
case. I installed the attached to try to explain this better.
>From b2c8d02c9750f335549781d20fa37415c9a1edb3 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Sat, 23 May 2020 09:41:54 -0700
Subject: [PATCH] =?UTF-8?q?verify:=20document=20=E2=80=98assume=E2=80=99?=
 =?UTF-8?q?=20better?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* lib/verify.h (assume): Say it’s for static analysis, not dynamic.
---
 ChangeLog|  5 +
 lib/verify.h | 20 
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index a4473284c..44450a354 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2020-05-23  Paul Eggert  
+
+	verify: document ‘assume’ better
+	* lib/verify.h (assume): Say it’s for static analysis, not dynamic.
+
 2020-05-22  Asher Gordon  
 
 	gendocs: Clarify licenses for templates.
diff --git a/lib/verify.h b/lib/verify.h
index d9ab89a57..f10976127 100644
--- a/lib/verify.h
+++ b/lib/verify.h
@@ -277,10 +277,22 @@ template 
 #endif
 
 /* Assume that R always holds.  Behavior is undefined if R is false,
-   fails to evaluate, or has side effects.  Although assuming R can
-   help a compiler generate better code or diagnostics, performance
-   can suffer if R uses hard-to-optimize features such as function
-   calls not inlined by the compiler.  */
+   fails to evaluate, or has side effects.
+
+   'assume (R)' is a directive from the programmer telling the
+   compiler that R is true so the compiler needn't generate code to
+   test R.  This is why 'assume' is in verify.h: it's related to
+   static checking (in this case, static checking done by the
+   programmer), not dynamic checking.
+
+   'assume (R)' can affect compilation of all the code, not just code
+   that happens to be executed after the assume (R) is "executed".
+   For example, if the code mistakenly does 'assert (R); assume (R);'
+   the compiler is entitled to optimize away the 'assert (R)'.
+
+   Although assuming R can help a compiler generate better code or
+   diagnostics, performance can suffer if R uses hard-to-optimize
+   features such as function calls not inlined by the compiler.  */
 
 #if _GL_HAS_BUILTIN_UNREACHABLE
 # define assume(R) ((R) ? (void) 0 : __builtin_unreachable ())
-- 
2.17.1

Re: coreutils and GCC -fanalyzer

[Tim Rühsen]

On 23.05.20 18:08, Paul Eggert wrote:
> On 5/23/20 8:56 AM, Pádraig Brady wrote:
>> Note -Wanalyzer... flags are ineffective without -fanalyzer.
>> Also -Wanalyzer-too-complex (now auto enabled by gnulib) is currently quite
>> problematic with -fanalyzer.
> 
> Hmm, that's odd, since I recall getting more warnings with those flags 
> enabled.
> Perhaps not all of them are disabled when you don't use -fanalyzer? Or maybe 
> my
> memory is faulty.
> 
> Anyway, cleary the Gnulib manywarnings module needs to be fixed to do a better
> job here, and this reminds me of another problem that has been bothering me 
> for
> quite some time: ./configure spends way too much time testing for individual
> warnings when the manywarnings module is in use. 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.
> 
> This should make 'configure' much faster. It'll also fix the issue you
> mentioned. And we can make sure that -Wanalyzer-too-complex is not mentioned 
> in
> step (2) so that Gnulib won't generate it.

This has been done for Wget2
https://gitlab.com/gnuwget/wget2/-/blob/master/m4/wget_manywarnings.m4

Example usage at L123+:
https://gitlab.com/gnuwget/wget2/-/blob/master/configure.ac

To make it usable for other projects, it needs some merging with the
manywarnings.m4 from gnulib, IMO. I would support any volunteer as good
as i can.

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: coreutils and GCC -fanalyzer

[Paul Eggert]

On 5/23/20 8:56 AM, Pádraig Brady wrote:
> Note -Wanalyzer... flags are ineffective without -fanalyzer.
> Also -Wanalyzer-too-complex (now auto enabled by gnulib) is currently quite
> problematic with -fanalyzer.

Hmm, that's odd, since I recall getting more warnings with those flags enabled.
Perhaps not all of them are disabled when you don't use -fanalyzer? Or maybe my
memory is faulty.

Anyway, cleary the Gnulib manywarnings module needs to be fixed to do a better
job here, and this reminds me of another problem that has been bothering me for
quite some time: ./configure spends way too much time testing for individual
warnings when the manywarnings module is in use. 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.

This should make 'configure' much faster. It'll also fix the issue you
mentioned. And we can make sure that -Wanalyzer-too-complex is not mentioned in
step (2) so that Gnulib won't generate it.

Ccing to bug-gnulib since this is really more of a bug-gnulib issue.

Re: stack module

[Marc Nieper-Wißkirchen]

Am Sa., 23. Mai 2020 um 17:39 Uhr schrieb Paul Eggert :
>
> On 5/23/20 7:36 AM, Marc Nieper-Wißkirchen wrote:
> > The verify  also contains a runtime check `assume', which uses
> > __builtin_unreachable if available to help the compiler in optimizing
> > modes.
>
> I wouldn't call 'assume (X)' a "runtime check". It's more an 
> anti-runtime-check.
>
> 'assume (X)' is a directive from the programmer to the compiler that X is true
> so that the compiler needn't generate code to test whether X is true. This is
> why 'assume' is in verify.h: it's related to static checking (in this case,
> static checking done by the programmer), not dynamic checking.
>
> Perhaps I should add a comment to this effect

A combination of assure and assume would be helpful:

#define checked_assume(X) do { assure (X); assume (X); } while (0)

In debug builds, we get assertions and can check our assumptions. In
non-debug (NDEBUG) builds, we just have the compiler hints.

Re: Easy Accurate Reading and Writing of Floating-Point Numbers

[Marc Nieper-Wißkirchen]

Am Sa., 23. Mai 2020 um 16:52 Uhr schrieb Bruno Haible :

> > In a future version, I may add a flag to select between %e, %f, and %g.
>
> Hmm, that route is likely going the path to sprintf: First a flag to
> select among %e, %f, %g. Then an optional width. Then a flag to select
> whether to fill with spaces or with zeroes.
> If you go that route, it's better to start with full sprintf and define
> a particular "precision" that would mean "as many digits as needed to
> guarantee that the result can be unambiguously read back in".

So you mean that we should hack it into
http://git.savannah.gnu.org/cgit/gnulib.git/tree/lib/vasnprintf.c?

How can we ensure that this module is (at least) in one-way sync with
glibc's (va)s(n)printf?

But it is an intriguing idea!

Re: stack module

[Paul Eggert]

On 5/23/20 7:36 AM, Marc Nieper-Wißkirchen wrote:
> The verify  also contains a runtime check `assume', which uses
> __builtin_unreachable if available to help the compiler in optimizing
> modes.

I wouldn't call 'assume (X)' a "runtime check". It's more an anti-runtime-check.

'assume (X)' is a directive from the programmer to the compiler that X is true
so that the compiler needn't generate code to test whether X is true. This is
why 'assume' is in verify.h: it's related to static checking (in this case,
static checking done by the programmer), not dynamic checking.

Perhaps I should add a comment to this effect

Re: Easy Accurate Reading and Writing of Floating-Point Numbers

[Paul Eggert]

On 5/23/20 5:26 AM, Marc Nieper-Wißkirchen wrote:
> I sent such an assignment request to the FSF a few days ago but
> haven't heard back from them yet.

It may take a few days more. If you don't hear back in a couple of weeks, please
let us know.

Thanks for writing that, by the way. It's a good improvement.

Re: Easy Accurate Reading and Writing of Floating-Point Numbers

[Bruno Haible]

Hello Marc,

> In a future version, I may add a flag to select between %e, %f, and %g.

Hmm, that route is likely going the path to sprintf: First a flag to
select among %e, %f, %g. Then an optional width. Then a flag to select
whether to fill with spaces or with zeroes.
If you go that route, it's better to start with full sprintf and define
a particular "precision" that would mean "as many digits as needed to
guarantee that the result can be unambiguously read back in".

Bruno

Re: stack module

[Marc Nieper-Wißkirchen]

Am Sa., 23. Mai 2020 um 16:02 Uhr schrieb Bruno Haible :

> I was expecting that you write
>
>   struct
>   {
> void *base; ...
>   }

This removes type safety. The benefit of the current approach is that
stack types of different types are not compatible.

> Then macro should better be called STACK_FREE or STACK_DESTROY.

> The name STACK_CLEAR suggests that the result is still a valid stack, and 
> empty.

Thank you for this suggestion; much better.

> > In the form of assure of the assure module? Or, to facilitate
> > optimization, better assume from verify module? In non-debug builds, I
> > want to make sure that no superfluous checks are made.
>
> The 'verify' module is for compile-time checks.
>
> 'assure' is an improved form of 'assert'.

The verify  also contains a runtime check `assume', which uses
__builtin_unreachable if available to help the compiler in optimizing
modes.

Re: stack module

[Bruno Haible]

Hi Marc,

> > In Gnulib, we usually avoid extensive use of multi-line macros, because
> > it hampers debuggability. Here, however, one needs macros, in order to
> > accommodate the TYPE argument, which is not necessarily compatible to 'void 
> > *'.
> > Nevertheless, we would try to put as much code as possible into functions.
> > The STACK_INIT macro, for example, could be implemented as a function.
> 
> How? This would mean that the function stack_init has to take a void *
> argument, which has to be cast to
> 
> struct
> {
>   void *base; ...
> }
> 
> and we have to hope that this structure is guaranteed to be compatible to
> 
> struct
> {
>   type *base; ...
> }
> 
> by the C standard.

I was expecting that you write

  struct
  {
void *base; ...
  }

and the cast the base to 'type *' each time you fetch it. I don't think this
would go the C standard, nor defeat any possible compiler optimization.

> > > #define STACK_CLEAR(stack)\
> > >   free ((stack).base)
> >
> > Shouldn't this one also set .size and .allocated to 0 ?
> 
> A stack can be uninitialized or initialized. An uninitialized stack is
> initialized by STACK_INIT. It is cleared (and uninitialized) by
> STACK_CLEAR. An uninitialized stack does not have to maintain any
> invariant. The only way to use it is to initialize it again. Thus,
> setting .size and .allocated seems pointless.

Then macro should better be called STACK_FREE or STACK_DESTROY.

The name STACK_CLEAR suggests that the result is still a valid stack, and empty.

> > > #define STACK_POP(stack)\
> > >   (stack).base [--(stack).size]
> > >
> > > #define STACK_DISCARD(stack)\
> > >   (--(stack).size)
> > >
> > > #define STACK_TOP(stack)\
> > >   (stack).base[(stack).size - 1]
> >
> > In these three macros, I would consider to abort() when (stack).size == 0.
> 
> In the form of assure of the assure module? Or, to facilitate
> optimization, better assume from verify module? In non-debug builds, I
> want to make sure that no superfluous checks are made.

The 'verify' module is for compile-time checks.

'assure' is an improved form of 'assert'.

Bruno

Re: stack module

[Marc Nieper-Wißkirchen]

Hi Bruno,

> In Gnulib, we usually avoid extensive use of multi-line macros, because
> it hampers debuggability. Here, however, one needs macros, in order to
> accommodate the TYPE argument, which is not necessarily compatible to 'void 
> *'.
> Nevertheless, we would try to put as much code as possible into functions.
> The STACK_INIT macro, for example, could be implemented as a function.

How? This would mean that the function stack_init has to take a void *
argument, which has to be cast to

struct
{
  void *base; ...
}

and we have to hope that this structure is guaranteed to be compatible to

struct
{
  type *base; ...
}

by the C standard.

> > #define STACK_CLEAR(stack)\
> >   free ((stack).base)
>
> Shouldn't this one also set .size and .allocated to 0 ?

A stack can be uninitialized or initialized. An uninitialized stack is
initialized by STACK_INIT. It is cleared (and uninitialized) by
STACK_CLEAR. An uninitialized stack does not have to maintain any
invariant. The only way to use it is to initialize it again. Thus,
setting .size and .allocated seems pointless.

> > #define STACK_POP(stack)\
> >   (stack).base [--(stack).size]
> >
> > #define STACK_DISCARD(stack)\
> >   (--(stack).size)
> >
> > #define STACK_TOP(stack)\
> >   (stack).base[(stack).size - 1]
>
> In these three macros, I would consider to abort() when (stack).size == 0.

In the form of assure of the assure module? Or, to facilitate
optimization, better assume from verify module? In non-debug builds, I
want to make sure that no superfluous checks are made.

Marc

Re: stack module

[Bruno Haible]

Hi Marc,

> Alternatively, one could implement a universally usable stack through
> the following header file (mimicking somewhat C++ templates). What do
> you think? It will be a lot faster than using the general list modules
> of Gnulib.

I agree that a generic 'stack' module is useful. I also agree that a single
implementation, based on an array, is the way to go. Then it is already
faster than the generic list module.

In Gnulib, we usually avoid extensive use of multi-line macros, because
it hampers debuggability. Here, however, one needs macros, in order to
accommodate the TYPE argument, which is not necessarily compatible to 'void *'.
Nevertheless, we would try to put as much code as possible into functions.
The STACK_INIT macro, for example, could be implemented as a function.

> #define STACK_CLEAR(stack)\
>   free ((stack).base)

Shouldn't this one also set .size and .allocated to 0 ?

> #define STACK_POP(stack)\
>   (stack).base [--(stack).size]
> 
> #define STACK_DISCARD(stack)\
>   (--(stack).size)
> 
> #define STACK_TOP(stack)\
>   (stack).base[(stack).size - 1]

In these three macros, I would consider to abort() when (stack).size == 0.

Bruno

Re: Easy Accurate Reading and Writing of Floating-Point Numbers

[Marc Nieper-Wißkirchen]

Hi Bruno,

[...]

> The only (tiny) issue is a matter of style: Two of the new module files don't
> end in a newline. In Gnulib, we try to let every non-empty text file end in a
> newline, since "cat FILE" and "sed -e ... FILE" work better this way.

I will correct this when I send you the final patch. In a future
version, I may add a flag to select between %e, %f, and %g.

> I would apply this patch. But first, we need a copyright assignment of yours.
> As mentioned in [1], I think a simple copyright assignment is enough. You 
> start
> the process by taking the gnulib/doc/Copyright/request-assign.future template
> and sending it the FSF.

I sent such an assignment request to the FSF a few days ago but
haven't heard back from them yet.

Marc

Re: Easy Accurate Reading and Writing of Floating-Point Numbers

[Bruno Haible]

Hi Marc,

Marc Nieper-Wißkirchen wrote:
> Please see the attached patch file, my first attempt (and first
> contribution to Gnulib).

The patch looks all correct.

The only (tiny) issue is a matter of style: Two of the new module files don't
end in a newline. In Gnulib, we try to let every non-empty text file end in a
newline, since "cat FILE" and "sed -e ... FILE" work better this way.

I would apply this patch. But first, we need a copyright assignment of yours.
As mentioned in [1], I think a simple copyright assignment is enough. You start
the process by taking the gnulib/doc/Copyright/request-assign.future template
and sending it the FSF.

Bruno

[1] https://lists.gnu.org/archive/html/bug-gnulib/2020-05/msg00204.html

Re: find_in_given_path(): not handling directories properly

[Bruno Haible]

Hi Paul,

> In gnulib commit 7b1de4a62f876:
> 
>   commit 7b1de4a62f8766787160f785217671b074e0f0f2
>   Author: Bruno Haible 
>   Date:   2020-04-10 15:57:10 +0200
> 
>   findprog, relocatable-prog: Ignore directories during PATH search.
> 
>   Reported by Frederick Eaton via Dmitry Goncharov in
>   .
> 
> The lib/findprog.c implementation of the find_in_path() function was
> fixed by this commit.
> 
> However, the same issue exists in lib/findprog-in.c, in the
> find_in_given_path() (which is what GNU make actually uses), and that
> function wasn't updated by the above commit.

Oops, I misread the original report. Thanks for the reminder.


2020-05-23  Bruno Haible  

findprog-in: Ignore directories.
Reported by Frederick Eaton via Dmitry Goncharov in
.
* lib/findprog-in.c (find_in_given_path): When the file found is a
directory, set errno to EACCES and, during a PATH search, continue
searching.
* modules/findprog-in (Depends-on): Add sys_stat, stat.

diff --git a/lib/findprog-in.c b/lib/findprog-in.c
index c254f2f..0f76e36 100644
--- a/lib/findprog-in.c
+++ b/lib/findprog-in.c
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "filename.h"
 #include "concat-filename.h"
@@ -58,8 +59,8 @@ static const char * const suffixes[] =
 /* Note: The cmd.exe program does a different lookup: It searches according
to the PATHEXT environment variable.
See .
-   Also, it executes files ending .bat and .cmd directly without letting 
the
-   kernel interpret the program file.  */
+   Also, it executes files ending in .bat and .cmd directly without letting
+   the kernel interpret the program file.  */
 #elif defined __CYGWIN__
 "", ".exe", ".com"
 #elif defined __EMX__
@@ -136,14 +137,26 @@ find_in_given_path (const char *progname, const char 
*path,
call access() despite its design flaw.  */
 if (eaccess (progpathname, X_OK) == 0)
   {
-/* Found!  */
-if (strcmp (progpathname, progname) == 0)
+/* Check that the progpathname does not point to a
+   directory.  */
+struct stat statbuf;
+
+if (stat (progpathname, ) >= 0)
   {
-free (progpathname);
-return progname;
+if (! S_ISDIR (statbuf.st_mode))
+  {
+/* Found!  */
+if (strcmp (progpathname, progname) == 0)
+  {
+free (progpathname);
+return progname;
+  }
+else
+  return progpathname;
+  }
+
+errno = EACCES;
   }
-else
-  return progpathname;
   }
 
 if (errno != ENOENT)
@@ -210,25 +223,37 @@ find_in_given_path (const char *progname, const char 
*path,
call access() despite its design flaw.  */
 if (eaccess (progpathname, X_OK) == 0)
   {
-/* Found!  */
-if (strcmp (progpathname, progname) == 0)
+/* Check that the progpathname does not point to a
+   directory.  */
+struct stat statbuf;
+
+if (stat (progpathname, ) >= 0)
   {
-free (progpathname);
-
-/* Add the "./" prefix for real, that
-   xconcatenated_filename() optimized away.  This
-   avoids a second PATH search when the caller uses
-   execl/execv/execlp/execvp.  */
-progpathname =
-  XNMALLOC (2 + strlen (progname) + 1, char);
-progpathname[0] = '.';
-progpathname[1] = NATIVE_SLASH;
-memcpy (progpathname + 2, progname,
-strlen (progname) + 1);
-  }
+if (! S_ISDIR (statbuf.st_mode))
+  {
+/* Found!  */
+if (strcmp (progpathname, progname) == 0)
+  {
+free (progpathname);
+
+  

Re: installing patches with git

[Bruno Haible]

Karl Berry wrote:
> But can I ask someone else
> to install it, please? Bruno? I have no faith in my ability to use git
> in expected ways.

Installing a patch into Gnulib is indeed a bit complicated, due to the
fact that we use a ChangeLog file and we try to use the same title and
message with git and in the ChangeLog.

Usually I proceed in these steps:

  1. git pull

  2. git status
 Verify that my checkout is clean.

  3. Save the patch attachment or, if it's a git-formatted mail, the
 entire mail to a file.

  4. git am < FILE
 If that does not succeed because of a change of ChangeLog:
 git am --abort
 Edit FILE, to remove the ChangeLog change.
 git am < FILE

  5. gitk
 Review the commit message's style and whether there is a ChangeLog change.

  6. Add/tweak the ChangeLog entry to match the Gnulib style.
 git commit --amend ChangeLog
 Reuse the ChangeLog entry for the commit message.

  If the patch was not produced by "git format-patch", do it the old way:

  3'. Save the patch to a file.

  4'. patch -p1 < FILE

  5'. Add/tweak the ChangeLog entry to git the Gnulib style.

  6'. git commit --author="FIRST LAST NAME " .
  Reuse the ChangeLog entry for the commit message.

  7. gitk
 Review again.

  8. git push

Bruno

Re: [gnu.org #1534539] Re: Licensing issues for gendocs_template_min

[Bruno Haible]

> Whoops, I meant to amend the commit, not create a new one. Here's the
> full patch:

Thanks. Pushed.

Bruno