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

Handle PInvoke analysis similar to other methods #2091

Merged
merged 4 commits into from
Jun 16, 2021

Conversation

LakshanF
Copy link
Member

@LakshanF LakshanF commented Jun 14, 2021

This fixes PInvoke analysis similar to other methods as discussed in issue #1989.

  • We now generate the warnings from the callsite as opposed to for the PInvokecall itself. We do this by moving the analysis to MethodBodyScannerand ReflectionMethodBodyScanner
  • Dangerous PInvokes (the ones that we generate warnings) have an implicit relationship with built-in COM interop support. Specifically, IsComInterop method in the linker tries to match the logic in MarshalInfo::MarshalInfo. This means that when BuiltInComInteropSupport property is false (netcore app default), dangerous PInvokes will not work. Created a doc bug to track this.
  • Fixed some bugs in scanning IL bodies as flushed out by C++/CLI methods that had PInvoke calls
  • Updated ComPInvokeWarningtest to now check for warnings from the call sites

Plan regarding the OOB library warnings

  • Runtime libraries fix plan: We have around 40 new warnings since we moved the warning location. Created a draft PR in the runtime repo that will address this that can be used when the linker gets merged

@LakshanF LakshanF requested a review from sbomer June 14, 2021 11:02
@LakshanF
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@LakshanF LakshanF merged commit fb9ef5b into dotnet:main Jun 16, 2021
@LakshanF LakshanF deleted the PInvokeFix branch June 16, 2021 21:54
agocke pushed a commit to dotnet/runtime that referenced this pull request Nov 16, 2022
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.

4 participants