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

[Gabriel Ivăncescu]

On 06/05/2022 02:56, Jacek Caban wrote:
> On 5/6/22 01:50, Jacek Caban wrote:
>>> 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. 
> 
> 
> I attached a wrong test, here is the right one.
> 
> 
> Jacek
> 

Because without handling detached scopes we actually *trigger the 
assertion* so it's quite bad. But that's just for toString, it's not 
really important. I've added a test with hasOwnProperty now, which 
*must* fail on detached scopes, but it doesn't currently (since they're 
js objects). They need special handling either way.

The attached test looks to me like a different issue: it passes null 
instead of undefined, which is an ES5 specific thing, and probably 
deserves a separate fix. It's not much of a problem either way.

Note that we are *already* handling detached scopes in exec_source, so 
it's not like I'm adding custom code to deal with that separately; we 
already do. I'm just moving it out of exec_source because:

1) Nothing in the (specified) spec says it's only for source functions.
2) This will become a problem with proxy functions later (which is where 
it's needed); I'd rather not duplicate that logic in them for no reason.
3) We are already handling it in exec_source "as a special case".

I'm trying to find fix for the attach problem and send it separately but 
honestly I can't see how such a fix would fix detached scopes at all.