1159973 - crash in js::frontend::Parser<js::frontend::SyntaxParseHandler>::unaryExpr(js::frontend::Parser<js::frontend::SyntaxParseHandler>::InvokedPrediction)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Mats Palmgren (inactive)]

This bug was filed from the Socorro interface and is 
report bp-228e68c8-0202-48c2-b763-7ddd72150423.
=============================================================

New signature in 38/39/40, first report in build 2015022400.

Reported crashes over the past 28 days:

Product	Version	Percentage	Number Of Crashes
Firefox 	38.0b6 	26.81 %	740
Firefox 	38.0b1 	16.30 %	450
Firefox 	38.0b5 	15.62 %	431
Firefox 	38.0b4 	14.64 %	404
Firefox 	38.0b3 	13.55 %	374
Firefox 	38.0b2 	7.14 %	197
Firefox 	39.0a2 	4.24 %	117
Firefox 	40.0a1 	1.16 %	32
Firefox 	38.0a2 	0.43 %	12
Firefox 	38.0b8 	0.04 %	1
Firefox 	39.0a1 	0.04 %	1
FennecAndroid 	38.0b4 	0.04 %	1

Comment 1

[Mats Palmgren (inactive)]

[Tracking Requested - why for this release]:
It's currently #25 in the Top Crashers for Firefox 38.0b list.

Comment 2

[Sylvestre Ledru [:Sylvestre]]

Top crash, tracking.

Comment 3

[Sylvestre Ledru [:Sylvestre]]

Brian, Jason, could you help? This is one of the top crash of 38. The gtb of the rc is next Sunday. Thanks

Comment 4

[Brian Hackett [Laid off!]]

I haven't touched the parser much during the last year and don't know about the changes that have been going into it.  A regression window would be nice; there are several parser changes around 2/24 but I don't know if any of them are suspicious here.

Comment 5

[Jason Orendorff [:jorendorff]]

I'll look into it today.

Comment 6

[Jason Orendorff [:jorendorff]]

Having trouble getting a toe-hold here. Windows-only...

How can I get disassembly for the exact xul.dll file that was in this build? I see the crashing instruction address in the raw dump... I'd like to know which instruction actually caused the crash, because it's not at all obvious what could have caused it.

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Too late for 38 but I would be happy to take a patch for 38.0.5 or 38 esr.

Comment 8

[Sylvestre Ledru [:Sylvestre]]

This is in the top 10 top crash list. We might take a patch for 38.0.5 if there is a patch.

Comment 9

[Tooru Fujisawa [:arai]]

Attachment: Disassemble for js::frontend::Parser<js::frontend::SyntaxParseHandler>::unaryExpr (and some more)

Attached the source-and-assembly-mixed code for js::frontend::Parser<js::frontend::SyntaxParseHandler>::unaryExpr for build 20150415030206, generated using symbol server and source server, on VS2013.

Here is related part (line 6610).

