[PATCH 4/5] vbscript: Implement separate script dispatch objects for each named item.

Gabriel Ivăncescu gabrielopcode at gmail.com
Mon Feb 3 10:59:29 CST 2020


Hi Jacek,

On 03/02/2020 19:17, Jacek Caban wrote:
> Hi Gabriel,
> 
> On 31.01.2020 14:29, Gabriel Ivăncescu wrote:
>> diff --git a/dlls/vbscript/interp.c b/dlls/vbscript/interp.c
>> index b328674..8ded5cd 100644
>> --- a/dlls/vbscript/interp.c
>> +++ b/dlls/vbscript/interp.c
>> @@ -112,7 +112,7 @@ static BOOL lookup_global_vars(ScriptDisp *script, const WCHAR *name, ref_t *ref
>>   
>>   static HRESULT lookup_identifier(exec_ctx_t *ctx, BSTR name, vbdisp_invoke_type_t invoke_type, ref_t *ref)
>>   {
>> -    ScriptDisp *script_obj = ctx->script->script_obj;
>> +    ScriptDisp *script_obj = ctx->code->script_obj;
> 
> 
> Does it really use this script instead of global script global context? 
> My guess would be that both should be looked up in this case. A test to 
> clarify that would be nice.
> 

Yes, sorry about that, I forgot to add this test to the actual patch (I 
did fiddle manually with it though).

I double checked and added such a test, it does appear that it was 
correct: it cannot access the global context or dispatch at all.

See: https://testbot.winehq.org/JobDetails.pl?Key=64413

In fact, for jscript (which I'll send after these), it cannot even 
access the built-in identifiers! (since they are attached to the global 
object). For example, it cannot access the 'Math' object.

> 
>> +    if (!obj)
>> +    {
>> +        /* A persistent script with a dangling context calls these,
>> +           for some reason, even though it won't be executed. */
>> +        IActiveScriptSite_OnEnterScript(ctx->site);
>> +        IActiveScriptSite_OnLeaveScript(ctx->site);
>> +        return S_OK;
>> +    }
> 
> 
> Is it just to satisfy a test? I'd suggest to leave it in todo_wine for 
> OnEnterScript/OnLeaveScript. Otherwise, it would deserve a closer look 
> to see if we can solve it more generally and avoid such special case.
> 

It's mostly to satisfy the tests, yes. I didn't realize it would be an 
issue though. Note that we'd still have to check for if (!obj) and 
return S_OK, otherwise the code will crash.

> 
>>           return hres;
>>   
>> +    /* Fix the dangling pointers to the old script dispatch */
>> +    LIST_FOR_EACH_ENTRY(code, &This->ctx->code_list, vbscode_t, entry)
>> +        if(code->script_obj)
>> +            code->script_obj = This->ctx->script_obj;
> 
> 
> That's not pretty. Depending on result of test I mentioned earlier, we 
> may want to simply not store global script context in vbscode_t (and 
> only store named item-based script object).
> 

Yeah it's not pretty, but unfortunately I couldn't find a simpler way 
without adding extra fields just for this, which to me would be even worse.

Note that the "fix" has to be done for the global context, otherwise 
persistent code would be useless. For other contexts it seems they are 
completely "lost" when script is restarted (even adding a named item of 
same name will not work to gain access to them -- it's in the tests), 
and they will also do nothing *except* that they'll trigger the 
OnEnterScript/OnLeaveScript for some reason.

Obviously, this is only for persistent code.

> Thanks,
> 
> Jacek
> 




More information about the wine-devel mailing list