Last Comment Bug 623351 - virtual call through dangling style rule pointer resulting from incorrect parent on @imported sheet after COW cloning (Crash running Firebug test suite)
: virtual call through dangling style rule pointer resulting from incorrect par...
Status: RESOLVED FIXED
[firebug-p1][hardblocker][sg:critical?]
:
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: unspecified
: All All
: P2 normal (vote)
: mozilla2.0b10
Assigned To: David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch)
:
Mentors:
: 614212 (view as bug list)
Depends on:
Blocks: 614212 614527
  Show dependency treegraph
 
Reported: 2011-01-05 14:54 PST by Steve Fink [:sfink] [:s:]
Modified: 2011-06-17 14:48 PDT (History)
12 users (show)
dbaron: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
final+
.17+
.17-fixed
.19+
.19-fixed


Attachments
Stack trace of crash (18.61 KB, text/plain)
2011-01-05 14:54 PST, Steve Fink [:sfink] [:s:]
no flags Details
valgrind log of net/ section of tests (96.47 KB, text/plain)
2011-01-07 15:16 PST, Steve Fink [:sfink] [:s:]
no flags Details
Valgrind log of net/ section of Firebug tests, run twice (103.42 KB, text/plain)
2011-01-07 15:36 PST, Steve Fink [:sfink] [:s:]
no flags Details
valgrind log of net/ section of tests (19.65 KB, text/plain)
2011-01-07 17:29 PST, Steve Fink [:sfink] [:s:]
no flags Details
Prefs file triggering crash (6.59 KB, text/plain)
2011-01-13 13:55 PST, Steve Fink [:sfink] [:s:]
no flags Details
valgrind warnings (526.38 KB, text/plain)
2011-01-13 15:28 PST, David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch)
no flags Details
patch (1.70 KB, patch)
2011-01-14 01:16 PST, David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch)
bzbarsky: review+
Details | Diff | Splinter Review
crashtest (to be landed later) (2.92 KB, patch)
2011-01-14 10:33 PST, David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch)
no flags Details | Diff | Splinter Review
crashtest (to be landed later) (2.84 KB, patch)
2011-01-14 12:24 PST, David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch)
no flags Details | Diff | Splinter Review
1.9.2 patch (1.70 KB, patch)
2011-01-14 23:00 PST, David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch)
dveditz: approval1.9.2.17+
Details | Diff | Splinter Review
1.9.1 patch (1.77 KB, patch)
2011-01-14 23:01 PST, David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch)
dveditz: approval1.9.1.19+
Details | Diff | Splinter Review

Description Steve Fink [:sfink] [:s:] 2011-01-05 14:54:54 PST
Created attachment 501467 [details]
Stack trace of crash

When running the Firebug 1.7 test suite, I get erratic crashes during various of the tests. It looks like it's accessing freed memory:

ContentEnumFunc (aRule=0x7fb908cd94c0, aSelector=0x7fb908cd9470, aData=0x7fff621211b0) at /home/sfink/src/TM-singlestep/layout/style/nsCSSRuleProcessor.cpp:2368
(gdb) p *aRule
$6 = {<nsICSSRule> = {<nsIStyleRule> = {<nsISupports> = {_vptr.nsISupports = 0x5a5a5a5a5a5a5a5a}, <No data fields>}, <No data fields>}, <No data fields>}

(note the 0x5a... pattern.)

Steps to reproduce:
1. Install Firebug 1.7 and FBTest 1.7. I did it by checking out (via subversion) http://fbug.googlecode.com/ and making the appropriate symlinks from my extensions/ directory:

  fbtest@mozilla.com -> /home/sfink/src/fbtest1.7
  firebug@software.joehewitt.com -> /home/sfink/src/firebug1.7

2. Click on the firebug icon in the bottom right. Focus on the firebug pane and press Shift-T to bring up the test console.

3. In the console, do "Run All".

For me, this goes on for a while, then at some random point it crashes with a stack trace similar to the attached one.

I have gotten the crash in net/1456, net/activation and one of the script/ ones. You may want to expand the "net" test suite and then run just it, so you can see how far it makes it.
Comment 1 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-05 15:18:41 PST
What Firefox version?
Comment 2 Steve Fink [:sfink] [:s:] 2011-01-05 23:59:57 PST
tracemonkey, recent (32ff6481fba8).
Comment 3 Boris Zbarsky [:bz] 2011-01-06 00:15:04 PST
Can you repeat that experiment under valgrind?
Comment 4 John J. Barton 2011-01-06 15:42:53 PST
Ok I think I fixed this one on the Firebug side:
http://code.google.com/p/fbug/source/detail?r=8842

domUtils.getCSSStyleRules has some limitations which crash Firefox unless you do a certain dance before you call it. Someone added a call to get the style rules in net.js for a UI hack, but failed to dance first. That explains why it happens in net testing and why its intermittent.

I'll leave this open until we try a bit more. If we stop seeing the crash we win.
Comment 5 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-06 17:09:14 PST
That shouldn't be the case anymore following bug 536379 and bug 578810.
Comment 6 John J. Barton 2011-01-06 17:18:20 PST
For reference the 'dance' bit is discussed in bug 500365 and http://code.google.com/p/fbug/issues/detail?id=2440
Comment 7 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-06 17:38:43 PST
As noted in bug 536379 comment 14, that should no longer be needed.  If it is, there's a bug (which appears to be the case), but I'm not sure if the information given is enough of a testcase to be useful in fixing it.
Comment 8 Boris Zbarsky [:bz] 2011-01-06 21:15:16 PST
Well, information on whether the change made in comment 4 fixes the crash or not would be a good start.

Note that depending on what Firebug is doing, if they're messing with a _ua_ sheet they would run into issues still, right?  Of course then the firebug-side fix wouldn't help either.
Comment 9 Steve Fink [:sfink] [:s:] 2011-01-07 12:50:51 PST
I'm valgrinding away, using the original (known-to-crash) 1.7. Unfortunately, everything's timing out a little before 30s and failing, which may interfere with the diagnostic utility a bit.

The valgrind log shows 17 invalid reads, but they're all during startup, and I think I get them independently of firebug or fbtest. (Though I haven't confirmed, and I need to wait a while for this run before I can.) Is there a better way to valgrind? I have --enable-valgrind, and --smc-check=yes (for JaegerMonkey). Looks like it's valgrind fighting with jemalloc (see bug 503249).
Comment 10 Steve Fink [:sfink] [:s:] 2011-01-07 15:16:45 PST
Created attachment 502114 [details]
valgrind log of net/ section of tests

It made it all the way through the whole net/ section without crashing, but every test timed out. I just noticed the "No Timeout" button at the top, so I'm trying again with that.

