NS_IMPL_CYCLE_COLLECTION_TRAVERSE_SCRIPT_OBJECTS uses wrong pointer

RESOLVED FIXED in mozilla7

Status

()

Core
XPCOM
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: peterv, Assigned: peterv)

Tracking

Trunk
mozilla7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

v1
1.28 KB, patch
Ben Turner (not reading bugmail, use the needinfo flag!)
: review+
Details | Diff | Splinter Review
(Assignee)

Description

6 years ago
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_SCRIPT_OBJECTS uses tmp, which is a pointer to the concrete class that we're traversing. However, this passes tmp to the participant's Trace hook, which tries to cast the pointer to the concrete class again (through the CC nsISupports pointer). If the object has multiple inheritance from nsISupports and the interface we use to cast to the CC nsISupports isn't the first interface that the object inherits from we'll end up doing a bad cast.

Here's an example:

class A : public nsIFoo,
          public nsIBar
{}

We use nsIBar to cast to the CC nsISupports.

In Traverse:

p is void*, gotten by doing static_cast<void*>(static_cast<nsISupports*>(static_cast<nsIBar*>(anObject))).

We set tmp to static_cast<A*>(static_cast<nsIBar*>(static_cast<nsISupports*>(p)));

We pass (void*)tmp to Trace via NS_IMPL_CYCLE_COLLECTION_TRAVERSE_SCRIPT_OBJECTS.

In Trace:

p is void*, gotten by doing (void*)tmp, which is equal to static_cast<void*>(static_cast<nsISupports*>(static_cast<nsIFoo*>(anObject))).

We try to get tmp by doing static_cast<A*>(static_cast<nsIBar*>(static_cast<nsISupports*>(p))), which is a bad cast.

Passing p to NS_IMPL_CYCLE_COLLECTION_TRAVERSE_SCRIPT_OBJECTS in Traverse will make p in Trace be static_cast<void*>(static_cast<nsISupports*>(static_cast<nsIBar*>(anObject))), making the cast in Trace correct.

I think this currently isn't a problem because we always used the first interface to cast to/from nsISupports, but it's fragile and we're going to use this with objects where that isn't true in the future.
(Assignee)

Comment 1

6 years ago
Created attachment 539468 [details] [diff] [review]
v1
Attachment #539468 - Flags: review?(bent.mozilla)
Comment on attachment 539468 [details] [diff] [review]
v1

Review of attachment 539468 [details] [diff] [review]:
-----------------------------------------------------------------

Makes sense to me.
Attachment #539468 - Flags: review?(bent.mozilla) → review+
(Assignee)

Comment 3

6 years ago
http://hg.mozilla.org/mozilla-central/rev/c36e6d83d89d
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
You need to log in before you can comment on or make changes to this bug.