-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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] Fix bug in String.Equals unrolling for CJK single-char strings #78234
[release/7.0] Fix bug in String.Equals unrolling for CJK single-char strings #78234
Conversation
@EgorBo when this is ready, please send the email to Tactics, add the |
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsBackport of #78190 to release/7.0 Customer ImpactFixes #78169 .NET 7.0 regression introduced in the String.Equals unrolling optimization. const String v1 = "鉄";
var v2 = v1;
Console.WriteLine($"{v1 == v2}, {v1.Equals(v2, StringComparison.Ordinal)}, {v1.Equals(v2,StringComparison.OrdinalIgnoreCase)}"); Prints different values depending on code being optimized or not (by jit). All comparisons against single-char literals of certain symbols of CJK languages are affected so we probably want to land the fix it as soon as possible. TestingWe had a pretty big test coverage for all sorts of inputs for that optimizations, unfortunately, for the non-ASCII one we didn't use any of CJK symbols. Added CJK and did a fuzzy testing locally via a hand-written test generator for this API. RiskLow
|
@jakobbotsch PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approved. we will take for consideration in 7.0.x
Approved by Tactics via email. |
Manual backport of #78190 to release/7.0
Customer Impact
Fixes #78169 regression introduced in the String.Equals/StartsWith unrolling optimization in NET 7.0.
Prints different values depending on code being optimized or not (by jit). All comparisons against single-char literals of certain symbols (all two-bytes chars which pass this check:
bool IsAffected(char c) => (((ushort)c) >> 15) == 1;
) of CJK languages are affected so we probably want to land the fix it as soon as possible.Testing
We had a pretty big test coverage for all sorts of inputs for that optimizations, unfortunately, for the non-ASCII one we didn't use any of CJK symbols. Added CJK and did a fuzzy testing locally via a hand-written test generator for this API.
Risk
Low