Remove asserts that were firing during rejit tests #33492
Conversation
|
I couldn't add an area label to this PR. Checkout this page to find out which area owner to ping, or please add exactly one area label to help train me in the future. |
src/coreclr/src/vm/callcounting.cpp
Outdated
| @@ -448,7 +448,7 @@ bool CallCountingManager::IsCallCountingEnabled(NativeCodeVersion codeVersion) | |||
| CONTRACTL_END; | |||
|
|
|||
| _ASSERTE(!codeVersion.IsNull()); | |||
| _ASSERTE(codeVersion.IsDefaultVersion()); | |||
| _ASSERTE(codeVersion.IsDefaultVersion() || codeVersion.GetILCodeVersionId() != 0); | |||
There was a problem hiding this comment.
This function shouldn't be called for non-default native code versions. It effectively is a storage location just for the default code version that affects the tier for the default code version. Non-default code versions have a storage location for the tier. It looks like this caller would be the issue (there are only two callers):
runtime/src/coreclr/src/vm/prestub.cpp
Lines 1051 to 1056 in 2b27323
That assert is actually unnecessary since the assert above it checks for tier 0, which implies what is intended to be verified. Could remove that assert instead (including the call to this function) and the only other call to this function should be fine. Do you have the stack trace of the failure to confirm? If you can take care of it then great, otherwise I'm working on a fix and I can include this fix in that, let me know.
There was a problem hiding this comment.
The issue I saw is that non-default IL versions will have a tier 0 that returns false for NativeCodeVersion::IsDefaultVersion. I haven't seen any evidence that it's called for tier 1 methods
There was a problem hiding this comment.
I don't have the stack trace any more, I can get it but it would have to be later today
There was a problem hiding this comment.
Yea makes sense. It would be great if you could confirm that as the cause, I'll go ahead and remove the assert in my change anyway. Also can you point me to the test that is failing?
ghost
locked as resolved and limited conversation to collaborators
#32250 had some asserts that would fire under certain situations with our rejit tests. The rejit path no longer suspends the runtime so that assert was removed, and call counting can happen for a non default code version with rejit since the IL is updated, so that assert was updated.