Fennec crash on mousedown on an iframe

RESOLVED FIXED in mozilla1.9.2a1

Status

()

P1
critical
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: vingtetun, Assigned: jorendorff)

Tracking

Trunk
mozilla1.9.2a1
x86
All
Points:
---
Bug Flags:
blocking1.9.2 +

Firefox Tracking Flags

(status1.9.2 beta1-fixed, fennec1.0b3+)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(2 attachments, 3 obsolete attachments)

Created attachment 394148 [details]
URL to test

Go to the attachment with Fennec and try to click on the iframe.

Actual results: 
 Crash by hitting an assert
 dragStart got element  Assertion failure: pc < endpc, at /scratchbox/users/steakdepapillon/home/steakdepapillon/mozilla-central/js/src/jsobj.cpp:2291

Expected results:
 Nothing special
Summary: Fennec crash onmousedown on an iframe → Fennec crash on mousedown on an iframe
(Assignee)

Comment 1

9 years ago
Taking. Stuart's going to help me build Fennec tomorrow.
Assignee: nobody → jorendorff
output from js_DumpScript(cx, script) from call to Detecting

main:
00000:  64  zero
00001:  64  setlocal 0
00004:  64  pop
00005:  64  goto 29 (24)
00008:  64  loop
00009:  64  callname ""
00012:  64  arguments
00013:  64  getlocal 0
00016:  64  getelem
00017:  64  string "false"
00020:  64  add
00021:  64  call 1
00024:  64  pop
00025:  64  localinc 0
00028:  64  pop
00029:  64  getlocal 0
00032:  64  argcnt
00033:  64  lt
00034:  64  ifne 8 (-26)
00037:  65  callname ""
00040:  65  string "true"
00043:  65  call 1
00046:  65  pop
00047:  65  stop
Inside Detecting, script->code is 0x34c37dc, yet the pc value passed in is 0x5fc89910.

 	ntdll.dll!777e8b2e() 	
 	[Frames below may be incorrect and/or missing, no symbols loaded for ntdll.dll]	
 	js3250.dll!JS_Assert(const char * s=0x5fc2bc90, const char * file=0x5fc2bc68, int ln=0x000008f3)  Line 65	C++
>	js3250.dll!Detecting(JSContext * cx=0x034b7d20, unsigned char * pc=0x5fc89910)  Line 2291 + 0x1e bytes	C++
 	js3250.dll!js_GetPropertyHelper(JSContext * cx=0x034b7d20, JSObject * obj=0x04363680, int id=0x01d23244, int cacheResult=0x00000001, int * vp=0x002bd05c)  Line 4298 + 0xd bytes	C++
 	js3250.dll!js_Interpret(JSContext * cx=0x034b7d20)  Line 4506 + 0x23 bytes	C++
 	js3250.dll!js_Invoke(JSContext * cx=0x034b7d20, unsigned int argc=0x00000001, int * vp=0x0374e228, unsigned int flags=0x00000000)  Line 1379 + 0x9 bytes	C++
....
The failure it earlier... the reason Detecting is called is because op is read from pc as JSOP_GETPROP.  That opcode isn't in the script disassembly.
(Assignee)

Comment 5

9 years ago
Interesting.  Building fennec now, so I'll be there in a bit.

The code for the script in comment #2 doesn't make sense to me. Maybe brendan can shed some light on the meaning of `callname ""`. But, it's like,

  function () {
      for (var i = 0; i < arguments.length; i++)
          ???(arguments[x] + "false");
      ???("true");
  }
(Assignee)

Comment 6

9 years ago
Brendan, what can cause this code:

>63  dumpLn: function dumpLn() {
>64    for (var i = 0; i < arguments.length; i++) { dump(arguments[i] + ' '); }
>65    dump("\n");
>66  }

to look like the disassembly in comment 2?

The source is from:
http://mxr.mozilla.org/mobile-browser/source/chrome/content/Util.js#63

I'm debugging with bcombee right now...
(Assignee)

Comment 8

9 years ago
Created attachment 394345 [details] [diff] [review]
v1

The assertion is bogus when we're in an imacro.

Reproducing this requires strict mode and an object that doesn't have a .valueOf method (anything that has Object.prototype on its prototype chain will have the default one).

This reproduces sporadically because we're only asserting the upper bound ("don't walk off the end of the script") and not the lower one--so the assertion may pass even though pc is nowhere near the script code.

So, the first hunk fixes the assertion.

But we probably shouldn't emit warnings for imacro bytecode, hmm? So the second hunk takes care of that.
Attachment #394345 - Flags: review?(jwalden+bmo)
(Assignee)

Comment 9

9 years ago
Created attachment 394349 [details] [diff] [review]
v2 - Now with 33% fewer humiliating errors!

v2 has the advantage of actually compiling. You could even theoretically run it.
Attachment #394345 - Attachment is obsolete: true
Attachment #394349 - Flags: review?(jwalden+bmo)
Attachment #394345 - Flags: review?(jwalden+bmo)
(Assignee)

Comment 10

9 years ago
Created attachment 394352 [details] [diff] [review]
v3 - now perfect in every way
Attachment #394349 - Attachment is obsolete: true
Attachment #394352 - Flags: review?(jwalden+bmo)
Attachment #394349 - Flags: review?(jwalden+bmo)
(Assignee)

Updated

9 years ago
Attachment #394349 - Attachment description: v2 → v2 - Now with 50% fewer humiliating errors!

Updated

9 years ago
Flags: wanted-fennec1.0?
(Assignee)

Comment 11

9 years ago
Created attachment 394360 [details] [diff] [review]
v5 - now perfect in every way and then some

Thanks to Waldo for the review.
Attachment #394352 - Attachment is obsolete: true
Attachment #394360 - Flags: review?(jwalden+bmo)
Attachment #394352 - Flags: review?(jwalden+bmo)
(Assignee)

Updated

9 years ago
Attachment #394349 - Attachment description: v2 - Now with 50% fewer humiliating errors! → v2 - Now with 33% fewer humiliating errors!
Comment on attachment 394360 [details] [diff] [review]
v5 - now perfect in every way and then some

o/~ Oh Lord, it's hard to be humble when you're perfect in every way o/~
Attachment #394360 - Flags: review?(jwalden+bmo) → review+
(Assignee)

Comment 13

9 years ago
http://hg.mozilla.org/tracemonkey/rev/b8b9d78f12d2
Whiteboard: fixed-in-tracemonkey
v5 patch fixes the issue I saw in Fennec.
Got behind on bugmail, sorry -- glad you figured it all out.

Boy, do we need to get rid of bytecode inspection from the VM.

/be
(JS bug, not Fennec bug, just the latter happened to trigger it)
Component: General → JavaScript Engine
Product: Fennec → Core
QA Contact: general → general
Target Milestone: --- → mozilla1.9.2a1

Updated

9 years ago
tracking-fennec: ? → 1.0b3+
Flags: wanted-fennec1.0? → blocking1.9.2+

Comment 17

9 years ago
http://hg.mozilla.org/mozilla-central/rev/b8b9d78f12d2
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED

Updated

9 years ago
Priority: -- → P1
You need to log in before you can comment on or make changes to this bug.