git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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


  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).