Baldr: update text-format warning message

RESOLVED FIXED in Firefox 54

Status

()

Core
JavaScript Engine
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: luke, Assigned: luke)

Tracking

(Blocks: 1 bug)

unspecified
mozilla54
Points:
---

Firefox Tracking Flags

(firefox54 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

a year ago
I think we can now take off the ASCII-art experimental text format message.  Instead, until we do source-per-function, I think we should print a warning when the source is huge (explaining the issues are known and intended to be fixed).
(Assignee)

Comment 1

a year ago
Created attachment 8842647 [details] [diff] [review]
warning-message

I started by writing a patch that actually cut off after N lines, but even for really small N, after the BinaryToText() was done, there would be a hang in debugger.js.  Anyhow, this patch just disables it entirely for modules with bytecode bigger than 1mb.
Attachment #8842647 - Flags: review?(ydelendik)
Comment on attachment 8842647 [details] [diff] [review]
warning-message

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

Looks good.

::: js/src/wasm/WasmCode.cpp
@@ +735,5 @@
>  {
>      StringBuffer buffer(cx);
> +    if (maybeBytecode_ && maybeBytecode_->bytes.length() > TooBig) {
> +        if (!buffer.append(tooBigMessage))
> +            return nullptr;

can you add something like `MOZ_ASSERT(!maybeSourceMap_);` just to indicate we are keeping source map not set?
Attachment #8842647 - Flags: review?(ydelendik) → review+
(Assignee)

Comment 3

a year ago
Created attachment 8843086 [details] [diff] [review]
rm-func-locs

I noticed these are used anymore.
Attachment #8843086 - Flags: review?(ydelendik)
(Assignee)

Comment 4

a year ago
*aren't
Comment on attachment 8843086 [details] [diff] [review]
rm-func-locs

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

Thanks

::: js/src/wasm/WasmCode.cpp
@@ +771,2 @@
>  #if DEBUG
>          // Checking source map invariant: expression and function locations must be sorted

Can you remove function locations reference from the comment?
Attachment #8843086 - Flags: review?(ydelendik) → review+

Comment 6

a year ago
Pushed by lwagner@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d2ad3e67b80b
Baldr: remove dead functionlocs (r=yury)
https://hg.mozilla.org/integration/mozilla-inbound/rev/c5c15cca280a
Baldr: issue warning for binary-to-text on huge modules (r=yury)

Comment 7

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d2ad3e67b80b
https://hg.mozilla.org/mozilla-central/rev/c5c15cca280a
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.