510091 - Fennec crash on mousedown on an iframe

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: 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

Comment 1

[Jason Orendorff [:jorendorff]]

Taking. Stuart's going to help me build Fennec tomorrow.

Comment 2

[Ben Combee (:bcombee)]

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

Comment 3

[Ben Combee (:bcombee)]

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++
....

Comment 4

[Ben Combee (:bcombee)]

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.

Comment 5

[Jason Orendorff [:jorendorff]]

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");
  }

Comment 6

[Jason Orendorff [:jorendorff]]

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...

Comment 7

[Ben Combee (:bcombee)]

The callsite for dumpLn in this case is http://mxr.mozilla.org/mobile-browser/source/chrome/content/browser.js#1253

Comment 8

[Jason Orendorff [:jorendorff]]

Attachment: 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.

Comment 9

[Jason Orendorff [:jorendorff]]

Attachment: v2 - Now with 33% fewer humiliating errors!

v2 has the advantage of actually compiling. You could even theoretically run it.

Comment 10

[Jason Orendorff [:jorendorff]]

Attachment: v3 - now perfect in every way

Comment 11

[Jason Orendorff [:jorendorff]]

Attachment: v5 - now perfect in every way and then some

Thanks to Waldo for the review.

Comment 12

[Jeff Walden [:Waldo]]

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/~

Comment 13

[Jason Orendorff [:jorendorff]]

http://hg.mozilla.org/tracemonkey/rev/b8b9d78f12d2

Comment 14

[Ben Combee (:bcombee)]

v5 patch fixes the issue I saw in Fennec.

Comment 15

[Brendan Eich [:brendan]]

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

Comment 16

[Jeff Walden [:Waldo]]

(JS bug, not Fennec bug, just the latter happened to trigger it)

Comment 17

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/b8b9d78f12d2

Comment 18

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/e784431dfa61