GenerationalGC: Store buffer entries are not always removed on relocation

RESOLVED FIXED in mozilla28

Status

()

Core
JavaScript Engine
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jonco, Assigned: jonco)

Tracking

unspecified
mozilla28
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
The destructor of a RelocatablePtr/Value calls relocate() to remove the associated store buffer entry.  However, that will only remove the store buffer entry if the current pointer points into the nursery.

So in the following situation, the entry is not removed:
 - create relocatable pointer pointing into nursery (store buffer entry added)
 - assign pointer to tenured thing to (store buffer entry not added)
 - destroy relocatable pointer (store buffer entry not removed)

This is bad because then we can access the pointer after the memory containing it has been freed.

The solution is to make relocate() always remove the store buffer entry.
(Assignee)

Comment 1

4 years ago
Created attachment 8336450 [details] [diff] [review]
always-relocate

Here's a patch to make relocation always remove the store buffer entry.

On the way I added some test code and fixed another bug where assigning nullptr to a RelocatablePtr caused a crash.
Attachment #8336450 - Flags: review?(terrence)
Comment on attachment 8336450 [details] [diff] [review]
always-relocate

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

This is excellent! If I'd been smart, I would have written that test a year ago.

> JSObject *badObject = reinterpret_cast<JSObject*>(1);
> JSObject *punnedPtr = nullptr;
> RelocatablePtrObject* relocPtr = reinterpret_cast<RelocatablePtrObject*>(&punnedPtr);
> new (relocPtr) RelocatablePtrObject;
> *relocPtr = NurseryObject();
> relocPtr->~RelocatablePtrObject();
> punnedPtr = badObject;
> JS_GC(cx->runtime());

This code snippet should be part of every CS student's final exam; not so much that they know what it does, but so they know what will be done to them.

::: js/src/jsapi-tests/testGCStoreBufferRemoval.cpp
@@ +3,5 @@
> +*/
> +/* This Source Code Form is subject to the terms of the Mozilla Public
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +

Extra \n.
Attachment #8336450 - Flags: review?(terrence) → review+
(Assignee)

Comment 3

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/4de097a9f7a6
https://hg.mozilla.org/mozilla-central/rev/4de097a9f7a6
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
You need to log in before you can comment on or make changes to this bug.