Closed Bug 334541 Opened 14 years ago Closed 13 years ago

Use after free in XPCOM testcase

Categories

(Core :: XPCOM, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: dveditz, Assigned: dveditz)

References

(Blocks 1 open bug)

Details

(Keywords: coverity)

Attachments

(1 file, 2 obsolete files)

in TestHashtables.cpp IFoo::Release references its refcount_ member after deleting 'this'. Found by coverity
Attached patch return after delete (obsolete) — Splinter Review
Looking for trivial r/sr
Attachment #218889 - Flags: superreview?
Attachment #218889 - Flags: review?(benjamin)
Attachment #218889 - Attachment is obsolete: true
Attachment #218891 - Flags: superreview?
Attachment #218891 - Flags: review?(benjamin)
Attachment #218889 - Flags: superreview?
Attachment #218889 - Flags: review?(benjamin)
I think this patch changes the behavior when wrap_message is true, by making it skip a printf.  I don't understand what the code is trying to accomplish or why wrap_message is a different test than !refcount_, though.
Comment on attachment 218891 [details] [diff] [review]
TestCOMPtr.cpp has the same problem (combined patch)

You would just be introducing another coverity warning, I think... it seems like

if (!refcount_) and if (wrap_message) are the exact same test.

The easy way out is to store int stored_refcount = --refcount_ and return that.
Attachment #218891 - Flags: superreview?
Attachment #218891 - Flags: review?(benjamin)
Attachment #218891 - Flags: review-
> if (!refcount_) and if (wrap_message) are the exact same test.

Not quite: refcount_ is a member, wrap_message is safely on the stack.

This version keeps us from dropping the closing wrap message.
Attachment #218891 - Attachment is obsolete: true
Attachment #219385 - Flags: review?(benjamin)
Attachment #219385 - Flags: review?(benjamin) → review+
/cvsroot/mozilla/xpcom/tests/TestCOMPtr.cpp,v  <--  TestCOMPtr.cpp
new revision: 1.29; previous revision: 1.28
/cvsroot/mozilla/xpcom/tests/TestHashtables.cpp,v  <--  TestHashtables.cpp
new revision: 1.9; previous revision: 1.8
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.