Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Enable clearing initlocals when running dotnet msbuild.#27345

Merged
stephentoub merged 2 commits into
dotnet:masterfrom
erozenfeld:ClearInitLocalsCore
Feb 23, 2018
Merged

Enable clearing initlocals when running dotnet msbuild.#27345
stephentoub merged 2 commits into
dotnet:masterfrom
erozenfeld:ClearInitLocalsCore

Conversation

@erozenfeld
Copy link
Copy Markdown
Member

Linker step to clear initlocals was only enabled in Windows builds that use
desktop msbuild. dotnet msbuild has a bug in assembly loading: dotnet/msbuild#3016
that was preventing ILLink.CustomSteps from loading by a Type.GetType() call. This PR adds a workaround by appending ILLink.CustomSteps version to the string that is eventually passed to Type.GetType().

Fixes #27045.

@ericstj @stephentoub PTAL

Linker step to clear initlocals was only enabled in Windows builds that use
desktop msbuild. dotnet msbuild has a bug in assembly loading: dotnet/msbuild#3016
that was preventing ILLink.CustomSteps from loading by a Type.GetType() call. This PR adds a workaround by
appending ILLink.CustomSteps version to the string that is eventually passed to Type.GetType().
@erozenfeld
Copy link
Copy Markdown
Member Author

@dotnet-bot test Linux x64 Release Build

@danmoseley
Copy link
Copy Markdown
Member

Yay for teamwork on this one!

@erozenfeld
Copy link
Copy Markdown
Member Author

@dotnet-bot test Linux x64 Release Build

@stephentoub
Copy link
Copy Markdown
Member

@dotnet-bot test Outerloop Linux x64 Release Build

Copy link
Copy Markdown
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

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

Thanks!

@stephentoub
Copy link
Copy Markdown
Member

Some of these Linux failures look legit. In particular, the System.Net.Sockets failures are all in tests related to ReceiveMessageFrom, and ReceiveMessageFrom uses a few stackallocs (that said, it's not obvious to me why those stackalloc'd bytes would need to be cleared, as they're meant to be used as output space, so maybe that's not actually it).

For now, I'm going to run the tests again, and then assuming they still fail, let's make the setting of ILLinkClearInitLocals in System.Net.Sockets conditional on it being the Windows build, and I can investigate subsequently this particular failure.

@stephentoub
Copy link
Copy Markdown
Member

@dotnet-bot test Outerloop Linux x64 Release Build please

@erozenfeld
Copy link
Copy Markdown
Member Author

@stephentoub What about System.Security.Cryptography.Pkcs.Tests
and System.Net.WebSockets.Client.Tests failures? Are they unrelated?

@stephentoub
Copy link
Copy Markdown
Member

The System.Diagnostics.Process failures are all https://github.com/dotnet/corefx/issues/27365; unrelated.

The System.Runtime.Environment failures are all https://github.com/dotnet/corefx/issues/27366; unrelated.

The System.Security.Cryptography.Pkcs failures were just fixed by #27356; unrelated.

The System.Net.WebSockets.Client failure isn't known, but I don't believe it's related. It looks like it falls into the larger bucket of failures we know about where we have transient issues connecting to a remote endpoint in the cloud. Still... probably worth re-running just to see if it shows up again.

@stephentoub
Copy link
Copy Markdown
Member

@dotnet-bot test Outerloop Linux x64 Release Build please please

@erozenfeld
Copy link
Copy Markdown
Member Author

I pushed a change to make the setting of ILLinkClearInitLocals in System.Net.Sockets conditional on it being the Windows build.

@erozenfeld
Copy link
Copy Markdown
Member Author

@dotnet-bot test Outerloop Linux x64 Release Build please please

@erozenfeld
Copy link
Copy Markdown
Member Author

@stephentoub Are System.Net.Http.Functional failures known?

@erozenfeld
Copy link
Copy Markdown
Member Author

@dotnet-bot test Linux x64 Release Build

@stephentoub
Copy link
Copy Markdown
Member

Are System.Net.Http.Functional failures known?

Yeah, those are https://github.com/dotnet/corefx/issues?utf8=%E2%9C%93&q=is%3Aissue+is%3Aopen+GetAsync_AllowedSSLVersion_Succeeds.

This looks good. Thanks!

@stephentoub stephentoub merged commit 173d9e6 into dotnet:master Feb 23, 2018
@karelz karelz added this to the 2.1.0 milestone Mar 10, 2018
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
…x#27345)

* Enable clearing initlocals when running dotnet msbuild.

Linker step to clear initlocals was only enabled in Windows builds that use
desktop msbuild. dotnet msbuild has a bug in assembly loading: dotnet/msbuild#3016
that was preventing ILLink.CustomSteps from loading by a Type.GetType() call. This PR adds a workaround by
appending ILLink.CustomSteps version to the string that is eventually passed to Type.GetType().

* Make the setting of ILLinkClearInitLocals in System.Net.Sockets conditional on it being the Windows build.


Commit migrated from dotnet/corefx@173d9e6
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants