The default bug view has changed. See this FAQ.

Rewrite stack objects in garburator

RESOLVED FIXED

Status

()

Core
Rewriting and Analysis
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: bsmedberg, Unassigned)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

9 years ago
This is an enhancement for garburator:

I will be manually marking certain classes as being stack allocated using the annotation __attribute__((stack))

If these classes have member nsCOMPtrs, they should be rewritten to be raw pointers just as if they occurred directly on the stack.

This will involve changing the initializer list in constructors:

class __attribute__((stack)) Example
{
public:
  Example(int i) : mI(i) { }

private:
  int mI;
  nsCOMPtr<nsIFoo> mFoo;
};

the constructor needs to become

Example(int i) : mI(i), mFoo(nsnull) { }

but of course only in cases where mFoo isn't already initialized.

I don't have anything checked in yet with this annotation, but I will soon.

Comment 1

9 years ago
Try it with latest oink/elsa revision. 

I implemented it in the testcase, I'll test it on Mozilla when you give me something to play with.
Status: NEW → ASSIGNED
(Reporter)

Comment 2

9 years ago
I tried it... it seems that it's trying to add member initializers to destructor methods, not just constructors.

The real-world testcase is now up at actionmonkey+xpcomgc-patches, through the end (prerewrite_fixes.patch2).
(Reporter)

Comment 3

9 years ago
Sorry, the real-world testcase is nsAutoPopupStatePusherExternal, and the error shows up as:

   }
 
   ~NS_AUTO_POPUP_STATE_PUSHER()
-  {
+: mWindow(nsnull)  {
     if (mWindow) {
       mWindow->PopPopupControlState(mOldState);
     }
   }
(Reporter)

Comment 4

9 years ago
I think I failed to mention an important requirement ;-)

The members have to be rewritten, not just the constructors:

-  nsCOMPtr<nsIFoo> mFoo;
+  nsIFoo *mFoo;
(Reporter)

Comment 5

9 years ago
Which implies that all the client code of those members will also need to be rewritten, as if those were stack nsCOMPtrs.

Comment 6

9 years ago
> Which implies that all the client code of those members will also need to be
> rewritten, as if those were stack nsCOMPtrs.
> 

Right, I think I have that detail covered now in the current pork. I don't have a tree setup to test this extensively, but I'll set one up if you continue having trouble. For now I just modified the testcase.
(Reporter)

Comment 7

9 years ago
Appears to work now... I'll file new bugs if there are edge-cases that I find as I go.
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
This work is making me tingle all over! I love you guys! *snif* (Ok, back to work! On to exceptions! :-P)

/be
(Reporter)

Updated

9 years ago
Depends on: 430315
You need to log in before you can comment on or make changes to this bug.