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

Include all dlerror messages to DllNotFoundException #70964

Merged
merged 4 commits into from Jun 22, 2022

Conversation

HJLeee
Copy link
Contributor

@HJLeee HJLeee commented Jun 20, 2022

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Jun 20, 2022
@jkotas
Copy link
Member

jkotas commented Jun 20, 2022

I expect that there are many situations where the dlerror error message is actually helpful. Those cases will be less diagnosable after this change.

If there are situations where the error message returned by dlerror is misleading, would it be more appropriate to fix that in libdl?

@HJLeee
Copy link
Contributor Author

HJLeee commented Jun 20, 2022

Narrow down the impact by eliminating overridden error string.

The root cause of incorrect error message is LoadNativeLibraryBySearch tries its best for loading p/invoke library.
For a given p/invoke library name libpinvoke.so, it attempts 4 times with below name combinations to load the library.

currLibNameVariation = {<SBuffer> = {m_size = 34, m_allocation = 82, m_flags = 12, {m_buffer = 0x5583e6df10 "l", m_asStr = 0x5583e6df10 u"libpinvoke.so.so"}},
  static MINIMUM_GUESS = 20, static s_EmptyBuffer = "\000", static s_ACP = 65001, static s_Empty = 0x7fad806290 <s_EmptySpace>}

currLibNameVariation = {<SBuffer> = {m_size = 40, m_allocation = 82, m_flags = 12, {m_buffer = 0x5583e6df10 "l", m_asStr = 0x5583e6df10 u"liblibpinvoke.so.so"}},
  static MINIMUM_GUESS = 20, static s_EmptyBuffer = "\000", static s_ACP = 65001, static s_Empty = 0x7fad806290 <s_EmptySpace>}

currLibNameVariation = {<SBuffer> = {m_size = 28, m_allocation = 82, m_flags = 12, {m_buffer = 0x5583e6df10 "l", m_asStr = 0x5583e6df10 u"libpinvoke.so"}}, static MINIMUM_GUESS = 20,
  static s_EmptyBuffer = "\000", static s_ACP = 65001, static s_Empty = 0x7fad806290 <s_EmptySpace>}

currLibNameVariation = {<SBuffer> = {m_size = 34, m_allocation = 82, m_flags = 12, {m_buffer = 0x5583e6df10 "l", m_asStr = 0x5583e6df10 u"liblibpinvoke.so"}},
  static MINIMUM_GUESS = 20, static s_EmptyBuffer = "\000", static s_ACP = 65001, static s_Empty = 0x7fad806290 <s_EmptySpace>}

In this scenario, the last error message for dlopen() attempts is kept and it is not possible to determine what error message is beneficial to the developer.
With update, I think error message from dlerror() will be printed when it is not overwritten.

Another possible patch would be keeping error message for exact libname only.

@jkotas
Copy link
Member

jkotas commented Jun 20, 2022

If I am reading the code correctly, this will pretty much always erase the dlerror message. There is pretty much always more than one attempt to load library. The first attempt will set the error message, the second attempt will erase it, and so the error message will be lost.

@jkotas
Copy link
Member

jkotas commented Jun 20, 2022

Since we cannot tell which error message is the best one, should keep all error messages (eliminate the duplicates) and include all of them in the exception?

@HJLeee
Copy link
Contributor Author

HJLeee commented Jun 20, 2022

System.DllNotFoundException: Unable to load shared library 'libpinvoke.so' or one of its dependencies. In order to help diagnose loading problems, consider setting the LD_DEBUG environment variable: libpinvoke.so.so: cannot open shared object file: No such file or directory liblibpinvoke.so.so: cannot open shared object file: No such file or directory /opt/share/libpinvoke.so: wrong ELF class: ELFCLASS32 libpinvoke.so: cannot open shared object file: No such file or directory liblibpinvoke.so: cannot open shared object file: No such file or directory

@HJLeee HJLeee changed the title exclude dlerror message from DllNotFoundException Include all dlerror messages to DllNotFoundException Jun 20, 2022
@jkotas
Copy link
Member

jkotas commented Jun 20, 2022

Could you please make the same change in the managed version at

?

@HJLeee
Copy link
Contributor Author

HJLeee commented Jun 21, 2022

I'd love to make a patch for NativeAot.
Currently, I'm having a trouble with setting up NativeAot test env like below.

dlerror.csproj : error NU1102: Unable to find package Microsoft.NETCore.App.Runtime.linux-x64 with version (= 7.0.0-preview.5.22301.12)
dlerror.csproj : error NU1102:   - Found 1 version(s) in local [ Nearest version: 7.0.0-dev ]
dlerror.csproj : error NU1101: Unable to find package Microsoft.AspNetCore.App.Runtime.linux-x64. No packages exist with this id in source(s): local

This is my nuget.config.

<?xml version="1.0" encoding="utf-8"?>
<configuration>
  <packageSources>
    <!--To inherit the global NuGet package sources remove the <clear/> line below -->
    <clear />
    <add key="local" value="${runtime}/artifacts/packages/Release/Shipping" />
    <!--add key="nuget" value="https://api.nuget.org/v3/index.json" /-->
  </packageSources>
</configuration>

Please help me setting up the test env.

@HJLeee
Copy link
Contributor Author

HJLeee commented Jun 21, 2022

NativeAot message is checked by editing nativeaot smoke test.

      nativeaot/SmokeTests/PInvoke/PInvoke/PInvoke.sh [FAIL]
        Unhandled Exception: System.DllNotFoundException: Unable to load shared library 'PInvokeNative' or one of its dependencies. In order to help diagnose loading problems, consider using a tool like strace. If you're using glibc, consider setting the LD_DEBUG environment variable:
        PInvokeNative.so: cannot open shared object file: No such file or directory
        libPInvokeNative.so: wrong ELF class: ELFCLASS32
        PInvokeNative: cannot open shared object file: No such file or directory
        libPInvokeNative: cannot open shared object file: No such file or directory

           at System.Runtime.InteropServices.NativeLibrary.LoadLibErrorTracker.Throw(String) + 0x4e
           at Internal.Runtime.CompilerHelpers.InteropHelpers.FixupModuleCell(InteropHelpers.ModuleFixupCell*) + 0x103
           at Internal.Runtime.CompilerHelpers.InteropHelpers.ResolvePInvokeSlow(InteropHelpers.MethodFixupCell*) + 0x35
           at PInvokeTests.Program.Square(Int32) + 0x21
           at PInvokeTests.Program.TestBlittableType() + 0x1a
           at PInvokeTests.Program.Main(String[]) + 0x9
           at PInvoke!<BaseAddress>+0x2ac21b

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

LGTM.

@dotnet/interop-contrib Do you have any feedback?

@AaronRobinsonMSFT
Copy link
Member

@dotnet/interop-contrib Do you have any feedback?

Nope. Seems reasonable to me. @HJLeee Thank you.

@HJLeee
Copy link
Contributor Author

HJLeee commented Jun 21, 2022

It's my pleasure and happy to participate in dotnet runtime.

@AaronRobinsonMSFT AaronRobinsonMSFT merged commit fdc3b51 into dotnet:main Jun 22, 2022
@HJLeee HJLeee deleted the incorrect_err_msg branch June 22, 2022 06:24
@dotnet dotnet locked as resolved and limited conversation to collaborators Jul 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Interop-coreclr community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants