This is the mail archive of the 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]

Re: [PATCH][BZ #13724] Do not segfault in pthread_setname_np (x, NULL)

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

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.


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