Re: Configure warning/error in m4/frexp.m4

[Sam James]

Bruno Haible  writes:

> Sam James wrote:
>> > It's good if you have the time to test not-yet-released compiler versions.
>> > I don't have that time, and therefore will prefer to wait until the
>> > clang release is out.
>> 
>> OK. You've appreciated reports in the past (even quite recently), I can
>> avoid making them in future if you'd prefer me to avoid it for
>> gnulib.
>
> Let me say it like this:
>   - If a new compiler feature is still being discussed or likely to be 
> changed,
> I would consider it too early to involve Gnulib.
>   - If a new compiler feature is finalized and it's only a matter of time
> until it becomes official, then you're welcome to involve us, as it will
> allow us to react sooner.
>
> This is for general things. There may be special situations which need
> different handling.
>

Fair enough. Thanks, and sorry for the noise.

> Bruno

sam

Re: Configure warning/error in m4/frexp.m4

[Bruno Haible]

Sam James wrote:
> > It's good if you have the time to test not-yet-released compiler versions.
> > I don't have that time, and therefore will prefer to wait until the
> > clang release is out.
> 
> OK. You've appreciated reports in the past (even quite recently), I can
> avoid making them in future if you'd prefer me to avoid it for
> gnulib.

Let me say it like this:
  - If a new compiler feature is still being discussed or likely to be changed,
I would consider it too early to involve Gnulib.
  - If a new compiler feature is finalized and it's only a matter of time
until it becomes official, then you're welcome to involve us, as it will
allow us to react sooner.

This is for general things. There may be special situations which need
different handling.

Bruno

Re: Configure warning/error in m4/frexp.m4

[Sam James]

Bruno Haible  writes:

> Sam James wrote:
>> it's still going to be an issue if (as
>> planned) Clang flips '-Wincompatible-pointer-types' to error out by
>> default unless they carve out an exception for qualifiers here.
>> 
>> I'll advocate that they do, of course. I just came across it while
>> checking for configure changes w/ recent GCC changes.
>
> It's good if you have the time to test not-yet-released compiler versions.
> I don't have that time, and therefore will prefer to wait until the
> clang release is out.

OK. You've appreciated reports in the past (even quite recently), I can
avoid making them in future if you'd prefer me to avoid it for
gnulib. This tends to be particularly important for gnulib given it's included
in other projects and changes take time to propagate into releases.

Perhaps I should have been clearer that in this instance, I was asking
for opinions rather than saying this is definitely a problem to be fixed, as 
well.

>
> If they really want to make an implicit assignment from 'const char *'
> or 'volatile char *' to 'char *' an error, it will cause problems in
> many packages, namely where argument arrays for calling exec* are
> constructed: Since
>

Anyway, I'll communicate the desire that Clang be consistent with GCC
and not create busywork as these cases aren't really the problematic
type anyway.

thanks,
sam

Re: [PATCH] Update portability doc for CHERI, C23

[Paul Eggert]

On 2023-12-01 22:12, Bruno Haible wrote:

Which integer types on CHERI have "holes"?


intptr_t and uintptr_t, since they consume 128 bits of storage but have 
only 64 bits of integer value payload. Because of the holes, for 
example, intprops.h's TYPE_MAXIMUM doesn't work on CHERI-BSD for those 
two types. (I think I could work around this particular bug but it's 
likely not worth the effort as people should just use INTPTR_MAX and 
UINTPTR_MAX instead.)


Arguably the 129th bit of each intptr_t/uintptr_t is also a "hole". 
However, this extra bit doesn't cause a problem with intprops.h etc. as 
far as I can see.

Re: Configure warning/error in m4/frexp.m4

[Bruno Haible]

Paul Eggert wrote:
> We can conform to standard C without a cast with something like the 
> attached (which I haven't installed).

Looks good to me. Please install it; likewise in m4/frexpf.m4.

> (I assume the 'volatile' is there because of the funky things compilers 
> do with x86 double)

It's there more to block the constant propagation that the compilers would
otherwise do.

Bruno

Re: Configure warning/error in m4/frexp.m4

[Bruno Haible]

Sam James wrote:
> it's still going to be an issue if (as
> planned) Clang flips '-Wincompatible-pointer-types' to error out by
> default unless they carve out an exception for qualifiers here.
> 
> I'll advocate that they do, of course. I just came across it while
> checking for configure changes w/ recent GCC changes.

It's good if you have the time to test not-yet-released compiler versions.
I don't have that time, and therefore will prefer to wait until the
clang release is out.

If they really want to make an implicit assignment from 'const char *'
or 'volatile char *' to 'char *' an error, it will cause problems in
many packages, namely where argument arrays for calling exec* are
constructed: Since

  const char *argv[] = { "sh", "-c", "--", "pwd" };
  execv (argv[0], (char * const *) argv);

is dangerous (it violates the strict-aliasing rule), people write

  char *argv[] = { "sh", "-c", "--", "pwd" };
  execv (argv[0], argv);

And we don't want to write it as

  char *argv[] = { (char *) "sh", (char *) "-c", (char *) "--", (char *) "pwd" 
};
  execv (argv[0], argv);

because that is just silly.

Bruno

Re: Configure warning/error in m4/frexp.m4

[Paul Eggert]

On 2023-12-01 21:52, Bruno Haible wrote:

There is nothing wrong with this code (m4/frexp.m4:159, m4/frexpf.m4:83).


Isn't there? I thought that the C standard doesn't let you access a 
'double volatile' object via a 'void *' or 'void const *' pointer. It'd 
need to be 'void volatile *' or 'void const volatile *'.




We don't want to add a cast here, because — as Paul argues — casts can make
code more difficult to maintain in the long run.


We can conform to standard C without a cast with something like the 
attached (which I haven't installed).


(I assume the 'volatile' is there because of the funky things compilers 
do with x86 double)From 006cf2337a8ca2e3a0b41c488fbe3d95eda201f3 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Fri, 1 Dec 2023 22:19:22 -0800
Subject: [PATCH] frexp: pacify clang re address-of-volatile
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Problem reported by Sam James in:
https://lists.gnu.org/r/bug-gnulib/2023-12/msg00013.html
* m4/frexp.m4 (gl_FUNC_FREXP_WORKS): Don’t try to convert
‘double volatile *’ to ‘void const *’ as the C standard
doesn’t allow accessing volatile variables through
pointer-to-nonvolatile.
---
 ChangeLog   | 8 
 m4/frexp.m4 | 5 +++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index c713de7b42..f30430d3de 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
 2023-12-01  Paul Eggert  
 
+	frexp: pacify clang re address-of-volatile
+	Problem reported by Sam James in:
+	https://lists.gnu.org/r/bug-gnulib/2023-12/msg00013.html
+	* m4/frexp.m4 (gl_FUNC_FREXP_WORKS): Don’t try to convert
+	‘double volatile *’ to ‘void const *’ as the C standard
+	doesn’t allow accessing volatile variables through
+	pointer-to-nonvolatile.
+
 	Update portability doc for CHERI, C23
 	* doc/gnulib-readme.texi:
 	Prefer “null pointer” to “@code{NULL}” since C23 has nullptr.
diff --git a/m4/frexp.m4 b/m4/frexp.m4
index 0293765c59..b23aa470c3 100644
--- a/m4/frexp.m4
+++ b/m4/frexp.m4
@@ -1,4 +1,4 @@
-# frexp.m4 serial 18
+# frexp.m4 serial 19
 dnl Copyright (C) 2007-2023 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -156,7 +156,8 @@ int main()
   {
 int exp;
 double y = frexp (x, );
-if (memcmp (, , sizeof x))
+double x1 = x;
+if (memcmp (, , sizeof x1))
   result |= 4;
   }
   return result;
-- 
2.40.1

Re: [PATCH] Update portability doc for CHERI, C23

[Bruno Haible]

Paul Eggert wrote:
> * doc/gnulib-readme.texi:
> Prefer “null pointer” to “@code{NULL}” since C23 has nullptr.
> (Portability guidelines): Mention C99 instead of C89 for what
> Gnulib assumes of headers.
> (C99 features avoided): Mention CHERI issue with intptr_t etc.
> (Other portability assumptions): Say that C23 requires two’s
> complement.  Mention CHERI’s holes.

Thanks, especially for the update from C89 to C99.

>  @item
>  There are no ``holes'' in integer values: all the bits of an integer
>  contribute to its value in the usual way.
>  In particular, an unsigned type and its signed counterpart have the
>  same number of bits when you count the latter's sign bit.
> +(As an exception, Gnulib code is portable to CHERI platforms
> +even though this assumption is false for CHERI.)

Huh? Which integer types on CHERI have "holes"?

If you are referring to
,
this patch was necessary because on CHERI it's invalid to fetch 8 bytes
from memory if only 1 to 7 bytes were allocated, not because of holes
in the 'unsigned long' type.

Bruno

Re: Configure warning/error in m4/frexp.m4

[Sam James]

Sam James  writes:

> Bruno Haible  writes:
>
>> Hi Sam,
>
> Hi Bruno,
>
>>
>>> The configure test in gl_FUNC_FREXP_WORKS within m4/frexp.m4 triggers
>>> a Clang warning/error when Clang is passed
>>> -Werror=incompatible-pointer-types.
>>
>> Gnulib does not support CC or CPPFLAGS or CFLAGS values with -Werror at
>> configure time. Neither with gcc nor with clang.
>
> Yes, I understand that. But Clang is likely to make this change to its
> default and -Werror=... was just a way of emulating that. gnulib _does_
> have to cater to the default strictness of compilers.
>
>>
>> The reason is that [1]
>>   "Many GCC warning options usually don’t point to mistakes in the code;
>>these warnings enforce a certain programming style. It is a project
>>management decision whether you want your code to follow any of these
>>styles. Note that some of these programming styles are conflicting.
>>You cannot have them all; you have to choose among them."
>>
>> For example, there is a warning option that attempts to enforce
>> explicit casts (no implicit conversions), and there is a warning option
>> that attempts to enforce no explicit casts. When you use such a warning
>> option together with -Werror, you are attempting to enforce a certain
>> programming style on the configure tests. Obviously the configure test
>> snippets cannot obey different, conflicting programming styles.
>
> Yep, completely on board with that & I get it. The only reason I
> reported this is because there's a reasonable chance this will be Clang's
> new behaviour by default (see below).
>
>>
>>> /tmp/foo.c: In function ‘main’:
>>> /tmp/foo.c:59:21: warning: passing argument 2 of ‘memcmp’ discards 
>>> ‘volatile’ qualifier from pointer target type [-Wdiscarded-qualifiers]
>>>59 | if (memcmp (, , sizeof x))
>>>   | ^~
>>
>> There is nothing wrong with this code (m4/frexp.m4:159, m4/frexpf.m4:83).
>>
>> We don't want to add a cast here, because — as Paul argues — casts can make
>> code more difficult to maintain in the long run.
>
> Yes, I agree it's pedantic, but it's still going to be an issue if (as
> planned) Clang flips '-Wincompatible-pointer-types' to error out by
> default unless they carve out an exception for qualifiers here.
>
> I'll advocate that they do, of course. I just came across it while
> checking for configure changes w/ recent GCC changes.

I should add: if your position is, reasonably, "clang really shouldn't
do that unless they carve out an exception for this kind of case",
then that's fine. But the report was not about me just doing -Werror=x
for the sake of it and misunderstands the motivation here.

>
>>
>> Bruno
>>
>> [1] https://www.gnu.org/software/gnulib/manual/html_node/manywarnings.html

Re: Configure warning/error in m4/frexp.m4

[Sam James]

Bruno Haible  writes:

> Hi Sam,

Hi Bruno,

>
>> The configure test in gl_FUNC_FREXP_WORKS within m4/frexp.m4 triggers
>> a Clang warning/error when Clang is passed
>> -Werror=incompatible-pointer-types.
>
> Gnulib does not support CC or CPPFLAGS or CFLAGS values with -Werror at
> configure time. Neither with gcc nor with clang.

Yes, I understand that. But Clang is likely to make this change to its
default and -Werror=... was just a way of emulating that. gnulib _does_
have to cater to the default strictness of compilers.

>
> The reason is that [1]
>   "Many GCC warning options usually don’t point to mistakes in the code;
>these warnings enforce a certain programming style. It is a project
>management decision whether you want your code to follow any of these
>styles. Note that some of these programming styles are conflicting.
>You cannot have them all; you have to choose among them."
>
> For example, there is a warning option that attempts to enforce
> explicit casts (no implicit conversions), and there is a warning option
> that attempts to enforce no explicit casts. When you use such a warning
> option together with -Werror, you are attempting to enforce a certain
> programming style on the configure tests. Obviously the configure test
> snippets cannot obey different, conflicting programming styles.

Yep, completely on board with that & I get it. The only reason I
reported this is because there's a reasonable chance this will be Clang's
new behaviour by default (see below).

>
>> /tmp/foo.c: In function ‘main’:
>> /tmp/foo.c:59:21: warning: passing argument 2 of ‘memcmp’ discards 
>> ‘volatile’ qualifier from pointer target type [-Wdiscarded-qualifiers]
>>59 | if (memcmp (, , sizeof x))
>>   | ^~
>
> There is nothing wrong with this code (m4/frexp.m4:159, m4/frexpf.m4:83).
>
> We don't want to add a cast here, because — as Paul argues — casts can make
> code more difficult to maintain in the long run.

Yes, I agree it's pedantic, but it's still going to be an issue if (as
planned) Clang flips '-Wincompatible-pointer-types' to error out by
default unless they carve out an exception for qualifiers here.

I'll advocate that they do, of course. I just came across it while
checking for configure changes w/ recent GCC changes.

>
> Bruno
>
> [1] https://www.gnu.org/software/gnulib/manual/html_node/manywarnings.html

Re: Configure warning/error in m4/frexp.m4

[Bruno Haible]

Hi Sam,

> The configure test in gl_FUNC_FREXP_WORKS within m4/frexp.m4 triggers
> a Clang warning/error when Clang is passed
> -Werror=incompatible-pointer-types.

Gnulib does not support CC or CPPFLAGS or CFLAGS values with -Werror at
configure time. Neither with gcc nor with clang.

The reason is that [1]
  "Many GCC warning options usually don’t point to mistakes in the code;
   these warnings enforce a certain programming style. It is a project
   management decision whether you want your code to follow any of these
   styles. Note that some of these programming styles are conflicting.
   You cannot have them all; you have to choose among them."

For example, there is a warning option that attempts to enforce
explicit casts (no implicit conversions), and there is a warning option
that attempts to enforce no explicit casts. When you use such a warning
option together with -Werror, you are attempting to enforce a certain
programming style on the configure tests. Obviously the configure test
snippets cannot obey different, conflicting programming styles.

> /tmp/foo.c: In function ‘main’:
> /tmp/foo.c:59:21: warning: passing argument 2 of ‘memcmp’ discards ‘volatile’ 
> qualifier from pointer target type [-Wdiscarded-qualifiers]
>59 | if (memcmp (, , sizeof x))
>   | ^~

There is nothing wrong with this code (m4/frexp.m4:159, m4/frexpf.m4:83).

We don't want to add a cast here, because — as Paul argues — casts can make
code more difficult to maintain in the long run.

Bruno

[1] https://www.gnu.org/software/gnulib/manual/html_node/manywarnings.html

Configure warning/error in m4/frexp.m4

[Sam James]

Hi,

The configure test in gl_FUNC_FREXP_WORKS within m4/frexp.m4 triggers
a Clang warning/error when Clang is passed
-Werror=incompatible-pointer-types.

This _isn't_ an issue with GCC 14, as it doesn't consider the lost
const to be a problem, like so:

/tmp/foo.c: In function ‘main’:
/tmp/foo.c:59:21: warning: passing argument 2 of ‘memcmp’ discards ‘volatile’ 
qualifier from pointer target type [-Wdiscarded-qualifiers]
   59 | if (memcmp (, , sizeof x))
  | ^~
In file included from /tmp/foo.c:3:
/usr/include/string.h:64:50: note: expected ‘const void *’ but argument is of 
type ‘volatile double *’
   64 | extern int memcmp (const void *__s1, const void *__s2, size_t __n)
  |  ^~~~

But Clang on the other hand, when directed to treat incompatible
ptr. types as errors (which they may well do given GCC is) gives:

/tmp/foo.c:59:21: error: passing 'volatile double *' to parameter of type 
'const void *' discards qualifiers 
[-Werror,-Wincompatible-pointer-types-discards-qualifiers]
   59 | if (memcmp (, , sizeof x))
  | ^~
/usr/include/string.h:64:50: note: passing argument to parameter '__s2' here
   64 | extern int memcmp (const void *__s1, const void *__s2, size_t __n)
  |  ^
1 error generated.

I'm naturally testing with GCC 14 at the moment but this was left over
in some of the older testing we did with Clang so I'm working through
the backlog there as well.

thanks,
sam

Re: undefined-behavior obstack.c:139

[Paul Eggert]

On 2023-12-01 12:36, Marc Nieper-Wißkirchen wrote:

It may not be a false alarm in future versions of the compiler.


If that happens, Gnulib (and lots of other software) won't work in those 
future versions. This is so unlikely that we needn't worry about it now.

Re: undefined-behavior obstack.c:139

[Andreas F. Borchert]

On Fri, Dec 01, 2023 at 05:01:45PM +0100, Bruno Haible wrote:
> Jeffrey Walton wrote:
> > I think that's a valid finding. NULL is not a valid address. You can't
> > add anything to it.
> 
> Can you back this opinion with citations from ISO C 23 [1] ?

See N3096 (most recent draft of C23 [1]), quotes from § 6.5.6, (2), (8) and (9):

   For addition, either both operands shall have arithmetic type,
   or one operand shall be a pointer to a complete object type
   and the other shall have integer type.
   [..]
   For the purposes of these operators, a pointer to an object that
   is not an element of an array behaves the same as a pointer to the
   first elemenet of an array of length one with the type of
   the object as its element type.
   [..]
   If the pointer operand points to an element of an array object,
   and the array is large enough, the result points to an element
   offset from the original element such that the difference of
   the subscripts of the resulting and original array elements
   equals the integer expression.

NULL per  is defined per § 7.21, (4):

   The macros are NULL which expands to an implementation-defined
   null pointer constant; [..]

The definition of null pointer constant is given at § 6.3.2.3 (3):

   An integer constant expression with the value 0, such an
   expression cast to type void *, or the predefined constant nullptr
   is called a null pointer constant.

In summary, null pointer constants are not pointers to an object
or an element of an array object and thereby must not be used
for pointer arithmetic.

Andreas.

[1] https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3096.pdf

-- 
Dr. Andreas F. Borchert,  Institut für Numerische Mathematik, Universität Ulm
Helmholtzstr. 20, 89081 Ulm, +49 7315023572 https://mathematik.uni-ulm.de/afb


smime.p7s
Description: S/MIME cryptographic signature

Re: undefined-behavior obstack.c:139

[Marc Nieper-Wißkirchen]

Am Fr., 1. Dez. 2023 um 21:01 Uhr schrieb Paul Eggert :

> On 2023-12-01 10:40, Bruno Haible wrote:
>
> > Indeed, this sentence appears to forbid ((char *) NULL) + something.
>
> Yes. However, Gnulib code can still use ((char *) NULL) + something)
> because the Gnulib portability guidelines allow it.
>
> The issue with clang false positives is covered here:
>
>
> https://www.gnu.org/software/gnulib/manual/html_node/Unsupported-Platforms.html
>
> which lists "clang -fsanitize=undefined" as an unsupported platform
> unless you also specify "-fno-sanitize=pointer-overflow".
>
> The obstack patch you installed is fine, as it's clearer and just as
> fast as the original. However, we needn't go through Gnulib and change
> other code merely because it runs afoul of this false alarm from clang.
>

It may not be a false alarm in future versions of the compiler.  At any
time, the compiler may decide to replace ((char *) NULL + 0) by
__builtin_unreachable.  It can make sense to find an ISO C-compliant
replacement of this idiom at some point.

[PATCH] Update portability doc for CHERI, C23

[Paul Eggert]

* doc/gnulib-readme.texi:
Prefer “null pointer” to “@code{NULL}” since C23 has nullptr.
(Portability guidelines): Mention C99 instead of C89 for what
Gnulib assumes of headers.
(C99 features avoided): Mention CHERI issue with intptr_t etc.
(Other portability assumptions): Say that C23 requires two’s
complement.  Mention CHERI’s holes.
---
 ChangeLog  | 11 ++
 doc/gnulib-readme.texi | 46 ++
 2 files changed, 40 insertions(+), 17 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index b48c7912e9..c713de7b42 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2023-12-01  Paul Eggert  
+
+   Update portability doc for CHERI, C23
+   * doc/gnulib-readme.texi:
+   Prefer “null pointer” to “@code{NULL}” since C23 has nullptr.
+   (Portability guidelines): Mention C99 instead of C89 for what
+   Gnulib assumes of headers.
+   (C99 features avoided): Mention CHERI issue with intptr_t etc.
+   (Other portability assumptions): Say that C23 requires two’s
+   complement.  Mention CHERI’s holes.
+
 2023-12-01  Bruno Haible  
 