I'm still running with jemalloc, so there's some noise. The startup stuff is over by line 442. The locations in the log are all QI-related, which is where the actual crash is happening, too.
Comment 11 Steve Fink [:sfink] [:s:] 2011-01-07 15:36:04 PST
Created attachment 502121 [details]
Valgrind log of net/ section of Firebug tests, run twice

It finally managed to crash at the same place as the real (non-valgrind) run, with this message:

##!!! ASSERTION: Please fix QI so this performance optimization is valid: 'static_cast<nsIStyleRule*>(aRule) == iRule.get()', file /home/sfink/src/TM-singlestep/layout/style/nsCSSRuleProcessor.cpp, line 2370

I was a little surprised, because it was just sitting in the XHR test. I thought it would stay there forever, because I had turned off timeouts and XHR debugging is broken right now such that it'll never get the XHR response callback. But it did crash, yay. (Not sure if it made it past that test or not; after it crashes, you can't see the status window anymore and I didn't do the config that would dump it out to an fd.)

I've attached the full valgrind log, which is the same as the other but with more stuff.
Comment 12 John J. Barton 2011-01-07 17:10:47 PST
To confirm dbaron and boris' theory, I crashed running net panel even after comment 4. Slightly different stack
http://crash-stats.mozilla.com/report/index/23f6df42-8ad9-4767-90c3-882cb2110107
Comment 13 John J. Barton 2011-01-07 17:15:25 PST
Bug 614527 is probably a dup, but it shows a normal Firebug user (not FBTest) crashing.
Comment 14 Steve Fink [:sfink] [:s:] 2011-01-07 17:29:09 PST
Created attachment 502153 [details]
valgrind log of net/ section of tests

Bleh. I recompiled with --disable-jemalloc and reran, getting the same crash. This time, the valgrind log showed no more than what you could figure out from attaching with gdb. Everything else seems to be a result of jemalloc.
Comment 15 Boris Zbarsky [:bz] 2011-01-07 17:38:03 PST
> the valgrind log showed no more than what you could figure out from
> attaching with gdb

Uh... it showed where the block was freed.  How is that no more???
Comment 16 Boris Zbarsky [:bz] 2011-01-07 18:22:25 PST
Though I guess for only one of the addresses...  the other was just a pointer to dead memory. 

