Skip to content

Fix SAFEARRAY VARIANT copy-back and typed class array marshalling - #134729

Merged
jkoritzinsky merged 7 commits into
dotnet:mainfrom
jkoritzinsky:jkoritzinsky-fix-issues-134579-134581
Oct 6, 2026
Merged

jkoritzinsky merged 7 commits into
dotnet:mainfrom
jkoritzinsky:jkoritzinsky-fix-issues-134579-134581

Conversation

@jkoritzinsky

@jkoritzinsky jkoritzinsky commented Sep 26, 2026 •

Copy link
Copy Markdown
Member

SAFEARRAY interop can coerce a changed VARIANT element back to its original by-ref type, lose the existing native element during static array copy-back, or accept an incompatible COM object into a class-typed array. These cases can produce incorrect values or bypass the managed array's element-type constraint.

Restore non-coercing replacement for VARIANT array elements, mark existing static SAFEARRAY contents valid during copy-back, and retain the managed class element type while marshaling through its COM interface. Add native and managed regression cases for all three scenarios.

Validated with a checked CoreCLR build, the complete SafeArrayMarshallingTest class (9 passed), and the native Dispatch test. The broader Interop run had unrelated missing test executables and DisabledRuntimeMarshalling layout files.

Resolves #134579
Resolves #134580
Resolves #134581
Resolves #134574

Note

This pull request description was generated by GitHub Copilot.

Restore non-coercing VARIANT element replacement, preserve static SAFEARRAY by-ref elements, and enforce class element types during COM conversion. Add regressions for the three reported failures.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 58746a48-2bb6-476b-9993-639ef190f89d
@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: @dotnet/interop-contrib
See info in area-owners.md if you want to be subscribed.

Route AutoDual class arrays through their default IDispatch interface instead of treating the class MethodTable as an interface. Cover LPArray and SAFEARRAY marshalling.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 58746a48-2bb6-476b-9993-639ef190f89d
…t SkipOnMono

Add ModifyStaticVariantArray, CreateUnrelatedArrayElement, and AcceptExpectedArray to the native IDispatchTesting contract and native server. Remove SkipOnMono attributes already covered by IsBuiltInComEnabled.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 58746a48-2bb6-476b-9993-639ef190f89d
@jkoritzinsky
jkoritzinsky marked this pull request as ready for review September 28, 2026 17:40
Comment thread src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs Outdated
Comment thread src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs
jkoritzinsky and others added 2 commits September 30, 2026 10:02
- Remove HeterogeneousInterfaceArrayElementMarshaler, which threw
  Arg_MustBeInterface for any real-world managed class instance because it
  passed managed.GetType() (the concrete runtime type, not an interface) to
  Marshal.GetComInterfaceForObject. This class backed the single
  bHeterogeneous=TRUE call site in DispatchInfo::MarshalParamManagedToNativeRef
  (byref VARIANT-typed SAFEARRAY marshalling during IDispatch invoke). Route
  that case through the existing, already-correct
  InterfaceArrayElementMarshaler<TIsDispatch>, which resolves the COM
  interface per-element via Marshal.GetIDispatchForObject/
  GetIUnknownForObject without needing an interface Type argument. Removed
  the now-unused bHeterogeneous parameter from GetMarshalerMTForSafeArrayVarType
  and GetInstantiatedSafeArrayMethod and updated all call sites.
- Add a Debug.Assert(pDstVariant != IntPtr.Zero) plus an explanatory comment
  to ObjectMarshaler.ConvertToNativeVariantArrayElement, documenting that
  objSrc itself has no narrower type assumption than the pre-existing
  ConvertToNative (a VT_VARIANT array element can legitimately be any
  VARIANT-compatible value, including null).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 58746a48-2bb6-476b-9993-639ef190f89d
The .NET 10 heterogeneous wrapper path resolves each wrapped object's default COM interface, which can differ from the SAFEARRAY VARTYPE's IUnknown or IDispatch. Keep that per-element selection for UnknownWrapper and DispatchWrapper arrays while leaving ordinary object and typed array marshalling on the managed path. Add native pointer-identity checks for heterogeneous, untyped, and typed arrays.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 58746a48-2bb6-476b-9993-639ef190f89d
Comment thread src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs
@jkoritzinsky

Copy link
Copy Markdown
Member Author

