From f4a1bd3d99e569d764e60b7c27b2833031b71e00 Mon Sep 17 00:00:00 2001 From: Thierry Treyer Date: Fri, 29 Sep 2023 15:09:15 -0700 Subject: [PATCH] Remove Primitive::Kind::Incomplete --- oi/type_graph/DrgnParser.cpp | 11 +++++++++-- oi/type_graph/EnforceCompatibility.cpp | 8 -------- oi/type_graph/Flattener.cpp | 4 +--- oi/type_graph/Printer.cpp | 14 ++++++++------ oi/type_graph/TypeGraph.cpp | 3 --- oi/type_graph/Types.cpp | 2 -- oi/type_graph/Types.h | 25 +++++++++++++++++++------ test/TypeGraphParser.cpp | 14 ++++++++++---- test/test_drgn_parser.cpp | 3 +-- test/test_enforce_compatibility.cpp | 3 ++- test/test_flattener.cpp | 2 +- 11 files changed, 51 insertions(+), 38 deletions(-) diff --git a/oi/type_graph/DrgnParser.cpp b/oi/type_graph/DrgnParser.cpp index 1333869..952cc73 100644 --- a/oi/type_graph/DrgnParser.cpp +++ b/oi/type_graph/DrgnParser.cpp @@ -123,10 +123,17 @@ Type& DrgnParser::enumerateType(struct drgn_type* type) { std::to_string(kind)}; } } catch (const DrgnParserError& e) { + depth_--; if (isTypeIncomplete) { - t = &makeType(type, Primitive::Kind::Incomplete); + const char* typeName = ""; + if (drgn_type_has_name(type)) { + typeName = drgn_type_name(type); + } else if (drgn_type_has_tag(type)) { + typeName = drgn_type_tag(type); + } + + return makeType(nullptr, typeName); } else { - depth_--; throw e; } } diff --git a/oi/type_graph/EnforceCompatibility.cpp b/oi/type_graph/EnforceCompatibility.cpp index e97b5d4..934649f 100644 --- a/oi/type_graph/EnforceCompatibility.cpp +++ b/oi/type_graph/EnforceCompatibility.cpp @@ -83,14 +83,6 @@ void EnforceCompatibility::visit(Class& c) { // the pointer's address in this case. return true; } - - if (auto* primitive = dynamic_cast(&ptr->pointeeType())) { - if (primitive->kind() == Primitive::Kind::Incomplete) { - // This is a pointer to an incomplete type. CodeGen v1 does not record - // the pointer's address in this case. - return true; - } - } } return false; diff --git a/oi/type_graph/Flattener.cpp b/oi/type_graph/Flattener.cpp index 23c73f4..f8ee3af 100644 --- a/oi/type_graph/Flattener.cpp +++ b/oi/type_graph/Flattener.cpp @@ -56,9 +56,7 @@ void flattenParent(const Parent& parent, // Create a new member to represent this parent container flattenedMembers.emplace_back(*parentContainer, Flattener::ParentPrefix, parent.bitOffset); - } else if (auto* parentPrimitive = dynamic_cast(&parentType); - parentPrimitive && - parentPrimitive->kind() == Primitive::Kind::Incomplete) { + } else if (auto* parentPrimitive = dynamic_cast(&parentType)) { // Bad DWARF can lead to us seeing incomplete parent types. Just ignore // these as there is nothing we can do to recover the missing info. } else { diff --git a/oi/type_graph/Printer.cpp b/oi/type_graph/Printer.cpp index 9e9d173..e7d70eb 100644 --- a/oi/type_graph/Printer.cpp +++ b/oi/type_graph/Printer.cpp @@ -38,8 +38,13 @@ void Printer::print(const Type& type) { void Printer::visit(const Incomplete& i) { prefix(); - out_ << "Incomplete:" << std::endl; - print(i.underlyingType()); + out_ << "Incomplete"; + if (auto underlyingType = i.underlyingType()) { + out_ << std::endl; + print(underlyingType.value().get()); + } else { + out_ << ": [" << i.inputName() << "]" << std::endl; + } } void Printer::visit(const Class& c) { @@ -97,10 +102,7 @@ void Printer::visit(const Container& c) { void Printer::visit(const Primitive& p) { prefix(); - out_ << "Primitive: " << p.name(); - if (p.kind() == Primitive::Kind::Incomplete) - out_ << " (incomplete)"; - out_ << std::endl; + out_ << "Primitive: " << p.name() << std::endl; } void Printer::visit(const Enum& e) { diff --git a/oi/type_graph/TypeGraph.cpp b/oi/type_graph/TypeGraph.cpp index 95bf82b..28e3a3b 100644 --- a/oi/type_graph/TypeGraph.cpp +++ b/oi/type_graph/TypeGraph.cpp @@ -65,9 +65,6 @@ Primitive& TypeGraph::makeType(Primitive::Kind kind) { case Primitive::Kind::Void: static Primitive pVoid{kind}; return pVoid; - case Primitive::Kind::Incomplete: - static Primitive pIncomplete{kind}; - return pIncomplete; } } diff --git a/oi/type_graph/Types.cpp b/oi/type_graph/Types.cpp index f67c914..bed5f5d 100644 --- a/oi/type_graph/Types.cpp +++ b/oi/type_graph/Types.cpp @@ -62,7 +62,6 @@ std::string Primitive::getName(Kind kind) { case Kind::StubbedPointer: return "StubbedPointer"; case Kind::Void: - case Kind::Incomplete: return "void"; } } @@ -105,7 +104,6 @@ std::size_t Primitive::size() const { case Kind::StubbedPointer: return sizeof(uintptr_t); case Kind::Void: - case Kind::Incomplete: return 0; } } diff --git a/oi/type_graph/Types.h b/oi/type_graph/Types.h index 9659366..40f1628 100644 --- a/oi/type_graph/Types.h +++ b/oi/type_graph/Types.h @@ -34,6 +34,7 @@ #include #include #include +#include #include #include "oi/ContainerInfo.h" @@ -195,6 +196,10 @@ class Incomplete : public Type { Incomplete(Type& underlyingType) : underlyingType_(underlyingType) { } + Incomplete(std::string underlyingTypeName) + : underlyingType_(std::move(underlyingTypeName)) { + } + static inline constexpr bool has_node_id = false; DECLARE_ACCEPT @@ -204,7 +209,13 @@ class Incomplete : public Type { } std::string_view inputName() const override { - return underlyingType_.inputName(); + if (std::holds_alternative(underlyingType_)) { + return std::get(underlyingType_); + } + + return std::get>(underlyingType_) + .get() + .inputName(); } size_t size() const override { @@ -219,12 +230,16 @@ class Incomplete : public Type { return -1; } - Type& underlyingType() const { - return underlyingType_; + std::optional> underlyingType() const { + if (std::holds_alternative(underlyingType_)) { + return std::nullopt; + } + + return std::get>(underlyingType_); } private: - Type& underlyingType_; + std::variant> underlyingType_; static const std::string kName; }; @@ -551,8 +566,6 @@ class Primitive : public Type { StubbedPointer, Void, - Incomplete, // Behaves the same as Void, but alerts us that the type was - // stubbed out due to incomplete DWARF }; explicit Primitive(Kind kind) : kind_(kind), name_(getName(kind)) { diff --git a/test/TypeGraphParser.cpp b/test/TypeGraphParser.cpp index bc1eade..1eca163 100644 --- a/test/TypeGraphParser.cpp +++ b/test/TypeGraphParser.cpp @@ -49,8 +49,6 @@ Primitive::Kind getKind(std::string_view kindStr) { return Primitive::Kind::StubbedPointer; if (kindStr == "void") return Primitive::Kind::Void; - if (kindStr == "void (incomplete)") - return Primitive::Kind::Incomplete; throw TypeGraphParserError{"Invalid Primitive::Kind: " + std::string{kindStr}}; } @@ -212,8 +210,16 @@ Type& TypeGraphParser::parseType(std::string_view& input, size_t rootIndent) { type = &it->second.get(); } else if (nodeTypeName == "Incomplete") { - auto& underlyingType = parseType(input, indent + 2); - type = &typeGraph_.makeType(underlyingType); + if (line[nodeEndPos] == ':') { + auto nameStartPos = line.find('[', nodeEndPos) + 1; + auto nameEndPos = line.find(']', nameStartPos); + auto underlyingTypeName = + line.substr(nameStartPos, nameEndPos - nameStartPos); + type = &typeGraph_.makeType(std::string(underlyingTypeName)); + } else { + auto& underlyingType = parseType(input, indent + 2); + type = &typeGraph_.makeType(underlyingType); + } } else if (nodeTypeName == "Class" || nodeTypeName == "Struct" || nodeTypeName == "Union") { // Format: "Class: MyClass (size: 12)" diff --git a/test/test_drgn_parser.cpp b/test/test_drgn_parser.cpp index 72b345b..7777639 100644 --- a/test/test_drgn_parser.cpp +++ b/test/test_drgn_parser.cpp @@ -367,8 +367,7 @@ TEST_F(DrgnParserTest, PointerNoFollow) { TEST_F(DrgnParserTest, PointerIncomplete) { test("oid_test_case_pointers_incomplete_raw", R"( [0] Pointer - Incomplete: - Primitive: void (incomplete) + Incomplete: [IncompleteType] )"); } diff --git a/test/test_enforce_compatibility.cpp b/test/test_enforce_compatibility.cpp index 87c4107..89b92d7 100644 --- a/test/test_enforce_compatibility.cpp +++ b/test/test_enforce_compatibility.cpp @@ -36,7 +36,8 @@ TEST(EnforceCompatibilityTest, VoidPointer) { [0] Class: MyClass (size: 8) Member: p (offset: 0) [1] Pointer - Primitive: void (incomplete) + Incomplete + Primitive: void )", R"( [0] Class: MyClass (size: 8) diff --git a/test/test_flattener.cpp b/test/test_flattener.cpp index 1b07341..ae6900b 100644 --- a/test/test_flattener.cpp +++ b/test/test_flattener.cpp @@ -958,7 +958,7 @@ TEST(FlattenerTest, IncompleteParent) { test(Flattener::createPass(), R"( [0] Class: MyClass (size: 4) Parent (offset: 0) - Primitive: void (incomplete) + Incomplete: [IncompleteParent] )", R"( [0] Class: MyClass (size: 4)