JakeHillion/object-introspection
1
0
Fork 0
mirror of https://github.com/JakeHillion/object-introspection.git synced 2024-09-19 11:09:05 +01:00
Code Issues Packages Projects Releases Wiki Activity

tbv2: support capture-thrift-isset

Browse Source 
Support the capture-thrift-isset feature with TreeBuilder-v2. Fairly minor
changes here except the type of the Enum in a template parameter now matters.

We follow the previous behaviour of capturing a value for each field in a
struct that has an `isset_bitset`. This value is a VarInt captured before the
C++ contents of the member. It has 3 values: 0 (not set), 1 (set), and 2
(unavailable). These are handled by the processor and represented in the output
as `false`, `true`, and `std::nullopt_t` respectively.

Changes:
- Add a simple Thrift isset processor before any fields that have Thrift isset.
- Store the fully qualified names of enum types in DrgnParser - it already
  worked out this information anyway for naming the values and this is
  consistent with classes.
- Forward all enum template parameters under their input name under the
  assumption that they will all be policy type things like `IssetBitsetOption`.
  This could turn out to be wrong.

Test plan:
- CI (doesn't test thrift changes but covers other regressions)
- Updated Thrift enum tests for new format.
- `FILTER='OilIntegration.*' make test` - Thrift tests failed before, succeed
  after.
This commit is contained in:
Jake Hillion 2024-01-16 16:23:52 +00:00 committed by Jake Hillion
parent 4975b6e9fa
commit 40af807d8b
9 changed files with 263 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
include/oi/exporters/Json.h
View File

@ -158,7 +158,7 @@ inline void Json::printFields(const result::Element& el,
printUnsignedField("capacity", el.container_stats->capacity, indent);
}
if (el.is_set_stats.has_value())
printUnsignedField("is_set", el.is_set_stats->is_set, indent);
printBoolField("is_set", el.is_set_stats->is_set, indent);
printBoolField("is_primitive", el.is_primitive, indent);
}

78
oi/CodeGen.cpp
View File

@ -687,7 +687,9 @@ void CodeGen::genClassTraversalFunction(const Class& c, std::string& code) {
}
if (thriftIssetMember != nullptr && thriftIssetMember != &member) {
code += "\n .write(getThriftIsset(t, " + std::to_string(i) + "))";
code += "\n .write(getThriftIsset(t, ";
code += std::to_string(i);
code += "))";
}
code += "\n .";
@ -765,6 +767,12 @@ void CodeGen::genClassStaticType(const Class& c, std::string& code) {
void CodeGen::genClassTreeBuilderInstructions(const Class& c,
std::string& code) {
const Member* thriftIssetMember = nullptr;
if (const auto it = thriftIssetMembers_.find(&c);
it != thriftIssetMembers_.end()) {
thriftIssetMember = it->second;
}
code += " private:\n";
size_t index = 0;
for (const auto& m : c.members) {
@ -799,12 +807,29 @@ void CodeGen::genClassTreeBuilderInstructions(const Class& c,
continue;
std::string fullName = c.name() + "::" + m.name;
bool isPrimitive = dynamic_cast<const Primitive*>(&m.type());
code += " inst::Field{sizeof(" + fullName + "), " +
std::to_string(calculateExclusiveSize(m.type())) + ",\"" +
m.inputName + "\", member_" + std::to_string(index) +
"_type_names, TypeHandler<Ctx, decltype(" + fullName +
")>::fields, TypeHandler<Ctx, decltype(" + fullName +
")>::processors, ";
code += " inst::Field{sizeof(";
code += fullName;
code += "), ";
code += std::to_string(calculateExclusiveSize(m.type()));
code += ",\"";
code += m.inputName;
code += "\", member_";
code += std::to_string(index);
code += "_type_names, TypeHandler<Ctx, decltype(";
code += fullName;
code += ")>::fields, ";
if (thriftIssetMember != nullptr && thriftIssetMember != &m) {
code += "ThriftIssetHandler<";
}
code += "TypeHandler<Ctx, decltype(";
code += fullName;
code += ")>";
if (thriftIssetMember != nullptr && thriftIssetMember != &m) {
code += '>';
}
code += "::processors, ";
code += isPrimitive ? "true" : "false";
code += "},\n";
}
@ -825,10 +850,10 @@ void CodeGen::genClassTypeHandler(const Class& c, std::string& code) {
static int getThriftIsset(const %1%& t, size_t i) {
using thrift_data = apache::thrift::TStructDataStorage<%2%>;
if (&thrift_data::isset_indexes == nullptr) return -1;
if (&thrift_data::isset_indexes == nullptr) return 2;
auto idx = thrift_data::isset_indexes[i];
if (idx == -1) return -1;
if (idx == -1) return 2;
return t.%3%.get(idx);
}
@ -900,7 +925,15 @@ void genContainerTypeHandler(std::unordered_set<const ContainerInfo*>& used,
for (const auto& p : templateParams) {
if (p.value) {
code += ", ";
code += p.type().name();
// HACK: forward all enums directly. this might turn out to be a problem
// if there are enums we should be regenerating/use in the body.
if (const auto* e = dynamic_cast<const Enum*>(&p.type())) {
code += e->inputName();
} else {
code += p.type().name();
}
code += " N" + std::to_string(values++);
} else {
code += ", typename T" + std::to_string(types++);
@ -1048,11 +1081,32 @@ void addCaptureKeySupport(std::string& code) {
)";
}
void addThriftIssetSupport(std::string& code) {
code += R"(
void processThriftIsset(result::Element& el, std::function<void(inst::Inst)> stack_ins, ParsedData d) {
auto v = std::get<ParsedData::VarInt>(d.val).value;
if (v <= 1) {
el.is_set_stats.emplace(result::Element::IsSetStats { v == 1 });
}
}
static constexpr exporters::inst::ProcessorInst thriftIssetProcessor{
types::st::VarInt<int>::describe,
&processThriftIsset,
};
template <typename Handler>
struct ThriftIssetHandler {
static constexpr auto processors = arrayPrepend(Handler::processors, thriftIssetProcessor);
};
)";
}
void addStandardTypeHandlers(TypeGraph& typeGraph,
FeatureSet features,
std::string& code) {
if (features[Feature::TreeBuilderV2])
addCaptureKeySupport(code);
addCaptureKeySupport(code);
if (features[Feature::CaptureThriftIsset])
addThriftIssetSupport(code);
// Provide a wrapper function, getSizeType, to infer T instead of having to
// explicitly specify it with TypeHandler<Ctx, T>::getSizeType every time.

23
oi/OITraceCode.cpp
View File

@ -40,6 +40,13 @@ constexpr int oidMagicId = 0x01DE8;
namespace {
template <typename T, size_t N>
constexpr std::array<T, N + 1> arrayPrepend(std::array<T, N> a, T t);
template <typename T, size_t N, size_t... I>
constexpr std::array<T, N + 1> arrayPrependHelper(std::array<T, N> a,
T t,
std::index_sequence<I...>);
template <size_t Size = (1 << 20) / sizeof(uintptr_t)>
class PointerHashSet {
private:
@ -182,6 +189,22 @@ bool isStorageInline(const auto& c) {
(uintptr_t)std::data(c) >= (uintptr_t)&c;
}
namespace {
template <typename T, size_t N, size_t... I>
constexpr std::array<T, N + 1> arrayPrependHelper(std::array<T, N> a,
T t,
std::index_sequence<I...>) {
return {t, a[I]...};
}
template <typename T, size_t N>
constexpr std::array<T, N + 1> arrayPrepend(std::array<T, N> a, T t) {
return arrayPrependHelper(a, t, std::make_index_sequence<N>());
}
} // namespace
namespace OIInternal {
namespace {

5
oi/type_graph/ClangTypeParser.cpp
View File

@ -137,6 +137,8 @@ Typedef& ClangTypeParser::enumerateTypedef(const clang::TypedefType& ty) {
}
Enum& ClangTypeParser::enumerateEnum(const clang::EnumType& ty) {
std::string fqName = clang::TypeName::getFullyQualifiedName(
clang::QualType(&ty, 0), *ast, {ast->getLangOpts()});
std::string name = ty.getDecl()->getNameAsString();
auto size = ast->getTypeSize(clang::QualType(&ty, 0)) / 8;
@ -148,7 +150,8 @@ Enum& ClangTypeParser::enumerateEnum(const clang::EnumType& ty) {
}
}
return makeType<Enum>(ty, std::move(name), size, std::move(enumeratorMap));
return makeType<Enum>(
ty, std::move(name), std::move(fqName), size, std::move(enumeratorMap));
}
Array& ClangTypeParser::enumerateArray(const clang::ConstantArrayType& ty) {

34
oi/type_graph/DrgnParser.cpp
View File

@ -73,6 +73,8 @@ void warnForDrgnError(struct drgn_type* type,
const std::string& msg,
struct drgn_error* err);
std::string getDrgnFullyQualifiedName(struct drgn_type* type);
} // namespace
Type& DrgnParser::parse(struct drgn_type* root) {
@ -150,6 +152,8 @@ Class& DrgnParser::enumerateClass(struct drgn_type* type) {
std::string fqName;
char* nameStr = nullptr;
size_t length = 0;
// HACK: Leak this error. Freeing it causes a SEGV which suggests our
// underlying implementation is bad.
auto* err = drgn_type_fully_qualified_name(type, &nameStr, &length);
if (err == nullptr && nameStr != nullptr) {
fqName = nameStr;
@ -307,14 +311,7 @@ void DrgnParser::enumerateTemplateParam(struct drgn_type* type,
// This template parameter is a value
std::string value;
if (drgn_type_kind(obj->type) == DRGN_TYPE_ENUM) {
char* nameStr = nullptr;
size_t length = 0;
err = drgn_type_fully_qualified_name(obj->type, &nameStr, &length);
if (err != nullptr || nameStr == nullptr) {
throw DrgnParserError{"Failed to get enum's fully qualified name", err};
}
if (const auto* e = dynamic_cast<const Enum*>(&ttype)) {
uint64_t enumVal;
switch (obj->encoding) {
case DRGN_OBJECT_ENCODING_SIGNED:
@ -333,7 +330,9 @@ void DrgnParser::enumerateTemplateParam(struct drgn_type* type,
size_t numEnumerators = drgn_type_num_enumerators(obj->type);
for (size_t j = 0; j < numEnumerators; j++) {
if (enumerators[j].uvalue == enumVal) {
value = std::string{nameStr} + "::" + enumerators[j].name;
value = e->inputName();
value += "::";
value += enumerators[j].name;
break;
}
}
@ -416,6 +415,8 @@ void DrgnParser::enumerateClassFunctions(struct drgn_type* type,
}
Enum& DrgnParser::enumerateEnum(struct drgn_type* type) {
std::string fqName = getDrgnFullyQualifiedName(type);
const char* typeTag = drgn_type_tag(type);
std::string name = typeTag ? typeTag : "";
uint64_t size = get_drgn_type_size(type);
@ -432,7 +433,8 @@ Enum& DrgnParser::enumerateEnum(struct drgn_type* type) {
}
}
return makeType<Enum>(type, name, size, std::move(enumeratorMap));
return makeType<Enum>(
type, std::move(name), std::move(fqName), size, std::move(enumeratorMap));
}
Typedef& DrgnParser::enumerateTypedef(struct drgn_type* type) {
@ -528,6 +530,18 @@ void warnForDrgnError(struct drgn_type* type,
<< ": " << err->code << " " << err->message;
drgn_error_destroy(err);
}
std::string getDrgnFullyQualifiedName(drgn_type* type) {
char* nameStr = nullptr;
size_t length = 0;
auto* err = drgn_type_fully_qualified_name(type, &nameStr, &length);
if (err != nullptr)
throw DrgnParserError("failed to get fully qualified name!", err);
if (nameStr != nullptr)
return nameStr;
return {};
}
} // namespace
} // namespace oi::detail::type_graph

10
oi/type_graph/Types.h
View File

@ -477,14 +477,20 @@ class Container : public Type {
class Enum : public Type {
public:
explicit Enum(std::string name,
std::string inputName,
size_t size,
std::map<int64_t, std::string> enumerators = {})
: name_(name),
inputName_(std::move(name)),
: name_(std::move(name)),
inputName_(std::move(inputName)),
size_(size),
enumerators_(std::move(enumerators)) {
}
explicit Enum(std::string name,
size_t size,
std::map<int64_t, std::string> enumerators = {})
: Enum{name, std::move(name), size, std::move(enumerators)} {};
static inline constexpr bool has_node_id = false;
DECLARE_ACCEPT

6
test/integration/enums.toml
View File

@ -36,12 +36,12 @@ definitions = '''
param_types = ["ScopedEnum"]
setup = "return {};"
expect_json = '[{"staticSize":4, "dynamicSize":0}]'
expect_json_v2 = '[{"typeNames": ["ScopedEnum"], "staticSize":4, "exclusiveSize":4, "size":4}]'
expect_json_v2 = '[{"typeNames": ["ns_enums::ScopedEnum"], "staticSize":4, "exclusiveSize":4, "size":4}]'
[cases.scoped_uint8]
param_types = ["ScopedEnumUint8"]
setup = "return {};"
expect_json = '[{"staticSize":1, "dynamicSize":0}]'
expect_json_v2 = '[{"typeNames": ["ScopedEnumUint8"], "staticSize":1, "exclusiveSize":1, "size":1}]'
expect_json_v2 = '[{"typeNames": ["ns_enums::ScopedEnumUint8"], "staticSize":1, "exclusiveSize":1, "size":1}]'
[cases.unscoped]
param_types = ["UNSCOPED_ENUM"]
setup = "return {};"
@ -60,7 +60,7 @@ definitions = '''
expect_json = '[{"staticSize":2, "dynamicSize":0}]'
expect_json_v2 = '''[
{"staticSize": 2, "exclusiveSize": 0, "size":2, "members": [
{"typeNames": ["ScopedEnumUint8"], "staticSize":1, "exclusiveSize":1, "size":1},
{"typeNames": ["ns_enums::ScopedEnumUint8"], "staticSize":1, "exclusiveSize":1, "size":1},
{"typeNames": ["uint8_t"], "staticSize":1, "exclusiveSize":1, "size":1}
]}
]'''

92
test/integration/thrift_isset.toml
View File

@ -77,7 +77,6 @@ namespace cpp2 {
[cases]
[cases.unpacked]
oil_skip = 'oil does not support thrift isset yet' # https://github.com/facebookexperimental/object-introspection/issues/296
param_types = ["const cpp2::MyThriftStructUnpacked&"]
setup = '''
cpp2::MyThriftStructUnpacked ret;
@ -95,9 +94,19 @@ namespace cpp2 {
{"name":"__fbthrift_field_c", "staticSize":4, "isset":true},
{"name":"__isset", "staticSize":3}
]}]'''
expect_json_v2 = '''[{
"staticSize":16,
"exclusiveSize":1,
"size":16,
"members":[
{"name":"__fbthrift_field_a", "staticSize":4, "is_set":true},
{"name":"__fbthrift_field_b", "staticSize":4, "is_set":false},
{"name":"__fbthrift_field_c", "staticSize":4, "is_set":true},
{"name":"__isset", "staticSize":3, "NOT":"is_set"}
]
}]'''
[cases.packed]
oil_skip = 'oil does not support thrift isset yet' # https://github.com/facebookexperimental/object-introspection/issues/296
param_types = ["const cpp2::MyThriftStructPacked&"]
setup = '''
cpp2::MyThriftStructPacked ret;
@ -126,9 +135,25 @@ namespace cpp2 {
{"name":"__fbthrift_field_j", "staticSize":4, "isset":true},
{"name":"__isset", "staticSize":2}
]}]'''
expect_json_v2 = '''[{
"staticSize":44,
"exclusiveSize":2,
"size":44,
"members":[
{"name":"__fbthrift_field_a", "staticSize":4, "is_set":true},
{"name":"__fbthrift_field_b", "staticSize":4, "is_set":false},
{"name":"__fbthrift_field_c", "staticSize":4, "is_set":true},
{"name":"__fbthrift_field_d", "staticSize":4, "is_set":true},
{"name":"__fbthrift_field_e", "staticSize":4, "is_set":false},
{"name":"__fbthrift_field_f", "staticSize":4, "is_set":false},
{"name":"__fbthrift_field_g", "staticSize":4, "is_set":true},
{"name":"__fbthrift_field_h", "staticSize":4, "is_set":true},
{"name":"__fbthrift_field_i", "staticSize":4, "is_set":false},
{"name":"__fbthrift_field_j", "staticSize":4, "is_set":true},
{"name":"__isset", "staticSize":2, "NOT":"is_set"}
]}]'''
[cases.packed_non_atomic]
oil_skip = 'oil does not support thrift isset yet' # https://github.com/facebookexperimental/object-introspection/issues/296
param_types = ["const cpp2::MyThriftStructPackedNonAtomic&"]
setup = '''
cpp2::MyThriftStructPackedNonAtomic ret;
@ -147,9 +172,19 @@ namespace cpp2 {
{"name":"__fbthrift_field_d", "staticSize":4, "isset":false},
{"name":"__isset", "staticSize":1}
]}]'''
expect_json_v2 = '''[{
"staticSize":20,
"exclusiveSize":3,
"size":20,
"members":[
{"name":"__fbthrift_field_a", "staticSize":4, "is_set":true},
{"name":"__fbthrift_field_b", "staticSize":4, "is_set":false},
{"name":"__fbthrift_field_c", "staticSize":4, "is_set":true},
{"name":"__fbthrift_field_d", "staticSize":4, "is_set":false},
{"name":"__isset", "staticSize":1, "NOT":"is_set"}
]}]'''
[cases.out_of_order]
oil_skip = 'oil does not support thrift isset yet' # https://github.com/facebookexperimental/object-introspection/issues/296
param_types = ["const cpp2::MyThriftStructOutOfOrder&"]
setup = '''
cpp2::MyThriftStructOutOfOrder ret;
@ -166,9 +201,18 @@ namespace cpp2 {
{"name":"__fbthrift_field_c", "staticSize":4, "isset":false},
{"name":"__isset", "staticSize":3}
]}]'''
expect_json_v2 = '''[{
"staticSize":16,
"exclusiveSize":1,
"size":16,
"members":[
{"name":"__fbthrift_field_a", "staticSize":4, "is_set":false},
{"name":"__fbthrift_field_b", "staticSize":4, "is_set":true},
{"name":"__fbthrift_field_c", "staticSize":4, "is_set":false},
{"name":"__isset", "staticSize":3, "NOT":"is_set"}
]}]'''
[cases.required]
oil_skip = 'oil does not support thrift isset yet' # https://github.com/facebookexperimental/object-introspection/issues/296
param_types = ["const cpp2::MyThriftStructRequired&"]
setup = '''
cpp2::MyThriftStructRequired ret;
@ -189,9 +233,21 @@ namespace cpp2 {
{"name":"__fbthrift_field_f", "staticSize":4, "isset":true},
{"name":"__isset", "staticSize":3}
]}]'''
expect_json_v2 = '''[{
"staticSize":28,
"exclusiveSize":1,
"size":28,
"members":[
{"name":"__fbthrift_field_a", "staticSize":4, "NOT":"is_set"},
{"name":"__fbthrift_field_b", "staticSize":4, "is_set":true},
{"name":"__fbthrift_field_c", "staticSize":4, "is_set":false},
{"name":"__fbthrift_field_d", "staticSize":4, "NOT":"is_set"},
{"name":"__fbthrift_field_e", "staticSize":4, "NOT":"is_set"},
{"name":"__fbthrift_field_f", "staticSize":4, "is_set":true},
{"name":"__isset", "staticSize":3, "NOT":"is_set"}
]}]'''
[cases.box]
oil_skip = 'oil does not support thrift isset yet' # https://github.com/facebookexperimental/object-introspection/issues/296
param_types = ["const cpp2::MyThriftStructBoxed&"]
setup = '''
cpp2::MyThriftStructBoxed ret;
@ -211,6 +267,18 @@ namespace cpp2 {
{"name":"__fbthrift_field_e", "staticSize":4, "isset":true},
{"name":"__isset", "staticSize":3}
]}]'''
expect_json_v2 = '''[{
"staticSize":32,
"exclusiveSize":1,
"size":32,
"members":[
{"name":"__fbthrift_field_a", "staticSize":8, "NOT":"is_set"},
{"name":"__fbthrift_field_b", "staticSize":8, "NOT":"is_set"},
{"name":"__fbthrift_field_c", "staticSize":4, "is_set":false},
{"name":"__fbthrift_field_d", "staticSize":4, "is_set":true},
{"name":"__fbthrift_field_e", "staticSize":4, "is_set":true},
{"name":"__isset", "staticSize":3, "NOT":"is_set"}
]}]'''
[cases.no_capture]
param_types = ["const cpp2::MyThriftStructBoxed&"]
@ -231,3 +299,15 @@ namespace cpp2 {
{"name":"__fbthrift_field_e", "staticSize":4, "NOT":"isset"},
{"name":"__isset", "staticSize":3}
]}]'''
expect_json_v2 = '''[{
"staticSize":32,
"exclusiveSize":1,
"size":32,
"members":[
{"name":"__fbthrift_field_a", "staticSize":8, "NOT":"is_set"},
{"name":"__fbthrift_field_b", "staticSize":8, "NOT":"is_set"},
{"name":"__fbthrift_field_c", "staticSize":4, "NOT":"is_set"},
{"name":"__fbthrift_field_d", "staticSize":4, "NOT":"is_set"},
{"name":"__fbthrift_field_e", "staticSize":4, "NOT":"is_set"},
{"name":"__isset", "staticSize":3, "NOT":"is_set"}
]}]'''

51
test/integration/thrift_isset_missing.toml
View File

@ -58,7 +58,6 @@ raw_definitions = '''
'''
[cases]
[cases.present]
oil_skip = 'oil does not support thrift isset yet' # https://github.com/facebookexperimental/object-introspection/issues/296
param_types = ["const FakeThriftWithData&"]
setup = '''
FakeThriftWithData ret;
@ -78,8 +77,17 @@ raw_definitions = '''
{"name":"__fbthrift_field_c", "staticSize":4, "isset":true},
{"name":"__isset", "staticSize":3}
]}]'''
expect_json_v2 = '''[{
"staticSize":16,
"exclusiveSize":1,
"size":16,
"members":[
{"name":"__fbthrift_field_a", "staticSize":4, "is_set":true},
{"name":"__fbthrift_field_b", "staticSize":4, "is_set":false},
{"name":"__fbthrift_field_c", "staticSize":4, "is_set":true},
{"name":"__isset", "staticSize":3, "NOT":"is_set"}
]}]'''
[cases.missing]
oil_skip = 'oil does not support thrift isset yet' # https://github.com/facebookexperimental/object-introspection/issues/296
param_types = ["const FakeThriftWithoutData&"]
setup = '''
FakeThriftWithoutData ret;
@ -99,8 +107,17 @@ raw_definitions = '''
{"name":"__fbthrift_field_c", "staticSize":4, "NOT":"isset"},
{"name":"__isset", "staticSize":3}
]}]'''
expect_json_v2 = '''[{
"staticSize":16,
"exclusiveSize":1,
"size":16,
"members":[
{"name":"__fbthrift_field_a", "staticSize":4, "NOT":"is_set"},
{"name":"__fbthrift_field_b", "staticSize":4, "NOT":"is_set"},
{"name":"__fbthrift_field_c", "staticSize":4, "NOT":"is_set"},
{"name":"__isset", "staticSize":3, "NOT":"is_set"}
]}]'''
[cases.mixed]
oil_skip = 'oil does not support thrift isset yet' # https://github.com/facebookexperimental/object-introspection/issues/296
param_types = ["const Mixed&"]
setup = '''
Mixed ret;
@ -143,3 +160,31 @@ raw_definitions = '''
]
}
]}]'''
expect_json_v2 = '''[{
"staticSize":32,
"exclusiveSize":0,
"size":32,
"members":[
{
"staticSize":16,
"exclusiveSize":1,
"size":16,
"members":[
{"name":"__fbthrift_field_a", "staticSize":4, "is_set":true},
{"name":"__fbthrift_field_b", "staticSize":4, "is_set":false},
{"name":"__fbthrift_field_c", "staticSize":4, "is_set":true},
{"name":"__isset", "staticSize":3, "NOT":"is_set"}
]
},
{
"staticSize":16,
"exclusiveSize":1,
"size":16,
"members":[
{"name":"__fbthrift_field_a", "staticSize":4, "NOT":"is_set"},
{"name":"__fbthrift_field_b", "staticSize":4, "NOT":"is_set"},
{"name":"__fbthrift_field_c", "staticSize":4, "NOT":"is_set"},
{"name":"__isset", "staticSize":3, "NOT":"is_set"}
]
}
]}]'''