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.
This commit is contained in:
Alastair Robertson 2023-06-28 08:17:45 -07:00 committed by Alastair Robertson
parent e86ebb7aff
commit 0330ef8945
15 changed files with 92 additions and 62 deletions

View File

@ -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"],
]

View File

@ -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());

View File

@ -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<ContainerInfo> containerInfos_;
std::unordered_set<const ContainerInfo*> definedContainers_;

View File

@ -28,6 +28,10 @@ namespace fs = std::filesystem;
class OICache {
public:
OICache(const OICodeGen::Config& generatorConfig)
: generatorConfig(generatorConfig) {
}
fs::path basePath{};
std::shared_ptr<SymbolService> 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!

View File

@ -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<std::pair<std::string_view, std::string_view>>
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;

View File

@ -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<fs::path> containerConfigPaths{};
std::set<std::string> defaultHeaders{};
std::set<std::string> defaultNamespaces{};
std::vector<std::pair<std::string, std::string>> membersToStub{};
FeatureSet features;
std::set<fs::path> containerConfigPaths;
std::set<std::string> defaultHeaders;
std::set<std::string> defaultNamespaces;
std::vector<std::pair<std::string, std::string>> membersToStub;
std::vector<ContainerInfo> passThroughTypes;
std::string toString() const;
std::vector<std::string> 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<drgn_type*, drgn_type*> pointerToTypeMap;
std::set<drgn_type*> thriftIssetStructTypes;
std::vector<drgn_type*> topoSortedStructTypes;
std::vector<std::pair<std::string, std::string>> membersToStub;
ContainerInfoRefSet containerTypesFuncDef;

View File

@ -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

View File

@ -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<SymbolService>(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<SymbolService>(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;
}

View File

@ -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<SymbolService> symbols;
OICache cache{};
OICache cache;
/*
* Map address of valid INT3 instruction to metadata for that interrupt.
@ -231,7 +234,7 @@ class OIDebugger {
std::optional<std::vector<uintptr_t>> findRetLocs(FuncDesc&);
OICompiler::Config compilerConfig{};
OICodeGen::Config generatorConfig{};
const OICodeGen::Config& generatorConfig;
TreeBuilder::Config treeBuilderConfig{};
std::optional<std::string> generateCode(const irequest&);

View File

@ -84,6 +84,25 @@ std::optional<ObjectIntrospection::FeatureSet> 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()) {

View File

@ -20,18 +20,10 @@
namespace type_graph {
// TODO:
// - read these from a TOML file
// - don't require specifying ctype and header
const std::array<ContainerInfo, 3> 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<ContainerInfo>& 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<Class*>(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 =

View File

@ -17,6 +17,7 @@
#include <array>
#include <unordered_set>
#include <vector>
#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<ContainerInfo>& passThroughTypes);
static bool isAllocator(Type& t);
TypeIdentifier(TypeGraph& typeGraph) : typeGraph_(typeGraph) {
TypeIdentifier(TypeGraph& typeGraph,
const std::vector<ContainerInfo>& passThroughTypes)
: typeGraph_(typeGraph), passThroughTypes_(passThroughTypes) {
}
using RecursiveVisitor::visit;
@ -48,8 +51,7 @@ class TypeIdentifier : public RecursiveVisitor {
private:
std::unordered_set<Type*> visited_;
TypeGraph& typeGraph_;
static const std::array<ContainerInfo, 3> dummyContainers_;
const std::vector<ContainerInfo>& passThroughTypes_;
};
} // namespace type_graph

View File

@ -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 = [

View File

@ -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

View File

@ -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: