Re: test-pthread-rwlock failure on Pop!_OS 22.04 LTS

[Paul Eggert]

On 6/29/24 02:48, Bruno Haible wrote:

   1. Install a toolchain for creating binaries linked against musl-libc:
  $ sudo apt install musl-gcc
   2. Create a testdir for the module 'pthread-rwlock'.
   3. Build it with CC="gcc". You should still see the test failure after
  10 minutes.
   4. Build it with CC="musl-gcc". Run
  $ time gltests/test-pthread-rwlock

If 4. takes more than 30 seconds, your OS is probably running on top of
a hypervisor, and that is causing the problem.

If 4. takes just a few seconds, it looks like a glibc bug.


4. takes just a few seconds on my platform.

Re: [PATCH] tests: switch nap() to use file creation to gauge delay

[Jeff Layton]

On Sat, 2024-06-29 at 03:33 +0200, Bruno Haible wrote:
> Jeff Layton wrote:
> > Failure of the test-stat-time test is what triggered us to revert the
> > multigrain timestamp series from the Linux kernel last October.  With
> > that failure, we'd sometimes see timestamps showing files being modified
> > in reverse order (if one got a fine-grained and another got a
> > coarse-grained timestamp).
> 
> I remember. Thanks for having listened to Jan Kara, Paul, me, and others.
> 
> > Switch this code over to use file creation of two files to gauge the
> > delay. Since that is operating on two different files, it should still
> > be possible to gauge the coarse-grained timer tick from that.
> 
> So, if I understand it right, there will now be two concepts of "file
> system time stamp resolution":
>   a. regarding writes in a file,
>   b. regarding file creation,
> and   a <= b. Your patch to make nap() return b, not a, seems reasonable.
> 
> Let's apply it when the copyright assignment process is complete.
> 

It's a little more subtle than that:

When watching for changes on a single file (via stat() and the like),
we ensure that timestamps are distinct so that ctime changes are
visible to userland.

When a file timestamps are not being observed, then we use the
traditional coarse-grained timestamps that you're used to, but that's
now subject to a floor value that represents that latest fine-grained
stamp handed out.

That's why creating two files and checking their timestamps afterward
is still a viable method to gauge the coarse-grained timer tick length.
These are new files that have never been observed before, so they end
up being stamped with the current coarse grained clock value.
-- 
Jeff Layton 

Re: a pthread_cond_timedwait bug

[Bruno Haible]

I wrote:
> I turned to the VirtualBox settings, more precisely to the 
> "Paravirtualization"
> acceleration setting. Result:
>   Default, KVM   -> test fails occasionally (about 1 in 3 
> times)
>   None, Legacy, Minimal, Hyper-V -> test succeeds (no failure in 20 runs)

The test-pthread-cond, test-cond, test-cnd tests are not the only ones that
fail in this VM configuration. test-pthread_sigmask[12] and test-sigprocmask
occasionally fail as well. Both in glibc and in musl libc systems.

This patch avoids the test failures.


2024-06-29  Bruno Haible  

tests: Avoid test failures due to VirtualBox specific bug.
* tests/virtualbox.h: New file.
* tests/test-pthread-cond.c: Include virtualbox.h.
(main): Skip the test under VirtualBox with KVM paravirtualization and
more than 1 CPU.
* tests/test-cnd.c: Include virtualbox.h.
(main): Skip the test under VirtualBox with KVM paravirtualization and
more than 1 CPU.
* tests/test-cond.c: Include virtualbox.h.
(main): Skip the test under VirtualBox with KVM paravirtualization and
more than 1 CPU.
* tests/test-pthread_sigmask1.c: Include virtualbox.h.
(main): Skip the test under VirtualBox with KVM paravirtualization and
more than 1 CPU.
* tests/test-pthread_sigmask2.c: Include virtualbox.h.
(main): Skip the test under VirtualBox with KVM paravirtualization and
more than 1 CPU.
* tests/test-sigprocmask.c: Include virtualbox.h.
(main): Skip the test under VirtualBox with KVM paravirtualization and
more than 1 CPU.
* modules/pthread-cond-tests (Files): Add tests/virtualbox.h.
* modules/cnd-tests (Files): Likewise.
* modules/cond-tests (Files): Likewise.
* modules/pthread_sigmask-tests (Files): Likewise.
* modules/sigprocmask-tests (Files): Likewise.

diff --git a/modules/cnd-tests b/modules/cnd-tests
index f9dc79ea7d..be3c9de0a6 100644
--- a/modules/cnd-tests
+++ b/modules/cnd-tests
@@ -1,5 +1,6 @@
 Files:
 tests/test-cnd.c
+tests/virtualbox.h
 tests/macros.h
 
 Depends-on:
diff --git a/modules/cond-tests b/modules/cond-tests
index 6b4c45e2c0..d984ca84b1 100644
--- a/modules/cond-tests
+++ b/modules/cond-tests
@@ -1,5 +1,6 @@
 Files:
 tests/test-cond.c
+tests/virtualbox.h
 
 Depends-on:
 lock
diff --git a/modules/pthread-cond-tests b/modules/pthread-cond-tests
index fc0a1c8e06..baafb48993 100644
--- a/modules/pthread-cond-tests
+++ b/modules/pthread-cond-tests
@@ -1,5 +1,6 @@
 Files:
 tests/test-pthread-cond.c
+tests/virtualbox.h
 tests/macros.h
 
 Depends-on:
diff --git a/modules/pthread_sigmask-tests b/modules/pthread_sigmask-tests
index 00185a0d3e..508c02a16f 100644
--- a/modules/pthread_sigmask-tests
+++ b/modules/pthread_sigmask-tests
@@ -2,6 +2,7 @@ Files:
 tests/test-pthread_sigmask1.c
 tests/test-pthread_sigmask2.c
 tests/signature.h
+tests/virtualbox.h
 tests/macros.h
 
 Depends-on:
diff --git a/modules/sigprocmask-tests b/modules/sigprocmask-tests
index 8e6523e785..7880b64b9b 100644
--- a/modules/sigprocmask-tests
+++ b/modules/sigprocmask-tests
@@ -1,6 +1,7 @@
 Files:
 tests/test-sigprocmask.c
 tests/signature.h
+tests/virtualbox.h
 tests/macros.h
 
 Depends-on:
diff --git a/tests/test-cnd.c b/tests/test-cnd.c
index b8906a15c0..bcd0e7786c 100644
--- a/tests/test-cnd.c
+++ b/tests/test-cnd.c
@@ -41,6 +41,7 @@
 # include 
 #endif
 
+#include "virtualbox.h"
 #include "macros.h"
 
 #if ENABLE_DEBUGGING
@@ -230,6 +231,16 @@ test_cnd_timedwait (void)
 int
 main ()
 {
+  /* This test occasionally fails on Linux (glibc or musl libc), in a
+ VirtualBox VM with paravirtualization = Default or KVM, with ≥ 2 CPUs.
+ Skip the test in this situation.  */
+  if (is_running_under_virtualbox_kvm () && num_cpus () > 1)
+{
+  fputs ("Skipping test: avoiding VirtualBox bug with KVM 
paravirtualization\n",
+ stderr);
+  return 77;
+}
+
 #if HAVE_DECL_ALARM
   /* Declare failure if test takes too long, by using default abort
  caused by SIGALRM.  */
diff --git a/tests/test-cond.c b/tests/test-cond.c
index 712220aaac..113dd392f6 100644
--- a/tests/test-cond.c
+++ b/tests/test-cond.c
@@ -43,6 +43,8 @@
 #include "glthread/thread.h"
 #include "glthread/yield.h"
 
+#include "virtualbox.h"
+
 #if ENABLE_DEBUGGING
 # define dbgprintf printf
 #else
@@ -219,6 +221,16 @@ test_timedcond (void)
 int
 main ()
 {
+  /* This test occasionally fails on Linux (glibc or musl libc), in a
+ VirtualBox VM with paravirtualization = Default or KVM, with ≥ 2 CPUs.
+ Skip the test in this situation.  */
+  if (is_running_under_virtualbox_kvm () && num_cpus () > 1)
+{
+  fputs ("Skipping test: avoiding VirtualBox bug with KVM 
paravirtualization\n",
+ stderr);
+  return 77;
+}
+
 #if DO_TEST_COND
   printf ("Starting test_cond ..."); fflush (stdout);
   {
diff --git a/tests/test-pthread-cond.c b/tests/test-pthread-cond.

Re: virt-manager vs. VirtualBox

[Bruno Haible]

Hi Collin,

> but you might like 'virt-manager' for setting up
> KVM virtual machines [1]. I can never remember all the correct flags for
> qemu. But 'virt-manager' has an easy UI and configuration similar to
> VirtualBox.

Thanks for the suggestion.

While VirtualBox has its share of bugs:
  - Unable to install OpenBSD 6.[6-9] as a guest,
  - Occasionally does a mouse grab when switching window focus while
the host machine is under high load,
  - Rarely fails to boot some Linux VM, need to try a second time.
QEMU-based emulation also has a problem:
  - The mouse pointer flickers and is nearly invisible while I'm moving
the mouse.

Since VirtualBox VMs and QEMU/KVM based VMs cannot be run at the same time
(the kernel prevents this), I effectively have to choose between one and
the other.

Bruno

rename *z*s*printf functions

[Bruno Haible]

The naming of functions that I proposed in

is inconsistent: The 'z' letter comes before the 's' for the string
functions but after the 'd', 'f' for the file descriptor or file stream
based functions.

Recall that the 'z' means essentially the same thing in all cases
(because ptrdiff_t and off64_t are the same thing on 64-bit platforms).

I like to keep 'd' and 'f' (and 'v', of course) as prefixes. So, to have
consistency, better rename
  *zsprintf -> *szprintf
  *zsnprintf -> *snzprintf

Done as follows.


2024-06-29  Bruno Haible  

Global rename *zs[n]printf -> *s[n]zprintf.
* lib/aszprintf.c: Renamed from lib/azsprintf.c.
* lib/c-aszprintf.c: Renamed from lib/c-azsprintf.c.
* lib/c-vaszprintf.c: Renamed from lib/c-vazsprintf.c.
* lib/c-vsnzprintf.c: Renamed from lib/c-vzsnprintf.c.
* lib/c-snzprintf.c: Renamed from lib/c-zsnprintf.c.
* lib/vaszprintf.c: Renamed from lib/vazsprintf.c.
* lib/vsnzprintf.c: Renamed from lib/vzsnprintf.c.
* lib/vszprintf.c: Renamed from lib/vzsprintf.c.
* lib/snzprintf.c: Renamed from lib/zsnprintf.c.
* lib/szprintf.c: Renamed from lib/zsprintf.c.
* tests/test-c-vaszprintf-gnu.c: Renamed from
tests/test-c-vazsprintf-gnu.c.
* tests/test-c-vaszprintf-gnu.sh: Renamed from
tests/test-c-vazsprintf-gnu.sh.
* tests/test-c-vsnzprintf-gnu.c: Renamed from
tests/test-c-vzsnprintf-gnu.c.
* tests/test-c-vsnzprintf-gnu.sh: Renamed from
tests/test-c-vzsnprintf-gnu.sh.
* tests/test-c-snzprintf-gnu.c: Renamed from
tests/test-c-zsnprintf-gnu.c.
* tests/test-c-snzprintf-gnu.sh: Renamed from
tests/test-c-zsnprintf-gnu.sh.
* tests/test-vaszprintf-gnu.c: Renamed from tests/test-vazsprintf-gnu.c.
* tests/test-vaszprintf-gnu.h: Renamed from tests/test-vazsprintf-gnu.h.
* tests/test-vaszprintf-posix.c: Renamed from
tests/test-vazsprintf-posix.c.
* tests/test-vsnzprintf-gnu.c: Renamed from tests/test-vzsnprintf-gnu.c.
* tests/test-vsnzprintf-posix.c: Renamed from
tests/test-vzsnprintf-posix.c.
* tests/test-vszprintf-gnu.c: Renamed from tests/test-vzsprintf-gnu.c.
* tests/test-vszprintf-posix.c: Renamed from
tests/test-vzsprintf-posix.c.
* tests/test-snzprintf-gnu.c: Renamed from tests/test-zsnprintf-gnu.c.
* tests/test-snzprintf-gnu.h: Renamed from tests/test-zsnprintf-gnu.h.
* tests/test-snzprintf-posix.c: Renamed from
tests/test-zsnprintf-posix.c.
* tests/test-snzprintf-posix.h: Renamed from
tests/test-zsnprintf-posix.h.
* tests/test-szprintf-gnu.c: Renamed from tests/test-zsprintf-gnu.c.
* tests/test-szprintf-gnu.h: Renamed from tests/test-zsprintf-gnu.h.
* tests/test-szprintf-posix.c: Renamed from tests/test-zsprintf-posix.c.
* tests/test-szprintf-posix.h: Renamed from tests/test-zsprintf-posix.h.
* modules/c-vaszprintf: Renamed from modules/c-vazsprintf.
* modules/c-vaszprintf-gnu: Renamed from modules/c-vazsprintf-gnu.
* modules/c-vaszprintf-gnu-tests: Renamed from
modules/c-vazsprintf-gnu-tests.
* modules/c-vsnzprintf: Renamed from modules/c-vzsnprintf.
* modules/c-vsnzprintf-gnu: Renamed from modules/c-vzsnprintf-gnu.
* modules/c-vsnzprintf-gnu-tests: Renamed from
modules/c-vzsnprintf-gnu-tests.
* modules/c-snzprintf: Renamed from modules/c-zsnprintf.
* modules/c-snzprintf-gnu: Renamed from modules/c-zsnprintf-gnu.
* modules/c-snzprintf-gnu-tests: Renamed from
modules/c-zsnprintf-gnu-tests.
* modules/vaszprintf: Renamed from modules/vazsprintf.
* modules/vaszprintf-gnu: Renamed from modules/vazsprintf-gnu.
* modules/vaszprintf-gnu-tests: Renamed from
modules/vazsprintf-gnu-tests.
* modules/vaszprintf-posix: Renamed from modules/vazsprintf-posix.
* modules/vaszprintf-posix-tests: Renamed from
modules/vazsprintf-posix-tests.
* modules/vsnzprintf: Renamed from modules/vzsnprintf.
* modules/vsnzprintf-gnu: Renamed from modules/vzsnprintf-gnu.
* modules/vsnzprintf-gnu-tests: Renamed from
modules/vzsnprintf-gnu-tests.
* modules/vsnzprintf-posix: Renamed from modules/vzsnprintf-posix.
* modules/vsnzprintf-posix-tests: Renamed from
modules/vzsnprintf-posix-tests.
* modules/vszprintf: Renamed from modules/vzsprintf.
* modules/vszprintf-gnu: Renamed from modules/vzsprintf-gnu.
* modules/vszprintf-gnu-tests: Renamed from modules/vzsprintf-gnu-tests.
* modules/vszprintf-posix: Renamed from modules/vzsprintf-posix.
* modules/vszprintf-posix-tests: Renamed from
modules/vzsprintf-posix-tests.
* modules/snzprintf: Renamed from modules/zsnprint

Re: rename *z*s*printf functions

[Paul Eggert]

While we're on the subject of the *z*printf API, I was surprised to see 
the obstack-related functions in stdio.h. Shouldn't they be in obstack.h?

Re: bootstrap-funclib.sh: shrink or split too-long lines

[Bruno Haible]

Hi Jim,

>  # The subdirectory 'gnulib' does not yet exist. Clone into it.
>  echo "$0: getting gnulib files..."
>  trap cleanup_gnulib HUP INT PIPE TERM
> +gnulib_url=${GNULIB_URL:-$default_gnulib_url}
>  shallow=
>  if test -z "$GNULIB_REVISION"; then

This variable is useful; thanks.

>  || cleanup_gnulib
>git -C "$gnulib_path" reset --hard FETCH_HEAD
> -  (cd "$gnulib_path" && git checkout "$GNULIB_REVISION") || 
> cleanup_gnulib
> +  (cd "$gnulib_path" && git checkout 
> "$GNULIB_REVISION")||cleanup_gnulib
>  fi
>  trap - HUP INT PIPE TERM

This one is not useful. The spaces help readability, and therefore help
maintainability. I am therefore fixing it up:


2024-06-29  Bruno Haible  

bootstrap-funclib.sh: Restore readability.
* top/bootstrap-funclib.sh (prepare_GNULIB_SRCDIR): Add a line break
instead of removing spaces.
* build-aux/bootstrap: Regenerated.

diff --git a/top/bootstrap-funclib.sh b/top/bootstrap-funclib.sh
index 87f804eb16..5005d9b4a9 100644
--- a/top/bootstrap-funclib.sh
+++ b/top/bootstrap-funclib.sh
@@ -1,6 +1,6 @@
 # A library of shell functions for autopull.sh, autogen.sh, and bootstrap.
 
-scriptlibversion=2024-04-28.09; # UTC
+scriptlibversion=2024-06-29.23; # UTC
 
 # Copyright (C) 2003-2024 Free Software Foundation, Inc.
 #
@@ -515,7 +515,7 @@ prepare_GNULIB_SRCDIR ()
   if git clone -h 2>&1 | grep -- --depth > /dev/null; then
 shallow='--depth 2'
   fi
-  git clone $shallow "$gnulib_url" "$gnulib_path"\
+  git clone $shallow "$gnulib_url" "$gnulib_path" \
 || cleanup_gnulib
 else
   if git fetch -h 2>&1 | grep -- --depth > /dev/null; then
@@ -536,7 +536,8 @@ prepare_GNULIB_SRCDIR ()
 || git -C "$gnulib_path" fetch origin \
 || cleanup_gnulib
   git -C "$gnulib_path" reset --hard FETCH_HEAD
-  (cd "$gnulib_path" && git checkout 
"$GNULIB_REVISION")||cleanup_gnulib
+  (cd "$gnulib_path" && git checkout "$GNULIB_REVISION") \
+|| cleanup_gnulib
 fi
 trap - HUP INT PIPE TERM
   else

Re: obstack_printf

[Bruno Haible]

Paul Eggert wrote:
> While we're on the subject of the *z*printf API, I was surprised to see 
> the obstack-related functions in stdio.h. Shouldn't they be in obstack.h?

glibc has obstack_printf and obstack_vprintf in /usr/include/stdio.h.

Gnulib's obstack* modules are intended to mimic glibc, mutatis mutandis.

Bruno

*zprintf as overrides

[Bruno Haible]

Hi Paul,

In  you
suggested:

> I'm tempted to use "#define printf zprintf" and leave most of the source
> code alone. Perhaps there should be a Gnulib option for that.

We can do this through one or more gnulib modules.

What I'm wondering is whether it's best to have

* one module per function, i.e.
  - one module for sprintf -> szprintf,
  - one module for snprintf -> snzprintf,
  - one module for vsprintf -> vszprintf,
  - one module for vsnprintf -> vsnzprintf,
  - one module for dprintf -> dzprintf,
  - one module for vdprintf -> vdzprintf,
  - one module for fprintf -> fzprintf,
  - one module for vfprintf -> vfzprintf,
  etc.

* or one module per function group, i.e.
  - one module for *s*printf -> *s*zprintf,
  - one module for *d*printf -> *d*zprintf,
  - one module for *f*printf -> *f*zprintf,

* or one module for all of them together.

And in the first case, what to do if the package requests redirection
of, say, sprintf but then uses vsprintf without redirection? Emit a
warning "vsprintf used but is not 64-bit clean, use vszprintf instead"?

What do you think?

  Bruno

Re: *zprintf as overrides

[Paul Eggert]

On 6/30/24 01:27, Bruno Haible wrote:

* or one module for all of them together.


That might be simplest. But first let's see where it would really be useful.

Re: bootstrap-funclib.sh: shrink or split too-long lines

[Collin Funk]

Bruno Haible  writes:

>> +gnulib_url=${GNULIB_URL:-$default_gnulib_url}
>>  shallow=
>>  if test -z "$GNULIB_REVISION"; then
>
> This variable is useful; thanks.

This would allow gnulib_url to be set in bootstrap.conf so a mirror
could be used right?

Even before the recent Savannah outages apparently a full clone of
Gnulib can cause trouble on CI machines [1] [2].

Maybe it makes sense to document safe mirrors, like the Coreutils GitHub
group [3]. I'm not sure if there is an issue with that since the website
requires nonfree javascript.

Collin

[1] 
https://lists.gnu.org/archive/html/savannah-hackers-public/2024-05/msg0.html
[2] https://octave.discourse.group/t/buildbot-issues-with-gnulib-repository/5395
[3] https://github.com/coreutils/gnulib

Re: GNULIB_URL and mirrors

[Bruno Haible]

Hi Collin,

> >> +gnulib_url=${GNULIB_URL:-$default_gnulib_url}
> >>  shallow=
> >>  if test -z "$GNULIB_REVISION"; then
> >
> > This variable is useful; thanks.
> 
> This would allow gnulib_url to be set in bootstrap.conf so a mirror
> could be used right?

No, setting gnulib_url would have no effect, since what you have here
is a shell assignment, not a Makefile default.

But GNULIB_URL is documented as being settable as an environment
variable (see `bootstrap --help`).

> Even before the recent Savannah outages apparently a full clone of
> Gnulib can cause trouble on CI machines [1] [2].
> 
> Maybe it makes sense to document safe mirrors, like the Coreutils GitHub
> group [3].

That's an interesting idea. It would take off some load from savannah.
But it would advertise GitHub which, unlike savannah, is powered by
proprietary software. Not sure which is better...

Bruno

Re: Integer overflows in memchr

[Po Lu]

Po Lu  writes:

> Btw, doesn't this also mean that lib/strnlen.c (in Emacs, at least) is
> incorrect?
>
> size_t
> strnlen (const char *string, size_t maxlen)
> {
>   const char *end = memchr (string, '\0', maxlen);
>   return end ? (size_t) (end - string) : maxlen;
> }
>
> is virtually identical to Android's implementation and equally liable to
> overflow in memchr with excessive falues of MAXLEN.

Ping!

Re: bootstrap-funclib.sh: shrink or split too-long lines

[Jim Meyering]

On Sat, Jun 29, 2024 at 5:11 PM Bruno Haible  wrote:
> 2024-06-29  Bruno Haible  
>
> bootstrap-funclib.sh: Restore readability.
> * top/bootstrap-funclib.sh (prepare_GNULIB_SRCDIR): Add a line break
> instead of removing spaces.
> * build-aux/bootstrap: Regenerated.

Thanks. That does look better.

Re: a pthread_cond_timedwait bug

[Bruno Haible]

>   * tests/virtualbox.h: New file.

This produces compiler warnings on non-Linux platforms. Fixed through:


2024-06-30  Bruno Haible  

tests: Avoid warning "is_running_under_virtualbox defined but not used".
* tests/virtualbox.h (is_running_under_virtualbox): Mark as possibly
unused.

diff --git a/tests/virtualbox.h b/tests/virtualbox.h
index 86a47fa98d..2cc970012f 100644
--- a/tests/virtualbox.h
+++ b/tests/virtualbox.h
@@ -24,7 +24,7 @@
 
 /* This function determines whether the current system is Linux and running
under the VirtualBox emulator.  */
-static bool
+_GL_ATTRIBUTE_MAYBE_UNUSED static bool
 is_running_under_virtualbox (void)
 {
 #ifdef __linux__