| Summary: | -fno-math-errno causes GCC to consider that malloc does not set errno | ||
|---|---|---|---|
| Product: | gcc | Reporter: | Aurelien Jarno <aurelien> |
| Component: | middle-end | Assignee: | Richard Biener <rguenth> |
| Status: | ASSIGNED --- | ||
| Severity: | normal | CC: | bugdal, dimhen, dushistov, egallager, fw, gabravier, rguenther, sjames, zack+srcbugz |
| Priority: | P3 | Keywords: | wrong-code |
| Version: | 9.0 | ||
| Target Milestone: | --- | ||
| See Also: |
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64101 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=42944 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98070 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98396 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113082 |
||
| Host: | Target: | ||
| Build: | Known to work: | ||
| Known to fail: | Last reconfirmed: | 2018-12-22 00:00:00 | |
| Attachments: | patch | ||
IIRC malloc setting errno also non standard. (In reply to Andrew Pinski from comment #1) > IIRC malloc setting errno also non standard. According to POSIX: The malloc() function shall fail if: [ENOMEM] [CX] [Option Start] Insufficient storage space is available. [Option End] In that case malloc returns a NULL pointer. However even after adding a check for the return value, GCC still optimizes-out saving and restoring errno. (In reply to Aurelien Jarno from comment #2) > (In reply to Andrew Pinski from comment #1) > > IIRC malloc setting errno also non standard. > > According to POSIX: > > The malloc() function shall fail if: > > [ENOMEM] > [CX] [Option Start] Insufficient storage space is available. [Option > End] > > In that case malloc returns a NULL pointer. However even after adding a > check for the return value, GCC still optimizes-out saving and restoring > errno. POSIX says one thing but C99 says another thing. The C standard doesn't say malloc _will_ set errno on failure, but it also doesn't say it _won't_, and all library functions are allowed to clobber the value of errno unless it is specifically documented that they won't (N1570 7.5 [Errors], para 3, last sentence). In any case, an option named -fno-math-errno has no business affecting the treatment of functions that have nothing to do with mathematics. Has this got to do anything with errno? It seems to me that with -fno-math-errno, GCC assumes that malloc does not set *any* TLS variable. That doesn't look right to me. (In reply to Zack Weinberg from comment #5) > The C standard doesn't say malloc _will_ set errno on failure, but it also Well at least POSIX says: Otherwise, it shall return a null pointer and set errno to indicate the error. > doesn't say it _won't_, and all library functions are allowed to clobber the > value of errno unless it is specifically documented that they won't (N1570 > 7.5 [Errors], para 3, last sentence). I fully agree with that. (In reply to Aurelien Jarno from comment #7) > (In reply to Zack Weinberg from comment #5) > > The C standard doesn't say malloc _will_ set errno on failure, but it also > > Well at least POSIX says: > Otherwise, it shall return a null pointer and set errno to indicate the > error. Yeah, I wasn't denying that, I was responding to Andrew taking the attitude that this was fine because ISO C proper _didn't_ say that. I dug into the code a little and it looks like this was an intentional re-use of -fno-math-errno to also mean "and neither will malloc", in the patch for PR 42944. I think that's wrong, but perhaps Richard Biener would like to argue otherwise... ... whoops, hit send a little too early. AFAICT, the relevant code is call_may_clobber_ref_p_1 in tree-ssa-alias.c; I would say that the uses of flag_errno_math under the cases BUILT_IN_MALLOC, ALIGNED_ALLOC, CALLOC, STRDUP, STRNDUP, POSIX_MEMALIGN, REALLOC in that function are all wrong, and GCC should unconditionally assume errno may be clobbered by those builtins. If this behavior is felt to be valuable, it should get its own -f switch. (The uses of flag_errno_math under BUILT_IN_GAMMA*, LGAMMA*, and REMQUO* are appropriate, though, as those _are_ math functions.) Read the thread starting at: https://gcc.gnu.org/ml/gcc-patches/2018-10/msg00232.html (In reply to Florian Weimer from comment #6) > Has this got to do anything with errno? It seems to me that with > -fno-math-errno, GCC assumes that malloc does not set *any* TLS variable. > That doesn't look right to me. GCC assumes that memory allocation does not affect global heap state. That's correct unless you compile the memory allocator itself. Note the original "alias" with -fno-math-errno was probably too lazy but allocations being a barrier for any load/store of global memory is quite bad for optimization in _fortran_ (which has no business with errno) where we (unfortunately) also get everything passed by reference. I'll introduce -f[no-]alloc-errno, defaulted to the -f[no-]math-errno state and enabled at -Ofast (but not with -ffast-math). Created attachment 45315 [details]
patch
What I have sofar - this doesn't work for Fortran behaving differently by default. I think a proper fix would be to refactor the existing
_set_by_frontend stuff triggered via SetByCombined into a frontend specific
default_options_table (lang_hooks.default_options_table I guess) which
gets merged with the global default_options_table, overriding any settings
there.
Note there's a related bug complaining about -fno-math-errno not doing what
is documented. That probably changed because of fortran as well.
(In reply to Richard Biener from comment #11) > (In reply to Florian Weimer from comment #6) > > Has this got to do anything with errno? It seems to me that with > > -fno-math-errno, GCC assumes that malloc does not set *any* TLS variable. > > That doesn't look right to me. > > GCC assumes that memory allocation does not affect global heap state. > That's correct unless you compile the memory allocator itself. > > Note the original "alias" with -fno-math-errno was probably too lazy but > allocations being a barrier for any load/store of global memory is quite > bad for optimization in _fortran_ (which has no business with errno) > where we (unfortunately) also get everything passed by reference. > > I'll introduce -f[no-]alloc-errno, defaulted to the -f[no-]math-errno > state and enabled at -Ofast (but not with -ffast-math). Darwin has -fno-math-errno set by default, but its malloc can set errno (to ENOMEM), so it wouldn't make sense to have -fno-alloc-errno set by -fno-math-errno on Darwin I don't see why it would _ever_ make sense for -fno-alloc-errno to default to the setting of -fno-math-errno. The math functions and the memory allocation functions are independent components of the C library. Each toggle's default should be settable independently by the target configuration. (In reply to Zack Weinberg from comment #14) > I don't see why it would _ever_ make sense for -fno-alloc-errno to default > to the setting of -fno-math-errno. The math functions and the memory > allocation functions are independent components of the C library. Each > toggle's default should be settable independently by the target > configuration. true, good point |
With the -fno-math-errno option, GCC optimizes-out saving and restoring errno around a malloc call. Here is a testcase, derived from the GNU libc string/strerror.c, to reproduce it: typedef long unsigned int size_t; extern char *__strerror_r (int __errnum, char *__buf, size_t __buflen); extern void *malloc (size_t __size) __attribute__ ((__nothrow__)) __attribute__ ((__malloc__)); extern __thread int __libc_errno __attribute__ ((tls_model ("initial-exec"))); static char *buf; char *strerror (int errnum) { int saved_errno; saved_errno = __libc_errno; buf = malloc (1024); (__libc_errno = (saved_errno)); return __strerror_r (errnum, buf, 1024); } Compile with: gcc -fmath-errno -Wall -O2 -fPIC -S -c strerror.i Without -fno-math-errno, we can see in the output assembly code that errno is saved around the malloc call: strerror: .LFB0: .cfi_startproc pushq %r12 .cfi_def_cfa_offset 16 .cfi_offset 12, -16 pushq %rbp .cfi_def_cfa_offset 24 .cfi_offset 6, -24 movl %edi, %ebp movl $1024, %edi pushq %rbx .cfi_def_cfa_offset 32 .cfi_offset 3, -32 movq __libc_errno@gottpoff(%rip), %rbx movl %fs:(%rbx), %r12d call malloc@PLT movl %ebp, %edi movl $1024, %edx movl %r12d, %fs:(%rbx) movq %rax, %rsi popq %rbx .cfi_def_cfa_offset 24 popq %rbp .cfi_def_cfa_offset 16 popq %r12 .cfi_def_cfa_offset 8 jmp __strerror_r@PLT .cfi_endproc With -fno-math-errno, saving and restoring errno is optimized out: strerror: .LFB0: .cfi_startproc pushq %rbx .cfi_def_cfa_offset 16 .cfi_offset 3, -16 movl %edi, %ebx movl $1024, %edi call malloc@PLT movl %ebx, %edi movl $1024, %edx popq %rbx .cfi_def_cfa_offset 8 movq %rax, %rsi jmp __strerror_r@PLT .cfi_endproc This is reproducible with 6.5, 7.4, 8.2 and a snapshot of trunk from 2018-12-17.