Last Comment Bug 409088 - Rewrite stack objects in garburator
: Rewrite stack objects in garburator
Product: Core
Classification: Components
Component: Rewriting and Analysis (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
: Michael Layzell [:mystor] [:mrl]
Depends on: 430315
  Show dependency treegraph
Reported: 2007-12-19 13:59 PST by Benjamin Smedberg [:bsmedberg]
Modified: 2008-04-22 12:07 PDT (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Description Benjamin Smedberg [:bsmedberg] 2007-12-19 13:59:38 PST
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
  Example(int i) : mI(i) { }

  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 (dormant account) 2007-12-28 11:16:20 PST
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.
Comment 2 Benjamin Smedberg [:bsmedberg] 2008-01-02 11:41:48 PST
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).
Comment 3 Benjamin Smedberg [:bsmedberg] 2008-01-02 11:43:40 PST
Sorry, the real-world testcase is nsAutoPopupStatePusherExternal, and the error shows up as:

-  {
+: mWindow(nsnull)  {
     if (mWindow) {
Comment 4 Benjamin Smedberg [:bsmedberg] 2008-01-02 14:00:34 PST
I think I failed to mention an important requirement ;-)

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

-  nsCOMPtr<nsIFoo> mFoo;
+  nsIFoo *mFoo;
Comment 5 Benjamin Smedberg [:bsmedberg] 2008-01-02 14:03:18 PST
Which implies that all the client code of those members will also need to be rewritten, as if those were stack nsCOMPtrs.
Comment 6 (dormant account) 2008-01-02 15:27:43 PST
> 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.
Comment 7 Benjamin Smedberg [:bsmedberg] 2008-01-04 09:00:53 PST
Appears to work now... I'll file new bugs if there are edge-cases that I find as I go.
Comment 8 Brendan Eich [:brendan] 2008-01-04 13:08:38 PST
This work is making me tingle all over! I love you guys! *snif* (Ok, back to work! On to exceptions! :-P)


Note You need to log in before you can comment on or make changes to this bug.