Skip to content

Treat the generated assembly as having runtime marshalling disabled - #133914

Open
jkoritzinsky wants to merge 1 commit into
dotnet:mainfrom
jkoritzinsky:delegate-thunks-disable-runtime-marshalling
Open

jkoritzinsky wants to merge 1 commit into
dotnet:mainfrom
jkoritzinsky:delegate-thunks-disable-runtime-marshalling

Conversation

@jkoritzinsky

Copy link
Copy Markdown
Member

We generate our interop thunks into this assembly, so nothing in this assembly should require interop thunks. As a result, everything in this assembly should be treated as "runtime marshalling disabled".

Fixes https://developercommunity.visualstudio.com/t/NativeAOT-emits-false-IL1005-for-Disable/11151152

We generate our interop thunks into this assembly, so nothing in this assembly should require interop thunks. As a result, everything in this assembly should be treated as "runtime marshalling disabled".
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @agocke, @dotnet/ilc-contrib
See info in area-owners.md if you want to be subscribed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

CanonModule.Assembly can remain null, causing a NullReferenceException in new marshalling paths.

Get a fresh assessment by requesting another Copilot review.

Review tier: Lite
Findings: 1 High severity

Open findings (1)
What changed in this PR

Updates runtime-marshalling policy to assembly-level state so generated interop assemblies are treated as marshalling-disabled, with regression coverage.

Changes:

  • Adds assembly-level marshalling policy metadata.
  • Updates interop, stub, delegate, and ReadyToRun consumers.
  • Adds reverse-P/Invoke coverage.
File Summary
src/​tests/​Interop/​DisabledRuntimeMarshalling/​PInvokeAssemblyMarshallingDisabled/​Delegates.cs Adds reverse-P/Invoke regression coverage.
src/​coreclr/​tools/​dotnet-pgo/​TypeRefTypeSystem/​TypeRefTypeSystemModule.cs Supplies marshalling policy for type-reference assemblies.
src/​coreclr/​tools/​Common/​TypeSystem/​Interop/​InteropStateManager.cs Uses assembly policy for calli stub handling.
src/​coreclr/​tools/​Common/​TypeSystem/​Interop/​IL/​Marshaller.cs Uses assembly policy when creating marshallers.
src/​coreclr/​tools/​Common/​TypeSystem/​Interop/​IL/​MarshalHelpers.cs Removes the obsolete policy helper.
src/​coreclr/​tools/​Common/​TypeSystem/​IL/​Stubs/​PInvokeILEmitter.cs Applies assembly policy during stub emission.
src/​coreclr/​tools/​Common/​TypeSystem/​IL/​Stubs/​DelegateMarshallingMethodThunk.cs Applies assembly policy to delegate thunks.
src/​coreclr/​tools/​Common/​TypeSystem/​Ecma/​EcmaAssembly.cs Reads the runtime-marshalling attribute.
src/​coreclr/​tools/​Common/​TypeSystem/​Common/​IAssemblyDesc.cs Defines the assembly marshalling-policy contract.
src/​coreclr/​tools/​aot/​ILCompiler.ReadyToRun/​PortableCallHelpers/​PortableCallHelpersGenerator.cs Uses assembly policy for ReadyToRun diagnostics.
src/​coreclr/​tools/​aot/​ILCompiler.ReadyToRun/​IBC/​MIbcProfileParser.cs Updates synthetic assembly policy; CanonModule.Assembly may remain null and cause a critical null-reference failure.
src/​coreclr/​tools/​aot/​ILCompiler.Compiler/​Compiler/​CompilerTypeSystemContext.GeneratedAssembly.cs Marks generated assemblies as marshalling-disabled.

Comment on lines +627 to +633
public bool IsRuntimeMarshallingEnabled
{
get
{
return true;
}
}
Comment on lines -984 to -988
public static bool IsRuntimeMarshallingEnabled(ModuleDesc module)
{
return module.Assembly is not EcmaAssembly assembly || !assembly.HasAssemblyCustomAttribute("System.Runtime.CompilerServices", "DisableRuntimeMarshallingAttribute");
}

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.

Did you consider just making this return false for non-ECMA assemblies?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants