Closed Bug 1125770 Opened 6 years ago Closed 6 years ago
Backtracking started crashing on kraken-stanford-crypto-aes
http://www.arewefastyet.com/#machine=28&view=single&suite=kraken&start=1421927078&end=1422097812 Arewefastyet doesn't report numbers on "backtracking" anymore since a subtest started to fail/crash. Tested manually this is test "kraken-stanford-crypto-aes" which segfaults. On the given graph we have two times it started crashing: First regression range: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=7cea69231f77&tochange=920ae6ac243b 6d56dfa4e845 Brian Hackett -- Bug 934502 - Remove unnecessary pushedArgumentSlots, track argument slots explicitly in safepoints, r=jandem. Range that fixed it again: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=5cd5233505fc&tochange=bd9169b924a1 a92e6bed098d Ryan VanderMeulen -- Backed out changeset 6d56dfa4e845 (bug 934502) for SM(ggc) failures. Second regression range: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=ef538bd60b1c&tochange=92582779bc79 629c8aac3ece Brian Hackett — Bug 934502 - Remove unnecessary pushedArgumentSlots, track argument slots explicitly in safepoints, r=jandem. 4d669523c7dd Brian Hackett -- Bug 1123011 - Box 'this' values when eval'ing strict scripts from a non-strict Ion script, r=jandem. The revision are just educated guesses. I didn't bisect, but it seems to make sense.
This is a calling conventions issue. When an Ion frame calls another one and is constructing, after the callee finishes executing the caller will reload the value it pushed for |this| if the callee returned a primitive. Bug 934502 changed JIT frame tracing so that the |this| slot is no longer required to hold a value, and might not be marked by a GC within the callee if it is not live there. The attached patch restores the earlier convention of always marking the |this| value as part of the frame, so that the caller can always assume it is live (and holds a valid |this| value for the frame). I added some logic to the backtracking allocator so that it can't group vregs spilling to the |this| slot with other vregs that don't; this will prevent the allocator from spilling those other vregs to the |this| slot.
Assignee: nobody → bhackett1024
Attachment #8554644 - Flags: review?(jdemooij)
Attachment #8554644 - Flags: review?(jdemooij) → review+
sorry had to back this out for bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=5959773&repo=mozilla-inbound
The first patch had a bug where |this| values for non-function Ion frames were not being marked. Fixing this also fixes bug 1125840.
Attachment #8555345 - Flags: review?(jdemooij) → review+
Thanks. Arewefastyet reports numbers again for kraken-standford-crypto-aes (and shumway, which was also affected).
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.