[llvm-branch-commits] [llvm-branch] r318333 - Merging r310475:

Tom Stellard via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Wed Nov 15 12:48:56 PST 2017


Author: tstellar
Date: Wed Nov 15 12:48:55 2017
New Revision: 318333

URL: http://llvm.org/viewvc/llvm-project?rev=318333&view=rev
Log:
Merging r310475:

------------------------------------------------------------------------
r310475 | belleyb | 2017-08-09 06:47:01 -0700 (Wed, 09 Aug 2017) | 28 lines

[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
------------------------------------------------------------------------

Modified:
    llvm/branches/release_50/include/llvm/Support/FormatVariadic.h
    llvm/branches/release_50/unittests/Support/FormatVariadicTest.cpp

Modified: llvm/branches/release_50/include/llvm/Support/FormatVariadic.h
URL: http://llvm.org/viewvc/llvm-project/llvm/branches/release_50/include/llvm/Support/FormatVariadic.h?rev=318333&r1=318332&r2=318333&view=diff
==============================================================================
--- llvm/branches/release_50/include/llvm/Support/FormatVariadic.h (original)
+++ llvm/branches/release_50/include/llvm/Support/FormatVariadic.h Wed Nov 15 12:48:55 2017
@@ -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.

Modified: llvm/branches/release_50/unittests/Support/FormatVariadicTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/branches/release_50/unittests/Support/FormatVariadicTest.cpp?rev=318333&r1=318332&r2=318333&view=diff
==============================================================================
--- llvm/branches/release_50/unittests/Support/FormatVariadicTest.cpp (original)
+++ llvm/branches/release_50/unittests/Support/FormatVariadicTest.cpp Wed Nov 15 12:48:55 2017
@@ -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);




More information about the llvm-branch-commits mailing list