Closed Bug 1901450 Opened 7 months ago Closed 1 month ago

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

Categories

(Core :: Gecko Profiler, defect, P3)

All
Linux
defect

Tracking

()

RESOLVED FIXED
135 Branch
Tracking Status
firefox135 --- fixed

People

(Reporter: mstange, Assigned: canova, Mentored)

References

Details

(Whiteboard: [fxp])

Attachments

(1 file)

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.)

Whiteboard: [fxp]
Severity: -- → S3
Priority: -- → P3
Assignee: nobody → canaltinova
Status: NEW → ASSIGNED
Pushed by canaltinova@gmail.com: https://hg.mozilla.org/integration/autoland/rev/54158e89d7f1 Read the ELF file identifier only once for breakpadId and codeId r=aabh,profiler-reviewers
Status: ASSIGNED → RESOLVED
Closed: 1 month ago
Resolution: --- → FIXED
Target Milestone: --- → 135 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: