Last Comment Bug 718025 - Add support for stacktraces on Windows to the built-in profiler
: Add support for stacktraces on Windows to the built-in profiler
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: General (show other bugs)
: Trunk
: x86_64 Windows 7
: -- normal (vote)
: mozilla12
Assigned To: :Ehsan Akhgari
:
:
Mentors:
Depends on:
Blocks: 713227
  Show dependency treegraph
 
Reported: 2012-01-13 12:24 PST by :Ehsan Akhgari
Modified: 2012-01-17 17:52 PST (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (v1) (4.61 KB, patch)
2012-01-13 12:25 PST, :Ehsan Akhgari
jmuizelaar: review-
Details | Diff | Splinter Review
Patch (v2) (4.83 KB, patch)
2012-01-16 13:16 PST, :Ehsan Akhgari
jmuizelaar: review+
Details | Diff | Splinter Review
Patch (v2) which actually compiles (4.83 KB, patch)
2012-01-17 08:47 PST, :Ehsan Akhgari
no flags Details | Diff | Splinter Review

Description :Ehsan Akhgari 2012-01-13 12:24:24 PST
I have a patch which enables us to walk the stack on Windows.  The symbolication part is missing, so it's not quite useful yet, but stepping through the code, things seem to be working fine.
Comment 1 :Ehsan Akhgari 2012-01-13 12:25:13 PST
Created attachment 588491 [details] [diff] [review]
Patch (v1)
Comment 2 Jeff Muizelaar [:jrmuizel] 2012-01-16 12:45:19 PST
Comment on attachment 588491 [details] [diff] [review]
Patch (v1)

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

::: tools/profiler/sps/TableTicker.cpp
@@ +361,5 @@
>  }
>  #endif
>  
> +#ifdef USE_NS_STACKWALK
> +typedef nsTArray<void*> PCArray;

I don't think you can safely use an nsTArray when a thread is paused.
nsTArray will do mallocs which will deadlock if the stopped thread is inside malloc.

::: tools/profiler/sps/platform.h
@@ +204,5 @@
>    PlatformData* platform_data() { return data_; }
>  
> +#ifdef XP_WIN
> +  // xxxehsan sucky hack :(
> +  static uintptr_t GetThreadHandle(PlatformData*);

gross
Comment 3 :Ehsan Akhgari 2012-01-16 13:16:55 PST
Created attachment 588984 [details] [diff] [review]
Patch (v2)

Good catch.  I wonder why I didn't get hangs while testing. :/
Comment 5 Phil Ringnalda (:philor) 2012-01-16 18:35:38 PST
Backed you out in https://hg.mozilla.org/integration/mozilla-inbound/rev/e40e72d6264d since I was pretty sure you'd rather have Windows continue to build ;)
Comment 6 :Ehsan Akhgari 2012-01-16 18:40:50 PST
Ouch...  Some day I will learn how to program.  Until that day comes, here's a link to the log for reference: https://tbpl.mozilla.org/php/getParsedLog.php?id=8595118&tree=Mozilla-Inbound
Comment 7 :Ehsan Akhgari 2012-01-17 08:47:16 PST
Created attachment 589214 [details] [diff] [review]
Patch (v2) which actually compiles
Comment 9 Joe Drew (not getting mail) 2012-01-17 17:52:03 PST
https://hg.mozilla.org/mozilla-central/rev/419f8a0f6374

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