Closed Bug 854810 Opened 8 years ago Closed 8 years ago

Use shared-libraries-*.cc instead of home-grown equivalents

Categories

(Core :: Gecko Profiler, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla22

People

(Reporter: jseward, Assigned: jseward)

References

Details

Attachments

(1 file)

Bug 779291 inadvertantly duplicated some functionality from
tools/profiler/shared-libraries.cc.  This patch removes that
duplication.
Attached patch PatchSplinter Review
Assignee: nobody → jseward
Attachment #729438 - Flags: review?(bgirard)
Depends on: 851748
Comment on attachment 729438 [details] [diff] [review]
Patch

Review of attachment 729438 [details] [diff] [review]:
-----------------------------------------------------------------

::: tools/profiler/UnwinderThread2.cpp
@@ +1402,5 @@
> +  SharedLibraryInfo info = SharedLibraryInfo::GetInfoForSelf();
> +  for (size_t i = 0; i < info.GetSize(); i++) {
> +    const SharedLibrary& lib = info.GetEntry(i);
> +    // On Linux, this pulls out two mappings with no names: the VDSO
> +    // (understandable but harmless), and the main executable (bad).

Why is the main executable bad? If we can reason about these segments being misleading we can modify SharedLibraryInfo to not include them. This code is only used for symbolication and is shared between the profiler and Telemetry.
Attachment #729438 - Flags: review?(bgirard) → review+
(In reply to Benoit Girard (:BenWa) from comment #2)
> Why is the main executable bad?

I meant only that it's bad that shared-libraries.cc produces a mapping
with no name for the main executable, since that means we breakpad
won't be able to read CFI for it, so we won't be able to unwind into
it.  I don't think it matters much -- it's just an annoying
inconsistency.
I'll look into modifying the symbolication script to avoid empty name libraries.
https://hg.mozilla.org/mozilla-central/rev/b5c4adef1523
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in before you can comment on or make changes to this bug.