basic_nsAWritableString<char>::Cut assumes that memcpys are done front to back which mustn't be true. It works fine as long as memcpy work from lower addresses to higher, but will break completely if it is implemented differently. String before Cut: ....XXXX------- (Cutting away the XXXX part) String after Cut: ....------- ^^^ overlap This fell out of bug 40027 because Purify warns when it encounters these overlapping memcpy:s.
I don't really now what to do here. The fix seems to use memmove instead of memcpy, or in this case nsCharTraits<char>::move instead of nsCharTraits<char>::copy but the caller isn't Cut itself. Instead the call chain goes through copy_string and two write functions. nsCharTraits<char>::copy(char *,char const*,UINT nsWritingIterator<char>::write(char const*,UINT) nsCharSinkTraits<nsWritingIterator<char>>::write(nsWritingIterator<char>&,char const*,UINT) copy_string(nsReadingIterator<char>,nsReadingIterator<char>,nsWritingIterator<ch ar>) basic_nsAWritableString<char>::Cut(UINT,UINT) There is a copy_string_backward which I guess is supposed to do the opposite of copy_string so copy_string is apparently supposed to copy forward but it isn't guaranteed to do that. I'm tempted to just change nsWritingIterator<char>::write to use move instead of copy, but I don't know if there are any drawbacks with using memmove? Performance? (Reassigning to scc and cc:ing waterson who seems to be the ones who know about strings in mozilla)
Assignee: rayw → scc
I actually just changing the |copy|s to |move|s in the char traits classes is the most reasonable answer for the current architecture (which doesn't differentiate between the cases that are moving things within a string, and writing into it, or copying out from/to an external source).
We should measure; if the performance penalty is unacceptable, then we should fix the architecture to differentiate intra vs. inter string character moving
there are actually a couple more |copy|s that must become |move|s in "nsAWritableString.h", I already got that patch approved by waterson along with some other changes in that file
the changes are checked in. As I said earlier: we'll want to profile this to see if |move| introduces a non-negligible performance impact, and if so, separate the code path for moves between strings versus within a string
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED
Marking Verified. Please reopen if problem reoccurs.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.