From 312bc94ac922ed8ffa76b606be409858c37fc329 Mon Sep 17 00:00:00 2001 From: Matthew Olsson Date: Sat, 6 Apr 2024 09:44:32 -0700 Subject: [PATCH] LibJSGCVerifier: Detect missing JS_DECLARE_ALLOCATOR() calls C++ classes that inherit from JS::Cell and are leaf classes should have their own type-specific allocator. We also do this for non-leaf classes that are constructable from JS. To do this, JSON messages are passed to communicate information about each class the Clang tool comes across. This is the only message we have to worry about for now, but in the future if we want to transmit different kinds of information, we can make this message format more generic. --- .../LibJSGCVerifier/src/CellsHandler.cpp | 30 +++++++++++++++++++ Meta/Lagom/Tools/LibJSGCVerifier/src/main.py | 14 +++++++++ 2 files changed, 44 insertions(+) diff --git a/Meta/Lagom/Tools/LibJSGCVerifier/src/CellsHandler.cpp b/Meta/Lagom/Tools/LibJSGCVerifier/src/CellsHandler.cpp index 9c242ae6d9f..91bc064ce07 100644 --- a/Meta/Lagom/Tools/LibJSGCVerifier/src/CellsHandler.cpp +++ b/Meta/Lagom/Tools/LibJSGCVerifier/src/CellsHandler.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -230,6 +231,33 @@ FieldValidationResult validate_field(clang::FieldDecl const* field_decl) return result; } +void emit_record_json_data(clang::CXXRecordDecl const& record) +{ + llvm::json::Object obj; + obj.insert({ "name", record.getQualifiedNameAsString() }); + + std::vector bases; + record.forallBases([&](clang::CXXRecordDecl const* base) { + bases.push_back(base->getQualifiedNameAsString()); + return true; + }); + obj.insert({ "parents", bases }); + + bool has_cell_allocator = false; + bool has_js_constructor = false; + for (auto const& decl : record.decls()) { + if (auto* var_decl = llvm::dyn_cast(decl); var_decl && var_decl->getQualifiedNameAsString().ends_with("::cell_allocator")) { + has_cell_allocator = true; + } else if (auto* fn_decl = llvm::dyn_cast(decl); fn_decl && fn_decl->getQualifiedNameAsString().ends_with("::construct_impl")) { + has_js_constructor = true; + } + } + obj.insert({ "has_cell_allocator", has_cell_allocator }); + obj.insert({ "has_js_constructor", has_js_constructor }); + + llvm::outs() << std::move(obj) << "\n"; +} + void CollectCellsHandler::run(clang::ast_matchers::MatchFinder::MatchResult const& result) { check_cells(result); @@ -278,6 +306,8 @@ void CollectCellsHandler::check_cells(clang::ast_matchers::MatchFinder::MatchRes if (!record_inherits_from_cell(*record)) return; + emit_record_json_data(*record); + clang::DeclarationName const name = &result.Context->Idents.get("visit_edges"); auto const* visit_edges_method = record->lookup(name).find_first(); if (!visit_edges_method && !fields_that_need_visiting.empty()) { diff --git a/Meta/Lagom/Tools/LibJSGCVerifier/src/main.py b/Meta/Lagom/Tools/LibJSGCVerifier/src/main.py index ef1e7c0441a..9a1f8d38ed1 100755 --- a/Meta/Lagom/Tools/LibJSGCVerifier/src/main.py +++ b/Meta/Lagom/Tools/LibJSGCVerifier/src/main.py @@ -92,3 +92,17 @@ with multiprocessing.Pool(processes=multiprocessing.cpu_count() - 2, initializer except KeyboardInterrupt: pool.terminate() pool.join() + +# Process output data +clang_results = {r['name']: r for r in clang_results} +leaf_objects = set(clang_results.keys()) +for result in clang_results.values(): + leaf_objects.difference_update(result['parents']) + +for key, value in clang_results.items(): + if key == 'JS::HeapBlock::FreelistEntry' or key == 'JS::HeapFunction': + # These are Heap-related classes and don't need their own allocator + continue + + if not value['has_cell_allocator'] and (key in leaf_objects or value['has_js_constructor']): + print(f'Class {key} is missing a JS_DECLARE_ALLOCATOR() declaration in its header file')