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

Load C++/CLI DLLs built against .NET 7.0+ in default ALC #66486

Merged
merged 6 commits into from
Mar 18, 2022
Merged

Load C++/CLI DLLs built against .NET 7.0+ in default ALC #66486

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a3bb8a5
Load C++/CLI DLLs in default ALC
elinor-fung Mar 10, 2022
61ec887
Use get_function_pointer
elinor-fung Mar 11, 2022
2ccb5ba
PR feedback
elinor-fung Mar 15, 2022
73d6bd6
Root GetFunctionPointer. Add hook for erroring when disabled
elinor-fung Mar 17, 2022
d703fa9
Apply suggestions from code review
elinor-fung Mar 17, 2022
bb8d6ae
Add try-catch
elinor-fung 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
3 changes: 2 additions & 1 deletion src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@
<!-- Sources -->
<ItemGroup>
<Compile Include="$(BclSourcesRoot)\Internal\Runtime\InteropServices\ComActivationContextInternal.cs" />
<Compile Include="$(BclSourcesRoot)\Internal\Runtime\InteropServices\InMemoryAssemblyLoader.cs" />
<Compile Include="$(BclSourcesRoot)\System\__Canon.cs" />
<Compile Include="$(BclSourcesRoot)\System\ArgIterator.cs" />
<Compile Include="$(BclSourcesRoot)\System\Array.CoreCLR.cs" />
Expand Down Expand Up @@ -285,10 +284,12 @@
<Compile Include="$(BclSourcesRoot)\Internal\Runtime\InteropServices\ComActivator.PlatformNotSupported.cs" />
</ItemGroup>
<ItemGroup Condition="'$(TargetsUnix)' == 'true'">
<Compile Include="$(BclSourcesRoot)\Internal\Runtime\InteropServices\InMemoryAssemblyLoader.PlatformNotSupported.cs" />
<Compile Include="$(BclSourcesRoot)\Interop\Unix\Interop.Libraries.cs" />
<Compile Include="$(BclSourcesRoot)\System\Threading\LowLevelLifoSemaphore.Unix.cs" />
</ItemGroup>
<ItemGroup Condition="'$(TargetsWindows)' == 'true'">
<Compile Include="$(BclSourcesRoot)\Internal\Runtime\InteropServices\InMemoryAssemblyLoader.cs" />
<Compile Include="$(CommonPath)Interop\Windows\OleAut32\Interop.VariantClear.cs">
<Link>Common\Interop\Windows\OleAut32\Interop.VariantClear.cs</Link>
</Compile>
Expand Down
7 changes: 0 additions & 7 deletions src/coreclr/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.Shared.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,6 @@
<type fullname="System.SZArrayHelper">
<method name=".ctor" />
</type>

<!-- Enables the .NET IJW host to load an in-memory module as a .NET assembly.
These are always rooted to ensure native calls get trimmer related errors
but will be trimmed away by the related feature switch -->
<type fullname="Internal.Runtime.InteropServices.InMemoryAssemblyLoader">
<method name="LoadInMemoryAssembly" />
</type>
</assembly>

<!-- The private Event methods are accessed by private reflection in the base EventSource class. -->
Expand Down
8 changes: 8 additions & 0 deletions src/coreclr/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.Windows.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,13 @@
<method name="RegisterClassForTypeInternal" />
<method name="UnregisterClassForTypeInternal" />
</type>

<!-- Enables the .NET IJW host to load an in-memory module as a .NET assembly.
These are always rooted to ensure native calls get trimmer related errors
but will be trimmed away by the related feature switch -->
elinor-fung marked this conversation as resolved.
Show resolved Hide resolved
<type fullname="Internal.Runtime.InteropServices.InMemoryAssemblyLoader">
<method name="LoadInMemoryAssembly" />
<method name="LoadInMemoryAssemblyInContext" />
</type>
</assembly>
</linker>
23 changes: 23 additions & 0 deletions ...reLib/src/Internal/Runtime/InteropServices/InMemoryAssemblyLoader.PlatformNotSupported.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Runtime.InteropServices;
using System.Runtime.Loader;

namespace Internal.Runtime.InteropServices
{
/// <summary>
/// This class enables the .NET IJW host to load an in-memory module as a .NET assembly
/// </summary>
public static class InMemoryAssemblyLoader
{
/// <summary>
/// Loads into an isolated AssemblyLoadContext an assembly that has already been loaded into memory by the OS loader as a native module.
/// </summary>
/// <param name="moduleHandle">The native module handle for the assembly.</param>
/// <param name="assemblyPath">The path to the assembly (as a pointer to a UTF-16 C string).</param>
public static unsafe void LoadInMemoryAssembly(IntPtr moduleHandle, IntPtr assemblyPath)
=> throw new PlatformNotSupportedException();
}
}
69 changes: 54 additions & 15 deletions ...clr/System.Private.CoreLib/src/Internal/Runtime/InteropServices/InMemoryAssemblyLoader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,48 +2,87 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.InteropServices;
using System.Runtime.Loader;
using System.Runtime.Versioning;

