Summary: | Crash under ICU with ASAN during editing/selection/move-by-word-visually-crash-test-5.html | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Myles C. Maxfield <mmaxfield> | ||||
Component: | New Bugs | Assignee: | Myles C. Maxfield <mmaxfield> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | ap, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Myles C. Maxfield
2015-05-27 22:58:34 PDT
Created attachment 253836 [details]
Patch
Comment on attachment 253836 [details] 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. Comment on attachment 253836 [details] 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. Committed r184965: <http://trac.webkit.org/changeset/184965> 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" 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? (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. |