In Gecko, style structs are allocated from the pres arena. I don't know if this provides any performance benefits, but it does provide the security benefit of poisoning deleted objects to prevent dangling pointer issues and type confusion with dangling pointers to deleted structs (per http://robert.ocallahan.org/2010/10/mitigating-dangling-pointer-bugs-using_15.html). We should consider doing something similar for style structs allocated by Servo so that we don't regress our security.

Another thing to consider is actual allocated memory size. jemalloc is rather coarse grained: http://jemalloc.net/jemalloc.3.html#size_classes whereas the pres shell arena allocates to the nearest 8-byte boundary for all requested sizes. Then again, it doesn't really free() anything until it dies so there's that. :-)

As discussed, I don't think the security issue here is as big as it is in the Gecko case. I think we still may want to do this for perf or memory reasons though. Profiling will tell us.

> As discussed, I don't think the security issue here is as big as it is in the Gecko case. I think we still may want to do this for perf or memory reasons though. Profiling will tell us. I think it may be important too, to some extent. Even if we use Rust in the style system, which gives us more confidence than C++, there's nothing preventing layout or DOM code holding a style context weak pointer for too long, or similar stuff.

I agree that it is possible, but I don't think it's more likely than any other UAF in Gecko. We have the presshell arena because there were a lot of programming patterns in layout (specifically lots of weak references to nsIFrames, and style contexts weakly borrowing style structs from other style contexts) that led to lots of UAFs. With Stylo, we have nsStyleContext (which is refcounted) holding a strong reference to the ServoComputedValues, which holds a strong reference to all the style structs exposed by that style context. Somebody can still grab a bare pointer and store it somewhere, but that's not unique to the style system. I'm not saying there are no benefits, just that I wouldn't block shipping because of the security considerations here.

We may want to consider doing this to improve allocation performance per bug 1360881. But I want to remeasure once we fix bug 1360882 and see how much malloc overhead remains.

Note that this would require us to fork Arc, since that handles its allocation internally. But I think we should do that anyway in bug 1360883.

(Changing the summary because I think the decision of whether or not to poison depends on a measurement of the overhead).

The other big benefit of this is it would make it easier to account for style structs in about:memory. I might poke at this a bit.

(In reply to Boris Zbarsky [:bz] (work week until 5/26) (if a patch has no decent message, automatic r-) from comment #8) > The other big benefit of this is it would make it easier to account for > style structs in about:memory. > > I might poke at this a bit. If you want to work on this, we should sync up first. I have some partial patches, and have done a fair bit of thinking about the implementation. I think it's non-trivial, and at the moment I think we have other work that's probably higher priority.

status-firefox57=wontfix unless someone thinks this bug should block 57