135380 2014-07-29 04:36:16 -0700 [ARM] Incorrect handling of Unicode characters 2014-08-21 01:59:12 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 108645 1 dbatyai.u-szeged webkit-unassigned bunhere commit-queue darin gyuyoung.kim llango.u-szeged mark.lam ossy rakuco sergio oldest_to_newest 1025433 0 dbatyai.u-szeged 2014-07-29 04:36:16 -0700 In c++ the signedness of char can be implementation dependent. Unfortunately, this caused the incorrect handling of Unicode characters in JavaScriptCore on ARM, since the code in stringFromUTF8 in jsc.cpp (http://trac.webkit.org/browser/trunk/Source/JavaScriptCore/jsc.cpp#L658) assumes that char is signed, but that was not the case on ARM. 1025435 1 235676 dbatyai.u-szeged 2014-07-29 04:39:46 -0700 Created attachment 235676 Patch Force GCC to use signed char for char 1026883 2 235676 mark.lam 2014-08-05 10:54:16 -0700 Comment on attachment 235676 Patch I think the better fix is to make change stringFromUTF() in jsc.cpp to explicitly use a signed char since it is dependent on signed behavior for correctness. This ensures that the code is correct independent of build configurations. Are there other places where you’ve found the “sign”-ness of chars to be an issue? 1026995 3 235676 darin 2014-08-05 16:33:14 -0700 Comment on attachment 235676 Patch I think this change is OK, but I also think we should fix the code to not depend on char being signed. I don’t think we need to change the type to “signed char” — we can and should just remove the dependency. 1026997 4 darin 2014-08-05 16:36:11 -0700 Let me go further. We should remove the silly optimization in stringFromUTF. If we need a fast case for ASCII, that should be inside the fromUTF8WithLatin1Fallback function, not in the JSC tool. Please submit a patch that deletes the misguided “fast case” code from jsc.cpp. 1027152 5 236100 dbatyai.u-szeged 2014-08-06 07:23:12 -0700 Created attachment 236100 Patch Removed fast case, as suggested. 1027191 6 236100 commit-queue 2014-08-06 10:27:57 -0700 Comment on attachment 236100 Patch Clearing flags on attachment: 236100 Committed r172152: <http://trac.webkit.org/changeset/172152> 1027192 7 commit-queue 2014-08-06 10:28:01 -0700 All reviewed patches have been landed. Closing bug. 235676 2014-07-29 04:39:46 -0700 2014-08-06 07:14:49 -0700 Patch patch text/plain 1378 dbatyai.u-szeged ZGlmZiAtLWdpdCBhL0NoYW5nZUxvZyBiL0NoYW5nZUxvZwppbmRleCBkMTI2MTQzLi5kZGVhYTIy IDEwMDY0NAotLS0gYS9DaGFuZ2VMb2cKKysrIGIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTQgQEAK KzIwMTQtMDctMjkgIETDoW5pZWwgQsOhdHlhaSAgPGRiYXR5YWkudS1zemVnZWRAcGFydG5lci5z YW1zdW5nLmNvbT4KKworICAgICAgICBbQVJNXSBBbWJpZ3VvdXMgaW1wbGVtZW50YXRpb24gb2Yg Y2hhcgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTM1 MzgwCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgRm9y Y2UgR0NDIHRvIHVzZSBzaWduZWQgY2hhciBmb3IgY2hhcgorCisgICAgICAgICogU291cmNlL2Nt YWtlL09wdGlvbnNDb21tb24uY21ha2U6CisKIDIwMTQtMDctMjUgIE1pY2hhZWwgQ2F0YW56YXJv ICA8bWNhdGFuemFyb0BpZ2FsaWEuY29tPgogCiAgICAgICAgIFtHVEtdIENNYWtlIHRyaWVzIHRv IGluc3RhbGwgSmF2YVNjcmlwdENvcmUtMy4wLmdpciBvdXRzaWRlIG9mIGluc3RhbGwgcHJlZml4 CmRpZmYgLS1naXQgYS9Tb3VyY2UvY21ha2UvT3B0aW9uc0NvbW1vbi5jbWFrZSBiL1NvdXJjZS9j bWFrZS9PcHRpb25zQ29tbW9uLmNtYWtlCmluZGV4IGUzMDRhOTkuLjkxNzA4MmMgMTAwNjQ0Ci0t LSBhL1NvdXJjZS9jbWFrZS9PcHRpb25zQ29tbW9uLmNtYWtlCisrKyBiL1NvdXJjZS9jbWFrZS9P cHRpb25zQ29tbW9uLmNtYWtlCkBAIC0yNSw4ICsyNSw4IEBAIGVuZGlmICgpCiBzZXRfcHJvcGVy dHkoR0xPQkFMIFBST1BFUlRZIFVTRV9GT0xERVJTIE9OKQogCiBpZiAoQ01BS0VfQ09NUElMRVJf SVNfR05VQ1hYIE9SICIke0NNQUtFX0NYWF9DT01QSUxFUl9JRH0iIFNUUkVRVUFMICJDbGFuZyIp Ci0gICAgc2V0KENNQUtFX0NfRkxBR1MgIiR7Q01BS0VfQ1hYX0ZMQUdTfSAtZm5vLWV4Y2VwdGlv bnMgLWZuby1zdHJpY3QtYWxpYXNpbmciKQotICAgIHNldChDTUFLRV9DWFhfRkxBR1MgIiR7Q01B S0VfQ1hYX0ZMQUdTfSAtc3RkPWMrKzExIC1mbm8tZXhjZXB0aW9ucyAtZm5vLXN0cmljdC1hbGlh c2luZyAtZm5vLXJ0dGkiKQorICAgIHNldChDTUFLRV9DX0ZMQUdTICIke0NNQUtFX0NYWF9GTEFH U30gLWZuby1leGNlcHRpb25zIC1mbm8tc3RyaWN0LWFsaWFzaW5nIC1mc2lnbmVkLWNoYXIiKQor ICAgIHNldChDTUFLRV9DWFhfRkxBR1MgIiR7Q01BS0VfQ1hYX0ZMQUdTfSAtc3RkPWMrKzExIC1m bm8tZXhjZXB0aW9ucyAtZm5vLXN0cmljdC1hbGlhc2luZyAtZm5vLXJ0dGkgLWZzaWduZWQtY2hh ciIpCiBlbmRpZiAoKQogCiBvcHRpb24oREVCVUdfRklTU0lPTiAiVXNlIERlYnVnIEZpc3Npb24g c3VwcG9ydCIpCg== 236100 2014-08-06 07:23:12 -0700 2014-08-06 10:27:57 -0700 Patch patch.patch text/plain 1691 dbatyai.u-szeged ZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cgYi9Tb3VyY2UvSmF2 YVNjcmlwdENvcmUvQ2hhbmdlTG9nCmluZGV4IGJiYjEwOTAuLjcxNzAwNmQgMTAwNjQ0Ci0tLSBh L1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL0phdmFTY3JpcHRD b3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2IEBACisyMDE0LTA4LTA2ICBEw6FuaWVsIELDoXR5 YWkgIDxkYmF0eWFpLnUtc3plZ2VkQHBhcnRuZXIuc2Ftc3VuZy5jb20+CisKKyAgICAgICAgW0FS TV0gSW5jb3JyZWN0IGhhbmRsaW5nIG9mIFVuaWNvZGUgY2hhcmFjdGVycworICAgICAgICBodHRw czovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTM1MzgwCisKKyAgICAgICAgUmV2 aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgUmVtb3ZlZCBlcnJvbmVvdXMgZmFz dCBjYXNlIGZyb20gc3RyaW5nRnJvbVVURigpLCBzaW5jZSBpdCBhc3N1bWVkIHRoYXQgCisgICAg ICAgIGNoYXIgaXMgYWx3YXlzIGltcGxlbWVudGVkIGFzIHNpZ25lZC4KKworICAgICAgICAqIGpz Yy5jcHA6CisgICAgICAgIChzdHJpbmdGcm9tVVRGKToKKwogMjAxNC0wOC0wNSAgQ3NhYmEgT3N6 dHJvZ29uw6FjICA8b3NzeUB3ZWJraXQub3JnPgogCiAgICAgICAgIFVSVEJGIGFmdGVyIHIxNzIx MjkuIChmdGxvcHQgYnJhbmNoIG1lcmdlKQpkaWZmIC0tZ2l0IGEvU291cmNlL0phdmFTY3JpcHRD b3JlL2pzYy5jcHAgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvanNjLmNwcAppbmRleCA5NTc3NDgw Li5jYjI2NzcwIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvanNjLmNwcAorKysg Yi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvanNjLmNwcApAQCAtNjYxLDIwICs2NjEsNyBAQCBHbG9i YWxPYmplY3Q6Okdsb2JhbE9iamVjdChWTSYgdm0sIFN0cnVjdHVyZSogc3RydWN0dXJlKQogCiBz dGF0aWMgaW5saW5lIFN0cmluZyBzdHJpbmdGcm9tVVRGKGNvbnN0IGNoYXIqIHV0ZjgpCiB7Ci0g ICAgLy8gRmluZCB0aGUgdGhlIGZpcnN0IG5vbi1hc2NpaSBjaGFyYWN0ZXIsIG9yIG51bC4KLSAg ICBjb25zdCBjaGFyKiBwb3MgPSB1dGY4OwotICAgIHdoaWxlICgqcG9zID4gMCkKLSAgICAgICAg cG9zKys7Ci0gICAgc2l6ZV90IGFzY2lpTGVuZ3RoID0gcG9zIC0gdXRmODsKLSAgICAKLSAgICAv LyBGYXN0IGNhc2UgLSBzdHJpbmcgaXMgYWxsIGFzY2lpLgotICAgIGlmICghKnBvcykKLSAgICAg ICAgcmV0dXJuIFN0cmluZyh1dGY4LCBhc2NpaUxlbmd0aCk7Ci0gICAgCi0gICAgLy8gU2xvdyBj YXNlIC0gY29udGFpbnMgbm9uLWFzY2lpIGNoYXJhY3RlcnMsIHVzZSBmcm9tVVRGOFdpdGhMYXRp bjFGYWxsYmFjay4KLSAgICBBU1NFUlQoKnBvcyA8IDApOwotICAgIEFTU0VSVChzdHJsZW4odXRm OCkgPT0gYXNjaWlMZW5ndGggKyBzdHJsZW4ocG9zKSk7Ci0gICAgcmV0dXJuIFN0cmluZzo6ZnJv bVVURjhXaXRoTGF0aW4xRmFsbGJhY2sodXRmOCwgYXNjaWlMZW5ndGggKyBzdHJsZW4ocG9zKSk7 CisgICAgcmV0dXJuIFN0cmluZzo6ZnJvbVVURjhXaXRoTGF0aW4xRmFsbGJhY2sodXRmOCwgc3Ry bGVuKHV0ZjgpKTsKIH0KIAogc3RhdGljIGlubGluZSBTb3VyY2VDb2RlIGpzY1NvdXJjZShjb25z dCBjaGFyKiB1dGY4LCBjb25zdCBTdHJpbmcmIGZpbGVuYW1lKQo=