From: Kevin Bracey <kevin@bracey.fi>
To: GIT Mailing-list <git@vger.kernel.org>
Subject: Re: [PATCH 1/3] add QSORT
Date: Mon, 03 Oct 2016 19:46:35 +0300 [thread overview]
Message-ID: <57F28B6B.1010204@bracey.fi> (raw)
In-Reply-To: <83398160-555f-adab-6b1e-3283c533b5ff@web.de>
On 01/10/2016 19:19, René Scharfe wrote:
> It's hard to imagine an implementation of qsort(3) that can't handle
> zero elements. QSORT's safety feature is that it prevents the compiler
> from removing NULL checks for the array pointer. E.g. the last two
> lines in the following example could be optimized away:
>
> qsort(ptr, n, sizeof(*ptr), fn);
> if (!ptr)
> do_stuff();
>
> You can see that on https://godbolt.org/g/JwS99b -- an awesome website
> for exploring compilation results for small snippets, by the way.
>
> This optimization is dangerous when combined with the convention of
> using a NULL pointer for empty arrays. Diagnosing an affected NULL
> check is probably quite hard -- it's right there in the code after all
> and not all compilers remove it.
Hang on, hang on. This is either a compiler bug, or you're wrong on your
assumption about the specification of qsort.
Either way, the extra layer of indirection is not proper protection. The
unwanted compiler optimisation you're inadvertently triggering could
still be triggered through the inline.
Now, looking at the C standard, this isn't actually clear to me. The
standard says that if you call qsort with nmemb zero, the pointer still
has to be "valid". Not totally clear to me if NULL is valid, by their
definition in C99 7.1.4. Googling hasn't given me a concrete answer.
The compiler seems to think that NULL wouldn't be valid, so because
you've called qsort on it, you've invoked undefined behaviour if it's
NULL, so it's free to elide the NULL check.
Kevin
next prev parent reply other threads:[~2016-10-03 18:04 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-29 15:23 [PATCH 1/3] add QSORT René Scharfe
2016-09-29 15:27 ` [PATCH 2/3] use QSORT René Scharfe
2016-09-29 15:29 ` [PATCH 3/3] remove unnecessary check before QSORT René Scharfe
2016-09-29 22:36 ` [PATCH 1/3] add QSORT Junio C Hamano
2016-09-29 23:21 ` René Scharfe
2016-09-29 23:40 ` René Scharfe
2016-10-01 16:19 ` René Scharfe
2016-10-03 16:46 ` Kevin Bracey [this message]
2016-10-03 17:09 ` Kevin Bracey
2016-10-03 22:00 ` René Scharfe
2016-10-04 5:28 ` Kevin Bracey
2016-10-04 20:31 ` René Scharfe
2016-10-05 15:00 ` Kevin Bracey
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=57F28B6B.1010204@bracey.fi \
--to=kevin@bracey.fi \
--cc=git@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).