Bug 140427 - Correct calculation of 16-bit text iterator decode offsets
Summary: Correct calculation of 16-bit text iterator decode offsets
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Brent Fulgham
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-01-13 20:49 PST by Brent Fulgham
Modified: 2015-01-14 15:47 PST (History)
1 user (show)

See Also:


Attachments
Patch (2.63 KB, patch)
2015-01-13 20:51 PST, Brent Fulgham
darin: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2015-01-13 20:49:54 PST
The TextCodecUTF8 and TextCodecLatin1 decoding routines have a calculation error in the update to the 'destination16' memory location. This was found by static analysis of the code.

The 'destination16' variable (in both files) is a pointer to a 16-bit character value, while the 'source' value is an 8-bit value.

We updated the 'source' pointer by incrementing it by the sizeof(MachineWord), which is the number of UTF8 characters we have consumed during the decode.

However, the 'destination16' variable is a UChar* (a 16-bit value). If we increment it by the number of bytes, that has the effect of moving us twice the number of 16-bit characters than we should be.

We should be incrementing by sizeof(MachineWord) / sizeof(UChar).
Comment 1 Brent Fulgham 2015-01-13 20:51:56 PST
Created attachment 244576 [details]
Patch
Comment 2 Brent Fulgham 2015-01-13 20:56:55 PST
I misunderstood what copyASCIIMachineWord was doing here. The sizeof() is the correct thing to be doing, and the Static Analyzer warning is spurious.
Comment 3 Darin Adler 2015-01-14 15:46:51 PST
Comment on attachment 244576 [details]
Patch

These three fixes look good. Why no regression tests for any of them? Didn’t these bugs cause any symptoms? We normally require regression tests for all bug fixes.
Comment 4 Darin Adler 2015-01-14 15:47:36 PST
Comment on attachment 244576 [details]
Patch

Oops, as you said, the warning was wrong for the destination16 lines!