This is the mail archive of the
mailing list for the glibc project.
Re: [PATCH] Don't divide by zero when trying to destroy an uninitialised barrier.
- From: Torvald Riegel <triegel at redhat dot com>
- To: Mark Thompson <mark dot thompson at starleaf dot com>
- Cc: Szabolcs Nagy <szabolcs dot nagy at arm dot com>, GNU C Library <libc-alpha at sourceware dot org>, nd <nd at arm dot com>
- Date: Thu, 21 Apr 2016 18:23:49 +0200
- Subject: Re: [PATCH] Don't divide by zero when trying to destroy an uninitialised barrier.
- Authentication-results: sourceware.org; auth=none
- References: <5717B2F4 dot 9050105 at starleaf dot com> <5717B657 dot 6040007 at arm dot com> <5717D575 dot 40806 at starleaf dot com>
On Wed, 2016-04-20 at 20:16 +0100, Mark Thompson wrote:
> On 20/04/16 18:03, Szabolcs Nagy wrote:
> > On 20/04/16 17:48, Mark Thompson wrote:
> >> ---
> >> nptl/pthread_barrier_destroy.c | 9 +++++++++
> >> 1 file changed, 9 insertions(+)
> >> diff --git a/nptl/pthread_barrier_destroy.c b/nptl/pthread_barrier_destroy.c
> >> index 92d2027..d114084 100644
> >> --- a/nptl/pthread_barrier_destroy.c
> >> +++ b/nptl/pthread_barrier_destroy.c
> >> @@ -36,6 +36,15 @@ pthread_barrier_destroy (pthread_barrier_t *barrier)
> >> they have exited as well. To get the notification, pretend that we have
> >> reached the reset threshold. */
> >> unsigned int count = bar->count;
> >> +
> >> + /* For an initialised barrier, count must be greater than zero here. An
> >> + uninitialised barrier may still have zero, however, and in this case it is
> >> + preferable to fail immediately rather than to invoke undefined behaviour
> >> + by dividing by zero on the next line. (POSIX allows the implementation to
> >> + diagnose invalid state and return EINVAL in this case.) */
> >> + if (__glibc_unlikely (count == 0))
> >> + return EINVAL;
> >> +
> > this case is undefined behaviour in posix, and
> > i think the best way to handle that is crashing.
> > (because no behaviour can be portably relied upon)
> The undefined behaviour is not necessarily crashing - systems which do not trap on divide by zero (such as aarch64) will do something else, such as returning success or hanging forever. Would you prefer an abort() be added to make the behavior consistent?
IMO, abort() would be better than returning EINVAL. See
> > nowadays posix says
> > "The [EINVAL] error for an uninitialized barrier
> > attributes object is removed; this condition
> > results in undefined behavior."
> It also says:
> "If an implementation detects that the value specified by the barrier argument to pthread_barrier_destroy() does not refer to an initialized barrier object, it is recommended that the function should fail and report an [EINVAL] error."
But there are no error conditions listed for pthread_barrier_destroy,
which I read as no errors being allowed in a correct program.
Therefore, this is really UB, and we should call abort().