718025 - Add support for stacktraces on Windows to the built-in profiler

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[(no longer active)]

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

[(no longer active)]

Attachment: Patch (v1)

Comment 2

[Jeff Muizelaar [:jrmuizel]]

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

[(no longer active)]

Attachment: Patch (v2)

Good catch.  I wonder why I didn't get hangs while testing. :/

Comment 4

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2273e0264d4a

Comment 5

[Phil Ringnalda (:philor)]

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

[(no longer active)]

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

[(no longer active)]

Attachment: Patch (v2) which actually compiles

Comment 8

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/419f8a0f6374

Comment 9

[Joe Drew (not getting mail)]

https://hg.mozilla.org/mozilla-central/rev/419f8a0f6374