1901450 - shared-libraries-linux.cc is reading ELF build ID twice - instead it should reuse the same ID to compute the debug ID and the code ID

Status: RESOLVED FIXED

(Core :: Gecko Profiler, defect, P3)

Description

[Markus Stange [:mstange]]

Profile: https://share.firefox.dev/3Xh5n26

Both getBreakpadId and getCodeId load the file in order to read the ELF build ID from it. We should just read the file once and compute both IDs from it instead.

https://searchfox.org/mozilla-central/rev/51f395e7d26987bb2bf5201a96f53a3559c43943/mozglue/baseprofiler/core/shared-libraries-linux.cc#667-706

// Get the breakpad Id for the binary file pointed by bin_name
static std::string getBreakpadId(const char* bin_name) {
  std::vector<uint8_t> identifier;
  identifier.reserve(kDefaultBuildIdSize);

  FileID file_id(bin_name);
  if (file_id.ElfFileIdentifier(identifier)) {
    return IDtoUUIDString(identifier);
  }

  return {};
}

// Get the code Id for the binary file pointed by bin_name
static std::string getCodeId(const char* bin_name) {
  std::vector<uint8_t> identifier;
  identifier.reserve(kDefaultBuildIdSize);

  FileID file_id(bin_name);
  if (file_id.ElfFileIdentifier(identifier)) {
    return IDtoString(identifier);
  }

  return {};
}

static SharedLibrary SharedLibraryAtPath(const char* path,
                                         unsigned long libStart,
                                         unsigned long libEnd,
                                         unsigned long offset = 0) {
  std::string pathStr = path;

  size_t pos = pathStr.rfind('\\');
  std::string nameStr =
      (pos != std::string::npos) ? pathStr.substr(pos + 1) : pathStr;

  return SharedLibrary(libStart, libEnd, offset, getBreakpadId(path),
                       getCodeId(path), nameStr, pathStr, nameStr, pathStr,
                       std::string{}, "");
}

(Note that we have two copies of this code. Both need to be fixed.)

Comment 1

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Attachment: Bug 1901450 - Read the ELF file identifier only once for breakpadId and codeId r?mstange,aabh

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/54158e89d7f1
Read the ELF file identifier only once for breakpadId and codeId r=aabh,profiler-reviewers

Comment 3

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/54158e89d7f1