[libcxx-commits] [PATCH] D146421: [release/16.x][libc++] Revert the bitset sort optimization

[Danila Kutenin via Phabricator via libcxx-commits]

danlark added a comment.

> Shuffling the range wouldn't help here -- the problem wasn't non-determinism in the output, it's that we literally go and read out-of-bounds when the comparator is wrong. I think this is happening in __insertion_sort_unguarded, and I think I can add an assertion check just in there to "fix" the issue but I haven't had time to get the reproducer integrated in our test suite yet.

In the previous implementation, out of bounds is possible, see Howard Hinnant on stackoverflow <https://stackoverflow.com/questions/38966516/should-sorting-algorithm-pass-same-element-in-the-comparison-function>. The sort will sigsegv with 31 elements with the comparator that always returns true.

In this patch these lines start with 561. New implementation triggers unguarded optimaztion with fewer elements. That's the root cause. The rollback likely only hides the problem for customers which sort fewer elements.

FWIW, shuffling can help finding these problems, say, pivot takes NaN and thus it can go into the unguarded case.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146421/new/

https://reviews.llvm.org/D146421