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