namespace Internal.Runtime.InteropServices
{
/// <summary>
/// This class enables the .NET IJW host to load an in-memory module as a .NET assembly
/// </summary>
[SupportedOSPlatform("windows")]
public static class InMemoryAssemblyLoader
{
#if TARGET_WINDOWS
private static bool IsSupported { get; } = InitializeIsSupported();

private static bool InitializeIsSupported() => AppContext.TryGetSwitch("System.Runtime.InteropServices.EnableCppCLIHostActivation", out bool isSupported) ? isSupported : true;
#endif

/// <summary>
/// Loads into an isolated AssemblyLoadContext an assembly that has already been loaded into memory by the OS loader as a native module.
/// Loads an assembly that has already been loaded into memory by the OS loader as a native module
/// into an isolated AssemblyLoadContext.
/// </summary>
/// <param name="moduleHandle">The native module handle for the assembly.</param>
/// <param name="assemblyPath">The path to the assembly (as a pointer to a UTF-16 C string).</param>
public static unsafe void LoadInMemoryAssembly(IntPtr moduleHandle, IntPtr assemblyPath)
{
#if TARGET_WINDOWS
if (!IsSupported)
throw new NotSupportedException("This API is not enabled in trimmed scenarios. see https://aka.ms/dotnet-illink/nativehost for more details");

#pragma warning disable IL2026 // suppressed in ILLink.Suppressions.LibraryBuild.xml
LoadInMemoryAssemblyInContextImpl(moduleHandle, assemblyPath);
#pragma warning restore IL2026
}

/// <summary>
/// Loads into an assembly that has already been loaded into memory by the OS loader as a native module
/// into the specified load context.
/// </summary>
/// <param name="moduleHandle">The native module handle for the assembly.</param>
/// <param name="assemblyPath">The path to the assembly (as a pointer to a UTF-16 C string).</param>
/// <param name="loadContext">Load context (currently must be IntPtr.Zero)</param>
[UnmanagedCallersOnly]
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",
Justification = "The same C++/CLI feature switch applies to LoadInMemoryAssembly and this function. We rely on the warning from LoadInMemoryAssembly.")]
internal static unsafe void LoadInMemoryAssemblyInContext(IntPtr moduleHandle, IntPtr assemblyPath, IntPtr loadContext)
Copy link
Member

Choose a reason for hiding this comment

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

We could go either with the same mechanism as with the LoadInMemoryAssembly (remove the RUC on this method, suppress it inside).
Or since this is internal API we could root it conditionally based on the same feature switch the IsSupported uses - in that case linker would remove this entire method in trimmed apps (better for size).

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought we had intentionally rooted the native hosting APIs unconditionally, so that users can actually get an error message, since the component doesn't have a say in trimming / the app might not 'know' it is loading a component via hosting APIs?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, I guess we changed that based on #62903? But just for ComponentActivator and not COM or C++/CLI.

Copy link
Member Author

Choose a reason for hiding this comment

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

So I'd also need to update the sdk so that if you set EnableCppCLIHostActivation=true, it also sets _EnableConsumingManagedCodeFromNativeHosting=true (or EnableNativeHostActivation since it is being renamed) for an app to at least try to go through C++/CLI activation.

And then with the default settings (EnableCppCLIHostActivation=false), even with LoadInMemoryAssemblyInContext rooted unconditionally as I have it right now, users won't get a nice message. Unless I also root ComponentActivator.GetFunctionPointer when System.Runtime.InteropServices.EnableCppCLIHostActivation=true and do some sort of handling so that

runtime/src/libraries/System.Private.CoreLib/src/Internal/Runtime/InteropServices/ComponentActivator.cs

Lines 117 to 118 in ba962fd

if (!IsSupported)
return HostFeatureDisabled;

also enables support when C++/CLI activation is supported.

The non-nice message is:

Unhandled exception. System.Runtime.InteropServices.SEHException (0x80004005): External component has thrown an exception.

for trying to call a native entry point (which then calls into managed) in a C++/CLI library.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm - that's a good point - I didn't realize that by relying on the get_function_pointer it breaks the existing assumptions around feature switches... I told you there are some dragons hidden here 😇 .

Is this because C++/CLI is the only hosting which can propagate full error messages?

I guess what you describe would be a solution - enable native hosting if CPP/CLI is enabled. I would probably do it fully - if you want C++/CLI, you also get full native hosting support - I would not try to do magic in the ComponentActivator. Trimming with C++/CLI activation enabled is a very bad idea anyway.

Copy link
Member Author

