From 2d1cc92bb4935d5eb44edb142571349f30ba448c Mon Sep 17 00:00:00 2001 From: Alastair Robertson Date: Tue, 25 Jul 2023 05:45:22 -0700 Subject: [PATCH] Rename RemoveIgnored -> RemoveMembers Also reshuffle CodeGen's passes to fix an alignment bug with removed members. Change RemoveMembers to actually remove members instead of replacing them with padding. AddPadding must be run afterwards to fill in the gaps. --- oi/CodeGen.cpp | 20 ++++-- oi/type_graph/CMakeLists.txt | 2 +- .../{RemoveIgnored.cpp => RemoveMembers.cpp} | 29 +++----- .../{RemoveIgnored.h => RemoveMembers.h} | 16 ++--- test/CMakeLists.txt | 2 +- test/test_codegen.cpp | 72 +++++++++++++++++++ ...ve_ignored.cpp => test_remove_members.cpp} | 45 ++++-------- 7 files changed, 119 insertions(+), 67 deletions(-) rename oi/type_graph/{RemoveIgnored.cpp => RemoveMembers.cpp} (67%) rename oi/type_graph/{RemoveIgnored.h => RemoveMembers.h} (80%) rename test/{test_remove_ignored.cpp => test_remove_members.cpp} (77%) diff --git a/oi/CodeGen.cpp b/oi/CodeGen.cpp index f722bac..9220b4d 100644 --- a/oi/CodeGen.cpp +++ b/oi/CodeGen.cpp @@ -31,7 +31,7 @@ #include "type_graph/DrgnParser.h" #include "type_graph/Flattener.h" #include "type_graph/NameGen.h" -#include "type_graph/RemoveIgnored.h" +#include "type_graph/RemoveMembers.h" #include "type_graph/RemoveTopLevelPointer.h" #include "type_graph/TopoSorter.h" #include "type_graph/TypeGraph.h" @@ -783,28 +783,34 @@ void CodeGen::addDrgnRoot(struct drgn_type* drgnType, void CodeGen::transform(type_graph::TypeGraph& typeGraph) { type_graph::PassManager pm; - // 1. Transformation passes + // Simplify the type graph first so there is less work for later passes + pm.addPass(type_graph::RemoveTopLevelPointer::createPass()); pm.addPass(type_graph::Flattener::createPass()); pm.addPass(type_graph::TypeIdentifier::createPass(config_.passThroughTypes)); + if (config_.features[Feature::PolymorphicInheritance]) { + // Parse new children nodes type_graph::DrgnParser drgnParser{ typeGraph, containerInfos_, config_.features[Feature::ChaseRawPointers]}; pm.addPass(type_graph::AddChildren::createPass(drgnParser, symbols_)); + // Re-run passes over newly added children pm.addPass(type_graph::Flattener::createPass()); pm.addPass( type_graph::TypeIdentifier::createPass(config_.passThroughTypes)); } - pm.addPass(type_graph::RemoveIgnored::createPass(config_.membersToStub)); - pm.addPass(type_graph::RemoveTopLevelPointer::createPass()); - // 2. Fixup passes to repair type graph after partial transformations + // Calculate alignment before removing members, as those members may have an + // influence on the class' overall alignment. + pm.addPass(type_graph::AlignmentCalc::createPass()); + pm.addPass(type_graph::RemoveMembers::createPass(config_.membersToStub)); + + // Add padding to fill in the gaps of removed members and ensure their + // alignments pm.addPass(type_graph::AddPadding::createPass(config_.features)); - // 3. Annotation passes pm.addPass(type_graph::NameGen::createPass()); - pm.addPass(type_graph::AlignmentCalc::createPass()); pm.addPass(type_graph::TopoSorter::createPass()); pm.run(typeGraph); diff --git a/oi/type_graph/CMakeLists.txt b/oi/type_graph/CMakeLists.txt index 38a3ca9..eea08eb 100644 --- a/oi/type_graph/CMakeLists.txt +++ b/oi/type_graph/CMakeLists.txt @@ -7,7 +7,7 @@ add_library(type_graph NameGen.cpp PassManager.cpp Printer.cpp - RemoveIgnored.cpp + RemoveMembers.cpp RemoveTopLevelPointer.cpp TopoSorter.cpp TypeGraph.cpp diff --git a/oi/type_graph/RemoveIgnored.cpp b/oi/type_graph/RemoveMembers.cpp similarity index 67% rename from oi/type_graph/RemoveIgnored.cpp rename to oi/type_graph/RemoveMembers.cpp index 6972e6b..834846e 100644 --- a/oi/type_graph/RemoveIgnored.cpp +++ b/oi/type_graph/RemoveMembers.cpp @@ -13,26 +13,26 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -#include "RemoveIgnored.h" +#include "RemoveMembers.h" #include "AddPadding.h" #include "TypeGraph.h" namespace type_graph { -Pass RemoveIgnored::createPass( +Pass RemoveMembers::createPass( const std::vector>& membersToIgnore) { auto fn = [&membersToIgnore](TypeGraph& typeGraph) { - RemoveIgnored removeIgnored{typeGraph, membersToIgnore}; + RemoveMembers removeMembers{membersToIgnore}; for (auto& type : typeGraph.rootTypes()) { - removeIgnored.accept(type); + removeMembers.accept(type); } }; - return Pass("RemoveIgnored", fn); + return Pass("RemoveMembers", fn); } -void RemoveIgnored::accept(Type& type) { +void RemoveMembers::accept(Type& type) { if (visited_.count(&type) != 0) return; @@ -40,7 +40,7 @@ void RemoveIgnored::accept(Type& type) { type.accept(*this); } -void RemoveIgnored::visit(Class& c) { +void RemoveMembers::visit(Class& c) { for (const auto& param : c.templateParams) { accept(param.type()); } @@ -54,19 +54,12 @@ void RemoveIgnored::visit(Class& c) { accept(child); } - for (size_t i = 0; i < c.members.size(); i++) { - if (!ignoreMember(c.name(), c.members[i].name)) { - continue; - } - auto& primitive = typeGraph_.makeType(Primitive::Kind::Int8); - auto& paddingArray = - typeGraph_.makeType(primitive, c.members[i].type().size()); - c.members[i] = - Member{paddingArray, AddPadding::MemberPrefix, c.members[i].bitOffset}; - } + std::erase_if(c.members, [this, &c](Member member) { + return ignoreMember(c.name(), member.name); + }); } -bool RemoveIgnored::ignoreMember(const std::string& typeName, +bool RemoveMembers::ignoreMember(const std::string& typeName, const std::string& memberName) const { for (const auto& [ignoredType, ignoredMember] : membersToIgnore_) { if (typeName == ignoredType && memberName == ignoredMember) { diff --git a/oi/type_graph/RemoveIgnored.h b/oi/type_graph/RemoveMembers.h similarity index 80% rename from oi/type_graph/RemoveIgnored.h rename to oi/type_graph/RemoveMembers.h index 49db3e0..e60a608 100644 --- a/oi/type_graph/RemoveIgnored.h +++ b/oi/type_graph/RemoveMembers.h @@ -23,23 +23,20 @@ namespace type_graph { -class TypeGraph; - /* - * RemoveIgnored + * RemoveMembers * - * Remove types and members as requested by the [[codegen.ignore]] section in - * the OI config. + * Removes members as requested by the [[codegen.ignore]] section in the OI + * config. */ -class RemoveIgnored : public RecursiveVisitor { +class RemoveMembers : public RecursiveVisitor { public: static Pass createPass( const std::vector>& membersToIgnore); - RemoveIgnored( - TypeGraph& typeGraph, + RemoveMembers( const std::vector>& membersToIgnore) - : typeGraph_(typeGraph), membersToIgnore_(membersToIgnore) { + : membersToIgnore_(membersToIgnore) { } using RecursiveVisitor::accept; @@ -52,7 +49,6 @@ class RemoveIgnored : public RecursiveVisitor { const std::string& memberName) const; std::unordered_set visited_; - TypeGraph& typeGraph_; const std::vector>& membersToIgnore_; }; diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 20b90d4..287ba49 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -44,7 +44,7 @@ add_executable(test_type_graph test_flattener.cpp test_name_gen.cpp test_node_tracker.cpp - test_remove_ignored.cpp + test_remove_members.cpp test_remove_top_level_pointer.cpp test_topo_sorter.cpp test_type_identifier.cpp diff --git a/test/test_codegen.cpp b/test/test_codegen.cpp index 27bebfd..c9d63ff 100644 --- a/test/test_codegen.cpp +++ b/test/test_codegen.cpp @@ -5,6 +5,7 @@ #include #include +#include "TypeGraphParser.h" #include "mocks.h" #include "oi/CodeGen.h" #include "oi/type_graph/Printer.h" @@ -33,6 +34,33 @@ void testTransform(Type& type, check({type}, expectedAfter, "after transform"); } + +void testTransform(OICodeGen::Config& config, + std::string_view input, + std::string_view expectedAfter) { + input.remove_prefix(1); // Remove initial '\n' + TypeGraph typeGraph; + TypeGraphParser parser{typeGraph}; + try { + parser.parse(input); + } catch (const TypeGraphParserError& err) { + FAIL() << "Error parsing input graph: " << err.what(); + } + + // Validate input formatting + check(typeGraph.rootTypes(), input, "parsing input graph"); + + MockSymbolService symbols; + CodeGen codegen{config, symbols}; + codegen.transform(typeGraph); + + check(typeGraph.rootTypes(), expectedAfter, "after transform"); +} + +void testTransform(std::string_view input, std::string_view expectedAfter) { + OICodeGen::Config config; + testTransform(config, input, expectedAfter); +} } // namespace TEST(CodeGenTest, TransformContainerAllocator) { @@ -135,3 +163,47 @@ TEST(CodeGenTest, TransformContainerAllocatorParamInParent) { Primitive: int32_t )"); } + +TEST(CodeGenTest, RemovedMemberAlignment) { + OICodeGen::Config config; + config.membersToStub = {{"MyClass", "b"}}; + testTransform(config, R"( +[0] Class: MyClass (size: 24) + Member: a (offset: 0) + Primitive: int8_t + Member: b (offset: 8) + Primitive: int64_t + Member: c (offset: 16) + Primitive: int8_t +)", + R"( +[0] Class: MyClass_0 (size: 24, align: 8) + Member: a_0 (offset: 0, align: 1) + Primitive: int8_t + Member: __oi_padding_1 (offset: 1) +[1] Array: (length: 15) + Primitive: int8_t + Member: c_2 (offset: 16, align: 1) + Primitive: int8_t + Member: __oi_padding_3 (offset: 17) +[2] Array: (length: 7) + Primitive: int8_t +)"); +} + +TEST(CodeGenTest, UnionMembersAlignment) { + testTransform(R"( +[0] Union: MyUnion (size: 8) + Member: a (offset: 0) + Primitive: int8_t + Member: b (offset: 8) + Primitive: int64_t +)", + R"( +[0] Union: MyUnion_0 (size: 8, align: 8) + Member: a_0 (offset: 0, align: 1) + Primitive: int8_t + Member: b_1 (offset: 8, align: 8) + Primitive: int64_t +)"); +} diff --git a/test/test_remove_ignored.cpp b/test/test_remove_members.cpp similarity index 77% rename from test/test_remove_ignored.cpp rename to test/test_remove_members.cpp index 6b988a2..146b8f7 100644 --- a/test/test_remove_ignored.cpp +++ b/test/test_remove_members.cpp @@ -1,12 +1,12 @@ #include -#include "oi/type_graph/RemoveIgnored.h" +#include "oi/type_graph/RemoveMembers.h" #include "oi/type_graph/Types.h" #include "test/type_graph_utils.h" using namespace type_graph; -TEST(RemoveIgnoredTest, Match) { +TEST(RemoveMembersTest, Match) { auto classB = Class{1, Class::Kind::Class, "ClassB", 4}; auto classA = Class{0, Class::Kind::Class, "ClassA", 12}; @@ -18,7 +18,7 @@ TEST(RemoveIgnoredTest, Match) { {"ClassA", "b"}, }; - test(RemoveIgnored::createPass(membersToIgnore), {classA}, R"( + test(RemoveMembers::createPass(membersToIgnore), {classA}, R"( [0] Class: ClassA (size: 12) Member: a (offset: 0) [1] Class: ClassB (size: 4) @@ -31,15 +31,12 @@ TEST(RemoveIgnoredTest, Match) { [0] Class: ClassA (size: 12) Member: a (offset: 0) [1] Class: ClassB (size: 4) - Member: __oi_padding (offset: 4) -[2] Array: (length: 4) - Primitive: int8_t Member: c (offset: 8) [1] )"); } -TEST(RemoveIgnoredTest, TypeMatchMemberMiss) { +TEST(RemoveMembersTest, TypeMatchMemberMiss) { auto classB = Class{1, Class::Kind::Class, "ClassB", 4}; auto classA = Class{0, Class::Kind::Class, "ClassA", 12}; @@ -51,7 +48,7 @@ TEST(RemoveIgnoredTest, TypeMatchMemberMiss) { {"ClassA", "x"}, }; - test(RemoveIgnored::createPass(membersToIgnore), {classA}, R"( + test(RemoveMembers::createPass(membersToIgnore), {classA}, R"( [0] Class: ClassA (size: 12) Member: a (offset: 0) [1] Class: ClassB (size: 4) @@ -62,7 +59,7 @@ TEST(RemoveIgnoredTest, TypeMatchMemberMiss) { )"); } -TEST(RemoveIgnoredTest, MemberMatchWrongType) { +TEST(RemoveMembersTest, MemberMatchWrongType) { auto classB = Class{1, Class::Kind::Class, "ClassB", 4}; auto classA = Class{0, Class::Kind::Class, "ClassA", 12}; @@ -74,7 +71,7 @@ TEST(RemoveIgnoredTest, MemberMatchWrongType) { {"ClassB", "b"}, }; - test(RemoveIgnored::createPass(membersToIgnore), {classA}, R"( + test(RemoveMembers::createPass(membersToIgnore), {classA}, R"( [0] Class: ClassA (size: 12) Member: a (offset: 0) [1] Class: ClassB (size: 4) @@ -85,11 +82,11 @@ TEST(RemoveIgnoredTest, MemberMatchWrongType) { )"); } -TEST(RemoveIgnoredTest, RecurseClassParam) { +TEST(RemoveMembersTest, RecurseClassParam) { const std::vector>& membersToIgnore = { {"ClassA", "b"}, }; - test(RemoveIgnored::createPass(membersToIgnore), R"( + test(RemoveMembers::createPass(membersToIgnore), R"( [0] Class: MyClass (size: 0) Param [1] Class: ClassA (size: 12) @@ -106,19 +103,16 @@ TEST(RemoveIgnoredTest, RecurseClassParam) { [1] Class: ClassA (size: 12) Member: a (offset: 0) Primitive: int32_t - Member: __oi_padding (offset: 4) -[2] Array: (length: 4) - Primitive: int8_t Member: c (offset: 8) Primitive: int32_t )"); } -TEST(RemoveIgnoredTest, RecurseClassParent) { +TEST(RemoveMembersTest, RecurseClassParent) { const std::vector>& membersToIgnore = { {"ClassA", "b"}, }; - test(RemoveIgnored::createPass(membersToIgnore), R"( + test(RemoveMembers::createPass(membersToIgnore), R"( [0] Class: MyClass (size: 0) Parent (offset: 0) [1] Class: ClassA (size: 12) @@ -135,19 +129,16 @@ TEST(RemoveIgnoredTest, RecurseClassParent) { [1] Class: ClassA (size: 12) Member: a (offset: 0) Primitive: int32_t - Member: __oi_padding (offset: 4) -[2] Array: (length: 4) - Primitive: int8_t Member: c (offset: 8) Primitive: int32_t )"); } -TEST(RemoveIgnoredTest, RecurseClassMember) { +TEST(RemoveMembersTest, RecurseClassMember) { const std::vector>& membersToIgnore = { {"ClassA", "b"}, }; - test(RemoveIgnored::createPass(membersToIgnore), R"( + test(RemoveMembers::createPass(membersToIgnore), R"( [0] Class: MyClass (size: 0) Member: xxx (offset: 0) [1] Class: ClassA (size: 12) @@ -164,19 +155,16 @@ TEST(RemoveIgnoredTest, RecurseClassMember) { [1] Class: ClassA (size: 12) Member: a (offset: 0) Primitive: int32_t - Member: __oi_padding (offset: 4) -[2] Array: (length: 4) - Primitive: int8_t Member: c (offset: 8) Primitive: int32_t )"); } -TEST(RemoveIgnoredTest, RecurseClassChild) { +TEST(RemoveMembersTest, RecurseClassChild) { const std::vector>& membersToIgnore = { {"ClassA", "b"}, }; - test(RemoveIgnored::createPass(membersToIgnore), R"( + test(RemoveMembers::createPass(membersToIgnore), R"( [0] Class: MyClass (size: 0) Child [1] Class: ClassA (size: 12) @@ -193,9 +181,6 @@ TEST(RemoveIgnoredTest, RecurseClassChild) { [1] Class: ClassA (size: 12) Member: a (offset: 0) Primitive: int32_t - Member: __oi_padding (offset: 4) -[2] Array: (length: 4) - Primitive: int8_t Member: c (offset: 8) Primitive: int32_t )");