Skip to content

[MonoVM] Report original DLL name when P/Invoke lookup fails #58206

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.

Sign up for GitHub

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

Jump to bottom
Merged
akoeplinger merged 1 commit into from
Sep 9, 2021
Merged

[MonoVM] Report original DLL name when P/Invoke lookup fails #58206

akoeplinger merged 1 commit into from
Sep 9, 2021

Conversation

@filipnavara
Copy link
Member

@filipnavara filipnavara commented Aug 26, 2021

Ref: #58185

@ghost ghost added community-contribution Indicates that the PR has been added by a community member area-AssemblyLoader-mono and removed community-contribution Indicates that the PR has been added by a community member labels Aug 26, 2021
@ghost
Copy link

ghost commented Aug 26, 2021

Tagging subscribers to this area:
See info in area-owners.md if you want to be subscribed.

Issue Details

Ref: #58185

Author: filipnavara
Assignees: -
Labels:

area-AssemblyLoader-mono
Milestone: -

akoeplinger
akoeplinger approved these changes Aug 30, 2021
View reviewed changes
Copy link
Member

@akoeplinger akoeplinger left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM but I think we should also add ifdefs so we don't even try the retry_with_libcoreclr on mobile platforms where we don't have libcoreclr anyway

src/mono/mono/metadata/native-library.c Outdated
Copy link
Member

@akoeplinger akoeplinger Aug 30, 2021

Choose a reason for hiding this comment

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

nit: check whether new_scope and orig_scope differ so we don't get duplicate library name in the normal case where a library is not redirected.

@akoeplinger
Copy link
Member

akoeplinger commented Aug 30, 2021

@filipnavara actually, I just saw that we define MONO_LOADER_LIBRARY_NAME for android so we might be using retry_with_libcoreclr there somehow? probably safer to just leave it for now

@filipnavara
Copy link
Member Author

filipnavara commented Aug 30, 2021

I'll clean this up when I get a moment this week. I created it mostly as a reminder that it should be done and then I forgot about it... 😅

@filipnavara filipnavara marked this pull request as ready for review September 8, 2021 07:17
@filipnavara
Copy link
Member Author

filipnavara commented Sep 8, 2021

I've reworked the logic a bit so this needs to be re-reviewed.

lambdageek
lambdageek approved these changes Sep 8, 2021
View reviewed changes
Copy link
Member

@lambdageek lambdageek left a comment

Choose a reason for hiding this comment

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

Ok.

(not something to address in this PR) This function is turning into spaghetti again.

@akoeplinger akoeplinger merged commit 29a5ff0 into dotnet:main Sep 9, 2021
@akoeplinger
Copy link
Member

akoeplinger commented Sep 9, 2021

/backport to release/6.0

@github-actions
Copy link
Contributor

github-actions bot commented Sep 9, 2021

Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/1217169308

@github-actions github-actions bot mentioned this pull request Sep 9, 2021
Merged
@ghost ghost locked as resolved and limited conversation to collaborators Oct 9, 2021
@filipnavara filipnavara deleted the mono_dllimport branch June 5, 2025 07:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@lambdageek lambdageek lambdageek approved these changes

@akoeplinger akoeplinger akoeplinger approved these changes

@thaystg thaystg Awaiting requested review from thaystg thaystg is a code owner

@vargaz vargaz Awaiting requested review from vargaz

Assignees

No one assigned

Labels

area-AssemblyLoader-mono

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@filipnavara @akoeplinger @lambdageek