Fix memleak in getdelim.m4

Fix memleak in getdelim.m4

[Tim Rühsen]

[-- Attachment #1.1.1: Type: text/plain, Size: 1104 bytes --]

With leak sanitizer on, the test for getdelim fails due to a memory leak.

The attached patch fixes it. (Please feel free to amend.)

Regards, Tim

Output from config.log:

configure:26259: checking for working getdelim function
configure:26325: gcc-10 -o conftest -O1 -g -fno-omit-frame-pointer
-DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION -fsanitize
=undefined,bool,alignment,null,enum,address,leak,nonnull-attribute
-fno-sanitize-recover=all -fsanitize-address-use-afte
r-scope   conftest.c  >&5
configure:26325: $? = 0
configure:26325: ./conftest

=================================================================
==551573==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 120 byte(s) in 1 object(s) allocated from:
    #0 0x7fb818f56e8f in malloc
(/usr/lib/x86_64-linux-gnu/libasan.so.6+0xa9e8f)
    #1 0x7fb8183f51bf in _IO_getdelim
/build/glibc-WZtAaN/glibc-2.30/libio/iogetdelim.c:62
    #2 0x7fb8198dd72f  (<unknown module>)

SUMMARY: AddressSanitizer: 120 byte(s) leaked in 1 allocation(s).
configure:26325: $? = 1
configure: program exited with status 1

[-- Attachment #1.1.2: 0001-Fix-memleak-in-getdelim-C-code-to-pacify-leak-saniti.patch --]
[-- Type: text/x-patch, Size: 1284 bytes --]

From 0b9451c7c8919121df67b71fdd2f993605c1abc1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tim=20R=C3=BChsen?= <tim.ruehsen@gmx.de>
Date: Mon, 18 May 2020 12:36:16 +0200
Subject: [PATCH] Fix memleak in getdelim C code to pacify leak sanitizer

---
 ChangeLog      | 4 ++++
 m4/getdelim.m4 | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index 9551d9934..2f7d6d4bb 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2020-05-18  Tim Rühsen  <tim.ruehsenqgmx.de>
+
+	* m4/getdelim.m4: Fix memleak.
+
 2020-05-17  Bruno Haible  <bruno@clisp.org>
 
 	Clarify intended usage of the license file modules.
diff --git a/m4/getdelim.m4 b/m4/getdelim.m4
index 9f4c7f6e9..ac3917b11 100644
--- a/m4/getdelim.m4
+++ b/m4/getdelim.m4
@@ -1,4 +1,4 @@
-# getdelim.m4 serial 14
+# getdelim.m4 serial 15
 
 dnl Copyright (C) 2005-2007, 2009-2020 Free Software Foundation, Inc.
 dnl
@@ -42,6 +42,7 @@ AC_DEFUN([gl_FUNC_GETDELIM],
         int len = getdelim (&line, &siz, '\n', in);
         if (!(len == 4 && line && strcmp (line, "foo\n") == 0))
           { free (line); fclose (in); return 2; }
+        free (line);
       }
       {
         /* Test result for a NULL buffer and a non-zero size.
-- 
2.26.2


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Fix memleak in getdelim.m4

[Bruno Haible]

Hi Tim,

> With leak sanitizer on, the test for getdelim fails due to a memory leak.

Is this the only configure test that fails this way?

I mean, it's 3 years since we last looked for such configure test failures. [1]

The way to determine the answer is:
1. Create a test dir of all gnulib modules.
2. Configure it with --config-cache.
3. Configure it with --config-cache and your sanitizer options.
4. Compare the generated config.cache and config.status files.

Bruno

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

Re: Fix memleak in getdelim.m4

[Tim Rühsen]

[-- Attachment #1.1: Type: text/plain, Size: 981 bytes --]

Hi Bruno,

On 18.05.20 13:50, Bruno Haible wrote:
> Hi Tim,
> 
>> With leak sanitizer on, the test for getdelim fails due to a memory leak.
> 
> Is this the only configure test that fails this way?

The only one that fails when running GNU Poke ./configure.

> I mean, it's 3 years since we last looked for such configure test failures. [1]
> 
> The way to determine the answer is:
> 1. Create a test dir of all gnulib modules.
> 2. Configure it with --config-cache.
> 3. Configure it with --config-cache and your sanitizer options.
> 4. Compare the generated config.cache and config.status files.

I am short on time and would like to prevent being distracted too much
from my (random) work at GNU Poke.

If you could give me a quick instruction on how to do 1., I'll happily
go for steps 2-4. I already have a fresh cloned gnulib locally.

Regards, Tim

> 
> Bruno
> 
> [1] https://lists.gnu.org/archive/html/bug-gnulib/2017-05/msg00183.html
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Fix memleak in getdelim.m4

[Bruno Haible]

Hi Tim,

> > The way to determine the answer is:
> > 1. Create a test dir of all gnulib modules.
> > 2. Configure it with --config-cache.
> > 3. Configure it with --config-cache and your sanitizer options.
> > 4. Compare the generated config.cache and config.status files.
> 
> I am short on time and would like to prevent being distracted too much

Everyone here is probably in the same situation...

> If you could give me a quick instruction on how to do 1.

I typically use this command:

  rm -rf ../testdir-all; ./gnulib-tool --create-testdir --dir=../testdir-all --single-configure

> I'll happily
> go for steps 2-4. I already have a fresh cloned gnulib locally.

Thanks! We can then go through the findings one by one.

Bruno

Re: Fix memleak in getdelim.m4

[Tim Rühsen]

[-- Attachment #1.1: Type: text/plain, Size: 1869 bytes --]



On 18.05.20 21:44, Bruno Haible wrote:
> Hi Tim,
> 
>>> The way to determine the answer is:
>>> 1. Create a test dir of all gnulib modules.
>>> 2. Configure it with --config-cache.
>>> 3. Configure it with --config-cache and your sanitizer options.
>>> 4. Compare the generated config.cache and config.status files.
>>
>> I am short on time and would like to prevent being distracted too much
> 
> Everyone here is probably in the same situation...
> 
>> If you could give me a quick instruction on how to do 1.
> 
> I typically use this command:
> 
>   rm -rf ../testdir-all; ./gnulib-tool --create-testdir --dir=../testdir-all --single-configure

This results in:

executing autopoint --force
autopoint: *** The AM_GNU_GETTEXT_VERSION declaration in your configure.ac
               file requires the infrastructure from gettext-0.20 but
this version
               is older. Please upgrade to gettext-0.20 or newer.
autopoint: *** Stop.


As Debian unstable is at gettext 0.19.8.1, I tried to build gettext from
git master. This results in

make[7]: Entering directory
'/home/tim/src/gettext/gettext-tools/examples/tmp-hello-pascal'
make[7]: warning: jobserver unavailable: using -j1.  Add '+' to parent
make rule.
LOCALEDIR='/usr/local/share/locale' /usr/bin/ppcx64 -o./hello ./hello.pas
Free Pascal Compiler version 3.0.4+dfsg-23 [2019/11/25] for x86_64
Copyright (c) 1993-2017 by Florian Klaempfl and others
Target OS: Linux for x86-64
Compiling ./hello.pas
hello.pas(9,6) Fatal: Can't find unit gettext used by hello
Fatal: Compilation aborted
make[7]: *** [Makefile:798: hello.rsj] Error 1


Is it really needed to fail the whole build just because a *PASCAL*
example doesn't compile ?
Wow, I used Pascal in the 1980s the last time, didn't know it is still
used except for fun projects :)

Regards, Tim


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Fix memleak in getdelim.m4

[Bruno Haible]

Hi Tim,

> >   rm -rf ../testdir-all; ./gnulib-tool --create-testdir --dir=../testdir-all --single-configure
> 
> This results in:
> 
> executing autopoint --force
> autopoint: *** The AM_GNU_GETTEXT_VERSION declaration in your configure.ac
>                file requires the infrastructure from gettext-0.20 but
> this version
>                is older. Please upgrade to gettext-0.20 or newer.
> autopoint: *** Stop.

You may try to pass '--avoid=gettext' to overcome this.

> As Debian unstable is at gettext 0.19.8.1, I tried to build gettext from
> git master. This results in
> 
> make[7]: Entering directory
> '/home/tim/src/gettext/gettext-tools/examples/tmp-hello-pascal'
> make[7]: warning: jobserver unavailable: using -j1.  Add '+' to parent
> make rule.
> LOCALEDIR='/usr/local/share/locale' /usr/bin/ppcx64 -o./hello ./hello.pas
> Free Pascal Compiler version 3.0.4+dfsg-23 [2019/11/25] for x86_64
> Copyright (c) 1993-2017 by Florian Klaempfl and others
> Target OS: Linux for x86-64
> Compiling ./hello.pas
> hello.pas(9,6) Fatal: Can't find unit gettext used by hello
> Fatal: Compilation aborted
> make[7]: *** [Makefile:798: hello.rsj] Error 1
> 
> 
> Is it really needed to fail the whole build just because a *PASCAL*
> example doesn't compile ?

I may sound a bit old-fashioned when I recommend tarballs over building from
git checkouts. But sometimes it has its advantages. gettext 0.20.2 is not
that old. [1]

Bruno

[1] https://ftp.gnu.org/gnu/gettext/gettext-0.20.2.tar.gz

Re: Fix memleak in getdelim.m4

[Tim Rühsen]

[-- Attachment #1.1: Type: text/plain, Size: 2074 bytes --]

Hi Bruno,

On 20.05.20 00:46, Bruno Haible wrote:
> Hi Tim,
> 
>>>   rm -rf ../testdir-all; ./gnulib-tool --create-testdir --dir=../testdir-all --single-configure
>>
>> This results in:
>>
>> executing autopoint --force
>> autopoint: *** The AM_GNU_GETTEXT_VERSION declaration in your configure.ac
>>                file requires the infrastructure from gettext-0.20 but
>> this version
>>                is older. Please upgrade to gettext-0.20 or newer.
>> autopoint: *** Stop.
> 
> You may try to pass '--avoid=gettext' to overcome this.

./configure --config-cache ends (stops ?) with
The BISON_I18N macro is used without being preceded by AM_GNU_GETTEXT.

$ ls -la config.cache
-rw-r--r-- 1 tim tim 0 Mai 20 20:40 config.cache

Is that expected ?

And I see this output here as well:

gnulib-tool: warning: module euidaccess depends on a module with an
incompatible license: group-member

> 
>> As Debian unstable is at gettext 0.19.8.1, I tried to build gettext from
>> git master. This results in
>>
>> make[7]: Entering directory
>> '/home/tim/src/gettext/gettext-tools/examples/tmp-hello-pascal'
>> make[7]: warning: jobserver unavailable: using -j1.  Add '+' to parent
>> make rule.
>> LOCALEDIR='/usr/local/share/locale' /usr/bin/ppcx64 -o./hello ./hello.pas
>> Free Pascal Compiler version 3.0.4+dfsg-23 [2019/11/25] for x86_64
>> Copyright (c) 1993-2017 by Florian Klaempfl and others
>> Target OS: Linux for x86-64
>> Compiling ./hello.pas
>> hello.pas(9,6) Fatal: Can't find unit gettext used by hello
>> Fatal: Compilation aborted
>> make[7]: *** [Makefile:798: hello.rsj] Error 1
>>
>>
>> Is it really needed to fail the whole build just because a *PASCAL*
>> example doesn't compile ?
> 
> I may sound a bit old-fashioned when I recommend tarballs over building from
> git checkouts. But sometimes it has its advantages. gettext 0.20.2 is not
> that old. [1]
> 
> Bruno
> 
> [1] https://ftp.gnu.org/gnu/gettext/gettext-0.20.2.tar.gz

I will install that version and try again.

Regards, Tim


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

relicense module 'group-member'

[Bruno Haible]

Hi Jim, Paul, Eric,

You are the contributors of the module 'group-member', which is under GPLv3+.
Can you agree to relicensing it under LGPLv2+?

Rationale:

The 'euidaccess' module, which is under LGPLv2+, needs to use this module
(for portability to many platforms). Currently gnulib-tool emits a warning
about the license mismatch:

  gnulib-tool: warning: module euidaccess depends on a module with an incompatible license: group-member

The first obstacle to fix this was done by Paul on 2012-10-22, when he replaced
uses of xmalloc() with malloc().

The remaining obstacle is simply the approvals of the contributors.

Bruno

Re: relicense module 'group-member'

[Eric Blake]

On 5/21/20 9:22 AM, Bruno Haible wrote:
> Hi Jim, Paul, Eric,
> 
> You are the contributors of the module 'group-member', which is under GPLv3+.
> Can you agree to relicensing it under LGPLv2+?
> 
> Rationale:
> 
> The 'euidaccess' module, which is under LGPLv2+, needs to use this module
> (for portability to many platforms).

Yes, the looser license for my contributions makes sense.


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: relicense module 'group-member'

[Jim Meyering]

On Thu, May 21, 2020 at 7:44 AM Eric Blake <eblake@redhat.com> wrote:
> On 5/21/20 9:22 AM, Bruno Haible wrote:
> > Hi Jim, Paul, Eric,
> >
> > You are the contributors of the module 'group-member', which is under GPLv3+.
> > Can you agree to relicensing it under LGPLv2+?
> >
> > Rationale:
> >
> > The 'euidaccess' module, which is under LGPLv2+, needs to use this module
> > (for portability to many platforms).
>
> Yes, the looser license for my contributions makes sense.

Same here. Thanks.

Re: relicense module 'group-member'

[Bruno Haible]

> > Yes, the looser license for my contributions makes sense.
> 
> Same here. Thanks.

Thanks for your approvals. Done:


2020-05-21  Bruno Haible  <bruno@clisp.org>

	group-member: Relicense under LGPLv2+.
	Jim Meyering's approval is in
	<https://lists.gnu.org/archive/html/bug-gnulib/2020-05/msg00218.html>.
	Paul Eggert's approval is in
	<https://lists.gnu.org/archive/html/bug-gnulib/2020-05/msg00225.html>.
	Eric Blake's approval is in
	<https://lists.gnu.org/archive/html/bug-gnulib/2020-05/msg00217.html>.
	* modules/group-member (License): Change to LGPLv2+.

diff --git a/modules/group-member b/modules/group-member
index 9baf8e9..1b743a3 100644
--- a/modules/group-member
+++ b/modules/group-member
@@ -25,7 +25,7 @@ Include:
 <unistd.h>
 
 License:
-GPL
+LGPLv2+
 
 Maintainer:
 Jim Meyering

Re: relicense module 'group-member'

[Paul Eggert]

On 5/21/20 7:22 AM, Bruno Haible wrote:
> Can you agree to relicensing it under LGPLv2+?

Yes, that's fine.

Re: Fix memleak in getdelim.m4

[Bruno Haible]

Hi Tim,

> > You may try to pass '--avoid=gettext' to overcome this.
> 
> ./configure --config-cache ends (stops ?) with
> The BISON_I18N macro is used without being preceded by AM_GNU_GETTEXT.

Oh well. Then you apparently need '--avoid=gettext --avoid=bison-i18n'.

> And I see this output here as well:
> 
> gnulib-tool: warning: module euidaccess depends on a module with an
> incompatible license: group-member

This points to a long-standing problem we have with the 'euidaccess' module;
we should be able to resolve it now. See the other mail.

Bruno

Re: Fix memleak in getdelim.m4

[Tim Rühsen]

[-- Attachment #1.1: Type: text/plain, Size: 1976 bytes --]

Hi Bruno,

On 18.05.20 21:44, Bruno Haible wrote:
> Hi Tim,
> 
>>> The way to determine the answer is:
>>> 1. Create a test dir of all gnulib modules.
>>> 2. Configure it with --config-cache.
>>> 3. Configure it with --config-cache and your sanitizer options.
>>> 4. Compare the generated config.cache and config.status files.
>>
>> I am short on time and would like to prevent being distracted too much
> 
> Everyone here is probably in the same situation...
> 
>> If you could give me a quick instruction on how to do 1.
> 
> I typically use this command:
> 
>   rm -rf ../testdir-all; ./gnulib-tool --create-testdir --dir=../testdir-all --single-configure
> 
>> I'll happily
>> go for steps 2-4. I already have a fresh cloned gnulib locally.
> 
> Thanks! We can then go through the findings one by one.

With my getdelim patch applied, this is the summary of sanitizer findings:

$ egrep 'SUMM|checking' config.log|grep -B1 SUMM
configure:14038: checking for working C stack overflow detection
SUMMARY: UndefinedBehaviorSanitizer: implicit-integer-sign-change
conftest.c:379:30 in
--
configure:36833: checking whether memmem works
SUMMARY: UndefinedBehaviorSanitizer: invalid-null-argument
conftest.c:513:32 in
--
configure:55268: checking whether fchownat works with AT_SYMLINK_NOFOLLOW
SUMMARY: UndefinedBehaviorSanitizer: implicit-integer-sign-change
conftest.c:812:50 in
configure:55326: checking whether fchownat works with an empty file name
SUMMARY: UndefinedBehaviorSanitizer: implicit-integer-sign-change
conftest.c:817:37 in
--
configure:67322: checking whether glob lists broken symlinks
SUMMARY: AddressSanitizer: 37 byte(s) leaked in 2 allocation(s).
--
configure:94107: checking for working re_compile_pattern
SUMMARY: AddressSanitizer: 20072 byte(s) leaked in 116 allocation(s).


The lzip'ed tar archive with all 4 files is 137k. Is it ok to send it
here or do you better like via PM ?

Regards, Tim


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Fix memleak in getdelim.m4

[Bruno Haible]

Hi Tim,

> With my getdelim patch applied, this is the summary of sanitizer findings:
> 
> $ egrep 'SUMM|checking' config.log|grep -B1 SUMM
> configure:14038: checking for working C stack overflow detection
> SUMMARY: UndefinedBehaviorSanitizer: implicit-integer-sign-change
> conftest.c:379:30 in
> --
> configure:36833: checking whether memmem works
> SUMMARY: UndefinedBehaviorSanitizer: invalid-null-argument
> conftest.c:513:32 in
> --
> configure:55268: checking whether fchownat works with AT_SYMLINK_NOFOLLOW
> SUMMARY: UndefinedBehaviorSanitizer: implicit-integer-sign-change
> conftest.c:812:50 in
> configure:55326: checking whether fchownat works with an empty file name
> SUMMARY: UndefinedBehaviorSanitizer: implicit-integer-sign-change
> conftest.c:817:37 in
> --
> configure:67322: checking whether glob lists broken symlinks
> SUMMARY: AddressSanitizer: 37 byte(s) leaked in 2 allocation(s).
> --
> configure:94107: checking for working re_compile_pattern
> SUMMARY: AddressSanitizer: 20072 byte(s) leaked in 116 allocation(s).

Interesting! Thanks!

> The lzip'ed tar archive with all 4 files is 137k. Is it ok to send it
> here or do you better like via PM ?

I don't know the actual size limit for attachments on lists.gnu.org.
But 137 kB is perfectly OK.

Bruno

Re: Fix memleak in getdelim.m4

[Tim Rühsen]

[-- Attachment #1.1.1: Type: text/plain, Size: 314 bytes --]

Hi Bruno,

On 21.05.20 16:26, Bruno Haible wrote:
>> The lzip'ed tar archive with all 4 files is 137k. Is it ok to send it
>> here or do you better like via PM ?
> 
> I don't know the actual size limit for attachments on lists.gnu.org.
> But 137 kB is perfectly OK.

Good, here it is :-)

Regards, Tim

[-- Attachment #1.1.2: config.tar.lz --]
[-- Type: application/octet-stream, Size: 137066 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Fix memleak in getdelim.m4

[Bruno Haible]

Hi Tim,

These comparisons are not so useful, because they not only come from different
compilers (gcc vs. clang) but also from systems with different libcs: there
are differences regarding libm, calloc, thrd_create, pthread_sigmask, and other
functions. It would be more useful to compare, on the _same_ system:
  - gcc vs. clang,
  - clag vs. clang -fsanitize=...

Bruno

Re: Fix memleak in getdelim.m4

[Tim Rühsen]

[-- Attachment #1.1: Type: text/plain, Size: 778 bytes --]

Hi Bruno,

On 21.05.20 21:31, Bruno Haible wrote:
> Hi Tim,
> 
> These comparisons are not so useful, because they not only come from different
> compilers (gcc vs. clang) but also from systems with different libcs: there
> are differences regarding libm, calloc, thrd_create, pthread_sigmask, and other
> functions. It would be more useful to compare, on the _same_ system:
>   - gcc vs. clang,
>   - clag vs. clang -fsanitize=...

Well, all made on the _same_ system (my desktop) with just ~35 minutes
in between.

Since system updates are made manually, it is _very_ unlikely that I
made an update between the tests.

But yes, I made one test without $CC set (so likely with gcc) and the
other one with CC=clang and CFLAGS=-fsanitize...

Regards, Tim


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Fix memleak in getdelim.m4

[Bruno Haible]

Hi Tim,

> Well, all made on the _same_ system (my desktop) with just ~35 minutes
> in between.
> 
> Since system updates are made manually, it is _very_ unlikely that I
> made an update between the tests.

:-)

> But yes, I made one test without $CC set (so likely with gcc) and the
> other one with CC=clang and CFLAGS=-fsanitize...

Then it would be useful to have the config.{cache,log,status} results also
for a build where CC=clang but CFLAGS unset. So as to distinguish which
differences come from clang vs. gcc and which come from the sanitizers.

Each of these differences points to a possible problem in our *.m4 macros.

Bruno

Re: Fix memleak in getdelim.m4

[Tim Rühsen]

[-- Attachment #1.1.1: Type: text/plain, Size: 1056 bytes --]

Hi Bruno,

On 22.05.20 17:25, Bruno Haible wrote:
>> But yes, I made one test without $CC set (so likely with gcc) and the
>> other one with CC=clang and CFLAGS=-fsanitize...
> 
> Then it would be useful to have the config.{cache,log,status} results also
> for a build where CC=clang but CFLAGS unset. So as to distinguish which
> differences come from clang vs. gcc and which come from the sanitizers.
> 
> Each of these differences points to a possible problem in our *.m4 macros.

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).

The sanitizer flags for gcc and clang differ in that clang supports
"integer" and "nullability" while gcc does not.

You find the exact flags in config.log.*.sanitize files.

The gnulib commit was 9bf6b3cafa2a39c09143182f2271d6c877eabfdc (latest
master from this evening).

Regards, Tim

[-- Attachment #1.1.2: config.tar.lz --]
[-- Type: application/octet-stream, Size: 256828 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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  <bruno@clisp.org>

	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 (&m, 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 (&samples[j]);
   /* Remove bits that are common (e.g. if representation of the first mantissa
      bit is explicit).  */
   for (j = 0; j < NWORDS; j++)

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 <stdlib.h>
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
	.p2align	4, 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 <https://bugs.llvm.org/show_bug.cgi?id=37304>. 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  <bruno@clisp.org>

	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);

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 <https://bugs.llvm.org/show_bug.cgi?id=37304>. 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 <stdlib.h>
  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;
  }

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 <stdlib.h>
>   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 <stdlib.h>
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  <bruno@clisp.org>

	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

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: Fix calloc.m4 test

[Bruno Haible]

> > Does this look like a reportable bug? :-)
> 
> Absolutely!
> 

I've reported it at <https://bugs.llvm.org/show_bug.cgi?id=46055>.

Bruno

Re: Fix calloc.m4 test

[Bruno Haible]

On 2020-05-23 I wrote:
> 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 <https://bugs.llvm.org/show_bug.cgi?id=37304>. 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  <bruno@clisp.org>
> 
> 	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.

The same problem occurs also in the test suite. The continuous integration [1]
detected it.

[1] https://gitlab.com/gnulib/gnulib-ci/-/pipelines/153500376


2020-06-06  Bruno Haible  <bruno@clisp.org>

	calloc-gnu tests: Avoid a test failure with clang.
	* tests/test-calloc-gnu.c (main): Mark the pointer variable as
	'volatile', to defeat compiler optimizations.

diff --git a/tests/test-calloc-gnu.c b/tests/test-calloc-gnu.c
index c13ba47..2ae3925 100644
--- a/tests/test-calloc-gnu.c
+++ b/tests/test-calloc-gnu.c
@@ -36,20 +36,26 @@ int
 main ()
 {
   /* Check that calloc (0, 0) is not a NULL pointer.  */
-  void *p = calloc (0, 0);
-  if (p == NULL)
-    return 1;
-  free (p);
+  {
+    void * volatile p = calloc (0, 0);
+    if (p == NULL)
+      return 1;
+    free (p);
+  }
 
   /* Check that calloc fails when requested to allocate a block of memory
      larger than SIZE_MAX bytes.
-     We use eight (), not 8, to avoid a compiler warning from GCC 7.  */
-  p = calloc ((size_t) -1 / 8 + 1, eight ());
-  if (p != NULL)
-    {
-      free (p);
-      return 1;
-    }
+     We use eight (), not 8, to avoid a compiler warning from GCC 7.
+     'volatile' is needed to defeat an incorrect optimization by clang 10,
+     see <https://bugs.llvm.org/show_bug.cgi?id=46055>.  */
+  {
+    void * volatile p = calloc ((size_t) -1 / 8 + 1, eight ());
+    if (p != NULL)
+      {
+        free (p);
+        return 2;
+      }
+  }
 
   return 0;
 }

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  <bruno@clisp.org>

	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 <math.h>
