Skip to content

Resolves: tdf#90579 crash on undo formula-to-value on two formula cells

username-removed-584209 requested to merge caolanm/mdds:master into master

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.

Merge request reports