This is the mail archive of the glibc-bugs@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[Bug libc/20544] RFE: atexit, __cxa_atexit, on_exit should assert function pointer argument is non-NULL


https://sourceware.org/bugzilla/show_bug.cgi?id=20544

Carlos O'Donell <carlos at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
   Last reconfirmed|                            |2016-09-08
                 CC|                            |carlos at redhat dot com
         Resolution|INVALID                     |---
            Summary|RFE: atexit, __cxa_atexit,  |RFE: atexit, __cxa_atexit,
                   |on_exit detect NULL         |on_exit should assert
                   |function pointer argument   |function pointer argument
                   |                            |is non-NULL
     Ever confirmed|0                           |1

--- Comment #9 from Carlos O'Donell <carlos at redhat dot com> ---
(In reply to John Reiser from comment #8)
> (In reply to Florian Weimer from comment #4)
> > If it is only about finding the culprit, a conditional breakpoint or a
> > Systemtap script should help here.
> > 
> > I don't think the code generated by GCC checks the return value of
> > __cxa_atexit, so an error return would not help anyway, we would have to
> > terminate the process.
> 
> It's not just about finding the culprit today.  It's also about the next
> time, and the time after that, etc.  I consider that a couple CPU cycles
> spent testing for NULL, plus the development and maintenance burden of the
> code to handle the case when the pointer is NULL, are worthwhile here.  If
> nothing else, then the call site in exit() should check as a last resort in
> order to prevent SIGSEGV.  
> I prefer to get a diagnostic on stderr and then skip over the NULL pointer,
> but I can accept a "graceful" abort().

You are right that this is a QoI issue.

The C11 and POSIX standards say that an error should be returned if atexit
fails, but IMO this means "failure to register function" not "registered
function is invalid."

Therefore I am willing to accept that adding a check for NULL that calls
assert(ptr != NULL) in order to find the program fault quickly under the
community "invalid pointers" and "assertion" clause:

https://sourceware.org/glibc/wiki/Style_and_Conventions#Invalid_pointers

Here we are recording a pointer into internal structures for later usage, and
so should assert the pointer is non-null.

The assertions can be disabled if glibc is built without assertions perhaps for
production code and thus gain back the speed loss that comes with the check.

> [Aside: in the particular case of x86_64, I consider that "check for NULL
> pointer" by default should be extended to include "check for virtual address
> width > 48 bits".  All x86_64 implementations that exist or are in
> development today use at most 48 bits of virtual address (see /proc/cpuinfo
> on Linux).  Checking can be done inexpensively by "if ((unsigned long)ptr >>
> 48)".  This detects all negative 64-bit signed integers, many UTF-8 text
> strings, most double precision floating point numbers.]

I believe this is outside the scope of this request and would require more
consideration given the architectures we support.

I'm reopening this bug to consider the assert addition.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]