From 213dba4325c1fedfd65973de6f04fb23beaed5b7 Mon Sep 17 00:00:00 2001 From: Jake Hillion Date: Thu, 4 Jan 2024 11:53:19 +0000 Subject: [PATCH] oilgen: add to integration test framework TODO: Replace the references to local paths. oilgen (the basis of Ahead Of Time compilation for OIL) has never been passed through our large test suite and has instead had more focused testing and large examples. When consuming DWARF information in a similar fashion to JIT OIL this was okay, but with the new Clang AST based mechanism it means we have very little coverage. This change adds an oilgen test for every test case that has an oil test. Relying on the build system to create the test target as before would make it difficult to have failing tests, so we move the build into the integration test runner. This involves: 1. Writing the input source to a file. 2. Consuming it with oilgen to get the implementation object file. 3. Compiling the input source and linking it with this file. 4. Running the newly created target. This approach can give the full error message at any stage that fails and will fail the test appropriately. The downside is the build system integration is more difficult, as we need the correct compiler flags for the target and to use the correct compiler. It would be very tricky to replicate this in a build system that's not CMake, so we will likely only run these tests in open source. Test plan: - CI --- .circleci/config.yml | 2 +- oi/type_graph/ClangTypeParser.cpp | 4 +- oi/type_graph/Types.h | 2 +- test/integration/CMakeLists.txt | 11 +- test/integration/gen_tests.py | 153 ++++++++++++++++++++------ test/integration/runner_common.cpp | 165 +++++++++++++++++++++++++++++ test/integration/runner_common.h | 14 +++ 7 files changed, 314 insertions(+), 37 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index b91cc2c..a6c6b8f 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -14,7 +14,7 @@ workflows: name: test-gcc requires: - build-gcc - exclude_regex: ".*inheritance_polymorphic.*|.*arrays_member_int0" + exclude_regex: "OilgenIntegration\\..*|.*inheritance_polymorphic.*|.*arrays_member_int0" - coverage: name: coverage requires: diff --git a/oi/type_graph/ClangTypeParser.cpp b/oi/type_graph/ClangTypeParser.cpp index 24d5835..dfa3a07 100644 --- a/oi/type_graph/ClangTypeParser.cpp +++ b/oi/type_graph/ClangTypeParser.cpp @@ -350,6 +350,7 @@ Primitive& ClangTypeParser::enumeratePrimitive(const clang::BuiltinType& ty) { case clang::BuiltinType::WChar_U: return makeType(ty, Primitive::Kind::UInt32); + case clang::BuiltinType::Char8: case clang::BuiltinType::Char_S: case clang::BuiltinType::SChar: return makeType(ty, Primitive::Kind::Int8); @@ -380,8 +381,9 @@ Primitive& ClangTypeParser::enumeratePrimitive(const clang::BuiltinType& ty) { case clang::BuiltinType::Float: return makeType(ty, Primitive::Kind::Float32); case clang::BuiltinType::Double: - case clang::BuiltinType::LongDouble: return makeType(ty, Primitive::Kind::Float64); + case clang::BuiltinType::LongDouble: + return makeType(ty, Primitive::Kind::Float128); case clang::BuiltinType::UInt128: case clang::BuiltinType::Int128: diff --git a/oi/type_graph/Types.h b/oi/type_graph/Types.h index 1d5dd79..81cab11 100644 --- a/oi/type_graph/Types.h +++ b/oi/type_graph/Types.h @@ -592,7 +592,7 @@ class Primitive : public Type { Float32, Float64, Float80, // TODO worth including? - Float128, // TODO can we generate this? + Float128, Bool, StubbedPointer, diff --git a/test/integration/CMakeLists.txt b/test/integration/CMakeLists.txt index 4d235b5..b4fe3de 100644 --- a/test/integration/CMakeLists.txt +++ b/test/integration/CMakeLists.txt @@ -58,12 +58,18 @@ target_link_libraries(integration_test_runner PRIVATE GTest::gmock_main Boost::headers ${Boost_LIBRARIES} + range-v3 toml ) target_compile_definitions(integration_test_runner PRIVATE - TARGET_EXE_PATH="${CMAKE_CURRENT_BINARY_DIR}/integration_test_target" + TARGET_EXE_PATH="$" OID_EXE_PATH="$" - CONFIG_FILE_PATH="${CMAKE_BINARY_DIR}/testing.oid.toml") + OILGEN_EXE_PATH="$" + CONFIG_FILE_PATH="${CMAKE_BINARY_DIR}/testing.oid.toml" + + CXX="${CMAKE_CXX_COMPILER}" + TARGET_INCLUDE_DIRECTORIES="$,:>" +) if (${THRIFT_FOUND}) foreach(THRIFT_TEST IN LISTS THRIFT_TESTS) @@ -85,6 +91,7 @@ if (${THRIFT_FOUND}) add_custom_target(integration_test_thrift_sources_${THRIFT_TEST} DEPENDS ${THRIFT_TYPES_H}) add_dependencies(integration_test_target integration_test_thrift_sources_${THRIFT_TEST}) + add_dependencies(integration_test_runner integration_test_thrift_sources_${THRIFT_TEST}) target_sources(integration_test_target PRIVATE ${THRIFT_DATA_CPP}) endforeach() diff --git a/test/integration/gen_tests.py b/test/integration/gen_tests.py index b72e371..d9fe054 100644 --- a/test/integration/gen_tests.py +++ b/test/integration/gen_tests.py @@ -51,6 +51,41 @@ def add_headers(f, custom_headers, thrift_headers): f.write(f'#include "{header}"\n') +def add_test_getters(f, case_name, case): + param_types = ", ".join( + f"std::remove_cvref_t<{param}>" for param in case["param_types"] + ) + if "arg_types" in case: + arg_types = ", ".join(case["arg_types"]) + else: + arg_types = param_types + + f.write( + f"\n" + f" std::tuple<{arg_types}> get_{case_name}() {{\n" + f'{case["setup"]}\n' + f" }}\n" + ) + + +def get_param_str(param, i): + if "]" in param: + # Array param + + if ")" in param: + # "int(&)[5]" -> "int (&a0)[5]" + start, end = param.split(")") + return f"{start}a{i}){end}" + + # "int[5]" -> "int a0[5]" + # "int[5][10]" -> "int a0[5][10]" + type_name, array_size = param.split("[", 1) + return f"{type_name} a{i}[{array_size}" + + # Non-array param, e.g. "int&" -> "int& a0" + return f"{param} a{i}" + + def add_test_setup(f, config): ns = get_namespace(config["suite"]) # fmt: off @@ -65,23 +100,6 @@ def add_test_setup(f, config): ) # fmt: on - def get_param_str(param, i): - if "]" in param: - # Array param - - if ")" in param: - # "int(&)[5]" -> "int (&a0)[5]" - start, end = param.split(")") - return f"{start}a{i}){end}" - - # "int[5]" -> "int a0[5]" - # "int[5][10]" -> "int a0[5][10]" - type_name, array_size = param.split("[", 1) - return f"{type_name} a{i}[{array_size}" - - # Non-array param, e.g. "int&" -> "int& a0" - return f"{param} a{i}" - def define_traceable_func(name, params, body): return ( f"\n" @@ -99,21 +117,7 @@ def add_test_setup(f, config): # target func for it continue - # generate getter for an object of this type - param_types = ", ".join( - f"std::remove_cvref_t<{param}>" for param in case["param_types"] - ) - if "arg_types" in case: - arg_types = ", ".join(case["arg_types"]) - else: - arg_types = param_types - - f.write( - f"\n" - f" std::tuple<{arg_types}> get_{case_name}() {{\n" - f'{case["setup"]}\n' - f" }}\n" - ) + add_test_getters(f, case_name, case) # generate oid and oil targets params_str = ", ".join( @@ -266,6 +270,7 @@ def add_tests(f, config): for case_name, case in config["cases"].items(): add_oid_integration_test(f, config, case_name, case) add_oil_integration_test(f, config, case_name, case) + add_oilgen_integration_test(f, config, case_name, case) def add_oid_integration_test(f, config, case_name, case): @@ -400,6 +405,90 @@ def add_oil_integration_test(f, config, case_name, case): f.write(f"}}\n") +def add_oilgen_integration_test(f, config, case_name, case): + case_str = get_case_name(config["suite"], case_name) + exit_code = case.get("expect_oil_exit_code", 0) + + if "oil_disable" in case or "target_function" in case: + return + + config_prefix = case.get("config_prefix", "") + config_suffix = case.get("config_suffix", "") + + f.write( + f"\n" + f"TEST_F(OilgenIntegration, {case_str}) {{\n" + f"{generate_skip(case, 'oil')}" + ) + + f.write(' constexpr std::string_view targetSrc = R"--(') + headers = set(config.get("includes", [])) + thrift_headers = [f"thrift/annotation/gen-cpp2/{config['suite']}_types.h"] if is_thrift_test(config) else [] + add_headers(f, sorted(headers), thrift_headers) + + f.write( + f"\n" + f'{config.get("raw_definitions", "")}\n' + f"#pragma clang diagnostic push\n" + f'#pragma clang diagnostic ignored "-Wunused-private-field"\n' + f'{config.get("definitions", "")}\n' + f"#pragma clang diagnostic pop\n" + ) + add_test_getters(f, case_name, case) + + main = "int main() {\n" + main += " auto pr = oi::exporters::Json(std::cout);\n" + main += " pr.setPretty(true);\n" + main += f" auto val = get_{case_name}();\n" + for i in range(len(case["param_types"])): + main += f" auto ret{i} = oi::result::SizedResult(oi::introspect" + if "arg_types" in case: + main += f">" + main += f"(std::get<{i}>(val)));\n" + main += f" pr.print(ret{i});\n" + main += "}\n" + + f.write(main) + f.write(')--";\n') + + f.write( + f' std::string configPrefix = R"--({config_prefix})--";\n' + f' std::string configSuffix = R"--({config_suffix})--";\n' + f" ba::io_context ctx;\n" + f" auto target = runOilgenTarget({{\n" + f" .ctx = ctx,\n" + f" .targetSrc = targetSrc,\n" + f" }}, std::move(configPrefix), std::move(configSuffix));\n\n" + f" ASSERT_EQ(exit_code(target), {exit_code});\n" + ) + + key = "expect_json" + if "expect_json_v2" in case: + key = "expect_json_v2" + if key in case: + try: + json.loads(case[key]) + except json.decoder.JSONDecodeError as error: + print( + f"\x1b[31m`expect_json` value for test case {config['suite']}.{case_name} was invalid JSON: {error}\x1b[0m", + file=sys.stderr, + ) + sys.exit(1) + + f.write( + f"\n" + f" std::stringstream expected_json_ss;\n" + f' expected_json_ss << R"--({case[key]})--";\n' + f" auto result_json_ss = std::stringstream(stdout_);\n" + f" bpt::ptree expected_json, actual_json;\n" + f" bpt::read_json(expected_json_ss, expected_json);\n" + f" bpt::read_json(result_json_ss, actual_json);\n" + f" compare_json(expected_json, actual_json);\n" + ) + + f.write(f"}}\n") + + def generate_skip(case, specific): possibly_skip = "" skip_reason = case.get("skip", False) diff --git a/test/integration/runner_common.cpp b/test/integration/runner_common.cpp index 3c8525a..f67d3cc 100644 --- a/test/integration/runner_common.cpp +++ b/test/integration/runner_common.cpp @@ -4,11 +4,13 @@ #include #include #include +#include #include #include #include #include #include +#include #include #include @@ -447,3 +449,166 @@ Proc OilIntegration::runOilTarget(OilOpts opts, std::move(std_out), std::move(std_err)}; } + +std::string OilgenIntegration::TmpDirStr() { + return std::string("/tmp/oilgen-integration-XXXXXX"); +} + +Proc OilgenIntegration::runOilgenTarget(OilgenOpts opts, + std::string configPrefix, + std::string configSuffix) { + // Run an oilgen test in three stages. + // 1. Fake up a compilation database from the real CMake one. + // 1. Generate the OIL implementation .o from the input source. + // 2. Compile the input source and link to the implementation. + // 3. Run the now complete target. + + static constexpr std::string_view InputPath = "input_src.cpp"; + static constexpr std::string_view ObjectPath = "oil_generated.o"; + static constexpr std::string_view TargetPath = "generated_target"; + static const std::string CompileCommandsOutPath = "compile_commands.json"; + + static const char* CompileCommandsInPath = "/data/users/jakehillion/object-introspection-sl/build/compile_commands.json"; + + { + std::ofstream file{std::string(InputPath), std::ios_base::app}; + file << opts.targetSrc; + } + + bpt::ptree compile_commands_in; + bpt::ptree compile_commands_out; + bpt::read_json(CompileCommandsInPath, compile_commands_in); + for (bpt::ptree::value_type& entry : compile_commands_in) { + if (!entry.second.get("file").ends_with("integration_test_target.cpp")) + continue; + + auto input = std::filesystem::absolute(InputPath).string(); + entry.second.put("file", input); + compile_commands_out.add_child(bpt::ptree::path_type(input, ':'), entry.second); + break; + } + bpt::write_json(CompileCommandsOutPath, compile_commands_out); + + std::string generatorExe{OILGEN_EXE_PATH}; + generatorExe += " "; + generatorExe += InputPath; + generatorExe += " --output="; + generatorExe += ObjectPath; + generatorExe += " --debug-level=3"; + if (auto prefix = writeCustomConfig("prefix", configPrefix)) { + generatorExe += " --config-file "; + generatorExe += *prefix; + } + generatorExe += " --config-file "; + generatorExe += configFile; + if (auto suffix = writeCustomConfig("suffix", configSuffix)) { + generatorExe += " --config-file "; + generatorExe += *suffix; + } + + + std::vector clangArgs{ + "-DOIL_AOT_COMPILATION=1", + "--std=c++20", + "-resource-dir", + "/usr/lib64/clang/15.0.7", + }; + + for(auto&& rng : TARGET_INCLUDE_DIRECTORIES | ranges::views::split(':')) { + clangArgs.emplace_back("-I"); + clangArgs.emplace_back(&*rng.begin(), ranges::distance(rng)); + } + + for (const auto& arg : clangArgs) { + generatorExe += " --extra-arg="; + generatorExe += arg; + } + + if (verbose) { + std::cerr << "Running: " << generatorExe << std::endl; + } + + bp::child generatorProc{generatorExe, opts.ctx}; + generatorProc.wait(); + if (generatorProc.exit_code() != 0) + throw std::runtime_error("generation failed!"); + + std::string compilerExe{CXX}; + compilerExe += " input_src.cpp "; + compilerExe += ObjectPath; + compilerExe += " -o "; + compilerExe += TargetPath; + compilerExe += " /data/users/jakehillion/object-introspection-sl/oi/IntrospectionResult.cpp"; + compilerExe += " /data/users/jakehillion/object-introspection-sl/oi/exporters/ParsedData.cpp"; + + compilerExe += " -Wl,-rpath,/data/users/jakehillion/object-introspection-sl/extern/drgn/build/.libs:/data/users/jakehillion/object-introspection-sl/extern/drgn/build/velfutils/libdw:/data/users/jakehillion/object-introspection-sl/extern/drgn/build/velfutils/libelf:/data/users/jakehillion/object-introspection-sl/extern/drgn/build/velfutils/libdwelf:/home/jakehillion/fbsource/fbcode/third-party-buck/platform010/build/icu/lib:/home/jakehillion/fbsource/fbcode/third-party-buck/platform010/build/gflags/lib"; + for (const auto& arg : clangArgs) { + compilerExe += ' '; + compilerExe += arg; + } + + if (verbose) { + std::cerr << "Running: " << compilerExe << std::endl; + } + + bp::child compilerProc{compilerExe, opts.ctx}; + compilerProc.wait(); + if (compilerProc.exit_code() != 0) + throw std::runtime_error("compilation failed"); + + std::string targetExe = "./"; + targetExe += TargetPath; + + if (verbose) { + std::cerr << "Running: " << targetExe << std::endl; + } + + // Use tee to write the output to files. If verbose is on, also redirect the + // output to stderr. + bp::async_pipe std_out_pipe(opts.ctx), std_err_pipe(opts.ctx); + bp::child std_out, std_err; + if (verbose) { + // clang-format off + std_out = bp::child(bp::search_path("tee"), + (workingDir / "stdout").string(), + bp::std_in < std_out_pipe, + bp::std_out > stderr, + opts.ctx); + std_err = bp::child(bp::search_path("tee"), + (workingDir / "stderr").string(), + bp::std_in < std_err_pipe, + bp::std_out > stderr, + opts.ctx); + // clang-format on + } else { + // clang-format off + std_out = bp::child(bp::search_path("tee"), + (workingDir / "stdout").string(), + bp::std_in < std_out_pipe, + bp::std_out > bp::null, + opts.ctx); + std_err = bp::child(bp::search_path("tee"), + (workingDir / "stderr").string(), + bp::std_in < std_err_pipe, + bp::std_out > bp::null, + opts.ctx); + // clang-format on + } + + /* Spawn `oid` with tracing on and IOs redirected */ + // clang-format off + bp::child targetProc( + targetExe, + bp::std_in < bp::null, + bp::std_out > std_out_pipe, + bp::std_err > std_err_pipe, + opts.ctx); + // clang-format on + + return Proc{ + opts.ctx, + std::move(targetProc), + std::move(std_out), + std::move(std_err), + }; +} diff --git a/test/integration/runner_common.h b/test/integration/runner_common.h index d2b5e57..8c6c8eb 100644 --- a/test/integration/runner_common.h +++ b/test/integration/runner_common.h @@ -22,6 +22,11 @@ struct OilOpts { std::string targetArgs; }; +struct OilgenOpts { + boost::asio::io_context& ctx; + std::string_view targetSrc; +}; + struct Proc { boost::asio::io_context& ctx; boost::process::child proc; @@ -81,3 +86,12 @@ class OilIntegration : public IntegrationBase { std::string configPrefix, std::string configSuffix); }; + +class OilgenIntegration : public IntegrationBase { + protected: + std::string TmpDirStr() override; + + Proc runOilgenTarget(OilgenOpts opts, + std::string configPrefix, + std::string configSuffix); +};