Closed Bug 1167895 Opened 5 years ago Closed 4 years ago

JS functions with non-ascii characters make the profiler fail

Categories

(Core :: Gecko Profiler, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
mozilla41
Tracking Status
firefox41 --- verified

People

(Reporter: mstange, Assigned: shu)

References

Details

Attachments

(2 files)

Attached file testcase
STR:
1. Start the profiler from the Gecko profiler addon with the "js" feature enabled.
2. Load the attached testcase.
3. Try to get a profile using Ctrl+Shift+2.

A cleopatra tab opens but nothing happens.

I can reproduce this on OS X but not on Linux. I hit this bug because I have the µBlock extension installed, which regularly runs a function with the name µb.bindTabToPageStats.

Debugging this brought me to NS_ConvertUTF8toUTF16 js_string(nsDependentCString(buf.get())); in TableTicker::ToJSObject, which fails because the conversion encounters a byte with the value 181 at the µ character, and nsUTF8Utils.h:430 complains that that's not valid UTF-8.

I'm pretty sure this is a regression from bug 1154115 - it looks like the old JSON writer ignored invalid characters in strings.
Hmm... I wonder what's not getting escaped to UTF-8 properly.

Do you know if it's a JIT frame or an interpreter frame? Their function names are stringified differently.

For JIT frames, this is how the function names are fetched:

https://dxr.mozilla.org/mozilla-central/source/js/src/jit/JitcodeMap.cpp?from=Jitcodemap.cpp&case=true#320

That call to |CharsToNewUTF8CharsZ| should do the right thing about escaping.

For interpreter frames, I think it's this:

https://dxr.mozilla.org/mozilla-central/source/js/src/vm/SPSProfiler.cpp?from=spsprofiler.cpp&case=true#342

If I had to guess I'd guess that "%hs" doesn't do the right thing.
Flags: qe-verify+
Hm I actually can't reproduce this.
Attachment #8616306 - Flags: review?(kvijayan)
Assignee: nobody → shu
Status: NEW → ASSIGNED
This patch fixes the problem for me. Thanks!
Comment on attachment 8616306 [details] [diff] [review]
Escape SPS profile strings to UTF8 properly.

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

::: js/src/vm/SPSProfiler.cpp
@@ +341,5 @@
>      // Construct the descriptive string.
>      DebugOnly<size_t> ret;
>      if (atom) {
>          JS::AutoCheckCannotGC nogc;
> +        mozilla::UniquePtr<char, JS::FreePolicy> atomStr =

can we use 'auto' here for the var?  The constructor makes it clear what type is being returned.
Attachment #8616306 - Flags: review?(kvijayan) → review+
https://hg.mozilla.org/mozilla-central/rev/b8f85d4648fc
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
I think I've managed to reproduce this on a Nightly 41 build from 2015-05-23, here's a screenshot: http://i.imgur.com/d0O6511.png / http://i.imgur.com/XyCRkXH.png. This is what get's displayed after pressing [ctrl] + [shift] + [2], instead of actual data from the profiler, on both Mac OS X and Windows.

If that is what Markus described in Comment 0, then this is verified fixed on Firefox 41.0b3 (20150820142145) using Windows 10 x64 (10525), Mac OS X 10.10.4 and Ubuntu 14.04 (x64). Markus, could you please confirm?
Flags: needinfo?(mstange)
(In reply to Andrei Vaida, QA [:avaida] from comment #8)
> If that is what Markus described in Comment 0

Yes, that's what it looked like when it failed.
Status: RESOLVED → VERIFIED
Flags: needinfo?(mstange)
(In reply to Markus Stange [:mstange] (away until 2015-08-24) from comment #9)
> Yes, that's what it looked like when it failed.

Great, thanks for confirming! This is verified fixed then, based on Comment 8 and Comment 9.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.