Closed Bug 666501 Opened 9 years ago Closed 9 years ago

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

Categories

(Firefox Build System :: General, defect)

x86
Linux
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jesup, Assigned: jesup)

References

Details

Attachments

(1 file, 1 obsolete file)

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)
Blocks: 664453
Attached patch Patch (obsolete) — Splinter Review
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-
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+
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+
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.