988476 - [jsdbg2] Create |Debugger.Memory|, a place to expose our memory tools to JS

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

This bug is only for creating an empty shell of an object, so that we have an existing place to attach stuff later on, like a |DominatorTree| object like in bug 961329. This isn't to create the whole API, which would be bug 961331.

I propose:

* attaching all of our memory related classes to |Debugger.Memory|, eg |Debugger.Memory.DominatorTree|

* Creating a |Debugger| instance should create a |Debugger.Memory| instance on the |memory| property of said |Debugger| instance.

    let dbg = new Debugger();
    dbg.memory instanceof Debugger.Memory // true

* None of the memory debugging and profiling stuff is activated until |dbg.memory.enabled| is set to |true|. Just because we are adding this |memory| property shouldn't add overhead.

Was going to just add this stuff in bug 961329, but it felt better to split it out to a new patch.

Comment 1

[Jim Blandy :jimb]

You could just make Debugger.prototype.memory an accessor, allocate the Debugger.Memory instance lazily, and use that to enable things.

Err, what does 'enabled' enable, exactly?

Comment 2

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

(In reply to Jim Blandy :jimb from comment #1)
> You could just make Debugger.prototype.memory an accessor, allocate the
> Debugger.Memory instance lazily, and use that to enable things.
> 
> Err, what does 'enabled' enable, exactly?

I was imagining it enabling the tracking of allocation sites and all that. Wasn't going to actually implement it in this bug, however.

Comment 3

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: debugger-memory.patch

https://tbpl.mozilla.org/?tree=Try&rev=7eae7cf32106

Comment 4

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: debugger-memory.patch

Interesting, I only get the compile errors from the try push when clobber building but subsequent incremental builds are just fine...

Couldn't get |JS_DefineDebuggerObject| to be a proper friend function so I just gave up and made the relevant bits public.

Also added the proper license header bits to the new files.

New try push: https://tbpl.mozilla.org/?tree=Try&rev=fd3574db64f0

Comment 5

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: debugger-memory.patch

https://tbpl.mozilla.org/?tree=Try&rev=6063b45fae43

No idea why builds work fine for me locally, but not when I've been pushing to try...

This adds a bunch of headers for stuff that seemed to be complained about in the last try push.

Comment 6

[Jim Blandy :jimb]

Comment on attachment 8398083 [details] [diff] [review]
debugger-memory.patch

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

::: js/src/vm/Debugger.cpp
@@ -5890,5 @@
>      JS_FS_END
>  };
>  
>  
> -

Is this deleting the form-feed character? Could we leave those in? They work nicely with Emacs's pages-directory.

::: js/src/vm/DebuggerMemory.cpp
@@ +11,5 @@
> +/* static */ bool
> +DebuggerMemory::construct(JSContext *cx, unsigned argc, Value *vp)
> +{
> +    JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_NO_CONSTRUCTOR,
> +                         "Memory");

I think this wants to say "Debugger.Memory".

Comment 7

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: debugger-memory.patch

Addressed jimb's comments; carrying over r+.

New try push: https://tbpl.mozilla.org/?tree=Try&rev=23183ea9d2e8

Nick, should I be worried about this ASAN Bo failure from the last try push[0]? Any help getting a useful stack trace from that? Thanks!

[0] https://tbpl.mozilla.org/?tree=Try&rev=6063b45fae43

Comment 8

[Nicholas Nethercote [inactive]]

> Nick, should I be worried about this ASAN Bo failure from the last try
> push[0]? Any help getting a useful stack trace from that? Thanks!
> 
> [0] https://tbpl.mozilla.org/?tree=Try&rev=6063b45fae43

You should probably be worried -- it's a heap-use-after-free error. It's odd (and annoying) that the stack trace doesn't have filenames and line numbers. I don't know why that is.

decoder knows a lot about ASAN, he might have an idea.

Comment 9

[Andrew McCreight [:mccr8]]

My guess would be that the environment variables that point ASAN at its symbolizer isn't set up for the build-check run, but I could be wrong.

Comment 10

[Christian Holler (:decoder)]

