1010269 - Crash [@ js::jit::Simulator::decodeSpecialCondition]

Status: VERIFIED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Christian Holler (:decoder)]

Attachment: Testcase for shell

The attached testcase crashes on mozilla-central revision 2f8af55d6e9a (run with --fuzzing-safe --ion-eager --ion-check-range-analysis).

Comment 1

[Christian Holler (:decoder)]

Marked s-s because this seems to fail during instruction decoding and this could indicate another issue with the assembler buffer or some other corruption.

Comment 2

[Marty Rosenberg [:mjrosenb]]

Attachment: MoreAccurateToggles-r0.patch

HA-HA! Not s-s, defensive coding ftw!
the illegal instruction it executed was the pool header, which is guaranteed to not be a legal instruction, and produce a sigill before executing anything we don't control as tightly.

Comment 3

[Marty Rosenberg [:mjrosenb]]

Also, this may fix a bunch of other fuzzbugs that have been found recently.

Comment 4

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to Marty Rosenberg [:mjrosenb] from comment #3)
> Also, this may fix a bunch of other fuzzbugs that have been found recently.

If that's the case, can we add testcases to this patch since it's non-s-s?

Comment 5

[Douglas Crosher [:dougc]]

Comment on attachment 8425259 [details] [diff] [review]
MoreAccurateToggles-r0.patch

Looks good, thank you.

Comment 6

[Marty Rosenberg [:mjrosenb]]

landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/1b76d9de2612

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1b76d9de2612

Comment 8

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.

Comment 9

[Douglas Crosher [:dougc]]

Comment on attachment 8425259 [details] [diff] [review]
MoreAccurateToggles-r0.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Long standing issue.

User impact if declined: Crashes. Would like to get this uplift to 31 as it is an ESR. There are fixes and tests for related issues pending and this bug might block these.

Testing completed (on m-c, etc.): Locally, and on m-c.

Risk to taking this patch (and alternatives if risky): low.

String or IDL/UUID changes made by this patch: n/a

Comment 10

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8425259 [details] [diff] [review]
MoreAccurateToggles-r0.patch

Rejecting for aurora, already in 32.

Comment 11

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed on Fx32

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/38bcf45f4222

Is this something we might expect to improve stability on B2G v1.4 (based off Gecko 30)?

Comment 13

[Marty Rosenberg [:mjrosenb]]

It certainly can't hurt.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8425259 [details] [diff] [review]
MoreAccurateToggles-r0.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Long standing issue.
User impact if declined: Crashes. May improve v1.4 stability.
Testing completed: Landed on v2.0 with no known issues for 3 weeks now.
Risk to taking this patch (and alternatives if risky): Low.
String or UUID changes made by this patch: None.

Comment 15

[Preeti Raghunath(:Preeti)]

Comment on attachment 8425259 [details] [diff] [review]
MoreAccurateToggles-r0.patch

Nervous about taking JS fixes in 1.4

Comment 16

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to Marty Rosenberg [:mjrosenb] from comment #3)
> Also, this may fix a bunch of other fuzzbugs that have been found recently.

(In reply to Preeti Raghunath(:Preeti) from comment #15)
> Comment on attachment 8425259 [details] [diff] [review]
> MoreAccurateToggles-r0.patch
> 
> Nervous about taking JS fixes in 1.4

Are you really sure? This fix for an arguably non-s-s bug fixes a bunch of fuzzbugs found, so by not taking it, you might have more js crashes in 1.4, so please consider the risk/reward ratio carefully here.