Last Comment Bug 666501 - --enable-jprof broken by build system changes in bug 552864
: --enable-jprof broken by build system changes in bug 552864
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: ---
Assigned To: [:jesup] on pto until 2016/8/1 Randell Jesup
:
Mentors:
Depends on:
Blocks: 664453
  Show dependency treegraph
 
Reported: 2011-06-22 22:27 PDT by [:jesup] on pto until 2016/8/1 Randell Jesup
Modified: 2011-06-26 08:37 PDT (History)
0 users
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (2.24 KB, patch)
2011-06-22 22:32 PDT, [:jesup] on pto until 2016/8/1 Randell Jesup
mh+mozilla: review-
Details | Diff | Splinter Review
Patch - built and tested (3.46 KB, patch)
2011-06-22 23:33 PDT, [:jesup] on pto until 2016/8/1 Randell Jesup
mh+mozilla: review+
rjesup: checkin+
Details | Diff | Splinter Review

Description [:jesup] on pto until 2016/8/1 Randell Jesup 2011-06-22 22:27:46 PDT
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)
Comment 1 [:jesup] on pto until 2016/8/1 Randell Jesup 2011-06-22 22:32:44 PDT
Created attachment 541288 [details] [diff] [review]
Patch

Broken out (and added to) from bug 664453.  Running a build now to test this patch
Comment 2 Mike Hommey [:glandium] 2011-06-22 22:41:53 PDT
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.
Comment 3 [:jesup] on pto until 2016/8/1 Randell Jesup 2011-06-22 23:33:33 PDT
Created attachment 541296 [details] [diff] [review]
Patch - built and tested
Comment 4 Mike Hommey [:glandium] 2011-06-22 23:38:12 PDT
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.
Comment 5 [:jesup] on pto until 2016/8/1 Randell Jesup 2011-06-24 07:26:03 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.