[PATCH 1/2] jscript: Handle detached scope objects 'this' in Object.toString.

[Jacek Caban]

On 5/5/22 19:01, Gabriel Ivăncescu wrote:
> On 05/05/2022 18:18, Jacek Caban wrote:
>> Hi Gabriel,
>>
>> On 5/4/22 18:10, Gabriel Ivăncescu wrote:
>>> diff --git a/dlls/jscript/object.c b/dlls/jscript/object.c
>>> index a675e45..453ea95 100644
>>> --- a/dlls/jscript/object.c
>>> +++ b/dlls/jscript/object.c
>>> @@ -73,8 +73,9 @@ static HRESULT Object_toString(script_ctx_t *ctx, 
>>> jsval_t vthis, WORD flags, uns
>>>           str = L"[object Object]";
>>>       }else if(names[jsdisp->builtin_info->class]) {
>>>           str = names[jsdisp->builtin_info->class];
>>> +    }else if(jsdisp->builtin_info->class == JSCLASS_NONE) {
>>> +        hres = JS_E_OBJECT_EXPECTED;
>>>       }else {
>>> -        assert(jsdisp->builtin_info->class != JSCLASS_NONE);
>>>           FIXME("jsdisp->builtin_info->class = %d\n", 
>>> jsdisp->builtin_info->class);
>>>           hres = E_FAIL;
>>>       }
>>
>>
>> It seems like something is still not working as intended and I'm not 
>> sure if it's the right place to fix it. Why would it be specific to 
>> detached scopes? I was wondering why you need so complicated test and 
>> tried the attached one. It works fine on Windows, but fails with your 
>> patch.
>>
>>
>> Thanks,
>>
>> Jacek
>
> Interesting, I need to investigate it more, but I'm pretty sure that 
> detached scopes are not supposed to be actual objects ever exposed to 
> jscript code, which is why it's the way it is.


Yes, that's why it would be interesting to understand the root of the 
problem.


> Honestly, I don't particularly care about toString, it's just the only 
> builtin I could think off the top of my head that could be tested for 
> this. Because right now we handle it in exec_source, which is only for 
> interpreted functions... but it's a good test case of such behavior.
>
> Do you think a fix like in 2/2 is more appropriate (also for quirks 
> modes, but with null instead of undefined)? And perhaps fix whatever 
> is causing toString to fail, which seems unrelated I guess.
>
> That might mean moving it out of exec_source though. 


Same story there, why do you think it's somehow related to detached 
scopes? The attached test fails with your patch.


Thanks,

Jacek
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch.diff
Type: text/x-patch
Size: 699 bytes
Desc: not available
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20220506/ed6d1a12/attachment.bin>