maybe_fix_legacy_secondary_index_mv_schema() could reference dangling pointer #13720

tchaikov · 2023-04-29T08:37:59Z

GCC-13 rightly points out:

/home/kefu/dev/scylladb/db/schema_tables.cc:3654:30: error: possibly dangling reference to a temporary [-Werror=dangling-reference]
 3654 |     const column_definition& first_view_ck = v->clustering_key_columns().front();
      |                              ^~~~~~~~~~~~~
/home/kefu/dev/scylladb/db/schema_tables.cc:3654:79: note: the temporary was destroyed at the end of the full expression ‘(& v)->view_ptr::operator->()->schema::clustering_key_columns().boost::iterator_range<__gnu_cxx::__normal_iterator<const column_definition*, std::vector<column_definition> > >::<anonymous>.boost::iterator_range_detail::iterator_range_base<__gnu_cxx::__normal_iterator<const column_definition*, std::vector<column_definition> >, boost::iterators::random_access_traversal_tag>::<anonymous>.boost::iterator_range_detail::iterator_range_base<__gnu_cxx::__normal_iterator<const column_definition*, std::vector<column_definition> >, boost::iterators::bidirectional_traversal_tag>::<anonymous>.boost::iterator_range_detail::iterator_range_base<__gnu_cxx::__normal_iterator<const column_definition*, std::vector<column_definition> >, boost::iterators::incrementable_traversal_tag>::front()’
 3654 |     const column_definition& first_view_ck = v->clustering_key_columns().front();
      |                                              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~

The text was updated successfully, but these errors were encountered:

`clustering_key_columns()` returns a range view, and `front()` returns the reference to its first element. so we cannot assume the availability of this reference after the expression is evaluated. to address this issue, let's capture the reference by value. this also silences warning from GCC-13: ``` /home/kefu/dev/scylladb/db/schema_tables.cc:3654:30: error: possibly dangling reference to a temporary [-Werror=dangling-reference] 3654 | const column_definition& first_view_ck = v->clustering_key_columns().front(); | ^~~~~~~~~~~~~ /home/kefu/dev/scylladb/db/schema_tables.cc:3654:79: note: the temporary was destroyed at the end of the full expression ‘(& v)->view_ptr::operator->()->schema::clustering_key_columns().boost::iterator_range<__gnu_cxx::__normal_iterator<const column_definition*, std::vector<column_definition> > >::<anonymous>.boost::iterator_range_detail::iterator_range_base<__gnu_cxx::__normal_iterator<const column_definition*, std::vector<column_definition> >, boost::iterators::random_access_traversal_tag>::<anonymous>.boost::iterator_range_detail::iterator_range_base<__gnu_cxx::__normal_iterator<const column_definition*, std::vector<column_definition> >, boost::iterators::bidirectional_traversal_tag>::<anonymous>.boost::iterator_range_detail::iterator_range_base<__gnu_cxx::__normal_iterator<const column_definition*, std::vector<column_definition> >, boost::iterators::incrementable_traversal_tag>::front()’ 3654 | const column_definition& first_view_ck = v->clustering_key_columns().front(); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~ ``` Fixes scylladb#13720 Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>

`clustering_key_columns()` returns a range view, and `front()` returns the reference to its first element. so we cannot assume the availability of this reference after the expression is evaluated. to address this issue, let's capture the returned range by value, and keep the first element by reference. this also silences warning from GCC-13: ``` /home/kefu/dev/scylladb/db/schema_tables.cc:3654:30: error: possibly dangling reference to a temporary [-Werror=dangling-reference] 3654 | const column_definition& first_view_ck = v->clustering_key_columns().front(); | ^~~~~~~~~~~~~ /home/kefu/dev/scylladb/db/schema_tables.cc:3654:79: note: the temporary was destroyed at the end of the full expression ‘(& v)->view_ptr::operator->()->schema::clustering_key_columns().boost::iterator_range<__gnu_cxx::__normal_iterator<const column_definition*, std::vector<column_definition> > >::<anonymous>.boost::iterator_range_detail::iterator_range_base<__gnu_cxx::__normal_iterator<const column_definition*, std::vector<column_definition> >, boost::iterators::random_access_traversal_tag>::<anonymous>.boost::iterator_range_detail::iterator_range_base<__gnu_cxx::__normal_iterator<const column_definition*, std::vector<column_definition> >, boost::iterators::bidirectional_traversal_tag>::<anonymous>.boost::iterator_range_detail::iterator_range_base<__gnu_cxx::__normal_iterator<const column_definition*, std::vector<column_definition> >, boost::iterators::incrementable_traversal_tag>::front()’ 3654 | const column_definition& first_view_ck = v->clustering_key_columns().front(); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~ ``` Fixes scylladb#13720 Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>

`clustering_key_columns()` returns a range view, and `front()` returns the reference to its first element. so we cannot assume the availability of this reference after the expression is evaluated. to address this issue, let's capture the returned range by value, and keep the first element by reference. this also silences warning from GCC-13: ``` /home/kefu/dev/scylladb/db/schema_tables.cc:3654:30: error: possibly dangling reference to a temporary [-Werror=dangling-reference] 3654 | const column_definition& first_view_ck = v->clustering_key_columns().front(); | ^~~~~~~~~~~~~ /home/kefu/dev/scylladb/db/schema_tables.cc:3654:79: note: the temporary was destroyed at the end of the full expression ‘(& v)->view_ptr::operator->()->schema::clustering_key_columns().boost::iterator_range<__gnu_cxx::__normal_iterator<const column_definition*, std::vector<column_definition> > >::<anonymous>.boost::iterator_range_detail::iterator_range_base<__gnu_cxx::__normal_iterator<const column_definition*, std::vector<column_definition> >, boost::iterators::random_access_traversal_tag>::<anonymous>.boost::iterator_range_detail::iterator_range_base<__gnu_cxx::__normal_iterator<const column_definition*, std::vector<column_definition> >, boost::iterators::bidirectional_traversal_tag>::<anonymous>.boost::iterator_range_detail::iterator_range_base<__gnu_cxx::__normal_iterator<const column_definition*, std::vector<column_definition> >, boost::iterators::incrementable_traversal_tag>::front()’ 3654 | const column_definition& first_view_ck = v->clustering_key_columns().front(); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~ ``` Fixes #13720 Signed-off-by: Kefu Chai <kefu.chai@scylladb.com> Closes #13721 (cherry picked from commit 135b4fd)

denesb · 2023-12-15T11:56:12Z

Backported to 5.2.

tchaikov mentioned this issue Apr 29, 2023

db: schema_tables: capture reference to temporary value by value #13721

Closed

scylladb-promoter closed this as completed in 135b4fd May 2, 2023

scylladb-promoter added the Backport candidate label May 2, 2023

DoronArazii added this to the 5.3 milestone May 7, 2023

denesb removed the Backport candidate label Dec 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

maybe_fix_legacy_secondary_index_mv_schema() could reference dangling pointer #13720

maybe_fix_legacy_secondary_index_mv_schema() could reference dangling pointer #13720

tchaikov commented Apr 29, 2023

denesb commented Dec 15, 2023

maybe_fix_legacy_secondary_index_mv_schema() could reference dangling pointer #13720

maybe_fix_legacy_secondary_index_mv_schema() could reference dangling pointer #13720

Comments

tchaikov commented Apr 29, 2023

denesb commented Dec 15, 2023