This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [patch] For BZ #17328, mark __errno_location with __attribute__((returns_nonnull)) for gcc >=4.9.0
- From: Zack Weinberg <zackw at panix dot com>
- To: Paul Pluzhnikov <ppluzhnikov at gmail dot com>
- Cc: Rich Felker <dalias at libc dot org>, GLIBC Devel <libc-alpha at sourceware dot org>
- Date: Mon, 2 Mar 2015 17:23:58 -0500
- Subject: Re: [patch] For BZ #17328, mark __errno_location with __attribute__((returns_nonnull)) for gcc >=4.9.0
- Authentication-results: sourceware.org; auth=none
- References: <CALoOobOuAEpw+zxRrrDyHB7UVbAZMzreXqpujzZOWNLS7+aRUA at mail dot gmail dot com> <20150301011753 dot GV23507 at brightrain dot aerifal dot cx> <CAPC3xapQBMH+DJdup2Y8_tt6xdcFAnQLB_K8VpT3ouCavvzXXA at mail dot gmail dot com>
On Sat, Feb 28, 2015 at 8:54 PM, Paul Pluzhnikov <ppluzhnikov@gmail.com> wrote:
> On Sat, Feb 28, 2015 at 5:17 PM, Rich Felker <dalias@libc.org> wrote:
>>
>> No objection, but I'm curious if there's any practical benefit of
>> this. I can't think of many situations where knowledge that
>> &errno!=NULL would assist the compiler in optimizing or diagnostics.
>
> Hmm, maybe you are right.
>
> I was reading https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58689 and
> threads linked from it, and it really looks like not much is gained
> inside (or outside) GLIBC -- it's not like anyone ever checks whether
> __errno_location() returned NULL.
>
> Zack, do you have any additional motivation for filing
> https://sourceware.org/bugzilla/show_bug.cgi?id=17328
My original motivation was to improve code generation with
-fsanitize=undefined, which, among other things, instruments *every
use of errno* with a check to ensure that the pointer returned by
__errno_location is non-null. For instance, the admittedly silly code
#include <errno.h>
int get_errno (void) { return errno; }
with GCC 4.9 and glibc 2.19, preprocesses to
extern int *__errno_location (void) __attribute__ ((__nothrow__ ,
__leaf__)) __attribute__ ((__const__));
int get_errno (void) { return (*__errno_location ()); }
and compiling with -O2 -fsanitize=undefined produces (.cfi chatter elided):
get_errno:
subq $24, %rsp
call __errno_location
testq %rax, %rax
je .L5
.L2:
movl (%rax), %eax
addq $24, %rsp
ret
.L5:
xorl %esi, %esi
movl $.Lubsan_data0, %edi
movq %rax, 8(%rsp)
call __ubsan_handle_type_mismatch
movq 8(%rsp), %rax
jmp .L2
clang 3.5 does essentially the same thing. However, as is, adding
__attribute__((returns_nonnull)) doesn't improve matters, because
neither gcc nor clang will optimize out the checks based on that. I
did file a GCC bug for that, but people were reluctant to make it
smarter for fear of losing necessary checks:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62307
... so I gotta wonder if it mightn't be a better use of people's time
to make errno a proper thread-local variable instead. What's blocking
that?
zw