diff --git a/include/oi/exporters/Json.h b/include/oi/exporters/Json.h index 3dd37f4..3416ef8 100644 --- a/include/oi/exporters/Json.h +++ b/include/oi/exporters/Json.h @@ -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); } diff --git a/oi/CodeGen.cpp b/oi/CodeGen.cpp index cb5bf54..a125508 100644 --- a/oi/CodeGen.cpp +++ b/oi/CodeGen.cpp @@ -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(&m.type()); - code += " inst::Field{sizeof(" + fullName + "), " + - std::to_string(calculateExclusiveSize(m.type())) + ",\"" + - m.inputName + "\", member_" + std::to_string(index) + - "_type_names, TypeHandler::fields, TypeHandler::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; - 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& 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(&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 stack_ins, ParsedData d) { + auto v = std::get(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::describe, + &processThriftIsset, +}; + +template +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::getSizeType every time. diff --git a/oi/OITraceCode.cpp b/oi/OITraceCode.cpp index 323c83e..362ce69 100644 --- a/oi/OITraceCode.cpp +++ b/oi/OITraceCode.cpp @@ -40,6 +40,13 @@ constexpr int oidMagicId = 0x01DE8; namespace { +template +constexpr std::array arrayPrepend(std::array a, T t); +template +constexpr std::array arrayPrependHelper(std::array a, + T t, + std::index_sequence); + template class PointerHashSet { private: @@ -182,6 +189,22 @@ bool isStorageInline(const auto& c) { (uintptr_t)std::data(c) >= (uintptr_t)&c; } +namespace { + +template +constexpr std::array arrayPrependHelper(std::array a, + T t, + std::index_sequence) { + return {t, a[I]...}; +} + +template +constexpr std::array arrayPrepend(std::array a, T t) { + return arrayPrependHelper(a, t, std::make_index_sequence()); +} + +} // namespace + namespace OIInternal { namespace { diff --git a/oi/type_graph/ClangTypeParser.cpp b/oi/type_graph/ClangTypeParser.cpp index 24d5835..cb33377 100644 --- a/oi/type_graph/ClangTypeParser.cpp +++ b/oi/type_graph/ClangTypeParser.cpp @@ -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(ty, std::move(name), size, std::move(enumeratorMap)); + return makeType( + ty, std::move(name), std::move(fqName), size, std::move(enumeratorMap)); } Array& ClangTypeParser::enumerateArray(const clang::ConstantArrayType& ty) { diff --git a/oi/type_graph/DrgnParser.cpp b/oi/type_graph/DrgnParser.cpp index 0116edf..73eb531 100644 --- a/oi/type_graph/DrgnParser.cpp +++ b/oi/type_graph/DrgnParser.cpp @@ -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(&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(type, name, size, std::move(enumeratorMap)); + return makeType( + 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 diff --git a/oi/type_graph/Types.h b/oi/type_graph/Types.h index f09f926..45b7d06 100644 --- a/oi/type_graph/Types.h +++ b/oi/type_graph/Types.h @@ -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 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 enumerators = {}) + : Enum{name, std::move(name), size, std::move(enumerators)} {}; + static inline constexpr bool has_node_id = false; DECLARE_ACCEPT diff --git a/test/integration/enums.toml b/test/integration/enums.toml index fa91cbc..f5274a3 100644 --- a/test/integration/enums.toml +++ b/test/integration/enums.toml @@ -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} ]} ]''' diff --git a/test/integration/thrift_isset.toml b/test/integration/thrift_isset.toml index 98e19fd..cdbcdb5 100644 --- a/test/integration/thrift_isset.toml +++ b/test/integration/thrift_isset.toml @@ -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"} + ]}]''' diff --git a/test/integration/thrift_isset_missing.toml b/test/integration/thrift_isset_missing.toml index 3e7ad94..311f736 100644 --- a/test/integration/thrift_isset_missing.toml +++ b/test/integration/thrift_isset_missing.toml @@ -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"} + ] + } + ]}]'''