Skip to content

Commit

Permalink
Update ElfReaderTest's nm usage to be hermetic (pixie-io#1980)
Browse files Browse the repository at this point in the history
Summary: Update `ElfReaderTest`'s nm usage to be hermetic

This was feedback from pixie-io#1976. See [this
comment](pixie-io#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]>
GitOrigin-RevId: 887401c
  • Loading branch information
ddelnano authored and cosmic-copybara committed Aug 27, 2024
1 parent 9a3dc86 commit 74ef591
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 7 deletions.
8 changes: 8 additions & 0 deletions bazel/cc_toolchains/clang/toolchain_files.BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
Expand Down Expand Up @@ -110,6 +116,7 @@ filegroup(
"as",
"cov",
"objcopy",
"nm",
"strip",
"dwp",
]
Expand All @@ -124,6 +131,7 @@ filegroup(
":toolchain_dwp_files",
":toolchain_linker_files",
":toolchain_objcopy_files",
":toolchain_nm_files",
":toolchain_strip_files",
],
visibility = ["//visibility:public"],
Expand Down
1 change: 1 addition & 0 deletions src/stirling/obj_tools/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
19 changes: 13 additions & 6 deletions src/stirling/obj_tools/elf_reader_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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.

Expand Down Expand Up @@ -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));

Expand All @@ -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));

Expand Down Expand Up @@ -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";
Expand Down
13 changes: 12 additions & 1 deletion src/stirling/obj_tools/testdata/cc/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
)

Expand Down
5 changes: 5 additions & 0 deletions src/stirling/obj_tools/testdata/cc/test_exe_fixture.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_);
Expand All @@ -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
Expand Down
8 changes: 8 additions & 0 deletions src/stirling/obj_tools/testdata/go/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 74ef591

Please sign in to comment.