Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Don't trim IMarshal #32491

Merged
merged 1 commit into from
Sep 26, 2018
Merged

Don't trim IMarshal #32491

merged 1 commit into from
Sep 26, 2018

Conversation

ericstj
Copy link
Member

@ericstj ericstj commented Sep 26, 2018

The linker was trimming out IMarshal because it saw the interface was never used by managed code.

This does get used by COM and the interface being present but empty breaks native code when it attempts to call an method that is missing from the interface.

/cc @luqunl @Tanya-Solyanik

The linker was trimming out IMarshal because it saw the interface was never used by managed code.

This does get used by COM and the interface being present but empty breaks native code when it attempts to call an method that is missing from the interface.
@ericstj ericstj added this to the 3.0 milestone Sep 26, 2018
@ericstj ericstj self-assigned this Sep 26, 2018
@ericstj
Copy link
Member Author

ericstj commented Sep 26, 2018

It strikes me that we'd probably have discovered this if we had some simple testcases to test the functionality of StandardOleMarshal. I can see about adding those.

Copy link
Member

@danmoseley danmoseley left a comment

Choose a reason for hiding this comment

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

I agree a test would probably be wise

@jkotas
Copy link
Member

jkotas commented Sep 26, 2018

Do we need to actually fix the linker to not trim [ComImport] interfaces? There are number of other [ComInterfaces] used throughout the CoreFX that may be hitting similar issue.

cc @sbomer

@ericstj
Copy link
Member Author

ericstj commented Sep 26, 2018

@jkotas, yes, I'm typing up that issue now. Also I did a new pass at looking at the linker diffs to find more of these. Working on uploading those reports to azure so I can link them in the deadcode issue.

@ericstj ericstj merged commit 2bb6824 into dotnet:master Sep 26, 2018
@ericstj
Copy link
Member Author

ericstj commented Sep 26, 2018

To circle back, I reviewed all the [ComImport] interfaces and found that this appears to be the only one trimmed.

We have a lot in S.R.WindowsRuntime which will get wacked if I force trimming on that assembly, but we're already opting out due to this:

corefx/illink.targets

Lines 56 to 57 in 0b0f63d

<!-- Currently ILLink cannot handle type projections from Windows.winmd, disable if the project references it -->
<ILLinkTrimAssembly Condition="'%(ReferencePath.FileName)%(ReferencePath.Extension)' == 'Windows.winmd'">false</ILLinkTrimAssembly>

picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants