Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't validate signature for _VTblGap* methods #55616

Merged
merged 1 commit into from
Jul 14, 2021

Conversation

elinor-fung
Copy link
Member

@elinor-fung elinor-fung commented Jul 14, 2021

Fixes #47849

The _VTblGap* methods that get generated for the interop assembly are missing the HasThis flag, resulting in 0x8013123b : Method is not marked static but is not HASTHIS or EXPLICITTHIS..

We used to avoid checking the signature for _VTblGap* methods, but with beb215b, we ended up doing that validation. This change makes it so that we skip the validation again for _VTblGap* methods.

I didn't add a test because I couldn't figure out a good way to do it, since the flag gets set properly based on the absence of static in IL.

cc @AaronRobinsonMSFT @jkoritzinsky

@jkoritzinsky
Copy link
Member

It should be possible to write an IL test under the TypeEquivalence test folder/suite that validates that we can load a type with a VtblGap function with the incorrect flags, I think.

@elinor-fung
Copy link
Member Author

My issue was that I don't know how to express 'not static and not instance' in IL? Since the lack of static in

.method public specialname rtspecialname 
        void  _VtblGap8() runtime managed

implies instance, so the resulting assembly has the flag properly set on that method signature.

@jkoritzinsky
Copy link
Member

I didn't realize that non-static implied instance in the IL textual format. In that case, I agree that testing this isn't easily doable.

@elinor-fung
Copy link
Member Author

I didn't realize it until I tried to write tests yesterday 🙁

Copy link
Member

@davidwrighton davidwrighton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@elinor-fung elinor-fung merged commit 2e2670e into dotnet:main Jul 14, 2021
@elinor-fung elinor-fung deleted the fixVtblGap branch July 14, 2021 23:30
@ghost ghost locked as resolved and limited conversation to collaborators Aug 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reference type in public field causes TypeLoadException for Com Interop
3 participants