This is the mail archive of the
mailing list for the glibc project.
Re: [PATCH][BZ #13724] Do not segfault in pthread_setname_np (x, NULL)
- From: Torvald Riegel <triegel at redhat dot com>
- To: Jeff Law <law at redhat dot com>
- Cc: "Carlos O'Donell" <carlos at redhat dot com>, Rich Felker <dalias at aerifal dot cx>, OndÅej BÃlka <neleai at seznam dot cz>, libc-alpha at sourceware dot org
- Date: Tue, 08 Oct 2013 23:58:51 +0300
- Subject: Re: [PATCH][BZ #13724] Do not segfault in pthread_setname_np (x, NULL)
- Authentication-results: sourceware.org; auth=none
- References: <20131003122009 dot GA8891 at domone dot podge> <524DCA52 dot 2030609 at redhat dot com> <20131007141928 dot GV20515 at brightrain dot aerifal dot cx> <52542C63 dot 10305 at redhat dot com> <20131008162738 dot GG20515 at brightrain dot aerifal dot cx> <52545389 dot 6000901 at redhat dot com> <52545730 dot 6090306 at redhat dot com>
On Tue, 2013-10-08 at 13:04 -0600, Jeff Law wrote:
> On 10/08/13 12:48, Carlos O'Donell wrote:
> > On 10/08/2013 12:27 PM, Rich Felker wrote:
> >> On Tue, Oct 08, 2013 at 12:01:39PM -0400, Carlos O'Donell wrote:
> >>> On 10/07/2013 10:19 AM, Rich Felker wrote:
> >>>> If you're going to check for NULL pointer arguments where you have
> >>>> not entered into a contract to accept and interpret them, do it
> >>>> with an assert, not a conditional error return. This way the bugs
> >>>> in the caller will be immediately detected and can be fixed, and
> >>>> it makes it easy to disable the overhead in production builds. I
> >>>> question the value of the assert except as documentation however;
> >>>> a segfault from dereferencing the NULL pointer is just as
> >>>> effective for debugging.
> >>> Should we have an assert there then to document the contract and provide
> >>> a more meaningful error message like a backtrace?
> >> There are probably hundreds of places all over glibc where this same
> >> issue applies. I don't think documenting them all at the source level
> >> via assertions is a productive use of developer time, source code
> >> size, binary size, or changelog and git log clutter. C programmers
> >> should not be treating the passing of NULL where a string is expected
> >> as a diagnosable error but as what it is: undefined behavior. Usually,
> >> a crash will naturally result and be easy enough to debug.
> > Doesn't this violate the previous reason for wanting to avoid the NULL
> > check?
> > If we don't assert the NULL may get stored in a structure or array and
> > eventually find it's way to code that dereferences it much much later
> > and cause a crash that is not near the place at which the application
> > entered into undefined behaviour.
> > It seems incredibly useful to enable the asserts and trigger these
> > violations as early as possible. If you don't care you can disable
> > the asserts?
> Another approach would be similar to what we're doing with memstomp.
> ie, build a set of wrappers which check for these argument goofs and
> allow users to dl-preload DSOs with the wrappers.
> When I first proposed the idea for these sanity checking dl-preload
> libraries for Fedora I envisioned that we could go beyond just checking
> for overlapping memory areas in the mem* and str* functions. There
> could be a set of pthread wrapper functions that check for whatever
> invariants we can in the pthread* functions without a huge performance hit.
This sounds like it could be a very useful debugging tool. If we had
something like this, it could perhaps also be required to use this
before posting a bug report?