diff options
author | Dennis Francis <dennis.francis@collabora.com> | 2019-12-12 22:22:48 +0530 |
---|---|---|
committer | Eike Rathke <erack@redhat.com> | 2019-12-18 01:14:01 +0100 |
commit | c8079949bcccb20caf7fc91ee4567d811606c8e4 (patch) | |
tree | af1d5f8cec2693fee7433fc3611d46320346a6e0 /sc | |
parent | 580f2b93a9c754743341e5e2f40084bceb8a1680 (diff) |
Resolves tdf#129127: do a safe swap of patterns
Following swap of patterns is not safe :
"""
const ScPatternAttr* pPat1 = GetPattern(nRow);
const ScPatternAttr* pPat2 = rOther.GetPattern(nRow);
SetPattern(nRow, *pPat2);
rOther.SetPattern(nRow, *pPat1);
"""
as the first SetPattern can cause deallocation of the pattern object
pointed to by pPat1 if it had a reference count of 1 to begin with.
In such cases, increase the reference count of first pattern by
putting it into document pool, thereby evading deallocation.
The stacktrace of the crash without the fix looks like :
Thread 1 "soffice.bin" received signal SIGSEGV, Segmentation fault.
602 SfxPoolItem *pPoolItem = rItem.Clone(pImpl->mpMaster);
0 0x00007ffff4ad2905 in SfxItemPool::PutImpl(SfxPoolItem const&, unsigned short, bool) (this=0x16c6830, rItem=..., nWhich=39321, bPassingOwnership=false) at /ssd1/work/dennis/core/svl/source/items/itempool.cxx:602
1 0x00007fffd179e5a5 in ScDocumentPool::PutImpl(SfxPoolItem const&, unsigned short, bool) (this=0x16c6830, rItem=..., nWhich=0, bPassingOwnership=false) at /ssd1/work/dennis/core/sc/source/core/data/docpool.cxx:333
2 0x00007fffd14e88fb in SfxItemPool::Put<ScPatternAttr>(ScPatternAttr const&, unsigned short) (this=0x16c6830, rItem=..., nWhich=0) at /ssd1/work/dennis/core/include/svl/itempool.hxx:154
3 0x00007fffd14dd228 in ScAttrArray::SetPatternAreaImpl(int, int, ScPatternAttr const*, bool, ScEditDataArray*, bool)
(this=0x2cbc020, nStartRow=9, nEndRow=9, pPattern=0x2da5d80, bPutToPool=true, pDataArray=0x0, bPassingOwnership=false) at /ssd1/work/dennis/core/sc/source/core/data/attarray.cxx:464
4 0x00007fffd1653fcd in ScAttrArray::SetPattern(int, ScPatternAttr const*, bool) (this=0x2cbc020, nRow=9, pPattern=0x2da5d80, bPutToPool=true) at /ssd1/work/dennis/core/sc/inc/attarray.hxx:142
5 0x00007fffd163cd41 in ScColumn::SetPattern(int, ScPatternAttr const&) (this=0x2cb4610, nRow=9, rPatAttr=...) at /ssd1/work/dennis/core/sc/source/core/data/column.cxx:694
6 0x00007fffd170e65b in ScColumn::Swap(ScColumn&, int, int, bool) (this=0x2339270, rOther=..., nRow1=0, nRow2=9, bPattern=true) at /ssd1/work/dennis/core/sc/source/core/data/column4.cxx:1112
7 0x00007fffd1ada654 in ScTable::SortReorderByColumn(ScSortInfoArray const*, int, int, bool, ScProgress*) (this=0x2cb9ea0, pArray=0x5ef71a0, nRow1=0, nRow2=9, bPattern=true, pProgress=0x7fffffff09a0)
at /ssd1/work/dennis/core/sc/source/core/data/table3.cxx:922
8 0x00007fffd1adf991 in ScTable::Sort(ScSortParam const&, bool, bool, ScProgress*, sc::ReorderParam*) (this=0x2cb9ea0, rSortParam=..., bKeepQuery=false, bUpdateRefs=false, pProgress=0x7fffffff09a0, pUndo=0x7fffffff0950)
at /ssd1/work/dennis/core/sc/source/core/data/table3.cxx:1750
9 0x00007fffd17cc70a in ScDocument::Sort(short, ScSortParam const&, bool, bool, ScProgress*, sc::ReorderParam*) (
this=0x2cc4db0, nTab=0, rSortParam=..., bKeepQuery=false, bUpdateRefs=false, pProgress=0x7fffffff09a0, pUndo=0x7fffffff0950) at /ssd1/work/dennis/core/sc/source/core/data/documen3.cxx:1411
10 0x00007fffd22a2f76 in ScDBDocFunc::Sort(short, ScSortParam const&, bool, bool, bool) (this=0x7fffffff0b50, nTab=0, rSortParam=..., bRecord=true, bPaint=true, bApi=false)
at /ssd1/work/dennis/core/sc/source/ui/docshell/dbdocfun.cxx:578
11 0x00007fffd2744e29 in ScDBFunc::Sort(ScSortParam const&, bool, bool) (this=0x2e3b560, rSortParam=..., bRecord=true, bPaint=true) at /ssd1/work/dennis/core/sc/source/ui/view/dbfunc.cxx:217
12 0x00007fffd2744d7b in ScDBFunc::UISort(ScSortParam const&) (this=0x2e3b560, rSortParam=...) at /ssd1/work/dennis/core/sc/source/ui/view/dbfunc.cxx:208
13 0x00007fffd2735e3d in ScCellShell::ExecuteDB(SfxRequest&) (this=0x1e42330, rReq=...) at /ssd1/work/dennis/core/sc/source/ui/view/cellsh2.cxx:528
Change-Id: I70f8b95a6ff59f372b909fd173117a114906deff
Reviewed-on: https://gerrit.libreoffice.org/85072
Tested-by: Jenkins
Reviewed-by: Dennis Francis <dennis.francis@collabora.com>
(cherry picked from commit f86824cca0c442d371379d2ea0bff2241f18ab3d)
Reviewed-on: https://gerrit.libreoffice.org/85279
Reviewed-by: Eike Rathke <erack@redhat.com>
Diffstat (limited to 'sc')
-rw-r--r-- | sc/qa/unit/data/xls/tdf129127.xls | bin | 0 -> 6656 bytes | |||
-rw-r--r-- | sc/qa/unit/filters-test.cxx | 33 | ||||
-rw-r--r-- | sc/source/core/data/column4.cxx | 2 |
3 files changed, 35 insertions, 0 deletions
diff --git a/sc/qa/unit/data/xls/tdf129127.xls b/sc/qa/unit/data/xls/tdf129127.xls Binary files differnew file mode 100644 index 000000000000..4862d2319691 --- /dev/null +++ b/sc/qa/unit/data/xls/tdf129127.xls diff --git a/sc/qa/unit/filters-test.cxx b/sc/qa/unit/filters-test.cxx index 4551ee4d9fb7..dc097180cbbd 100644 --- a/sc/qa/unit/filters-test.cxx +++ b/sc/qa/unit/filters-test.cxx @@ -77,6 +77,7 @@ public: void testSortWithSheetExternalReferencesODS(); void testSortWithSheetExternalReferencesODS_Impl( ScDocShellRef const & xDocShRef, SCROW nRow1, SCROW nRow2, bool bCheckRelativeInSheet ); + void testSortWithFormattingXLS(); CPPUNIT_TEST_SUITE(ScFiltersTest); CPPUNIT_TEST(testCVEs); @@ -100,6 +101,7 @@ public: CPPUNIT_TEST(testEnhancedProtectionXLSX); CPPUNIT_TEST(testSortWithSharedFormulasODS); CPPUNIT_TEST(testSortWithSheetExternalReferencesODS); + CPPUNIT_TEST(testSortWithFormattingXLS); CPPUNIT_TEST_SUITE_END(); @@ -792,6 +794,37 @@ void ScFiltersTest::testSortWithSheetExternalReferencesODS_Impl( ScDocShellRef c } } +void ScFiltersTest::testSortWithFormattingXLS() +{ + ScDocShellRef xDocSh = loadDoc("tdf129127.", FORMAT_XLS, true); + CPPUNIT_ASSERT(xDocSh.is()); + ScDocument& rDoc = xDocSh->GetDocument(); + + // Set as an anonymous database range to sort. + std::unique_ptr<ScDBData> pDBData( + new ScDBData(STR_DB_LOCAL_NONAME, 0, 0, 0, 4, 9, false, false)); + rDoc.SetAnonymousDBData(0, std::move(pDBData)); + + // Sort ascending by Row 1 + ScSortParam aSortData; + aSortData.nCol1 = 0; + aSortData.nCol2 = 4; + aSortData.nRow1 = 0; + aSortData.nRow2 = 9; + aSortData.bHasHeader = false; + aSortData.bByRow = false; + aSortData.maKeyState[0].bDoSort = true; + aSortData.maKeyState[0].nField = 0; + aSortData.maKeyState[0].bAscending = true; + + // Do the sorting. + ScDBDocFunc aFunc(*xDocSh); + // Without the fix, sort would crash. + bool bSorted = aFunc.Sort(0, aSortData, true, true, true); + CPPUNIT_ASSERT(bSorted); + xDocSh->DoClose(); +} + ScFiltersTest::ScFiltersTest() : ScBootstrapFixture( "sc/qa/unit/data" ) , mbUpdateReferenceOnSort(false) diff --git a/sc/source/core/data/column4.cxx b/sc/source/core/data/column4.cxx index 8e0ae07bbef1..329fd93eecdb 100644 --- a/sc/source/core/data/column4.cxx +++ b/sc/source/core/data/column4.cxx @@ -1108,6 +1108,8 @@ void ScColumn::Swap( ScColumn& rOther, SCROW nRow1, SCROW nRow2, bool bPattern ) const ScPatternAttr* pPat2 = rOther.GetPattern(nRow); if (pPat1 != pPat2) { + if (pPat1->GetRefCount() == 1) + pPat1 = &rOther.GetDoc()->GetPool()->Put(*pPat1); SetPattern(nRow, *pPat2); rOther.SetPattern(nRow, *pPat1); } |