Closed Bug 939206 Opened 6 years ago Closed 6 years ago

GenerationalGC: Postbarriers are broken for nsTArrays of JS::Values

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: jonco, Assigned: jonco)

References

Details

(Whiteboard: [qa-])

Attachments

(2 files)

Back in bug 877762 I added machinery to enable postbarriering of nsTArrays of JS::Values, by using nsTArray<JS::Heap<JS::Value>>.

One issue was that nsTArray could memcpy its contents around, which would circumvent our overloaded assignment operator in JS::Heap<T>.  So a change was made to call constructors/destructors when copying if the elements of the array were of type JS::Heap<T>.

Unfortunately I got it slightly wrong, to the effect that although JS::Value constructors were called, the JS::Heap<T> constructors were not, so barriering didn't actually happen.

This is the fix for that.
Attachment #833034 - Flags: review?(benjamin)
Comment on attachment 833034 [details] [diff] [review]
fix-nstarray-barriering

This really really needs some unit tests to ensure that we're doing the right thing. The gtest framework should make this possible.
Attachment #833034 - Flags: review?(benjamin) → review+
Test code to check postbarriers for nsTArray<JS::Heap<T> > arrays.
Attachment #8343130 - Flags: review?(sphink)
Depends on: 946732
Comment on attachment 8343130 [details] [diff] [review]
nstarray-barrier-tests

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

This is awesome!
Attachment #8343130 - Flags: review?(sphink) → review+
The test code cause a build failure for B2G builds, so I pushed the following fix:

https://hg.mozilla.org/integration/mozilla-inbound/rev/496daf656806
https://hg.mozilla.org/mozilla-central/rev/2c149457eb67
https://hg.mozilla.org/mozilla-central/rev/496daf656806
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.