AuroraMiddleware/scuffed-code
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

ICU-21356 Fix memory handling in MemoryPool::operator=()

Browse Source 
See #1437
This commit is contained in:
Hugo van der Merwe 2020-10-28 15:45:52 +00:00
parent 21dde41f9e
commit 7c8f857da8
3 changed files with 101 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
icu4c/source/common/cmemory.h
View File

@ -725,9 +725,14 @@ public:
}
MemoryPool& operator=(MemoryPool&& other) U_NOEXCEPT {
fCount = other.fCount;
fPool = std::move(other.fPool);
other.fCount = 0;
// Since `this` may contain instances that need to be deleted, we can't
// just throw them away and replace them with `other`. The normal way of
// dealing with this in C++ is to swap `this` and `other`, rather than
// simply overwrite: the destruction of `other` can then take care of
// running MemoryPool::~MemoryPool() over the still-to-be-deallocated
// instances.
std::swap(fCount, other.fCount);
std::swap(fPool, other.fPool);
return *this;
}
@ -796,9 +801,6 @@ protected:
template<typename T, int32_t stackCapacity = 8>
class MaybeStackVector : protected MemoryPool<T, stackCapacity> {
public:
using MemoryPool<T, stackCapacity>::MemoryPool;
using MemoryPool<T, stackCapacity>::operator=;
template<typename... Args>
T* emplaceBack(Args&&... args) {
return this->create(args...);

57
icu4c/source/test/intltest/measfmttest.cpp
View File

@ -17,6 +17,9 @@
#if !UCONFIG_NO_FORMATTING
#include "charstr.h"
#include "cstr.h"
#include "measunit_impl.h"
#include "unicode/decimfmt.h"
#include "unicode/measfmt.h"
#include "unicode/measure.h"
@ -25,8 +28,6 @@
#include "unicode/tmunit.h"
#include "unicode/plurrule.h"
#include "unicode/ustring.h"
#include "charstr.h"
#include "cstr.h"
#include "unicode/reldatefmt.h"
#include "unicode/rbnf.h"
@ -88,6 +89,7 @@ private:
void TestDimensionlessBehaviour();
void Test21060_AddressSanitizerProblem();
void Test21223_FrenchDuration();
void TestInternalMeasureUnitImpl();
void verifyFormat(
const char *description,
@ -216,6 +218,7 @@ void MeasureFormatTest::runIndexedTest(
TESTCASE_AUTO(TestDimensionlessBehaviour);
TESTCASE_AUTO(Test21060_AddressSanitizerProblem);
TESTCASE_AUTO(Test21223_FrenchDuration);
TESTCASE_AUTO(TestInternalMeasureUnitImpl);
TESTCASE_AUTO_END;
}
@ -4037,6 +4040,56 @@ void MeasureFormatTest::Test21223_FrenchDuration() {
// }
}
void MeasureFormatTest::TestInternalMeasureUnitImpl() {
IcuTestErrorCode status(*this, "TestInternalMeasureUnitImpl");
MeasureUnitImpl mu1 = MeasureUnitImpl::forIdentifier("meter", status);
status.assertSuccess();
assertEquals("mu1 initial identifier", "", mu1.identifier.data());
assertEquals("mu1 initial complexity", UMEASURE_UNIT_SINGLE, mu1.complexity);
assertEquals("mu1 initial units length", 1, mu1.units.length());
assertEquals("mu1 initial units[0]", "meter", mu1.units[0]->getSimpleUnitID());
// Producing identifier via build(): the std::move() means mu1 gets modified
// while it also gets assigned to tmp's internal fImpl.
MeasureUnit tmp = std::move(mu1).build(status);
status.assertSuccess();
assertEquals("mu1 post-move-build identifier", "meter", mu1.identifier.data());
assertEquals("mu1 post-move-build complexity", UMEASURE_UNIT_SINGLE, mu1.complexity);
assertEquals("mu1 post-move-build units length", 1, mu1.units.length());
assertEquals("mu1 post-move-build units[0]", "meter", mu1.units[0]->getSimpleUnitID());
assertEquals("MeasureUnit tmp identifier", "meter", tmp.getIdentifier());
// This temporary variable is used when forMeasureUnit's first parameter
// lacks an fImpl instance:
MeasureUnitImpl tmpMemory;
const MeasureUnitImpl &tmpImplRef = MeasureUnitImpl::forMeasureUnit(tmp, tmpMemory, status);
status.assertSuccess();
assertEquals("tmpMemory identifier", "", tmpMemory.identifier.data());
assertEquals("tmpMemory complexity", UMEASURE_UNIT_SINGLE, tmpMemory.complexity);
assertEquals("tmpMemory units length", 1, tmpMemory.units.length());
assertEquals("tmpMemory units[0]", "meter", tmpMemory.units[0]->getSimpleUnitID());
assertEquals("tmpImplRef identifier", "", tmpImplRef.identifier.data());
assertEquals("tmpImplRef complexity", UMEASURE_UNIT_SINGLE, tmpImplRef.complexity);
MeasureUnitImpl mu2 = MeasureUnitImpl::forIdentifier("newton-meter", status);
status.assertSuccess();
mu1 = std::move(mu2);
assertEquals("mu1 = move(mu2): identifier", "", mu1.identifier.data());
assertEquals("mu1 = move(mu2): complexity", UMEASURE_UNIT_COMPOUND, mu1.complexity);
assertEquals("mu1 = move(mu2): units length", 2, mu1.units.length());
assertEquals("mu1 = move(mu2): units[0]", "newton", mu1.units[0]->getSimpleUnitID());
assertEquals("mu1 = move(mu2): units[1]", "meter", mu1.units[1]->getSimpleUnitID());
mu1 = MeasureUnitImpl::forIdentifier("hour-and-minute-and-second", status);
status.assertSuccess();
assertEquals("mu1 = HMS: identifier", "", mu1.identifier.data());
assertEquals("mu1 = HMS: complexity", UMEASURE_UNIT_MIXED, mu1.complexity);
assertEquals("mu1 = HMS: units length", 3, mu1.units.length());
assertEquals("mu1 = HMS: units[0]", "hour", mu1.units[0]->getSimpleUnitID());
assertEquals("mu1 = HMS: units[1]", "minute", mu1.units[1]->getSimpleUnitID());
assertEquals("mu1 = HMS: units[2]", "second", mu1.units[2]->getSimpleUnitID());
}
void MeasureFormatTest::verifyFieldPosition(
const char *description,

77
icu4c/source/test/intltest/units_test.cpp
View File

@ -415,26 +415,25 @@ void UnitsTest::testComplexUnitsConverter() {
assertEquals("1.9999: measures[0] value", 1.0, measures[0]->getNumber().getDouble(status));
assertEquals("1.9999: measures[0] unit", MeasureUnit::getFoot().getIdentifier(),
measures[0]->getUnit().getIdentifier());
assertEqualsNear("1.9999: measures[1] value", 11.9988, measures[1]->getNumber().getDouble(status), 0.0001);
assertEqualsNear("1.9999: measures[1] value", 11.9988,
measures[1]->getNumber().getDouble(status), 0.0001);
assertEquals("1.9999: measures[1] unit", MeasureUnit::getInch().getIdentifier(),
measures[1]->getUnit().getIdentifier());
}
// TODO(icu-units#100): consider factoring out the set of tests to make this function more
// data-driven, *after* dealing appropriately with the memory leaks that can
// be demonstrated by this code.
// data-driven.
// TODO(icu-units#100): reusing measures results in a leak.
// A minimal nudge under 2.0.
MaybeStackVector<Measure> measures2 = converter.convert((2.0 - DBL_EPSILON), nullptr, status);
assertEquals("measures length", 2, measures2.length());
if (2 == measures2.length()) {
assertEquals("1 - eps: measures[0] value", 2.0, measures2[0]->getNumber().getDouble(status));
measures = converter.convert((2.0 - DBL_EPSILON), nullptr, status);
assertEquals("measures length", 2, measures.length());
if (2 == measures.length()) {
assertEquals("1 - eps: measures[0] value", 2.0, measures[0]->getNumber().getDouble(status));
assertEquals("1 - eps: measures[0] unit", MeasureUnit::getFoot().getIdentifier(),
measures2[0]->getUnit().getIdentifier());
assertEquals("1 - eps: measures[1] value", 0.0, measures2[1]->getNumber().getDouble(status));
measures[0]->getUnit().getIdentifier());
assertEquals("1 - eps: measures[1] value", 0.0, measures[1]->getNumber().getDouble(status));
assertEquals("1 - eps: measures[1] unit", MeasureUnit::getInch().getIdentifier(),
measures2[1]->getUnit().getIdentifier());
measures[1]->getUnit().getIdentifier());
}
// Testing precision with meter and light-year. 1e-16 light years is
@ -444,51 +443,51 @@ void UnitsTest::testComplexUnitsConverter() {
// An epsilon's nudge under one light-year: should give 1 ly, 0 m.
input = MeasureUnit::getLightYear();
output = MeasureUnit::forIdentifier("light-year-and-meter", status);
// TODO(icu-units#100): reusing tempInput and tempOutput results in a leak.
MeasureUnitImpl tempInput3, tempOutput3;
const MeasureUnitImpl &inputImpl3 = MeasureUnitImpl::forMeasureUnit(input, tempInput3, status);
const MeasureUnitImpl &outputImpl3 = MeasureUnitImpl::forMeasureUnit(output, tempOutput3, status);
// TODO(icu-units#100): reusing converter results in a leak.
ComplexUnitsConverter converter3 = ComplexUnitsConverter(inputImpl3, outputImpl3, rates, status);
// TODO(icu-units#100): reusing measures results in a leak.
MaybeStackVector<Measure> measures3 = converter3.convert((2.0 - DBL_EPSILON), nullptr, status);
assertEquals("measures length", 2, measures3.length());
if (2 == measures3.length()) {
assertEquals("light-year test: measures[0] value", 2.0, measures3[0]->getNumber().getDouble(status));
const MeasureUnitImpl &inputImpl3 = MeasureUnitImpl::forMeasureUnit(input, tempInput, status);
const MeasureUnitImpl &outputImpl3 = MeasureUnitImpl::forMeasureUnit(output, tempOutput, status);
converter = ComplexUnitsConverter(inputImpl3, outputImpl3, rates, status);
measures = converter.convert((2.0 - DBL_EPSILON), nullptr, status);
assertEquals("measures length", 2, measures.length());
if (2 == measures.length()) {
assertEquals("light-year test: measures[0] value", 2.0,
measures[0]->getNumber().getDouble(status));
assertEquals("light-year test: measures[0] unit", MeasureUnit::getLightYear().getIdentifier(),
measures3[0]->getUnit().getIdentifier());
assertEquals("light-year test: measures[1] value", 0.0, measures3[1]->getNumber().getDouble(status));
measures[0]->getUnit().getIdentifier());
assertEquals("light-year test: measures[1] value", 0.0,
measures[1]->getNumber().getDouble(status));
assertEquals("light-year test: measures[1] unit", MeasureUnit::getMeter().getIdentifier(),
measures3[1]->getUnit().getIdentifier());
measures[1]->getUnit().getIdentifier());
}
// 1e-15 light years is 9.46073 meters (calculated using "bc" and the CLDR
// conversion factor). With double-precision maths, we get 10.5. In this
// case, we're off by almost 1 meter.
MaybeStackVector<Measure> measures4 = converter3.convert((1.0 + 1e-15), nullptr, status);
assertEquals("measures length", 2, measures4.length());
if (2 == measures4.length()) {
assertEquals("light-year test: measures[0] value", 1.0, measures4[0]->getNumber().getDouble(status));
measures = converter.convert((1.0 + 1e-15), nullptr, status);
assertEquals("measures length", 2, measures.length());
if (2 == measures.length()) {
assertEquals("light-year test: measures[0] value", 1.0,
measures[0]->getNumber().getDouble(status));
assertEquals("light-year test: measures[0] unit", MeasureUnit::getLightYear().getIdentifier(),
measures4[0]->getUnit().getIdentifier());
measures[0]->getUnit().getIdentifier());
assertEqualsNear("light-year test: measures[1] value", 10,
measures4[1]->getNumber().getDouble(status), 1);
measures[1]->getNumber().getDouble(status), 1);
assertEquals("light-year test: measures[1] unit", MeasureUnit::getMeter().getIdentifier(),
measures4[1]->getUnit().getIdentifier());
measures[1]->getUnit().getIdentifier());
}
// 2e-16 light years is 1.892146 meters. We consider this in the noise, and
// thus expect a 0. (This test fails when 2e-16 is increased to 4e-16.)
MaybeStackVector<Measure> measures5 = converter3.convert((1.0 + 2e-16), nullptr, status);
assertEquals("measures length", 2, measures5.length());
if (2 == measures5.length()) {
assertEquals("light-year test: measures[0] value", 1.0, measures5[0]->getNumber().getDouble(status));
measures = converter.convert((1.0 + 2e-16), nullptr, status);
assertEquals("measures length", 2, measures.length());
if (2 == measures.length()) {
assertEquals("light-year test: measures[0] value", 1.0,
measures[0]->getNumber().getDouble(status));
assertEquals("light-year test: measures[0] unit", MeasureUnit::getLightYear().getIdentifier(),
measures5[0]->getUnit().getIdentifier());
measures[0]->getUnit().getIdentifier());
assertEquals("light-year test: measures[1] value", 0.0,
measures5[1]->getNumber().getDouble(status));
measures[1]->getNumber().getDouble(status));
assertEquals("light-year test: measures[1] unit", MeasureUnit::getMeter().getIdentifier(),
measures5[1]->getUnit().getIdentifier());
measures[1]->getUnit().getIdentifier());
}
// TODO(icu-units#63): test negative numbers!