Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release/7.0] Use System.Numerics.IEqualityOperators.op_Equality in SpanHelper.T.cs where possible. #74738

Merged
merged 2 commits into from
Aug 29, 2022

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Aug 29, 2022

Backport of #74567 to release/7.0

/cc @lateralusX

Fixes #74179.

Re-enable tests previously disabled by #74454.

Customer Impact

Testing

Re-enable failing tests as part of fix. Full test suites run and pass on main with tests re-enabled and fix implemented.

Risk

Low.

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

Workaround crash hit by #74179
making sure we avoid hitting codepath emitting this null pointer checks.
The full fix includes codegen fixes as well, but will be performed
in separate PR. There are still locations in SpanHelper.T.cs that uses
Equal virtual call on value types that could be managed pointers to
value types, but that code has remained the same for the last
4 years to 15 months and have not hit this issue in the past.
@ghost
Copy link

ghost commented Aug 29, 2022

Tagging subscribers to this area: @dotnet/area-system-globalization
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #74567 to release/7.0

/cc @lateralusX

Customer Impact

Testing

Risk

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

Author: github-actions[bot]
Assignees: -
Labels:

area-System.Globalization

Milestone: -

@ghost
Copy link

ghost commented Aug 29, 2022

Tagging subscribers to this area: @dotnet/area-system-memory
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #74567 to release/7.0

/cc @lateralusX

Fixes #74179.

Re-enable tests previously disabled by #74454.

Customer Impact

Testing

Re-enable failing tests as part of fix. Full test suites run and pass on main with tests re-enabled and fix implemented.

Risk

Low.

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

Author: github-actions[bot]
Assignees: -
Labels:

area-System.Memory

Milestone: -

@danmoseley
Copy link
Member

Approved for RC2 - basic functionality regressed. Will customers notice this in RC1 ?

@danmoseley danmoseley merged commit 099304f into release/7.0 Aug 29, 2022
@danmoseley danmoseley deleted the backport/pr-74567-to-release/7.0 branch August 29, 2022 18:33
@lateralusX
Copy link
Member

lateralusX commented Aug 29, 2022

Approved for RC2 - basic functionality regressed. Will customers notice this in RC1 ?

I think we can leave RC1 as is for now, in special cases code might read pass allocated buffer by max 3 bytes, so only if buffer is last on allocated page and next page is either unallocated or explicitly marked without read access this will crash in case were runtime runs with implicit null checks.

@ghost ghost locked as resolved and limited conversation to collaborators Sep 29, 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.

4 participants