From ea134e08d70d156bdd17714d5f9ab9c44c91d4fa Mon Sep 17 00:00:00 2001
From: Adam Lesinski <adamlesinski@google.com>
Date: Thu, 13 Apr 2017 12:55:19 -0700
Subject: [PATCH] AAPT2: Fix order-of-destruction crash

Make sure that users of StringPool are destroyed before
the StringPool itself.

Test: valgrind aapt2 optimize -o opt.apk out/target/common/obj/APPS/framework-res_intermediates/package-export.apk
Change-Id: I140c2d32f8449028976795d5d6865d83e1409b53
---
 tools/aapt2/cmd/Link.cpp   | 4 ++--
 tools/aapt2/xml/XmlDom.cpp | 5 +++--
 tools/aapt2/xml/XmlDom.h   | 7 ++++++-
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp
index 6e0809e624505..9fb087c23574f 100644
--- a/tools/aapt2/cmd/Link.cpp
+++ b/tools/aapt2/cmd/Link.cpp
@@ -376,8 +376,8 @@ bool ResourceFileFlattener::LinkAndVersionXmlFile(ResourceTable* table, FileOper
         versioned_file_desc.config.sdkVersion = (uint16_t)sdk_level;
 
         FileOperation new_file_op;
-        new_file_op.xml_to_flatten =
-            util::make_unique<xml::XmlResource>(versioned_file_desc, doc->root->Clone());
+        new_file_op.xml_to_flatten = util::make_unique<xml::XmlResource>(
+            versioned_file_desc, StringPool{}, doc->root->Clone());
         new_file_op.config = versioned_file_desc.config;
         new_file_op.entry = file_op->entry;
         new_file_op.dst_path =
diff --git a/tools/aapt2/xml/XmlDom.cpp b/tools/aapt2/xml/XmlDom.cpp
index 4a278f632a4ae..60551901fe8df 100644
--- a/tools/aapt2/xml/XmlDom.cpp
+++ b/tools/aapt2/xml/XmlDom.cpp
@@ -224,7 +224,8 @@ std::unique_ptr<XmlResource> Inflate(std::istream* in, IDiagnostics* diag, const
 
   XML_ParserFree(parser);
   if (stack.root) {
-    return util::make_unique<XmlResource>(ResourceFile{{}, {}, source}, std::move(stack.root));
+    return util::make_unique<XmlResource>(ResourceFile{{}, {}, source}, StringPool{},
+                                          std::move(stack.root));
   }
   return {};
 }
@@ -357,7 +358,7 @@ std::unique_ptr<XmlResource> Inflate(const void* data, size_t data_len, IDiagnos
       }
     }
   }
-  return util::make_unique<XmlResource>(ResourceFile{}, std::move(root), std::move(string_pool));
+  return util::make_unique<XmlResource>(ResourceFile{}, std::move(string_pool), std::move(root));
 }
 
 std::unique_ptr<Node> Namespace::Clone() {
diff --git a/tools/aapt2/xml/XmlDom.h b/tools/aapt2/xml/XmlDom.h
index f1d0953c5494c..6950c307abf34 100644
--- a/tools/aapt2/xml/XmlDom.h
+++ b/tools/aapt2/xml/XmlDom.h
@@ -128,8 +128,13 @@ class Text : public BaseNode<Text> {
 class XmlResource {
  public:
   ResourceFile file;
-  std::unique_ptr<xml::Node> root;
+
+  // StringPool must come before the xml::Node. Destructors are called in reverse order, and
+  // the xml::Node may have StringPool references that need to be destroyed before the StringPool
+  // is destroyed.
   StringPool string_pool;
+
+  std::unique_ptr<xml::Node> root;
 };
 
 /**