I can't seem to find the log given the link in comment 7 but if the ASan trace is unsymbolized, then :mccr8's guess is probably right. In order to automatically symbolize stuff, ASan needs to have an environment variable pointing to the llvm-symbolizer binary of the toolchain. It might be that this is not the case for when the failure occurs. Is this during jit-tests or at which stage? Because jit-tests get the environment variable. If it's still not working, then maybe the jit-tests swallow that env variable or something else is broken. Can you please link the log in question?

Comment 11

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

The only test I added is a jit-test.

Here is the full log: https://tbpl.mozilla.org/php/getParsedLog.php?id=36831239&tree=Try&full=1

Comment 12

[Christian Holler (:decoder)]

It looks like one of the existing tests during make check is failing. I symbolized the trace here:

==18368==ERROR: AddressSanitizer: heap-use-after-free on address 0x603000002e68 at pc 0x7f0a508634d3 bp 0x7f0a42ffe410 sp 0x7f0a42ffe408
READ of size 1 at 0x603000002e68 thread T1 (Gecko_IOThread)
    #0 0x7f0a508634d2 in RegisterSignalHandler build/xpcom/base/nsDumpUtils.cpp:151
    #1 0x7f0a50861d90 in StartWatching build/xpcom/base/nsDumpUtils.cpp:82
    #2 0x7f0a511408e1 in RunTask build/ipc/chromium/src/base/message_loop.cc:344
    #3 0x7f0a510f15ae in Run build/ipc/chromium/src/base/message_pump_libevent.cc:311
    #4 0x7f0a5113e463 in RunInternal build/ipc/chromium/src/base/message_loop.cc:226
    #5 0x7f0a51165962 in ThreadMain build/ipc/chromium/src/base/thread.cc:162
    #6 0x7f0a510f2c1c in ThreadFunc build/ipc/chromium/src/base/platform_thread_posix.cc:39
    #7 0x44bb33 in ThreadStart _asan_rtl_
0x603000002e68 is located 8 bytes inside of 24-byte region [0x603000002e60,0x603000002e78)
freed by thread T0 here:
    #0 0x44516b in __interceptor_realloc _asan_rtl_
    #1 0x7f0a4f4b6dcd in moz_xrealloc build/memory/mozalloc/mozalloc.cpp:84
    #2 0x7f0a507ff4a7 in Realloc build/obj-firefox/netwerk/wifi/../../dist/include/nsTArray.h:208
    #3 0x7f0a508625e2 in AppendElements<SignalInfo> build/obj-firefox/xpcom/base/../../dist/include/nsTArray.h:1236
    #4 0x7f0a50867e7b in Initialize build/xpcom/base/nsMemoryInfoDumper.cpp:194
    #5 0x7f0a5086e590 in Init build/xpcom/base/nsMemoryReporterManager.cpp:890
    #6 0x7f0a508220a3 in nsMemoryReporterManagerConstructor build/xpcom/build/nsXPComInit.cpp:217
    #7 0x7f0a5092be0a in CreateInstanceByContractID build/xpcom/components/nsComponentManager.cpp:1079
    #8 0x7f0a50923764 in GetServiceByContractID build/xpcom/components/nsComponentManager.cpp:1435
    #9 0x7f0a507f56dd in CallGetService build/xpcom/glue/nsComponentManagerUtils.cpp:62
    #10 0x7f0a5085cb49 in nsCOMPtr build/obj-firefox/xpcom/base/../../dist/include/nsCOMPtr.h:658
    #11 0x7f0a50b7d557 in Init build/netwerk/dns/nsDNSService2.cpp:555
    #12 0x7f0a50aa0c34 in SetOffline build/netwerk/base/src/nsIOService.cpp:720
    #13 0x7f0a50a9f942 in InitializeNetworkLinkService build/netwerk/base/src/nsIOService.cpp:273
    #14 0x7f0a50a9ee23 in Init build/netwerk/base/src/nsIOService.cpp:208
    #15 0x7f0a50aa17f6 in GetInstance build/netwerk/base/src/nsIOService.cpp:286
    #16 0x7f0a50a0bec5 in nsIOServiceConstructor build/netwerk/build/nsNetModule.cpp:58
    #17 0x7f0a5092be0a in CreateInstanceByContractID build/xpcom/components/nsComponentManager.cpp:1079
    #18 0x7f0a50923764 in GetServiceByContractID build/xpcom/components/nsComponentManager.cpp:1435
    #19 0x7f0a507f56dd in CallGetService build/xpcom/glue/nsComponentManagerUtils.cpp:62
    #20 0x7f0a5081a153 in nsCOMPtr build/xpcom/build/../glue/nsCOMPtr.h:658
    #21 0x7f0a5096eb90 in do_GetIOService build/obj-firefox/chrome/src/../../dist/include/nsNetUtil.h:101
    #22 0x7f0a5096edd0 in ResolveURI build/chrome/src/nsChromeRegistryChrome.cpp:787
    #23 0x7f0a5096f90b in ManifestLocale build/chrome/src/nsChromeRegistryChrome.cpp:859
    #24 0x7f0a50919889 in ParseManifest build/xpcom/components/ManifestParser.cpp:636
    #25 0x7f0a509273d2 in RegisterManifest build/xpcom/components/nsComponentManager.cpp:540
    #26 0x7f0a509276b7 in ManifestManifest build/xpcom/components/nsComponentManager.cpp:553
    #27 0x7f0a50919517 in ParseManifest build/xpcom/components/ManifestParser.cpp:647
    #28 0x7f0a509273d2 in RegisterManifest build/xpcom/components/nsComponentManager.cpp:540
    #29 0x7f0a509251be in RereadChromeManifests build/xpcom/components/nsComponentManager.cpp:716
previously allocated by thread T0 here:
    #0 0x444fc5 in __interceptor_malloc _asan_rtl_
    #1 0x7f0a4f4b6b3d in moz_xmalloc build/memory/mozalloc/mozalloc.cpp:52
    #2 0x7f0a507ff3e7 in Malloc build/obj-firefox/netwerk/wifi/../../dist/include/nsTArray.h:204
    #3 0x7f0a508625e2 in AppendElements<SignalInfo> build/obj-firefox/xpcom/base/../../dist/include/nsTArray.h:1236
    #4 0x7f0a50867e60 in Initialize build/xpcom/base/nsMemoryInfoDumper.cpp:191
    #5 0x7f0a5086e590 in Init build/xpcom/base/nsMemoryReporterManager.cpp:890
    #6 0x7f0a508220a3 in nsMemoryReporterManagerConstructor build/xpcom/build/nsXPComInit.cpp:217
    #7 0x7f0a5092be0a in CreateInstanceByContractID build/xpcom/components/nsComponentManager.cpp:1079
    #8 0x7f0a50923764 in GetServiceByContractID build/xpcom/components/nsComponentManager.cpp:1435
    #9 0x7f0a507f56dd in CallGetService build/xpcom/glue/nsComponentManagerUtils.cpp:62
    #10 0x7f0a5085cb49 in nsCOMPtr build/obj-firefox/xpcom/base/../../dist/include/nsCOMPtr.h:658
    #11 0x7f0a50b7d557 in Init build/netwerk/dns/nsDNSService2.cpp:555
    #12 0x7f0a50aa0c34 in SetOffline build/netwerk/base/src/nsIOService.cpp:720
    #13 0x7f0a50a9f942 in InitializeNetworkLinkService build/netwerk/base/src/nsIOService.cpp:273
    #14 0x7f0a50a9ee23 in Init build/netwerk/base/src/nsIOService.cpp:208
    #15 0x7f0a50aa17f6 in GetInstance build/netwerk/base/src/nsIOService.cpp:286
    #16 0x7f0a50a0bec5 in nsIOServiceConstructor build/netwerk/build/nsNetModule.cpp:58
    #17 0x7f0a5092be0a in CreateInstanceByContractID build/xpcom/components/nsComponentManager.cpp:1079
    #18 0x7f0a50923764 in GetServiceByContractID build/xpcom/components/nsComponentManager.cpp:1435
    #19 0x7f0a507f56dd in CallGetService build/xpcom/glue/nsComponentManagerUtils.cpp:62
    #20 0x7f0a5081a153 in nsCOMPtr build/xpcom/build/../glue/nsCOMPtr.h:658
    #21 0x7f0a5096eb90 in do_GetIOService build/obj-firefox/chrome/src/../../dist/include/nsNetUtil.h:101
    #22 0x7f0a5096edd0 in ResolveURI build/chrome/src/nsChromeRegistryChrome.cpp:787
    #23 0x7f0a5096f90b in ManifestLocale build/chrome/src/nsChromeRegistryChrome.cpp:859
    #24 0x7f0a50919889 in ParseManifest build/xpcom/components/ManifestParser.cpp:636
    #25 0x7f0a509273d2 in RegisterManifest build/xpcom/components/nsComponentManager.cpp:540
    #26 0x7f0a509276b7 in ManifestManifest build/xpcom/components/nsComponentManager.cpp:553
    #27 0x7f0a50919517 in ParseManifest build/xpcom/components/ManifestParser.cpp:647
    #28 0x7f0a509273d2 in RegisterManifest build/xpcom/components/nsComponentManager.cpp:540
    #29 0x7f0a509251be in RereadChromeManifests build/xpcom/components/nsComponentManager.cpp:716
Thread T1 (Gecko_IOThread) created by T0 here:
    #0 0x436431 in __interceptor_pthread_create _asan_rtl_
    #1 0x7f0a510f2a68 in CreateThread build/ipc/chromium/src/base/platform_thread_posix.cc:144
    #2 0x7f0a51165482 in StartWithOptions build/ipc/chromium/src/base/thread.cc:85
    #3 0x7f0a5081c7a9 in NS_InitXPCOM2 build/xpcom/build/nsXPComInit.cpp:594
    #4 0x7f0a537a2c42 in XRE_XPCShellMain build/js/xpconnect/src/XPCShellImpl.cpp:1453
Shadow bytes around the buggy address:
  0x0c067fff8570: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c067fff8580: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c067fff8590: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c067fff85a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c067fff85b0: fa fa fa fa 00 00 05 fa fa fa 00 00 02 fa fa fa
=>0x0c067fff85c0: 00 00 00 fa fa fa 00 00 00 00 fa fa fd[fd]fd fa
  0x0c067fff85d0: fa fa 00 00 00 fa fa fa 00 00 00 fa fa fa 00 00
  0x0c067fff85e0: 00 fa fa fa 00 00 00 fa fa fa 00 00 00 fa fa fa
  0x0c067fff85f0: 00 00 00 fa fa fa 00 00 00 fa fa fa 00 00 00 00
  0x0c067fff8600: fa fa 00 00 00 fa fa fa 00 00 00 00 fa fa 00 00
  0x0c067fff8610: 00 02 fa fa 00 00 00 00 fa fa 00 00 00 00 fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:     fa
  Heap right redzone:    fb
  Freed heap region:     fd
  Stack left redzone:    f1
  Stack mid redzone:     f2
  Stack right redzone:   f3
  Stack partial redzone: f4
  Stack after return:    f5
  Stack use after scope: f8
  Global redzone:        f9
  Global init order:     f6
  Poisoned by user:      f7
  ASan internal:         fe

Comment 13

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: debugger-memory.patch

Just adding the commit message to the patch.

Comment 14

[Christian Holler (:decoder)]

Did you solve the use-after-free problem?

Comment 15

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

(In reply to Christian Holler (:decoder) from comment #14)
> Did you solve the use-after-free problem?

No, but I was under the impression it was unrelated and already existing since it was a test that has nothing to do with the changes here made here.

I'll do another try push.

Comment 16

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

https://tbpl.mozilla.org/?tree=Try&rev=1a0c9a2b3098

Comment 17

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

And it looks like the ASan failure is gone.

Comment 18

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8fde70fb3b9b

Comment 19

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/8fde70fb3b9b