Re: [PATCH] Add qsort comparator consistency checking (PR71702)

[Richard Biener <richard dot guenther at gmail dot com>]

On Tue, Jul 19, 2016 at 3:27 PM, Alexander Monakov <amonakov@ispras.ru> wrote:
> On Tue, 19 Jul 2016, Richard Biener wrote:
>> Yes.  The other option is to enable this checking not with ENABLE_CHECKING
>> but some new checking option, say ENABLE_CHECKING_ALGORITHMS, and
>> do full checking in that case.
>
> Thanks - I'm going to fold in this idea when redoing the patch (i.e. check a
> subset of pairs under normal checking, all pairs under this option macro).
>
> While the topic is fresh, I'd like to mention a small complication with
> extending this checking to cover all qsort calls.  I mentioned in the opening
> mail that I was going to do that with a '#define qsort(..) qsort_chk (..)' in
> gcc/system.h, but I missed that vec::qsort would be subject to this macro
> expansion as well.
>
> I see two possible solutions.  The first is to use the argument counting trick
> to disambiguate between libc qsort(base, n, sz, cmp) and vec::qsort(cmp) on the
> preprocessor level.  I don't see a reason it wouldn't work, but in this context
> I'd consider that a last-resort measure rather than an appropriate solution.
>
> The second is to rename vec::qsort to vec::sort. While mass renaming is not very
> nice, I hope it is acceptable in this case (I think formally vec::qsort
> declaration in GCC is not portable, because it implicitly expects that stdlib.h
> wouldn't shadow qsort with a function-like macro).
>
>
> Actually, thinking about it more, instead of redirecting qsort in system.h, it
> may be more appropriate to introduce gcc_qsort that wraps qsort and does
> checking, add gcc_qsort_nochk as an escape hatch for cases where checking
> shouldn't be done, and poison qsort in system.h (this again depends on doing the
> vec::sort mass-rename).

I don't think macro expansion applies to vec.qsort or vec::qsort and if it did
such macro would be bogus and we'd simply undefine it.

To capture non-vec qsorts simply wrap qsort everywhere like you suggested.

Richard.

>
> Alexander