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.

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

Always append the "dll" extension for native libraries loads on Windows #66666

Merged
merged 6 commits into from
Mar 17, 2022
Merged

Always append the "dll" extension for native libraries loads on Windows #66666

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4df4603
Always append the "dll" extension for native libraries loads on Windows
AaronRobinsonMSFT Mar 15, 2022
0f725c3
Update managed logic.
AaronRobinsonMSFT Mar 15, 2022
c1e45c8
Update test for new behavior
AaronRobinsonMSFT Mar 16, 2022
fe27cfd
Review feedback
AaronRobinsonMSFT Mar 17, 2022
ed3a2ee
Make both native and manged version of path variation use the same
AaronRobinsonMSFT Mar 17, 2022
d38d595
Update src/coreclr/vm/nativelibrary.cpp
AaronRobinsonMSFT Mar 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 14 additions & 19 deletions src/coreclr/vm/nativelibrary.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ extern bool g_hostpolicy_embedded;
#define PLATFORM_SHARED_LIB_SUFFIX_W PAL_SHLIB_SUFFIX_W
#define PLATFORM_SHARED_LIB_PREFIX_W PAL_SHLIB_PREFIX_W
#else // !TARGET_UNIX
// The default for Windows OS is ".DLL". This causes issues with case-sensitive file systems on Windows.
// We are using the lowercase version due to historical precedence and how common it is now.
#define PLATFORM_SHARED_LIB_SUFFIX_W W(".dll")
#define PLATFORM_SHARED_LIB_PREFIX_W W("")
#endif // !TARGET_UNIX
Expand Down Expand Up @@ -574,26 +576,19 @@ namespace

int varCount = 0;

// The purpose of following code is to workaround LoadLibrary limitation:
// LoadLibrary won't append extension if filename itself contains '.'. Thus it will break the following scenario:
// [DllImport("A.B")] // The full name for file is "A.B.dll". This is common code pattern for cross-platform PInvoke
// The workaround for above scenario is to call LoadLibrary with "A.B" first, if it fails, then call LoadLibrary with "A.B.dll"
auto it = libName.Begin();
if (!libNameIsRelativePath ||
AaronRobinsonMSFT marked this conversation as resolved.
Show resolved Hide resolved
!libName.Find(it, W('.')) ||
libName.EndsWith(W(".")) ||
libName.EndsWithCaseInsensitive(W(".dll")) ||
libName.EndsWithCaseInsensitive(W(".exe")))
// Follow LoadLibrary rules in MSDN doc: https://docs.microsoft.com/windows/win32/api/libloaderapi/nf-libloaderapi-loadlibrarya
// To prevent the function from appending ".DLL" to the module name, include a trailing point character (.) in the module name string
// or provide an absolute path.
libNameVariations[varCount++] = NameFmt;

// The runtime will append the '.dll' extension if the path is relative and the name doesn't ends with a "."
AaronRobinsonMSFT marked this conversation as resolved.
Show resolved Hide resolved
// or an existing known extension. This is done due to issues with case-sensitive file systems
// on Windows. The Windows loader always appends ".DLL" as opposed to the more common ".dll".
if (libNameIsRelativePath
&& !libName.EndsWith(W("."))
&& !libName.EndsWithCaseInsensitive(W(".dll"))
&& !libName.EndsWithCaseInsensitive(W(".exe")))
{
// Follow LoadLibrary rules in MSDN doc: https://msdn.microsoft.com/en-us/library/windows/desktop/ms684175(v=vs.85).aspx
// If the string specifies a full path, the function searches only that path for the module.
// If the string specifies a module name without a path and the file name extension is omitted, the function appends the default library extension .dll to the module name.
// To prevent the function from appending .dll to the module name, include a trailing point character (.) in the module name string.
libNameVariations[varCount++] = NameFmt;
}
else
{
libNameVariations[varCount++] = NameFmt;
libNameVariations[varCount++] = NameSuffixFmt;
}

Expand Down
8 changes: 4 additions & 4 deletions ...ibraries/System.Private.CoreLib/src/System/Runtime/Loader/LibraryNameVariation.Windows.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ internal static IEnumerable<LibraryNameVariation> DetermineLibraryNameVariations

yield return new LibraryNameVariation(string.Empty, string.Empty);

