1332730 - Check TypedObject interior pointers with hazard analysis

Status: RESOLVED WONTFIX

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

Description

[Steve Fink [:sfink] [:s:]]

Cloned from bug 1296640. Comments repeated below:

(In reply to Jon Coppeard (:jonco) from comment #10)
> > But there has to be a way, using the static analysis, to detect this kind of issues more generally, right?
> 
> Yep, the hazard analysis should be able to catch this.  I think you need to
> annotate your Elem types with JS_HAZ_GC_INVALIDATED, but Steve will know
> more.

Yes, we should totally expose these to the analysis.

The usual difficulty is that you have to have a C++ type to distinguish a pointer to GC memory vs pointers to anything else. You kind of have that here, with Elem. Except that the pointer that would get invalidated is the Elem*, not the Elem.

So you'd sort of like to mark the type Elem* as either JS_HAZ_GC_INVALIDATED or JS_HAZ_GC_POINTER (depending on whether you want to consider interior pointers to be GC pointers or not; I could argue either way, but the analysis doesn't care.) But you can only annotate bare C++ types, not pointers to them or whatever. So I think you'd need to tell a little white lie and claim that Elem is a JS_HAZ_GC_THING. That will make a pointer to an Elem be considered a GC pointers that could be invalidated on GC.

But Elem is not a real type, it's a typedef to V::Elem, which is a template parameter, and V::Elem is generally something like int8_t. The compiler sees through typedefs for the most part, so this isn't going to work; we'd be declaring 'char' to be a GC type. :(

Maybe I'll see if we can make TypedObjectMemory return some sort of wrapper type instead of a plain Elem*, though I'm skeptical that we won't need it to degrade to a plain pointer immediately.

Or perhaps the big hammer -- requiring TypedObjectMemory to be given an AutoAssertOnGC&, and fixing up all callers.

This is the same sort of interior pointer pain we have with pointers to JSString contents.

Comment 1

[Steve Fink [:sfink] [:s:]]

The TypedObject implementation has been removed.