date:20201004

Re: AS_IF

2020-10-04 Thread Paul Eggert


On 10/4/20 4:40 PM, Bruno Haible wrote:

AFAICS, this is relevant for code written directly into configure.ac. But
inside an AC_DEFUN it is irrelevant, because required macros are hoisted
before the body of the AC_DEFUN anyway.


I was playing it safe based on this NEWS item from Autoconf 2.69c:

   - Autoconf macros that use AC_REQUIRE internally, are not safe to
 use inside of hand-written shell control-flow constructs.  Use
 AS_IF, AS_CASE, AS_FOR, etc. instead.  (See the “Prerequisite
 Macros” section of the manual for further explanation.)

Since I didn't know whether the macros in question used AC_REQUIRE internally, I 
played it safe.


But now that you mention it, AS_IF isn't needed here and I installed the 
attached further patch. Also, to help prevent others from making a similar 
mistake, I plan to reword the Autoconf NEWS item to start with something like 
"Autoconf macros that use AC_REQUIRE are not safe to use in hand-written shell 
control-flow constructs that appear outside of macros defined by AC_DEFUN."
>From f636f37983b32d8db3abea7ecdda928f3a0f542e Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Sun, 4 Oct 2020 18:46:44 -0700
Subject: [PATCH] c-stack: avoid AS_IF

Problem reported by Bruno Haible in:
https://lists.gnu.org/r/bug-gnulib/2020-10/msg00018.html
* m4/c-stack.m4 (gl_PREREQ_C_STACK): No need for AS_IF.
---
 ChangeLog |  5 +
 m4/c-stack.m4 | 14 --
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index e0c821f7d..4ec03efe4 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
 2020-10-04  Paul Eggert  
 
+	c-stack: avoid AS_IF
+	Problem reported by Bruno Haible in:
+	https://lists.gnu.org/r/bug-gnulib/2020-10/msg00018.html
+	* m4/c-stack.m4 (gl_PREREQ_C_STACK): No need for AS_IF.
+
 	c-stack: pacify GCC 9.3.1 when using libsigsegv
 	* lib/c-stack.c [USE_LIBSIGSEGV]: Disable --suggest-attribute=pure.
 
diff --git a/m4/c-stack.m4 b/m4/c-stack.m4
index 1523a724d..85107f465 100644
--- a/m4/c-stack.m4
+++ b/m4/c-stack.m4
@@ -7,7 +7,7 @@
 
 # Written by Paul Eggert.
 
