Skip to content

[Mono] Fix function pointer check - #80855

Merged
fanyang-mono merged 3 commits into
dotnet:mainfrom
fanyang-mono:fix_fnptr_type_check
Jan 23, 2023
Merged

[Mono] Fix function pointer check#80855
fanyang-mono merged 3 commits into
dotnet:mainfrom
fanyang-mono:fix_fnptr_type_check

Conversation

@fanyang-mono

@fanyang-monofanyang-mono commented Jan 19, 2023•
edited
Loading

Copy link
Copy Markdown
Member

Fix#80095

This PR fixed the function pointer checks for both interpreter and JIT. I also added runtime tests to lock down this behavior.

This fix needs to be backported to .NET7 when it gets approved.

@MattGal

MattGal commented Jan 19, 2023

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

azure-pipelinesBot commented Jan 19, 2023

Copy link
Copy Markdown
You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list.

@MattGalMattGal closed this Jan 19, 2023
@MattGalMattGal reopened this Jan 19, 2023
lambdageek
lambdageek reviewed Jan 19, 2023
View reviewed changes
Comment threadsrc/mono/mono/metadata/marshal.c Outdated
lambdageek
lambdageek approved these changes Jan 19, 2023
View reviewed changes

@lambdageeklambdageek left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

lambdageek
lambdageek reviewed Jan 19, 2023
View reviewed changes
Comment threadsrc/tests/baseservices/typeequivalence/functionpointer/Functionpointer.cs
@runfoapprunfoappBot mentioned this pull request Jan 19, 2023
Closed
@fanyang-mono

fanyang-mono commented Jan 20, 2023•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

Similar crash seen on Build Libraries Test Run release mono linux arm64 Debug showed up on rolling build as well. Not related to this PR.

@fanyang-mono

fanyang-mono commented Jan 20, 2023

Copy link
Copy Markdown
MemberAuthor

Verified that newly added test ran on CI. Inspected CI lane Build osx-x64 Release AllSubsets_Mono_Interpreter_RuntimeTests monointerpreter. For work item Loader.classloader, where new test was added to, in this PR, there were 79 tests being run. (Loader.classloader.XUnitWrapper Total: 79, Errors: 0, Failed: 0, Skipped: 0, Time: 8.450s). For another currently open PR, there were 78 tests being run. (Loader.classloader.XUnitWrapper Total: 78, Errors: 0, Failed: 0, Skipped: 0, Time: 8.849s)

@fanyang-mono

fanyang-mono commented Jan 20, 2023

Copy link
Copy Markdown
MemberAuthor

/backport to release/7.0

@github-actions

github-actionsBot commented Jan 20, 2023

Copy link
Copy Markdown
Contributor

Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/3969498723

@github-actionsgithub-actionsBot mentioned this pull request Jan 20, 2023
Merged
@fanyang-mono
fanyang-mono merged commit d6c2b43 into dotnet:mainJan 23, 2023
mdh1418 pushed a commit to mdh1418/runtime that referenced this pull request Jan 24, 2023
@fanyang-mono@mdh1418
[Mono] Fix function pointer check (dotnet#80855)
9e09943
* Fix function pointer check
* Make is_monomorphic_array return false when the element is function pointer
* Move test to a better location
@trylektrylek mentioned this pull request Jan 24, 2023
Closed
trylek added a commit that referenced this pull request Feb 2, 2023
@trylek
Backport of PR#81122to .NET 7.0, revert change to issues.targets
565cd48
Based on tactical discussion with Fan and Sam I have backported
the PR #81122 including the removal of the issues.targets entry
for function pointer tests as an extra commit on top of
the backport of Fan's PR #80855.
Thanks
Tomas
carlossanlop pushed a commit that referenced this pull request Feb 8, 2023
@github-actions@fanyang-mono@trylek
[release/7.0] [Mono] Fix function pointer check (#80927)
8502110
* Fix function pointer check
* Make is_monomorphic_array return false when the element is function pointer
* Move test to a better location
* Disable function pointer test for crossgen due to an existing issue
* Backport of PR #81122 to .NET 7.0, revert change to issues.targets
Based on tactical discussion with Fan and Sam I have backported
the PR #81122 including the removal of the issues.targets entry
for function pointer tests as an extra commit on top of
the backport of Fan's PR #80855.
Thanks
Tomas
---------
Co-authored-by: Fan Yang <yangfan@microsoft.com>
Co-authored-by: Fan Yang <52458914+fanyang-mono@users.noreply.github.com>
Co-authored-by: Tomas <trylek@microsoft.com>
@ghostghost locked as resolved and limited conversation to collaborators Feb 23, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@lambdageeklambdageeklambdageek approved these changes
@vargazvargazAwaiting requested review from vargaz
@thaystgthaystgAwaiting requested review from thaystgthaystg is a code owner
@SamMonoRTSamMonoRTAwaiting requested review from SamMonoRT

Assignees

@fanyang-monofanyang-mono

Labels

area-Codegen-JIT-mono

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ArrayTypeMismatchException in nested arrays with function pointers (android)

3 participants

@fanyang-mono@MattGal@lambdageek