[Bug c/98396] New: gcc wrongly assumes that free preserves errno

[Bug c/98396] New: gcc wrongly assumes that free preserves errno

[bruno at clisp dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98396

            Bug ID: 98396
           Summary: gcc wrongly assumes that free preserves errno
           Product: gcc
           Version: 10.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: bruno at clisp dot org
  Target Milestone: ---

Created attachment 49808
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49808&action=edit
Test case

On POSIX systems, free() can clobber the value of errno. This is implied by
1) https://pubs.opengroup.org/onlinepubs/9699919799/functions/errno.html
which says: "The setting of errno after a successful call to a function is
unspecified unless the description of that function specifies that errno shall
not be modified."
and 2) https://pubs.opengroup.org/onlinepubs/9699919799/functions/free.html
which does not mention errno.

A future version of POSIX will specify that a valid call to free() preserves
errno: https://www.austingroupbugs.net/view.php?id=385 . But this is not yet
standard.

In particular, on Linux/glibc systems, the glibc bug
https://sourceware.org/bugzilla/show_bug.cgi?id=17924 is still open, because
glibc does not guarantee that free() preserves errno — neither through the code
nor through the documentation. This glibc bug even has a test case of a
successful free() that sets errno to ENOMEM (via a call to munmap).

But GCC, when optimizing, eliminates tests of errno or assignments to errno
after 'free (ptr);' where ptr was the result of a malloc(...) call in the same
function.

How to reproduce:
$ gcc -O2 -S foo.c
Inspect the resulting foo.s. You see that
- In function 'check_errno_unmodified', GCC has eliminated the lines
  if (errno != 1789)
    abort ();
- In function 'ensure_errno_unmodified', GCC has eliminated the lines
  int saved_errno = errno;
  and
  errno = saved_errno;
  So, while the programmer knew that free() can clobber errno and added
statements to ensure that errno gets preserved, GCC optimized these statements
away!

[Bug c/98396] gcc wrongly assumes that free preserves errno

[bruno at clisp dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98396

Bruno Haible <bruno at clisp dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |4.0.4, 4.1.2, 4.2.4, 4.3.6,
                   |                            |4.4.7, 4.5.4, 4.6.4, 4.7.3,
                   |                            |4.8.5
      Known to fail|                            |10.2.0, 4.9.4, 5.5.0,
                   |                            |6.5.0, 7.5.0, 8.4.0, 9.3.0
             Target|                            |x86_64-pc-linux-gnu
              Build|                            |x86_64-pc-linux-gnu
               Host|                            |x86_64-pc-linux-gnu

--- Comment #1 from Bruno Haible <bruno at clisp dot org> ---
Related: bug 88576, bug 98070.

[Bug middle-end/98396] gcc wrongly assumes that free preserves errno

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98396

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2021-01-04
          Component|c                           |middle-end
     Ever confirmed|0                           |1
           Keywords|                            |alias, wrong-code
             Status|UNCONFIRMED                 |NEW
                 CC|                            |rguenth at gcc dot gnu.org

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
Confirmed.  Note we have

      /* Freeing memory kills the pointed-to memory.  More importantly
         the call has to serve as a barrier for moving loads and stores
         across it.  */
      case BUILT_IN_STACK_RESTORE:
      case BUILT_IN_FREE:
      case BUILT_IN_GOMP_FREE:
        return ".co ";

and free() should be ".Co " then (same on branches but in tree-ssa-alias.c).
I guess the GOMP free implementation could make sure to preserve errno
if it calls free().

[Bug middle-end/98396] gcc wrongly assumes that free preserves errno

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98396

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
But note that while free() may clobber errno the state after it is undefined
(it's not documented to set it to any specific value).  So I'd argue the
check_errno_unmodified testcase is not really relevant.

A relevant testcase might be

void foo (void *p)
{
  errno = 0;
  free (p);
  errno = 0;
}

where GCC might be tricked into elimiating the late errno = 0 store as
redundant with the earlier (it doesn't do that right now, but I didn't
try too hard but DSE elides the earlier store).

The ensure_errno_unmodified is relevant though.

[Bug middle-end/98396] gcc wrongly assumes that free preserves errno

[bruno at clisp dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98396

--- Comment #4 from Bruno Haible <bruno at clisp dot org> ---
(In reply to Richard Biener from comment #3)
> But note that while free() may clobber errno the state after it is undefined
> (it's not documented to set it to any specific value).  So I'd argue the
> check_errno_unmodified testcase is not really relevant.

The state of errno after calling free() is not "undefined", it is
"unspecified". See the POSIX citation in the description. See also the
definition of "undefined behavior" and "unspecified behavior" in ISO C 2018 §
3.4.3 and § 3.4.4.

The test case 'check_errno_unmodified' is therefore perfectly valid with glibc
versions newer than 2020-12-29 (where
https://sourceware.org/bugzilla/show_bug.cgi?id=17924 is fixed). In these glibc
versions glibc gives the guarantee that free() does not clobber errno.

If you (the GCC people) don't want to make this test case work unconditionally,
you at least need to tell in which way the free() declaration in glibc's
<stdlib.h> needs to be modified, so that the test case will work in these newer
versions of glibc.