[Libreoffice-commits] core.git: sc/qa sc/source
Dennis Francis (via logerrit)
logerrit at kemper.freedesktop.org
Tue Dec 17 08:55:41 UTC 2019
sc/qa/unit/data/xls/tdf129127.xls |binary
sc/qa/unit/filters-test.cxx | 33 +++++++++++++++++++++++++++++++++
sc/source/core/data/column4.cxx | 2 ++
3 files changed, 35 insertions(+)
New commits:
commit f86824cca0c442d371379d2ea0bff2241f18ab3d
Author: Dennis Francis <dennis.francis at collabora.com>
AuthorDate: Thu Dec 12 22:22:48 2019 +0530
Commit: Dennis Francis <dennis.francis at collabora.com>
CommitDate: Tue Dec 17 09:54:27 2019 +0100
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 at collabora.com>
diff --git a/sc/qa/unit/data/xls/tdf129127.xls b/sc/qa/unit/data/xls/tdf129127.xls
new file mode 100644
index 000000000000..4862d2319691
Binary files /dev/null and b/sc/qa/unit/data/xls/tdf129127.xls differ
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 3431f21ae80d..6bb39705079b 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);
}
More information about the Libreoffice-commits
mailing list