obstack: Avoid undefined behaviour.
diff --git a/doc/gnulib-readme.texi b/doc/gnulib-readme.texi
index a08d3c0100..b111216db5 100644
--- a/doc/gnulib-readme.texi
+++ b/doc/gnulib-readme.texi
@@ -288,7 +288,7 @@ platform is not the latest version.  @xref{Target 
Platforms}.
 Many Gnulib modules exist so that applications need not worry about
 undesirable variability in implementations.  For example, an
 application that uses the @code{malloc} module need not worry about
-@code{malloc@ (0)} returning @code{NULL} on some Standard C
+@code{malloc@ (0)} returning a null pointer on some Standard C
 platforms; and @code{glob} users need not worry about @code{glob}
 silently omitting symbolic links to nonexistent files on some
 platforms that do not conform to POSIX.
@@ -336,8 +336,8 @@ forever.
 
 Even if the include files exist, they may not conform to the C standard.
 However, GCC has a @command{fixincludes} script that attempts to fix most
-C89-conformance problems.  Gnulib currently assumes include files
-largely conform to C89 or better.  People still using ancient hosts
+conformance problems.  Gnulib currently assumes include files
+largely conform to C99 or better.  People still using ancient hosts
 should use fixincludes or fix their include files manually.
 
 Even if the include files conform, the library itself may not.
@@ -424,6 +424,14 @@ and to avoid large stack usage that may have security 
implications.
 variably modified types, such as array declarations in function
 prototype scope.
 
+@item
+Converting to pointers via integer types other than @code{intptr_t} or
+@code{uintptr_t}.  Although the C standard says that values of these
+integer types, if they exist, should be convertible to and from
+@code{intmax_t} and @code{uintmax_t} without loss of information, on
+CHERI platforms such conversions result in integers that, if converted
+back to a pointer, cannot be dereferenced.
+
 @item
 @code{extern inline} functions, without checking whether they are
 supported.  @xref{extern inline}.
@@ -445,10 +453,14 @@ Comments beginning with @samp{//}.  This is mostly for 
style reasons.
 @node Other portability assumptions
 @subsection Other portability assumptions made by Gnulib
 
-The GNU coding standards allow one departure from strict C: Gnulib
-code can assume that standard internal types like
-@code{ptrdiff_t} and @code{size_t} are no
-wider than @code{long}.  POSIX requires implementations to support at
+Gnulib code makes the following assumptions
+that go beyond what C and POSIX require:
+
+@itemize
+@item
+Standard internal types like @code{ptrdiff_t} and @code{size_t} are no
+wider than @code{long}.  The GNU coding standards allow code to make
+this assumption, POSIX requires implementations to support at
 least one programming environment where this is true, and such
 environments are recommended for Gnulib-using applications.  When it
 is easy to port to non-POSIX platforms like MinGW where these types
@@ -457,9 +469,6 @@ using @code{ptrdiff_t} instead of @code{long}.  However, it 
is not
 always that easy, and no effort has been made to check that all Gnulib
 modules work on MinGW-like environments.
 
-Gnulib code makes the following additional assumptions:
-
-@itemize
 @item
 @code{int} and @code{unsigned int} are at least 32 bits wide.  POSIX
 and the GNU coding standards both require this.
@@ -473,21 +482,24 @@ sometimes do not guarantee this, and Gnulib code with this
 assumption is now considered to be questionable.
 @xref{Integer Properties}.
 
-Although some Gnulib modules contain explicit support for the other signed
-integer representations allowed by the C standard (ones' complement and signed
-magnitude), these modules are the exception rather than the rule.
-All practical Gnulib targets use two's complement.
+Although some Gnulib modules contain explicit support for

Re: undefined-behavior obstack.c:139

[Paul Eggert]

On 2023-12-01 10:40, Bruno Haible wrote:


Indeed, this sentence appears to forbid ((char *) NULL) + something.


Yes. However, Gnulib code can still use ((char *) NULL) + something) 
because the Gnulib portability guidelines allow it.


The issue with clang false positives is covered here:

https://www.gnu.org/software/gnulib/manual/html_node/Unsupported-Platforms.html

which lists "clang -fsanitize=undefined" as an unsupported platform 
unless you also specify "-fno-sanitize=pointer-overflow".


The obstack patch you installed is fine, as it's clearer and just as 
fast as the original. However, we needn't go through Gnulib and change 
other code merely because it runs afoul of this false alarm from clang.

Re: undefined-behavior obstack.c:139

[Marc Nieper-Wißkirchen]

Am Fr., 1. Dez. 2023 um 19:42 Uhr schrieb Bruno Haible :

> Marc Nieper-Wißkirchen wrote:
> > By 6.5.6 "Additive Operators":
> >
> > (2) "... one operator shall be a pointer to a complete object type..."
> >
> > NULL, which is a null pointer constant, is not necessarily a pointer to a
> > complete object type.
>
> In my test program, I used a variable of type 'char *'. Which is a pointer
> to a complete object type.
>

Yes, I was referring to the expression "NULL + 0" you mentioned.

[...]

Re: undefined-behavior obstack.c:139

[Bruno Haible]

Marc Nieper-Wißkirchen wrote:
> By 6.5.6 "Additive Operators":
> 
> (2) "... one operator shall be a pointer to a complete object type..."
> 
> NULL, which is a null pointer constant, is not necessarily a pointer to a
> complete object type.

In my test program, I used a variable of type 'char *'. Which is a pointer
to a complete object type.

> (9) "... If the pointer operand and the result do not point to elements of
> the same array object or one past the last element of the array object, the
> behavior is undefined..."
> 
> NULL does not have to point to an element of an array object (or any
> object; see (8)).

Indeed, this sentence appears to forbid ((char *) NULL) + something.
Thanks for highlighting it; I had read this paragraph too quickly.

I'm therefore applying this fix.


2023-12-01  Bruno Haible  

obstack: Avoid undefined behaviour.
Reported by Alexey Palienko  in
.
* lib/obstack.in.h: Include .
(__BPTR_ALIGN): Remove macro.
(__PTR_ALIGN): For the optimized case, compute the alignment through
uintptr_t, instead of computing NULL + something.

diff --git a/lib/obstack.in.h b/lib/obstack.in.h
index 265203b6e2..468a797341 100644
--- a/lib/obstack.in.h
+++ b/lib/obstack.in.h
@@ -111,6 +111,7 @@
 #endif
 
 #include  /* For size_t and ptrdiff_t.  */
+#include  /* For uintptr_t.  */
 #include  /* For memcpy.  */
 
 #if __STDC_VERSION__ < 199901L || defined __HP_cc
@@ -134,20 +135,15 @@
 
 /* If B is the base of an object addressed by P, return the result of
aligning P to the next multiple of A + 1.  B and P must be of type
-   char *.  A + 1 must be a power of 2.  */
-
-#define __BPTR_ALIGN(B, P, A) ((B) + (((P) - (B) + (A)) & ~(A)))
-
-/* Similar to __BPTR_ALIGN (B, P, A), except optimize the common case
-   where pointers can be converted to integers, aligned as integers,
-   and converted back again.  If ptrdiff_t is narrower than a
-   pointer (e.g., the AS/400), play it safe and compute the alignment
-   relative to B.  Otherwise, use the faster strategy of computing the
-   alignment relative to 0.  */
-
-#define __PTR_ALIGN(B, P, A) \
-  __BPTR_ALIGN (sizeof (ptrdiff_t) < sizeof (void *) ? (B) : (char *) 0,  \
-P, A)
+   char *.  A + 1 must be a power of 2.
+   If ptrdiff_t is narrower than a pointer (e.g., the AS/400), play it
+   safe and compute the alignment relative to B.  Otherwise, use the
+   faster strategy of computing the alignment through uintptr_t.  */
+
+#define __PTR_ALIGN(B, P, A) \
+  (sizeof (ptrdiff_t) < sizeof (void *) \
+   ? (B) + (((P) - (B) + (A)) & ~(A))   \
+   : (P) + ((- (uintptr_t) (P)) & (A)))
 
 #ifndef __attribute_pure__
 # define __attribute_pure__ _GL_ATTRIBUTE_PURE

Re: undefined-behavior obstack.c:139

[Marc Nieper-Wißkirchen]

By 6.5.6 "Additive Operators":

(2) "... one operator shall be a pointer to a complete object type..."

NULL, which is a null pointer constant, is not necessarily a pointer to a
complete object type.

(9) "... If the pointer operand and the result do not point to elements of
the same array object or one past the last element of the array object, the
behavior is undefined..."

NULL does not have to point to an element of an array object (or any
object; see (8)).


Am Fr., 1. Dez. 2023 um 17:02 Uhr schrieb Bruno Haible :

> Jeffrey Walton wrote:
> > I think that's a valid finding. NULL is not a valid address. You can't
> > add anything to it.
>
> Can you back this opinion with citations from ISO C 23 [1] ?
>
> Bruno
>
> [1] https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3054.pdf
>
>
>
>
>

Re: undefined-behavior obstack.c:139

[Bruno Haible]

Jeffrey Walton wrote:
> I think that's a valid finding. NULL is not a valid address. You can't
> add anything to it.

Can you back this opinion with citations from ISO C 23 [1] ?

Bruno

[1] https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3054.pdf

Re: undefined-behavior obstack.c:139

[Jeffrey Walton]

On Fri, Dec 1, 2023 at 8:15 AM Bruno Haible  wrote:
>
> [CCing bug-gnulib because obstack.c comes from gnulib.]
>
> Alexey Palienko  wrote in
> :
>
> > It has been built by clang 13 with "-g -fsanitize=address,undefined 
> > -fno-omit-frame-pointer -fsanitize-address-use-after-scope 
> > -fno-sanitize-recover=all"
> > And we have an error:
> >
> > # 
> > /home/docker/.conan/data/m4/latest/_/_/package/3421fde5744f1eadef515027cbcbb9a8fbcd667c/bin/m4
> > obstack.c:139:35: runtime error: applying non-zero offset 107820858999056 
> > to null pointer
> > SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior obstack.c:139:35 in
>
> I reproduce the issue with the newest m4 snapshot and with clang 17.
>
> However, it's not a bug in m4, but rather a false alarm from clang's
> UndefinedBehaviorSanitizer.
>
> Namely, when I use the modified CC value
>   CC="clang 
> -fsanitize=address,undefined,signed-integer-overflow,shift,integer-divide-by-zero
>  -fno-sanitize=pointer-overflow -fsanitize-address-use-after-scope 
> -fno-sanitize-recover=all"
> there are no issues.
>
> The 'pointer-overflow' sanitizer considers adding NULL + 0 as undefined:
> 
> #include 
> int main ()
> {
>   char *p = NULL;
>   char *q = p + 0;
>   return 0;
> }
> 
>
> I don't see wording to this effect in ISO C 23 § 6.5.6.(9).
> Many programs use NULL + 0 in some cases, because it avoids a gratuitous
> test against NULL.
>
> So, I recommend turning off the 'pointer-overflow' sanitizer.
>
> Bruno
>
> [1] https://gitlab.com/gnu-m4/ci-distcheck

I think that's a valid finding. NULL is not a valid address. You can't
add anything to it.

Jeff

Re: undefined-behavior obstack.c:139

[Bruno Haible]

[CCing bug-gnulib because obstack.c comes from gnulib.]

Alexey Palienko  wrote in
:

> It has been built by clang 13 with "-g -fsanitize=address,undefined 
> -fno-omit-frame-pointer -fsanitize-address-use-after-scope 
> -fno-sanitize-recover=all"
> And we have an error:
>
> # 
> /home/docker/.conan/data/m4/latest/_/_/package/3421fde5744f1eadef515027cbcbb9a8fbcd667c/bin/m4
> obstack.c:139:35: runtime error: applying non-zero offset 107820858999056 to 
> null pointer
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior obstack.c:139:35 in

I reproduce the issue with the newest m4 snapshot and with clang 17.

However, it's not a bug in m4, but rather a false alarm from clang's
UndefinedBehaviorSanitizer.

Namely, when I use the modified CC value
  CC="clang 
-fsanitize=address,undefined,signed-integer-overflow,shift,integer-divide-by-zero
 -fno-sanitize=pointer-overflow -fsanitize-address-use-after-scope 
-fno-sanitize-recover=all"
there are no issues.

The 'pointer-overflow' sanitizer considers adding NULL + 0 as undefined:

#include 
int main ()
{
  char *p = NULL;
  char *q = p + 0;
  return 0;
}


I don't see wording to this effect in ISO C 23 § 6.5.6.(9).
Many programs use NULL + 0 in some cases, because it avoids a gratuitous
test against NULL.

So, I recommend turning off the 'pointer-overflow' sanitizer.

Bruno

[1] https://gitlab.com/gnu-m4/ci-distcheck

doc: Update for FreeBSD 14.0

[Bruno Haible]

I've updated the documentation regarding FreeBSD 14.0,
as well as the maint-tools database of headers and functions.

The patch [1] is quite repetitive; the interesting changes are that
  - The *printf functions are now up-to-date with ISO C 23.
  - New functions timerfd_* were added, like on Linux and illumos.
  - The functions mempcpy and wmempcpy were added.
  - Bugs were fixed in getgroups, mktime, strto*l, strfmon_l.

[1] 
https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commitdiff;h=6016e6766c633b22a393c7f009503419f7982161

$ diff freebsd13-config.cache freebsd14-config.cache | grep '^[<>]'
< ac_cv_func___secure_getenv=${ac_cv_func___secure_getenv=no}
< ac_cv_func_issetugid=${ac_cv_func_issetugid=yes}
< ac_cv_func_mempcpy=${ac_cv_func_mempcpy=no}
> ac_cv_func_mempcpy=${ac_cv_func_mempcpy=yes}
< 
ac_cv_func_posix_spawn_file_actions_addchdir_np=${ac_cv_func_posix_spawn_file_actions_addchdir_np=no}
> ac_cv_func_posix_spawn_file_actions_addchdir_np=${ac_cv_func_posix_spawn_file_actions_addchdir_np=yes}
< 
ac_cv_func_posix_spawn_file_actions_addfchdir_np=${ac_cv_func_posix_spawn_file_actions_addfchdir_np=no}
> ac_cv_func_posix_spawn_file_actions_addfchdir_np=${ac_cv_func_posix_spawn_file_actions_addfchdir_np=yes}
< ac_cv_func_sched_getaffinity=${ac_cv_func_sched_getaffinity=no}
> ac_cv_func_sched_getaffinity=${ac_cv_func_sched_getaffinity=yes}
< ac_cv_func_secure_getenv=${ac_cv_func_secure_getenv=no}
> ac_cv_func_secure_getenv=${ac_cv_func_secure_getenv=yes}
< ac_cv_func_strverscmp=${ac_cv_func_strverscmp=no}
> ac_cv_func_strverscmp=${ac_cv_func_strverscmp=yes}
< ac_cv_func_wmempcpy=${ac_cv_func_wmempcpy=no}
> ac_cv_func_wmempcpy=${ac_cv_func_wmempcpy=yes}
> ac_cv_have_decl_mempcpy=${ac_cv_have_decl_mempcpy=yes}
> ac_cv_have_decl_secure_getenv=${ac_cv_have_decl_secure_getenv=yes}
> ac_cv_have_decl_strverscmp=${ac_cv_have_decl_strverscmp=yes}
> ac_cv_have_decl_timespec_getres=${ac_cv_have_decl_timespec_getres=yes}
> ac_cv_have_decl_wmempcpy=${ac_cv_have_decl_wmempcpy=yes}
< ac_cv_header_byteswap_h=${ac_cv_header_byteswap_h=no}
> ac_cv_header_byteswap_h=${ac_cv_header_byteswap_h=yes}
< ac_cv_header_stdckdint_h=${ac_cv_header_stdckdint_h=no}
> ac_cv_header_stdckdint_h=${ac_cv_header_stdckdint_h=yes}
< gl_cv_func_getgroups_works=${gl_cv_func_getgroups_works=no}
> gl_cv_func_getgroups_works=${gl_cv_func_getgroups_works=yes}
> gl_cv_func_posix_spawn_secure_exec=${gl_cv_func_posix_spawn_secure_exec=yes}
> gl_cv_func_posix_spawn_works=${gl_cv_func_posix_spawn_works=yes}
> gl_cv_func_posix_spawnp_secure_exec=${gl_cv_func_posix_spawnp_secure_exec=no}
< gl_cv_func_printf_directive_b=${gl_cv_func_printf_directive_b=no}
> gl_cv_func_printf_directive_b=${gl_cv_func_printf_directive_b=yes}
< 
gl_cv_func_printf_directive_uppercase_b=${gl_cv_func_printf_directive_uppercase_b=no}
> gl_cv_func_printf_directive_uppercase_b=${gl_cv_func_printf_directive_uppercase_b=yes}
< gl_cv_func_printf_sizes_c23=${gl_cv_func_printf_sizes_c23=no}
> gl_cv_func_printf_sizes_c23=${gl_cv_func_printf_sizes_c23=yes}
> gl_cv_func_sched_getaffinity3=${gl_cv_func_sched_getaffinity3=yes}
< gl_cv_func_strtol_works=${gl_cv_func_strtol_works=no}
> gl_cv_func_strtol_works=${gl_cv_func_strtol_works=yes}
< gl_cv_func_strtoll_works=${gl_cv_func_strtoll_works=no}
< gl_cv_func_strtoul_works=${gl_cv_func_strtoul_works=no}
< gl_cv_func_strtoull_works=${gl_cv_func_strtoull_works=no}
> gl_cv_func_strtoll_works=${gl_cv_func_strtoll_works=yes}
> gl_cv_func_strtoul_works=${gl_cv_func_strtoul_works=yes}
> gl_cv_func_strtoull_works=${gl_cv_func_strtoull_works=yes}
< gl_cv_func_working_mktime=${gl_cv_func_working_mktime=no}
> gl_cv_func_working_mktime=${gl_cv_func_working_mktime=yes}
< gl_cv_have_raw_decl_mempcpy=${gl_cv_have_raw_decl_mempcpy=no}
> gl_cv_have_raw_decl_mempcpy=${gl_cv_have_raw_decl_mempcpy=yes}
< gl_cv_have_raw_decl_secure_getenv=${gl_cv_have_raw_decl_secure_getenv=no}
> gl_cv_have_raw_decl_secure_getenv=${gl_cv_have_raw_decl_secure_getenv=yes}
< gl_cv_have_raw_decl_strverscmp=${gl_cv_have_raw_decl_strverscmp=no}
> gl_cv_have_raw_decl_strverscmp=${gl_cv_have_raw_decl_strverscmp=yes}
< gl_cv_have_raw_decl_timespec_getres=${gl_cv_have_raw_decl_timespec_getres=no}
> gl_cv_have_raw_decl_timespec_getres=${gl_cv_have_raw_decl_timespec_getres=yes}
< gl_cv_have_raw_decl_wmempcpy=${gl_cv_have_raw_decl_wmempcpy=no}
> gl_cv_have_raw_decl_wmempcpy=${gl_cv_have_raw_decl_wmempcpy=yes}
< gl_cv_onwards_func_issetugid=${gl_cv_onwards_func_issetugid=yes}
< gl_cv_onwards_func_mempcpy=${gl_cv_onwards_func_mempcpy=no}
> gl_cv_onwards_func_mempcpy=${gl_cv_onwards_func_mempcpy=yes}
< 
gl_cv_onwards_func_sched_getaffinity=${gl_cv_onwards_func_sched_getaffinity=no}
> gl_cv_onwards_func_sched_getaffinity=${gl_cv_onwards_func_sched_getaffinity=yes}
< gl_cv_onwards_func_wmempcpy=${gl_cv_onwards_func_wmempcpy=no}
> gl_cv_onwards_func_wmempcpy=${gl_cv_onwards_func_wmempcpy=yes}
< gl_cv_qsort_r_signature=${gl_cv_qsort_r_signature=BSD}
> 

