The default bug view has changed. See this FAQ.

--enable-jprof broken by build system changes in bug 552864

RESOLVED FIXED

Status

()

Core
Build Config
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: jesup, Assigned: jesup)

Tracking

Trunk
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

6 years ago
Bug 552864 broke linking in libjprof.so and also broke atexit() used in jprof.

Patch in a sec (taken from bigger patch in bug 664453)
(Assignee)

Updated

6 years ago
Blocks: 664453
(Assignee)

Comment 1

6 years ago
Created attachment 541288 [details] [diff] [review]
Patch

Broken out (and added to) from bug 664453.  Running a build now to test this patch
Assignee: nobody → rjesup
Status: NEW → ASSIGNED
Attachment #541288 - Flags: review?(mh+mozilla)
Comment on attachment 541288 [details] [diff] [review]
Patch

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

::: tools/jprof/stub/libmalloc.cpp
@@ +541,5 @@
> +    // This instanciates the dummy class above, and will trigger the class
> +    // destructor when libxul is unloaded. This is equivalent to atexit(),
> +    // but gracefully handles dlclose().
> +    static JprofShutdown t;
> +}

You are missing the part where you replace the atexit() call. Note that you don't need the RegisterJprofShutdown function to be extern "C" (that was only required for trace-malloc because the function was defined in C++ and called from C), and you actually don't need it at all, you can just define static JprofShutdown t where atexit() is called.
Attachment #541288 - Flags: review?(mh+mozilla) → review-
(Assignee)

Comment 3

6 years ago
Created attachment 541296 [details] [diff] [review]
Patch - built and tested
Attachment #541288 - Attachment is obsolete: true
Attachment #541296 - Flags: review?(mh+mozilla)
Comment on attachment 541296 [details] [diff] [review]
Patch - built and tested

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

::: tools/jprof/stub/libmalloc.cpp
@@ +102,5 @@
> +        DumpAddressMap();
> +    }
> +};
> +
> +void RegisterJprofShutdown() {

You can make that function static, or as I wrote in my previous comment, just define static JprofShutdown t instead of the function call.

r=me with either.

@@ +543,5 @@
>      } else {
>          printf("setupProfilingStuff() called multiple times\n");
>      }
>  }
> +

You may want to avoid that empty line being added.
Attachment #541296 - Flags: review?(mh+mozilla) → review+
(Assignee)

Comment 5

6 years ago
Comment on attachment 541296 [details] [diff] [review]
Patch - built and tested

Checked in as http://hg.mozilla.org/mozilla-central/rev/0d5c92e9760e with nits addressed.
Attachment #541296 - Flags: checkin+
(Assignee)

Updated

6 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.