Open
Bug 877537
Opened 11 years ago
Updated 2 years ago
Possible hole in rooting analysis with Value[] on stack
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
NEW
People
(Reporter: bzbarsky, Unassigned, NeedInfo)
References
(Blocks 1 open bug)
Details
Consider this code from TestShellParent.cpp:
103 JS::Value argv[] = { STRING_TO_JSVAL(str) };
104 unsigned argc = ArrayLength(argv);
105
106 JS::Value rval;
107 JSBool ok = JS_CallFunctionValue(mCx, global, mCallback, argc, argv, &rval);
The analysis warns about rval, but says nothing about argv....
Assignee | ||
Updated•10 years ago
|
Assignee: general → nobody
Reporter | ||
Comment 1•9 years ago
|
||
Steve, is this likely to still be a problem?
Flags: needinfo?(sphink)
Comment 2•9 years ago
|
||
Hm, why don't I remember this bug?
Is there further code below that? I don't see anything like that in the current TestShellParent.cpp. And the above snippet doesn't have a hazard -- while argv *is* directly on the static, and the analysis knows about that, its live range dies at the function entry.
Of course, if within the function call argv[0] gets relocated, you have a hazard. But that function call just sees a pointer to a pointer (well, a pointer to a Value, here). And there are a lot of places that pass around pointers to GC pointers. Most of them are safe -- usually because they're pointers to rooted memory anyway. So we have a separate report on anything that takes the address of an unrooted location. It has too many false positives to be used as a stick to keep things safe, but we do check it periodically. But it should have listed the above.
Unless there *was* some code after the above, code that used argv[0]?
Flags: needinfo?(sphink)
Reporter | ||
Comment 3•9 years ago
|
||
> I don't see anything like that in the current TestShellParent.cpp.
Well, sure. I changed argv to use a .address() on a Rooted when I filed this bug, and then it got changed to a HandleValueArray at some point. So the hazard in this code is long-gone no matterwhat.
The code as of when I filed the bug looked like http://hg.mozilla.org/mozilla-central/file/ccddc9279bae/ipc/testshell/TestShellParent.cpp#l103
There was no useful code after the JS_CallFunctionValue here. Both rval and argv were unused after that call.
What I was worried about when I filed this bug (as best as I can recall; it's been 2+ years) was that the "&rval" was being reported as an unsafe reference (because it takes the address of an unrooted location). But at least at the time "argv" was not being reported as an unsafe reference. That's what the bug was about.
Comment 4•9 years ago
|
||
Ah! Ok. I'll write a test case for that, then, and see what it reports. Thanks!
Flags: needinfo?(sphink)
Comment 5•9 years ago
|
||
(Note that this is probably already fixed. For a long time the analysis didn't understand arrays on the stack.)
Flags: needinfo?(sphink)
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•