Re: [patch] For BZ #17328, mark __errno_location with __attribute__((returns_nonnull)) for gcc >=4.9.0

[Zack Weinberg <zackw at panix 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