Plus, now the pattern is different. 0xdadadada is a PL_FREE_PATTERN/JS_FREE_PATTERN (and something msvc-ish, but you're on Linux).

The 0x5a5a5a5a pattern in comment 0 is jemalloc's freed memory marker.

For what it's worth, we're crashing enumerating the document rules (at least if your line 633 in nsStyleSet is the same as what I'm seeing here).  So something is definitely fishy...
Comment 17 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-01-10 18:54:50 PST
Firebug worked around this, so we don't need to block.
Comment 18 Boris Zbarsky [:bz] 2011-01-10 19:03:45 PST
Their workaround doesn't work, as expected.  See comment 12.  Renominating.
Comment 19 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-11 18:35:15 PST
sfink, could you confirm what's on nsStyleSet.cpp line 633 for you (or what hg revision you did the valgrind run on)?
Comment 20 Steve Fink [:sfink] [:s:] 2011-01-11 23:29:27 PST
Ugh. I've rebased that repo a few times since then. It's a tracemonkey checkout, which currently has this:

    (*aCollectorFunc)(mRuleProcessors[eDocSheet], aData);

following an if. Starting at line 631:

  if (!skipUserStyles && !cutOffInheritance &&
      mRuleProcessors[eDocSheet]) // NOTE: different
    (*aCollectorFunc)(mRuleProcessors[eDocSheet], aData);
  aRuleWalker->SetLevel(eStyleAttrSheet, PR_FALSE,
                        aRuleWalker->GetCheckForImportantRules());
  if (mRuleProcessors[eStyleAttrSheet])
    (*aCollectorFunc)(mRuleProcessors[eStyleAttrSheet], aData);

I will now attempt to learn enough hg arcana to see if I can return to my directory state at that time...
Comment 21 Boris Zbarsky [:bz] 2011-01-11 23:31:28 PST
Doesn't matter what the directory state was.  What matters is which line of that is on your callstack when you crash.  But I bet it's still this line:

    (*aCollectorFunc)(mRuleProcessors[eDocSheet], aData);
Comment 22 Steve Fink [:sfink] [:s:] 2011-01-11 23:56:58 PST
(In reply to comment #21)
> Doesn't matter what the directory state was.  What matters is which line of
> that is on your callstack when you crash.  But I bet it's still this line:
> 
>     (*aCollectorFunc)(mRuleProcessors[eDocSheet], aData);

Sorry, I'm dense. I don't follow what you're saying. I want to be able to say what line 633 was in the binary that I built and ran valgrind on. To figure that out, I need to know what rev was sitting in my directory at the time I built.

Or are you saying I just need to know the times that I pulled in changes to that specific file? That is true too, but I don't know how to figure that out either. (I can see timestamps for when changes to that file were committed, but not timestamps for when those changes hit my local repo, nor for when I updated my working directory with those changes.) But I'm probably misinterpreting you.

Anyway, I still don't know how to figure it out properly, but fortunately I have logging hooks that give me a log saying my previous pull was of 8e119f847f97ba1e29da8192ca5fc93248e6c960, which has the same code at line 633
as I pasted above.

That should be the right rev, but my log doesn't show which repo I pulled into, so I'm not 100% certain.
Comment 23 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-12 00:04:01 PST
Actually, line numbers < 1000 in nsStyleSet.cpp haven't changed since mid-June, so I think it's safe, as long as that tree doesn't have any local patches to the file (which it sounds like it doesn't).
Comment 24 Steve Fink [:sfink] [:s:] 2011-01-12 00:18:48 PST
Heh. I guess I could have checked that first.

Just wanted to mention that I just did another valgrind run on roughly the same test suite for another issue, and oddly it managed to make it much farther without crashing. The valgrind log shows an unrelated invalid read (during GC triggered by CC), but nothing about this. It may not mean much, since the problem is sporadic to begin with, but I think I do have some firebug changes this time around so the problem may be legitimately masked right now. (I have the older version lying around, so I can repeat if needed.)

Let me know if I you need anything else from me to help.
Comment 25 Boris Zbarsky [:bz] 2011-01-12 14:07:28 PST
Will do.  I think it's pretty clear that the document sheets here are confused, though....
Comment 26 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-12 14:58:14 PST
(In reply to comment #0)
> 1. Install Firebug 1.7 and FBTest 1.7. I did it by checking out (via
> subversion) http://fbug.googlecode.com/ and making the appropriate symlinks
> from my extensions/ directory:
> 
>   fbtest@mozilla.com -> /home/sfink/src/fbtest1.7
>   firebug@software.joehewitt.com -> /home/sfink/src/firebug1.7

I'm assuming the right things to check out here are:
  http://fbug.googlecode.com/svn/fbtest/branches/fbtest1.7
  http://fbug.googlecode.com/svn/branches/firebug1.7

If not, please let me know.
Comment 27 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-12 15:13:04 PST
(In reply to comment #0)
> 3. In the console, do "Run All".

I have it running tests now.  (Hasn't crashed yet, but it just started.)
Comment 28 John J. Barton 2011-01-12 15:16:30 PST
In my experience and based on Steve's result, you should be able to save some time by clicking on the Net test collection.

Also you might want to remove the lines:
        // This call must precede all getCSSStyleRules calls
        Firebug.CSSModule.cleanupSheets(hrefLabel.ownerDocument, Firebug.currentContext);
from net.js, since these were added to workaround this bug, but Boris' thinks they do nothing. So nothing is probably better.
Comment 29 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-12 17:49:38 PST
So far I've run all tests once under valgrind, and then net tests again.  (The second time (the net tests only) I'd removed the lines in comment 28.)

I did this without crashing, and without any valgrind warnings that seem relevant to this crash bug (though I did discover bug 625249, which I saw on both runs; I don't *think* it's related to this bug).

I've been running on a clean build of
http://hg.mozilla.org/mozilla-central/rev/f9537a28ce19 with the line
"#define DEBUG_TRACEMALLOC_PRESARENA 1" in layout/base/nsPresArena.h uncommented.
Comment 30 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-13 12:02:38 PST
And I just ran the net tests 5 or 6 times without crashing (no valgrind involved), in case it was a timing issue.

And I turned the frame arena (and thus frame poisoning) back on, and ran the net tests another 4 times, and also didn't crash.

So I'm really not sure how to observe this bug.  I'm running the mozilla-central revision in comment 29, along with:

URL: http://fbug.googlecode.com/svn/fbtest/branches/fbtest1.7
Repository Root: http://fbug.googlecode.com/svn
Repository UUID: e969d3be-0e28-0410-a27f-dd5c76401a8b
Revision: 8918

URL: http://fbug.googlecode.com/svn/branches/firebug1.7
Repository Root: http://fbug.googlecode.com/svn
Repository UUID: e969d3be-0e28-0410-a27f-dd5c76401a8b
Revision: 8918
Comment 31 Steve Fink [:sfink] [:s:] 2011-01-13 13:00:39 PST
I'm still getting it easily with 4f71ecca94fe (tracemonkey), firebug1.7 revision 8916, fbtest 8835.

I'm trying now with firebug1.7 8935, fbtest 8935.... yep, still crashes. (Takes only about 15 seconds or so.)

I am on linux64. Perhaps that matters?

I'm doing a build of the same thing on my desktop box. If I can reproduce there, I can give you a login.

<groan> No, it made it all the way through there. Ok, I'll poke around...
Comment 32 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-13 13:05:15 PST
I'm on Linux64 as well.

What's the "desktop box" that you can't repro on?
Comment 33 Steve Fink [:sfink] [:s:] 2011-01-13 13:13:24 PST
It's not the build; I fortunately have identical directory structures so I rsync'ed over the same bits and it still didn't crash.

It's something in the profile. When I copied that over, it started crashing. I may have some extra Firebug tracing output. I will narrow that down.

The crash is very slightly different -- instead of a null deref, it's a junk deref. But the stack and object involved is the same.
Comment 34 Steve Fink [:sfink] [:s:] 2011-01-13 13:14:55 PST
(In reply to comment #32)
> I'm on Linux64 as well.
> 
> What's the "desktop box" that you can't repro on?

It's the same. Email me an SSH public key if you'd like access to it. (sfink@mozilla.com). I'm working on narrowing down the profile difference.
Comment 35 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-13 13:27:04 PST
It sounds like the profile, or some piece of it, would be more useful than access to the box.  (e.g., if the prefs file is sufficient to make a new profile show the problem, no need to bother reducing it to which pref...)
Comment 36 Steve Fink [:sfink] [:s:] 2011-01-13 13:31:51 PST
That's what I'm trying to get. But copying the firebug part of the prefs.js file didn't work (still didn't crash with the "bad" prefs), and copying the whole file somehow messes up fbtest. Still working on it.
Comment 37 Steve Fink [:sfink] [:s:] 2011-01-13 13:55:28 PST
Created attachment 503602 [details]
Prefs file triggering crash

Well, I'm deep in "wtf?!" territory, but I was able to go from no crash -> crash with the attached prefs.js file.

I'm not sure why it messed up fbtest before, but something also deleted all files from both extensions. Odd. Anyway, can you give this prefs.js a try? I don't know if you need to tweak anything to get it to work (delete extensions.installCache, maybe?), but maybe you do?
Comment 38 John J. Barton 2011-01-13 14:14:31 PST
I updated my nightly build (on Win7 32 bit), ran and crashed.

Another slightly different stack here:
http://crash-stats.mozilla.com/report/index/bp-8b61c8cc-afb2-4caa-93ff-155352110113

Reset Firebug options. Ran once no crash. Exited, ran again, crashed, yet another stack:
http://crash-stats.mozilla.com/report/index/bp-de3899e2-3b2f-4d36-8e30-53a832110113

deleted my prefs.js.  This file is recreated by Firefox. This file is read/write directly by code, no cache so you don't need to do anything but restart.

Ran net tests. No crash. 
Ran net tests. No crash.
Ran net tests. Crash.  This one is quite different:
http://crash-stats.mozilla.com/report/index/bp-de3899e2-3b2f-4d36-8e30-53a832110113
Exit restart
Ran net tests. No Crash.
Ran net tests. No Crash.
Ran net tests. No Crash.

Seems random to me. And the variable stacks means there are probably more of crashes caused by this bug than we think from the crash-stats results.
Comment 39 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-13 14:26:11 PST
I see the crash with that prefs.js (substituted with my old extensions.installCache).
Comment 40 Steve Fink [:sfink] [:s:] 2011-01-13 14:55:46 PST
I got a different stack trace, too.

Oddly enough, the profile difference that seems to matter is the presence of these lines:

user_pref("capability.principal.codebase.p0.id", "file://");
user_pref("capability.principal.codebase.p0.subjectName", "");
user_pref("capability.principal.codebase.p1.granted", "UniversalXPConnect");
user_pref("capability.principal.codebase.p1.id", "file://");
user_pref("capability.principal.codebase.p1.subjectName", "");
user_pref("capability.principal.codebase.p2.granted", "UniversalXPConnect");
user_pref("capability.principal.codebase.p2.id", "file://");
user_pref("capability.principal.codebase.p2.subjectName", "");
user_pref("capability.principal.codebase.p3.granted", "UniversalXPConnect");
user_pref("capability.principal.codebase.p3.id", "file://");
user_pref("capability.principal.codebase.p3.subjectName", "");
user_pref("capability.principal.codebase.p4.granted", "UniversalXPConnect");
user_pref("capability.principal.codebase.p4.id", "file://");
user_pref("capability.principal.codebase.p4.subjectName", "");
user_pref("capability.principal.codebase.p5.granted", "UniversalXPConnect");
user_pref("capability.principal.codebase.p5.id", "file://");
user_pref("capability.principal.codebase.p5.subjectName", "");
user_pref("capability.principal.codebase.p6.granted", "UniversalXPConnect");
user_pref("capability.principal.codebase.p6.id", "file://");
user_pref("capability.principal.codebase.p6.subjectName", "");

Crash with, no crash without. Those are from some completely unrelated stuff I was doing with a mochitest. jjb, do you have anything like that in your prefs.js?

Sorry for the spam, but bugzilla is refusing to allow me to attach, so I'm pasting it here:

#0  0x00007ffff58bd094 in ns_if_addref<nsIStyleRule*> (expr=0x8000ffff) at ../../dist/include/nsISupportsUtils.h:94
#1  0x00007ffff589fe1b in nsRuleNode::nsRuleNode (this=0x28b8028, aContext=0x2382bf0, aParent=0x28b8240, aRule=0x8000ffff, aLevel=4 '\004', aIsImportant=1) at /home/sfink/src/TM-crashme/layout/style/nsRuleNode.cpp:1174
#2  0x00007ffff58a0253 in nsRuleNode::Transition (this=0x28b8240, aRule=0x8000ffff, aLevel=4 '\004', aIsImportantRule=1 '\001') at /home/sfink/src/TM-crashme/layout/style/nsRuleNode.cpp:1236
#3  0x00007ffff58525e5 in nsRuleWalker::Forward (this=0x7fffffff8bc0, aRule=0x8000ffff) at /home/sfink/src/TM-crashme/layout/style/nsRuleWalker.h:59
#4  0x00007ffff58cf85a in nsStyleSet::AddImportantRules (this=0x2383d20, aCurrLevelNode=0x28b8240, aLastPrevLevelNode=0x272f0e0, aRuleWalker=0x7fffffff8bc0) at /home/sfink/src/TM-crashme/layout/style/nsStyleSet.cpp:538
#5  0x00007ffff58cff62 in nsStyleSet::FileRules (this=0x2383d20, aCollectorFunc=0x7ffff58d2af3 <EnumRulesMatching<ElementRuleProcessorData>(nsIStyleRuleProcessor*, void*)>, aData=0x7fffffff8b30, aContent=0x5299130, aRuleWalker=0x7fffffff8bc0) at /home/sfink/src/TM-crashme/layout/style/nsStyleSet.cpp:649
#6  0x00007ffff58d0efb in nsStyleSet::ResolveStyleFor (this=0x2383d20, aElement=0x5299130, aParentContext=0x4d3f928) at /home/sfink/src/TM-crashme/layout/style/nsStyleSet.cpp:794
#7  0x00007ffff56618b5 in nsCSSFrameConstructor::ResolveStyleContext (this=0x2385800, aParentStyleContext=0x4d3f928, aContent=0x5299130) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:4592
#8  0x00007ffff5661854 in nsCSSFrameConstructor::ResolveStyleContext (this=0x2385800, aParentFrame=0x2f58fa0, aContent=0x5299130) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:4582
#9  0x00007ffff5662618 in nsCSSFrameConstructor::AddFrameConstructionItems (this=0x2385800, aState=..., aContent=0x5299130, aSuppressWhiteSpaceOptimizations=0, aParentFrame=0x538b3d8, aItems=...) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:5048
#10 0x00007ffff566d7c6 in nsCSSFrameConstructor::ProcessChildren (this=0x2385800, aState=..., aContent=0x5298ff0, aStyleContext=0x4d3f928, aFrame=0x538b3d8, aCanHaveGeneratedContent=1, aFrameItems=..., aAllowBlockStyles=1, aPendingBinding=0x0) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:9616
#11 0x00007ffff565b73a in nsCSSFrameConstructor::ConstructTableCell (this=0x2385800, aState=..., aItem=..., aParentFrame=0xd5a3b0, aDisplay=0x5177190, aFrameItems=..., aNewFrame=0x7fffffff9180) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:2132
#12 0x00007ffff565fc10 in nsCSSFrameConstructor::ConstructFrameFromItemInternal (this=0x2385800, aItem=..., aState=..., aParentFrame=0xd5a3b0, aFrameItems=...) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:3720
#13 0x00007ffff56638e1 in nsCSSFrameConstructor::ConstructFramesFromItem (this=0x2385800, aState=..., aIter=..., aParentFrame=0xd5a3b0, aFrameItems=...) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:5490
#14 0x00007ffff56770bf in nsCSSFrameConstructor::ConstructFramesFromItemList (this=0x2385800, aState=..., aItems=..., aParentFrame=0xd5a3b0, aFrameItems=...) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:9515
#15 0x00007ffff566d8b4 in nsCSSFrameConstructor::ProcessChildren (this=0x2385800, aState=..., aContent=0x44813c0, aStyleContext=0x37802e8, aFrame=0xd5a3b0, aCanHaveGeneratedContent=1, aFrameItems=..., aAllowBlockStyles=0, aPendingBinding=0x0) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:9631
#16 0x00007ffff565b138 in nsCSSFrameConstructor::ConstructTableRow (this=0x2385800, aState=..., aItem=..., aParentFrame=0x1f1e968, aDisplay=0x2724098, aFrameItems=..., aNewFrame=0x7fffffff9740) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:1992
#17 0x00007ffff565fc10 in nsCSSFrameConstructor::ConstructFrameFromItemInternal (this=0x2385800, aItem=..., aState=..., aParentFrame=0x1f1e968, aFrameItems=...) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:3720
#18 0x00007ffff56638e1 in nsCSSFrameConstructor::ConstructFramesFromItem (this=0x2385800, aState=..., aIter=..., aParentFrame=0x1f1e968, aFrameItems=...) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:5490
#19 0x00007ffff56770bf in nsCSSFrameConstructor::ConstructFramesFromItemList (this=0x2385800, aState=..., aItems=..., aParentFrame=0x1f1e968, aFrameItems=...) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:9515
#20 0x00007ffff566d8b4 in nsCSSFrameConstructor::ProcessChildren (this=0x2385800, aState=..., aContent=0x52e66e0, aStyleContext=0x4786890, aFrame=0x1f1e968, aCanHaveGeneratedContent=1, aFrameItems=..., aAllowBlockStyles=0, aPendingBinding=0x0) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:9631
#21 0x00007ffff56600d3 in nsCSSFrameConstructor::ConstructFrameFromItemInternal (this=0x2385800, aItem=..., aState=..., aParentFrame=0x1ebe580, aFrameItems=...) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:3810
#22 0x00007ffff56638e1 in nsCSSFrameConstructor::ConstructFramesFromItem (this=0x2385800, aState=..., aIter=..., aParentFrame=0x1ebe580, aFrameItems=...) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:5490
#23 0x00007ffff56770bf in nsCSSFrameConstructor::ConstructFramesFromItemList (this=0x2385800, aState=..., aItems=..., aParentFrame=0x1ebe580, aFrameItems=...) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:9515
#24 0x00007ffff566d8b4 in nsCSSFrameConstructor::ProcessChildren (this=0x2385800, aState=..., aContent=0x5674b80, aStyleContext=0x377edc0, aFrame=0x1ebe580, aCanHaveGeneratedContent=1, aFrameItems=..., aAllowBlockStyles=0, aPendingBinding=0x0) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:9631
#25 0x00007ffff565aebc in nsCSSFrameConstructor::ConstructTable (this=0x2385800, aState=..., aItem=..., aParentFrame=0x3773528, aDisplay=0x2723bb0, aFrameItems=..., aNewFrame=0x7fffffffa250) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:1937
#26 0x00007ffff565fc10 in nsCSSFrameConstructor::ConstructFrameFromItemInternal (this=0x2385800, aItem=..., aState=..., aParentFrame=0x3773528, aFrameItems=...) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:3720
#27 0x00007ffff56638e1 in nsCSSFrameConstructor::ConstructFramesFromItem (this=0x2385800, aState=..., aIter=..., aParentFrame=0x3773528, aFrameItems=...) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:5490
#28 0x00007ffff56770bf in nsCSSFrameConstructor::ConstructFramesFromItemList (this=0x2385800, aState=..., aItems=..., aParentFrame=0x3773528, aFrameItems=...) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:9515
#29 0x00007ffff566d8b4 in nsCSSFrameConstructor::ProcessChildren (this=0x2385800, aState=..., aContent=0x29a89c0, aStyleContext=0x377c8f0, aFrame=0x3773528, aCanHaveGeneratedContent=1, aFrameItems=..., aAllowBlockStyles=1, aPendingBinding=0x0) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:9631
#30 0x00007ffff566ffb6 in nsCSSFrameConstructor::ConstructBlock (this=0x2385800, aState=..., aDisplay=0x2723820, aContent=0x29a89c0, aParentFrame=0x4974470, aContentParentFrame=0x4974470, aStyleContext=0x377c8f0, aNewFrame=0x7fffffffa808, aFrameItems=..., aAbsPosContainer=1, aPendingBinding=0x0) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:10681
#31 0x00007ffff56613b5 in nsCSSFrameConstructor::ConstructScrollableBlock (this=0x2385800, aState=..., aItem=..., aParentFrame=0x27143a8, aDisplay=0x460c238, aFrameItems=..., aNewFrame=0x7fffffffa960) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:4476
#32 0x00007ffff565fc10 in nsCSSFrameConstructor::ConstructFrameFromItemInternal (this=0x2385800, aItem=..., aState=..., aParentFrame=0x27143a8, aFrameItems=...) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:3720
#33 0x00007ffff56638e1 in nsCSSFrameConstructor::ConstructFramesFromItem (this=0x2385800, aState=..., aIter=..., aParentFrame=0x27143a8, aFrameItems=...) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:5490
#34 0x00007ffff56770bf in nsCSSFrameConstructor::ConstructFramesFromItemList (this=0x2385800, aState=..., aItems=..., aParentFrame=0x27143a8, aFrameItems=...) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:9515
#35 0x00007ffff5667c43 in nsCSSFrameConstructor::ContentRangeInserted (this=0x2385800, aContainer=0x25968d0, aStartChild=0x29a89c0, aEndChild=0x4b1f630, aFrameState=0x2387240, aAllowLazyConstruction=0) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:7190
#36 0x00007ffff5666c26 in nsCSSFrameConstructor::ContentInserted (this=0x2385800, aContainer=0x25968d0, aChild=0x29a89c0, aFrameState=0x2387240, aAllowLazyConstruction=0) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:6807
#37 0x00007ffff566d027 in nsCSSFrameConstructor::RecreateFramesForContent (this=0x2385800, aContent=0x29a89c0, aAsyncInsert=0) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:9156
#38 0x00007ffff566c4cb in nsCSSFrameConstructor::MaybeRecreateFramesForElement (this=0x2385800, aElement=0x29a89c0) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:8918
#39 0x00007ffff566a044 in nsCSSFrameConstructor::RestyleElement (this=0x2385800, aElement=0x29a89c0, aPrimaryFrame=0x0, aMinHint=0, aRestyleTracker=..., aRestyleDescendants=0) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:8107
#40 0x00007ffff5655cc7 in mozilla::css::RestyleTracker::ProcessOneRestyle (this=0x2385898, aElement=0x29a89c0, aRestyleHint=eRestyle_Self, aChangeHint=0) at /home/sfink/src/TM-crashme/layout/base/RestyleTracker.cpp:156
#41 0x00007ffff565512e in mozilla::css::RestyleTracker::ProcessRestyles (this=0x2385898) at /home/sfink/src/TM-crashme/layout/base/RestyleTracker.cpp:240
#42 0x00007ffff5671f8b in nsCSSFrameConstructor::ProcessPendingRestyles (this=0x2385800) at /home/sfink/src/TM-crashme/layout/base/nsCSSFrameConstructor.cpp:11653
#43 0x00007ffff56e8594 in PresShell::ResizeReflowIgnoreOverride (this=0x2384020, aWidth=56400, aHeight=12060) at /home/sfink/src/TM-crashme/layout/base/nsPresShell.cpp:2860
#44 0x00007ffff56e82e0 in PresShell::ResizeReflow (this=0x2384020, aWidth=56400, aHeight=12060) at /home/sfink/src/TM-crashme/layout/base/nsPresShell.cpp:2812
#45 0x00007ffff56f7a42 in PresShell::ResizeReflow (this=0x2384020, aView=0x2383c50, aWidth=56400, aHeight=12060) at /home/sfink/src/TM-crashme/layout/base/nsPresShell.cpp:7390
#46 0x00007ffff5d02c99 in nsViewManager::DoSetWindowDimensions (this=0x2383bd0, aWidth=56400, aHeight=12060) at /home/sfink/src/TM-crashme/view/src/nsViewManager.cpp:308
#47 0x00007ffff5d02d1e in nsViewManager::SetWindowDimensions (this=0x2383bd0, aWidth=56400, aHeight=12060) at /home/sfink/src/TM-crashme/view/src/nsViewManager.cpp:317
#48 0x00007ffff56abdf2 in DocumentViewerImpl::SetBounds (this=0x2379130, aBounds=...) at /home/sfink/src/TM-crashme/layout/base/nsDocumentViewer.cpp:1879
#49 0x00007ffff63c4f6a in nsDocShell::SetPositionAndSize (this=0x19a1a70, x=0, y=0, cx=940, cy=201, fRepaint=0) at /home/sfink/src/TM-crashme/docshell/base/nsDocShell.cpp:4611
#50 0x00007ffff5a0d372 in nsFrameLoader::UpdateBaseWindowPositionAndSize (this=0x19a1880, aIFrame=0x1d059e8) at /home/sfink/src/TM-crashme/content/base/src/nsFrameLoader.cpp:1499
#51 0x00007ffff5a0d20d in nsFrameLoader::UpdatePositionAndSize (this=0x19a1880, aIFrame=0x1d059e8) at /home/sfink/src/TM-crashme/content/base/src/nsFrameLoader.cpp:1473
#52 0x00007ffff57f2c4b in nsSubDocumentFrame::ReflowFinished (this=0x1d059e8) at /home/sfink/src/TM-crashme/layout/generic/nsSubDocumentFrame.cpp:658
#53 0x00007ffff56eebd1 in PresShell::HandlePostedReflowCallbacks (this=0x1119500, aInterruptible=1) at /home/sfink/src/TM-crashme/layout/base/nsPresShell.cpp:4740
#54 0x00007ffff56f85a8 in PresShell::DidDoReflow (this=0x1119500, aInterruptible=1) at /home/sfink/src/TM-crashme/layout/base/nsPresShell.cpp:7649
#55 0x00007ffff56f961f in PresShell::ProcessReflowCommands (this=0x1119500, aInterruptible=1) at /home/sfink/src/TM-crashme/layout/base/nsPresShell.cpp:7937
#56 0x00007ffff56ef12a in PresShell::FlushPendingNotifications (this=0x1119500, aType=Flush_InterruptibleLayout) at /home/sfink/src/TM-crashme/layout/base/nsPresShell.cpp:4887
#57 0x00007ffff56f7b1b in PresShell::WillPaint (this=0x1119500, aWillSendDidPaint=1) at /home/sfink/src/TM-crashme/layout/base/nsPresShell.cpp:7422
#58 0x00007ffff5d069d7 in nsViewManager::CallWillPaintOnObservers (this=0x1118e60, aWillSendDidPaint=1) at /home/sfink/src/TM-crashme/view/src/nsViewManager.cpp:1656
#59 0x00007ffff5d047c7 in nsViewManager::DispatchEvent (this=0x1118e60, aEvent=0x7fffffffc810, aView=0x1118ee0, aStatus=0x7fffffffc684) at /home/sfink/src/TM-crashme/view/src/nsViewManager.cpp:896
#60 0x00007ffff5cfe7f0 in HandleEvent (aEvent=0x7fffffffc810) at /home/sfink/src/TM-crashme/view/src/nsView.cpp:161
#61 0x00007ffff672d2be in nsWindow::DispatchEvent (this=0x1118f70, aEvent=0x7fffffffc810, aStatus=@0x7fffffffc93c) at /home/sfink/src/TM-crashme/widget/src/gtk2/nsWindow.cpp:571
#62 0x00007ffff673130e in nsWindow::OnExposeEvent (this=0x1118f70, aWidget=0x9a8cb0 [MozContainer], aEvent=0x7fffffffd010) at /home/sfink/src/TM-crashme/widget/src/gtk2/nsWindow.cpp:2234
#63 0x00007ffff673a4a3 in expose_event_cb (widget=0x9a8cb0 [MozContainer], event=0x7fffffffd010) at /home/sfink/src/TM-crashme/widget/src/gtk2/nsWindow.cpp:5524
#64 0x0000003e7ab4e223 in _gtk_marshal_BOOLEAN__BOXED (closure=0x10ffda0, return_value=0x7fffffffcca0, n_param_values=<value optimized out>, param_values=0x2986670, invocation_hint=<value optimized out>, marshal_data=<value optimized out>) at gtkmarshalers.c:86
#65 0x00000038d4a0e03e in g_closure_invoke (closure=0x10ffda0, return_value=0x7fffffffcca0, n_param_values=2, param_values=0x2986670, invocation_hint=0x7fffffffcc60) at gclosure.c:766
#66 0x00000038d4a1ee87 in signal_emit_unlocked_R (node=<value optimized out>, detail=0, instance=0x9a8cb0, emission_return=0x7fffffffce10, instance_and_params=0x2986670) at gsignal.c:3252
#67 0x00000038d4a28555 in g_signal_emit_valist (instance=<value optimized out>, signal_id=<value optimized out>, detail=<value optimized out>, var_args=<value optimized out>) at gsignal.c:2993
#68 0x00000038d4a28983 in g_signal_emit (instance=<value optimized out>, signal_id=<value optimized out>, detail=<value optimized out>) at gsignal.c:3040
#69 0x0000003e7ac85aef in gtk_widget_event_internal (widget=0x9a8cb0 [MozContainer], event=0x7fffffffd010) at gtkwidget.c:4992
#70 0x0000003e7ab4c30f in IA__gtk_main_do_event (event=0x7fffffffd010) at gtkmain.c:1601
#71 0x0000003e7a6453da in _gdk_window_process_updates_recurse (window=0x63f480 [GdkWindow], expose_region=0x17df130) at gdkwindow.c:5424
#72 0x0000003e7a63fefb in gdk_window_process_updates_internal (window=0x63f480 [GdkWindow]) at gdkwindow.c:5583
#73 0x0000003e7a642299 in IA__gdk_window_process_all_updates () at gdkwindow.c:5691
#74 0x0000003e7a6422f9 in gdk_window_update_idle (data=<value optimized out>) at gdkwindow.c:5317
#75 0x0000003e7a61e1a6 in gdk_threads_dispatch (data=0x1aba2c0) at gdk.c:512
#76 0x00000038d3a41e33 in g_main_dispatch (context=0x6424a0) at gmain.c:2149
#77 g_main_context_dispatch (context=0x6424a0) at gmain.c:2702
#78 0x00000038d3a42610 in g_main_context_iterate (context=0x6424a0, block=0, dispatch=1, self=<value optimized out>) at gmain.c:2780
#79 0x00000038d3a428ad in g_main_context_iteration (context=0x6424a0, may_block=0) at gmain.c:2843
#80 0x00007ffff673f680 in nsAppShell::ProcessNextNativeEvent (this=0x99b000, mayWait=0) at /home/sfink/src/TM-crashme/widget/src/gtk2/nsAppShell.cpp:144
#81 0x00007ffff6764dd5 in nsBaseAppShell::DoProcessNextNativeEvent (this=0x99b000, mayWait=0) at /home/sfink/src/TM-crashme/widget/src/xpwidgets/nsBaseAppShell.cpp:173
#82 0x00007ffff6765169 in nsBaseAppShell::OnProcessNextEvent (this=0x99b000, thr=0x6baba0, mayWait=0, recursionDepth=0) at /home/sfink/src/TM-crashme/widget/src/xpwidgets/nsBaseAppShell.cpp:315
#83 0x00007ffff6afdaf6 in nsThread::ProcessNextEvent (this=0x6baba0, mayWait=0, result=0x7fffffffd45c) at /home/sfink/src/TM-crashme/xpcom/threads/nsThread.cpp:597
#84 0x00007ffff6a87718 in NS_ProcessNextEvent_P (thread=0x6baba0, mayWait=0) at nsThreadUtils.cpp:250
#85 0x00007ffff68ca9a8 in mozilla::ipc::MessagePump::Run (this=0x6b9be0, aDelegate=0x6a7f20) at /home/sfink/src/TM-crashme/ipc/glue/MessagePump.cpp:110
#86 0x00007ffff6b6703b in MessageLoop::RunInternal (this=0x6a7f20) at /home/sfink/src/TM-crashme/ipc/chromium/src/base/message_loop.cc:219
#87 0x00007ffff6b66fc0 in MessageLoop::RunHandler (this=0x6a7f20) at /home/sfink/src/TM-crashme/ipc/chromium/src/base/message_loop.cc:202
#88 0x00007ffff6b66f51 in MessageLoop::Run (this=0x6a7f20) at /home/sfink/src/TM-crashme/ipc/chromium/src/base/message_loop.cc:176
#89 0x00007ffff6764e61 in nsBaseAppShell::Run (this=0x99b000) at /home/sfink/src/TM-crashme/widget/src/xpwidgets/nsBaseAppShell.cpp:192
#90 0x00007ffff64aed11 in nsAppStartup::Run (this=0x975da0) at /home/sfink/src/TM-crashme/toolkit/components/startup/src/nsAppStartup.cpp:191
#91 0x00007ffff53acc93 in XRE_main (argc=4, argv=0x7fffffffe0b8, aAppData=0x608cd0) at /home/sfink/src/TM-crashme/toolkit/xre/nsAppRunner.cpp:3695
#92 0x000000000040123c in main (argc=4, argv=0x7fffffffe0b8) at /home/sfink/src/TM-crashme/browser/app/nsBrowserApp.cpp:158
(gdb)
Comment 41 John J. Barton 2011-01-13 14:58:04 PST
(In reply to comment #37)
> Created attachment 503602 [details]
> Prefs file triggering crash
> 
> Well, I'm deep in "wtf?!" territory, but I was able to go from no crash ->
> crash with the attached prefs.js file.

Just FYI, you have FBS_CREATION set which produces a huge amount of tracing data: for every function compiled it issues about 4 records and decompiles the source twice.
Comment 42 John J. Barton 2011-01-13 15:12:40 PST
I created a new profile with just released Firebug 1.7a8 and FBTest from SVN. I can't get it to crash.  Added some tracing prefs, no crash yet.

Steve, I don't have the those entries in the profile that does crash.
Comment 43 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-13 15:15:45 PST
I got some more-sensible results from valgrind with --freelist-vol=2000000000... though the best I can think of for what it means is that there was a reference-counting error.
Comment 44 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-13 15:18:48 PST
... or, I suppose, a failure to rebuild the rule hash when we needed to, since the rule hash doesn't have strong pointers.
Comment 45 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-13 15:28:48 PST
Created attachment 503648 [details]
valgrind warnings
Comment 46 John J. Barton 2011-01-13 15:57:27 PST
In the profiles where you are sure you see this crash, do you have firebug in an .xpi file or in a folder (by link file or not)? 

In running the test list, are you using http or https?

I tried a lot of times to crash with firebug in .xpi and http test list: no. Tried your prefs.js. no.

Then I expanded the .xpi into a folder and used https test list. crashed
http://crash-stats.mozilla.com/report/index/bp-66190696-3fc4-47c2-9f2b-eb0f92110113
http://crash-stats.mozilla.com/report/index/d50cf66e-a1a3-4b7a-99d0-4b0fc2110113
Comment 47 Steve Fink [:sfink] [:s:] 2011-01-13 16:06:56 PST
All of my tests are with both firebug and fbtest installed via links to a directory elsewhere.

I run the test list with https.
Comment 48 John J. Barton 2011-01-13 16:20:27 PST
Ok I can make it crash a lot with this patch to net.js

Index: net.js

===================================================================

--- net.js	(revision 8935)

+++ net.js	(working copy)

@@ -1629,8 +1629,22 @@

             maxWidth = "15%";
 
         // This call must precede all getCSSStyleRules calls
-        Firebug.CSSModule.cleanupSheets(hrefLabel.ownerDocument, Firebug.currentContext);
+        //Firebug.CSSModule.cleanupSheets(hrefLabel.ownerDocument, Firebug.currentContext);
         var rules = domUtils.getCSSStyleRules(hrefLabel);
+        for (var crash = 0; crash < 20; crash++)
+        {
+            for (var i = 0; i < rules.Count(); ++i)
+            {
+                var rule = QI(rules.GetElementAt(i), Ci.nsIDOMCSSStyleRule);
+                if (rule.selectorText == ".netHrefLabel")
+                {
+                    var style = rule.style;
+                    var paddingLeft = parseInt(style.getPropertyValue("padding-left"));
+                    window.dump("paddingLeft before "+paddingLeft+this.context.window.location+"\n");
+                }
+            }
+
+        }
         for (var i = 0; i < rules.Count(); ++i)
         {
             var rule = QI(rules.GetElementAt(i), Ci.nsIDOMCSSStyleRule);
@@ -1642,6 +1656,21 @@

                 break;
             }
         }
+        for (var crash = 0; crash < 20; crash++)
+        {
+            for (var i = 0; i < rules.Count(); ++i)
+            {
+                var rule = QI(rules.GetElementAt(i), Ci.nsIDOMCSSStyleRule);
+                if (rule.selectorText == ".netHrefLabel")
+                {
+                    var style = rule.style;
+                    var paddingLeft = parseInt(style.getPropertyValue("padding-left"));
+                    window.dump("paddingLeft "+paddingLeft+" "+this.context.window.location+"\n");
+                }
+            }
+
+        }
+
     },
 });
Comment 49 Steve Fink [:sfink] [:s:] 2011-01-13 16:21:46 PST
*** Bug 614212 has been marked as a duplicate of this bug. ***
Comment 50 John J. Barton 2011-01-13 16:50:13 PST
Well I take it back. The extra loops *seem* to make it crash more in the profile where I crash. But the crash does not happen in those loops and it does not make the crash-free profile crash either.  

So the profile does make a difference. Somehow.
Comment 51 Steve Fink [:sfink] [:s:] 2011-01-13 17:03:00 PST
The trigger seems to be somewhat subtle, and probably isn't directly associated with what's actually going wrong, so I doubt there's a whole lot of benefit in narrowing it down further. At this point, someone with a clue (dbaron) is in possession of both a reproducible test case and some juicy valgrind traces, so we're in fairly good shape.

dbaron, anything else we could provide that might help out?
Comment 52 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-13 20:01:29 PST
I can repro the crash now, so I think I'm fine.


The reference counting looks ok, so it seems like a failure to clear rule cascades.  That said, it goes through nsCSSStyleSheet::ReplaceStyleRule, which calls DidDirty, which clears rule cascades.  So I'm not sure what went wrong.
Comment 53 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-14 00:34:20 PST
The rule in question is in chrome://firebug/skin/net.css, which is an @import-ed sheet.  The sheet seems to incorrectly have a null mParent, which makes ClearRuleCascades() ineffective.  I think this may be because the way we rebuild a child list after a clone sets the child list correctly, but doesn't set the parent pointer correctly.
Comment 54 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-14 00:39:15 PST
Except ChildSheetListBuilder should have taken care of that, so that's probably not the cause.

But, nevertheless, it has a non-null mOwnerRule and a null mParent and null mDocument.

If I follow its mOwnerRule->mSheet, it is in fact in its "parent"'s child list.
Comment 55 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-14 00:52:49 PST
I bet the problem is that nsCSSStyleSheetInner::RemoveSheet only calls SetStyleSheetReference, which only sets the sheet on the import rule (and doesn't propagate that to setting the parent on the import rule's sheet).  Then we later destroy the sheet that was removed, which nulls out the mParent on the child sheet that is really no longer its own.
Comment 56 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-14 01:12:58 PST
I'd also note that a trick that made this substantially more easily reproducable was commenting out these lines:

@@ -1092,19 +1092,19 @@ nsStyleSet::NotifyStyleContextDestroyed(
   // could be a style context from the new ruletree!
   if (!aStyleContext->GetParent()) {
     mRoots.RemoveElement(aStyleContext);
   }
 
   if (mInReconstruct)
     return;
 
-  if (mUnusedRuleNodeCount == kGCInterval) {
+  //if (mUnusedRuleNodeCount == kGCInterval) {
     GCRuleTrees();
-  }
+  //}
 }
Comment 57 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-14 01:16:47 PST
Created attachment 503775 [details] [diff] [review]
patch
Comment 58 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-14 01:18:15 PST
Also, I think this should be hidden until we fix it on branches; it seems likely to be exploitable.
Comment 59 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-14 01:29:10 PST
And at a cursory glance it looks like the bug dates back to:

3.62 <peterl@netscape.com> 1999-06-02 18:57
added modified state
added cloning with actual copy on write
Comment 60 Boris Zbarsky [:bz] 2011-01-14 06:59:39 PST
Comment on attachment 503775 [details] [diff] [review]
patch

r=me
Comment 61 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-14 10:33:29 PST
Created attachment 503886 [details] [diff] [review]
crashtest (to be landed later)
Comment 62 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-14 12:24:29 PST
Created attachment 503942 [details] [diff] [review]
crashtest (to be landed later)

somewhat more minimal
Comment 63 Steve Fink [:sfink] [:s:] 2011-01-14 14:07:47 PST
The patch is working great for me. It's really nice to be able to make it all the way through the test suite so you can tell what passed and failed.

Thanks for the late night!
Comment 64 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-14 21:52:20 PST
http://hg.mozilla.org/mozilla-central/rev/3d4620449437
Comment 65 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-14 23:00:54 PST
Created attachment 504073 [details] [diff] [review]
1.9.2 patch
Comment 66 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-14 23:01:19 PST
Created attachment 504074 [details] [diff] [review]
1.9.1 patch
Comment 67 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-14 23:09:37 PST
And I confirmed that my crashtest (attachment 503942 [details] [diff] [review]) crashes on 3.5.16:
bp-c3ae3f50-1a41-4329-90da-1ae382110114
and on 3.6.13:
bp-3a2e0342-4906-4758-904d-4877f2110114

(Socorro hasn't yet processed the crashes... I should probably double-check those links in a bit once it has.)
Comment 68 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-15 10:04:44 PST
I think those crash reports got lost due to the outage.  Tried again:
3.6.13: bp-718ec687-e607-4bf6-8a36-f0ae62110115
3.5.16: bp-a82075b0-d8c2-48d9-92c2-4cf5a2110115
Comment 69 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-15 12:44:14 PST
I verified via try server (using the 32-bit Linux build) that the 1.9.2 patch in fact fixes the crash in the crashtest.

And, for comparison, the two crashes in the current 1.9.2 nightly build that I got while testing the nightly and the try server build with the same steps were:
bp-93d3f979-3ee3-4d28-b552-68fc12110115
bp-041c3567-1e0d-4a2a-8835-ef8302110115
Comment 70 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-15 16:35:13 PST
(In reply to comment #69)
> I verified via try server (using the 32-bit Linux build) that the 1.9.2 patch
> in fact fixes the crash in the crashtest.

And I did the same thing for a try server build of the 1.9.1 patch (against a current 1.9.1 nightly).

> And, for comparison, the two crashes in the current 1.9.2 nightly build that I
> got while testing the nightly and the try server build with the same steps
> were:

And the crashes from the 1.9.1 nightly were:
bp-d52db773-2733-42b6-b914-4e7f42110115
bp-ee1c21a1-9d5e-4339-adc5-7120f2110115
Comment 71 Daniel Veditz [:dveditz] 2011-01-21 11:01:20 PST
The crash-test doesn't require Firebug or any special privs, right? (doesn't look like a chrome test.)
Comment 72 Daniel Veditz [:dveditz] 2011-01-24 10:17:27 PST
Comment on attachment 504073 [details] [diff] [review]
1.9.2 patch

Approved for 1.9.2.15 and 1.9.1.18, a=dveditz for release-drivers
Comment 73 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-24 12:45:02 PST
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/472fdb1e2bda
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/ed900f665114
Comment 74 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2011-01-24 14:10:07 PST
(In reply to comment #71)
> The crash-test doesn't require Firebug or any special privs, right? (doesn't
> look like a chrome test.)

Nope, the crash test is just a Web page that crashes Firefox.  That's why I want to get this fixed on branches.
Comment 76 Daniel Veditz [:dveditz] 2011-03-04 10:15:06 PST
The "3.6.15" we're releasing today does not fix this bug, the release containing this bug fix has been renamed to "3.6.16" and the bugzilla flags will be updated to reflect that soon. Today's release is a re-release of 3.6.14 plus a fix for a bug that prevented many Java applets from starting up.

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