Re: free-posix: New module, renamed from 'free'

[Bruno Haible]

Paul Eggert wrote:
> > I'm adding a unit test for the "free() preserves errno" feature.
> 
> Thanks. Unfortunately the test fails on Ubuntu 20.10 (and I assume on other 
> GNU/Linux hosts). So I installed the attached patch to port the free-posix 
> module to GNU/Linux (!).

Thanks; I intended to propose this change and ask you whether it's OK with you.

I noted the test failure on:
  Linux 2.4.22 / glibc 2.3.2 (Fedora 1)
  Linux 4.4 / glibc 2.23 (Ubuntu 16.04)
  Linux 4.18 / glibc 2.28 (CentOS 8)
  Linux 5.8 / glibc 2.32 (Ubuntu 20.10)
as well as
  Linux 5.4.43 / musl libc 1.1.24 (Alpine Linux 3.12)

> Perhaps someday we can move the test to m4/free.m4 with AC_RUN_IFELSE, but 
> there's no rush if the problem is ubiquitous.

I don't think we will ever have a reliable AC_RUN_IFELSE test for this
feature, because to get free() on the failure path, often you need to put
the process into a near-out-of-memory state; on Linux this might kill other
processes, whereas on other OSes it may crash the entire machine.

Such a guarantee can only be given through documentation or by a code
inspection.

Also, I don't trust _POSIX_VERSION to be the right indicator, because we
have seen so often that a system has only some traits of a standard, but
not all. Even the glibc people do this, cf.
https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=c70824b9a4645c0ecd049da8cfdb2c28ae7ada23

Inspecting the various implementations of free() — in the source code or
with strace / truss — , here are my findings:

* OpenBSD is good since version 4.5, since they explicitly save errno.
  See
  
https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/lib/libc/stdlib/malloc.c.diff?r1=1.100&r2=1.101&f=h

* Solaris (10, 11, OpenIndiana) is good, since malloc() only ever invokes
  brk(), not mmap(). Therefore free() does not make system calls.

* In various other BSDs, free() invokes munmap() without protecting errno.

* In glibc, there's also another risky code path: an experimental feature,
  where the first free() in a thread invokes malloc(). It should be possible
  to do a malloc() in the main thread, then exhaust nearly all of the ulimit,
  create a thread, and call free() of the block in this thread.

* Also, in glibc, the user can install their custom free() handler. As long
  as it is not documented that this free() handler must preserve errno, glibc
  free() cannot be considered safe either.


2020-12-19  Bruno Haible  <bruno@clisp.org>

        free-posix: Assume future POSIX compliance only on OpenBSD and Solaris.
        * m4/free.m4 (gl_FUNC_FREE): Guess yes only on OpenBSD and Solaris.
        Don't trust _POSIX_VERSION for this test.

diff --git a/m4/free.m4 b/m4/free.m4
index e7a7203..53df743 100644
--- a/m4/free.m4
+++ b/m4/free.m4
@@ -1,33 +1,43 @@
-# free.m4 serial 3
+# free.m4 serial 4
 # Copyright (C) 2003-2005, 2009-2020 Free Software Foundation, Inc.
 # This file is free software; the Free Software Foundation
 # gives unlimited permission to copy and/or distribute it,
 # with or without modifications, as long as this notice is preserved.
 
-# Written by Paul Eggert.
+# Written by Paul Eggert and Bruno Haible.
 
 AC_DEFUN([gl_FUNC_FREE],
 [
   AC_REQUIRE([gl_STDLIB_H_DEFAULTS])
+  AC_REQUIRE([AC_CANONICAL_HOST])
 
   dnl In the next release of POSIX, free must preserve errno.
   dnl https://www.austingroupbugs.net/view.php?id=385
   dnl https://sourceware.org/bugzilla/show_bug.cgi?id=17924
-  dnl For now, assume implementations other than glibc do not preserve errno
-  dnl unless they set _POSIX_VERSION to the next release number,
-  dnl whatever that happens to be.
+  dnl So far, we know of two platforms that do this:
+  dnl * OpenBSD >= 4.5, thanks to this commit:
+  dnl   
<https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/lib/libc/stdlib/malloc.c.diff?r1=1.100&r2=1.101&f=h>
+  dnl * Solaris, because its malloc() implementation is based on brk(),
+  dnl   not mmap(); hence its free() implementation makes no system calls.
+  dnl For other platforms, you can only be sure if they state it in their
+  dnl documentation, or by code inspection of the free() implementation in 
libc.
   AC_CACHE_CHECK([whether free is known to preserve errno],
     [gl_cv_func_free_preserves_errno],
-    [AC_COMPILE_IFELSE(
-       [AC_LANG_PROGRAM(
-          [[#include <unistd.h>
-          ]],
-          [[#if _POSIX_VERSION <= 200809
-              #error "'free' is not known to preserve errno"
-            #endif
-          ]])],
-       [gl_cv_func_free_preserves_errno=yes],
-       [gl_cv_func_free_preserves_errno=no])
+    [case "$host_os" in
+       # Say yes only if we know it.
+       openbsd* | solaris*)
+         gl_cv_func_free_preserves_errno=yes
+         ;;
+       # It's no on Linux, for implementations that call munmap(), due to
+       # /proc/sys/vm/max_map_count.
+       linux*)
+         gl_cv_func_free_preserves_errno=no
+         ;;
+       # If we don't know, obey --enable-cross-guesses.
+       *)
+         gl_cv_func_free_preserves_errno="$gl_cross_guess_normal"
+         ;;
+     esac
     ])
 
   case $gl_cv_func_free_preserves_errno in