ODFstudio/odfstudio
2
0
Fork 0
Code Issues Packages Activity

prevent stale SwNode objects from reporting wrong value in GetIndex

Browse Source 
Sometimes we end up in situations where a SwNode(BigPtrEntry) is still alive,
but has been removed from the SwNodes(BigPtrArray) data structure.
And then we call SwNode::GetIndex and it reports a bogus value.

So clear those back-pointers to avoid reading incorrect data.

Change-Id: I95d4f99bf169eeb1c3290432ff8b64ca0aa69275
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/180126
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
This commit is contained in:
Noel Grandin
2025-01-13 09:06:24 +01:00
committed by Noel Grandin
parent d59aa7ddfd
commit 91abc3df61
5 changed files with 58 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
sw/inc/bparr.hxx
+3
View File
@@ -41,6 +41,7 @@ public:
inline sal_Int32 GetPos() const;
inline BigPtrArray& GetArray() const;
bool IsDisconnected() const { return m_pBlock == nullptr; }
};
// 1000 entries per Block = a bit less than 4K
@@ -76,6 +77,8 @@ protected:
BlockInfo* InsBlock( sal_uInt16 ); ///< insert block
void BlockDel( sal_uInt16 ); ///< some blocks were deleted
void UpdIndex( sal_uInt16 ); ///< recalculate indices
void ImplRemove( sal_Int32 pos, sal_Int32 n, bool bClearElement );
void ImplReplace( sal_Int32 idx, BigPtrEntry* pElem, bool bClearElement );
// fill all blocks
sal_uInt16 Compress();
sw/inc/node.hxx
+3
View File
@@ -116,6 +116,9 @@ public:
}
void SetRedlineMergeFlag(Merge const eMerge) { m_eMerge = eMerge; }
Merge GetRedlineMergeFlag() const { return m_eMerge; }
using BigPtrEntry::IsDisconnected; // make this public
private:
Merge m_eMerge;
sw/qa/core/Test-BigPtrArray.cxx
+20 -6
View File
@@ -237,8 +237,9 @@ public:
{
sal_Int32 oldCount = bparr.Count();
delete bparr[0]; // release content
auto p = bparr[0];
bparr.Remove(0); // remove item from container
delete p; // release content
CPPUNIT_ASSERT_EQUAL_MESSAGE
(
@@ -272,8 +273,9 @@ public:
for (int i = NUM_ENTRIES - 1; i >= 0; i--)
{
sal_Int32 oldCount = bparr.Count();
delete bparr[i];
auto p = bparr[i];
bparr.Remove(i);
delete p;
CPPUNIT_ASSERT_EQUAL_MESSAGE
(
@@ -309,8 +311,9 @@ public:
sal_Int32 oldCount = bparr.Count();
sal_Int32 oldElement = static_cast<BigPtrEntryMock*>(bparr[bparr.Count() / 2])->getCount();
delete bparr[bparr.Count() / 2];
auto p = bparr[bparr.Count() / 2];
bparr.Remove(bparr.Count() / 2);
delete p;
CPPUNIT_ASSERT_EQUAL_MESSAGE
(
@@ -346,11 +349,15 @@ public:
sal_Int32 nRemove = std::min<sal_Int32>(bparr.Count(), 3);
sal_Int32 oldCount = bparr.Count();
std::vector<BigPtrEntry*> vToDelete;
for (sal_Int32 i = 0; i < nRemove; i++)
delete bparr[i];
vToDelete.push_back(bparr[i]);
bparr.Remove(0, nRemove);
for (auto p : vToDelete)
delete p;
CPPUNIT_ASSERT_EQUAL_MESSAGE
(
"test_remove_multiple_elements_at_once failed",
@@ -371,9 +378,15 @@ public:
fillBigPtrArray(bparr, NUM_ENTRIES);
releaseBigPtrArrayContent(bparr);
std::vector<BigPtrEntry*> vToDelete;
for (sal_Int32 i = 0; i < bparr.Count(); i++)
vToDelete.push_back(bparr[i]);
bparr.Remove(0, bparr.Count());
for (auto p : vToDelete)
delete p;
CPPUNIT_ASSERT_EQUAL_MESSAGE
(
"test_remove_all_elements_at_once failed",
@@ -488,8 +501,9 @@ public:
for (sal_Int32 i = 0, j = NUM_ENTRIES - 1; i < NUM_ENTRIES; i++, j--)
{
delete bparr[i];
auto p = bparr[i];
bparr.Replace(i, new BigPtrEntryMock(j));
delete p;
}
for (sal_Int32 i = 0; i < NUM_ENTRIES; i++)
sw/source/core/bastyp/bparr.cxx
+28 -2
View File
@@ -75,7 +75,7 @@ void BigPtrArray::Move( sal_Int32 from, sal_Int32 to )
BlockInfo* p = m_ppInf[ cur ];
BigPtrEntry* pElem = p->mvData[ from - p->nStart ];
Insert( pElem, to ); // insert first, then delete!
Remove( ( to < from ) ? ( from + 1 ) : from );
ImplRemove( ( to < from ) ? ( from + 1 ) : from, 1, /*bClearElement*/false );
}
}
@@ -304,6 +304,11 @@ void BigPtrArray::Insert( BigPtrEntry* pElem, sal_Int32 pos )
}
void BigPtrArray::Remove( sal_Int32 pos, sal_Int32 n )
{
ImplRemove(pos, n, true);
}
void BigPtrArray::ImplRemove( sal_Int32 pos, sal_Int32 n, bool bClearElement )
{
CHECKIDX( m_ppInf.get(), m_nBlock, m_nSize, m_nCur );
@@ -320,6 +325,13 @@ void BigPtrArray::Remove( sal_Int32 pos, sal_Int32 n )
sal_uInt16 nel = p->nElem - sal_uInt16(pos);
if( sal_Int32(nel) > nElem )
nel = sal_uInt16(nElem);
// clear the back-pointers from the node back to node array. helps to flush out stale accesses.
if (bClearElement)
for(sal_uInt16 i=0; i < nel; ++i)
{
p->mvData[pos+i]->m_pBlock = nullptr;
p->mvData[pos+i]->m_nOffset = 0;
}
// move elements if needed
if( ( pos + nel ) < sal_Int32(p->nElem) )
{
@@ -388,12 +400,26 @@ void BigPtrArray::Remove( sal_Int32 pos, sal_Int32 n )
}
void BigPtrArray::Replace( sal_Int32 idx, BigPtrEntry* pElem)
{
ImplReplace(idx, pElem, /*bClearElement*/ true);
}
void BigPtrArray::ImplReplace( sal_Int32 idx, BigPtrEntry* pElem, bool bClearElement)
{
assert(idx < m_nSize); // Index out of bounds
m_nCur = Index2Block( idx );
BlockInfo* p = m_ppInf[ m_nCur ];
pElem->m_nOffset = sal_uInt16(idx - p->nStart);
pElem->m_pBlock = p;
// clear the back-pointers from the old element back to element array. helps to flush out stale accesses.
if (bClearElement)
{
p->mvData[idx - p->nStart]->m_pBlock = nullptr;
p->mvData[idx - p->nStart]->m_nOffset = 0;
}
// update with new element
p->mvData[ idx - p->nStart ] = pElem;
}
@@ -419,7 +445,7 @@ BigPtrEntry* BigPtrArray::ReplaceTheOneAfter( BigPtrEntry* pNotTheOne, BigPtrEnt
else
{
// slow path
BigPtrArray::Replace( pNotTheOne->GetPos()+1, pNewEntry );
BigPtrArray::ImplReplace( pNotTheOne->GetPos()+1, pNewEntry, /*bClearElement*/false );
}
// if the previous node is inside the current block
sw/source/core/fields/reffld.cxx
+4 -2
View File
@@ -1604,12 +1604,14 @@ SwTextNode* SwGetRefFieldType::FindAnchorRefStyleOther(SwDoc* pDoc,
// For references, styleref acts from the position of the reference not the field
// Happily, the previous code saves either one into pReference, so the following is generic for both
SwNodeOffset nReference = pReference->GetIndex();
const SwNodes& nodes = pDoc->GetNodes();
// It is possible to end up here, with a pReference pointer which points to a node which has already been
// removed from the nodes array, which means that calling GetIndex() returns an incorrect index.
if (nReference >= nodes.Count() || nodes[nReference] != pReference)
SwNodeOffset nReference;
if (!pReference->IsDisconnected())
nReference = pReference->GetIndex();
else
nReference = nodes.Count() - 1;
SwTextNode* pTextNd = nullptr;