Closed Bug 851067 Opened 11 years ago Closed 11 years ago

IonMonkey: Assertion failure: isInt32(), at ./dist/include/js/Value.h:1087

Categories

(Core :: JavaScript Engine, defect)

x86
Linux
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla22
Tracking Status
firefox21 --- unaffected
firefox22 --- fixed
firefox-esr17 --- unaffected
b2g18 --- unaffected

People

(Reporter: decoder, Assigned: nbp)

References

Details

(4 keywords, Whiteboard: [jsbugmon:update,ignore][adv-main22-])

Attachments

(1 file, 1 obsolete file)

The following testcase asserts on mozilla-central revision b672877ed046 (run with --ion-eager):


function toPrinted(value) {
  value = String(value);
}
String = Array;
toPrinted(123);
evaluate('toPrinted("foo");');
Filing s-s because it seems that int and string are confused. The test doesn't crash though, so it might be that the situation is handled properly anyway.
Blocks: IonFuzz
Whiteboard: [jsbugmon:update,bisect]
Summary: IonMonkey: Assertion failure: Assertion failure: isInt32(), at ./dist/include/js/Value.h:1087 → IonMonkey: Assertion failure: isInt32(), at ./dist/include/js/Value.h:1087
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
JSBugMon: Bisection requested, failed due to error (try manually).
Trying this once more with a fix.
Whiteboard: [jsbugmon:update] → [jsbugmon:update,bisect]
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   122585:437c955ff06d
user:        Nicolas B. Pierron
date:        Wed Jan 30 07:41:01 2013 -0800
summary:     Bug 796114 - Inline with type-checked arguments. r=h4writer

This iteration took 111.518 seconds to run.
Nicolas, can you take a look based on comment 4? Thanks.
Flags: needinfo?(nicolas.b.pierron)
This test case looks similar to one that I added recently in the test suite which was also a regression on the same patch.  The previous one was a problem in the TypePolicy, where some instruction can be used with arguments which are not accounted by the type inference which put Ion in an ambiguous state.
Assignee: general → nicolas.b.pierron
Status: NEW → ASSIGNED
Flags: needinfo?(nicolas.b.pierron)
Blocks: 796114
Add a guard to prevent inlining cases.

Strangely I feel like Bug 796114 has open a new class of errors to fuzzers, because this error pre-dates Bug 796114 and this should probably be backported to all Ion branches.
Attachment #726363 - Flags: review?(hv1989)
(In reply to Nicolas B. Pierron [:nbp] from comment #7)
> Strangely I feel like Bug 796114 has open a new class of errors to fuzzers,
> because this error pre-dates Bug 796114 and this should probably be
> backported to all Ion branches.

I haven't been able to extract a test case showing an original issue, so I will just add a simplified version of the test case with the patch.
Not sure this is the right fix.

It would fix this particular issue, but I think there is a bigger error here. I think we assume all over the place that if TI says it is a MIRType_... and it is a isContstant(), that the type of the constant is MIRType_... That is not correct ?anymore? after your patch.

So from what I understand these errors are because the type we see in IonBuilder (on constant) is not present in the TI information. This can only happen during inlining. Wouldn't it make sense to disable an inline when the inputs of the function we are inlining are not in the subset of observed types of the function? I thought all these issues have a MConstant at the base. 
1) Just checking the type of the MConstant before inlining should work or
2) It would be possible to Box the MConstant when it doesn't correspond with the observed types. That way it would be a MIRType_Value.
Note: doing so also fix bug 852140, because it prevent the original input type to flow into the compare adjust type.
Attachment #726363 - Attachment is obsolete: true
Attachment #726363 - Flags: review?(hv1989)
Attachment #726814 - Flags: review?(hv1989)
Comment on attachment 726814 [details] [diff] [review]
Box non-matching argument types.

Review of attachment 726814 [details] [diff] [review]:
-----------------------------------------------------------------

With the adjustments talked on IRC.
Attachment #726814 - Flags: review?(hv1989) → review+
Whiteboard: [jsbugmon:update] → [jsbugmon:update,ignore]
JSBugMon: The testcase found in this bug no longer reproduces (tried revision 1d6fe70c79c5).
https://hg.mozilla.org/mozilla-central/rev/cf0ec8e2c809
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
Whiteboard: [jsbugmon:update,ignore] → [jsbugmon:update,ignore][adv-main22-]
Group: core-security
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: