Closed Bug 808023 Opened 12 years ago Closed 12 years ago

Crash [@ js::EncapsulatedPtr]

Categories

(Core :: JavaScript Engine, defect)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED DUPLICATE of bug 807047

People

(Reporter: gkw, Unassigned)

References

Details

(4 keywords, Whiteboard: [sg:dupe 807047][jsbugmon:])

Attachments

(1 file)

Attached file stack
t = ""
function f(code) {
  eval("(function(){" + code + "})")()
}
evalcx("")
f("var r=({a:1})[\"\"];t(r)")

crashes js debug shell on m-c changeset 556b9cfb269f with --ion-eager at js::EncapsulatedPtr
During the course of reduction, I ran into bug 807047 and I confirm that the patch there fixes the issue.
Flags: in-testsuite?
Flags: needinfo?(nicolas.b.pierron)
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:   111708:4a2c17905a17
user:        Nicolas B. Pierron
date:        Mon Oct 29 14:48:45 2012 -0700
summary:     Bug 792631 - Add IC for missing properties. r=dvander

This iteration took 112.973 seconds to run.
(In reply to Gary Kwong [:gkw, :nth10sd] from comment #1)
> During the course of reduction, I ran into bug 807047 and I confirm that the
> patch there fixes the issue.

(needinfo added for testcase to be landed together with the patch)
Blocks: 792631
This is the same error as in Bug 807047, meaning that JSOP_GETELEM access a missing property name, and it is assumed as being idempotent.  This add a Undefined type to "r" which is then read by IonMonkey to make a call to "t".  "t" isn't a function, it causes a bailout which recover "r" without monitoring it.

js::types::TypeFailure (cx=0x101215bb0, fmt=0x1006d901a "Missing type pushed %u: %s")

One thing I am not sure is how this can have corrupted the typeset.
bhackett, any idea ?
Flags: needinfo?(nicolas.b.pierron) → needinfo?(bhackett1024)
If it's a type failure it's likely s-s - assuming sec-critical worse-case till otherwise shown.
Group: core-security
Keywords: sec-critical
Whiteboard: [jsbugmon:update] → [jsbugmon:update,ignore]
JSBugMon: The testcase found in this bug no longer reproduces (tried revision f4aeed115e54).
(In reply to Nicolas B. Pierron [:pierron] [:nbp] from comment #4)
> This is the same error as in Bug 807047, meaning that JSOP_GETELEM access a
> missing property name, and it is assumed as being idempotent.  This add a
> Undefined type to "r" which is then read by IonMonkey to make a call to "t".
> "t" isn't a function, it causes a bailout which recover "r" without
> monitoring it.
> 
> js::types::TypeFailure (cx=0x101215bb0, fmt=0x1006d901a "Missing type pushed
> %u: %s")
> 
> One thing I am not sure is how this can have corrupted the typeset.
> bhackett, any idea ?

Nicolas, probably this was fixed by bug 807047 ?
Whiteboard: [jsbugmon:update,ignore] → [jsbugmon:bisectfix]
Whiteboard: [jsbugmon:bisectfix] → [jsbugmon:]
JSBugMon: Fix Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first good revision is:
changeset:   112365:197b182baf4f
user:        Nicolas B. Pierron
date:        Mon Nov 05 16:40:41 2012 -0800
summary:     Bug 807047 - Only use missing property cache on non-idempotent IC. r=jandem

This iteration took 107.181 seconds to run.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → DUPLICATE
(In reply to Gary Kwong [:gkw, :nth10sd] from comment #8)
> (In reply to Nicolas B. Pierron [:pierron] [:nbp] from comment #4)
> > This is the same error as in Bug 807047, meaning that JSOP_GETELEM access a
> > missing property name, and it is assumed as being idempotent.  This add a
> > Undefined type to "r" which is then read by IonMonkey to make a call to "t".
> > "t" isn't a function, it causes a bailout which recover "r" without
> > monitoring it.
> > 
> > js::types::TypeFailure (cx=0x101215bb0, fmt=0x1006d901a "Missing type pushed
> > %u: %s")
> > 
> > One thing I am not sure is how this can have corrupted the typeset.
> > bhackett, any idea ?
> 
> Nicolas, probably this was fixed by bug 807047 ?

The patch fix this bug, but I don't understand how a lack of information can produce a wrong pointer.
Whiteboard: [jsbugmon:] → [sg:dupe 807047][jsbugmon:]
(In reply to Nicolas B. Pierron [:pierron] [:nbp] from comment #4)
> This is the same error as in Bug 807047, meaning that JSOP_GETELEM access a
> missing property name, and it is assumed as being idempotent.  This add a
> Undefined type to "r" which is then read by IonMonkey to make a call to "t".
> "t" isn't a function, it causes a bailout which recover "r" without
> monitoring it.
> 
> js::types::TypeFailure (cx=0x101215bb0, fmt=0x1006d901a "Missing type pushed
> %u: %s")
> 
> One thing I am not sure is how this can have corrupted the typeset.
> bhackett, any idea ?

What is the corruption you are wondering about?  The diagnosis/patch in bug 807047 looks correct, if undefined values are generated for a missing property access and the type sets are not updated properly due to GVN/LICM then the type info is incorrect and you'll get these TypeFailures.
Flags: needinfo?(bhackett1024)
(In reply to Brian Hackett (:bhackett) from comment #12)
> (In reply to Nicolas B. Pierron [:pierron] [:nbp] from comment #4)
> > This is the same error as in Bug 807047, meaning that JSOP_GETELEM access a
> > missing property name, and it is assumed as being idempotent.  This add a
> > Undefined type to "r" which is then read by IonMonkey to make a call to "t".
> > "t" isn't a function, it causes a bailout which recover "r" without
> > monitoring it.
> > 
> > js::types::TypeFailure (cx=0x101215bb0, fmt=0x1006d901a "Missing type pushed
> > %u: %s")
> > 
> > One thing I am not sure is how this can have corrupted the typeset.
> > bhackett, any idea ?
> 
> What is the corruption you are wondering about?  The diagnosis/patch in bug
> 807047 looks correct, if undefined values are generated for a missing
> property access and the type sets are not updated properly due to GVN/LICM
> then the type info is incorrect and you'll get these TypeFailures.

What I don't understand is how this invalid state is translated to an invalid pointer.  My point is that the shell should be robust against such kind of incoherent monitored types, and reading a bad pointer “because we are not monitoring” a value sounds crazy to me.

I feel like our engine miss a mechanism which disable TI and invalidate all scripts as soon as an incoherent state is detected instead of crashing. The JS shell should be more robust against bad type information, such as TI bugs are not always s-s bugs.

A first step would be to raise an assertion instead of crashing in the debug shell with test case reported on this bug.
Invalid heap state is generally going to show up because of something that jitcode does due to having incorrect type information.  Testing for correct type information in jitcode would wipe out any gains to be had from using that type information in the first place, and trying to test for coherent (not corrupted) state within the rest of the VM is generally impossible.
Group: core-security
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: