921470 - Assertion failure: [barrier verifier] Unmarked edge: baseline-monitor-typeobject, at gc/Verifier.cpp:569

Status: VERIFIED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Christian Holler (:decoder)]

The following testcase asserts on mozilla-central revision e4cd2242cc7d (threadsafe build, run with --fuzzing-safe --ion-eager --thread-count=2 --ion-parallel-compile=on):


var lfcode = new Array();
lfcode.push("");
lfcode.push("4");
lfcode.push("\
	gczeal(4);\
	function callback() {}\
	callback({});\
	setObjectMetadataCallback(callback);\
");
while (true) {
	var file = lfcode.shift(); if (file == undefined) { break; }
        loadFile(file)
}
function loadFile(lfVarx) {
    try {
        if (lfVarx.substr(-3) != ".js" && lfVarx.length != 1) {
            switch (lfRunTypeId) {
                case 4: eval("(function() { " + lfVarx + " })();"); break;
            }
        } else if (!isNaN(lfVarx)) {
            lfRunTypeId = parseInt(lfVarx);
        }
    } catch (lfVare) {
        if (lfVare instanceof SyntaxError) {}
    }
}

Comment 1

[Christian Holler (:decoder)]

This one doesn't reproduce on every run, might need to run it a few times.

Comment 2

[Christian Holler (:decoder)]

Marking as fuzzblocker because this triggers again quite often and with a lot of signatures. Djvj, can you take a look?

Comment 3

[Christian Holler (:decoder)]

Attachment: [crash-signature] Machine-readable crash signature

Comment 4

[Kannan Vijayan [:djvj]]

Can I get the specific build command line?  I'm building a threadsafe debug build and running as you indicate (on the m-c revision you note), and I'm not able to replicate it.

Comment 5

[Kannan Vijayan [:djvj]]

Got your build and checked it out.  Still can't reproduce on my machine (tried it 200 times):

for n in $(seq 200); do LD_LIBRARY_PATH=. ./js --fuzzing-safe --ion-eager --thread-count=2 --ion-parallel-compile=on /tmp/test.js ; done


Note sure what to do at this point outside of set up VPN for remote access and debug it on your guys' machines.  Can we start setting that up?

Comment 6

[Nicolas B. Pierron [:nbp]]

Are you assigned on this bug?

Comment 7

[Kannan Vijayan [:djvj]]

Just set up VPN access to MountainView.  I can take this bug now, but I need login creds to a machine on which I can reproduce this issue.

Decoder, can you set this up?

Comment 8

[Christian Holler (:decoder)]

I finally managed to reproduce this on one of the MV machines. This test is for a non-threadsafe build (m-c rev aa986b6ce882, opt and debug worked for me, both 32 and 64 bit, no options):


var lfcode = new Array();
lfcode.push("");
lfcode.push("const libdir = \"x\";");
lfcode.push("");
lfcode.push("2");
lfcode.push("");
lfcode.push("");
lfcode.push("2");
lfcode.push("gczeal(4,10);");
lfcode.push("");
lfcode.push("0");
lfcode.push("const libdir = \"x\";");
lfcode.push("setObjectMetadataCallback(function(obj) {});");
var lfRunTypeId = -1;
while (true) {
        var file = lfcode.shift(); if (file == undefined) { break; }
        loadFile(file)
}
function loadFile(lfVarx) {
  try {
    switch (lfRunTypeId) {
       case 1: eval(lfVarx); break;
       default: evaluate(lfVarx); break;
    }
  } catch (lfVare) {
    if (lfVare instanceof SyntaxError) {}
  }
}


If this again doesn't reproduce, then I can now easily give you access to the machine the test is sitting on.

Comment 9

[Christian Holler (:decoder)]

JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/be1399f8f973
user:        Brian Hackett
date:        Thu May 30 17:37:22 2013 -0600
summary:     Bug 850026 - Allow metadata objects to be associated with JS objects, and add a hook for attaching metadata to newly created objects, r=luke.

This iteration took 2.091 seconds to run.

Comment 10

[Kannan Vijayan [:djvj]]

I'm unassigning myself from this bug because of the reproduction issues and because there area  bunch of other bugs on my plate.  keeping myself assigned to this prevents someone else who has time from picking it up and fixing it.

Comment 11

[Christian Holler (:decoder)]

Attachment: [crash-signature] Machine-readable crash signature

Comment 12

[Jan de Mooij [:jandem]]

decoder, are you still seeing this?

Comment 13

[Christian Holler (:decoder)]

Yes, all the time :) one of the most annoying asserts (it's actually a whole group of asserts)

Comment 14

[Jan de Mooij [:jandem]]

I can reproduce this with the testcase in comment 8.

Comment 15

[Jan de Mooij [:jandem]]

Attachment: Patch

The tracing code was correct when hasFallbackStub_, but did nothing when hasFallbackStub_ == false.

Comment 16

[Jan de Mooij [:jandem]]

Comment on attachment 8344724 [details] [diff] [review]
Patch

[Security approval request comment]
> How easily could an exploit be constructed based on the patch?
Not easily but it may be possible.

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

> Which older supported branches are affected by this flaw?
23+.

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

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

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

Comment 17

[Al Billings [:abillings - ex-MoCo]]

Comment on attachment 8344724 [details] [diff] [review]
Patch

sec-approval for trunk.

We'd like this on Aurora and Beta too so it gets good bake time before the release in six weeks.

Comment 18

[Al Billings [:abillings - ex-MoCo]]

ESR24 as well.

Comment 19

[Jan de Mooij [:jandem]]

Rewriting the for-loop to a while-loop isn't really necessary, so to be safe this patch just removes the bogus if-condition.

https://hg.mozilla.org/integration/mozilla-inbound/rev/d887626fd6d6

Comment 20

[Jan de Mooij [:jandem]]

Comment on attachment 8344724 [details] [diff] [review]
Patch

[Approval Request Comment]
User impact if declined: Crashes, security issues.
Fix Landed on Version: 29, will be backported to 27 and 28.
Risk to taking this patch (and alternatives if risky): Very low.
String or UUID changes made by this patch: None.

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Baseline JIT.
User impact if declined: Crashes, security issues.
Testing completed (on m-c, etc.): On m-i.
Risk to taking this patch (and alternatives if risky): Very low.
String or IDL/UUID changes made by this patch: None.

Comment 21

[Carsten Book [:Tomcat]]

landed on central https://hg.mozilla.org/mozilla-central/rev/d887626fd6d6

Comment 22

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.

Comment 23

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/86e26e292344
https://hg.mozilla.org/releases/mozilla-beta/rev/90af7e816b88

ESR24 tree is closed. Ryan, can you take care of b2g and esr24? Thanks.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

In my queue.

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/d965e4f200f4
https://hg.mozilla.org/releases/mozilla-esr24/rev/17babc2c47ec