This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 1/5] Fix asymmetric comparison functions


On 12/17/2015 03:25 PM, Richard Biener wrote:
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.

Well, this was detected by calling qsort(x, x) and checking that return value is zero in qsort interceptor. So I guess it's more of "theoretical" sort.

/Yura


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]