723773 - Liveness analysis makes mistakes when analyzing try/catch

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: patch

This was basically a mistake over the meaning of some arguments to the Lifetime constructor, I think. The fix is simple.

Most of the work of writing this patch was in figuring out the meaning of the savedEnd field, so I tried to improve the comment describing it. Let me know if you have any ideas for this.

I don't understand precisely how the JIT uses this information, so I decided to be conservative and close the bug.

Comment 1

[Brian Hackett [Laid off!]]

Comment on attachment 594024 [details] [diff] [review]
patch

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

Thanks!

::: js/src/jsanalyze.cpp
@@ +1008,5 @@
> +     *
> +     * Each time we create a Lifetime, we store the start of the most recently
> +     * seen sequence of conditional code in the Lifetime's savedEnd field. So,
> +     * when creating the Lifetime at #2, we set the Lifetime's savedEnd to
> +     * #5. (The most start of the most recent conditional is cached in each

rm 'most'

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0d2ab3f2e9b9

Comment 3

[Ed Morley [:emorley]]

From philor:
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/71f5bf4df2f6 - one of the six in that push was crashing in js::gc::Mark<JSString>

Comment 4

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3d8b2998c103

Comment 5

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

https://hg.mozilla.org/mozilla-central/rev/3d8b2998c103

Comment 6

[Daniel Veditz [:dveditz]]

regression from bug 714645 which landed in Firefox 12 so we don't have to worry about ESR. Not sure if this is serious enough to warrant fixing on the beta branch.

Comment 7

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

What security rating should be attached to this issue?

Comment 8

[u279076]

Is there something QA can do to verify this fix?