sethostname tests: Fix a compilation error on FreeBSD 14.0

[Bruno Haible]

On FreeBSD 14.0/x86_64, I see this compilation error in a testdir:

../../gltests/test-sethostname1.c:23:1: error: incompatible function pointer 
types initializing 'int (*)(const char *, size_t)' (aka 'int (*)(const char *, 
unsigned long)') with an expression of type 'int (const char *, int)' 
[-Wincompatible-function-pointer-types]
SIGNATURE_CHECK (sethostname, int, (const char *, size_t));
^~~~
../../gltests/signature.h:39:3: note: expanded from macro 'SIGNATURE_CHECK'
  SIGNATURE_CHECK1 (fn, ret, args, __LINE__)
  ^ ~~
../../gltests/signature.h:44:3: note: expanded from macro 'SIGNATURE_CHECK1'
  SIGNATURE_CHECK2 (fn, ret, args, id) /* macroexpand line */
  ^ ~~
../../gltests/signature.h:46:27: note: expanded from macro 'SIGNATURE_CHECK2'
  _GL_UNUSED static ret (*signature_check ## id) args = fn
  ^ ~~
:122:1: note: expanded from here
signature_check23
^
1 error generated.

The sethostname() prototype has not changed. What has changed is that
FreeBSD is now using clang 16, and clang 16 is more picky regarding function
prototype mismatches than earlier releases.

This patch provides a fix.


2023-12-01  Bruno Haible  

sethostname tests: Fix a compilation error on FreeBSD 14.0.
* tests/test-sethostname1.c: Skip the SIGNATURE_CHECK on some platforms.
* doc/glibc-functions/sethostname.texi: Update platforms list.

diff --git a/doc/glibc-functions/sethostname.texi 
b/doc/glibc-functions/sethostname.texi
index 9d6852d63e..fdfe1af4ed 100644
--- a/doc/glibc-functions/sethostname.texi
+++ b/doc/glibc-functions/sethostname.texi
@@ -39,5 +39,5 @@
 @item
 The second parameter is @code{int} instead of @code{size_t}
 on some platforms:
-macOS 11.1, MidnightBSD 2.0, Solaris 11 2010-11.
+macOS 12.5, FreeBSD 14.0, MidnightBSD 3.0, IRIX 6.5, Solaris 11 2010-11, 
Solaris 11 OpenIndiana, Solaris 11 OmniOS.
 @end itemize
diff --git a/tests/test-sethostname1.c b/tests/test-sethostname1.c
index f3fa7cb89f..7dc62ac7a2 100644
--- a/tests/test-sethostname1.c
+++ b/tests/test-sethostname1.c
@@ -20,7 +20,9 @@
 #include 
 
 #include "signature.h"
+#if !((defined __APPLE__ && defined __MACH__) || defined __FreeBSD__ || 
defined __sgi || defined __sun)
 SIGNATURE_CHECK (sethostname, int, (const char *, size_t));
+#endif
 
 int do_dangerous_things;