145429 2015-05-27 22:58:34 -0700 Crash under ICU with ASAN during editing/selection/move-by-word-visually-crash-test-5.html 2015-05-28 21:31:00 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 mmaxfield mmaxfield ap webkit-bug-importer oldest_to_newest 1097846 0 mmaxfield 2015-05-27 22:58:34 -0700 Crash under ICU with ASAN during editing/selection/move-by-word-visually-crash-test-5.html 1097849 1 253836 mmaxfield 2015-05-27 23:12:38 -0700 Created attachment 253836 Patch 1097852 2 mmaxfield 2015-05-27 23:13:45 -0700 <rdar://problem/20992218> 1097915 3 253836 ap 2015-05-28 11:16:05 -0700 Comment on attachment 253836 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=253836&action=review > Source/WebCore/platform/text/icu/UTextProviderLatin1.cpp:108 > - uText->chunkOffset = uText->chunkLength; > + uText->chunkOffset = static_cast<int32_t>(index - uText->chunkNativeStart); > return FALSE; Did you test both code paths? It's not obvious to me what the expectation is when FALSE is returned. Notably, the "Already at the beginning; can't go any farther" case below sets the offset to 0. 1097920 4 253836 mmaxfield 2015-05-28 11:54:02 -0700 Comment on attachment 253836 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=253836&action=review >> Source/WebCore/platform/text/icu/UTextProviderLatin1.cpp:108 >> return FALSE; > > Did you test both code paths? It's not obvious to me what the expectation is when FALSE is returned. > > Notably, the "Already at the beginning; can't go any farther" case below sets the offset to 0. The documentation states "Returns True if the requested index could be accessed. The chunk will contain the requested text. False value if a chunk cannot be accessed." According to the code, chunkOffset seems to be disregarded in the False case. I updated the code here for consistency with below. 1097982 5 mmaxfield 2015-05-28 15:41:42 -0700 Committed r184965: <http://trac.webkit.org/changeset/184965> 1098038 6 ap 2015-05-28 18:07:56 -0700 This broke an API test: https://build.webkit.org/builders/Apple%20Yosemite%20Release%20WK2%20(Tests)?numbuilds=50 FAIL WebKit1.StringTruncator /Volumes/Data/slave/yosemite-release/build/Tools/TestWebKitAPI/Tests/mac/StringTruncator.mm:36 Value of: "abcdef…tuvwxyz" Expected: [[WebStringTruncator centerTruncateString:@"abcdefghijklmnopqrstuvwxyz" toWidth:100 withFont:[NSFont fontWithName:@"Helvetica" size:12]] UTF8String] Which is: "abcdefg…tuvwxyz" 1098060 7 ap 2015-05-28 21:07:12 -0700 I landed the new results in <http://trac.webkit.org/r184975>. However, I don't know if the new results are correct, or if they are telling us about an off by one mistake in this patch. Myles, could you please take a look? 1098062 8 mmaxfield 2015-05-28 21:25:16 -0700 (In reply to comment #7) > I landed the new results in <http://trac.webkit.org/r184975>. However, I > don't know if the new results are correct, or if they are telling us about > an off by one mistake in this patch. > > Myles, could you please take a look? Verified that this is a progression. 1098063 9 mmaxfield 2015-05-28 21:31:00 -0700 You beat me! r184976 253836 2015-05-27 23:12:38 -0700 2015-05-28 11:16:05 -0700 Patch bug-145429-20150527231226.patch text/plain 3350 mmaxfield U3VidmVyc2lvbiBSZXZpc2lvbjogMTg0OTE1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggOWVlMzdkZmY2YWQzZDQz ZGZlNzA2MzFkNzkyZGJhMjIwMzJiODVmMC4uZGE4NTRiNzNmODQ5YmFjYmVkMGVhN2UwNDg5NWI0 M2I5ZDA2ZDM5OSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDMyIEBACisyMDE1LTA1LTI3ICBNeWxl cyBDLiBNYXhmaWVsZCAgPG1tYXhmaWVsZEBhcHBsZS5jb20+CisKKyAgICAgICAgQ3Jhc2ggdW5k ZXIgSUNVIHdpdGggQVNBTiBkdXJpbmcgZWRpdGluZy9zZWxlY3Rpb24vbW92ZS1ieS13b3JkLXZp c3VhbGx5LWNyYXNoLXRlc3QtNS5odG1sCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3Jn L3Nob3dfYnVnLmNnaT9pZD0xNDU0MjkKKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzIwOTkyMjE4 PgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFdlYktp dCB1c2VzIHNvbWUgc3RyaW5ncyB3aGljaCBjb250YWluIHRoZSBsb3dlciA4LWJpdHMgb2YgVVRG LTE2ICh0aGVyZWJ5IHNhdmluZyBzcGFjZSkuIEhvd2V2ZXIsCisgICAgICAgIElDVSBkb2Vzbid0 IHVuZGVyc3RhbmQgdGhpcyBlbmNvZGluZy4gV2hlbiB3ZSB3YW50IHRvIHVzZSBJQ1UgZnVuY3Rp b25zIHdpdGggc3RyaW5ncyBpbiB0aGlzIGVuY29kaW5nLAorICAgICAgICB3ZSBjcmVhdGUgYSBV VGV4dFByb3ZpZGVyIHdoaWNoIGNvbnZlcnRzIG91ciBlbmNvZGVkIHN0cmluZ3MgdG8gVVRGLTE2 IGZvciBJQ1UsIG9uZSBjaHVuayBhdCBhIHRpbWUuCisgICAgICAgIFRoaXMgb2JqZWN0IGNvbnRh aW5zIGEgdnRhYmxlIHdoaWNoIHdlIHBvcHVsYXRlIHRvIHBlcmZvcm0gdGhlIGNvbnZlcnNpb24u CisKKyAgICAgICAgVGhlIFdlYktpdCBmdW5jdGlvbiB3aGljaCBhY3R1YWxseSByZXR1cm5zIHRo ZSBVVEYtMTYgY2h1bmtzIGhhcyB0d28gcmVsZXZhbnQgYXJndW1lbnRzOiBhbiBpbmRleCBpbnRv CisgICAgICAgIHRoZSBlbmNvZGVkIHN0cmluZyB3aGljaCBJQ1UgaXMgcmVxdWVzdGluZywgYW5k IGEgZGlyZWN0aW9uIGZyb20gdGhhdCBpbmRleCB3aGljaCBJQ1UgaXMgaW50ZXJlc3RlZAorICAg ICAgICBpbi4gVGhpcyBmdW5jdGlvbiBwb3B1bGF0ZXMgYSAiY2h1bmsiIHdoaWNoIGlzIGNoYXJh Y3Rlcml6ZWQgYnkgYSBwb2ludGVyIHRvIGEgYnVmZmVyLCB0aGUgbGVuZ3RoIG9mCisgICAgICAg IHRoZSBwb3B1bGF0ZWQgZGF0YSBpbiB0aGUgYnVmZmVyLCBhbmQgYW4gb2Zmc2V0IGludG8gdGhl IGNodW5rIHdoaWNoIHJlcHJlc2VudHMgdGhlIGluZGV4IHRoYXQgdGhlCisgICAgICAgIHJlcXVl c3RlZCBjaGFyYWN0ZXIgd2FzIHB1dCBpbnRvLgorCisgICAgICAgIFdoZW4gSUNVIHJlcXVlc3Rz IGRhdGEgZ29pbmcgYmFja3dhcmQsIHdlIGZpbGwgaW4gdGhlIGNodW5rIGFjY29yZGluZ2x5LCB3 aXRoIHRoZSByZXF1ZXN0ZWQgY2hhcmFjdGVyCisgICAgICAgIGFsbCB0aGUgd2F5IGF0IHRoZSBl bmQuIFdlIHRoZW4gc2V0IHRoZSBvZmZzZXQgZXF1YWwgdG8gdGhlIGxlbmd0aCBvZiB0aGUgYnVm ZmVyLiBIb3dldmVyLCB0aGlzIGxlbmd0aAorICAgICAgICB2YWx1ZSBpcyBzdGFsZSBmcm9tIHRo ZSBwcmV2aW91cyB0aW1lIHRoZSBmdW5jdGlvbiByYW4uIFRoZXJlZm9yZSwgSUNVIHdhcyByZWFk aW5nIHRoZSB3cm9uZyBpbmRleCBpbgorICAgICAgICB0aGUgY2h1bmsgd2hlbiBleHBlY3Rpbmcg dGhlIHJlcXVlc3RlZCBjaGFyYWN0ZXIuCisKKyAgICAgICAgQ292ZXJlZCBieSBlZGl0aW5nL3Nl bGVjdGlvbi9tb3ZlLWJ5LXdvcmQtdmlzdWFsbHktY3Jhc2gtdGVzdC01Lmh0bWwuCisKKyAgICAg ICAgKiBwbGF0Zm9ybS90ZXh0L2ljdS9VVGV4dFByb3ZpZGVyTGF0aW4xLmNwcDoKKyAgICAgICAg KFdlYkNvcmU6OnVUZXh0TGF0aW4xQWNjZXNzKToKKwogMjAxNS0wNS0yNyAgRXJpYyBDYXJsc29u ICA8ZXJpYy5jYXJsc29uQGFwcGxlLmNvbT4KIAogICAgICAgICBbTWFjXSBzaG9ydC1jaXJjdWl0 IE1JTUUgdHlwZSBsb29rdXAgd2hlbiBwb3NzaWJsZQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNv cmUvcGxhdGZvcm0vdGV4dC9pY3UvVVRleHRQcm92aWRlckxhdGluMS5jcHAgYi9Tb3VyY2UvV2Vi Q29yZS9wbGF0Zm9ybS90ZXh0L2ljdS9VVGV4dFByb3ZpZGVyTGF0aW4xLmNwcAppbmRleCA3NDhk MmE1NGZhZjhmMDVhOTc5YjhhNmZhMTc3MjQ0MjczMTRiMmQ3Li5jZDY4NTJjNzFlNTVlZjk3Y2Ew YjBiYTFjZmY4ODBiOTk3MGY3M2M1IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9y bS90ZXh0L2ljdS9VVGV4dFByb3ZpZGVyTGF0aW4xLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9w bGF0Zm9ybS90ZXh0L2ljdS9VVGV4dFByb3ZpZGVyTGF0aW4xLmNwcApAQCAtMTA0LDcgKzEwNCw3 IEBAIHN0YXRpYyBVQm9vbCB1VGV4dExhdGluMUFjY2VzcyhVVGV4dCogdVRleHQsIGludDY0X3Qg aW5kZXgsIFVCb29sIGZvcndhcmQpCiAgICAgICAgIH0KICAgICAgICAgaWYgKGluZGV4ID49IGxl bmd0aCAmJiB1VGV4dC0+Y2h1bmtOYXRpdmVMaW1pdCA9PSBsZW5ndGgpIHsKICAgICAgICAgICAg IC8vIE9mZiB0aGUgZW5kIG9mIHRoZSBidWZmZXIsIGJ1dCB3ZSBjYW4ndCBnZXQgaXQuCi0gICAg ICAgICAgICB1VGV4dC0+Y2h1bmtPZmZzZXQgPSB1VGV4dC0+Y2h1bmtMZW5ndGg7CisgICAgICAg ICAgICB1VGV4dC0+Y2h1bmtPZmZzZXQgPSBzdGF0aWNfY2FzdDxpbnQzMl90PihpbmRleCAtIHVU ZXh0LT5jaHVua05hdGl2ZVN0YXJ0KTsKICAgICAgICAgICAgIHJldHVybiBGQUxTRTsKICAgICAg ICAgfQogICAgIH0gZWxzZSB7CkBAIC0xMzYsNyArMTM2LDcgQEAgc3RhdGljIFVCb29sIHVUZXh0 TGF0aW4xQWNjZXNzKFVUZXh0KiB1VGV4dCwgaW50NjRfdCBpbmRleCwgVUJvb2wgZm9yd2FyZCkK ICAgICAgICAgaWYgKHVUZXh0LT5jaHVua05hdGl2ZVN0YXJ0IDwgMCkKICAgICAgICAgICAgIHVU ZXh0LT5jaHVua05hdGl2ZVN0YXJ0ID0gMDsKIAotICAgICAgICB1VGV4dC0+Y2h1bmtPZmZzZXQg PSB1VGV4dC0+Y2h1bmtMZW5ndGg7CisgICAgICAgIHVUZXh0LT5jaHVua09mZnNldCA9IHN0YXRp Y19jYXN0PGludDMyX3Q+KGluZGV4IC0gdVRleHQtPmNodW5rTmF0aXZlU3RhcnQpOwogICAgIH0K ICAgICB1VGV4dC0+Y2h1bmtMZW5ndGggPSBzdGF0aWNfY2FzdDxpbnQzMl90Pih1VGV4dC0+Y2h1 bmtOYXRpdmVMaW1pdCAtIHVUZXh0LT5jaHVua05hdGl2ZVN0YXJ0KTsKIAo=