@elinor-fung elinor-fung Mar 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I think COM also would get the full error message propagated when it is a managed app loading a managed COM component (didn't test it though).

I definitely don't want to do any magic in ComponentActivator - my thought for potentially having to do something there would purely be for trying to get a nice error message when someone tries to use C++/CLI when it is disabled. Reread my comment above and realized that this is not at all what I actually wrote... I was trying to say that because ComponentActivator entry points are no longer rooted, we don't get a nice error. So unless we always root them (or at least GetFunctionPointer) on Windows and somehow make it do something other than just return the error code, we will just have the SEHException when activating C++/CLI when it is trimmed/disabled.

🐉 🐉 🐉

Copy link
Member

Choose a reason for hiding this comment

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

I think I finally understand it - if the feature switches are off, we remove the main entry point and that leads to bad error experience. I think it would make sense to keep the GetFunctionPointer always. I like it a lot as the main entry point for all native->managed (even though that's not what it does today), and as such we should keep it. On Windows eventually we could migrate all hosts to use that, in which case we could completely remove COM/C++/CLI entry points if the feature switches are off, since we would only need the GetFunctionPointer to fail. That would actually be an overall size win. On non-Windows this will be a very small size regression, but I think it's worth it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I definitely agree that GetFunctionPointer is nice as the main entry point. In order to have the nicer error experience (like the managed exception with an informative message), we would probably still need it to have some hook to do something for the COM and C++/CLI cases other than just return an error code if the native hosting feature switch is off.

We could also only keep it for Windows, since those are the provided host scenarios we currently have and only do it for non-Windows when we need to - not sure if it is worth the split/discrepancy .

I'll try always keeping GetFunctionPointer and working out a decent error experience.

{
if (!IsSupported)
throw new NotSupportedException("This API is not enabled in trimmed scenarios. see https://aka.ms/dotnet-illink/nativehost for more details");

if (loadContext != IntPtr.Zero)
throw new ArgumentOutOfRangeException(nameof(loadContext));

LoadInMemoryAssemblyInContextImpl(moduleHandle, assemblyPath, AssemblyLoadContext.Default);
}

[RequiresUnreferencedCode("C++/CLI is not trim-compatible", Url = "https://aka.ms/dotnet-illink/nativehost")]
private static void LoadInMemoryAssemblyInContextImpl(IntPtr moduleHandle, IntPtr assemblyPath, AssemblyLoadContext? alc = null)
{
string? assemblyPathString = Marshal.PtrToStringUni(assemblyPath);
if (assemblyPathString == null)
{
throw new ArgumentOutOfRangeException(nameof(assemblyPath));
}

// We don't cache the ALCs here since each IJW assembly will call this method at most once
// We don't cache the ALCs or resolvers here since each IJW assembly will call this method at most once
// (the load process rewrites the stubs that call here to call the actual methods they're supposed to)
#pragma warning disable IL2026 // suppressed in ILLink.Suppressions.LibraryBuild.xml
AssemblyLoadContext context = new IsolatedComponentLoadContext(assemblyPathString);
#pragma warning restore IL2026
context.LoadFromInMemoryModule(moduleHandle);
#else
throw new PlatformNotSupportedException();
#endif
if (alc is null)
{
alc = new IsolatedComponentLoadContext(assemblyPathString);
}
else if (alc == AssemblyLoadContext.Default)
{
var resolver = new AssemblyDependencyResolver(assemblyPathString);
AssemblyLoadContext.Default.Resolving +=
[RequiresUnreferencedCode("C++/CLI is not trim-compatible", Url = "https://aka.ms/dotnet-illink/nativehost")]
elinor-fung marked this conversation as resolved.
Show resolved Hide resolved
(context, assemblyName) =>
{
string? assemblyPath = resolver.ResolveAssemblyToPath(assemblyName);
return assemblyPath != null
? context.LoadFromAssemblyPath(assemblyPath)
: null;
};
}

alc.LoadFromInMemoryModule(moduleHandle);
}
}
}
4 changes: 2 additions & 2 deletions src/installer/tests/HostActivation.Tests/NativeHosting/Ijwhost.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public void LoadLibrary()

result.Should().Pass()
.And.HaveStdOutContaining("[C++/CLI] NativeEntryPoint: calling managed class")
.And.HaveStdOutContaining("[C++/CLI] ManagedClass: AssemblyLoadContext = \"IsolatedComponentLoadContext");
.And.HaveStdOutContaining("[C++/CLI] ManagedClass: AssemblyLoadContext = \"Default\" System.Runtime.Loader.DefaultAssemblyLoadContext");
}

[Theory]
Expand All @@ -57,7 +57,7 @@ public void ManagedHost(bool selfContained)