/ba-g osx arm64 timeout

Preserve upstream TypeHandle-based element lookup while retaining the simplified SAFEARRAY helper signature.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 58746a48-2bb6-476b-9993-639ef190f89d
@jkoritzinsky
jkoritzinsky merged commit 74e4ec9 into dotnet:main Oct 6, 2026
117 of 119 checks passed
@jkoritzinsky

Copy link
Copy Markdown
Member Author

/backport to backport/pr-134284-to-release/11.0

@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Started backporting to backport/pr-134284-to-release/11.0 (link to workflow run)

@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

@jkoritzinsky backporting to backport/pr-134284-to-release/11.0 failed, the patch most likely resulted in conflicts. Please backport manually!

git am output
$ git cherry-pick 74e4ec9ed917ee6ee6b3903f87794da8b4544067

Auto-merging src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs
Auto-merging src/coreclr/vm/corelib.h
Auto-merging src/coreclr/vm/dispparammarshaler.cpp
Auto-merging src/coreclr/vm/ilmarshalers.cpp
Auto-merging src/coreclr/vm/interoputil.cpp
Auto-merging src/coreclr/vm/olevariant.cpp
Auto-merging src/coreclr/vm/olevariant.h
Auto-merging src/coreclr/vm/qcallentrypoints.cpp
Auto-merging src/coreclr/vm/stubhelpers.cpp
CONFLICT (content): Merge conflict in src/coreclr/vm/stubhelpers.cpp
Auto-merging src/coreclr/vm/stubhelpers.h
CONFLICT (content): Merge conflict in src/coreclr/vm/stubhelpers.h
error: could not apply 74e4ec9ed91... Fix SAFEARRAY VARIANT copy-back and typed class array marshalling (#134729)
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
hint: Disable this message with "git config set advice.mergeConflict false"


$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch

Applying: Fix SAFEARRAY VARIANT copy-back and typed array marshalling
Using index info to reconstruct a base tree...
M	src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs
M	src/coreclr/vm/corelib.h
M	src/coreclr/vm/dispatchinfo.cpp
M	src/coreclr/vm/ilmarshalers.cpp
M	src/coreclr/vm/olevariant.cpp
M	src/coreclr/vm/olevariant.h
M	src/coreclr/vm/qcallentrypoints.cpp
M	src/coreclr/vm/stubhelpers.cpp
M	src/coreclr/vm/stubhelpers.h
Falling back to patching base and 3-way merge...
Auto-merging src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs
Auto-merging src/coreclr/vm/corelib.h
Auto-merging src/coreclr/vm/dispatchinfo.cpp
Auto-merging src/coreclr/vm/ilmarshalers.cpp
Auto-merging src/coreclr/vm/olevariant.cpp
Auto-merging src/coreclr/vm/olevariant.h
Auto-merging src/coreclr/vm/qcallentrypoints.cpp
Auto-merging src/coreclr/vm/stubhelpers.cpp
CONFLICT (content): Merge conflict in src/coreclr/vm/stubhelpers.cpp
Auto-merging src/coreclr/vm/stubhelpers.h
CONFLICT (content): Merge conflict in src/coreclr/vm/stubhelpers.h
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 Fix SAFEARRAY VARIANT copy-back and typed array marshalling
Error: The process '/usr/bin/git' failed with exit code 128

Link to workflow output

jkoritzinsky added a commit that referenced this pull request Oct 6, 2026
…34729)

SAFEARRAY interop can coerce a changed VARIANT element back to its
original by-ref type, lose the existing native element during static
array copy-back, or accept an incompatible COM object into a class-typed
array. These cases can produce incorrect values or bypass the managed
array's element-type constraint.

Restore non-coercing replacement for VARIANT array elements, mark
existing static SAFEARRAY contents valid during copy-back, and retain
the managed class element type while marshaling through its COM
interface. Add native and managed regression cases for all three
scenarios.

Validated with a checked CoreCLR build, the complete
`SafeArrayMarshallingTest` class (9 passed), and the native Dispatch
test. The broader Interop run had unrelated missing test executables and
DisabledRuntimeMarshalling layout files.

Resolves #134579
Resolves #134580
Resolves #134581
Resolves #134574

> [!NOTE]
> This pull request description was generated by GitHub Copilot.

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 58746a48-2bb6-476b-9993-639ef190f89d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment