[Bug middle-end/113082] New: builtin transforms do not honor errno

[Bug middle-end/113082] New: builtin transforms do not honor errno

[rguenth at gcc dot gnu.org]

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

            Bug ID: 113082
           Summary: builtin transforms do not honor errno
           Product: gcc
           Version: 13.2.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: rguenth at gcc dot gnu.org
  Target Milestone: ---

This was split out from PR56888.

gcc is changing a program that can rely on errno not being changed to one where
the C library can change it.  (The current C library or any future library that
the resulting binary may be dynamically linked against.)  The library is
allowed to alter errno when not documented otherwise by 7.5p3

Quoting from the PRs last comment:

(In reply to M Welinder from comment #48)
> It's your (1).  gcc is changing a program that can rely on errno not being
> changed to one where the C library can change it.  (The current C library or
> any future library that the resulting binary may be dynamically linked
> against.)

Ick.  Standards continue to surprise me ;)

> Is there any real-world situation that benefits from introducing these
> calls?  It has the feel of optimizing for a benchmark.

People are good in writing inefficient code and replacing say, an open
coded strlen by an actual call to strlen enables followup transforms
that rely on strlen appearing as strlen and not an open-coded variant
(I realize that technically one might find a way to implement that without
actually emitting a call in the end).

And yes, optimizing (repeated) calls of strlen or replacing open-coded
large memcpy by a library call to optimized functions can make a noticable
difference even for non-benchmarks.

We're currently generating calls to memcpy, memmove, memset and strlen.

We are also replacing memmove with memcpy, printf with puts or putc, all
of those transforms are then invalid because of (1) as well.

We are treating -fno-math-errno as applying to non-math functions and
we don't have any -fno-errno or way of analyzing/annotating whether a
program is interested in the state of errno (not only but mainly because
identifying accesses to errno is non-trivial).

[Bug middle-end/113082] builtin transforms do not honor errno

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2023-12-19
             Status|UNCONFIRMED                 |NEW
                 CC|                            |jsm28 at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Joseph - I wonder if the standard folks can be convinced to amend most library
function documentation as to not altering 'errno' (like memcpy, strlen, etc.)?

Should we simply document our constraints on supported library implementations?

[Bug middle-end/113082] builtin transforms do not honor errno

[fw at gcc dot gnu.org]

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

--- Comment #2 from Florian Weimer <fw at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #1)
> Joseph - I wonder if the standard folks can be convinced to amend most
> library function documentation as to not altering 'errno' (like memcpy,
> strlen, etc.)?
> 
> Should we simply document our constraints on supported library
> implementations?

We can add attributes to the glibc headers, similar to the throw and leaf
annotation we have today. It would act as a reminder that if we clobber errno
in these functions due to some implementation detail, we need to save/restore
errno.

[Bug middle-end/113082] builtin transforms do not honor errno

[rguenther at suse dot de]

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

--- Comment #3 from rguenther at suse dot de <rguenther at suse dot de> ---
On Tue, 19 Dec 2023, fw at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113082
> 
> --- Comment #2 from Florian Weimer <fw at gcc dot gnu.org> ---
> (In reply to Richard Biener from comment #1)
> > Joseph - I wonder if the standard folks can be convinced to amend most
> > library function documentation as to not altering 'errno' (like memcpy,
> > strlen, etc.)?
> > 
> > Should we simply document our constraints on supported library
> > implementations?
> 
> We can add attributes to the glibc headers, similar to the throw and leaf
> annotation we have today. It would act as a reminder that if we clobber errno
> in these functions due to some implementation detail, we need to save/restore
> errno.

I guess a new 'noerrno' attribute would make sense.  What I've always
wanted is also a reliable way to distinguish accesses to 'errno'
from other accesses (unfortunately 'errno' is an lvalue so it's
address can be taken).  glibc uses __errno_location (), so an
additional attribute indicating the function returns the address
of 'errno' would be nice to have as well ('errno', just like we
have 'malloc' for malloc results?).  At the moment we can just
use TBAA ('errno' is an lvalue of type 'int') for disambiguation
but 'int' is a bit generic for that to be of much help.

[Bug middle-end/113082] builtin transforms do not honor errno

[amonakov at gcc dot gnu.org]

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

Alexander Monakov <amonakov at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |amonakov at gcc dot gnu.org

--- Comment #4 from Alexander Monakov <amonakov at gcc dot gnu.org> ---
re. comment #3, you'd need to be careful to avoid miscompiling

#include <stdlib.h>

int f(size_t sz, void **out, int *eptr)
{
    int e = *eptr;
    *out = malloc(sz);
    return *eptr - e;
}

to asm that unconditionally returns 0, because that changes the outcome for

  errno = 0;
  f(SIZE_MAX, &ptr, &errno);

IOW, I'm not sure how you can go beyond TBAA since user code can pass around
the address of errno in a plain 'int *' anyway.


re. comment #2, Glibc has

* lazy PLT resolver calling back into the dynamic linker
* LD_AUDIT callbacks
* LD_PROFILE hooks
* IFUNC resolvers

and you'd have to guarantee they won't clobber errno either. For lazy PLT and
LD_PROFILE it is necessary anyway (otherwise it's a Glibc bug), but audit and
ifunc callbacks are provided by the user, not Glibc, and might accidentally
clobber errno.

[Bug middle-end/113082] builtin transforms do not honor errno

[joseph at codesourcery dot com]

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

--- Comment #5 from joseph at codesourcery dot com <joseph at codesourcery dot com> ---
I think it would be reasonable for glibc to require that audit modules 
don't change errno, at least when acting for libc function calls where 
glibc guarantees not changing errno.  I think user-provided IFUNC 
resolvers are only relevant for user-provided functions and so shouldn't 
be relevant to this issue (if a user declares their own function with a 
noerrno attribute, and also has an IFUNC resolver for that function, they 
need to make sure the IFUNC resolver behaves consistently with the 
attribute).

It would also seem reasonable for glibc to guarantee that most string and 
memory functions (maybe excluding a few that involve the locale or other 
external state, such as strcoll or strerror, and definitely excluding 
those such as strdup that involve memory allocation) don't change errno.  
We may need to be careful about what "obviously" shouldn't affect errno 
(consider e.g. the ongoing discussions around qsort - where avoiding 
memory allocation should as a side effect also avoid affecting errno, but 
it's unclear how we might simultaneously avoid memory allocation, keep a 
stable sort, achieve O(n log(n)) worst case performance, and keep good 
performance for typical inputs).