Wrong gczeal check in RefillDoubleFreeList

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: Igor Bukanov, Assigned: Igor Bukanov)

Tracking

({fixed1.9.1})

Trunk
fixed1.9.1
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.1 +
in-testsuite -
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 attachment)

(Assignee)

Description

10 years ago
RefillDoubleFreeList from js/src/jsgc.cpp contains:

    if (rt->gcMallocBytes >= rt->gcMaxMallocBytes && rt->gcPoke
#ifdef JS_GC_ZEAL
        && (rt->gcZeal >= 2 || (rt->gcZeal >= 1 && rt->gcPoke))
#endif
        ) {
        goto do_gc;
    }

This is wrong as the the gc zeal check should use ||, not &&, condition. With this bug the gc zeal checks during double allocation becomes effectively disabled.

The patch for bug 474801 fixes this regression from bug 456826 due to yet unclear oranges that it triggers I file this bug separately.
(Assignee)

Comment 1

10 years ago
Created attachment 358582 [details] [diff] [review]
v1

The fix uses ||, not &&, in the gczeal check.
Attachment #358582 - Flags: review?(mrbkap)
Comment on attachment 358582 [details] [diff] [review]
v1

Sorry, I should have caught this in review.
Attachment #358582 - Flags: review?(mrbkap) → review+
(Assignee)

Comment 3

10 years ago
(In reply to comment #2)
> (From update of attachment 358582 [details] [diff] [review])
> Sorry, I should have caught this in review.

It was I who did a review for the regressed patch. So blame me, no yourself! 

Nominating for 1.9.1 as this is a regression.
Flags: blocking1.9.1?

Updated

10 years ago
Flags: blocking1.9.1? → blocking1.9.1+

Updated

10 years ago
Keywords: verified1.9.1
(Assignee)

Comment 4

10 years ago
landed to TM - http://hg.mozilla.org/tracemonkey/rev/92ddc88a2f16
Whiteboard: fixed-in-tracemonkey

Comment 5

10 years ago
http://hg.mozilla.org/mozilla-central/rev/92ddc88a2f16
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED

Updated

10 years ago
Flags: in-testsuite-
Flags: in-litmus-
You need to log in before you can comment on or make changes to this bug.