Closed Bug 1113641 Opened 7 years ago Closed 7 years ago

Enable debug build run on non-4k page boxes

Categories

(Core :: JavaScript: GC, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla37

People

(Reporter: stransky, Assigned: stransky)

Details

Attachments

(1 file, 1 obsolete file)

On boxes which does not have 4k pages debug build of js engine fails on bogus assertion in MarkPagesInUse().
Attached patch debug-gc.patch (obsolete) — Splinter Review
Not sure who is a good reviewer for this so please reassing if I'm wrong. Thanks!
Attachment #8539256 - Flags: review?(igor)
Attachment #8539256 - Flags: review?(igor) → review?(terrence)
I don't think that needs to be wrapped in #ifdef DEBUG - it looks like that call to DecommitEnabled() should've been there all along.
Comment on attachment 8539256 [details] [diff] [review]
debug-gc.patch

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

Thank you for the fix and sorry for having broken it in the first place!

::: js/src/gc/Memory.cpp
@@ +663,5 @@
>  {
> +#ifdef DEBUG
> +    if (!DecommitEnabled())
> +        return false;
> +#endif

As Emanuel said, this is a straight-up logic error on our part; please remove the #ifdef DEBUG and add these two lines to all of the MarkPagesInUse in Memory.cpp.

Secondly, I think this should be |return true|, as the call "succeeded": it is safe to touch p after the return.
Attachment #8539256 - Flags: review?(terrence) → review+
Thanks, there's the updated one. Try build: https://tbpl.mozilla.org/?tree=Try&rev=3f31bb693dcb
Attachment #8539256 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/aa17209aefd2
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
You need to log in before you can comment on or make changes to this bug.