-# serial 19
+# serial 20
 
 AC_DEFUN([AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC],
   [
@@ -360,11 +360,13 @@ AC_DEFUN([gl_PREREQ_C_STACK],
AC_CHECK_TYPES([stack_t], , , [[#include ]])
 
dnl c-stack does not need -lsigsegv if the system has XSI heuristics.
-   AS_IF([test "$gl_cv_sys_xsi_stack_overflow_heuristic" != yes],
- [gl_LIBSIGSEGV
-  AS_IF([test "$gl_cv_lib_sigsegv" = yes],
-[AC_SUBST([LIBCSTACK], [$LIBSIGSEGV])
- AC_SUBST([LTLIBCSTACK], [$LTLIBSIGSEGV])])])
+   if test "$gl_cv_sys_xsi_stack_overflow_heuristic" != yes; then
+ gl_LIBSIGSEGV
+ if test "$gl_cv_lib_sigsegv" = yes; then
+   AC_SUBST([LIBCSTACK], [$LIBSIGSEGV])
+   AC_SUBST([LTLIBCSTACK], [$LTLIBSIGSEGV])
+ fi
+   fi
 ])
 
 AC_DEFUN([gl_C_STACK],
-- 
2.25.1

Re: AS_IF

2020-10-04 Thread Bruno Haible

Hi Paul,

> The third patch merely streamlines 'configure' when running on platforms like 
> Solaris that need not use libsigsegv.
> 
> -   if test "$gl_cv_lib_sigsegv" = yes \
> -   && test "$gl_cv_sys_xsi_stack_overflow_heuristic" != yes; then
> - AC_SUBST([LIBCSTACK], [$LIBSIGSEGV])
> - AC_SUBST([LTLIBCSTACK], [$LTLIBSIGSEGV])
> -   fi
> +   AS_IF([test "$gl_cv_sys_xsi_stack_overflow_heuristic" != yes],
> + [gl_LIBSIGSEGV
> +  AS_IF([test "$gl_cv_lib_sigsegv" = yes],
> +[AC_SUBST([LIBCSTACK], [$LIBSIGSEGV])
> + AC_SUBST([LTLIBCSTACK], [$LTLIBSIGSEGV])])])

Any particular reason why AS_IF is used here?

I typically don't use AS_IF because
  - I find a line of shell code more readable than a mix between m4 syntax
and shell syntax,
  - it's yet another step in the learning curve, for someone who wants to
understand how Autoconf macros work.

The Autoconf documentation says
  AS_IF "ensures any required macros ... are expanded before
  the first test."
AFAICS, this is relevant for code written directly into configure.ac. But
inside an AC_DEFUN it is irrelevant, because required macros are hoisted
before the body of the AC_DEFUN anyway.

Bruno

Re: grep-3.5 fails to build on Solaris when libsigsegv is installed

2020-10-04 Thread Paul Eggert


On 9/28/20 7:02 PM, Bruno Haible wrote:

#define USE_LIBSIGSEGV (HAVE_XSI_STACK_OVERFLOW_HEURISTIC && HAVE_LIBSIGSEGV)

There seems to be a logic mistake, here.


You're right, it's a typo: there should be a "!" before the 
HAVE_XSI_STACK_OVERFLOW_HEURISTIC. As a result of this mistake, libsigsegv is 
used only on Solaris-like platforms, where it's not needed, and libsigsegv is 
not used on other platforms like GNU/Linux where it can be helpful.


This mistake causes some configuration failures on Solaris on platforms where 
libsigsegv has been installed but grep was not configured correctly. It also 
means that the stack-overflow reporting on GNU/Linux is too generous, in that 
segmentation violations that are not stack overflows get reported as stack 
overflows. I installed the first attached patch to correct this. I would guess 
that this is not much of a problem in practice except when installing on Solaris 
with badly-configured libsigsegv, since from the user's point of view the 
problem is only that when grep crashes the wrong message might be printed.


While looking into this error I noticed that c-stack assumes SIGSTKSZ is an 
integer constant expression, but there have been moves to make it non-constant 
(and this may already be in effect on some platforms). I installed the second 
attached patch to work around this potential portability bug; it has a URL 
pointing to recent discussions in this area.


The third patch merely streamlines 'configure' when running on platforms like 
Solaris that need not use libsigsegv.
>From f20816f7d25b75c111defa27081b2b614c31dc52 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Tue, 29 Sep 2020 14:11:22 -0700
Subject: [PATCH 1/3] c-stack: fix libsigsegv typo

Problem reported by Bruno Haible in:
https://lists.gnu.org/r/bug-gnulib/2020-09/msg00175.html
* lib/c-stack.c (USE_LIBSIGSEGV): Fix typo that caused libsigsegv
to be used only on Solaris (exactly where it is not needed!).
---
 ChangeLog | 8 
 lib/c-stack.c | 2 +-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index e6c8079a8..76a76fbc4 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2020-10-03  Paul Eggert  
+
+	c-stack: fix libsigsegv typo
+	Problem reported by Bruno Haible in:
+	https://lists.gnu.org/r/bug-gnulib/2020-09/msg00175.html
+	* lib/c-stack.c (USE_LIBSIGSEGV): Fix typo that caused libsigsegv
+	to be used only on Solaris (exactly where it is not needed!).
+
 2020-10-03  Thien-Thi Nguyen  
 
 	MODULES.html.sh: Fix typo.
diff --git a/lib/c-stack.c b/lib/c-stack.c
index 742eb023e..80ebcbf00 100644
--- a/lib/c-stack.c
+++ b/lib/c-stack.c
@@ -66,7 +66,7 @@ typedef struct sigaltstack stack_t;
 
 /* Use libsigsegv only if needed; kernels like Solaris can detect
stack overflow without the overhead of an external library.  */
-#define USE_LIBSIGSEGV (HAVE_XSI_STACK_OVERFLOW_HEURISTIC && HAVE_LIBSIGSEGV)
+#define USE_LIBSIGSEGV (!HAVE_XSI_STACK_OVERFLOW_HEURISTIC && HAVE_LIBSIGSEGV)
 
 #if USE_LIBSIGSEGV
 # include 
-- 
2.25.1

>From f9e2b20a12a230efa30f1d479563ae07d276a94b Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Wed, 30 Sep 2020 13:50:36 -0700
Subject: [PATCH 2/3] c-stack: stop using SIGSTKSZ
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

It’s been proposed to stop making SIGSTKSZ an integer constant:
https://sourceware.org/pipermail/libc-alpha/2020-September/118028.html
Also, using SIGSTKSZ in #if did not conform to current POSIX.
Also, avoiding SIGSTKSZ makes the code simpler and easier to grok.
* lib/c-stack.c (SIGSTKSZ): Remove.
(alternate_signal_stack): Now a 64 KiB array, for simplicity.
All uses changed.
---
 ChangeLog |  9 +
 lib/c-stack.c | 42 ++
 lib/c-stack.h |  2 +-
 3 files changed, 28 insertions(+), 25 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 76a76fbc4..7f54b7860 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,14 @@
 2020-10-03  Paul Eggert  
 
+	c-stack: stop using SIGSTKSZ
+	It’s been proposed to stop making SIGSTKSZ an integer constant:
+	https://sourceware.org/pipermail/libc-alpha/2020-September/118028.html
+	Also, using SIGSTKSZ in #if did not conform to current POSIX.
+	Also, avoiding SIGSTKSZ makes the code simpler and easier to grok.
+	* lib/c-stack.c (SIGSTKSZ): Remove.
+	(alternate_signal_stack): Now a 64 KiB array, for simplicity.
+	All uses changed.
+
 	c-stack: fix libsigsegv typo
 	Problem reported by Bruno Haible in:
 	https://lists.gnu.org/r/bug-gnulib/2020-09/msg00175.html
diff --git a/lib/c-stack.c b/lib/c-stack.c
index 80ebcbf00..cf0fe8da0 100644
--- a/lib/c-stack.c
+++ b/lib/c-stack.c
@@ -70,15 +70,6 @@ typedef struct sigaltstack stack_t;
 
 #if USE_LIBSIGSEGV
 # include 
-/* libsigsegv 2.6 through 2.8 have a bug where some architectures use
-   more than the Linux default of an 8k alternate stack when deciding
-   if a fault was caused by stack overflow.  */
-# if LIBSIGSEGV_VERSION <=

Re: Unused parameter warnings

2020-10-04 Thread Bruno Haible

Hi Marc,

Marc Nieper-Wißkirchen wrote:
> When compiling Gnulib with -Wunused-parameter, I get the following
> report from GCC:
> 
> lib/localename.c: In function 'gl_locale_name_thread_unsafe':
> lib/localename.c:3117:57: error: unused parameter 'categoryname'
> [-Werror=unused-parameter]
>  3117 | gl_locale_name_thread_unsafe (int category, const char *categoryname)
>   | ^~~~
> lib/localename.c: In function 'gl_locale_name_posix':
> lib/localename.c:3256:49: error: unused parameter 'categoryname'
> [-Werror=unused-parameter]
>  3256 | gl_locale_name_posix (int category, const char *categoryname)
>   | ^~~~
> lib/localename.c: In function 'gl_locale_name_environ':
> lib/localename.c:3321:29: error: unused parameter 'category'
> [-Werror=unused-parameter]
>  3321 | gl_locale_name_environ (int category, const char *categoryname)
>   | ^~~~
> 
> Wouldn't it make sense to insert MAYBE_UNUSED from "attribute.h" here?

Yes. -Wunused-parameter is part of -Wall, unfortunately. Sigh.

Here I prefer _GL_UNUSED, because it does not require '#include "attribute.h"'.


2020-10-04  Bruno Haible  

localename: Fix a couple of "unused parameter" warnings.
Reported by Marc Nieper-Wißkirchen  in
.
* lib/localename.c (gl_locale_name_thread_unsafe, gl_locale_name_thread,
gl_locale_name_posix, gl_locale_name_environ): Add _GL_UNUSED in
parameter list.

diff --git a/lib/localename.c b/lib/localename.c
index 5731ceb..1bf47ed 100644
--- a/lib/localename.c
+++ b/lib/localename.c
@@ -3114,7 +3114,7 @@ freelocale (locale_t locale)
 static
 # endif
 const char *
-gl_locale_name_thread_unsafe (int category, const char *categoryname)
+gl_locale_name_thread_unsafe (int category, const char *categoryname 
_GL_UNUSED)
 {
 # if HAVE_GOOD_USELOCALE
   {
@@ -3229,7 +3229,7 @@ gl_locale_name_thread_unsafe (int category, const char 
*categoryname)
 #endif
 
 const char *
-gl_locale_name_thread (int category, const char *categoryname)
+gl_locale_name_thread (int category, const char *categoryname _GL_UNUSED)
 {
 #if HAVE_GOOD_USELOCALE
   const char *name = gl_locale_name_thread_unsafe (category, categoryname);
@@ -3253,7 +3253,7 @@ gl_locale_name_thread (int category, const char 
*categoryname)
 #endif
 
 const char *
-gl_locale_name_posix (int category, const char *categoryname)
+gl_locale_name_posix (int category, const char *categoryname _GL_UNUSED)
 {
 #if defined WINDOWS_NATIVE
   if (LC_MIN <= category && category <= LC_MAX)
@@ -3318,7 +3318,7 @@ gl_locale_name_posix (int category, const char 
*categoryname)
 }
 
 const char *
-gl_locale_name_environ (int category, const char *categoryname)
+gl_locale_name_environ (int category _GL_UNUSED, const char *categoryname)
 {
   const char *retval;

Re: vasnprintf: avoid using %n in the general case

2020-10-04 Thread Jeremie Courreges-Anglas

On Sun, Oct 04 2020, Bruno Haible  wrote:
> Hi Jeremie,
>
>> The attached patch changes vasnprintf.c to avoid using %n in the general
>> case, ie when the return value of snprintf is usable.  This should help
>> if more systems decide to make tighten %n usage.  There are plans for
>> that in OpenBSD land.
>
> Thanks for the suggestion. Yes, this is a good idea. (I didn't know about
> the OpenBSD plans.)
>
>> The existing comments in vasnprintf.c mention systems where
>> gl_SNPRINTF_RETVAL_C99 and gl_SNPRINTF_TRUNCATION_C99 pass.  This patch
>> only considers the usability of the return value of snprintf, as lack
>> of truncation seems to be a different problem (apparently handled later
>> in the code).
>
> I prefer to limit the shortcut to those platforms where both
> gl_SNPRINTF_RETVAL_C99 and gl_SNPRINTF_TRUNCATION_C99 pass. These are
> the conditions I found when considering this code in detail a couple of
> years ago. Maybe you are right that only one of the conditions is needed;
> but I don't want to live on the risky edge here.

Your approach certainly makes sense.  I considered something similar
earlier.  Your changes look good to me, thanks!

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE


signature.asc
Description: PGP signature

Unused parameter warnings

2020-10-04 Thread Marc Nieper-Wißkirchen

When compiling Gnulib with -Wunused-parameter, I get the following
report from GCC:

lib/localename.c: In function 'gl_locale_name_thread_unsafe':
lib/localename.c:3117:57: error: unused parameter 'categoryname'
[-Werror=unused-parameter]
 3117 | gl_locale_name_thread_unsafe (int category, const char *categoryname)
  | ^~~~
lib/localename.c: In function 'gl_locale_name_posix':
lib/localename.c:3256:49: error: unused parameter 'categoryname'
[-Werror=unused-parameter]
 3256 | gl_locale_name_posix (int category, const char *categoryname)
  | ^~~~
lib/localename.c: In function 'gl_locale_name_environ':
lib/localename.c:3321:29: error: unused parameter 'category'
[-Werror=unused-parameter]
 3321 | gl_locale_name_environ (int category, const char *categoryname)
  | ^~~~

Wouldn't it make sense to insert MAYBE_UNUSED from "attribute.h" here?

Thanks,

Marc

Re: vasnprintf: avoid using %n in the general case

2020-10-04 Thread Bruno Haible

Hi Jeremie,

> The attached patch changes vasnprintf.c to avoid using %n in the general
> case, ie when the return value of snprintf is usable.  This should help
> if more systems decide to make tighten %n usage.  There are plans for
> that in OpenBSD land.

Thanks for the suggestion. Yes, this is a good idea. (I didn't know about
the OpenBSD plans.)

> The existing comments in vasnprintf.c mention systems where
> gl_SNPRINTF_RETVAL_C99 and gl_SNPRINTF_TRUNCATION_C99 pass.  This patch
> only considers the usability of the return value of snprintf, as lack
> of truncation seems to be a different problem (apparently handled later
> in the code).

I prefer to limit the shortcut to those platforms where both
gl_SNPRINTF_RETVAL_C99 and gl_SNPRINTF_TRUNCATION_C99 pass. These are
the conditions I found when considering this code in detail a couple of
years ago. Maybe you are right that only one of the conditions is needed;
but I don't want to live on the risky edge here.


2020-10-04  Bruno Haible  

vasnprintf: Don't use %n on modern, ISO C 99 compliant platforms.
Suggested by Jeremie Courreges-Anglas  in
.
* m4/vasnprintf.m4 (gl_PREREQ_VASNPRINTF): Define
HAVE_SNPRINTF_TRUNCATION_C99.
* lib/vasnprintf.c (VASNPRINTF): Don't use %n if
HAVE_SNPRINTF_RETVAL_C99 && HAVE_SNPRINTF_TRUNCATION_C99.

diff --git a/lib/vasnprintf.c b/lib/vasnprintf.c
index 7f75139..b232d14 100644
--- a/lib/vasnprintf.c
+++ b/lib/vasnprintf.c
@@ -5117,39 +5117,32 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp,
 #endif
   *fbp = dp->conversion;
 #if USE_SNPRINTF
-# if ! (((__GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 3))\
- && !defined __UCLIBC__)\
-|| (defined __APPLE__ && defined __MACH__)  \
-|| defined __ANDROID__  \
-|| (defined _WIN32 && ! defined __CYGWIN__))
-fbp[1] = '%';
-fbp[2] = 'n';
-fbp[3] = '\0';
-# else
-/* On glibc2 systems from glibc >= 2.3 - probably also older
-   ones - we know that snprintf's return value conforms to
-   ISO C 99: the tests gl_SNPRINTF_RETVAL_C99 and
-   gl_SNPRINTF_TRUNCATION_C99 pass.
-   Therefore we can avoid using %n in this situation.
-   On glibc2 systems from 2004-10-18 or newer, the use of %n
-   in format strings in writable memory may crash the program
-   (if compiled with _FORTIFY_SOURCE=2), so we should avoid it
-   in this situation.  */
-/* On Mac OS X 10.3 or newer, we know that snprintf's return
-   value conforms to ISO C 99: the tests gl_SNPRINTF_RETVAL_C99
-   and gl_SNPRINTF_TRUNCATION_C99 pass.
-   Therefore we can avoid using %n in this situation.
-   On Mac OS X 10.13 or newer, the use of %n in format strings
-   in writable memory by default crashes the program, so we
-   should avoid it in this situation.  */
-/* On Android, we know that snprintf's return value conforms to
-   ISO C 99: the tests gl_SNPRINTF_RETVAL_C99 and
-   gl_SNPRINTF_TRUNCATION_C99 pass.
-   Therefore we can avoid using %n in this situation.
-   Starting on 2018-03-07, the use of %n in format strings
-   produces a fatal error (see
-   
),
-   so we should avoid it.  */
+# if ((HAVE_SNPRINTF_RETVAL_C99 && HAVE_SNPRINTF_TRUNCATION_C99)\
+  || ((__GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 3))   \
+  && !defined __UCLIBC__)   \
+  || (defined __APPLE__ && defined __MACH__)\
+  || defined __ANDROID__\
+  || (defined _WIN32 && ! defined __CYGWIN__))
+/* On systems where we know that snprintf's return value
+   conforms to ISO C 99 (HAVE_SNPRINTF_RETVAL_C99) and that
+   snprintf always produces NUL-terminated strings
+   (HAVE_SNPRINTF_TRUNCATION_C99), it is possible to avoid
+   using %n.  And it is desirable to do so, because more and
+   more platforms no longer support %n, for "security reasons".
+   In particular, the following platforms:
+ - On glibc2 systems from 2004-10-18 or newer, the use of
+   %n in format strings in writable memory may crash the
+

Re: AS_IF

Re: AS_IF

Re: grep-3.5 fails to build on Solaris when libsigsegv is installed

Re: Unused parameter warnings

Re: vasnprintf: avoid using %n in the general case

Unused parameter warnings

Re: vasnprintf: avoid using %n in the general case

7 matches

Site Navigation

Mail list logo

Footer information