689101 - Binary compatibility of jsval on MSVC

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Firebug has been using jsParent to get the parent window (outer most
scope) to see where a debugger break happened.

Firebug uses iteration as follows:

var frame = // the current frame passed into a JSD hook
var scope = frame.scope;
while (scope.jsParent)
    scope = scope.jsParent;

Since Firefox Nightly 09/23/2011, the jsParent is null

This problem breaks Firebug's debugger.

Honza

Comment 1

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: Test extension

STR:

1) Install the extension
2) Load page with a script e.g: http://getfirebug.com/tests/content/branches/1.9/net/601/issue601.html
3) Check the system console (dump) or Firefox Error Console
4) You should see: Error: scope.jsParent: null
Source File: chrome://simpledebugger/content/debugger.js
Line: 189

The scope is coming from frame.scope and the frame is passed into a breakpoint hook.

Honza

Comment 2

[Jan Honza Odvarko [:Honza] (always need-info? me)]

I am marking as p1 since Firebug doesn't currently work on Nightlies.

Honza

Comment 3

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Also note that bug 689118 (Firefox crashes with Firebug) appeared around the same time.

Honza

Comment 4

[Steve Fink [:sfink] [:s:]]

I can reproduce this. Will track it down further.

For testing, I added a jsdIValue.jsGlobal to go directly to the global object rather than walking up the scope chain. It works, and gives the expected Window object. It works internally by calling JS_GetGlobalForObject, which does exactly the same thing as what you're doing via JSD (walk up the scopes one by one until you hit the root.)

So something must not be allowing you to see one of those parents.

Comment 5

[Steve Fink [:sfink] [:s:]]

What I am seeing is that frame.scope.jsParent can be null sometimes (and that will trigger the printout you describe in comment 1), but that's when frame.scope is already the parent window. So that seems ok. I'm now unsure what changed, and how it breaks firebug?

Comment 6

[Steve Fink [:sfink] [:s:]]

Attachment: Updated test extension

Here's an updated test extension that always grabs the top of the scope chain and prints it out. The only change is in content/debugger.js, in the getOutermostScope() function. When I use it on http://getfirebug.com, it shows that the outermost scope is always a Window.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Jan, what's the actual regression range here?

Comment 8

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Boris Zbarsky (:bz) from comment #7)
> Jan, what's the actual regression range here?

I have been testing mozilla-central:

1) OK
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2011-09-22-03-09-07-mozilla-central/
Built from http://hg.mozilla.org/mozilla-central/rev/4495e1f795c2

2) ERROR
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2011-09-23-03-08-57-mozilla-central/
Built from http://hg.mozilla.org/mozilla-central/rev/259d1556c221

Honza

Comment 9

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Steve Fink [:sfink] from comment #5)
> I'm now unsure what changed, and how it breaks firebug?
You can reproduce the problem in Firebug as follows:

1) Install Firebug 1.9a3
http://getfirebug.com/releases/firebug/1.9/firebug-1.9.0a3.xpi

2) Open Firebug, select and enable the Script panel.

3) Load any page with JavaScript (e.g. www.google.com)

4) Reload the page, the Script panel always shows: No Javascript on this page

---

The problem is on this line in Firebug (in getOutermostScope method):
http://code.google.com/p/fbug/source/browse/branches/firebug1.9/modules/firebug-service.js#2948

Firebug can't find the parent window and so, no script is tracked (that's why the panels shows 'no script on this page')

Honza

Comment 10

[Steve Fink [:sfink] [:s:]]

(In reply to Jan Honza Odvarko from comment #9)
> (In reply to Steve Fink [:sfink] from comment #5)
> > I'm now unsure what changed, and how it breaks firebug?
> You can reproduce the problem in Firebug as follows:
> 
> 1) Install Firebug 1.9a3
> http://getfirebug.com/releases/firebug/1.9/firebug-1.9.0a3.xpi
> 
> 2) Open Firebug, select and enable the Script panel.
> 
> 3) Load any page with JavaScript (e.g. www.google.com)
> 
> 4) Reload the page, the Script panel always shows: No Javascript on this page

In my current nightly, from 2011-10-01 (f25928e4847d) from a fresh restart, it seems to work for http://www.google.com. (I am able to see source code, set breakpoints, and it stops at those breakpoints.) Same thing for http://getfirebug.com/. But a little earlier, www.google.com was giving me "Debugger not activated."

Is this fixed, or is it being tricky and not reproducing reliably?

Comment 11

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Just tested with nightly (also: f25928e4847d) and I don't see any difference. When loading www.google.com, the Script panel shows "No Javascript on this page" just as before.

> Is this fixed, or is it being tricky and not reproducing reliably?
Definitely not fixed and I can reliably reproduce the problem on my machine.
(didn't see the source in the Script panel ever since the problem appeared)

Also note that I can reproduce the same problem in Aurora now (tested with http://hg.mozilla.org/releases/mozilla-aurora/rev/79eab065f0d0)


Honza

Comment 12

[Adesh]

I have been looking for some solution this problem from last 3-4 days and just got this bug filed here today only.

I am able to re-produce this on Nightly Build - 10.0a1 (2011-10-02). This is just to second the claim of Jan.

Comment 13

[Steve Fink [:sfink] [:s:]]

(In reply to Adesh from comment #12)
> I am able to re-produce this on Nightly Build - 10.0a1 (2011-10-02). This is
> just to second the claim of Jan.

Don't worry, I believe him! :) But your corroboration does help make the case that it's more likely to be my environment that is unusual, not his.

So, here are my exact steps:

Get the nightly (obviously this varies depending on your OS; I'm on Linux x86-64):

1. cd ~/Builds; wget -Onightly-2011-09-24.tar.bz2 'http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2011-09-24-03-08-58-mozilla-central/firefox-9.0a1.en-US.linux-x86_64.tar.bz2'

2. mkdir nightly-2011-09-24; cd nightly-2011-09-24; tar jxvf ../nightly-2011-09-24.tar.bz2

3. cd firefox; mv updater updater.disabled (my first run-through updated itself underneath me. Grr... Same behavior with the current nightly, though.)

Create a profile:

1. Run the nightly from 2011-09-24 with -ProfileManager, create a new profile named 'Bug689101'.
2. Exit out

Install Firebug 1.9.0a3:

1. Start up FF: ./firefox -no-remote -P Bug689101
2. No, I don't want this to be my default browser.
3. Go to http://getfirebug.com/releases/firebug/1.9/
4. Click on firebug-1.9.0a3.xpi to install
5. Restart

Try it out:

1. Go to www.google.com
2. Fail to see the firebug bug icon. Huh? Flail around for a while, finally discovering the Firebug menu item in the View menu. Redo all these instructions from the start. Please ignore this step.
3. Press F12 to display the Firebug console.
4. Click on the 'Script' panel, and click on 'Enable'.
5. Ctrl-R to reload.

at this point, I see the Script panel fill with minified source code, and all the line numbers on the left are green.

6. Click on lines 1 and 2 to set breakpoints.
7. Ctrl-R to reload

Now I see a yellow triangle on the line 2 breakpoint, showing me it's stopped there.

8. Go to about:buildconfig.

It says "http://hg.mozilla.org/mozilla-central/rev/c71229984353"

9. Go to the Help menu, choose About Nightly.

It says "9.0a1 (2011-09-24)." It also downloads the latest nightly, even with the updater binary moved out of the way. But it won't be able to apply it! Bwahahaha!!

Anyway, I'm guessing that you're seeing something different after that last step 5. Maybe it's OS-specific? I need to reboot into Windows 7 in order to root my Galaxy tablet, so I'll try again there. (Which would suck, since it'll be a lot more trouble for me to delve into there... maybe I'll try 32-bit Linux too.)

Comment 14

[Adesh]

Thanks for the detailed steps. That made life easy for me :)

I followed all the steps as you mentioned and installed Firefox & firebug. But on testing it out, it showed same old "No Javascript on this page" message. :(

My test environment is completely different than yours so that may be the starting point to explore this issue.
I am on Windows Xp - 32 bit.
Tested Firebug 1.9.0a3 with both 
- "10.0a1 (2011-10-03) - http://hg.mozilla.org/mozilla-central/rev/a896a9e237a0" 
- "9.0a1 (2011-09-24) - http://hg.mozilla.org/mozilla-central/rev/c71229984353"
with same result.
By the way, i tried disabling all add-ons except Firebug to see if any other add-on is troubling in background but there didn't change this behavior.

I will give it a shot tomorrow on Fedora box tomorrow to see what it comes up with.

Comment 15

[Steve Fink [:sfink] [:s:]]

I tried both the 32- and 64-bit builds under Windows 7, and both show the "No javascript on this page" problem. So unfortunately for me, this happens on Windows but not Linux. Dunno about OSX.

At least I can reproduce it now. I think I have a Windows VM lying around with a reasonable development/debugging environment.

Comment 16

[Steve Fink [:sfink] [:s:]]

When I run on Linux, I get a small handful of hits to the getOutermostScope function, and most of them result in a Window object. The rest are BackstagePasses.

On Windows, I get many more hits: a bunch of BackstagePasses, one Sandbox, and several ChromeWindows.

I'm wondering now if the problem has nothing to do with jsParent, but instead it's that the breakpoint (or script creation callback) is not getting hit at all on Windows.

I don't know why I see *more* total hits on Windows, though -- where are those extra hits on Linux?

Looking at the FBTrace stack for a Window object on Linux:
resource://firebug/firebug-service.js (2946) [...]
resource://firebug/firebug-service.js (3027) [...]
resource://firebug/firebug-service.js (2493) [...]
resource://firebug/firebug-service.js (2235) [...]
resource://firebug/firebug-service.js (1968) [...]
resource://firebug/firebug-service.js (4201) [...]
 () [...]
http://www.google.com/ (1) [...]

And for a ChromeWindow object on Windows (note that firebug-service.js:4201 above is the same as firebug-service:4232 here; I added some debugging prints):
resource://firebug/firebug-service.js (2946) [...]
resource://firebug/firebug-service.js (3058) [...]
resource://firebug/firebug-service.js (2436) [...]
resource://firebug/firebug-service.js (2235) [...]
resource://firebug/firebug-service.js (1968) [...]
resource://firebug/firebug-service.js (4232) [...]
resource://firebug_rjs/lib/domplate.js (257) [...]
resource://firebug_rjs/lib/domplate.js (257) [...]
resource://firebug_rjs/lib/domplate.js (158) [...]
resource://firebug_rjs/lib/domplate.js (1104) [...]
resource://firebug_rjs/chrome/reps.js (2471) [...]
resource://firebug_rjs/js/scriptPanel.js (1573) [...]
resource://firebug_rjs/js/scriptPanel.js (638) [...]
resource://firebug_rjs/js/scriptPanel.js (667) [...]
chrome://firebug/content/firefox/bindings.xml (272) [...]
chrome://firebug/content/firefox/bindings.xml (227) [...]
resource://firebug_rjs/chrome/chrome.js (738) [...]
resource://firebug_rjs/firebug.js (632) [...]
resource://firebug_rjs/firebug.js (709) [...]
resource://firebug_rjs/bti/inProcess/browser.js (697) [...]
resource://firebug_rjs/lib/events.js (39) [...]
resource://firebug_rjs/firefox/tabWatcher.js (548) [...]
resource://firebug_rjs/firefox/tabWatcher.js (269) [...]
resource://firebug_rjs/firefox/tabWatcher.js (247) [...]
resource://firebug_rjs/bti/inProcess/browser.js (263) [...]
resource://firebug_rjs/firebug.js (668) [...]
chrome://browser/content/browser.xul (1) [...]
 () [...]

So on Windows I'm seeing breakpoints for firebug-internal stuff that I do not see on Linux, and I am not seeing the content page's breakpoints. Odd.

Comment 17

[Steve Fink [:sfink] [:s:]]

Never mind. The difference in the set of scopes being passed in seems to be an artifact of the exact startup sequence. I currently have a Windows run and a Linux run showing the same series of scopes passed to getOutermostScope() -- including Windows getting Window scope objects. But Windows is still claiming there's no JS on the page.

Comment 18

[Jan Honza Odvarko [:Honza] (always need-info? me)]

All sounds like a promising progress, thanks!

Is there anything I could help with on the Firebug side (testing, more analysis, etc.)?

Honza

Comment 19

[Steve Fink [:sfink] [:s:]]

Aha! If I enable FBS_FINDDEBUGGER, on Windows I see:

askDebuggersForSupport using global 1.5611535421733788e-304 for http://www.google.com/

and on Linux:

askDebuggersForSupport using global [object Window] for http://www.google.com/

That object is what is returned from getOutermostScope. Strangely, I printed it out within getOutermostScope and it was not mangled into a number there. It's getting corrupted along the way.

This reminds me of a bug I recently encountered with jsval out parameters in IDL being broken, but it doesn't seem to be quite the same.

Anyway, I have something to go on tomorrow.

Comment 20

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Steve Fink [:sfink] from comment #19)
> Aha! If I enable FBS_FINDDEBUGGER, on Windows I see:
> askDebuggersForSupport using global 1.5611535421733788e-304 for
> http://www.google.com/
yep, I am seeing the same.

Honza

Comment 21

[Steve Fink [:sfink] [:s:]]

Ok, so the jsParent chain search is going fine. What's breaking is scope.getWrappedValue(). When I step into that in the debugger and examine the 'scope' object from within C++, it's corrupt. Whee.

Comment 22

[Steve Fink [:sfink] [:s:]]

I hadn't done a further bisection on the regression range because my builds were mysteriously failing. Well, it looks like it's not my builds, it's any debug build. They'll crash in various ways when you try to use JSD.

The problem seems to be that when it calls JSD_GetValueWrappedJSVal(mCx, mValue), it mysteriously passes in 3 arguments instead of 2. In a debug build, it does sanity checking on the arguments and asserts. In a release build, it looks at mValue->val as a jsval, doesn't see any special tag bits in whatever random address it ends up using, and interprets it as a plain double value that needs no wrapping.

The headers seem to be in order, and Linux does not seem to do this, though I'm looking at a 32-bit Windows build and a 64-bit Linux build, so it's a little hard to compare. I'm building 32-bit Linux now. (Note that I was getting the same failure on a 64-bit Windows build, so it seems like this is more MSVC vs gcc than 32- vs 64-bit.)

Comment 23

[Steve Fink [:sfink] [:s:]]

Aha! The compiler is correct (unsurprisingly). The caller of JSD_GetValueWrappedJSVal in jsd_xpc.cpp thinks that jsvals are structs, and so passes an implicit 3rd argument. The callee in jsd_val.c thinks that jsvals are unions of simple types, and so does not expect the 3rd argument. I remember there was some recent change to further sync up js::Value and jsval, and I'm guessing that the headers are mixed up. Which is easy to do when you're also crossing from C++ that needs internal type information to C that doesn't, as we still foolishly do with JSD.

I'm guessing this change:

changeset:   77299:4f9a7183a197
user:        Luke Wagner <[e-mail]>
date:        Mon Sep 19 09:34:49 2011 -0700
summary:     Bug 684526 - Unify jsval and js::Value (r=jorendorff)

Comment 24

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Great progress Steven!

Luke, could you please take a look at the changeset?

(note that Firebug is now broken also in Aurora)

Honza

Comment 25

[Luke Wagner [:luke]]

(In reply to Steve Fink [:sfink] from comment #23)
I'm a little confused: JSD_GetValueWrappedJSVal takes two arguments, neither of which is a jsval.  Are you saying that the compiler is changing the signature?

Yes, 4f9a7183a197 changed jsapi.h to make:
  typedef JS::Value jsval;            // for C++
  typedef uniong jsval_layout jsval;  // for C

I won't be available today.  One quick thing to try is making jsval_layout a struct (containing an anonymous union).

Comment 26

[Luke Wagner [:luke]]

(In reply to Luke Wagner [:luke] from comment #25)
> (In reply to Steve Fink [:sfink] from comment #23)
> I'm a little confused: JSD_GetValueWrappedJSVal takes two arguments, neither
> of which is a jsval. 

Ah, the return type, duh.

> Are you saying that the compiler is changing the signature?

sfink explains by email: the compiler is converting a return value to a outparam.

So this suggests a simple fix: change JSD_GetValueWrappedJSVal to return its value via outparam (and return a bool indicating failure (which is actually the standard style for doing this type of thing)).

Comment 27

[Luke Wagner [:luke]]

Attachment: fix?

Well, I did have enough time to write a patch.  If I didn't screw up too badly, try builds should pop out here:

  https://tbpl.mozilla.org/?tree=Try&rev=101384f2cbbc

Comment 28

[Steve Fink [:sfink] [:s:]]

(In reply to Luke Wagner [:luke] from comment #26)
> sfink explains by email: the compiler is converting a return value to a
> outparam.
> 
> So this suggests a simple fix: change JSD_GetValueWrappedJSVal to return its
> value via outparam (and return a bool indicating failure (which is actually
> the standard style for doing this type of thing)).

Right, the fix for this particular bug is straightforward. (Ok, less than I thought, having just skimmed your patch.)

But I was really pinging you because it seems like this is a larger API bug -- C++ and C users of the API see incompatible definitions of jsval. The fix that I assumed you'd go for is to make C see |typedef struct { jsval_layout data; } jsval|. You can patch away JSD's usage, and eliminating struct returns is probably better anyway. But fixing JSD is easy (now); I'm more concerned with bug 684526 introducing API problems for embedders.

Comment 29

[Luke Wagner [:luke]]

(In reply to Steve Fink [:sfink] from comment #28)
Yeah, in comment 25 I suggested making jsval a struct as well.  comments 26/27 were more in firefighting "get it working fast" mode.  In parallel we can see if making jsval_layout a struct fixes the issue.

Comment 30

[Luke Wagner [:luke]]

Here's the patch that makes jsval_layout a struct:

  https://tbpl.mozilla.org/?tree=Try&rev=3b1646b991d5

Steve or Honza: would you be able to test this and/or the previous try builds to see if they fix the problem?

Comment 31

[Jan Honza Odvarko [:Honza] (always need-info? me)]

So, according to my testing, the try build from:
https://tbpl.mozilla.org/?tree=Try&rev=101384f2cbbc
fixes the problem!

Great job Steve & Luke!

Just to note that bug 689118 (crash) appeared at the same time and could be related.

Honza

Comment 32

[Steve Fink [:sfink] [:s:]]

(In reply to Luke Wagner [:luke] from comment #30)
> Here's the patch that makes jsval_layout a struct:
> 
>   https://tbpl.mozilla.org/?tree=Try&rev=3b1646b991d5
> 
> Steve or Honza: would you be able to test this and/or the previous try
> builds to see if they fix the problem?

I could if AT&T didn't break my internet connection for a chunk of today.

Why did you do it that way instead of just wrapping the C version in an extra struct layer, as in the patch below? It seems to compile, at least. It hangs on jit-test/tests/basic/bug674776.js, but it seems to do that before this change too.

diff --git a/js/src/jsapi.h b/js/src/jsapi.h
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -838,12 +838,12 @@ IMPL_TO_JSVAL(jsval_layout l)
  * SpiderMonkey itself is always compiled as C++, this relies on the binary
  * compatibility of jsval_layout and JS::Value (statically asserted below).
  */
-typedef union jsval_layout jsval;
+typedef struct { union jsval_layout data; } jsval;
 
 static JS_ALWAYS_INLINE jsval_layout
 JSVAL_TO_IMPL(jsval v)
 {
-    return v;
+    return v.data;
 }
 
 static JS_ALWAYS_INLINE jsval

Comment 33

[Luke Wagner [:luke]]

(In reply to Steve Fink [:sfink] from comment #32)
Hmm, that does sound nicer :)

Comment 34

[Steve Fink [:sfink] [:s:]]

Attachment: xpcshell test for jsval binary compatibility problem

Here's a test that I'm running through the try server right now to see if it catches the issue properly. It depends on the xpcshell test modifications from bug 692722 and bug 692987.

Comment 35

[Steve Fink [:sfink] [:s:]]

Attachment: Make C use a struct for jsval too

Luke - do you think we should remove the jsval return values from JSD (i.e., use your patch in attachment 565261 [details] [diff] [review]) as well? I had some comments on that patch, but I won't bother reviewing if we don't need to use it.

Thank you very much, Luke!

Comment 36

[Luke Wagner [:luke]]

Comment on attachment 565685 [details] [diff] [review]
Make C use a struct for jsval too

If struct { jsval_layout } is ABI-compatible with JS::Value (which is what we are betting with this patc) then we shouldn't need to, I think.

Comment 37

[Steve Fink [:sfink] [:s:]]

(In reply to Luke Wagner [:luke] from comment #36)
> Comment on attachment 565685 [details] [diff] [review] [diff] [details] [review]
> Make C use a struct for jsval too
> 
> If struct { jsval_layout } is ABI-compatible with JS::Value (which is what
> we are betting with this patc) then we shouldn't need to, I think.

Darn. It isn't. Wrapping the extra struct isn't enough to fool MSVC into returning it on the stack.

Making JS::Value.data public instead of private would make everything return by value, but you probably don't want to do that? (Or maybe you do -- returning directly would probably be slightly faster.)

Comment 38

[Luke Wagner [:luke]]

(In reply to Steve Fink [:sfink] from comment #37)
> Darn. It isn't. Wrapping the extra struct isn't enough to fool MSVC into
> returning it on the stack.
> 
> Making JS::Value.data public instead of private would make everything return
> by value, but you probably don't want to do that? (Or maybe you do --
> returning directly would probably be slightly faster.)

No, I'd like to keep the data member private.  So you're saying there is a different ABI return convention for POD and non-POD classes/structs?

Comment 39

[Steve Fink [:sfink] [:s:]]

(In reply to Luke Wagner [:luke] from comment #38)
> (In reply to Steve Fink [:sfink] from comment #37)
> > Darn. It isn't. Wrapping the extra struct isn't enough to fool MSVC into
> > returning it on the stack.
> > 
> > Making JS::Value.data public instead of private would make everything return
> > by value, but you probably don't want to do that? (Or maybe you do --
> > returning directly would probably be slightly faster.)
> 
> No, I'd like to keep the data member private.  So you're saying there is a
> different ABI return convention for POD and non-POD classes/structs?

I'm not sure if the exact distinction is the official POD definition, but yes. It's the cdecl calling convention either way, but returning struct types is not defined precisely by the cdecl convention and so it's implementation-dependent. Whatever gcc does here works fine, but MSVC does it differently -- perhaps basing it off of POD vs non-POD; I haven't found a counterexample to that yet.

Comment 40

[Steve Fink [:sfink] [:s:]]

Oh, I should mention that there is only a gray area for objects of 8 bytes or less. Anything more than 8 bytes is defined by cdecl to be passed in memory as a retparam. (So MSVC isn't *just* returning POD directly and non-POD in memory; it takes the size into account as it must.)

Comment 41

[Luke Wagner [:luke]]

Comment on attachment 565261 [details] [diff] [review]
fix?

Well, it seems like the ideal solution doesn't work.  Another idea is whether throwing a calling convention specifier in their (like __cdecl) would make the issue go away.  The C/C++ split inside jsd seems weird so maybe this won't be a problem for your average embedding and we can just go with the quick fix.

Comment 42

[Luke Wagner [:luke]]

Comment on attachment 565261 [details] [diff] [review]
fix?

From IRL: sfink reminded me that (1) jsapi.h already returns jsvals by value and (2) making the 'data' private member public makes JS::Value POD enough for msvc to treat them the same.  So we settled on making the 'data' member public, but only on msvc so that, effectively, it stays private.

Comment 43

[Steve Fink [:sfink] [:s:]]

Attachment: Make jsval POD on MSVC

Comment 44

[Luke Wagner [:luke]]

Comment on attachment 566379 [details] [diff] [review]
Make jsval POD on MSVC

A JSAPI test; genius!

Comment 45

[Marco Bonardo [:mak]]

backed out bug 689101, bug 692987 and bug 692722, since one of these caused permaorange on OSX64 m-oth tests (bug 692605 went permaorange on inbound, exactly)

Comment 46

[Steve Fink [:sfink] [:s:]]

Comment on attachment 566379 [details] [diff] [review]
Make jsval POD on MSVC

re-landed just this one on inbound, since try server says it's ok (and adding both 692987 + 692722 on top reproducibly breaks m-oth, which makes no sense but whatever...)

Requesting Aurora approval: Firebug is currently broken on Aurora without this patch.

Comment 47

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/5f16d668ce5a

Comment 48

[Yann Brelière]

I believe Bug 689118 was fixed thanks to this fix.

Comment 50

[Steve Fink [:sfink] [:s:]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/021bf68f4d51

Comment 52

[Steve Fink [:sfink] [:s:]]

Test added: https://hg.mozilla.org/integration/mozilla-inbound/rev/d16ec357bd9e

Comment 53

[Matt Brubeck (:mbrubeck)]

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

Comment 54

[Paul Silaghi, QA [:pauly]]

(In reply to Steve Fink [:sfink] from comment #13)

> Try it out:
> 
> 1. Go to www.google.com
> 2. Fail to see the firebug bug icon. Huh? Flail around for a while, finally
> discovering the Firebug menu item in the View menu. Redo all these
> instructions from the start. Please ignore this step.
> 3. Press F12 to display the Firebug console.
> 4. Click on the 'Script' panel, and click on 'Enable'.
> 5. Ctrl-R to reload.
> 
> at this point, I see the Script panel fill with minified source code, and
> all the line numbers on the left are green.
> 
> 6. Click on lines 1 and 2 to set breakpoints.
> 7. Ctrl-R to reload
> 
> Now I see a yellow triangle on the line 2 breakpoint, showing me it's
> stopped there.

After this, if click again on lines 1 and 2 to remove the breakpoints and then reload, "No Javascript on this page" is displayed.
Verified on:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0) Gecko/20100101 Firefox/9.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:10.0a2) Gecko/20111128 Firefox/10.0a2
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111128 Firefox/11.0a1
What do you think?

Comment 55

[u279076]

Steve, does Paul's testing validate your the fix for this bug?

Comment 56

[Steve Fink [:sfink] [:s:]]

My opinion: yes, Paul's testing validates this fix (since the symptom in question is the complete inability to ever get debugging working at all). But it also reveals an unrelated bug, which should be filed separately. It may be a dupe, but I don't know of it. Honza might. Please file it anyway. Note that it is not Windows-specific, since I see it on Linux as well.

When I follow Paul's steps from comment 54, I see "No Javascript on this page", but also an error gets thrown. The first time, it was something like "google is not defined". Reloading again "fixes" it. Every time I am stopped at a breakpoint and reload, I get "No Javascript on this page" once, together with an error that varies from time to time. Paul, did you get an error message too?

The console output says:

JavaScript error: http://www.google.com/, line 4: google is not defined

or for one of the other errors I got using the same procedure:

JavaScript error: http://www.google.com/, line 6: google.startTick is not a function

Comment 57

[u279076]

Marking verified fixed. Paul, please file a new bug as per comment 56.

Comment 58

[Paul Silaghi, QA [:pauly]]

Yes Steve, I got that error too:
google is not defined
[a,b];return false}};(function(){var a=google.j;window.onpopstate=

New bug 713211 logged.

Comment 59

[Ginn Chen]

The patch mentioned bug 645111, is it a typo?

+  /* To make jsval binary compatible when linking across C and C++ with MSVC,
+   * JS::Value needs to be POD. Otherwise, jsval will be passed in memory
+   * in C++ but by value in C (bug 645111).
+   */

Comment 60

[Steve Fink [:sfink] [:s:]]

Yes. It looks like it should point back to here, bug 689101.