forked from OSchip/llvm-project
[Support] PR33388 - Fix formatv_object move constructor
formatv_object currently uses the implicitly defined move constructor, but it is buggy. In typical use-cases, the problem doesn't show-up because all calls to the move constructor are elided. Thus, the buggy constructors are never invoked. The issue especially shows-up when code is compiled using the -fno-elide-constructors compiler flag. For instance, this is useful when attempting to collect accurate code coverage statistics. The exact issue is the following: The Parameters data member is correctly moved, thus making the parameters occupy a new memory location in the target object. Unfortunately, the default copying of the Adapters blindly copies the vector of pointers, leaving each of these pointers referencing the parameters in the original object instead of the copied one. These pointers quickly become dangling when the original object is deleted. This quickly leads to crashes. The solution is to update the Adapters pointers when performing a move. The copy constructor isn't useful for format objects and can thus be deleted. This resolves PR33388. Differential Revision: https://reviews.llvm.org/D34463 llvm-svn: 310475
This commit is contained in:
parent
6110d3ad00
commit
d9017cc65e
|
@ -94,6 +94,15 @@ public:
|
|||
Adapters.reserve(ParamCount);
|
||||
}
|
||||
|
||||
formatv_object_base(formatv_object_base const &rhs) = delete;
|
||||
|
||||
formatv_object_base(formatv_object_base &&rhs)
|
||||
: Fmt(std::move(rhs.Fmt)),
|
||||
Adapters(), // Adapters are initialized by formatv_object
|
||||
Replacements(std::move(rhs.Replacements)) {
|
||||
Adapters.reserve(rhs.Adapters.size());
|
||||
};
|
||||
|
||||
void format(raw_ostream &S) const {
|
||||
for (auto &R : Replacements) {
|
||||
if (R.Type == ReplacementType::Empty)
|
||||
|
@ -149,6 +158,14 @@ public:
|
|||
Parameters(std::move(Params)) {
|
||||
Adapters = apply_tuple(create_adapters(), Parameters);
|
||||
}
|
||||
|
||||
formatv_object(formatv_object const &rhs) = delete;
|
||||
|
||||
formatv_object(formatv_object &&rhs)
|
||||
: formatv_object_base(std::move(rhs)),
|
||||
Parameters(std::move(rhs.Parameters)) {
|
||||
Adapters = apply_tuple(create_adapters(), Parameters);
|
||||
}
|
||||
};
|
||||
|
||||
// \brief Format text given a format string and replacement parameters.
|
||||
|
|
|
@ -553,6 +553,12 @@ TEST(FormatVariadicTest, Adapter) {
|
|||
formatv("{0,=34:X-}", fmt_repeat(fmt_pad(N, 1, 3), 5)).str());
|
||||
}
|
||||
|
||||
TEST(FormatVariadicTest, MoveConstructor) {
|
||||
auto fmt = formatv("{0} {1}", 1, 2);
|
||||
auto fmt2 = std::move(fmt);
|
||||
std::string S = fmt2;
|
||||
EXPECT_EQ("1 2", S);
|
||||
}
|
||||
TEST(FormatVariadicTest, ImplicitConversions) {
|
||||
std::string S = formatv("{0} {1}", 1, 2);
|
||||
EXPECT_EQ("1 2", S);
|
||||
|
|
Loading…
Reference in New Issue