Re: [PATCH] Async signal safe TLS accesses

[Rich Felker <dalias at aerifal dot cx>]

On Sat, Nov 30, 2013 at 03:50:45PM -0200, Alexandre Oliva wrote:
> On Nov 28, 2013, Rich Felker <dalias@aerifal.cx> wrote:
> 
> > On Thu, Nov 28, 2013 at 11:03:36PM -0200, Alexandre Oliva wrote:
> >> Since __signal_safe_malloc is guaranteed to return just-mmapped memory,
> >> and that is always zeroed out by the kernel, there's no real need for
> >> __signal_safe_calloc to dirty the pages scribbling over them with
> >> userland zeros; they're no different from kernel zeros ;-)
> 
> > Is this part of the interface contract, or could __signal_safe_malloc
> > simply call malloc when outside of a context where malloc would be
> > invoked reentrantly?
> 
> The implementation of __signal_safe_malloc in the patch I reviewed
> called mmap for each allocation.  Of course, if we were to optimize out
> the redundant memset in __signal_safe_calloc, there should be a comment
> in the __signal_safe_memalign implementation indicating that such
> callers as __signal_safe_calloc expect zeroed memory.

I am in favor of making these interfaces as simple as possible rather
than introducing gratuitous complexity, but I'd also like to make sure
you can avoid unnecessary memset of huge thread-local BSS which
dirties pages which might otherwise never be touched.

> > IMO the interface contract should be well-defined, and I don't think
> > calling malloc should ever be permitted, since then using
> > __signal_safe_free would not be possible from a signal handler in
> > general.
> 
> If we're discussing the interfaces, we might want to add a boolean*
> argument that, if non-NULL, will be set to indicate whether the memory
> is all-zeroes.
> 
> It would be nice if could also make the interface leak-safe in case of
> async cancellation

I'm against any attempt to make anything "safe" under async
cancellation. Calling any library functions at all (other than to
disable async cancellation) with async cancellation active is just a
serious programming error. There are way too many things that can go
wrong, and no good reason to do it.

Ideally, async cancellation should simply not be used; calling
pthread_testcancel periodically during long-running operations with no
cancellation points is much safer. Of course there are some situations
where you're calling existing third-party code that performs a long
computation and doesn't already have cancellation points in it, but in
order to use async cancellation in this situation, you'd need a
contract with the existing code that it's not going to use any
standard library functions internally. Also, using async cancellation
is problematic because pthread_setcanceltype is not itself a
cancellation point. So if there's already a cancellation request
pending when you call pthread_setcanceltype, it may be ignored; you
can't manually act on it with pthread_testcancel because
pthread_testcancel is not async-cancel-safe.

> (AFAICT that requires taking a pointer-to-pointer and
> setting it, rather than returning a pointer that might be lost), but
> mmap's interface itself doesn't make this sort of safety possible when
> AC is enabled.

Pointer-to-pointer allocation interfaces should be Considered Harmful
because they encourage UB via aliasing violations of the form:

	char *p;
	posix_memalign((void **)&p, a, n);

Rich