From bd826f9794903f64a63a38f5fbfa958d12e4426f Mon Sep 17 00:00:00 2001
From: Alastair Robertson <ajor@meta.com>
Date: Fri, 22 Sep 2023 08:35:12 -0700
Subject: [PATCH] CodeGen: Store list of ContainerInfos in unique_ptrs for
 reference stability

Lots of places rely on reference stability of ContainerInfo objects
(CodeGen's deduplication, Container nodes' containerInfo_ member).

In the key capture work, we need to be able to append to this list,
which would invalidate references before this change.
---
 oi/CodeGen.cpp               | 5 +++--
 oi/CodeGen.h                 | 3 ++-
 oi/type_graph/DrgnParser.cpp | 6 +++---
 oi/type_graph/DrgnParser.h   | 5 +++--
 test/test_drgn_parser.cpp    | 9 +++++----
 5 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/oi/CodeGen.cpp b/oi/CodeGen.cpp
index df7b26c..eda49a0 100644
--- a/oi/CodeGen.cpp
+++ b/oi/CodeGen.cpp
@@ -1061,8 +1061,9 @@ bool CodeGen::codegenFromDrgn(struct drgn_type* drgnType, std::string& code) {
 }
 
 void CodeGen::registerContainer(const fs::path& path) {
-  const auto& info = containerInfos_.emplace_back(path);
-  VLOG(1) << "Registered container: " << info.typeName;
+  auto info = std::make_unique<ContainerInfo>(path);
+  VLOG(1) << "Registered container: " << info->typeName;
+  containerInfos_.emplace_back(std::move(info));
 }
 
 void CodeGen::addDrgnRoot(struct drgn_type* drgnType, TypeGraph& typeGraph) {
diff --git a/oi/CodeGen.h b/oi/CodeGen.h
index 59c89bc..c6f2a5d 100644
--- a/oi/CodeGen.h
+++ b/oi/CodeGen.h
@@ -17,6 +17,7 @@
 
 #include <filesystem>
 #include <functional>
+#include <memory>
 #include <string>
 #include <unordered_map>
 #include <unordered_set>
@@ -65,7 +66,7 @@ class CodeGen {
  private:
   const OICodeGen::Config& config_;
   SymbolService& symbols_;
-  std::vector<ContainerInfo> containerInfos_;
+  std::vector<std::unique_ptr<ContainerInfo>> containerInfos_;
   std::unordered_set<const ContainerInfo*> definedContainers_;
   std::unordered_map<const type_graph::Class*, const type_graph::Member*>
       thriftIssetMembers_;
diff --git a/oi/type_graph/DrgnParser.cpp b/oi/type_graph/DrgnParser.cpp
index 9db935a..850fe5e 100644
--- a/oi/type_graph/DrgnParser.cpp
+++ b/oi/type_graph/DrgnParser.cpp
@@ -138,13 +138,13 @@ Container* DrgnParser::enumerateContainer(struct drgn_type* type,
   auto size = get_drgn_type_size(type);
 
   for (const auto& containerInfo : containers_) {
-    if (!std::regex_search(fqName, containerInfo.matcher)) {
+    if (!std::regex_search(fqName, containerInfo->matcher)) {
       continue;
     }
 
-    VLOG(2) << "Matching container `" << containerInfo.typeName << "` from `"
+    VLOG(2) << "Matching container `" << containerInfo->typeName << "` from `"
             << fqName << "`" << std::endl;
-    auto& c = makeType<Container>(type, containerInfo, size);
+    auto& c = makeType<Container>(type, *containerInfo, size);
     enumerateClassTemplateParams(type, c.templateParams);
     return &c;
   }
diff --git a/oi/type_graph/DrgnParser.h b/oi/type_graph/DrgnParser.h
index 2fab38a..9b00928 100644
--- a/oi/type_graph/DrgnParser.h
+++ b/oi/type_graph/DrgnParser.h
@@ -15,6 +15,7 @@
  */
 #pragma once
 
+#include <memory>
 #include <unordered_map>
 #include <vector>
 
@@ -53,7 +54,7 @@ struct DrgnParserOptions {
 class DrgnParser {
  public:
   DrgnParser(TypeGraph& typeGraph,
-             const std::vector<ContainerInfo>& containers,
+             const std::vector<std::unique_ptr<ContainerInfo>>& containers,
              DrgnParserOptions options)
       : typeGraph_(typeGraph), containers_(containers), options_(options) {
   }
@@ -97,7 +98,7 @@ class DrgnParser {
       drgn_types_;
 
   TypeGraph& typeGraph_;
-  const std::vector<ContainerInfo>& containers_;
+  const std::vector<std::unique_ptr<ContainerInfo>>& containers_;
   int depth_;
   DrgnParserOptions options_;
 };
diff --git a/test/test_drgn_parser.cpp b/test/test_drgn_parser.cpp
index 319e853..1371ab7 100644
--- a/test/test_drgn_parser.cpp
+++ b/test/test_drgn_parser.cpp
@@ -21,13 +21,14 @@ using ::testing::HasSubstr;
 SymbolService* DrgnParserTest::symbols_ = nullptr;
 
 namespace {
-const std::vector<ContainerInfo>& getContainerInfos() {
+const std::vector<std::unique_ptr<ContainerInfo>>& getContainerInfos() {
   static auto res = []() {
     // TODO more container types, with various template parameter options
-    ContainerInfo std_vector{"std::vector", SEQ_TYPE, "vector"};
-    std_vector.stubTemplateParams = {1};
+    auto std_vector =
+        std::make_unique<ContainerInfo>("std::vector", SEQ_TYPE, "vector");
+    std_vector->stubTemplateParams = {1};
 
-    std::vector<ContainerInfo> containers;
+    std::vector<std::unique_ptr<ContainerInfo>> containers;
     containers.emplace_back(std::move(std_vector));
     return containers;
   }();