Skip to content

Commit

Permalink
Jitdump fix for wholehost (#297)
Browse files Browse the repository at this point in the history
* Jitdump fix for wholehost

In wholehost we were not considering the jitdump files. The format was required to include the PID in the name of the file.
However the namespace's PID is not known in wholehost. This relaxes the test to just look for jit-[0-9]+.dump in the name of the file.
  • Loading branch information
r1viollet authored Aug 29, 2023
1 parent cef8b46 commit 78b6920
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 14 deletions.
30 changes: 18 additions & 12 deletions src/dso.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "logger.hpp"
#include "string_format.hpp"

#include <algorithm>
#include <string_view>

namespace ddprof {
Expand Down Expand Up @@ -80,20 +81,25 @@ Dso::Dso(pid_t pid, ElfAddress_t start, ElfAddress_t end, ElfAddress_t pgoff,
}
}

// The string should end with: "jit-[0-9]+\\.dump"
// and the number should be the pid, however, in wholehost mode
// we don't have visibility on the namespace's PID value.
bool Dso::is_jit_dump_str(std::string_view file_path, pid_t pid) {
// test if we finish by .dump before creating a string
if (file_path.ends_with(".dump")) {
// llvm uses this format
std::string jit_dump_str = string_format("jit-%d.dump", pid);
// this is to remove a gcc warning
if (jit_dump_str.size() >= PTRDIFF_MAX) {
return false;
}
if (file_path.ends_with(jit_dump_str)) {
return true;
}
const std::string_view prefix = "jit-";
const std::string_view ext = ".dump";
if (!file_path.ends_with(ext))
return false;
file_path = file_path.substr(0, file_path.size() - ext.size());
auto pos = file_path.rfind('/');
if (pos != std::string_view::npos) {
file_path = file_path.substr(pos + 1);
}
return false;
if (!file_path.starts_with(prefix))
return false;
file_path = file_path.substr(prefix.size());
return std::all_of(file_path.begin(), file_path.end(), [](char c) {
return std::isdigit(static_cast<unsigned char>(c));
});
}

std::string Dso::to_string() const {
Expand Down
3 changes: 2 additions & 1 deletion src/jit/jitdump.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,14 @@ DDRes jit_read_header(std::ifstream &file_stream, JITHeader &header) {
if (!file_stream.read(reinterpret_cast<char *>(&header), sizeof(JITHeader))) {
DDRES_RETURN_WARN_LOG(DD_WHAT_JIT, "incomplete jit file");
}

if (header.magic == k_header_magic) {
// expected value (no need to swap data)
} else if (header.magic == k_header_magic_rev) {
// todo everything should be swapped throughout the parsing (not handled)
DDRES_RETURN_WARN_LOG(DD_WHAT_JIT, "Swap data not handled");
} else {
DDRES_RETURN_WARN_LOG(DD_WHAT_JIT, "Unknown jit format");
DDRES_RETURN_WARN_LOG(DD_WHAT_JIT, "Unknown jit format(%x)", header.magic);
}
int64_t remaining_size = header.total_size - sizeof(header);
if (remaining_size > 0) {
Expand Down
6 changes: 5 additions & 1 deletion test/dso-ut.cc
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,11 @@ TEST(DSOTest, dso_from_procline) {
DsoHdr::dso_from_procline(3237589, const_cast<char *>(s_jitdump_line));
EXPECT_EQ(jitdump_dso._type, dso::kJITDump);
}
{ // jitdump with a name different from PID (for wholehost)
Dso jitdump_dso =
DsoHdr::dso_from_procline(12, const_cast<char *>(s_jitdump_line));
EXPECT_EQ(jitdump_dso._type, dso::kJITDump);
}
}

// Retrieves instruction pointer
Expand Down Expand Up @@ -394,7 +399,6 @@ TEST(DSOTest, insert_jitdump) {
}

TEST(DSOTest, exe_name) {
LogHandle handle;
ElfAddress_t ip = _THIS_IP_;
DsoHdr dso_hdr;
DsoFindRes find_res = dso_hdr.dso_find_or_backpopulate(getpid(), ip);
Expand Down

0 comments on commit 78b6920

Please sign in to comment.