From 0330ef8945c743da4332c61605db1451a7f78a3c Mon Sep 17 00:00:00 2001 From: Alastair Robertson Date: Wed, 28 Jun 2023 08:17:45 -0700 Subject: [PATCH] Take list of pass-through types from config instead of hardcoding As we now store ContainerInfo objects in OICodeGen::Config, we can not copy it any more. Change all places that took copies to take const references instead. The copy in OICodeGen modified membersToStub, the contents of which form part of OICache's hash. However, as OICache also previously had its own copy, it would not have been OICodeGen's modifications. --- dev.oid.toml | 5 +++++ oi/CodeGen.cpp | 5 +++-- oi/CodeGen.h | 4 ++-- oi/OICache.h | 6 +++++- oi/OICodeGen.cpp | 8 ++++---- oi/OICodeGen.h | 26 ++++++++++++++------------ oi/OID.cpp | 7 +++---- oi/OIDebugger.cpp | 20 ++++++++------------ oi/OIDebugger.h | 13 ++++++++----- oi/OIUtils.cpp | 19 +++++++++++++++++++ oi/type_graph/TypeIdentifier.cpp | 18 +++++------------- oi/type_graph/TypeIdentifier.h | 10 ++++++---- test/ci.oid.toml | 5 +++++ test/test_type_identifier.cpp | 6 +++--- tools/combine_configs.py | 2 ++ 15 files changed, 92 insertions(+), 62 deletions(-) diff --git a/dev.oid.toml b/dev.oid.toml index 77a05c5..9e3ac26 100644 --- a/dev.oid.toml +++ b/dev.oid.toml @@ -47,3 +47,8 @@ containers = [ "PWD/types/thrift_isset_type.toml", "PWD/types/weak_ptr_type.toml", ] +pass_through = [ + ["std::allocator", "memory"], + ["std::char_traits", "string"], + ["folly::fbstring_core", "folly/FBString.h"], +] diff --git a/oi/CodeGen.cpp b/oi/CodeGen.cpp index 928ee5f..19658d3 100644 --- a/oi/CodeGen.cpp +++ b/oi/CodeGen.cpp @@ -712,7 +712,7 @@ void CodeGen::addDrgnRoot(struct drgn_type* drgnType, void CodeGen::transform(type_graph::TypeGraph& typeGraph) { type_graph::PassManager pm; pm.addPass(type_graph::Flattener::createPass()); - pm.addPass(type_graph::TypeIdentifier::createPass()); + pm.addPass(type_graph::TypeIdentifier::createPass(config_.passThroughTypes)); if (config_.features[Feature::PolymorphicInheritance]) { type_graph::DrgnParser drgnParser{ typeGraph, containerInfos_, @@ -720,7 +720,8 @@ void CodeGen::transform(type_graph::TypeGraph& typeGraph) { 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()); + pm.addPass( + type_graph::TypeIdentifier::createPass(config_.passThroughTypes)); } pm.addPass(type_graph::RemoveIgnored::createPass(config_.membersToStub)); pm.addPass(type_graph::AddPadding::createPass()); diff --git a/oi/CodeGen.h b/oi/CodeGen.h index 8bea7c3..18c8174 100644 --- a/oi/CodeGen.h +++ b/oi/CodeGen.h @@ -37,7 +37,7 @@ class TypeGraph; class CodeGen { public: - CodeGen(OICodeGen::Config& config, SymbolService& symbols) + CodeGen(const OICodeGen::Config& config, SymbolService& symbols) : config_(config), symbols_(symbols) { } @@ -58,7 +58,7 @@ class CodeGen { ); private: - OICodeGen::Config config_; + const OICodeGen::Config& config_; SymbolService& symbols_; std::vector containerInfos_; std::unordered_set definedContainers_; diff --git a/oi/OICache.h b/oi/OICache.h index a41abdc..c63e967 100644 --- a/oi/OICache.h +++ b/oi/OICache.h @@ -28,6 +28,10 @@ namespace fs = std::filesystem; class OICache { public: + OICache(const OICodeGen::Config& generatorConfig) + : generatorConfig(generatorConfig) { + } + fs::path basePath{}; std::shared_ptr symbols{}; bool downloadedRemote = false; @@ -37,7 +41,7 @@ class OICache { // We need the generator config to download the cache // with the matching configuration. - OICodeGen::Config generatorConfig{}; + const OICodeGen::Config& generatorConfig; // Entity is used to index the `extensions` array // So we must keep the Entity enum and `extensions` array in sync! diff --git a/oi/OICodeGen.cpp b/oi/OICodeGen.cpp index 9d7cbcd..9ca49ed 100644 --- a/oi/OICodeGen.cpp +++ b/oi/OICodeGen.cpp @@ -89,9 +89,9 @@ OICodeGen::OICodeGen(const Config& c, SymbolService& s) "event", }; - config.membersToStub.reserve(typesToStub.size()); + membersToStub = config.membersToStub; for (const auto& type : typesToStub) { - config.membersToStub.emplace_back(type, "*"); + membersToStub.emplace_back(type, "*"); } // `knownTypes` has been made obsolete by the introduction of using the @@ -1269,10 +1269,10 @@ bool OICodeGen::generateMemberDefinition(drgn_type* type, std::optional> OICodeGen::isMemberToStub(const std::string& typeName, const std::string& member) { - auto it = std::ranges::find_if(config.membersToStub, [&](auto& s) { + auto it = std::ranges::find_if(membersToStub, [&](auto& s) { return typeName.starts_with(s.first) && s.second == member; }); - if (it == std::end(config.membersToStub)) { + if (it == std::end(membersToStub)) { return std::nullopt; } return *it; diff --git a/oi/OICodeGen.h b/oi/OICodeGen.h index b9a7558..d0d9133 100644 --- a/oi/OICodeGen.h +++ b/oi/OICodeGen.h @@ -51,18 +51,19 @@ struct ParentMember { class OICodeGen { public: struct Config { - /* - * Note: don't set default values for the config so the user gets an - * uninitialized field" warning if they missed any. - */ + Config() = default; + Config(const Config& other) = delete; + Config& operator=(const Config& other) = delete; + Config(Config&& other) = delete; + Config& operator=(Config&& other) = delete; + bool useDataSegment; - - FeatureSet features{}; - - std::set containerConfigPaths{}; - std::set defaultHeaders{}; - std::set defaultNamespaces{}; - std::vector> membersToStub{}; + FeatureSet features; + std::set containerConfigPaths; + std::set defaultHeaders; + std::set defaultNamespaces; + std::vector> membersToStub; + std::vector passThroughTypes; std::string toString() const; std::vector toOptions() const; @@ -108,7 +109,7 @@ class OICodeGen { bool isDynamic(drgn_type* type) const; private: - Config config{}; + const Config& config; FuncGen funcGen; using ContainerTypeMapEntry = @@ -152,6 +153,7 @@ class OICodeGen { std::map pointerToTypeMap; std::set thriftIssetStructTypes; std::vector topoSortedStructTypes; + std::vector> membersToStub; ContainerInfoRefSet containerTypesFuncDef; diff --git a/oi/OID.cpp b/oi/OID.cpp index ed02305..0ed3f29 100644 --- a/oi/OID.cpp +++ b/oi/OID.cpp @@ -655,10 +655,9 @@ int main(int argc, char* argv[]) { OICompiler::Config compilerConfig{}; - OICodeGen::Config codeGenConfig{ - .useDataSegment = true, - .features = {}, // fill in after processing the config file - }; + OICodeGen::Config codeGenConfig; + codeGenConfig.useDataSegment = true; + codeGenConfig.features = {}; // fill in after processing the config file TreeBuilder::Config tbConfig{ .features = {}, // fill in after processing the config file diff --git a/oi/OIDebugger.cpp b/oi/OIDebugger.cpp index 104332a..3360ec8 100644 --- a/oi/OIDebugger.cpp +++ b/oi/OIDebugger.cpp @@ -1878,24 +1878,23 @@ bool OIDebugger::removeTrap(pid_t pid, const trapInfo& t) { return true; } -OIDebugger::OIDebugger(OICodeGen::Config genConfig, +OIDebugger::OIDebugger(const OICodeGen::Config& genConfig, OICompiler::Config ccConfig, TreeBuilder::Config tbConfig) - : compilerConfig{std::move(ccConfig)}, - generatorConfig{std::move(genConfig)}, + : cache{genConfig}, + compilerConfig{std::move(ccConfig)}, + generatorConfig{genConfig}, treeBuilderConfig{std::move(tbConfig)} { debug = true; - cache.generatorConfig = generatorConfig; VLOG(1) << "CodeGen config: " << generatorConfig.toString(); } OIDebugger::OIDebugger(pid_t pid, - OICodeGen::Config genConfig, + const OICodeGen::Config& genConfig, OICompiler::Config ccConfig, TreeBuilder::Config tbConfig) - : OIDebugger( - std::move(genConfig), std::move(ccConfig), std::move(tbConfig)) { + : OIDebugger(genConfig, std::move(ccConfig), std::move(tbConfig)) { traceePid = pid; symbols = std::make_shared(traceePid); setDataSegmentSize(dataSegSize); @@ -1904,11 +1903,10 @@ OIDebugger::OIDebugger(pid_t pid, } OIDebugger::OIDebugger(fs::path debugInfo, - OICodeGen::Config genConfig, + const OICodeGen::Config& genConfig, OICompiler::Config ccConfig, TreeBuilder::Config tbConfig) - : OIDebugger( - std::move(genConfig), std::move(ccConfig), std::move(tbConfig)) { + : OIDebugger(genConfig, std::move(ccConfig), std::move(tbConfig)) { symbols = std::make_shared(std::move(debugInfo)); cache.symbols = symbols; } @@ -2699,8 +2697,6 @@ void OIDebugger::setDataSegmentSize(size_t size) { int pgsz = getpagesize(); dataSegSize = ((size + pgsz - 1) & ~(pgsz - 1)); - generatorConfig.useDataSegment = dataSegSize > 0; - VLOG(1) << "setDataSegmentSize: segment size: " << dataSegSize; } diff --git a/oi/OIDebugger.h b/oi/OIDebugger.h index 83d0afb..483012b 100644 --- a/oi/OIDebugger.h +++ b/oi/OIDebugger.h @@ -32,12 +32,15 @@ namespace fs = std::filesystem; class OIDebugger { - OIDebugger(OICodeGen::Config, OICompiler::Config, TreeBuilder::Config); + OIDebugger(const OICodeGen::Config&, OICompiler::Config, TreeBuilder::Config); public: - OIDebugger(pid_t, OICodeGen::Config, OICompiler::Config, TreeBuilder::Config); + OIDebugger(pid_t, + const OICodeGen::Config&, + OICompiler::Config, + TreeBuilder::Config); OIDebugger(fs::path, - OICodeGen::Config, + const OICodeGen::Config&, OICompiler::Config, TreeBuilder::Config); @@ -170,7 +173,7 @@ class OIDebugger { const int replayInstSize = 512; bool trapsRemoved{false}; std::shared_ptr symbols; - OICache cache{}; + OICache cache; /* * Map address of valid INT3 instruction to metadata for that interrupt. @@ -231,7 +234,7 @@ class OIDebugger { std::optional> findRetLocs(FuncDesc&); OICompiler::Config compilerConfig{}; - OICodeGen::Config generatorConfig{}; + const OICodeGen::Config& generatorConfig; TreeBuilder::Config treeBuilderConfig{}; std::optional generateCode(const irequest&); diff --git a/oi/OIUtils.cpp b/oi/OIUtils.cpp index 1368695..745c7bf 100644 --- a/oi/OIUtils.cpp +++ b/oi/OIUtils.cpp @@ -84,6 +84,25 @@ std::optional processConfigFile( } }); } + if (toml::array* arr = (*types)["pass_through"].as_array()) { + for (auto&& el : *arr) { + auto* type = el.as_array(); + if (type && type->size() == 2 && (*type)[0].is_string() && + (*type)[1].is_string()) { + std::string name = (*type)[0].as_string()->get(); + std::string header = (*type)[1].as_string()->get(); + generatorConfig.passThroughTypes.emplace_back( + std::move(name), DUMMY_TYPE, std::move(header)); + } else { + LOG(ERROR) << "pass_through elements must be lists of [type_name, " + "header_file]"; + return {}; + } + } + } else { + LOG(ERROR) << "pass_through must be a list"; + return {}; + } } if (toml::table* headers = config["headers"].as_table()) { diff --git a/oi/type_graph/TypeIdentifier.cpp b/oi/type_graph/TypeIdentifier.cpp index f9ac880..7c1738d 100644 --- a/oi/type_graph/TypeIdentifier.cpp +++ b/oi/type_graph/TypeIdentifier.cpp @@ -20,18 +20,10 @@ namespace type_graph { -// TODO: -// - read these from a TOML file -// - don't require specifying ctype and header -const std::array TypeIdentifier::dummyContainers_ = { - ContainerInfo{"std::allocator", DUMMY_TYPE, "memory"}, - ContainerInfo{"std::char_traits", DUMMY_TYPE, "string"}, - ContainerInfo{"folly::fbstring_core", DUMMY_TYPE, "folly/FBString.h"}, -}; - -Pass TypeIdentifier::createPass() { - auto fn = [](TypeGraph& typeGraph) { - TypeIdentifier typeId{typeGraph}; +Pass TypeIdentifier::createPass( + const std::vector& passThroughTypes) { + auto fn = [&passThroughTypes](TypeGraph& typeGraph) { + TypeIdentifier typeId{typeGraph, passThroughTypes}; for (auto& type : typeGraph.rootTypes()) { typeId.visit(type); } @@ -78,7 +70,7 @@ void TypeIdentifier::visit(Container& c) { if (Class* paramClass = dynamic_cast(param.type)) { bool replaced = false; - for (const auto& info : dummyContainers_) { + for (const auto& info : passThroughTypes_) { if (std::regex_search(paramClass->fqName(), info.matcher)) { // Create dummy containers auto* dummy = diff --git a/oi/type_graph/TypeIdentifier.h b/oi/type_graph/TypeIdentifier.h index e547e63..769b76d 100644 --- a/oi/type_graph/TypeIdentifier.h +++ b/oi/type_graph/TypeIdentifier.h @@ -17,6 +17,7 @@ #include #include +#include #include "PassManager.h" #include "Types.h" @@ -34,10 +35,12 @@ class TypeGraph; */ class TypeIdentifier : public RecursiveVisitor { public: - static Pass createPass(); + static Pass createPass(const std::vector& passThroughTypes); static bool isAllocator(Type& t); - TypeIdentifier(TypeGraph& typeGraph) : typeGraph_(typeGraph) { + TypeIdentifier(TypeGraph& typeGraph, + const std::vector& passThroughTypes) + : typeGraph_(typeGraph), passThroughTypes_(passThroughTypes) { } using RecursiveVisitor::visit; @@ -48,8 +51,7 @@ class TypeIdentifier : public RecursiveVisitor { private: std::unordered_set visited_; TypeGraph& typeGraph_; - - static const std::array dummyContainers_; + const std::vector& passThroughTypes_; }; } // namespace type_graph diff --git a/test/ci.oid.toml b/test/ci.oid.toml index 830ed96..2b39fe1 100644 --- a/test/ci.oid.toml +++ b/test/ci.oid.toml @@ -43,6 +43,11 @@ containers = [ "../types/thrift_isset_type.toml", "../types/weak_ptr_type.toml", ] +pass_through = [ + ["std::allocator", "memory"], + ["std::char_traits", "string"], + ["folly::fbstring_core", "folly/FBString.h"], +] [headers] user_paths = [ diff --git a/test/test_type_identifier.cpp b/test/test_type_identifier.cpp index b5ce6ab..ae41ef2 100644 --- a/test/test_type_identifier.cpp +++ b/test/test_type_identifier.cpp @@ -17,7 +17,7 @@ TEST(TypeIdentifierTest, StubbedParam) { container.templateParams.push_back(TemplateParam{&myparam}); container.templateParams.push_back(TemplateParam{&myint}); - test(TypeIdentifier::createPass(), {container}, R"( + test(TypeIdentifier::createPass({}), {container}, R"( [0] Container: std::vector (size: 24) Param Primitive: int32_t @@ -52,7 +52,7 @@ TEST(TypeIdentifierTest, Allocator) { container.templateParams.push_back(TemplateParam{&myalloc}); container.templateParams.push_back(TemplateParam{&myint}); - test(TypeIdentifier::createPass(), {container}, R"( + test(TypeIdentifier::createPass({}), {container}, R"( [0] Container: std::vector (size: 24) Param Primitive: int32_t @@ -90,7 +90,7 @@ TEST(TypeIdentifierTest, AllocatorSize1) { container.templateParams.push_back(TemplateParam{&myalloc}); container.templateParams.push_back(TemplateParam{&myint}); - test(TypeIdentifier::createPass(), {container}, R"( + test(TypeIdentifier::createPass({}), {container}, R"( [0] Container: std::vector (size: 24) Param Primitive: int32_t diff --git a/tools/combine_configs.py b/tools/combine_configs.py index 50e359b..02e685e 100644 --- a/tools/combine_configs.py +++ b/tools/combine_configs.py @@ -26,6 +26,7 @@ def main(): out = { "types": { "containers": [], + "pass_through": [], }, "headers": { "user_paths": [], @@ -42,6 +43,7 @@ def main(): types = cfg.get("types", None) if types is not None: out["types"]["containers"] += types.get("containers", []) + out["types"]["pass_through"] += types.get("pass_through", []) headers = cfg.get("headers", None) if headers is not None: