1293551 - Add debug-only function to dump Promise's allocation site and resolution site.

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Tooru Fujisawa [:arai]]

While investigating bug 1293546,
I felt it's useful to be able to dump Promise's allocation site call stack, in debugger, like:

  call JS::DumpPromiseAllocationSite(cx, promise)

it helps finding where the uncaught promise rejection happens.

Comment 1

[Tooru Fujisawa [:arai]]

Attachment: Add debug-only function to dump Promise allocation site and resolution site.

added 4 functions:
  JS::DumpPromiseAllocationSite
  JS::DumpPromiseResolutionSite
  PromiseObject::dumpAllocationSite
  PromiseObject::dumpResolutionSite
that encodes call site into UTF8 string and dumps it to stderr.

actually, what I used is JS::DumpPromiseAllocationSite (and PromiseObject::dumpAllocationSite internally) only, so not sure if JS::DumpPromiseResolutionSite helps much, but added just in case, as it doesn't take much code.

Comment 2

[Till Schneidereit [:till]]

Comment on attachment 8779183 [details] [diff] [review]
Add debug-only function to dump Promise allocation site and resolution site.

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

Yeah, this seems useful. I wonder if dumping stack objects isn't something we should have in other places, too. Perhaps something like `char * js::BuildUTF8StackString(JSContext* cx, HandleObject startFrame)`? Then you could just have

JS::DumpPromiseAllocationSite(JSContext* cx, JS::HandleObject promise)
{
    UniqueChars stack(BuildUTF8StackString(cx, promise->as<PromiseObject>().allocationSite());
    fputs(stack.get(), stderr);
}

BuildUTF8StackString would probably live in SavedFrame.{h,cpp}. Perhaps ask fitzgen for feedback on that, too.

::: js/src/jsapi.cpp
@@ +4762,5 @@
>  
> +JS_PUBLIC_API(void)
> +JS::DumpPromiseAllocationSite(JSContext* cx, JS::HandleObject promise)
> +{
> +    return promise->as<PromiseObject>().dumpAllocationSite(cx);

This shouldn't return anything.

@@ +4768,5 @@
> +
> +JS_PUBLIC_API(void)
> +JS::DumpPromiseResolutionSite(JSContext* cx, JS::HandleObject promise)
> +{
> +    return promise->as<PromiseObject>().dumpResolutionSite(cx);

Same here.

Comment 3

[Tooru Fujisawa [:arai]]

Attachment: Part 1: Add js::BuildUTF8StackString function.

Added js::BuildUTF8StackString function, that converts SavedFrame to UTF8 string of call stack, to call it from debug functions.

Comment 4

[Tooru Fujisawa [:arai]]

Attachment: Part 2: Add debug-only function to dump Promise allocation site and resolution site.

Changed to use js::BuildUTF8StackString, and removed PromiseObject methods.

Comment 5

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

Comment on attachment 8779466 [details] [diff] [review]
Part 1: Add js::BuildUTF8StackString function.

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

LGTM!

::: js/src/vm/SavedStacks.cpp
@@ +1597,5 @@
> +    RootedString stackStr(cx);
> +    if (!JS::BuildStackString(cx, stack, &stackStr))
> +        return nullptr;
> +
> +    return JS_EncodeStringToUTF8(cx, stackStr);

Unless it makes this function really awkward to use, it would be nice to add slightly stronger typing by returning UTF8Chars[0].

[0] http://searchfox.org/mozilla-central/rev/d83f1528dbcb1e837d99cf5b6c36a90b03bf0e77/js/public/CharacterEncoding.h#71

Comment 6

[Till Schneidereit [:till]]

Comment on attachment 8779469 [details] [diff] [review]
Part 2: Add debug-only function to dump Promise allocation site and resolution site.

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

Looks great, thanks.

Comment 7

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b706d46a20e55fcea6517f627113519a1d426c51
Bug 1293551 - Part 1: Add js::BuildUTF8StackString function. r=fitzgen

https://hg.mozilla.org/integration/mozilla-inbound/rev/1d6467628641cf48006d31e346e892c784da1b9e
Bug 1293551 - Part 2: Add debug-only function to dump Promise allocation site and resolution site. r=till

Comment 8

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

https://hg.mozilla.org/mozilla-central/rev/b706d46a20e5
https://hg.mozilla.org/mozilla-central/rev/1d6467628641