Summary: | StringImpl can corrupt the static empty string | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Michelle Bell <whereismichelleb> | ||||
Component: | Platform | Assignee: | Michelle Bell <whereismichelleb> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | ap, darin, fishd | ||||
Priority: | P2 | ||||||
Version: | 420+ | ||||||
Hardware: | PC | ||||||
OS: | OS X 10.4 | ||||||
Attachments: |
|
Description
Michelle Bell
2006-11-09 10:47:33 PST
Created attachment 11442 [details]
patch to String.cpp
BTW - it should be noted that once the static empty string is corrupted, any users of the empty string get the corrupted string. Basically, everything starts failing. Comment on attachment 11442 [details]
patch to String.cpp
Marking r? for attention
Confirming. Another failing code sequence (as alluded to in comment 2) is String s1 = ""; s1.insert("corrupted", 0); String s2 = ""; assert(s2.empty()); Comment on attachment 11442 [details]
patch to String.cpp
This looks like a design problem to me. According to a comment above the class declaration, Strings are supposed to be shared - so, the fact that append() implicitly copies (unlike insert(), replace() and truncate()) is formally a bug, and the constructors that initialize m_impl to StringImpl::empty() are incorrect...
Originally the class that used to be String was used in a very inconsistent way, but our current intent is that StringImpls should be shareable and effectively immutable so that two Strings with the same value will not both be modified if one is mutated. However, it's uncertain whether any code is still dependent on the old semantics of insert. I think this will be fixed by the patch in bug 16550. |