-# 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 <math.h>
-# 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 <math.h> 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 <math.h> 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 rpl_isnand (double x);
 #   define gl_isnan_d(x) rpl_isnand (x)
 #  endif
-#  if @HAVE_ISNANL@ && __GNUC__ >= 4
+#  if @HAVE_ISNANL@ && __GNUC__ >= 4 && (!defined __clang__ || __has_builtin (__builtin_isnanl))
 #   define gl_isnan_l(x) __builtin_isnanl ((long double)(x))
 #  else
 _GL_EXTERN_C int rpl_isnanl (long double x) _GL_ATTRIBUTE_CONST;
@@ -2408,7 +2414,7 @@ _GL_EXTERN_C int rpl_isnanl (long double x) _GL_ATTRIBUTE_CONST;
    (sizeof (x) == sizeof (long double) ? gl_isnan_l (x) : \
     sizeof (x) == sizeof (double) ? gl_isnan_d (x) : \
     gl_isnan_f (x))
-# elif __GNUC__ >= 4
+# elif __GNUC__ >= 4 && (!defined __clang__ || (__has_builtin (__builtin_isnanf) && __has_builtin (__builtin_isnanl)))
 #  undef isnan
 #  define isnan(x) \
    (sizeof (x) == sizeof (long double) ? __builtin_isnanl ((long double)(x)) : \
diff --git a/m4/isnanf.m4 b/m4/isnanf.m4
index 4e9fb48..7dd67bd 100644
--- a/m4/isnanf.m4
+++ b/m4/isnanf.m4
@@ -1,4 +1,4 @@
-# isnanf.m4 serial 15
+# isnanf.m4 serial 16
 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,
@@ -74,7 +74,10 @@ AC_DEFUN([gl_HAVE_ISNANF_NO_LIBM],
       AC_LINK_IFELSE(
         [AC_LANG_PROGRAM(
            [[#include <math.h>
-             #if __GNUC__ >= 4
+             #ifndef __has_builtin
+             # define __has_builtin(name) 0
+             #endif
+             #if __GNUC__ >= 4 && (!defined __clang__ || __has_builtin (__builtin_isnanf))
              # undef isnanf
              # define isnanf(x) __builtin_isnanf ((float)(x))
              #elif defined isnan
@@ -99,7 +102,10 @@ AC_DEFUN([gl_HAVE_ISNANF_IN_LIBM],
       AC_LINK_IFELSE(
         [AC_LANG_PROGRAM(
            [[#include <math.h>
-             #if __GNUC__ >= 4
+             #ifndef __has_builtin
+             # define __has_builtin(name) 0
+             #endif
+             #if __GNUC__ >= 4 && (!defined __clang__ || __has_builtin (__builtin_isnanf))
              # undef isnanf
              # define isnanf(x) __builtin_isnanf ((float)(x))
              #elif defined isnan
@@ -127,7 +133,10 @@ AC_DEFUN([gl_ISNANF_WORKS],
       AC_RUN_IFELSE(
         [AC_LANG_SOURCE([[
 #include <math.h>
-#if __GNUC__ >= 4
+#ifndef __has_builtin
+# define __has_builtin(name) 0
+#endif
+#if __GNUC__ >= 4 && (!defined __clang__ || __has_builtin (__builtin_isnanf))
 # undef isnanf
 # define isnanf(x) __builtin_isnanf ((float)(x))
 #elif defined isnan
diff --git a/m4/isnanl.m4 b/m4/isnanl.m4
index 9874418..75d54629 100644
--- a/m4/isnanl.m4
+++ b/m4/isnanl.m4
@@ -1,4 +1,4 @@
-# isnanl.m4 serial 20
+# isnanl.m4 serial 21
 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,
@@ -68,7 +68,10 @@ AC_DEFUN([gl_HAVE_ISNANL_NO_LIBM],
       AC_LINK_IFELSE(
         [AC_LANG_PROGRAM(
            [[#include <math.h>
-             #if __GNUC__ >= 4
+             #ifndef __has_builtin
+             # define __has_builtin(name) 0
+             #endif
+             #if __GNUC__ >= 4 && (!defined __clang__ || __has_builtin (__builtin_isnanl))
              # undef isnanl
              # define isnanl(x) __builtin_isnanl ((long double)(x))
              #elif defined isnan
@@ -93,7 +96,10 @@ AC_DEFUN([gl_HAVE_ISNANL_IN_LIBM],
       AC_LINK_IFELSE(
         [AC_LANG_PROGRAM(
            [[#include <math.h>
-             #if __GNUC__ >= 4
+             #ifndef __has_builtin
+             # define __has_builtin(name) 0
+             #endif
+             #if __GNUC__ >= 4 && (!defined __clang__ || __has_builtin (__builtin_isnanl))
              # undef isnanl
              # define isnanl(x) __builtin_isnanl ((long double)(x))
              #elif defined isnan
@@ -123,7 +129,10 @@ AC_DEFUN([gl_FUNC_ISNANL_WORKS],
 #include <float.h>
 #include <limits.h>
 #include <math.h>
-#if __GNUC__ >= 4
+#ifndef __has_builtin
+# define __has_builtin(name) 0
+#endif
+#if __GNUC__ >= 4 && (!defined __clang__ || __has_builtin (__builtin_isnanl))
 # undef isnanl
 # define isnanl(x) __builtin_isnanl ((long double)(x))
 #elif defined isnan

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  <bruno@clisp.org>

	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

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}
-gl_cv_func_logbl_no_libm=${gl_cv_func_logbl_no_libm=no}
+gl_cv_func_logbl_no_libm=${gl_cv_func_logbl_no_libm=yes}
-gl_cv_func_logl_in_libm=${gl_cv_func_logl_in_libm=yes}
-gl_cv_func_logl_no_libm=${gl_cv_func_logl_no_libm=no}
+gl_cv_func_logl_no_libm=${gl_cv_func_logl_no_libm=yes}
-gl_cv_func_pow_in_libm=${gl_cv_func_pow_in_libm=yes}
-gl_cv_func_pow_no_libm=${gl_cv_func_pow_no_libm=no}
+gl_cv_func_pow_no_libm=${gl_cv_func_pow_no_libm=yes}
-gl_cv_func_printf_enomem=${gl_cv_func_printf_enomem=yes}
+gl_cv_func_printf_enomem=${gl_cv_func_printf_enomem=no}
-gl_cv_func_sinl_in_libm=${gl_cv_func_sinl_in_libm=yes}
-gl_cv_func_sinl_no_libm=${gl_cv_func_sinl_no_libm=no}
+gl_cv_func_sinl_no_libm=${gl_cv_func_sinl_no_libm=yes}
-gl_cv_func_sqrtl_in_libm=${gl_cv_func_sqrtl_in_libm=yes}
-gl_cv_func_sqrtl_no_libm=${gl_cv_func_sqrtl_no_libm=no}
+gl_cv_func_sqrtl_no_libm=${gl_cv_func_sqrtl_no_libm=yes}
-gl_cv_func_tanl_in_libm=${gl_cv_func_tanl_in_libm=yes}
-gl_cv_func_tanl_no_libm=${gl_cv_func_tanl_no_libm=no}
+gl_cv_func_tanl_no_libm=${gl_cv_func_tanl_no_libm=yes}

Consequences of a failed gl_PRINTF_ENOMEM test:

-gl_cv_func_dprintf_posix=${gl_cv_func_dprintf_posix=yes}
+gl_cv_func_dprintf_posix=${gl_cv_func_dprintf_posix=no}
-gl_cv_func_fprintf_posix=${gl_cv_func_fprintf_posix=yes}
+gl_cv_func_fprintf_posix=${gl_cv_func_fprintf_posix=no}
-gl_cv_func_obstack_printf_posix=${gl_cv_func_obstack_printf_posix=yes}
+gl_cv_func_obstack_printf_posix=${gl_cv_func_obstack_printf_posix=no}
-gl_cv_func_snprintf_posix=${gl_cv_func_snprintf_posix=yes}
+gl_cv_func_snprintf_posix=${gl_cv_func_snprintf_posix=no}
-gl_cv_func_sprintf_posix=${gl_cv_func_sprintf_posix=yes}
+gl_cv_func_sprintf_posix=${gl_cv_func_sprintf_posix=no}
-gl_cv_func_vasprintf_posix=${gl_cv_func_vasprintf_posix=yes}
-gl_cv_func_vdprintf_posix=${gl_cv_func_vdprintf_posix=yes}
-gl_cv_func_vfprintf_posix=${gl_cv_func_vfprintf_posix=yes}
-gl_cv_func_vsnprintf_posix=${gl_cv_func_vsnprintf_posix=yes}
+gl_cv_func_vasprintf_posix=${gl_cv_func_vasprintf_posix=no}
+gl_cv_func_vdprintf_posix=${gl_cv_func_vdprintf_posix=no}
+gl_cv_func_vfprintf_posix=${gl_cv_func_vfprintf_posix=no}
+gl_cv_func_vsnprintf_posix=${gl_cv_func_vsnprintf_posix=no}
-gl_cv_func_vsprintf_posix=${gl_cv_func_vsprintf_posix=yes}
+gl_cv_func_vsprintf_posix=${gl_cv_func_vsprintf_posix=no}

So, all in all, it's more harmless that it looked at first sight. But
it showed 9 bugs in gnulib, which are now all fixed.

Bruno

Re: Fix memleak in getdelim.m4

[Tim Rühsen]

[-- Attachment #1.1: Type: text/plain, Size: 689 bytes --]

Hi Bruno,

On 23.05.20 22:47, Bruno Haible wrote:
> 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.

...

> 
> So, all in all, it's more harmless that it looked at first sight. But
> it showed 9 bugs in gnulib, which are now all fixed.

Thank you so much for investigating and fixing the issues !

Regards, Tim


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

SA_RESETHAND

[Bruno Haible]

Tim Rühsen reports:
> configure:14038: checking for working C stack overflow detection
> SUMMARY: UndefinedBehaviorSanitizer: implicit-integer-sign-change

Here's a reduced test case:

$ cat foo.c
#include <signal.h>

int
main ()
{
  struct sigaction act;
  act.sa_flags = /* SA_NODEFER | SA_ONSTACK | */ SA_RESETHAND;
  return 0;
}

$ clang -Wall foo.c -E | grep sa_flags
    int sa_flags;
  act.sa_flags = 0x80000000;

$ clang -Wall foo.c -fsanitize=implicit-integer-sign-change
$ ./a.out 
foo.c:7:50: runtime error: implicit conversion from type 'unsigned int' of value 2147483648 (32-bit, unsigned) to type 'int' changed the value to -2147483648 (32-bit, signed)

So, glibc defines the 'sa_flags' field as being of type 'int' (like
POSIX [1] mandates). glibc also defines SA_RESETHAND as 0x80000000.
The compiler interprets this constant as being of type 'unsigned int'.
The sanitizer then complains about an implicit conversion from 'unsigned int'
to 'int'.

How to resolve this?
- Should glibc define SA_RESETHAND as ((int)0x80000000) ?
  Then SA_RESETHAND could not be used in preprocessor directives any more.
- Should clang be silent about this case of implicit conversion?
- Should we discourage users from using -fsanitize=implicit-integer-sign-change?

Bruno

[1] https://pubs.opengroup.org/onlinepubs/9699919799/functions/sigaction.html

Re: SA_RESETHAND

[Paul Eggert]

On 5/21/20 8:15 AM, Bruno Haible wrote:

> - Should glibc define SA_RESETHAND as ((int)0x80000000) ?
>    Then SA_RESETHAND could not be used in preprocessor directives any more.

POSIX would allow that, as it doesn't require SA_RESETHAND to be usable 
in preprocessor directives. However, too much software uses it that way 
anyway (e.g., squid/src/tools.cc has "#if SA_RESETHAND == 0 && 
!_SQUID_WINDOWS_"). So I have my doubts whether this change would be 
adopted.

> - Should clang be silent about this case of implicit conversion?

That would solve the problem, although the people who want lots of 
warnings might want one here too.

> - Should we discourage users from using -fsanitize=implicit-integer-sign-change?

For me that flag tends to cause more problem than it cures. So we could 
tell people that Gnulib won't worry about that warning.

Re: SA_RESETHAND

[Bruno Haible]

Paul Eggert wrote:
> > - Should glibc define SA_RESETHAND as ((int)0x80000000) ?
> >    Then SA_RESETHAND could not be used in preprocessor directives any more.
> 
> POSIX would allow that, as it doesn't require SA_RESETHAND to be usable 
> in preprocessor directives. However, too much software uses it that way 
> anyway (e.g., squid/src/tools.cc has "#if SA_RESETHAND == 0 && 
> !_SQUID_WINDOWS_"). So I have my doubts whether this change would be 
> adopted.
> 
> > - Should clang be silent about this case of implicit conversion?
> 
> That would solve the problem, although the people who want lots of 
> warnings might want one here too.
> 
> > - Should we discourage users from using -fsanitize=implicit-integer-sign-change?
> 
> For me that flag tends to cause more problem than it cures. So we could 
> tell people that Gnulib won't worry about that warning.

Thanks for your evaluation. I think this should best be fixed by clang, and
therefore have registered a clang bug:
https://bugs.llvm.org/show_bug.cgi?id=46025

Bruno

Re: Fix sanitizer error in fchownat.m4

[Bruno Haible]

Tim Rühsen wrote:
> configure:55268: checking whether fchownat works with AT_SYMLINK_NOFOLLOW
> SUMMARY: UndefinedBehaviorSanitizer: implicit-integer-sign-change
> conftest.c:812:50 in
> configure:55326: checking whether fchownat works with an empty file name
> SUMMARY: UndefinedBehaviorSanitizer: implicit-integer-sign-change
> conftest.c:817:37 in

This patch should fix it.


2020-05-21  Bruno Haible  <bruno@clisp.org>

	fchownat: Support clang -fsanitize=implicit-integer-sign-change better.
	Reported by Tim Rühsen in
	<https://lists.gnu.org/archive/html/bug-gnulib/2020-05/msg00207.html>.
	* m4/fchownat.m4 (gl_FUNC_FCHOWNAT_DEREF_BUG,
	gl_FUNC_FCHOWNAT_EMPTY_FILENAME_BUG): Cast -1 to uid_t or git_t,
	respectively.

diff --git a/m4/fchownat.m4 b/m4/fchownat.m4
index cd5c301..0a5d637 100644
--- a/m4/fchownat.m4
+++ b/m4/fchownat.m4
@@ -1,4 +1,4 @@
-# fchownat.m4 serial 5
+# fchownat.m4 serial 6
 dnl Copyright (C) 2004-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,
@@ -64,7 +64,7 @@ AC_DEFUN([gl_FUNC_FCHOWNAT_DEREF_BUG],
 int
 main ()
 {
-  return (fchownat (AT_FDCWD, "$gl_dangle", -1, getgid (),
+  return (fchownat (AT_FDCWD, "$gl_dangle", (uid_t)(-1), getgid (),
                     AT_SYMLINK_NOFOLLOW) != 0
           && errno == ENOENT);
 }
@@ -98,7 +98,7 @@ AC_DEFUN([gl_FUNC_FCHOWNAT_EMPTY_FILENAME_BUG],
             fd = open ("conftestdir", O_RDONLY);
             if (fd < 0)
               return 3;
-            ret = fchownat (fd, "", -1, -1, 0);
+            ret = fchownat (fd, "", (uid_t)(-1), (gid_t)(-1), 0);
             close (fd);
             rmdir ("conftestdir");
             return ret == 0;

Fix memleak in glob.m4

[Bruno Haible]

Tim Rühsen wrote:
> configure:67322: checking whether glob lists broken symlinks
> SUMMARY: AddressSanitizer: 37 byte(s) leaked in 2 allocation(s).

This patch should fix it.


2020-05-21  Bruno Haible  <bruno@clisp.org>

	glob: Avoid wrong configure results with "clang -fsanitize=leak".
	Reported by Tim Rühsen in
	<https://lists.gnu.org/archive/html/bug-gnulib/2020-05/msg00207.html>.
	* m4/glob.m4 (gl_GLOB): Free allocated memory before returning.

diff --git a/m4/glob.m4 b/m4/glob.m4
index dbd09e8..d4cd03f 100644
--- a/m4/glob.m4
+++ b/m4/glob.m4
@@ -1,4 +1,4 @@
-# glob.m4 serial 23
+# glob.m4 serial 24
 dnl Copyright (C) 2005-2007, 2009-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,
@@ -47,6 +47,7 @@ char a[_GNU_GLOB_INTERFACE_VERSION == 1 || _GNU_GLOB_INTERFACE_VERSION == 2 ? 1
                   [[glob_t found;
                     if (glob ("conf*-globtest", 0, NULL, &found) == GLOB_NOMATCH)
                       return 1;
+                    globfree (&found);
                   ]])],
                [gl_cv_glob_lists_symlinks=yes],
                [gl_cv_glob_lists_symlinks=no],

Fix memleak in regex.m4

[Bruno Haible]

Tim Rühsen wrote:
> configure:94107: checking for working re_compile_pattern
> SUMMARY: AddressSanitizer: 20072 byte(s) leaked in 116 allocation(s).

This patch should fix it.


2020-05-21  Bruno Haible  <bruno@clisp.org>

	regex: Avoid wrong configure results with "clang -fsanitize=leak".
	Reported by Tim Rühsen in
	<https://lists.gnu.org/archive/html/bug-gnulib/2020-05/msg00207.html>.
	* m4/regex.m4 (gl_REGEX): Free compiled regexes and allocated registers
	before returning with status 0.

diff --git a/m4/regex.m4 b/m4/regex.m4
index 65f5185..e723f59 100644
--- a/m4/regex.m4
+++ b/m4/regex.m4
@@ -1,4 +1,4 @@
-# serial 69
+# serial 70
 
 # Copyright (C) 1996-2001, 2003-2020 Free Software Foundation, Inc.
 #
@@ -90,11 +90,14 @@ AC_DEFUN([gl_REGEX],
                   s = re_compile_pattern (pat, sizeof pat - 1, &regex);
                   if (s)
                     result |= 1;
-                  else if (re_search (&regex, data, sizeof data - 1,
-                                      0, sizeof data - 1, &regs)
-                           != -1)
-                    result |= 1;
-                  regfree (&regex);
+                  else
+                    {
+                      if (re_search (&regex, data, sizeof data - 1,
+                                     0, sizeof data - 1, &regs)
+                          != -1)
+                        result |= 1;
+                      regfree (&regex);
+                    }
                 }
 
                 {
@@ -125,8 +128,8 @@ AC_DEFUN([gl_REGEX],
                                      0, sizeof data - 1, 0);
                       if (i != 0 && i != 21)
                         result |= 1;
+                      regfree (&regex);
                     }
-                  regfree (&regex);
                 }
 
                 if (! setlocale (LC_ALL, "C"))
@@ -139,9 +142,13 @@ AC_DEFUN([gl_REGEX],
             s = re_compile_pattern ("a[^x]b", 6, &regex);
             if (s)
               result |= 2;
-            /* This should fail, but succeeds for glibc-2.5.  */
-            else if (re_search (&regex, "a\nb", 3, 0, 3, &regs) != -1)
-              result |= 2;
+            else
+              {
+                /* This should fail, but succeeds for glibc-2.5.  */
+                if (re_search (&regex, "a\nb", 3, 0, 3, &regs) != -1)
+                  result |= 2;
+                regfree (&regex);
+              }
 
             /* This regular expression is from Spencer ere test number 75
                in grep-2.3.  */
@@ -153,7 +160,10 @@ AC_DEFUN([gl_REGEX],
             s = re_compile_pattern ("a[[:@:>@:]]b\n", 11, &regex);
             /* This should fail with _Invalid character class name_ error.  */
             if (!s)
-              result |= 4;
+              {
+                result |= 4;
+                regfree (&regex);
+              }
 
             /* Ensure that [b-a] is diagnosed as invalid, when
                using RE_NO_EMPTY_RANGES. */
@@ -161,13 +171,18 @@ AC_DEFUN([gl_REGEX],
             memset (&regex, 0, sizeof regex);
             s = re_compile_pattern ("a[b-a]", 6, &regex);
             if (s == 0)
-              result |= 8;
+              {
+                result |= 8;
+                regfree (&regex);
+              }
 
             /* This should succeed, but does not for glibc-2.1.3.  */
             memset (&regex, 0, sizeof regex);
             s = re_compile_pattern ("{1", 2, &regex);
             if (s)
               result |= 8;
+            else
+              regfree (&regex);
 
             /* The following example is derived from a problem report
                against gawk from Jorge Stolfi <stolfi@ic.unicamp.br>.  */
@@ -175,17 +190,35 @@ AC_DEFUN([gl_REGEX],
             s = re_compile_pattern ("[an\371]*n", 7, &regex);
             if (s)
               result |= 8;
-            /* This should match, but does not for glibc-2.2.1.  */
-            else if (re_match (&regex, "an", 2, 0, &regs) != 2)
-              result |= 8;
+            else
+              {
+                /* This should match, but does not for glibc-2.2.1.  */
+                if (re_match (&regex, "an", 2, 0, &regs) != 2)
+                  result |= 8;
+                else
+                  {
+                    free (regs.start);
+                    free (regs.end);
+                  }
+                regfree (&regex);
+              }
 
             memset (&regex, 0, sizeof regex);
             s = re_compile_pattern ("x", 1, &regex);
             if (s)
               result |= 8;
-            /* glibc-2.2.93 does not work with a negative RANGE argument.  */
-            else if (re_search (&regex, "wxy", 3, 2, -2, &regs) != 1)
-              result |= 8;
+            else
+              {
+                /* glibc-2.2.93 does not work with a negative RANGE argument.  */
+                if (re_search (&regex, "wxy", 3, 2, -2, &regs) != 1)
+                  result |= 8;
+                else
+                  {
+                    free (regs.start);
+                    free (regs.end);
+                  }
+                regfree (&regex);
+              }
 
             /* The version of regex.c in older versions of gnulib
                ignored RE_ICASE.  Detect that problem too.  */
@@ -194,8 +227,17 @@ AC_DEFUN([gl_REGEX],
             s = re_compile_pattern ("x", 1, &regex);
             if (s)
               result |= 16;
-            else if (re_search (&regex, "WXY", 3, 0, 3, &regs) < 0)
-              result |= 16;
+            else
+              {
+                if (re_search (&regex, "WXY", 3, 0, 3, &regs) < 0)
+                  result |= 16;
+                else
+                  {
+                    free (regs.start);
+                    free (regs.end);
+                  }
+                regfree (&regex);
+              }
 
             /* Catch a bug reported by Vin Shelton in
                https://lists.gnu.org/r/bug-coreutils/2007-06/msg00089.html
@@ -207,6 +249,8 @@ AC_DEFUN([gl_REGEX],
             s = re_compile_pattern ("[[:alnum:]_-]\\\\+$", 16, &regex);
             if (s)
               result |= 32;
+            else
+              regfree (&regex);
 
             /* REG_STARTEND was added to glibc on 2004-01-15.
                Reject older versions.  */
@@ -221,8 +265,14 @@ AC_DEFUN([gl_REGEX],
             re_set_syntax (RE_SYNTAX_POSIX_EGREP);
             memset (&regex, 0, sizeof regex);
             s = re_compile_pattern ("0|()0|\\1|0", 10, &regex);
-            if (!s || strcmp (s, "Invalid back reference"))
+            if (!s)
               result |= 64;
+            else
+              {
+                if (strcmp (s, "Invalid back reference"))
+                  result |= 64;
+                regfree (&regex);
+              }
 
 #if 0
             /* It would be nice to reject hosts whose regoff_t values are too

Re: Fix memleak in getdelim.m4

[Bruno Haible]

Tim Rühsen wrote:
> configure:36833: checking whether memmem works
> SUMMARY: UndefinedBehaviorSanitizer: invalid-null-argument

This patch should fix it.


2020-05-21  Bruno Haible  <bruno@clisp.org>

	memmem: Avoid wrong configure results with "clang -fsanitize=undefined".
	Reported by Tim Rühsen in
	<https://lists.gnu.org/archive/html/bug-gnulib/2020-05/msg00207.html>.
	* m4/memmem.m4 (gl_FUNC_MEMMEM_SIMPLE): Use NULL + 1, not NULL.

diff --git a/m4/memmem.m4 b/m4/memmem.m4
index e034d7b..35a5bb1 100644
--- a/m4/memmem.m4
+++ b/m4/memmem.m4
@@ -1,4 +1,4 @@
-# memmem.m4 serial 26
+# memmem.m4 serial 27
 dnl Copyright (C) 2002-2004, 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,
@@ -37,7 +37,7 @@ AC_DEFUN([gl_FUNC_MEMMEM_SIMPLE],
     /* Check for empty needle behavior.  */
     {
       const char *haystack = "AAA";
-      if (memmem (haystack, 3, NULL, 0) != haystack)
+      if (memmem (haystack, 3, (const char *) 1, 0) != haystack)
         result |= 2;
     }
     return result;

Re: Fix memleak in getdelim.m4

[Paul Eggert]

On 5/21/20 11:40 AM, Bruno Haible wrote:
> -      if (memmem (haystack, 3, NULL, 0) != haystack)
> +      if (memmem (haystack, 3, (const char *) 1, 0) != haystack)

This has undefined behavior in general, no? So some future sanitizer may 
complain about it, for good reason.

How about using '""' instead of '(const char *) 1'?

Re: Fix memleak in getdelim.m4

[Bruno Haible]

Paul Eggert wrote:
> > -      if (memmem (haystack, 3, NULL, 0) != haystack)
> > +      if (memmem (haystack, 3, (const char *) 1, 0) != haystack)
> 
> This has undefined behavior in general, no?

No. memmem is not supposed to access more than NEEDLELEN bytes at NEEDLE.

> How about using '""' instead of '(const char *) 1'?

That would defeat the purpose of the test, which is to test for the glibc 2.0
bug [1].

Bruno

[1] http://man7.org/linux/man-pages/man3/memmem.3.html

Re: Fix memleak in getdelim.m4

[Bruno Haible]

> The attached patch fixes it. (Please feel free to amend.)

Thanks, I've pushed it in your name.

Bruno