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

Clear the init-locals bit for CoreLib to workaround #1279#13728

Merged
jkotas merged 2 commits into
dotnet:masterfrom
jkotas:zeroinit-hack
Sep 1, 2017
Merged

Clear the init-locals bit for CoreLib to workaround #1279#13728
jkotas merged 2 commits into
dotnet:masterfrom
jkotas:zeroinit-hack

Conversation

@jkotas
Copy link
Copy Markdown
Member

@jkotas jkotas commented Aug 31, 2017

No description provided.

@jkotas jkotas requested a review from erozenfeld August 31, 2017 20:22
@jkotas
Copy link
Copy Markdown
Member Author

jkotas commented Aug 31, 2017

cc @stephentoub @erozenfeld

stephentoub
stephentoub reviewed Aug 31, 2017
View reviewed changes
Comment thread src/mscorlib/src/System/RtType.cs
int numVirtuals = RuntimeTypeHandle.GetNumVirtuals(declaringType);

bool* overrides = stackalloc bool[numVirtuals];
new Span<bool>(overrides, numVirtuals).Clear();
Copy link
Copy Markdown
Member

@stephentoub stephentoub Aug 31, 2017

Choose a reason for hiding this comment

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

I'd previously tried running all of the corefx tests with that hack to disable init locals, and I'd seen maybe three or four tests fail total... I assume this is the fix for those?

Copy link
Copy Markdown
Member Author

@jkotas jkotas Aug 31, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, this place in CoreLib used to depend on stackalloced memory to be zero-initialized. I have just found a second one in TraceLoggingEventSource.cs.

stephentoub
stephentoub approved these changes Aug 31, 2017
View reviewed changes
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, Jan.

@jkotas
Copy link
Copy Markdown
Member Author

jkotas commented Aug 31, 2017

fatal error: error in backend: IO failure on output stream.

@jkotas
Copy link
Copy Markdown
Member Author

jkotas commented Aug 31, 2017

@dotnet-bot test this please

@jkotas jkotas closed this Aug 31, 2017
@jkotas jkotas reopened this Aug 31, 2017
@jkotas
Copy link
Copy Markdown
Member Author

jkotas commented Aug 31, 2017

@dotnet-bot test Ubuntu16.04 arm Cross Debug Build please

@erozenfeld erozenfeld mentioned this pull request Aug 31, 2017
Merged
@jkotas
Copy link
Copy Markdown
Member Author

jkotas commented Sep 1, 2017

@dotnet-bot test Ubuntu x64 Checked Build and Test please

1 similar comment
@jkotas
Copy link
Copy Markdown
Member Author

jkotas commented Sep 1, 2017

@dotnet-bot test Ubuntu x64 Checked Build and Test please

erozenfeld
erozenfeld approved these changes Sep 1, 2017
View reviewed changes
Copy link
Copy Markdown
Member

@erozenfeld erozenfeld left a comment

Choose a reason for hiding this comment

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

LGTM

@jkotas jkotas merged commit 707de9c into dotnet:master Sep 1, 2017
@jkotas jkotas deleted the zeroinit-hack branch September 1, 2017 17:58
@brianrob brianrob mentioned this pull request Feb 7, 2018
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@stephentoub stephentoub stephentoub approved these changes

@erozenfeld erozenfeld erozenfeld approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jkotas @stephentoub @erozenfeld @dnfclas