result.Should().Pass()
.And.HaveStdOutContaining("[C++/CLI] NativeEntryPoint: calling managed class")
.And.HaveStdOutContaining("[C++/CLI] ManagedClass: AssemblyLoadContext = \"IsolatedComponentLoadContext")
.And.HaveStdOutContaining("[C++/CLI] ManagedClass: AssemblyLoadContext = \"Default\" System.Runtime.Loader.DefaultAssemblyLoadContext")
.And.HaveStdErrContaining($"Executing as a {(selfContained ? "self-contained" : "framework-dependent")} app");
}

Expand Down
31 changes: 21 additions & 10 deletions src/native/corehost/ijwhost/ijwhost.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@
// The .NET Foundation licenses this file to you under the MIT license.

#include "ijwhost.h"
#include "hostfxr.h"
#include "fxr_resolver.h"
#include "pal.h"
#include "trace.h"
#include "error_codes.h"
#include "utils.h"
#include <coreclr_delegates.h>
#include <hostfxr.h>
#include <fxr_resolver.h>
#include <pal.h>
#include <trace.h>
#include <error_codes.h>
#include <utils.h>
#include "bootstrap_thunk.h"


#if defined(_WIN32)
// IJW entry points are defined without the __declspec(dllexport) attribute.
// The issue here is that the MSVC compiler links to the exact name _CorDllMain instead of their stdcall-managled names.
Expand All @@ -22,8 +22,9 @@

pal::hresult_t get_load_in_memory_assembly_delegate(pal::dll_t handle, load_in_memory_assembly_fn* delegate)
{
return load_fxr_and_get_delegate(
hostfxr_delegate_type::hdt_load_in_memory_assembly,
get_function_pointer_fn get_function_pointer;
int status = load_fxr_and_get_delegate(
hostfxr_delegate_type::hdt_get_function_pointer,
[handle](const pal::string_t& host_path, pal::string_t* config_path_out)
{
pal::string_t mod_path;
Expand All @@ -39,8 +40,18 @@ pal::hresult_t get_load_in_memory_assembly_delegate(pal::dll_t handle, load_in_m

return StatusCode::Success;
},
delegate
&get_function_pointer
);
if (status != StatusCode::Success)
return status;

return get_function_pointer(
_X("Internal.Runtime.InteropServices.InMemoryAssemblyLoader, System.Private.CoreLib"),
_X("LoadInMemoryAssemblyInContext"),
UNMANAGEDCALLERSONLY_METHOD,
nullptr, // load context
nullptr, // reserved
(void**)delegate);
}

IJW_API BOOL STDMETHODCALLTYPE _CorDllMain(HINSTANCE hInst,
Expand Down
2 changes: 1 addition & 1 deletion src/native/corehost/ijwhost/ijwhost.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ bool patch_vtable_entries(PEDecoder& decoder);
void release_bootstrap_thunks(PEDecoder& decoder);
bool are_thunks_installed_for_module(HMODULE instance);

using load_in_memory_assembly_fn = void(STDMETHODCALLTYPE*)(pal::dll_t handle, const pal::char_t* path);
using load_in_memory_assembly_fn = void(STDMETHODCALLTYPE*)(pal::dll_t handle, const pal::char_t* path, void* load_context);

pal::hresult_t get_load_in_memory_assembly_delegate(pal::dll_t handle, load_in_memory_assembly_fn* delegate);

Expand Down
8 changes: 4 additions & 4 deletions src/native/corehost/ijwhost/ijwthunk.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ namespace
HANDLE g_heapHandle;

bool patch_vtable_entries(PEDecoder& pe)
{
{
size_t numFixupRecords;
IMAGE_COR_VTABLEFIXUP* pFixupTable = pe.GetVTableFixups(&numFixupRecords);

Expand Down Expand Up @@ -68,7 +68,7 @@ bool patch_vtable_entries(PEDecoder& pe)
}

trace::setup();

error_writer_scope_t writer_scope(swallow_trace);

size_t currentThunk = 0;
Expand Down Expand Up @@ -115,7 +115,7 @@ extern "C" std::uintptr_t __stdcall start_runtime_and_get_target_address(std::ui
{
trace::setup();
error_writer_scope_t writer_scope(swallow_trace);

bootstrap_thunk *pThunk = bootstrap_thunk::get_thunk_from_cookie(cookie);
load_in_memory_assembly_fn loadInMemoryAssembly;
pal::dll_t moduleHandle = pThunk->get_dll_handle();
Expand Down Expand Up @@ -145,7 +145,7 @@ extern "C" std::uintptr_t __stdcall start_runtime_and_get_target_address(std::ui
#pragma warning (pop)
}

loadInMemoryAssembly(moduleHandle, app_path.c_str());
loadInMemoryAssembly(moduleHandle, app_path.c_str(), nullptr);

std::uintptr_t thunkAddress = *(pThunk->get_slot_address());

Expand Down