Closed Bug 957475 Opened 6 years ago Closed 6 years ago

B2G crash on bild.de - js::NewObjectWithClassProtoCommon(js::ExclusiveContext*, js::Class const*, JSObject*, JSObject*, js::gc::AllocKind, js::NewObjectKind)

Categories

(Core :: JavaScript Engine: JIT, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
mozilla29
blocking-b2g 1.3+
Tracking Status
firefox26 --- unaffected
firefox27 + fixed
firefox28 + fixed
firefox29 + verified
firefox-esr24 --- unaffected
b2g18 --- unaffected
b2g-v1.1hd --- unaffected
b2g-v1.2 --- unaffected
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed
fennec 27+ ---

People

(Reporter: gwagner, Assigned: nbp)

References

Details

(4 keywords, Whiteboard: [b2g-crash])

Crash Data

Attachments

(1 file)

I am running --enable-debug --enable-optimize on my nexus and get a 90% reproducible crash when loading bild.de.
Shu debugged a little bit.

Seems to be JIT related:
Program received signal SIGSEGV, Segmentation fault.
setAliasedVar (v=..., name=0xb2000660, fi=..., cx=<optimized out>, this=<optimized out>) at ../../../js/src/vm/ScopeObject-inl.h:35
35	    setSlot(fi.scopeSlot(), v);
(gdb) bt
#0  setAliasedVar (v=..., name=0xb2000660, fi=..., cx=<optimized out>, this=<optimized out>) at ../../../js/src/vm/ScopeObject-inl.h:35
#1  js::CallObject::createForFunction (cx=0xacec5e20, frame=...) at ../../../js/src/vm/ScopeObject.cpp:254
#2  0xb5aa53a6 in js::jit::BaselineFrame::initFunctionScopeObjects (this=0xbee7af10, cx=0xacec5e20) at ../../../js/src/jit/BaselineFrame.cpp:97
#3  0xaedfaa7c in ?? ()
#4  0xaedfaa7c in ?? ()

The script in question is http://www.bild.de/code/core,27210750.39-30104864.1-30104856.1-30104858.1-30104854.1-30104860.4-30104862.1-25425024.1-25425012.5-27210766.47-30104872.1-30407988.3-29329618.6-29329622.4-31550384.2-3005

and the function:
#7  0xb5aa5636 in js::jit::BaselineFrame::initFunctionScopeObjects (this=0xbe8e9f10, cx=0xac587b40) at ../../../js/src/jit/BaselineFrame.cpp:97
97	    CallObject *callobj = CallObject::createForFunction(cx, this);
(gdb) call this->script()->dump(cx)
loc   line  op
----- ----  --
main:
00000:   6  getaliasedvar "h" (hops = 0, slot = 6)
00005:   6  pop
00006:   6  getlocal 1
00009:   6  pop
00010:   6  getaliasedvar "a" (hops = 0, slot = 2)
00015:   6  typeof
00016:   6  string "object"
00021:   6  eq
00022:   6  ifeq 144 (+122)
00027:   6  getaliasedvar "c" (hops = 0, slot = 3)
00032:   6  typeof
00033:   6  string "string"
00038:   6  ne
00039:   6  and 77 (+38)
00044:   6  pop
00045:   6  getaliasedvar "d" (hops = 0, slot = 4)
00050:   6  or 61 (+11)
00055:   6  pop
00056:   6  getaliasedvar "c" (hops = 0, slot = 3)
00061:   6  setaliasedvar "d" (hops = 0, slot = 4)
00066:   6  pop
00067:   6  getaliasedvar "b" (hops = 1, slot = 3)
00072:   6  setaliasedvar "c" (hops = 0, slot = 3)
00077:   6  pop
00078:   6  getaliasedvar "a" (hops = 0, slot = 2)
00083:   6  iter 1
00085:   6  goto 133 (+48)
00090:   6  loophead
00091:   6  iternext
00092:   6  setlocal 1
00095:   6  pop
00096:   6  this
00097:   6  dup
00098:   6  callprop "on"
00103:   6  swap
00104:   6  getlocal 1
00107:   6  getaliasedvar "c" (hops = 0, slot = 3)
00112:   6  getaliasedvar "d" (hops = 0, slot = 4)
00117:   6  getaliasedvar "a" (hops = 0, slot = 2)
00122:   6  getlocal 1
00125:   6  getelem
00126:   6  getarg 4
00129:   6  call 5
00132:   6  pop
00133:   6  loopentry 1
00135:   6  moreiter
00136:   6  ifne 90 (-46)
00141:   6  enditer
00142:   6  this
00143:   6  return
00144:   6  getaliasedvar "d" (hops = 0, slot = 4)
00149:   6  null
00150:   6  eq
00151:   6  and 164 (+13)
00156:   6  pop
00157:   6  getaliasedvar "e" (hops = 0, slot = 5)
00162:   6  null
00163:   6  eq
00164:   6  ifeq 200 (+36)
00169:   6  getaliasedvar "c" (hops = 0, slot = 3)
00174:   6  setaliasedvar "e" (hops = 0, slot = 5)
00179:   6  pop
00180:   6  getaliasedvar "b" (hops = 1, slot = 3)
00185:   6  setaliasedvar "c" (hops = 0, slot = 3)
00190:   6  setaliasedvar "d" (hops = 0, slot = 4)
00195:   6  goto 288 (+93)
00200:   6  getaliasedvar "e" (hops = 0, slot = 5)
00205:   6  null
00206:   6  eq
00207:   6  and 288 (+81)
00212:   6  pop
00213:   6  getaliasedvar "c" (hops = 0, slot = 3)
00218:   6  typeof
00219:   6  string "string"
00224:   6  eq
00225:   6  ifeq 256 (+31)
00230:   6  getaliasedvar "d" (hops = 0, slot = 4)
00235:   6  setaliasedvar "e" (hops = 0, slot = 5)
00240:   6  pop
00241:   6  getaliasedvar "b" (hops = 1, slot = 3)
00246:   6  setaliasedvar "d" (hops = 0, slot = 4)
00251:   6  goto 288 (+37)
00256:   6  getaliasedvar "d" (hops = 0, slot = 4)
00261:   6  setaliasedvar "e" (hops = 0, slot = 5)
00266:   6  pop
00267:   6  getaliasedvar "c" (hops = 0, slot = 3)
00272:   6  setaliasedvar "d" (hops = 0, slot = 4)
00277:   6  pop
00278:   6  getaliasedvar "b" (hops = 1, slot = 3)
00283:   6  setaliasedvar "c" (hops = 0, slot = 3)
00288:   6  pop
00289:   6  getaliasedvar "e" (hops = 0, slot = 5)
00294:   6  false
00295:   6  stricteq
00296:   6  ifeq 317 (+21)
00301:   6  getaliasedvar "J" (hops = 1, slot = 28)
00306:   6  setaliasedvar "e" (hops = 0, slot = 5)
00311:   6  pop
00312:   6  goto 330 (+18)
00317:   6  getaliasedvar "e" (hops = 0, slot = 5)
00322:   6  not
00323:   6  ifeq 330 (+7)
00328:   6  this
00329:   6  return
00330:   6  getarg 4
00333:   6  one
00334:   6  stricteq
00335:   6  and 423 (+88)
00340:   6  pop
00341:   6  getaliasedvar "e" (hops = 0, slot = 5)
00346:   6  setaliasedvar "h" (hops = 0, slot = 6)
00351:   6  pop
00352:   6  lambda (function (a){f().off(a);return h.apply(this,arguments)})
00357:   6  setaliasedvar "e" (hops = 0, slot = 5)
00362:   6  pop
00363:   6  getaliasedvar "e" (hops = 0, slot = 5)
00368:   6  getaliasedvar "h" (hops = 0, slot = 6)
00373:   6  getprop "guid"
00378:   6  or 418 (+40)
00383:   6  pop
00384:   6  getaliasedvar "h" (hops = 0, slot = 6)
00389:   6  getaliasedvar "f" (hops = 1, slot = 35)
00394:   6  dup
00395:   6  getprop "guid"
00400:   6  pos
00401:   6  dup
00402:   6  one
00403:   6  add
00404:   6  pick 2
00406:   6  swap
00407:   6  setprop "guid"
00412:   6  pop
00413:   6  setprop "guid"
00418:   6  setprop "guid"
00423:   6  pop
00424:   6  this
00425:   6  dup
00426:   6  callprop "each"
00431:   6  swap
blocking-b2g: --- → 1.3?
I looked at this for a bit with Gregor. It's crashing when trying to copy the 5th argument of that function into a CallObject, because the 5th argument is a Value with an object tag but a 0x0 payload, which seems bad.

We weren't able to print the JS stack without crashing gdb, so no progress was made in figuring out where the Value came from. I initially suspected the arguments rectifier, but that seems like such a commonly called piece of code that we shouldn't be seeing a crash in it now.

Marty, could you take a look?
Also, can someone try to reduce the bild.de code to a test case (preferably runnable from the shell)? That'll make this easy.
Keywords: testcase-wanted
(In reply to Gregor Wagner [:gwagner] from comment #0)
> I am running --enable-debug --enable-optimize on my nexus and get a 90%
> reproducible crash when loading bild.de.

Are you running Fennec or B2G on your nexus?
Can you update the platform too. (unless this is an IPhone?)

I will see tomorrow if I can reproduce it on B2G.
Flags: needinfo?(nicolas.b.pierron)
(In reply to Nicolas B. Pierron [:nbp] from comment #3)
> (In reply to Gregor Wagner [:gwagner] from comment #0)
> > I am running --enable-debug --enable-optimize on my nexus and get a 90%
> > reproducible crash when loading bild.de.
> 
> Are you running Fennec or B2G on your nexus?

Its b2g.

> Can you update the platform too. (unless this is an IPhone?)

It's current trunk.

> 
> I will see tomorrow if I can reproduce it on B2G.

Thanks!
I have an unrelated problem (gfx/compositor issue on all Unagis) which makes my life impossible on B2G since 2 weeks.

If you can provide a marionette test case which cause the crash, maybe I can let hydra bisect inbound, you can find an example of a test case here:

https://github.com/nbp/gaia-ui-tests/blob/bench/gaiatest/tests/browser/benchmarks/test_bench_sunspider.py
Flags: needinfo?(nicolas.b.pierron)
(In reply to Nicolas B. Pierron [:nbp] from comment #5)
> I have an unrelated problem (gfx/compositor issue on all Unagis) which makes
> my life impossible on B2G since 2 weeks.
> 
> If you can provide a marionette test case which cause the crash, maybe I can
> let hydra bisect inbound, you can find an example of a test case here:
> 
> https://github.com/nbp/gaia-ui-tests/blob/bench/gaiatest/tests/browser/
> benchmarks/test_bench_sunspider.py

Unagis are no longer supported. For debugging purpose I would suggest to use a nexus 4 since it has enough ram and its the fastest phone we have. Otherwise switch your Unagi with a hamachi.
Depends on: 958073
My blind guess is that this might be similar to what jandem and djvj discussed on IRC previously:
  http://logbot.glob.com.au/?c=mozilla%23ionmonkey&s=19+Dec+2013&e=19+Dec+2013&h=payload#c69710

(In reply to Gregor Wagner [:gwagner] from comment #6)
> Unagis are no longer supported. For debugging purpose I would suggest to use
> a nexus 4 since it has enough ram and its the fastest phone we have.
> Otherwise switch your Unagi with a hamachi.

Unagis are the only devices that I have for bisecting, If you have a marionette test case, I can bisect through as this is a compositor issue as marionette does not care about the screen as long as the DOM is correct.
(In reply to Nicolas B. Pierron [:nbp] from comment #7)
> My blind guess is that this might be similar to what jandem and djvj
> discussed on IRC previously:
>  
> http://logbot.glob.com.au/
> ?c=mozilla%23ionmonkey&s=19+Dec+2013&e=19+Dec+2013&h=payload#c69710

By the way, we should have assertions to ensure that we are not producing such objects in bailouts, can you check if you can reproduce this issue with a debug build?  I n the mean time I will test if I can reproduce with a Keon with GP nightly image.
(In reply to Nicolas B. Pierron [:nbp] from comment #8)
> (In reply to Nicolas B. Pierron [:nbp] from comment #7)
> > My blind guess is that this might be similar to what jandem and djvj
> > discussed on IRC previously:
> >  
> > http://logbot.glob.com.au/
> > ?c=mozilla%23ionmonkey&s=19+Dec+2013&e=19+Dec+2013&h=payload#c69710
> 
> By the way, we should have assertions to ensure that we are not producing
> such objects in bailouts, can you check if you can reproduce this issue with
> a debug build?  I n the mean time I will test if I can reproduce with a Keon
> with GP nightly image.

This is an --enable-debug build but I also have to use --enable-optimize to make the image fit on the device.
(In reply to Gregor Wagner [:gwagner] from comment #9)
> (In reply to Nicolas B. Pierron [:nbp] from comment #8)
> > By the way, we should have assertions to ensure that we are not producing
> > such objects in bailouts, can you check if you can reproduce this issue with
> > a debug build?  I n the mean time I will test if I can reproduce with a Keon
> > with GP nightly image.
> 
> This is an --enable-debug build but I also have to use --enable-optimize to
> make the image fit on the device.

Have you checked if there is any output on stderr/stdout when you manually run b2g.sh ?
I managed to reproduce it with Geeksphone nightly, but I would have to make my own build to be able to debug this issue.

In the mean time, I will try to see if I can reproduce it with a snapshot of their website, in order to minimize it and make a test case.

CC: Marty, as this seems to be ARM specific.
CC: djvj, Do you think this can be related to the argument object being used within a lambda function?

Gregor:

> (gdb) bt
> #0  setAliasedVar (v=..., name=0xb2000660, fi=..., cx=<optimized out>,
> this=<optimized out>) at ../../../js/src/vm/ScopeObject-inl.h:35
> #1  js::CallObject::createForFunction (cx=0xacec5e20, frame=...) at
> ../../../js/src/vm/ScopeObject.cpp:254
> #2  0xb5aa53a6 in js::jit::BaselineFrame::initFunctionScopeObjects
> (this=0xbee7af10, cx=0xacec5e20) at ../../../js/src/jit/BaselineFrame.cpp:97
> #3  0xaedfaa7c in ?? ()
> #4  0xaedfaa7c in ?? ()
> 
> […]
> #7  0xb5aa5636 in js::jit::BaselineFrame::initFunctionScopeObjects
> (this=0xbe8e9f10, cx=0xac587b40) at ../../../js/src/jit/BaselineFrame.cpp:97
> 97	    CallObject *callobj = CallObject::createForFunction(cx, this);

Is this frame part of the same backtrace?  If so you need to be aware that we gdb is most of the time lying as soon as there is an unknown jitted frame in the middle.
(In reply to Nicolas B. Pierron [:nbp] from comment #10)
> (In reply to Gregor Wagner [:gwagner] from comment #9)
> > (In reply to Nicolas B. Pierron [:nbp] from comment #8)
> > > By the way, we should have assertions to ensure that we are not producing
> > > such objects in bailouts, can you check if you can reproduce this issue with
> > > a debug build?  I n the mean time I will test if I can reproduce with a Keon
> > > with GP nightly image.
> > 
> > This is an --enable-debug build but I also have to use --enable-optimize to
> > make the image fit on the device.
> 
> Have you checked if there is any output on stderr/stdout when you manually
> run b2g.sh ?
> I managed to reproduce it with Geeksphone nightly, but I would have to make
> my own build to be able to debug this issue.
> 
> In the mean time, I will try to see if I can reproduce it with a snapshot of
> their website, in order to minimize it and make a test case.
> 
> CC: Marty, as this seems to be ARM specific.
> CC: djvj, Do you think this can be related to the argument object being used
> within a lambda function?
> 
> Gregor:
> 
> > (gdb) bt
> > #0  setAliasedVar (v=..., name=0xb2000660, fi=..., cx=<optimized out>,
> > this=<optimized out>) at ../../../js/src/vm/ScopeObject-inl.h:35
> > #1  js::CallObject::createForFunction (cx=0xacec5e20, frame=...) at
> > ../../../js/src/vm/ScopeObject.cpp:254
> > #2  0xb5aa53a6 in js::jit::BaselineFrame::initFunctionScopeObjects
> > (this=0xbee7af10, cx=0xacec5e20) at ../../../js/src/jit/BaselineFrame.cpp:97
> > #3  0xaedfaa7c in ?? ()
> > #4  0xaedfaa7c in ?? ()
> > 
> > […]
> > #7  0xb5aa5636 in js::jit::BaselineFrame::initFunctionScopeObjects
> > (this=0xbe8e9f10, cx=0xac587b40) at ../../../js/src/jit/BaselineFrame.cpp:97
> > 97	    CallObject *callobj = CallObject::createForFunction(cx, this);
> 
> Is this frame part of the same backtrace?  If so you need to be aware that
> we gdb is most of the time lying as soon as there is an unknown jitted frame
> in the middle.

Yep, that's the same backtrace. If it's lying, how do you even debug JIT issues on ARM then?

Anywho, if you |p v| at a frame that doesn't have v optimized out, you see that it looks like it has an object tag and a NULL payload.
QA Contact: mvaughan
This issue appears to have started reproducing on the 10/01/13 1.3 build. Before this build, I was able to load bild.de on a Buri device (Hamachi) without a crash, granted the website seems to have still been a heavy load for my device and would sometimes not display properly.

- Works -
Environmental Variables:
Device: Buri v1.3 MOZ RIL
BuildID: 20130930071202
Gaia: e7c011371aa6af696033d4b867fb9152a6985efa
Gecko: 5a49762ee832
Version: 27.0a1
Firmware Version: V1.2_US_20131115

- Broken -
Environmental Variables:
Device: Buri v1.3 MOZ RIL
BuildID: 20131001040206
Gaia: 67243760c86561e365bdd6def519105366e24be3
Gecko: d71579c316c1
Version: 27.0a1
Firmware Version: V.12_US_20131115
Keywords: regression
Can you include a crash report URL with this bug?
Flags: needinfo?(mvaughan)
Bugs in regression range in JS engine:

* bug 921490
* bug 921437
* bug 921130
* bug 921120
* bug 913224
* bug 921881
* bug 921725
Keywords: crash, reproducible
Whiteboard: [b2g-crash]
Crash Signature: [@ js::NewObjectWithClassProtoCommon(js::ExclusiveContext*, js::Class const*, JSObject*, JSObject*, js::gc::AllocKind, js::NewObjectKind)]
Summary: B2G crash on bild.de → B2G crash on bild.de - js::NewObjectWithClassProtoCommon(js::ExclusiveContext*, js::Class const*, JSObject*, JSObject*, js::gc::AllocKind, js::NewObjectKind)
Note - this also crashes on FxAndroid Nightly.
OS: Mac OS X → All
Hardware: ARM → All
Alexa places this as a popular site in Germany:

http://www.alexa.com/siteinfo/bild.de

Global Rank: 214
German Rank: 10
Putting tracking-fennec on here since apparently FxAndroid is affected as well.
tracking-fennec: --- → ?
(In reply to Jason Smith [:jsmith] from comment #19)
> Putting tracking-fennec on here since apparently FxAndroid is affected as
> well.

Oh and the script in question here where this bug is triggering is JQuery 1.7.2.

http://www.bild.de/code/core,27210750.39-30104864.1-30104856.1-30104858.1-30104854.1-30104860.4-30104862.1-25425024.1-25425012.5-27210766.47-30104872.1-30407988.3-29329618.6-29329622.4-31550384.2-3005
Desktop appears to be unaffected. The bug here is probably specifically then triggering with the mobile version of the site.
Also adding rel mgmt tracking flags since this affects a top site in Germany on FxAndroid.
(In reply to Jason Smith [:jsmith] from comment #15)
> Bugs in regression range in JS engine:
> 
> * bug 921120

This is probably the regressor. Looks a lot like bug 951528, but it was fixed last week. Gregor, can you still reproduce this or is it a different crash now?
Flags: needinfo?(anygregor)
(In reply to Jan de Mooij [:jandem] from comment #23)
> (In reply to Jason Smith [:jsmith] from comment #15)
> > Bugs in regression range in JS engine:
> > 
> > * bug 921120
> 
> This is probably the regressor. Looks a lot like bug 951528, but it was
> fixed last week. Gregor, can you still reproduce this or is it a different
> crash now?

Note - I was able to reproduce this on today's FxAndroid nightly build.
OK, needinfo from Kannan then.
Flags: needinfo?(kvijayan)
Should we mark this bug as s-s?
Closing just in case it's another regalloc bug that mess up value tags/payloads.
Group: core-security
Keywords: sec-critical
(In reply to Shu-yu Guo [:shu] from comment #11)
> Yep, that's the same backtrace. If it's lying, how do you even debug JIT
> issues on ARM then?

In general, we cannot trust gdb backtraces below the ?? frames, if you even look at what is reported by Gregor, the frame is its own parent :/

(In reply to Jan de Mooij [:jandem] from comment #23)
> (In reply to Jason Smith [:jsmith] from comment #15)
> > Bugs in regression range in JS engine:
> > 
> > * bug 921120
> 
> This is probably the regressor. Looks a lot like bug 951528, but it was
> fixed last week. Gregor, can you still reproduce this or is it a different
> crash now?

I will try to add some assertions as I suggested in Bug 951528 comment 7, and that I was expecting in comment 8.
(In reply to Nicolas B. Pierron [:nbp] from comment #28)
> if you even look at what is reported by Gregor, the frame is its own parent :/

Sorry, if this appear rude, this is a bad mix of thoughts between:
 - if you look at …
 - even the backtrace reported …
We should try to backout/disable bug 921120's patch and see if it fixes this. If it does, we should consider doing that for beta and aurora.
Depends on: 958471
Depends on: 958494
This patch disable the optimization made in Bug 921120 to get more time to investigate these issues.  This patch is adding bits which are originally removed in Bug 921120, and prevent to go deeper.  Reverting the original patch would have made thing even more complex to backport.

I verified, with this patch applied, b2g browser is now able to access bild.de without crashing.
Attachment #8358384 - Flags: review?(kvijayan)
Given that we've got a path forward here, I'm going to remove the reduced test case request. If there ends up still being a need for this, then feel free to re-add the flag.
Keywords: testcase-wanted
blocking-b2g: 1.3? → 1.3+
Depends on: 944094
Depends on: 930735
Comment on attachment 8358384 [details] [diff] [review]
disable-bug921120.patch

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

Sorry for the delayed review.  Sad to have to disable this, though.
Attachment #8358384 - Flags: review?(kvijayan) → review+
Comment on attachment 8358384 [details] [diff] [review]
disable-bug921120.patch

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

This is a register mismatch, I have no idea what can be achieved with it, as I do not yet have any details on this issue, but disabling the generated code related to this register mismatch is easier to backport.

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

The function names are enough to infer that this issue is related to SetArg and the Arguments keyword, without reading the comment (only the patch).

> Which older supported branches are affected by this flaw?

27, 28, 29.

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

bug 921120

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

This patch is a tiny revert of Bug 921120.  It should be easy to revert on all branches even across added optimizations which are disabled by this revert.

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

This patch disable the feature added in September, this would be a performance regression on Dromaeo (JQuery), but on the other hand this can easily be backported to all branches.
Attachment #8358384 - Flags: sec-approval?
(In reply to Jason Smith [:jsmith] from comment #32)
> Given that we've got a path forward here, I'm going to remove the reduced
> test case request. If there ends up still being a need for this, then feel
> free to re-add the flag.

Jason, there is still a need for a reduced test-case, as the patch that I made revert the optimization, and does not fix the underlying issue.
Keywords: testcase-wanted
(In reply to Nicolas B. Pierron [:nbp] from comment #35)
> (In reply to Jason Smith [:jsmith] from comment #32)
> > Given that we've got a path forward here, I'm going to remove the reduced
> > test case request. If there ends up still being a need for this, then feel
> > free to re-add the flag.
> 
> Jason, there is still a need for a reduced test-case, as the patch that I
> made revert the optimization, and does not fix the underlying issue.

That's best done in a followup, as the backout does the fix the bug. I don't think that's a blocker to resolving the blocking factor on this bug, so I'm pulling the flag to get this out of QA queries.
Keywords: testcase-wanted
Comment on attachment 8358384 [details] [diff] [review]
disable-bug921120.patch

sec-approval+. Please get this in and nominate an Aurora and Beta patch ASAP as we're running out of time to fix this on Beta.
Attachment #8358384 - Flags: sec-approval? → sec-approval+
Flags: needinfo?(anygregor)
Assignee: nobody → nicolas.b.pierron
Comment on attachment 8358384 [details] [diff] [review]
disable-bug921120.patch

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

::: js/src/jit/IonBuilder.cpp
@@ +9142,5 @@
>          return true;
>      }
>  
> +    // :TODO: if hasArguments() is true, and the script has a JSOP_SETARG, then
> +    // convert all arg accesses to go through the arguments object.

Drive-by nit, but can you reference this bug (bug 957475) here and mention it's disabled because of crashes? Else somebody may remove this and everything works fine until we see crashes weeks later.
Comment on attachment 8358384 [details] [diff] [review]
disable-bug921120.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 921120

User impact if declined: Bad register allocation. (likely exploitable)

Testing completed (on m-c, etc.): Tested on inbound.

Risk to taking this patch (and alternatives if risky): This patch re-add the bits removed by Bug 921120, which means that the JS script will not run under Ion and will stay in baseline instead.  This is a likely a performance regression on Dromaeo.

String or IDL/UUID changes made by this patch: N/A
Attachment #8358384 - Flags: approval-mozilla-beta?
Attachment #8358384 - Flags: approval-mozilla-aurora?
Group: javascript-core-security
User impact is also a topcrash on Firefox for Android.
Attachment #8358384 - Flags: approval-mozilla-beta?
Attachment #8358384 - Flags: approval-mozilla-beta+
Attachment #8358384 - Flags: approval-mozilla-aurora?
Attachment #8358384 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/mozilla-central/rev/1e359c06de57
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Flags: needinfo?(kvijayan)
tracking-fennec: ? → 27+
This crash still reproduces on Beta 27  Firefox for Android. Is there some code difference between 27 and 28 here? Or something special about Firefox for Android?

* http://bild.de
* http://www.avianca.com/es-co/
Hum looking at https://hg.mozilla.org/releases/mozilla-beta/log/166961 this missed beta 6. Which is what we were basing this extra investigation upon.
(In reply to Kevin Brosnan [:kbrosnan] from comment #45)
> Hum looking at https://hg.mozilla.org/releases/mozilla-beta/log/166961 this
> missed beta 6. Which is what we were basing this extra investigation upon.

Should be in beta 8 now, though.
Group: javascript-core-security
I wasn't able to see a crash on bild.de but I did see one on the other URL mentioned in comment 44, using Fennec 29 from 2014-01-05. I assume we're confident that these are both the same issue.

Verified fixed with today's Fennec 29.
Status: RESOLVED → VERIFIED
Group: core-security
You need to log in before you can comment on or make changes to this bug.