Closed Bug 868307 Opened 12 years ago Closed 9 years ago

static rooting analysis: const references are treated as pointers

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED INVALID

People

(Reporter: sfink, Unassigned)

References

(Blocks 1 open bug)

Details

Example hazard: Function 'uint8 mozilla::dom::MouseEventInit::Init(JSContext*, JS::Value*)' has unrooted 'val' of type 'JS::Value' live across GC call 'uint8 mozilla::dom::MouseEventInit::InitIds(JSContext*)' at dom/bindings/MouseEventBinding.cpp:76 The actual method signature is: bool MouseEventInit::Init(JSContext* cx, const JS::Value& val); Because the analysis seems to think of 'val' as a pointer, it does not report when it is held live across GC calls. This particular instance is complicated by a different bug: there's a shadowing declaration of a 'JS::Value val' within the function, and the above hazard is reported for a gc call involving the wrong (outer) 'val'. But there are many, many instances of the original bug -- see obj/dom/bindings/ArchiveReaderBinding.cpp's ArchiveReaderOptions::Init(), for example. No hazards involving its 'val' parameter are reported.
Assignee: general → nobody
For the record: the Value *is* passed as a pointer, and so there is no actual hazard here. Still, it does seem like a const Value& still shouldn't be held live across a GC, since it makes a lie of the const-ness. Theoretically, at least, it seems like we're depending on undefined behavior with these.
Blocks: hazard
Is this really undefined behavior? I thought that with C++ if you pass a const T* to a function then the function is promising not to modify it *through that pointer* but that it could still be modified through another non-const T* somewhere. That is what is happening with properly rooted values anyhow --- there is some pointer to the stack allocated Value which is accessible to the GC via the root lists on the context or whatever, and which the GC is free to use to modify the contents of the Value.
I think you're right. I was thinking of a const Value& param as meaning that the function wouldn't modify that pointer when in fact it does by calling GC which changes it. But you're right, this seems totally valid: int x = 7; foo(x, x); int foo(const int& immut, int& mut) { mut = 8; return immut; } print(foo); // Should print 8 I guess mentally reading that as "immut holds a reference to a constant integer" isn't quite right. The question remains, though -- would it be worthwhile to have the rooting analysis complain about holding a const Value& live across a GC? Or would that net way too many false positives? I'm thinking our existing "taking an unsafe reference to an unrooted value" is probably the right thing here, and nothing more is needed.
Dropping this. If anything, it's "high hanging fruit".
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.