>   6609:         /* Don't look across a newline boundary for a postfix incop. */
>   6610:         if (!tokenStream.peekTokenSameLine(&tt, TokenStream::Operand))
> 57726791 8B 8E A0 02 00 00    mov         ecx,dword ptr [esi+2A0h]
> 57726797 8D 41 11             lea         eax,[ecx+11h]
> 5772679A C1 E0 05             shl         eax,5
> 5772679D 03 C6                add         eax,esi
> 5772679F 89 44 24 18          mov         dword ptr [esp+18h],eax
> 577267A3 39 9E A4 02 00 00    cmp         dword ptr [esi+2A4h],ebx
> 577267A9 0F 84 C9 EF 39 00    je          `js::irregexp::RegExpEmpty::GetInstance'::`2'::`dynamic atexit destructor for 'instance''+107D78h (57AC5778h)
> 577267AF 8B 50 08             mov         edx,dword ptr [eax+8]
> 577267B2 8B 86 A8 02 00 00    mov         eax,dword ptr [esi+2A8h]
> 577267B8 8B 7E 04             mov         edi,dword ptr [esi+4]
> 577267BB 2B 86 10 02 00 00    sub         eax,dword ptr [esi+210h]
> 577267C1 39 14 87             cmp         dword ptr [edi+eax*4],edx
> 577267C4 8B 7C 24 14          mov         edi,dword ptr [esp+14h]
> 577267C8 0F 87 AA EF 39 00    ja          `js::irregexp::RegExpEmpty::GetInstance'::`2'::`dynamic atexit destructor for 'instance''+107D78h (57AC5778h)
> 577267CE 8B 7E 04             mov         edi,dword ptr [esi+4]
> 577267D1 3B 54 87 04          cmp         edx,dword ptr [edi+eax*4+4]
> 577267D5 8B 7C 24 14          mov         edi,dword ptr [esp+14h]
> 577267D9 0F 83 99 EF 39 00    jae         `js::irregexp::RegExpEmpty::GetInstance'::`2'::`dynamic atexit destructor for 'instance''+107D78h (57AC5778h)
> 577267DF 8D 41 01             lea         eax,[ecx+1]
> 577267E2 83 E0 03             and         eax,3
> 577267E5 83 C0 11             add         eax,11h
> 577267E8 C1 E0 05             shl         eax,5
> 577267EB 8B 04 30             mov         eax,dword ptr [eax+esi]
>   6611:             return null();

if the lower word of offset is same as raw dump (0x3967c1 / 0x5f3267c1), the read access violation seems to be caused by following instruction. The access violation happens in 0x7fb00004, and setting eax=0x00600001 and edi=0x7e300000 matches it.

> 577267C1 39 14 87             cmp         dword ptr [edi+eax*4],edx

Not sure why jump to irregexp code exist there, and cannot figure out where those instructions come from. maybe it comes from a kind of optimization, or something goes wrong in the symbol file or disassembly display?

Comment 10

[Tooru Fujisawa [:arai]]

maybe it's inside SourceCoords::isOnThisLine ?
in that case, eax corresponds to lineIndex or similar value.

>         bool isOnThisLine(uint32_t offset, uint32_t lineNum) const {
>             uint32_t lineIndex = lineNumToIndex(lineNum);
>             MOZ_ASSERT(lineIndex + 1 < lineStartOffsets_.length());  // +1 due to sentinel
>             return lineStartOffsets_[lineIndex] <= offset &&
>                    offset < lineStartOffsets_[lineIndex + 1];
>         }

Comment 11

[Tooru Fujisawa [:arai]]

marking as security, to post the testcase

Comment 12

[Tooru Fujisawa [:arai]]

Found a testcase to cause similar crash.

==== a.html ====
<html>
<body>
<script src="a.js">
</script>
</body>
</html>

==== a.js ====
var s = "\n".repeat(0x6000000) + "a\n+1";
new Function(s);


Opening a.html in Nightly 32bit on Windows 7 crashes the tab.

crash report is here.
https://crash-stats.mozilla.com/report/index/3f7ba41c-4ee6-4c2d-8987-dbe912150505

Comment 13

[Tooru Fujisawa [:arai]]

with debug build (http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2015-05-06-mozilla-central-debug/ ),
following assertion in TokenStream::SourceCoords::add fails

https://dxr.mozilla.org/mozilla-central/source/js/src/frontend/TokenStream.cpp#171
> MOZ_ASSERT(lineStartOffsets_[0] == 0 && lineStartOffsets_[sentinelIndex] == MAX_PTR);

from reading register, one of `lineStartOffsets_[0]` or `lineStartOffsets_[sentinelIndex]` returned 0x01ffffff (maybe latter one?).

furthur debugging may need local build, not sure my machine is capable for building recent m-c :/

Comment 14

[Tooru Fujisawa [:arai]]

Testing on ESR31 branch which has same crash (I'm afraid my machine cannot build latest m-c).
In TokenStream::SourceCoords::add, lineStartOffsets_.length() grows up to 0x2000000, and stays same for subsequent call. so something goes wrong there.
maybe Vector::append fails to grow it's storage? (the returned value is ignored there)

Comment 15

[Tooru Fujisawa [:arai]]

Confirmed Vector::append fails when the length is 0x2000000, and firefox crashes after that, because it reads 0x6000001-th data, in TokenStream::SourceCoords::isOnThisLine.

Currently, even if Vector::append fails, TokenStream::SourceCoords::add ignores and the parse is continues.
In that case, srcCoords.lineNum() doesn't return correct line number for the area outside of the storage (always return the last line in the storage).
But peekTokenSameLine uses it to check whether two tokens are in same line or not.
So, we cannot parse the code correctly (either matching or ignoring all [no line terminator here]s) after Vector::append fails, it means that all code which depends on ASI will fail there.
Should we abort parsing if OOM happens? or is it better to handle that case specially (not sure it's possible in simple way)?

Comment 16

[Tooru Fujisawa [:arai]]

Attachment: Abort parsing when TokenStream::SourceCoords hits OOM.

prepared a patch to abort parsing when TokenStream::SourceCoords::add hits OOM, by changing some methods to return bool instead of void or some data.
(of course this patch is for m-c, not esr31 branch)

Actually, I also prepared another patch to avoid accessing out-of-range data, and continue parsing, but it hits OOM later anyway. so, if the actual crashes are similar to the testcase (I cannot believe that case can happen in practice though...), it won't worth to continue parsing.

jorendorff, how do you think about this approach, and the possibility of the testcase?

Comment 17

[Tooru Fujisawa [:arai]]

I forgot to note that the crash no more happens when applying this patch to esr31 branch. "out of memory" is shown in console soon after opening the testcase.
I hope this is also true for m-c.

Comment 18

[Jason Orendorff [:jorendorff]]

(In reply to Tooru Fujisawa [:arai] from comment #15)
> Confirmed Vector::append fails when the length is 0x2000000, and firefox
> crashes after that, because it reads 0x6000001-th data, in
> TokenStream::SourceCoords::isOnThisLine.

Yep.

f+ on this approach. But it'll be easier to backport a patch that fixes isOnThisLine narrowly. Either way is fine with me.

Comment 19

[Jason Orendorff [:jorendorff]]

Comment on attachment 8605831 [details] [diff] [review]
Abort parsing when TokenStream::SourceCoords hits OOM.

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

Better test the effect on parsing speed. We do call getChar() for every single character of input.

::: js/src/frontend/TokenStream.cpp
@@ -174,5 @@
>          // We haven't seen this newline before.  Update lineStartOffsets_.
> -        // We ignore any failures due to OOM -- because we always have a
> -        // sentinel node, it'll just be like the newline wasn't present.  I.e.
> -        // the line numbers will be wrong, but the code won't crash or anything
> -        // like that.

This comment was r=me, rev 1f3587e02361, March 2013. Should have known better.

Comment 20

[Tooru Fujisawa [:arai]]

Attachment: Abort parsing when TokenStream::SourceCoords hits OOM.

Thank you for your feedback :)

I noticed that we can check the OOM more lazily.  So, when we hit OOM, store true to hitOOM flags, and check it later at the end of TokenStream::getTokenInternal and TokenStream::advance.  It reduces the number of checks, compared to previous patch.  Only SourceCoords::isOnThisLine accesses lineStartOffsets_ directly without using/checking length, and which only happens after TokenStream::getTokenInternal.  For other places, index is lower than the length (calculated by SourceCoords::lineIndexOf, it won't return out of range index if sentinel exists).

The problem is that how to test this patch.  I prepared TokenStreamOOM.js, but it's extremely heavy (so added "slow" attribute), and the number of function required to cause OOM depends on the environment.  In js shell, it's 3 or 4 on my windows laptop with 1GB RAM, but it could be more than 100 on others.  What do you think is the best way here?

This patch also fixed the crash in esr31 branch.  Tried backporting this patch to mozilla-release also, and it can be done in almost straight-forward way.

I'll post performance comparison result in next comment.

Comment 21

[Tooru Fujisawa [:arai]]

Attachment: Performance comparison result with parsemark.py

Here's performance comparison result with js/src/tests/parsemark.py (modified to use `elapsed() / 1000` instead of `new Date()`, to increase precision).  In most case, current patch (patch-B) is faster than previous patch (patch-A), and almost same as before.

Comment 22

[Ryan VanderMeulen [:RyanVM]]

Can we get a sec rating for this bug?

Comment 23

[Tooru Fujisawa [:arai]]

iiuc, attacker can use (not direct access) two consequent 32bit unsigned integers at semi-random memory in heap (1GB range)
  X : a 32bit unsigned integer at B + I * 4
  Y : a 32bit unsigned integer at B + (I + 1) * 4
    where
      B = a malloc-ed address
      N <= I and I < 0x10000000
      N : a certain value, where previous malloc fails, e.g. 0x2000000 for me on windows laptop with 1GB RAM
those values can be compared with a semi-random value V
  V : a 32bit unsigned integer
    where I <= N and N < 0x10000000
and can detect following condition, as in result of JS error or behavior change, which is caused by ASI happes or not
  X <= V and V < Y

Currently I don't see any way to write a data, or read direct data.

So if that match to "non-sensitive information", sec-low might be appropriate?

Comment 24

[Robert Kaiser]

This is a very significant portion of 38.0.5 beta crashes, currently 4.6% of b3 crashes.

Jason, can we get this reviewed?

Ideally, we'd get this uplifted to 38.0.5 still but the release candidate goes to build within few hours from now, so not sure if that's even possible.

Comment 25

[Jason Orendorff [:jorendorff]]

Comment on attachment 8606671 [details] [diff] [review]
Abort parsing when TokenStream::SourceCoords hits OOM.

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

Hmm. It's harder to see that this is correct.

A 2% performance difference is worth a little effort, I guess. But for safety, can we turn the assertion in TokenStream::isOnThisLine into an if-statement? After all, with this weaker enforcement regime there's really nothing preventing us from having the same bug again, by failing to check for OOM anywhere downstream from that srcCoords.add() call.

Drop the test. It's not practical to have regression tests like this; no one is going to run them. If you can find another way to make this testable, perhaps only in debug builds, that's great, but that can happen in a separate bug.

Comment 26

[Tooru Fujisawa [:arai]]

Thank you for reviewing :)

> can we turn the assertion in TokenStream::isOnThisLine into an if-statement?

Do you mean something like following?

>    MOZ_ALWAYS_INLINE bool
>    peekTokenSameLine(TokenKind* ttp, Modifier modifier = None) {
>        ....
>        if (lookahead != 0) {
>            bool onThisLine;
>            if (!srcCoords.isOnThisLine(curr.pos.end, lineno, &onThisLine))
>                return reportError(JSMSG_OUT_OF_MEMORY);
>            if (onThisLine) {
>                MOZ_ASSERT(!flags.hadError);
>                *ttp = tokens[(cursor + 1) & ntokensMask].type;
>                return true;
>            }
>        }
>        ....
>    }
>        bool isOnThisLine(uint32_t offset, uint32_t lineNum, bool* onThisLine) const {
>            uint32_t lineIndex = lineNumToIndex(lineNum);
>            if (lineIndex + 1 >= lineStartOffsets_.length()) // +1 due to sentinel
>                return false;
>            *onThisLine = lineStartOffsets_[lineIndex] <= offset &&
>                          offset < lineStartOffsets_[lineIndex + 1];
>            return true;
>        }

Comment 27

[Gregor Wagner [:gwagner]]

B2G 2.2 (based on gecko 37) code complete is june 8th. Can we get this landed before that?

Comment 28

[Tooru Fujisawa [:arai]]

Attachment: Abort parsing when TokenStream::SourceCoords hits OOM. r=jorendorff

okay, applied the change in comment #26 (also to BytecodeEmitter.cpp).
I'll request sec-approval shortly (now writing).

Comment 29

[Tooru Fujisawa [:arai]]

Comment on attachment 8609619 [details] [diff] [review]
Abort parsing when TokenStream::SourceCoords hits OOM. r=jorendorff

[Security approval request comment]
> How easily could an exploit be constructed based on the patch?

It will be easy to create a web page which causes crash with segmentation fault (read). Not sure it's possible to steal any security-sensitive information based only on this bug.

> Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

Main part of this patch says there was unhandled OOM (comment, variable name, etc), and also the change in isOnThisLine method may explain that out-of-bounds read existed there.

> Which older supported branches are affected by this flaw?

All of them.

> If not all supported branches, which bug introduced the flaw?

Maybe one of bug 747831 or bug 900346.

> Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

Not yet, but should also be easy since there's almost no difference in all supported branches.

> How likely is this patch to cause regressions; how much testing does it need?

Automated tests should be sufficient to catch any regression.

Comment 30

[Tooru Fujisawa [:arai]]

for quick testing, it passes jstests and jit-test with --jitflags=none on OS X, 64bit build, locally.

Comment 31

[Robert Kaiser]

Ryan, given the high volume of those crashes on 38.0.5 beta, I really want release management to actively decide if this is wontfix or not. I know that what we have out there atm is supposed to be the final RC, but this is 4-5% of all our crashes and I'm personally walking the line between calling this a release blocker or not. I'd like relman to consciously evaluate that.

Comment 32

[Tooru Fujisawa [:arai]]

I'd like to make sure the things to do, not to slow down the approval/landing process,
if there're wrong things, or something I can do in addition to them (or do out of the order?), please tell me.

I'm going to do following:
  1. after sec-approval, land the patch to mozilla-inbound
  2. once the patch passed the build/test on mozilla-inbound (should I wait until it gets merged into mozilla-central?), request approval for following branches
    * mozilla-beta
    * mozilla-aurora
    * mozilla-b2g37_v2_2
    * mozilla-esr38 
    (is this correct list? is there any other branch should I request approval?)
  3. after approval, land them to each branch

Is it okay?

Comment 33

[Ryan VanderMeulen [:RyanVM]]

If KaiRo wants to make a push for 38.0.5 inclusion, you'll need to add mozilla-release to that list. I think you're OK requesting approval after it's on inbound, but recognize that often approval isn't granted until it actually makes it to m-c.

Comment 34

[Robert Kaiser]

(In reply to Tooru Fujisawa [:arai] from comment #32)
>   1. after sec-approval, land the patch to mozilla-inbound

I thought we only need sec-approval for sec-high and sec-crit, and not for sec-low?

The rest is correct, with the addition of what Ryan said.

Comment 35

[Ryan VanderMeulen [:RyanVM]]

(In reply to Robert Kaiser (:[e-mail]) from comment #34)
> I thought we only need sec-approval for sec-high and sec-crit, and not for
> sec-low?

Touche! Go ahead and get this on inbound, arai.

Comment 36

[Tooru Fujisawa [:arai]]

Comment on attachment 8609619 [details] [diff] [review]
Abort parsing when TokenStream::SourceCoords hits OOM. r=jorendorff

Thank you Ryan and KaiRo :)

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #33)
> If KaiRo wants to make a push for 38.0.5 inclusion, you'll need to add
> mozilla-release to that list. I think you're OK requesting approval after
> it's on inbound, but recognize that often approval isn't granted until it
> actually makes it to m-c.

Okay, I also prepared a patch for mozilla-release, and it passed tests locally.

(In reply to Robert Kaiser (:[e-mail]) from comment #34)
> (In reply to Tooru Fujisawa [:arai] from comment #32)
> >   1. after sec-approval, land the patch to mozilla-inbound
> 
> I thought we only need sec-approval for sec-high and sec-crit, and not for
> sec-low?
> 
> The rest is correct, with the addition of what Ryan said.

Ah, indeed. I'll land it after the tree open :D

Comment 37

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/de4736293b6a

Comment 38

[Tooru Fujisawa [:arai]]

Comment on attachment 8609619 [details] [diff] [review]
Abort parsing when TokenStream::SourceCoords hits OOM. r=jorendorff

Same patch is appricable to mozilla-aurora

Approval Request Comment (for mozilla-aurora, mozilla-beta, and mozilla-release)
> [Feature/regressing bug #]
Maybe one of bug 747831 or bug 900346.
> [User impact if declined]
topcrash on Windows.
a cetrain webpage (or perhaps add-on) can cause crash due to out-of-bounds read.
> [Describe test coverage new/current, TreeHerder]
Main part related to OOM is not tested on automated test though, it passed local test with comment #12, on Windows and OS X.
> [Risks and why]
Low, it changes the behavior almost only for OOM case, other (OOM checking) parts are testable on automated test. The part related to OOM is not yet tested on automated test, but it was tested locally with mozilla-central, mozilla-beta, and mozilla-esr31 branches.
> [String/UUID change made/needed]
None


I'll post the patch for other branches soon.

Comment 39

[Tooru Fujisawa [:arai]]

Attachment: (mozilla-beta) Abort parsing when TokenStream::SourceCoords hits OOM. r=jorendorff

Almost same patch as Attachment #8609619 [details] [diff], only trivial change due to other patches.

Comment 40

[Tooru Fujisawa [:arai]]

Attachment: (mozilla-release/mozilla-esr38) Abort parsing when TokenStream::SourceCoords hits OOM. r=jorendorff

Almost same patch as Attachment #8609619 [details] [diff], only trivial change due to other patches.

[Approval Request Comment] (for mozilla-esr38)
> If this is not a sec:{high,crit} bug, please state case for ESR consideration
topcrash on Windows
> User impact if declined
a cetrain webpage (or perhaps add-on) can cause crash due to out-of-bounds read.
> Fix Landed on Version
Will be 41, mozilla-inbound now.
> Risk to taking this patch (and alternatives if risky)
Low, it changes the behavior almost only for OOM case, other (OOM checking) parts are testable on automated test. The part related to OOM is not yet tested on automated test, but it was tested locally with mozilla-central, mozilla-beta, and mozilla-esr31 branches.
> String or UUID changes made by this patch
None

Comment 41

[Tooru Fujisawa [:arai]]

Attachment: (mozilla-b2g37_v2_2) Abort parsing when TokenStream::SourceCoords hits OOM. r=jorendorff

Almost same patch as Attachment #8609619 [details] [diff], only trivial change due to other patches.

[Approval Request Comment] (for mozilla-b2g37_v2_2)
> Bug caused by (feature/regressing bug #)
Maybe one of bug 747831 or bug 900346.
> User impact if declined
Possible crash due to out-of-bounds read.
Currently most of crashreports are for Windows, but same situation could be possible on others, since it's caused by OOM and platform independent bug.
> Testing completed
Passed automated test in mozilla-inbound.
> Risk to taking this patch (and alternatives if risky)
Low, it changes the behavior almost only for OOM case, other (OOM checking) parts are testable on automated test. The part related to OOM is not yet tested on automated test, but it was tested locally with mozilla-central, mozilla-beta, and mozilla-esr31 branches.
> String or UUID changes made by this patch
None

Comment 42

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/de4736293b6a

Comment 43

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8609798 [details] [diff] [review]
(mozilla-b2g37_v2_2) Abort parsing when TokenStream::SourceCoords hits OOM. r=jorendorff

Josh, do you have a strong opinion about whether we should consider this for v2.0/v2.1 as well? Normally we wouldn't worry too much about a sec-low on those branches, but I'm wondering if it'd still be wanted from a stability perspective. Happy to land it wherever you want it :)

Comment 44

[Sylvestre Ledru [:Sylvestre]]

For now, even if it is a topcrash, I am not planning to do a 38.0.5 for this fix. However, if there is another critical issue coming, we could take the two.

Comment 45

[Tooru Fujisawa [:arai]]

Thank you! :)

https://hg.mozilla.org/releases/mozilla-beta/rev/9ed526f22941
https://hg.mozilla.org/releases/mozilla-esr38/rev/876fb7e07554

Why approval-mozilla-aurora-?
Delay or something?

Comment 46

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8609619 [details] [diff] [review]
Abort parsing when TokenStream::SourceCoords hits OOM. r=jorendorff

No, big fingers :/

Comment 47

[Tooru Fujisawa [:arai]]

Thank you again :D

https://hg.mozilla.org/releases/mozilla-aurora/rev/262c4775bdd2

Comment 48

[Josh Cheng [:josh]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #43)
> Comment on attachment 8609798 [details] [diff] [review]
> (mozilla-b2g37_v2_2) Abort parsing when TokenStream::SourceCoords hits OOM.
> r=jorendorff
> 
> Josh, do you have a strong opinion about whether we should consider this for
> v2.0/v2.1 as well? Normally we wouldn't worry too much about a sec-low on
> those branches, but I'm wondering if it'd still be wanted from a stability
> perspective. Happy to land it wherever you want it :)

Dear Ryan,
I think we can take this for 2.1 as we are planning to launch device as 2.1S. For 2.0 I think we can wontfix it.
Please let me know if you have any concern. Thanks!

Comment 49

[Tooru Fujisawa [:arai]]

Thanks!
So, I should use mozilla-b2g34_v2_1s repository, right?

Comment 50

[Tooru Fujisawa [:arai]]

sorry the question wasn't clear.
there're mozilla-b2g34_v2_1 and mozilla-b2g34_v2_1s, and I wonder which to use to land the patch.

Comment 51

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/115112d51e08

Comment 52

[Josh Cheng [:josh]]

(In reply to Tooru Fujisawa [:arai] from comment #50)
> sorry the question wasn't clear.
> there're mozilla-b2g34_v2_1 and mozilla-b2g34_v2_1s, and I wonder which to
> use to land the patch.

Hi arai, it's mozilla-b2g34_v2_1.
mozilla-b2g34_v2_1s is for Dolphin device branch which device EPM Steven will decide whether to merge there.
Thanks!

Also NI Steven

Comment 53

[Ryan VanderMeulen [:RyanVM]]

The v2.1s branch is kept in sync with v2.1 via merges, so you don't need to worry about it. Thanks for handling all the backports! Looks like you're clear to land everywhere but mozilla-release now :-)

Comment 54

[Tooru Fujisawa [:arai]]

Thanks again :D

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/6e357036c54b

Comment 55

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/414d430012ee
https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/6e357036c54b

Comment 56

[Steven Yang [:styang]]

clear the ni since it has been landed in 2.1S.

Comment 57

[Bogdan Maris, Desktop Test Engineering]

Was unable to manually reproduce this crash on old builds. 

Socorro does not show any crashes on a build newer then Firefox 39.0b1, all crashes are on Firefox 38.0.1 or 38 betas, marking this bug as verified fixed across builds.

https://crash-stats.mozilla.com/report/list?product=Firefox&signature=js%3A%3Afrontend%3A%3AParser%3Cjs%3A%3Afrontend%3A%3ASyntaxParseHandler%3E%3A%3AunaryExpr%28js%3A%3Afrontend%3A%3AParser%3Cjs%3A%3Afrontend%3A%3ASyntaxParseHandler%3E%3A%3AInvokedPrediction%29
https://crash-stats.mozilla.com/report/list?signature=js%3A%3Afrontend%3A%3AParser%3Cjs%3A%3Afrontend%3A%3ASyntaxParseHandler%3E%3A%3AunaryExpr%28%29