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](#1976 (comment)) 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 <[email protected]>
pixie-io · Aug 26, 2024 · 887401c · 887401c
1 parent 33f0daf
commit 887401c
Show file tree

Hide file tree

Showing 6 changed files with 47 additions and 7 deletions.
diff --git 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
@@ -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
@@ -89,10 +89,11 @@ StatusOr<Section> ObjdumpSectionNameToAddr(const std::string& path,
   return section;
 }
 
-StatusOr<int64_t> NmSymbolNameToAddr(const std::string& path, const std::string& symbol_name) {
+StatusOr<int64_t> 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<absl::string_view> 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<ElfReader> 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<ElfReader> 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<ElfReader> 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
@@ -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
@@ -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
@@ -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,