From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 100185 invoked by alias); 17 Dec 2015 12:25:18 -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 100159 invoked by uid 89); 17 Dec 2015 12:25:17 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.4 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=analyzer, analyzers, H*Ad:D*ru X-Spam-User: qpsmtpd, 2 recipients X-HELO: mx2.suse.de Received: from mx2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (CAMELLIA256-SHA encrypted) ESMTPS; Thu, 17 Dec 2015 12:25:16 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 91FB0ABE5; Thu, 17 Dec 2015 12:25:12 +0000 (UTC) Date: Thu, 17 Dec 2015 12:25:00 -0000 From: Richard Biener To: Yury Gribov cc: GCC Patches , Andrey Belevantsev , Andrew MacLeod , Andrew Pinski , Diego Novillo , Geoff Keating , Jakub Jelinek , Jason Merrill , Steven Bosscher Subject: Re: [PATCH 1/5] Fix asymmetric comparison functions In-Reply-To: <5672A747.1070806@samsung.com> Message-ID: References: <5672787D.6040105@samsung.com> <56727936.4030605@samsung.com> <5672A0D1.2040207@samsung.com> <5672A747.1070806@samsung.com> User-Agent: Alpine 2.11 (LSU 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-SW-Source: 2015-12/txt/msg01753.txt.bz2 On Thu, 17 Dec 2015, Yury Gribov wrote: > On 12/17/2015 02:59 PM, Richard Biener wrote: > > On Thu, 17 Dec 2015, Yury Gribov wrote: > > > > > On 12/17/2015 02:41 PM, Richard Biener wrote: > > > > On Thu, 17 Dec 2015, Yury Gribov wrote: > > > > > > > > > Some obvious symmetry fixes. > > > > > > > > > > Cc-ing > > > > > * Andrey (Belevantsev) for bb_top_order_comparator > > > > > * Andrew (MacLeod) for compare_case_labels > > > > > * Andrew (Pinski) for resort_field_decl_cmp > > > > > * Diego for pair_cmp > > > > > * Geoff for resort_method_name_cmp > > > > > * Jakub for compare_case_labels > > > > > * Jason for method_name_cmp > > > > > * Richard for insert_phi_nodes_compare_var_infos, compare_case_labels > > > > > * Steven for cmp_v_in_regset_pool > > > > > > > > So for compare_case_labels we only ever have one label with > > > > !CASE_LOW - which means you only run into the case that needs > > > > !CASE_LOW && !CASE_LOW if comparing an element with itself, correct? > > > > > > > > In this case (missing "same element" handling rather than symmetry > > > > fixing) I'd prefer a > > > > > > > > if (case1 == case2) > > > > return 0; > > > > > > > > So just to confirm - do the patches also contain same element > > > > compare fixings? > > > > > > Yes, that's a fix for same element. How about adding if + gcc_assert that > > > both cases can't be NULL otherwise? > > > > Well, does qsort require the compare function to result in zero > > for same elements when the sequence to be sorted doesn't contain > > duplicates? > > Sure, that's part of total ordering requirement in standard. > > > If an assert works for you that hints at these places found via static > > analysis rather than a runtime fuzzer? > > Sorry, not sure I fully understood but - yes, adding assertion would typically > allow for better checking by static analyzers. The question was if you actually observed the case to happen with a testcase (and whatever mungled qsort implementation) or whether it was a theoretical outcome computed by a static analyzer. That is, whether you could hand me a testcase where it happens or not. Richard.