Skip to content

Avoid GetFunctionForDelegate call in delegate vtables #1971

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

Merged
merged 1 commit into from
Apr 8, 2025

Conversation

manodasanW
Copy link
Member

We saw when code mitigations are enabled to disable dynamic code, GetFunctionPointerForDelegate will run into issues. This is the first step to remove its usage specifically in delegate scenarios. We achieve this by using the same vtable for IReference which was the only scenario that needed it and instead as part of the shared implementation look up the registered IID which is the only reason why we couldn't share vtables before.

…ing of the same vtable ptr for nullable support.
Copy link
Member

@Sergio0694 Sergio0694 left a comment

Choose a reason for hiding this comment

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

Clever! Left just a couple notes 🙂

@@ -2421,7 +2474,7 @@ public static ComWrappers.ComInterfaceEntry[] GetExposedInterfaces(ComWrappers.C
entries[count++] = new ComWrappers.ComInterfaceEntry
{
IID = PIID,
Vtable = ABI.System.Nullable_Delegate<T>.AbiToProjectionVftablePtr
Vtable = ABI.System.Nullable_Delegate.AbiToProjectionVftablePtr
Copy link
Member

Choose a reason for hiding this comment

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

With this change, when will the generic vtable be used on NAOT? Could we drop that cctor? 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

We might be able to drop it, but I will defer on that to future PR.

@@ -1919,6 +1920,51 @@ public static unsafe Nullable GetValue(IInspectable inspectable)
}
}

[Guid("61C17706-2D65-11E0-9AE8-D48564015472")]
Copy link
Member

Choose a reason for hiding this comment

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

What's the point of this guid and the one on the generic one? Each IReference<T> interface will have its own IID, so what does this one represent, and when is it used?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is actually not used, but represents IReference.

Copy link
Member Author

Choose a reason for hiding this comment

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

Will remove this in future PR too.

@manodasanW manodasanW merged commit 49c829f into staging/2.3 Apr 8, 2025
10 checks passed
@manodasanW manodasanW deleted the manodasanw/shareDelegateVtable branch April 8, 2025 05:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants