[libcxx-commits] [PATCH] D129823: [libc++][ranges] Make range algorithms support proxy iterators

Mon Jul 25 18:10:31 PDT 2022

huixie90 added a comment.

In D129823#3677969 <https://reviews.llvm.org/D129823#3677969>, @jgorbe wrote:

> Some more information about the problem. The stack trace looks like follows. The problem manifests as an ASan out of memory error because the move constructor in varlen_sort.h:59 is getting incorrect data and it's trying to allocate an absurdly big amount of memory.
>
>   ==7713==ERROR: AddressSanitizer: out of memory: allocator is trying to allocate 0xc5600000e4e bytes
>       #0 0x55deed3caedd in operator new[](unsigned long) third_party/llvm/llvm-project/compiler-rt/lib/asan/asan_new_delete.cpp:98:3
>       #1 0x55deed6e733c in varlen_element MYSQLSRCHOME/include/varlen_sort.h:59:17
>       #2 0x55deed6e733c in bool std::__u::__insertion_sort_incomplete<void varlen_sort<unsigned char, DsMrr_impl::dsmrr_fill_buffer()::$_3>(unsigned char*, unsigned char*, unsigned long, DsMrr_impl::dsmrr_fill_buffer()::$_3)::'lambda'(varlen_element const&, varlen_element const&)&, varlen_iterator>(DsMrr_impl::dsmrr_fill_buffer()::$_3, DsMrr_impl::dsmrr_fill_buffer()::$_3, unsigned char) third_party/crosstool/v18/stable/toolchain/bin/../include/c++/v1/__algorithm/sort.h:373:18
>       #3 0x55deed6e4fcd in void std::__u::__introsort<std::__u::_ClassicAlgPolicy, void varlen_sort<unsigned char, DsMrr_impl::dsmrr_fill_buffer()::$_3>(unsigned char*, unsigned char*, unsigned long, DsMrr_impl::dsmrr_fill_buffer()::$_3)::'lambda'(varlen_element const&, varlen_element const&)&, varlen_iterator>(varlen_iterator, varlen_iterator, DsMrr_impl::dsmrr_fill_buffer()::$_3, std::__u::iterator_traits<varlen_iterator>::difference_type) third_party/crosstool/v18/stable/toolchain/bin/../include/c++/v1/__algorithm/sort.h:575:19
>       #4 0x55deed6db70b in __sort<(lambda at MYSQLSRCHOME/include/varlen_sort.h:199:13) &, varlen_iterator> third_party/crosstool/v18/stable/toolchain/bin/../include/c++/v1/__algorithm/sort.h:627:3
>       #5 0x55deed6db70b in __sort_impl<std::__u::_ClassicAlgPolicy, varlen_iterator, (lambda at MYSQLSRCHOME/include/varlen_sort.h:199:13)> third_party/crosstool/v18/stable/toolchain/bin/../include/c++/v1/__algorithm/sort.h:687:5
>       #6 0x55deed6db70b in sort<varlen_iterator, (lambda at MYSQLSRCHOME/include/varlen_sort.h:199:13)> third_party/crosstool/v18/stable/toolchain/bin/../include/c++/v1/__algorithm/sort.h:694:3
>       #7 0x55deed6db70b in varlen_sort<unsigned char, (lambda at MYSQLSRCHOME/sql/handler.cc:6738:7)> MYSQLSRCHOME/include/varlen_sort.h:197:3
>       #8 0x55deed6db70b in DsMrr_impl::dsmrr_fill_buffer() MYSQLSRCHOME/sql/handler.cc:6736:3
>       #9 0x55deed6da9c8 in DsMrr_impl::dsmrr_init(RANGE_SEQ_IF*, void*, unsigned int, unsigned int, HANDLER_BUFFER*) MYSQLSRCHOME/sql/handler.cc:6617:17
>       #10 0x55deedf21bb6 in MultiRangeRowIterator::Init() MYSQLSRCHOME/sql/bka_iterator.cc:368:18
>       #11 0x55deedf21100 in BKAIterator::ReadOuterRows() MYSQLSRCHOME/sql/bka_iterator.cc:228:22
>       #12 0x55deedf2174a in BKAIterator::Read() MYSQLSRCHOME/sql/bka_iterator.cc:274:19
>       #13 0x55deede3ed6b in Query_expression::ExecuteIteratorQuery(THD*) MYSQLSRCHOME/sql/sql_union.cc:1231:36
>       #14 0x55deede3f4e4 in Query_expression::execute(THD*) MYSQLSRCHOME/sql/sql_union.cc:1284:10
>       #15 0x55deedd5a4f6 in Sql_cmd_dml::execute_inner(THD*) MYSQLSRCHOME/sql/sql_select.cc:776:15
>       #16 0x55deedd59381 in Sql_cmd_dml::execute(THD*) MYSQLSRCHOME/sql/sql_select.cc:574:7
>       #17 0x55deedcb2863 in mysql_execute_command(THD*, bool) MYSQLSRCHOME/sql/sql_parse.cc:4438:29
>       #18 0x55deedcae283 in dispatch_sql_command(THD*, Parser_state*) MYSQLSRCHOME/sql/sql_parse.cc:5037:19
>       #19 0x55deedcaac9c in dispatch_command(THD*, COM_DATA const*, enum_server_command) MYSQLSRCHOME/sql/sql_parse.cc:1863:7
>       #20 0x55deedcacd8b in do_command(THD*) MYSQLSRCHOME/sql/sql_parse.cc:1342:18
>       #21 0x55deed3fe4f3 in handle_connection(void*) MYSQLSRCHOME/sql/conn_handler/connection_handler_per_thread.cc:310:13
>       #22 0x55def2434649 in pfs_spawn_thread(void*) MYSQLSRCHOME/storage/perfschema/pfs.cc:2905:3
>       #23 0x7f6af0285b54 in start_thread (/usr/grte/v5/lib64/libpthread.so.0+0xbb54) (BuildId: 64752de50ebd1a108f4b3f8d0d7e1a13)
>
> sort.h:373 does
>
>   	      value_type __t(_Ops::__iter_move(__i));
>
> When reaching this line the value of `__i` is correct. However, after the call to `__iter_move`, the `varlen_element` move constructor (source <https://fossies.org/dox/mysql-8.0.29/varlen__sort_8h_source.html#l00055>) gets incorrect values.
>
> I've managed to get the test to pass by replacing occurrences of `_Ops::__iter_move(iter)` back to `_VSTD::move(*iter)` and I don't see anything particularly wrong with the affected mysql code so I suspect there's some remaining problem in `__iter_move` that hasn't been solved with the follow-up patches so far.

Hi,

Thank you very much for posting the analysis. I think the problem is this line
https://fossies.org/dox/mysql-8.0.29/varlen__sort_8h_source.html#l00187

It sets the `iteartor_traits` to be `iterator_traits<varlen_element *>`. The `iterator_traits<varlen_element *>::reference` is `varlen_element&` (note the reference here). However, its `operator*` does not return `varlen_element&`, but a prvalue `varlen_element`
https://fossies.org/dox/mysql-8.0.29/varlen__sort_8h_source.html#l00090

So this is technically UB because its `iteartor_traits` lied.

Note that we relied on the `iterator_traits::reference` to be correct to dispatch the `iter_move`
https://github.com/llvm/llvm-project/blob/78015047b22dd64f7c647ab7ce04d42e761e7b8f/libcxx/include/__algorithm/iterator_operations.h#L81

We could in theory dispatch on the `decltype(*it)`, but this is somehow challenging in C++03 mode where `decltype` does not exist.
So in conclusion, it is mysql code that has UB. But it might still be better for us to keep the UB working.

@ldionne @var-const ,
what do you think?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129823