From 74ef5916c15fd0a923b51d79081c67eeacce8cbe Mon Sep 17 00:00:00 2001 From: Dom Delnano Date: Mon, 26 Aug 2024 14:49:38 -0700 Subject: [PATCH] Update `ElfReaderTest`'s nm usage to be hermetic (#1980) Summary: Update `ElfReaderTest`'s nm usage to be hermetic This was feedback from #1976. See [this comment](https://github.com/pixie-io/pixie/pull/1976#discussion_r1707595841) for the previous discussion. Relevant Issues: N/A Type of change: /kind cleanup Test Plan: Elf reader tests pass with updates --------- Signed-off-by: Dom Del Nano GitOrigin-RevId: 887401c9069b8a2136bda8e5aada082c18daf8eb --- .../cc_toolchains/clang/toolchain_files.BUILD | 8 ++++++++ src/stirling/obj_tools/BUILD.bazel | 1 + src/stirling/obj_tools/elf_reader_test.cc | 19 +++++++++++++------ .../obj_tools/testdata/cc/BUILD.bazel | 13 ++++++++++++- .../obj_tools/testdata/cc/test_exe_fixture.h | 5 +++++ .../obj_tools/testdata/go/BUILD.bazel | 8 ++++++++ 6 files changed, 47 insertions(+), 7 deletions(-) diff --git a/bazel/cc_toolchains/clang/toolchain_files.BUILD b/bazel/cc_toolchains/clang/toolchain_files.BUILD index c9ca5f70a88..f801d0939d9 100644 --- a/bazel/cc_toolchains/clang/toolchain_files.BUILD +++ b/bazel/cc_toolchains/clang/toolchain_files.BUILD @@ -59,6 +59,12 @@ filegroup( visibility = ["//visibility:public"], ) +filegroup( + name = "toolchain_nm_files", + srcs = [":nm"], + visibility = ["//visibility:public"], +) + filegroup( name = "toolchain_strip_files", srcs = [":strip"], @@ -110,6 +116,7 @@ filegroup( "as", "cov", "objcopy", + "nm", "strip", "dwp", ] @@ -124,6 +131,7 @@ filegroup( ":toolchain_dwp_files", ":toolchain_linker_files", ":toolchain_objcopy_files", + ":toolchain_nm_files", ":toolchain_strip_files", ], visibility = ["//visibility:public"], diff --git a/src/stirling/obj_tools/BUILD.bazel b/src/stirling/obj_tools/BUILD.bazel index 397a4ac9d64..5ce263f4ad6 100644 --- a/src/stirling/obj_tools/BUILD.bazel +++ b/src/stirling/obj_tools/BUILD.bazel @@ -53,6 +53,7 @@ pl_cc_test( "//src/stirling/obj_tools/testdata/cc:test_exe_debug_target", "//src/stirling/obj_tools/testdata/cc:test_exe_debuglink_target", "//src/stirling/obj_tools/testdata/go:test_go_1_19_binary", + "//src/stirling/obj_tools/testdata/go:test_go_1_19_nm_output", ], deps = [ ":cc_library", diff --git a/src/stirling/obj_tools/elf_reader_test.cc b/src/stirling/obj_tools/elf_reader_test.cc index a2c9a56a98b..3703d1187ce 100644 --- a/src/stirling/obj_tools/elf_reader_test.cc +++ b/src/stirling/obj_tools/elf_reader_test.cc @@ -89,10 +89,11 @@ StatusOr
ObjdumpSectionNameToAddr(const std::string& path, return section; } -StatusOr NmSymbolNameToAddr(const std::string& path, const std::string& symbol_name) { +StatusOr NmSymbolNameToAddr(const std::string& nm_output_path, + const std::string& symbol_name) { // Extract the address from nm as the gold standard. int64_t symbol_addr = -1; - std::string nm_out = px::Exec(absl::StrCat("nm ", path)).ValueOrDie(); + PX_ASSIGN_OR_RETURN(auto nm_out, px::ReadFileToString(nm_output_path)); std::vector nm_out_lines = absl::StrSplit(nm_out, '\n'); for (auto& line : nm_out_lines) { if (line.find(symbol_name) != std::string::npos) { @@ -163,8 +164,9 @@ TEST(ElfReaderTest, ListFuncSymbolsSuffixMatch) { TEST(ElfReaderTest, SymbolAddress) { const std::string path = kTestExeFixture.Path().string(); + const std::string nm_output_path = kTestExeFixture.NmOutputPath().string(); const std::string kSymbolName = "CanYouFindThis"; - ASSERT_OK_AND_ASSIGN(const int64_t symbol_addr, NmSymbolNameToAddr(path, kSymbolName)); + ASSERT_OK_AND_ASSIGN(const int64_t symbol_addr, NmSymbolNameToAddr(nm_output_path, kSymbolName)); // Actual tests of SymbolAddress begins here. @@ -195,8 +197,9 @@ TEST(ElfReaderTest, VirtualAddrToBinaryAddr) { TEST(ElfReaderTest, AddrToSymbol) { const std::string path = kTestExeFixture.Path().string(); + const std::string nm_output_path = kTestExeFixture.NmOutputPath().string(); const std::string kSymbolName = "CanYouFindThis"; - ASSERT_OK_AND_ASSIGN(const int64_t symbol_addr, NmSymbolNameToAddr(path, kSymbolName)); + ASSERT_OK_AND_ASSIGN(const int64_t symbol_addr, NmSymbolNameToAddr(nm_output_path, kSymbolName)); ASSERT_OK_AND_ASSIGN(std::unique_ptr elf_reader, ElfReader::Create(path)); @@ -216,8 +219,9 @@ TEST(ElfReaderTest, AddrToSymbol) { TEST(ElfReaderTest, InstrAddrToSymbol) { const std::string path = kTestExeFixture.Path().string(); + const std::string nm_output_path = kTestExeFixture.NmOutputPath().string(); const std::string kSymbolName = "CanYouFindThis"; - ASSERT_OK_AND_ASSIGN(const int64_t kSymbolAddr, NmSymbolNameToAddr(path, kSymbolName)); + ASSERT_OK_AND_ASSIGN(const int64_t kSymbolAddr, NmSymbolNameToAddr(nm_output_path, kSymbolName)); ASSERT_OK_AND_ASSIGN(std::unique_ptr elf_reader, ElfReader::Create(path)); @@ -300,12 +304,15 @@ TEST(ElfReaderTest, FuncByteCode) { TEST(ElfReaderTest, GolangAppRuntimeBuildVersion) { const std::string kPath = px::testing::BazelRunfilePath("src/stirling/obj_tools/testdata/go/test_go_1_19_binary"); + const std::string kGoBinNmOutput = + px::testing::BazelRunfilePath("src/stirling/obj_tools/testdata/go/test_go_1_19_nm_output"); ASSERT_OK_AND_ASSIGN(std::unique_ptr elf_reader, ElfReader::Create(kPath)); ASSERT_OK_AND_ASSIGN(ElfReader::SymbolInfo symbol, elf_reader->SearchTheOnlySymbol("runtime.buildVersion")); // Coverage build might alter the resultant binary. #ifndef PL_COVERAGE - ASSERT_OK_AND_ASSIGN(auto expected_addr, NmSymbolNameToAddr(kPath, "runtime.buildVersion")); + ASSERT_OK_AND_ASSIGN(auto expected_addr, + NmSymbolNameToAddr(kGoBinNmOutput, "runtime.buildVersion")); EXPECT_EQ(symbol.address, expected_addr); #endif EXPECT_EQ(symbol.size, 16) << "Symbol table entry size should be 16"; diff --git a/src/stirling/obj_tools/testdata/cc/BUILD.bazel b/src/stirling/obj_tools/testdata/cc/BUILD.bazel index 5fd009592a7..9ec254cef81 100644 --- a/src/stirling/obj_tools/testdata/cc/BUILD.bazel +++ b/src/stirling/obj_tools/testdata/cc/BUILD.bazel @@ -41,10 +41,21 @@ cc_clang_binary( ], ) +genrule( + name = "test_exe_nm_output_target", + srcs = [":test_exe"], + outs = ["test_exe_nm_output"], + cmd = "$(NM) $(location :test_exe) > $(location test_exe_nm_output)", + toolchains = ["@bazel_tools//tools/cpp:current_cc_toolchain"], +) + cc_library( name = "test_exe_fixture", hdrs = ["test_exe_fixture.h"], - data = [":test_exe"], + data = [ + ":test_exe", + ":test_exe_nm_output", + ], deps = ["//src/common/exec:cc_library"], ) diff --git a/src/stirling/obj_tools/testdata/cc/test_exe_fixture.h b/src/stirling/obj_tools/testdata/cc/test_exe_fixture.h index 13115258417..d5098715862 100644 --- a/src/stirling/obj_tools/testdata/cc/test_exe_fixture.h +++ b/src/stirling/obj_tools/testdata/cc/test_exe_fixture.h @@ -31,8 +31,11 @@ namespace obj_tools { class TestExeFixture { public: static constexpr char kTestExePath[] = "src/stirling/obj_tools/testdata/cc/test_exe_/test_exe"; + static constexpr char kTestExeNmOutputPath[] = + "src/stirling/obj_tools/testdata/cc/test_exe_nm_output"; const std::filesystem::path& Path() const { return test_exe_path_; } + const std::filesystem::path& NmOutputPath() const { return test_exe_nm_output_path_; } Status Run() const { auto stdout_or = Exec(test_exe_path_); @@ -41,6 +44,8 @@ class TestExeFixture { private: const std::filesystem::path test_exe_path_ = testing::BazelRunfilePath(kTestExePath); + const std::filesystem::path test_exe_nm_output_path_ = + testing::BazelRunfilePath(kTestExeNmOutputPath); }; } // namespace obj_tools diff --git a/src/stirling/obj_tools/testdata/go/BUILD.bazel b/src/stirling/obj_tools/testdata/go/BUILD.bazel index 8ee1ac31481..25c828800f9 100644 --- a/src/stirling/obj_tools/testdata/go/BUILD.bazel +++ b/src/stirling/obj_tools/testdata/go/BUILD.bazel @@ -47,6 +47,14 @@ pl_go_binary( for sdk_version in pl_supported_go_sdk_versions ] +genrule( + name = "test_go_1_19_nm_output_target", + srcs = [":test_go_1_19_binary"], + outs = ["test_go_1_19_nm_output"], + cmd = "$(NM) $(location :test_go_1_19_binary) > $(location test_go_1_19_nm_output)", + toolchains = ["@bazel_tools//tools/cpp:current_cc_toolchain"], +) + filegroup( name = "test_binaries", testonly = True,