[PATCH v6 4/8] jscript: Lookup and ref the named item's dispatch first, during interpretion.

[Jacek Caban]

Hi Gabriel,

On 09/03/2020 16:57, Gabriel Ivăncescu wrote:
> Hi Jacek,
>
> On 09/03/2020 16:05, Jacek Caban wrote:
>> Hi Gabriel,
>>
>> On 07.03.2020 14:40, Gabriel Ivăncescu wrote:
>>> diff --git a/dlls/jscript/dispex.c b/dlls/jscript/dispex.c
>>> index e986de6..a0c8f38 100644
>>> --- a/dlls/jscript/dispex.c
>>> +++ b/dlls/jscript/dispex.c
>>> @@ -1527,6 +1527,7 @@ static HRESULT WINAPI 
>>> DispatchEx_InvokeEx(IDispatchEx *iface, DISPID id, LCID lc
>>>       }
>>>       enter_script(This->ctx, &ei);
>>> +    ei.script_obj = This;
>>
>>
>> This doesn't look right. What are you trying to handle here? Note 
>> that IDispatchEx is used by regular JavaScript objects and using such 
>> objects and script_obj doesn't seem right. Even if the object is a 
>> script dispatch, I'm not sure this is right. In fact, I'm not yet 
>> convinced that we need script_obj in ei at all.
>>
>>
>> Thanks,
>>
>> Jacek
>>
>
> Oh, I was just concerned about 'eval' being called somehow from 
> IDispatch directly without going through exec_source, because the eval 
> code does seem to assume that 'frame' (i.e. ctx->call_ctx) can be 
> NULL, which would mean we can't access the bytecode's named item (this 
> check was there before my patches).
>
> I can drop it, of course, if it's not needed. Perhaps that NULL check 
> in eval is redundant, then?


NULL check in eval is still needed. I don't know what's the exact 
expected behavior in case of this, but treating it as global code does 
not seem wrong.


The way you set it will not do the right thing when used with a regular 
JS object:

o = { f: eval };

If you call o.f() by IDispatchEx, your patch would use o as a script object.


That said, I'm fine with treating such calls as global code, which 
simply means using NULL script object and not touching jsexcept_t. If 
you'd want to dig deeper, it would need more tests.


Thanks,

Jacek