From 2490cd164b926d248d4d3fe0d4d89130ccf3e5ac Mon Sep 17 00:00:00 2001 From: David Tardon Date: Wed, 22 Jun 2016 14:24:53 +0200 Subject: [PATCH] fix double delete in mtv::swap --- ...79-crash-on-undo-formula-to-value-on.patch | 91 +++++++++++++++++++ mdds.spec | 7 +- 2 files changed, 97 insertions(+), 1 deletion(-) create mode 100644 0001-Resolves-tdf-90579-crash-on-undo-formula-to-value-on.patch diff --git a/0001-Resolves-tdf-90579-crash-on-undo-formula-to-value-on.patch b/0001-Resolves-tdf-90579-crash-on-undo-formula-to-value-on.patch new file mode 100644 index 0000000..4ce4d04 --- /dev/null +++ b/0001-Resolves-tdf-90579-crash-on-undo-formula-to-value-on.patch @@ -0,0 +1,91 @@ +From 586230c6bc2b7ecb285d9eb9a0e3898c074791bf Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Caol=C3=A1n=20McNamara?= +Date: Fri, 17 Jun 2016 15:24:58 +0100 +Subject: [PATCH] Resolves: tdf#90579 crash on undo formula-to-value on two + formula cells + +when the two cells have different formula. + +On the undo we go from a single block of no-formulas to two blocks of one +formula each and the ScFormulaCell*s which belong to the undo are deleted +when they are transferred back to the ScDoc. Their pointers are still +use but they are now dangling pointers. + +so on the undo... + +in include/mdds/multi_type_vector_def.inl we enter +::swap_single_to_multi_blocks + +and do... + + // Get the new elements from the other container. + blocks_type new_blocks; + other.exchange_elements( + *blk_src->mp_data, src_offset, dst_block_index1, dst_offset1, dst_block_index2, dst_offset2, len, new_blocks); + +in ::exchange_elements + +after + + element_block_func::assign_values_from_block(*blk->mp_data, src_data, src_offset, len); + +using gdb and... +print ((const mdds::mtv::noncopyable_managed_element_block<54, ScFormulaCell> *)(m_blocks[0]->mp_data))->m_array[0] + +we can see that other now has the value of the ScFormulaCell* that will be deleted + +on return back to ::swap_single_to_multi_blocks we go to the src_offset == 0 and src_tail_len == 0 case which is + + if (src_tail_len == 0) + { + // the whole block needs to be replaced. + delete_block(blk_src); + m_blocks.erase(m_blocks.begin()+block_index); + } + +print ((const mdds::mtv::noncopyable_managed_element_block<54, ScFormulaCell> *)(blk_src->mp_data))->m_array[0] + +confirms that our ScFormulaCell* is here also and delete_block deletes the full +block and deletes the m_array which deletes the ScFormulaCell* and so its +deleted there while also still assigned in the new_block above. + +The new block goes on to be inserted, but its all broken now because it refers +to dead data. + +Presumably we need to either clear the part of the block that will be deleted +of its transferred data at exchange time. Or overwrite its data with zero. Or +swap at exchange time instead of copy. Or something of that nature anyway. + +Here I go with calling element_block_func::erase whether or not we delete the +block which seems to do the right thing. Crash goes away anyway and make check +here and there passes. +--- + include/mdds/multi_type_vector_def.inl | 5 +++-- + 1 file changed, 3 insertions(+), 2 deletions(-) + +diff --git a/include/mdds/multi_type_vector_def.inl b/include/mdds/multi_type_vector_def.inl +index 0e2a15a..fe9c767 100644 +--- a/include/mdds/multi_type_vector_def.inl ++++ b/include/mdds/multi_type_vector_def.inl +@@ -2306,6 +2306,9 @@ void multi_type_vector<_CellBlockFunc, _EventFunc>::swap_single_to_multi_blocks( + { + // Source range is at the top of a block. + ++ // Shrink the current block by erasing the top part. ++ element_block_func::erase(*blk_src->mp_data, 0, len); ++ + if (src_tail_len == 0) + { + // the whole block needs to be replaced. +@@ -2314,8 +2317,6 @@ void multi_type_vector<_CellBlockFunc, _EventFunc>::swap_single_to_multi_blocks( + } + else + { +- // Shrink the current block by erasing the top part. +- element_block_func::erase(*blk_src->mp_data, 0, len); + blk_src->m_size -= len; + } + +-- +2.7.4 + diff --git a/mdds.spec b/mdds.spec index abe98cb..17fb740 100644 --- a/mdds.spec +++ b/mdds.spec @@ -5,7 +5,7 @@ Name: mdds Version: 1.2.0 -Release: 1%{?dist} +Release: 2%{?dist} Summary: A collection of multi-dimensional data structures and indexing algorithms Group: Development/Libraries @@ -13,6 +13,8 @@ License: MIT URL: https://gitlab.com/mdds/mdds Source0: http://kohei.us/files/%{name}/src/%{name}-%{version}.tar.bz2 +Patch0: 0001-Resolves-tdf-90579-crash-on-undo-formula-to-value-on.patch + BuildRequires: boost-devel %description @@ -60,6 +62,9 @@ make check %{?_smp_mflags} %license LICENSE %changelog +* Wed Jun 22 2016 David Tardon - 1.2.0-2 +- fix double delete in mtv::swap + * Thu May 12 2016 David Tardon - 1.2.0-1 - new upstream release