From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 39464 invoked by alias); 17 Dec 2015 12:04:23 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 39446 invoked by uid 89); 17 Dec 2015 12:04:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.9 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 spammy=H*r:May, Hx-languages-length:2143, HContent-type:windows-1252, Hx-spam-relays-external:ESMTPA X-Spam-User: qpsmtpd, 2 recipients X-HELO: mailout1.w1.samsung.com Received: from mailout1.w1.samsung.com (HELO mailout1.w1.samsung.com) (210.118.77.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Thu, 17 Dec 2015 12:04:21 +0000 Received: from eucpsbgm2.samsung.com (unknown [203.254.199.245]) by mailout1.w1.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTP id <0NZI00FNV474WL70@mailout1.w1.samsung.com>; Thu, 17 Dec 2015 12:04:16 +0000 (GMT) Received: from eusync1.samsung.com ( [203.254.199.211]) by eucpsbgm2.samsung.com (EUCPMTA) with SMTP id 03.CA.21385.0C4A2765; Thu, 17 Dec 2015 12:04:16 +0000 (GMT) Received: from [106.109.9.145] by eusync1.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTPA id <0NZI008LN473OJ60@eusync1.samsung.com>; Thu, 17 Dec 2015 12:04:16 +0000 (GMT) Subject: Re: [PATCH 1/5] Fix asymmetric comparison functions To: Jakub Jelinek References: <5672787D.6040105@samsung.com> <56727936.4030605@samsung.com> <20151217113903.GS18720@tucnak.redhat.com> Cc: GCC Patches , Andrey Belevantsev , Andrew MacLeod , Andrew Pinski , Diego Novillo , Geoff Keating , Jason Merrill , Richard Biener , Steven Bosscher From: Yury Gribov Message-id: <5672A4D0.8090509@samsung.com> Date: Thu, 17 Dec 2015 12:04:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 MIME-version: 1.0 In-reply-to: <20151217113903.GS18720@tucnak.redhat.com> Content-type: text/plain; charset=windows-1252; format=flowed Content-transfer-encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2015-12/txt/msg01748.txt.bz2 On 12/17/2015 02:39 PM, Jakub Jelinek wrote: > On Thu, Dec 17, 2015 at 11:58:30AM +0300, Yury Gribov wrote: >> 2015-12-17 Yury Gribov >> >> * c-family/c-common.c (resort_field_decl_cmp): >> Make symmteric. >> * cp/class.c (method_name_cmp): Ditto. >> (resort_method_name_cmp): Ditto. >> * fortran/interface.c (pair_cmp): Ditto. > > Note, c-family, cp and fortran have their own ChangeLog files, so > the entries without those prefixes need to go into each one and can't > refer to other ChangeLog through Ditto/Likewise etc. > Typo in symmteric. Right, thanks. > That said, is this actually really a problem? I mean, is qsort > allowed to call the comparison function with the same arguments? > I think lots of the comparison functions just assume that > for int cmpfn (const void *x, const void *y) x != y. > And if qsort can't call the comparison function with the same argument, > then perhaps the caller has some knowledge your checker does not, say > that the entries that would compare equal by the comparison function > simply can't appear in the array (so the caller knows that the comparison > function should never return 0). Self-comparisons are certainly less dangerous than transitive ones. I personally not aware about libc's which can compare element to itself. However * comparing an element to itself still a valid thing for qsort to do * most other comparison functions in GCC support this >> --- a/gcc/tree-vrp.c >> +++ b/gcc/tree-vrp.c >> @@ -5882,7 +5882,9 @@ compare_case_labels (const void *p1, const void *p2) >> else if (idx1 == idx2) >> { >> /* Make sure the default label is first in a group. */ >> - if (!CASE_LOW (ci1->expr)) >> + if (!CASE_LOW (ci1->expr) && !CASE_LOW (ci2->expr)) >> + return 0; >> + else if (!CASE_LOW (ci1->expr)) >> return -1; >> else if (!CASE_LOW (ci2->expr)) >> return 1; >> -- >> 1.9.1 > > Say here, we know there is at most one default label in a switch, never > more. So, unless qsort is allowed to call compare_case_labels > with p1 == p2 (which really doesn't make sense), this case just won't > happen.