Closed Bug 355869 Opened 13 years ago Closed 13 years ago

Invalid read in nsCanvasRenderingContext2D::ContextState::ContextState

Categories

(Core :: Canvas: 2D, defect)

x86
Linux
defect
Not set

Tracking

()

VERIFIED FIXED

People

(Reporter: ajschult784, Assigned: ajschult784)

Details

(Keywords: testcase, verified1.8.1.1)

Attachments

(3 files)

In SeaMonkey, tab tooltips are drawn using canvas.  With linux seamonkey trunk CVS from 20061006, if I mouseover a tab to trigger the tooltip shows, I get

Invalid read of size 4 in nsCanvasRenderingContext2D::ContextState::ContextState

This might be a bug in nsTArray instead (that or it's perhaps abusing the API).  It seems to be using memory nsTArray dumped in a call to nsTArray::EnsureCapacity (via realloc).  I'll attach the full valgrind log.
Attached file testcase
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/content/canvas/src/nsCanvasRenderingContext2D.cpp&rev=1.70&mark=1102#1099

    mStyleStack.AppendElement(CurrentState());
is effectively
    mStyleStack.AppendElement(mStyleStack[mSaveCount]);
so it seems to evaluate mStyleStack[mSaveCount] lazily or some such thing.

If I change it to
    ContextState state = CurrentState();
    mStyleStack.AppendElement(state);

the invalid read goes away.  So this seems like a compiler bug.
I think it's not a compiler bug. That might be the right fix - AppendElement might reallocate the internal buffer, which invalidates pointers (and references) to elements in the array, leading to an invalid memory read; so the data needs to be copied before.
Attached patch use a copySplinter Review
Attachment #241599 - Flags: review?(vladimir)
Assignee: nobody → ajschult
Keywords: testcase
Comment on attachment 241599 [details] [diff] [review]
use a copy

r=me, thanks for catching this!
Attachment #241599 - Flags: review?(vladimir) → review+
Attachment #241599 - Flags: superreview?(roc)
Attachment #241599 - Flags: superreview?(roc) → superreview+
landed on trunk
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Comment on attachment 241599 [details] [diff] [review]
use a copy

This is low risk and avoids reading freed memory (potential crash).   Probably missed the boat for 1.8.1, so requesting 1.8.1.1.
Attachment #241599 - Flags: approval1.8.1.1?
Flags: blocking1.8.1.1?
Flags: blocking1.8.1.1? → blocking1.8.1.1+
Comment on attachment 241599 [details] [diff] [review]
use a copy

approved for 1.8 branch, a=dveditz for drivers
Attachment #241599 - Flags: approval1.8.1.1? → approval1.8.1.1+
Keywords: fixed1.8.1.1
WFM:

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.1pre) Gecko/20061201 BonEcho/2.0.0.1pre
Status: RESOLVED → VERIFIED
Keywords: verified1.8.1.1
You need to log in before you can comment on or make changes to this bug.