TypeGraph: Stop identifying containers in DrgnParser

Leave it to the new mutator pass IdentifyContainers to replace Class
nodes with Container nodes where appropriate.

This will allow us to run passes over the type graph before identifying
containers, and therefore before we have lost information about the
internal details of the container (e.g. alignment of member variables).

oi/CodeGen.cpp

@@ -32,6 +32,7 @@
 #include "type_graph/DrgnParser.h"
 #include "type_graph/EnforceCompatibility.h"
 #include "type_graph/Flattener.h"
+#include "type_graph/IdentifyContainers.h"
 #include "type_graph/KeyCapture.h"
 #include "type_graph/NameGen.h"
 #include "type_graph/Prune.h"
@@ -55,6 +56,7 @@ using type_graph::DrgnParserOptions;
 using type_graph::EnforceCompatibility;
 using type_graph::Enum;
 using type_graph::Flattener;
+using type_graph::IdentifyContainers;
 using type_graph::KeyCapture;
 using type_graph::Member;
 using type_graph::NameGen;
@@ -1137,21 +1139,25 @@ bool CodeGen::codegenFromDrgn(struct drgn_type* drgnType, std::string& code) {
   return true;
 }
 
+void CodeGen::registerContainer(std::unique_ptr<ContainerInfo> info) {
+  VLOG(1) << "Registered container: " << info->typeName;
+  containerInfos_.emplace_back(std::move(info));
+}
+
 void CodeGen::registerContainer(const fs::path& path) {
   auto info = std::make_unique<ContainerInfo>(path);
   if (info->requiredFeatures != (config_.features & info->requiredFeatures)) {
     VLOG(1) << "Skipping container (feature conflict): " << info->typeName;
     return;
   }
-  VLOG(1) << "Registered container: " << info->typeName;
+  registerContainer(std::move(info));
-  containerInfos_.emplace_back(std::move(info));
 }
 
 void CodeGen::addDrgnRoot(struct drgn_type* drgnType, TypeGraph& typeGraph) {
   DrgnParserOptions options{
       .chaseRawPointers = config_.features[Feature::ChaseRawPointers],
   };
-  DrgnParser drgnParser{typeGraph, containerInfos_, options};
+  DrgnParser drgnParser{typeGraph, options};
   Type& parsedRoot = drgnParser.parse(drgnType);
   typeGraph.addRoot(parsedRoot);
 }
@@ -1162,6 +1168,7 @@ void CodeGen::transform(TypeGraph& typeGraph) {
   // Simplify the type graph first so there is less work for later passes
   pm.addPass(RemoveTopLevelPointer::createPass());
   pm.addPass(Flattener::createPass());
+  pm.addPass(IdentifyContainers::createPass(containerInfos_));
   pm.addPass(TypeIdentifier::createPass(config_.passThroughTypes));
   if (config_.features[Feature::PruneTypeGraph])
     pm.addPass(Prune::createPass());
@@ -1171,11 +1178,12 @@ void CodeGen::transform(TypeGraph& typeGraph) {
     DrgnParserOptions options{
         .chaseRawPointers = config_.features[Feature::ChaseRawPointers],
     };
-    DrgnParser drgnParser{typeGraph, containerInfos_, options};
+    DrgnParser drgnParser{typeGraph, options};
     pm.addPass(AddChildren::createPass(drgnParser, symbols_));
 
     // Re-run passes over newly added children
     pm.addPass(Flattener::createPass());
+    pm.addPass(IdentifyContainers::createPass(containerInfos_));
     pm.addPass(TypeIdentifier::createPass(config_.passThroughTypes));
     if (config_.features[Feature::PruneTypeGraph])
       pm.addPass(Prune::createPass());

oi/CodeGen.h

@@ -53,6 +53,7 @@ class CodeGen {
                        std::string linkageName,
                        std::string& code);
 
+  void registerContainer(std::unique_ptr<ContainerInfo> containerInfo);
   void registerContainer(const std::filesystem::path& path);
   void addDrgnRoot(struct drgn_type* drgnType,
                    type_graph::TypeGraph& typeGraph);

oi/type_graph/DrgnParser.cpp

@@ -25,8 +25,6 @@ extern "C" {
 #include <drgn.h>
 }
 
-#include <regex>
-
 namespace oi::detail::type_graph {
 namespace {
 
@@ -148,31 +146,7 @@ Type& DrgnParser::enumerateType(struct drgn_type* type) {
   return *t;
 }
 
-/*
+Class& DrgnParser::enumerateClass(struct drgn_type* type) {
- * enumerateContainer
- *
- * Attempts to parse a drgn_type as a Container. Returns nullptr if not
- * sucessful.
- */
-Container* DrgnParser::enumerateContainer(struct drgn_type* type,
-                                          const std::string& fqName) {
-  auto size = get_drgn_type_size(type);
-
-  for (const auto& containerInfo : containers_) {
-    if (!std::regex_search(fqName, containerInfo->matcher)) {
-      continue;
-    }
-
-    VLOG(2) << "Matching container `" << containerInfo->typeName << "` from `"
-            << fqName << "`" << std::endl;
-    auto& c = makeType<Container>(type, *containerInfo, size);
-    enumerateClassTemplateParams(type, c.templateParams);
-    return &c;
-  }
-  return nullptr;
-}
-
-Type& DrgnParser::enumerateClass(struct drgn_type* type) {
   std::string fqName;
   char* nameStr = nullptr;
   size_t length = 0;
@@ -181,10 +155,6 @@ Type& DrgnParser::enumerateClass(struct drgn_type* type) {
     fqName = nameStr;
   }
 
-  auto* container = enumerateContainer(type, fqName);
-  if (container)
-    return *container;
-
   const char* typeTag = drgn_type_tag(type);
   std::string name = typeTag ? typeTag : "";
 

oi/type_graph/DrgnParser.h

@@ -46,25 +46,19 @@ struct DrgnParserOptions {
  * - Performance: reading no more info from drgn than necessary
  *
  * For the most part we try to move logic out of DrgnParser and have later
- * passes clean up the type graph, e.g. flattening parents. However, we do
+ * passes clean up the type graph, e.g. flattening parents and identifying
- * incorporate some extra logic here when it would allow us to read less DWARF
+ * containers.
- * information, e.g. matching containers in DrngParser means we don't read
- * details about container internals.
  */
 class DrgnParser {
  public:
-  DrgnParser(TypeGraph& typeGraph,
+  DrgnParser(TypeGraph& typeGraph, DrgnParserOptions options)
-             const std::vector<std::unique_ptr<ContainerInfo>>& containers,
+      : typeGraph_(typeGraph), options_(options) {
-             DrgnParserOptions options)
-      : typeGraph_(typeGraph), containers_(containers), options_(options) {
   }
   Type& parse(struct drgn_type* root);
 
  private:
   Type& enumerateType(struct drgn_type* type);
-  Container* enumerateContainer(struct drgn_type* type,
+  Class& enumerateClass(struct drgn_type* type);
-                                const std::string& fqName);
-  Type& enumerateClass(struct drgn_type* type);
   Enum& enumerateEnum(struct drgn_type* type);
   Typedef& enumerateTypedef(struct drgn_type* type);
   Type& enumeratePointer(struct drgn_type* type);
@@ -98,7 +92,6 @@ class DrgnParser {
       drgn_types_;
 
   TypeGraph& typeGraph_;
-  const std::vector<std::unique_ptr<ContainerInfo>>& containers_;
   int depth_;
   DrgnParserOptions options_;
 };

oi/type_graph/TypeIdentifier.cpp

@@ -86,6 +86,7 @@ void TypeIdentifier::visit(Container& c) {
           }
           c.templateParams[i] = *dummy;
           replaced = true;
+          break;
         }
       }
 

test/test_add_children.cpp

@@ -66,7 +66,7 @@ TEST_F(AddChildrenTest, InheritanceStatic) {
 }
 
 TEST_F(AddChildrenTest, InheritancePolymorphic) {
-  testMultiCompiler("oid_test_case_inheritance_polymorphic_a_as_a", R"(
+  testMultiCompilerGlob("oid_test_case_inheritance_polymorphic_a_as_a", R"(
 [1]  Pointer
 [0]    Class: A (size: 16)
          Member: _vptr$A (offset: 0)
@@ -78,11 +78,11 @@ TEST_F(AddChildrenTest, InheritancePolymorphic) {
          Function: A
          Function: A
          Child
-[8]        Class: B (size: 40)
+[17]       Class: B (size: 40)
              Parent (offset: 0)
                [0]
              Member: vec_b (offset: 16)
-[4]            Container: std::vector (size: 24)
+[4]            Class: vector<int, std::allocator<int> > (size: 24)
                  Param
                    Primitive: int32_t
                  Param
@@ -105,14 +105,15 @@ TEST_F(AddChildrenTest, InheritancePolymorphic) {
                      Function: ~allocator
                      Function: allocate
                      Function: deallocate
+                 *
              Function: ~B (virtual)
              Function: myfunc (virtual)
              Function: B
              Function: B
              Child
-[10]           Class: C (size: 48)
+[19]           Class: C (size: 48)
                  Parent (offset: 0)
-                   [8]
+                   [17]
                  Member: int_c (offset: 40)
                    Primitive: int32_t
                  Function: ~C (virtual)
@@ -133,11 +134,11 @@ TEST_F(AddChildrenTest, InheritancePolymorphic) {
          Function: ~A (virtual)
          Function: myfunc (virtual)
          Child
-[7]        Class: B (size: 40)
+[13]       Class: B (size: 40)
              Parent (offset: 0)
                [0]
              Member: vec_b (offset: 16)
-[4]            Container: std::vector (size: 24)
+[4]            Class: vector<int, std::allocator<int> > (size: 24)
                  Param
                    Primitive: int32_t
                  Param
@@ -157,15 +158,16 @@ TEST_F(AddChildrenTest, InheritancePolymorphic) {
                      Function: ~allocator
                      Function: allocate
                      Function: deallocate
+                 *
              Function: operator=
              Function: B
              Function: B
              Function: ~B (virtual)
              Function: myfunc (virtual)
              Child
-[9]            Class: C (size: 48)
+[15]           Class: C (size: 48)
                  Parent (offset: 0)
-                   [7]
+                   [13]
                  Member: int_c (offset: 40)
                    Primitive: int32_t
                  Function: operator=

test/test_codegen.cpp

@@ -35,6 +35,9 @@ void testTransform(OICodeGen::Config& config,
 
   MockSymbolService symbols;
   CodeGen codegen{config, symbols};
+  for (auto& info : getContainerInfos()) {
+    codegen.registerContainer(std::move(info));
+  }
   codegen.transform(typeGraph);
 
   check(typeGraph, expectedAfter, "after transform");
@@ -48,7 +51,7 @@ void testTransform(std::string_view input, std::string_view expectedAfter) {
 
 TEST(CodeGenTest, TransformContainerAllocator) {
   testTransform(R"(
-[0] Container: std::vector (size: 24)
+[0] Class: std::vector (size: 24)
       Param
         Primitive: int32_t
       Param
@@ -59,18 +62,18 @@ TEST(CodeGenTest, TransformContainerAllocator) {
           Function: deallocate
 )",
                 R"(
-[0] Container: std::vector<int32_t, DummyAllocator<int32_t, 8, 0, 2>> (size: 24)
+[2] Container: std::vector<int32_t, DummyAllocator<int32_t, 8, 0, 3>> (size: 24)
       Param
         Primitive: int32_t
       Param
-[2]     DummyAllocator [MyAlloc] (size: 8)
+[3]     DummyAllocator [MyAlloc] (size: 8)
           Primitive: int32_t
 )");
 }
 
 TEST(CodeGenTest, TransformContainerAllocatorParamInParent) {
   testTransform(R"(
-[0] Container: std::map (size: 24)
+[0] Class: std::map (size: 24)
       Param
         Primitive: int32_t
       Param
@@ -80,7 +83,7 @@ TEST(CodeGenTest, TransformContainerAllocatorParamInParent) {
           Parent (offset: 0)
 [2]         Struct: MyAllocBase<std::pair<const int, int>> (size: 1)
               Param
-[3]             Container: std::pair (size: 8)
+[3]             Class: std::pair (size: 8)
                   Param
                     Primitive: int32_t
                     Qualifiers: const
@@ -92,14 +95,14 @@ TEST(CodeGenTest, TransformContainerAllocatorParamInParent) {
           Function: deallocate
 )",
                 R"(
-[0] Container: std::map<int32_t, int32_t, DummyAllocator<std::pair<int32_t const, int32_t>, 0, 0, 4>> (size: 24)
+[4] Container: std::map<int32_t, int32_t, DummyAllocator<std::pair<int32_t const, int32_t>, 0, 0, 6>> (size: 24)
       Param
         Primitive: int32_t
       Param
         Primitive: int32_t
       Param
-[4]     DummyAllocator [MyAlloc<std::pair<const int, int>>] (size: 0)
+[6]     DummyAllocator [MyAlloc<std::pair<const int, int>>] (size: 0)
-[3]       Container: std::pair<int32_t const, int32_t> (size: 8)
+[5]       Container: std::pair<int32_t const, int32_t> (size: 8)
             Param
               Primitive: int32_t
               Qualifiers: const
@@ -150,3 +153,25 @@ TEST(CodeGenTest, UnionMembersAlignment) {
           Primitive: int8_t
 )");
 }
+
+TEST(CodeGenTest, ReplaceContainersAndDummies) {
+  testTransform(R"(
+[0] Class: std::vector (size: 24)
+      Param
+        Primitive: uint32_t
+      Param
+[1]     Class: allocator<int> (size: 1)
+          Param
+            Primitive: uint32_t
+          Function: allocate
+          Function: deallocate
+)",
+                R"(
+[2] Container: std::vector<uint32_t, DummyAllocator<uint32_t, 0, 0, 3>> (size: 24)
+      Param
+        Primitive: uint32_t
+      Param
+[3]     DummyAllocator [allocator<int>] (size: 0)
+          Primitive: uint32_t
+)");
+}

test/test_drgn_parser.cpp

@@ -1,16 +1,15 @@
+#include "test_drgn_parser.h"
+
 #include <gtest/gtest.h>
 
 #include <regex>
 
-#include "oi/SymbolService.h"
-// TODO needed?:
-#include "oi/ContainerInfo.h"
 #include "oi/OIParser.h"
+#include "oi/SymbolService.h"
 #include "oi/type_graph/NodeTracker.h"
 #include "oi/type_graph/Printer.h"
 #include "oi/type_graph/TypeGraph.h"
 #include "oi/type_graph/Types.h"
-#include "test_drgn_parser.h"
 
 using namespace type_graph;
 
@@ -19,25 +18,17 @@ using namespace type_graph;
 
 SymbolService* DrgnParserTest::symbols_ = nullptr;
 
-namespace {
+void DrgnParserTest::SetUpTestSuite() {
-const std::vector<std::unique_ptr<ContainerInfo>>& getContainerInfos() {
+  symbols_ = new SymbolService{TARGET_EXE_PATH};
-  static auto res = []() {
+}
-    // TODO more container types, with various template parameter options
+
-    auto std_vector =
+void DrgnParserTest::TearDownTestSuite() {
-        std::make_unique<ContainerInfo>("std::vector", SEQ_TYPE, "vector");
+  delete symbols_;
-    std_vector->stubTemplateParams = {1};
-
-    std::vector<std::unique_ptr<ContainerInfo>> containers;
-    containers.emplace_back(std::move(std_vector));
-    return containers;
-  }();
-  return res;
 }
-}  // namespace
 
 DrgnParser DrgnParserTest::getDrgnParser(TypeGraph& typeGraph,
                                          DrgnParserOptions options) {
-  DrgnParser drgnParser{typeGraph, getContainerInfos(), options};
+  DrgnParser drgnParser{typeGraph, options};
   return drgnParser;
 }
 
@@ -148,8 +139,9 @@ void DrgnParserTest::testGlob(std::string_view function,
   expected.remove_prefix(1);  // Remove initial '\n'
   auto [expectedIdx, actualIdx] = globMatch(expected, actual);
   if (expected.size() != expectedIdx) {
-    ADD_FAILURE() << prefixLines(expected.substr(expectedIdx), "-", 50) << "\n"
+    ADD_FAILURE() << prefixLines(expected.substr(expectedIdx), "-", 10000)
-                  << prefixLines(actual.substr(actualIdx), "+", 50);
+                  << "\n"
+                  << prefixLines(actual.substr(actualIdx), "+", 10000);
   }
 }
 
@@ -261,9 +253,9 @@ TEST_F(DrgnParserTest, InheritanceMultiple) {
 }
 
 TEST_F(DrgnParserTest, Container) {
-  testMultiCompiler("oid_test_case_std_vector_int_empty", R"(
+  testMultiCompilerGlob("oid_test_case_std_vector_int_empty", R"(
-[4] Pointer
+[13] Pointer
-[0]   Container: std::vector (size: 24)
+[0]    Class: vector<int, std::allocator<int> > (size: 24)
          Param
            Primitive: int32_t
          Param
@@ -286,10 +278,12 @@ TEST_F(DrgnParserTest, Container) {
              Function: ~allocator
              Function: allocate
              Function: deallocate
+         Parent (offset: 0)
+*
 )",
                         R"(
-[3] Pointer
+[9]  Pointer
-[0]   Container: std::vector (size: 24)
+[0]    Class: vector<int, std::allocator<int> > (size: 24)
          Param
            Primitive: int32_t
          Param
@@ -309,6 +303,8 @@ TEST_F(DrgnParserTest, Container) {
              Function: ~allocator
              Function: allocate
              Function: deallocate
+         Parent (offset: 0)
+*
 )");
 }
 // TODO test vector with custom allocator
@@ -455,70 +451,6 @@ TEST_F(DrgnParserTest, ClassTemplateInt) {
 )");
 }
 
-TEST_F(DrgnParserTest, ClassTemplateVector) {
-  testMultiCompiler("oid_test_case_templates_vector", R"(
-[5] Pointer
-[0]   Class: TemplatedClass1<std::vector<int, std::allocator<int> > > (size: 24)
-        Param
-[1]       Container: std::vector (size: 24)
-            Param
-              Primitive: int32_t
-            Param
-[2]           Class: allocator<int> (size: 1)
-                Param
-                  Primitive: int32_t
-                Parent (offset: 0)
-[4]               Typedef: __allocator_base<int>
-[3]                 Class: new_allocator<int> (size: 1)
-                      Param
-                        Primitive: int32_t
-                      Function: new_allocator
-                      Function: new_allocator
-                      Function: allocate
-                      Function: deallocate
-                      Function: _M_max_size
-                Function: allocator
-                Function: allocator
-                Function: operator=
-                Function: ~allocator
-                Function: allocate
-                Function: deallocate
-        Member: val (offset: 0)
-          [1]
-        Function: ~TemplatedClass1
-        Function: TemplatedClass1
-)",
-                    R"(
-[4] Pointer
-[0]   Class: TemplatedClass1<std::vector<int, std::allocator<int> > > (size: 24)
-        Param
-[1]       Container: std::vector (size: 24)
-            Param
-              Primitive: int32_t
-            Param
-[2]           Class: allocator<int> (size: 1)
-                Parent (offset: 0)
-[3]               Class: new_allocator<int> (size: 1)
-                    Param
-                      Primitive: int32_t
-                    Function: new_allocator
-                    Function: new_allocator
-                    Function: allocate
-                    Function: deallocate
-                    Function: _M_max_size
-                Function: allocator
-                Function: allocator
-                Function: operator=
-                Function: ~allocator
-                Function: allocate
-                Function: deallocate
-        Member: val (offset: 0)
-          [1]
-        Function: TemplatedClass1
-        Function: ~TemplatedClass1
-)");
-}
-
 TEST_F(DrgnParserTest, ClassTemplateTwo) {
   test("oid_test_case_templates_two", R"(
 [3] Pointer

test/test_drgn_parser.h

@@ -16,13 +16,8 @@ using namespace oi::detail;
 
 class DrgnParserTest : public ::testing::Test {
  protected:
-  static void SetUpTestSuite() {
+  static void SetUpTestSuite();
-    symbols_ = new SymbolService{TARGET_EXE_PATH};
+  static void TearDownTestSuite();
-  }
-
-  static void TearDownTestSuite() {
-    delete symbols_;
-  }
 
   static type_graph::DrgnParser getDrgnParser(
       type_graph::TypeGraph& typeGraph, type_graph::DrgnParserOptions options);