summaryrefslogtreecommitdiff
path: root/openmp
diff options
context:
space:
mode:
authorLouis Dionne <ldionne.2@gmail.com>2023-03-28 13:10:25 -0400
committerLouis Dionne <ldionne.2@gmail.com>2023-05-09 09:05:29 -0400
commit36d8b449cfc9850513bb2ed6c07b5b8cc9f1ae3a (patch)
treefed70d16e5c7b3b8a394eae0036d106289e4f182 /openmp
parent28bdff19e3ad981ef83e372e8c301f1407b82135 (diff)
downloadllvm-36d8b449cfc9850513bb2ed6c07b5b8cc9f1ae3a.tar.gz
[libc++] Add assertions for potential OOB reads in std::sort
We introduced an optimization to std::sort in 4eddbf9f10a6. However, that optimization led to issues where users that were passing invalid comparators to std::sort could start seeing OOB reads. This led to the revert of the std::sort optimization from the LLVM 16 release (see https://llvm.org/D146421). This patch introduces _LIBCPP_ASSERTs to the places in the algorithm where we make an assumption that the comparator will be consistent and hence avoid a bounds check based on that. If the comparator happens not to be consistent with itself, these are the places where we would incorrectly go out of bounds. This allows users that enable libc++ assertions to catch such misuse at the cost of basically a bounds check. For users that do not enable libc++ assertions (which is 99.9% of users since assertions are off by default), this is basically a no-op, and in fact the assertion will turn into a __builtin_assume, making it explicit to the compiler that it can rely on the fact that we're not going out of bounds. I think this patch strikes the right balance. Folks that want absolute performance will get what they want, since it is a precondition for the comparator to be consistent, so the bounds checks are technically not mandatory. Folks who want more safety *already* need to be enabling libc++ assertions to catch other types of bugs (like operator[] OOB), so this solution should also work for them. I do think we have a lot of work towards popularizing the use of libc++ assertions and integrating it better so that users don't have to know about the obscure _LIBCPP_ENABLE_ASSERTIONS macro to enable them, but that's a separate concern. rdar://106897934 Differential Revision: https://reviews.llvm.org/D147089
Diffstat (limited to 'openmp')
0 files changed, 0 insertions, 0 deletions