Closed Bug 611462 Opened 14 years ago Closed 14 years ago

Abort at startup, after installing Adblock Plus in debug build: "Assertion failure: OptionsHasXML(options) == VersionHasXML(version), at ../../../mozilla/js/src/jsapi.cpp:1027"

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: dholbert, Assigned: cdleary)

References

()

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(2 files, 1 obsolete file)

STEPS TO REPRODUCE:
 1. Create a debug mozilla-central build.
 2. Visit https://addons.mozilla.org/en-US/firefox/addon/1865/ and click "Add to Firefox"
 3. Click through the dialogs, and restart when prompted.

ACTUAL RESULTS:
Immediate abort with:
> Assertion failure: OptionsHasXML(options) == VersionHasXML(version), at ../../../mozilla/js/src/jsapi.cpp:1027

NOTE: This affects both my libxul and my non-libxul build. (not surprising)

I'll post a backtrace in a minute.
Attached patch backtraceSplinter Review
blocking2.0: --- → ?
We clearly don't have very good version-changing test coverage. I'll try to characterize this failure tonight -- should build up a small suite of version tests while I'm at it.
Assignee: general → cdleary
Status: NEW → ASSIGNED
Attached patch WIP: fix bad version behavior (obsolete) — Splinter Review
The assertion failed because we were copying the flags from the former version into the new version, instead of from the current options to the new version. When you set options from the API, you're indicating what compilation options you'd like for *future* scripts, so it may be different from the currently executing script version's flags.

Need to add tests.
The test demonstrates the failure mode and the patch resolves it.
Attachment #490193 - Attachment is obsolete: true
Attachment #490336 - Flags: review?(lw)
Comment on attachment 490336 [details] [diff] [review]
Fix bad JS_SetVersion behavior.

r+, only nits:

>-    void clearVersionOverride() { hasVersionOverride = false; }
>+    void clearVersionOverride() {
>+        hasVersionOverride = false;
>+    }
>+    
>     bool isVersionOverridden() const { return hasVersionOverride; }
> 
>     /* Set the default script compilation version. */
>-    void setDefaultVersion(JSVersion version) { defaultVersion = version; }
>+    void setDefaultVersion(JSVersion version) {
>+        defaultVersion = version;
>+    }

If you are going to make clearVersionOverride and setDefaultVersion not all on one line, could you do the same for isVersionOverridden?

>+    void optionFlagsToVersion(JSVersion *version) {
>+        js::VersionSetXML(version, js::OptionsHasXML(options));
>+    }

Could you make this a const memfun?
Attachment #490336 - Flags: review?(lw) → review+
Nits addressed! http://hg.mozilla.org/tracemonkey/rev/9420a20e5e5b
Whiteboard: fixed-in-tracemonkey
http://hg.mozilla.org/mozilla-central/rev/9420a20e5e5b
Status: ASSIGNED → RESOLVED
blocking2.0: ? → betaN+
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: