Invalidate IsInstInlineCache in Object.setPrototypeOf

[ShortDevelopment]

Using Object.setPrototypeOf may result in instanceof returning the wrong result, as the IsInstInlineCache is not invalidated (See #5915).

Steps:

• Iterate over the existing function -> IsInstInlineCache[] dictionary using the new ThreadContext.MapIsInstInlineCaches function
• Walk the prototype chain to check for the cached type
• Ignore if type is null
  

  ChakraCore/lib/Runtime/Library/JavascriptFunction.cpp

  Lines 3365 to 3367 in 0cfe82d

  	inlineCache->Cache(VarIs<RecyclableObject>(instance) ?
  	UnsafeVarTo<RecyclableObject>(instance)->GetType() : nullptr,
  	function, scriptContext->GetLibrary()->GetFalse(), scriptContext);

• If found, remove from list using ThreadContext.UnregisterIsInstInlineCache (corrects next) and ...
• Invalidate using memset

---

I couldn't come up with a scenario where the function could be affected (x instance of function).
Therefore, the following code should not be necessary (not included in latest commit):

ChakraCore/lib/Runtime/Library/JavascriptObject.cpp

Lines 255 to 272 in 2d7e459

	JavascriptFunction* jsFunction = VarTo<JavascriptFunction>(function);
	// Check if cached function type is same as the old prototype
	bool clearCurrentCacheList = jsFunction->GetType() == object->GetType();
	if (!clearCurrentCacheList)
	{
	// Check if function prototype contains old prototype
	JavascriptOperators::MapObjectAndPrototypes<true>(jsFunction->GetPrototype(), [&](RecyclableObject* obj)
	{
	if (object->GetType() == obj->GetType())
	clearCurrentCacheList = true;
	});
	}
	if (clearCurrentCacheList)
	{
	threadContext->InvalidateIsInstInlineCachesForFunction(function);
	return;
	}

---

@rhuanjl

Fixes #5915

[rhuanjl]

Thank you for the submission, I'm going to have think a bit more about this (probably on monday)

This certainly fixes the specific case, but I'd like to throw some more complex cases at it AND also see if we can test whether it's removing caches it shouldn't (which will be slightly harder to test).

[ShortDevelopment]

Yes, that's a good idea!
I already tried to figure that out, but I couldn't really figure out, what class is behind a Type in CC (e.g. reading constructor name).

The logic defenently doesn't remove everything all the time.

[rhuanjl]

Sorry that I'm not done with this yet, I've been struggling to think of how to do the additional tests I'd like to do, there is no obvious good way to verify cache hits, so, awkwardly we may need to add one (for debug builds only) in order to test this sufficiently.

[ShortDevelopment]

No problem, I’m struggling with that as well.
What exactly do you mean by “verify cache hits”? How could we do that?

[rhuanjl]

No problem, I’m struggling with that as well. What exactly do you mean by “verify cache hits”? How could we do that?

If you want to try and do it:

1. Add a phase for instanceofCache in here: https://github.com/chakra-core/ChakraCore/blob/master/lib/Common/ConfigFlagsList.h
2. In JavascriptFunction::HasInstance (in JavascriptFunction.cpp) add some code inside a #ifdef ENABLE_TEST_HOOKS that contains an if(PHASE_TRACE1(phase_you_created)) containing code to print/log the result of the cache check for instanceof.
3. With a debug or test build of ch the command line flag -trace:phase_you_created will then trigger that logging code
4. We could then create a test case using a baseline file to enable us to check that the right things are logged, specifically we'd want cases where some cache should be cleared but some should not be and check it works.

[ShortDevelopment]

Sorry, that it took me so long, but I still had some work to do.
I’ve now replaced the test with a baseline test using the trace output from the previous commit.
It would be great if you could check if I missed an interesting case.

The Problem is, that jit abstracts the calls to the test functions away.
Therefore, the baseline is "incorrect" in those tests.

EDIT: I'm now preventing the fast path in jit, kind of like it's done in some other scenarios for testing.

[rhuanjl]

I'm not ignoring this - thank you for your efforts, I've still got some more cases I'd like to check and out of time for today will look a bit further when I can.

[rhuanjl]

Apologies for the massive delay - I've paused working on other code changes until I've sorted apple silicon support which is taking me longer than I hoped...

[ShortDevelopment]

@rhuanjl
No problem; just take your time!