Closed Bug 1490972 Opened 6 years ago Closed 6 years ago

Writing marker bytes to unused parts of an XPCOM string's buffer is very slow with a large disparity between length and capacity

Categories

(Core :: XPCOM, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox64 --- fixed

People

(Reporter: hsivonen, Assigned: hsivonen)

References

Details

Attachments

(1 file)

With large data: URLs, NS_UnescapeURL() ends up calling SetCapacity() with a large argument and then calling Append() a large number of times with a small amount of data appended each time. This leaves a disproportionately long tail of the buffer to memset.

The length of the memset should be capped to some magic number. 16 code units worth of poison value is probably more than enough to make visible the issues that the poising is trying to surface.
I wonder if informing the memory checkers about logically uninitialized stretches of memory should be similarly capped in this case or not.
Blocks: 1484938
(In reply to Henri Sivonen (:hsivonen) from comment #1)
> I wonder if informing the memory checkers about logically uninitialized
> stretches of memory should be similarly capped in this case or not.

I'm assuming "yes" on the theory that quadratic writing to the shadow memory is slow, too.
PCOM strings mark logically unused parts of nsStringBuffer as uninitialized
in debug builds by writing a marker byte and if memory checking is active,
by telling the memory checking that the range of memory is uninitialized.

This patch limits such marking to up to 16 code units to avoid quadratic
behavior, which is especially bad when there's a large disparity between
length and capacity (after a call to SetCapacity()).

The assumption here is that even a small poisoned memory range is enough
to detect the bugs that the poisoning is intended to detect.

MozReview-Commit-ID: 178rp0ckztj
Comment on attachment 9009010 [details]
Bug 1490972 - Limit the number of bytes poisoned to avoid quadratic behavior.

Nathan Froyd [:froydnj] has approved the revision.
Attachment #9009010 - Flags: review+
Pushed by hsivonen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2fa27517819d
Limit the number of bytes poisoned to avoid quadratic behavior. r=froydnj
https://hg.mozilla.org/mozilla-central/rev/2fa27517819d
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Component: String → XPCOM
You need to log in before you can comment on or make changes to this bug.