Last Comment Bug 723773 - Liveness analysis makes mistakes when analyzing try/catch
: Liveness analysis makes mistakes when analyzing try/catch
Status: RESOLVED FIXED
[advisory-tracking+]
: regression
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla13
Assigned To: Bill McCloskey (:billm)
:
Mentors:
Depends on:
Blocks: 714645
  Show dependency treegraph
 
Reported: 2012-02-02 17:12 PST by Bill McCloskey (:billm)
Modified: 2012-07-11 20:00 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
affected
fixed
unaffected


Attachments
patch (4.02 KB, patch)
2012-02-02 17:12 PST, Bill McCloskey (:billm)
bhackett1024: review+
Details | Diff | Review

Description Bill McCloskey (:billm) 2012-02-02 17:12:52 PST
Created attachment 594024 [details] [diff] [review]
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 (:bhackett) 2012-02-02 17:26:02 PST
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 3 Ed Morley [:emorley] 2012-02-10 19:56:31 PST
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 5 Bill McCloskey (:billm) 2012-02-13 09:34:28 PST
https://hg.mozilla.org/mozilla-central/rev/3d8b2998c103
Comment 6 Daniel Veditz [:dveditz] 2012-03-17 15:39:16 PDT
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] 2012-05-18 17:08:05 PDT
What security rating should be attached to this issue?
Comment 8 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-05-29 11:30:23 PDT
Is there something QA can do to verify this fix?

Note You need to log in before you can comment on or make changes to this bug.