diff options
author | Noel Grandin <noel.grandin@collabora.co.uk> | 2020-06-10 15:27:48 +0200 |
---|---|---|
committer | Noel Grandin <noel.grandin@collabora.co.uk> | 2020-06-12 08:50:29 +0200 |
commit | 5af636c5021ecf7fba8f5f34cc6af929f1e04b4c (patch) | |
tree | 353cb10cd0703501bcfa967d04f318c65aa7045c /svl/qa | |
parent | 26483950760f0aac7bc45e93db4127f66a98fdc6 (diff) |
fix ASAN in SharedStringPool
regression from
commit 3581f1d71ae0d431ba28c0f3b7b263ff6212ce7b
optimize SharedStringPool::purge() and fix tests
which results in us potentially de-referencing an already de-allocated
OUString object in the first loop in purge().
So switch to a different strategy, which only needs one data structure,
instead of two.
Change-Id: Iaac6beda48459643afdb7b14ce7d39d68a93339c
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/95226
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Diffstat (limited to 'svl/qa')
-rw-r--r-- | svl/qa/unit/svl.cxx | 25 |
1 files changed, 20 insertions, 5 deletions
diff --git a/svl/qa/unit/svl.cxx b/svl/qa/unit/svl.cxx index 194dc550c278..c32017f7aea6 100644 --- a/svl/qa/unit/svl.cxx +++ b/svl/qa/unit/svl.cxx @@ -60,6 +60,7 @@ public: void testSharedString(); void testSharedStringPool(); void testSharedStringPoolPurge(); + void testSharedStringPoolPurgeBug1(); void testFdo60915(); void testI116701(); void testTdf103060(); @@ -77,6 +78,7 @@ public: CPPUNIT_TEST(testSharedString); CPPUNIT_TEST(testSharedStringPool); CPPUNIT_TEST(testSharedStringPoolPurge); + CPPUNIT_TEST(testSharedStringPoolPurgeBug1); CPPUNIT_TEST(testFdo60915); CPPUNIT_TEST(testI116701); CPPUNIT_TEST(testTdf103060); @@ -376,30 +378,30 @@ void Test::testSharedStringPoolPurge() std::optional<svl::SharedString> pStr3 = aPool.intern("ANDY"); std::optional<svl::SharedString> pStr4 = aPool.intern("Bruce"); - CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(4), aPool.getCount()); + CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(5), aPool.getCount()); CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2), aPool.getCountIgnoreCase()); // This shouldn't purge anything. aPool.purge(); - CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(4), aPool.getCount()); + CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(5), aPool.getCount()); CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2), aPool.getCountIgnoreCase()); // Delete one heap string object, and purge. That should purge one string. pStr1.reset(); aPool.purge(); - CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(3), aPool.getCount()); + CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(4), aPool.getCount()); CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2), aPool.getCountIgnoreCase()); // Nothing changes, because the upper-string is still in the map pStr3.reset(); aPool.purge(); - CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(3), aPool.getCount()); + CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(4), aPool.getCount()); CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2), aPool.getCountIgnoreCase()); // Again. pStr2.reset(); aPool.purge(); - CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), aPool.getCount()); + CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2), aPool.getCount()); CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), aPool.getCountIgnoreCase()); // Delete 'Bruce' and purge. @@ -409,6 +411,19 @@ void Test::testSharedStringPoolPurge() CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(0), aPool.getCountIgnoreCase()); } +void Test::testSharedStringPoolPurgeBug1() +{ + // We had a bug where, if we had two strings that mapped to the same uppercase string, + // purge() would de-reference a dangling pointer and consequently cause an ASAN failure. + SvtSysLocale aSysLocale; + svl::SharedStringPool aPool(*aSysLocale.GetCharClassPtr()); + aPool.intern("Andy"); + aPool.intern("andy"); + aPool.purge(); + CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(0), aPool.getCount()); + CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(0), aPool.getCountIgnoreCase()); +} + void Test::checkPreviewString(SvNumberFormatter& aFormatter, const OUString& sCode, double fPreviewNumber, |