Skip to content

[release/6.0] [mono] Use unsigned char when computing UTF8 string hashes - #83303

Merged
lambdageek merged 2 commits into
release/6.0-stagingfrom
backport/pr-83302-to-release/6.0
Mar 30, 2023
Merged

[release/6.0] [mono] Use unsigned char when computing UTF8 string hashes#83303
lambdageek merged 2 commits into
release/6.0-stagingfrom
backport/pr-83302-to-release/6.0

Conversation

@github-actions

@github-actionsgithub-actionsBot commented Mar 11, 2023

Copy link
Copy Markdown
Contributor

Backport of #83273 to release/6.0

Resolves#82187
Resolves#78638

The corresponding 7.0 PR is #83302

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.

@ghostghost added the area-VM-meta-mono label Mar 11, 2023
@lambdageeklambdageek changed the title [release/6.0] [release/7.0][mono] Use unsigned char when computing UTF8 string hashes[release/6.0] [mono] Use unsigned char when computing UTF8 string hashesMar 11, 2023
@lambdageeklambdageek added area-Codegen-AOT-mono Servicing-consider Issue for next servicing release review and removed area-VM-meta-mono labels Mar 11, 2023
@lambdageeklambdageek added this to the 6.0.x milestone Mar 11, 2023
@github-actions
github-actionsBotforce-pushed the backport/pr-83302-to-release/6.0 branch from 94292ee to 7b786c2CompareMarch 13, 2023 14:35
@carlossanlop

carlossanlop commented Mar 13, 2023

Copy link
Copy Markdown
Contributor

Hello - Today is code-complete day for merging changes that should go into the April servicing release. Todos:

  • Please make sure to fill out the template making sure to describe the customer impact.
  • Send an email to Tactics requesting approval.
  • Help confirm that the CI failures are unrelated to this change.

If it's not critical to get it merged in the April release, we can wait until next month.

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#82187Fixes#78638
@lambdageek
lambdageekforce-pushed the backport/pr-83302-to-release/6.0 branch from 0e4ca74 to e6dd087CompareMarch 14, 2023 14:24
@leecowleecow added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Mar 14, 2023
@leecowleecow modified the milestones: 6.0.x, 6.0.16Mar 14, 2023
@lambdageek
lambdageek changed the base branch from release/6.0 to release/6.0-stagingMarch 28, 2023 18:25
@carlossanlop

Copy link
Copy Markdown
Contributor

@lambdageek not sure if you saw but the new check-service-labels CI leg was stuck because there were some weird problems with GitHub Actions yesterday. The fix is to remove and readd the Servicing-approved label, and the check will get retriggered (and should pass). I did that, and you are now allowed to merge if the CI results look good to you.

@lambdageeklambdageek added Servicing-approved Approved for servicing release and removed Servicing-approved Approved for servicing release labels Mar 30, 2023
@lambdageek
lambdageek merged commit 0254cce into release/6.0-stagingMar 30, 2023
@lewing
lewing deleted the backport/pr-83302-to-release/6.0 branch March 30, 2023 19:15
@ghostghost locked as resolved and limited conversation to collaborators Apr 30, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-VM-meta-monoServicing-approvedApproved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@carlossanlop@vargaz@thaystg@lambdageek@leecow@teo-tsirpanis