// Follow LoadLibrary rules if forOSLoader is true
if (isRelativePath &&
!libName.EndsWith(".dll", StringComparison.OrdinalIgnoreCase) &&
!libName.EndsWith(".exe", StringComparison.OrdinalIgnoreCase))
if (isRelativePath
&& !libName.EndsWith(".", StringComparison.OrdinalIgnoreCase)
&& !libName.EndsWith(".dll", StringComparison.OrdinalIgnoreCase)
&& !libName.EndsWith(".exe", StringComparison.OrdinalIgnoreCase))
{
yield return new LibraryNameVariation(string.Empty, LibraryNameSuffix);
}
Expand Down
61 changes: 31 additions & 30 deletions ...oader/AssemblyDependencyResolver/AssemblyDependencyResolverTests/NativeDependencyTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,21 +51,21 @@ public void TestSimpleNameAndLibPrefixAndNoSuffix()
public void TestRelativeNameAndLibPrefixAndNoSuffix()
{
// The lib prefix is not added if the lookup is a relative path.
ValidateNativeLibraryWithRelativeLookupResolutions("lib{0}", "{0}", 0);
ValidateNativeLibraryWithRelativeLookupResolutions("lib{0}", "{0}", OS.None);
}

public void TestSimpleNameAndLibPrefixAndSuffix()
{
ValidateNativeLibraryResolutions("lib{0}.dll", "{0}", 0);
ValidateNativeLibraryResolutions("lib{0}.dll", "{0}", OS.None);
ValidateNativeLibraryResolutions("lib{0}.dylib", "{0}", OS.OSX);
ValidateNativeLibraryResolutions("lib{0}.so", "{0}", OS.Linux);
}

public void TestNameWithSuffixAndNoPrefixAndNoSuffix()
{
ValidateNativeLibraryResolutions("{0}", "{0}.dll", 0);
ValidateNativeLibraryResolutions("{0}", "{0}.dylib", 0);
ValidateNativeLibraryResolutions("{0}", "{0}.so", 0);
ValidateNativeLibraryResolutions("{0}", "{0}.dll", OS.None);
ValidateNativeLibraryResolutions("{0}", "{0}.dylib", OS.None);
ValidateNativeLibraryResolutions("{0}", "{0}.so", OS.None);
}

public void TestNameWithSuffixAndNoPrefixAndSuffix()
Expand All @@ -78,16 +78,16 @@ public void TestNameWithSuffixAndNoPrefixAndSuffix()
public void TestNameWithSuffixAndNoPrefixAndDoubleSuffix()
{
// Unixes add the suffix even if one is already present.
ValidateNativeLibraryResolutions("{0}.dll.dll", "{0}.dll", 0);
ValidateNativeLibraryResolutions("{0}.dll.dll", "{0}.dll", OS.None);
ValidateNativeLibraryResolutions("{0}.dylib.dylib", "{0}.dylib", OS.OSX);
ValidateNativeLibraryResolutions("{0}.so.so", "{0}.so", OS.Linux);
}

public void TestNameWithSuffixAndPrefixAndNoSuffix()
{
ValidateNativeLibraryResolutions("lib{0}", "{0}.dll", 0);
ValidateNativeLibraryResolutions("lib{0}", "{0}.dylib", 0);
ValidateNativeLibraryResolutions("lib{0}", "{0}.so", 0);
ValidateNativeLibraryResolutions("lib{0}", "{0}.dll", OS.None);
ValidateNativeLibraryResolutions("lib{0}", "{0}.dylib", OS.None);
ValidateNativeLibraryResolutions("lib{0}", "{0}.so", OS.None);
}

public void TestNameWithSuffixAndPrefixAndSuffix()
Expand All @@ -100,14 +100,14 @@ public void TestNameWithSuffixAndPrefixAndSuffix()
public void TestRelativeNameWithSuffixAndPrefixAndSuffix()
{
// The lib prefix is not added if the lookup is a relative path
ValidateNativeLibraryWithRelativeLookupResolutions("lib{0}.dll", "{0}.dll", 0);
ValidateNativeLibraryWithRelativeLookupResolutions("lib{0}.dylib", "{0}.dylib", 0);
ValidateNativeLibraryWithRelativeLookupResolutions("lib{0}.so", "{0}.so", 0);
ValidateNativeLibraryWithRelativeLookupResolutions("lib{0}.dll", "{0}.dll", OS.None);
ValidateNativeLibraryWithRelativeLookupResolutions("lib{0}.dylib", "{0}.dylib", OS.None);
ValidateNativeLibraryWithRelativeLookupResolutions("lib{0}.so", "{0}.so", OS.None);
}

public void TestNameWithPrefixAndNoPrefixAndNoSuffix()
{
ValidateNativeLibraryResolutions("{0}", "lib{0}", 0);
ValidateNativeLibraryResolutions("{0}", "lib{0}", OS.None);
}

public void TestNameWithPrefixAndPrefixAndNoSuffix()
Expand All @@ -117,9 +117,9 @@ public void TestNameWithPrefixAndPrefixAndNoSuffix()

public void TestNameWithPrefixAndNoPrefixAndSuffix()
{
ValidateNativeLibraryResolutions("{0}.dll", "lib{0}", 0);
ValidateNativeLibraryResolutions("{0}.dylib", "lib{0}", 0);
ValidateNativeLibraryResolutions("{0}.so", "lib{0}", 0);
ValidateNativeLibraryResolutions("{0}.dll", "lib{0}", OS.None);
ValidateNativeLibraryResolutions("{0}.dylib", "lib{0}", OS.None);
ValidateNativeLibraryResolutions("{0}.so", "lib{0}", OS.None);
}

public void TestNameWithPrefixAndPrefixAndSuffix()
Expand All @@ -136,10 +136,10 @@ public void TestWindowsAddsSuffixEvenWithOnePresent()

public void TestWindowsDoesntAddSuffixWhenExectubaleIsPresent()
{
ValidateNativeLibraryResolutions("{0}.dll.dll", "{0}.dll", 0);
ValidateNativeLibraryResolutions("{0}.dll.exe", "{0}.dll", 0);
ValidateNativeLibraryResolutions("{0}.exe.dll", "{0}.exe", 0);
ValidateNativeLibraryResolutions("{0}.exe.exe", "{0}.exe", 0);
ValidateNativeLibraryResolutions("{0}.dll.dll", "{0}.dll", OS.None);
ValidateNativeLibraryResolutions("{0}.dll.exe", "{0}.dll", OS.None);
ValidateNativeLibraryResolutions("{0}.exe.dll", "{0}.exe", OS.None);
ValidateNativeLibraryResolutions("{0}.exe.exe", "{0}.exe", OS.None);
}

private void TestLookupWithSuffixPrefersUnmodifiedSuffixOnUnixes()
Expand Down Expand Up @@ -174,21 +174,22 @@ public void TestFullPathLookupWithMatchingFileName()

public void TestFullPathLookupWithDifferentFileName()
{
ValidateFullPathNativeLibraryResolutions("lib{0}", "{0}", 0);
ValidateFullPathNativeLibraryResolutions("{0}.dll", "{0}", 0);
ValidateFullPathNativeLibraryResolutions("{0}.dylib", "{0}", 0);
ValidateFullPathNativeLibraryResolutions("{0}.so", "{0}", 0);
ValidateFullPathNativeLibraryResolutions("lib{0}.dll", "{0}", 0);
ValidateFullPathNativeLibraryResolutions("lib{0}.dylib", "{0}", 0);
ValidateFullPathNativeLibraryResolutions("lib{0}.so", "{0}", 0);
ValidateFullPathNativeLibraryResolutions("lib{0}.dll", "{0}.dll", 0);
ValidateFullPathNativeLibraryResolutions("lib{0}.dylib", "{0}.dylib", 0);
ValidateFullPathNativeLibraryResolutions("lib{0}.so", "{0}.so", 0);
ValidateFullPathNativeLibraryResolutions("lib{0}", "{0}", OS.None);
ValidateFullPathNativeLibraryResolutions("{0}.dll", "{0}", OS.None);
ValidateFullPathNativeLibraryResolutions("{0}.dylib", "{0}", OS.None);
ValidateFullPathNativeLibraryResolutions("{0}.so", "{0}", OS.None);
ValidateFullPathNativeLibraryResolutions("lib{0}.dll", "{0}", OS.None);
ValidateFullPathNativeLibraryResolutions("lib{0}.dylib", "{0}", OS.None);
ValidateFullPathNativeLibraryResolutions("lib{0}.so", "{0}", OS.None);
ValidateFullPathNativeLibraryResolutions("lib{0}.dll", "{0}.dll", OS.None);
ValidateFullPathNativeLibraryResolutions("lib{0}.dylib", "{0}.dylib", OS.None);
ValidateFullPathNativeLibraryResolutions("lib{0}.so", "{0}.so", OS.None);
}

[Flags]
private enum OS
{
None = 0,
AaronRobinsonMSFT marked this conversation as resolved.
Show resolved Hide resolved
Windows = 0x1,
OSX = 0x2,
Linux = 0x4
Expand Down