hookresult (in jsd_step.c:_callHook) is never initialised to a default value, and if JS_GetFrameScript returns NULL, then this uninited value will be returned. valgrind is picking this up in js_invoke, since the uninitialised return value is used as a condition for the jsd_FunctionCallHook value, which ends up as hookData var in line 826 to be uninitialised. I have no idea whether or not the default val should be JS_TRUE or JS_FALSE, though. I also noticed that the JSD_IS_PROFILE_ENABLED branch in _callHook seems to ignore the return value from jsd_CallCallHook, while the non-profile branch doesn't. I don't know if this is intentional or not.
The default should be JS_FALSE. JS_FALSE means that we don't want to be called again when we |return| from this stack frame. If we have profiling enabled, we always want to be called for |return| (regardless of what our hook says), so that we can mark the call duration.
Status: NEW → ASSIGNED
If profiling isn't enabled, and we don't have a hook, why is hookresult set to JS_TRUE? Also, if profiling is enabled, but pdata is null, do you still want JS_TRUE to be returned? I'm guessing no, unless a script can somehow add profile data to itsself during this call (in which case you'll be missing the start info anyway, so...
We return JS_TRUE in that case, because a hook might be created *during* the call. If we say "don't call me on function return", then that newly added hook has no chance to see the return. This is a specific problem with venkman, where "step out" registers a call hook, and expects to be called for the return of the function currently being debugged.
Created attachment 96934 [details] [diff] [review] try this OK, try this. Profiling a reload of www.google.com appears to work both before and after this patch. Is the comment I added correct, then?
This doesn't address comment 13 at all. We CAN'T return false by default. If we return false on call, and the debugger stops IN THE CALL, and the user selects "Step Out", we need to hear about the function return. If we returned false on the call, we'd never hear the return. We should only return false if the call hook says to.
Component is obsolete so resolving bugs as INCOMPLETE
Status: ASSIGNED → RESOLVED
Last Resolved: a month ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.