-
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][mono] Use unsigned char when computing UTF8 string hashes #83302
[release/7.0][mono] Use unsigned char when computing UTF8 string hashes #83302
Conversation
/backport to release/6.0 |
Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/4392646513 |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as resolved.
This comment was marked as resolved.
7bc56e9
to
5dada63
Compare
/backport to release/6.0 |
Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/4406095701 |
5dada63
to
e895dbb
Compare
/backport to release/6.0 |
Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/4407943691 |
Hey @lambdageek - Are you intending to get this approved for the April servicing release? Code-complete is today. |
@carlossanlop I don't think we'll make it. Let's try for the next one |
The C standard does not specify whether `char` is signed or unsigned, it is implementation defined. Apparently Android aarch64 makes a different choice than other platforms (at least macOS arm64 and Windows x64 give different results). Mono uses `mono_metadata_str_hash` in the AOT compiler and AOT runtime to optimize class name lookup. As a result, classes whose names include UTF-8 continuation bytes (with the high bit = 1) will hash differently in the AOT compiler and on the device. Fixes dotnet#82187 Fixes dotnet#78638
e895dbb
to
064e69d
Compare
I'm retargeting this PR to the new Repo maintainers will now be allowed to merge their own servicing PR as long as it meets the requirements:
The new process is described here: runtime/docs/project/library-servicing.md. The infra team will be actively monitoring servicing PRs to ensure all requirements are met and to help with any issues. Let me know if you have any questions. |
I see you already retargeted it, @lambdageek. You're awesome. |
Backport of #83273 to release/7.0
Resolves #82187
Resolves #78638
The corresponding 6.0 PR is #83303
Customer Impact
Resolves crashes in Release builds of Android apps that include classes with non-ASCII names that use AOT compilation.
Testing
Manual testing. New regression test.
Risk
Low. For code that uses ASCII names the hash code computation is unchanged.