Closed Bug 1485547 Opened Last year Closed 10 months ago

SlotsEdge::trace does not ever trace nursery values

Categories

(Core :: JavaScript: GC, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox63 --- wontfix
firefox64 --- wontfix
firefox65 --- fixed

People

(Reporter: pbone, Assigned: allstars.chh, Mentored)

Details

(Keywords: good-first-bug)

Attachments

(1 file)

I noticed that StoreBuffer::SlotsEdge::trace() checks if a value is in the nursery and if it is it returns without doing anything.  This should never happen since we would never add anything to the store buffers (a remembered set for the GC) that is in the nursery.  If we ever do then that itself is probably a bug.   Therefore this code:

https://searchfox.org/mozilla-central/source/js/src/gc/Marking.cpp#2741

Should be replaced with a MOZ_ASSERT to assert that the value is Never in the nursery.  Such an assertion is only checked in debug builds and therefore will mean that release builds are doing less work since they will no-longer have this check.

This is a good first bug.  After making the change you should check for similar problems in the other SlotsBuffer::*Edge::trace() methods.

You should build the shell with --enable-optimize and --enable-debug and run the jit test suite.  Also run some more complete tests (whole browser) via the try server.
Priority: -- → P3
Assignee: nobody → allstars.chh
Status: NEW → ASSIGNED
Attached patch PatchSplinter Review
Attachment #9027213 - Flags: review?(jcoppeard)
Comment on attachment 9027213 [details] [diff] [review]
Patch

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

Great, thanks for fixing.
Attachment #9027213 - Flags: review?(jcoppeard) → review+
Pushed by allstars.chh@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/606abc9b203e
Assert object in SlotsEdge::trace won't be in nursery. r=jonco
https://hg.mozilla.org/mozilla-central/rev/606abc9b203e
Status: ASSIGNED → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
You need to log in before you can comment on or make changes to this bug.