The default bug view has changed. See this FAQ.

virtual call through dangling style rule pointer resulting from incorrect parent on @imported sheet after COW cloning (Crash running Firebug test suite)

RESOLVED FIXED in mozilla2.0b10

Status

()

Core
CSS Parsing and Computation
P2
normal
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: sfink, Assigned: dbaron)

Tracking

unspecified
mozilla2.0b10
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(blocking2.0 final+, blocking1.9.2 .17+, status1.9.2 .17-fixed, blocking1.9.1 .19+, status1.9.1 .19-fixed)

Details

(Whiteboard: [firebug-p1][hardblocker][sg:critical?])

Attachments

(8 attachments, 3 obsolete attachments)

(Reporter)

Description

6 years ago
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.
(Assignee)

Comment 1

6 years ago
What Firefox version?
(Reporter)

Comment 2

6 years ago
tracemonkey, recent (32ff6481fba8).
Can you repeat that experiment under valgrind?

Comment 4

6 years ago
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.
Whiteboard: [firebug-p1]
(Assignee)

Comment 5

6 years ago
That shouldn't be the case anymore following bug 536379 and bug 578810.

Comment 6

6 years ago
For reference the 'dance' bit is discussed in bug 500365 and http://code.google.com/p/fbug/issues/detail?id=2440
(Assignee)

Comment 7

6 years ago
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.
(Assignee)

Updated

6 years ago
Component: Layout → Style System (CSS)
QA Contact: layout → style-system
(Assignee)

Updated

6 years ago
Summary: Crash in QueryInterface during RuleHash::EnumerateAllRules → Crash running Firebug test suite due to calling getCSSStyleRules without ensuring unique inners first
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.
(Reporter)

Comment 9

6 years ago
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).
(Reporter)

Comment 10

6 years ago
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.
(Reporter)

Comment 11

6 years ago
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.
Attachment #502114 - Attachment is obsolete: true

Comment 12

6 years ago
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

6 years ago
Bug 614527 is probably a dup, but it shows a normal Firebug user (not FBTest) crashing.
blocking2.0: --- → ?
(Reporter)

Comment 14

6 years ago
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.
Attachment #502121 - Attachment is obsolete: true
> 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???
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...

Updated

6 years ago
Blocks: 614212
Firebug worked around this, so we don't need to block.
blocking2.0: ? → -
Their workaround doesn't work, as expected.  See comment 12.  Renominating.
blocking2.0: - → ?
blocking2.0: ? → final+
(Assignee)

Updated

6 years ago
Assignee: nobody → dbaron
Priority: -- → P2
(Assignee)

Comment 19

6 years ago
sfink, could you confirm what's on nsStyleSet.cpp line 633 for you (or what hg revision you did the valgrind run on)?
(Reporter)

Comment 20

6 years ago
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...
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);
(Reporter)

Comment 22

6 years ago
(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.
(Assignee)

Comment 23

6 years ago
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).
(Reporter)

Comment 24

6 years ago
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.
Whiteboard: [firebug-p1] → [firebug-p1][hardblocker]
(Assignee)

Updated

6 years ago
Summary: Crash running Firebug test suite due to calling getCSSStyleRules without ensuring unique inners first → Crash running Firebug test suite
Will do.  I think it's pretty clear that the document sheets here are confused, though....
(Assignee)

Comment 26

6 years ago
(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.
(Assignee)

Comment 27

6 years ago
(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

6 years ago
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.
(Assignee)

Comment 29

6 years ago
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.
(Assignee)

Updated

6 years ago
Blocks: 614527
(Assignee)

Comment 30

6 years ago
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
(Reporter)

Comment 31

6 years ago
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...
(Assignee)

Comment 32

6 years ago
I'm on Linux64 as well.

What's the "desktop box" that you can't repro on?
(Reporter)

Comment 33

6 years ago
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.
(Reporter)

Comment 34

6 years ago
(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.
(Assignee)

Comment 35

6 years ago
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...)
(Reporter)

Comment 36

6 years ago
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.
(Reporter)

Comment 37

6 years ago
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

6 years ago
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.
(Assignee)

Comment 39

6 years ago
I see the crash with that prefs.js (substituted with my old extensions.installCache).
(Reporter)

Comment 40

6 years ago
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

6 years ago
(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

6 years ago
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.
(Assignee)

Comment 43

6 years ago
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.
(Assignee)

Comment 44

6 years ago
... or, I suppose, a failure to rebuild the rule hash when we needed to, since the rule hash doesn't have strong pointers.
(Assignee)

Comment 45

6 years ago
Created attachment 503648 [details]
valgrind warnings

Comment 46

6 years ago
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
(Reporter)

Comment 47

6 years ago
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

6 years ago
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");
+                }
+            }
+
+        }
+
     },
 });
OS: Linux → Windows CE
(Reporter)

Updated

6 years ago
Duplicate of this bug: 614212

Comment 50

6 years ago
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.
(Reporter)

Comment 51

6 years ago
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?
(Assignee)

Comment 52

6 years ago
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.
(Assignee)

Comment 53

6 years ago
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.
(Assignee)

Comment 54

6 years ago
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.
(Assignee)

Comment 55

6 years ago
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.
(Assignee)

Comment 56

6 years ago
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();
-  }
+  //}
 }
(Assignee)

Comment 57

6 years ago
Created attachment 503775 [details] [diff] [review]
patch
Attachment #503775 - Flags: review?(bzbarsky)
(Assignee)

Comment 58

6 years ago
Also, I think this should be hidden until we fix it on branches; it seems likely to be exploitable.
Group: core-security
(Assignee)

Updated

6 years ago
Flags: in-testsuite?
(Assignee)

Updated

6 years ago
OS: Windows CE → Linux
(Assignee)

Comment 59

6 years ago
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
(Assignee)

Updated

6 years ago
Whiteboard: [firebug-p1][hardblocker] → [firebug-p1][hardblocker][needs review]
Comment on attachment 503775 [details] [diff] [review]
patch

r=me
Attachment #503775 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 61

6 years ago
Created attachment 503886 [details] [diff] [review]
crashtest (to be landed later)
(Assignee)

Comment 62

6 years ago
Created attachment 503942 [details] [diff] [review]
crashtest (to be landed later)

somewhat more minimal
Attachment #503886 - Attachment is obsolete: true
(Reporter)

Comment 63

6 years ago
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!
(Assignee)

Comment 64

6 years ago
http://hg.mozilla.org/mozilla-central/rev/3d4620449437
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Updated

6 years ago
Whiteboard: [firebug-p1][hardblocker][needs review] → [firebug-p1][hardblocker]
Target Milestone: --- → mozilla2.0b10
(Assignee)

Updated

6 years ago
Summary: Crash running Firebug test suite → virtual call through dangling style rule pointer resulting from incorrect parent on @imported sheet after COW cloning (Crash running Firebug test suite)
Whiteboard: [firebug-p1][hardblocker] → [firebug-p1][hardblocker][sg:critical?]
(Assignee)

Comment 65

6 years ago
Created attachment 504073 [details] [diff] [review]
1.9.2 patch
Attachment #504073 - Flags: approval1.9.2.15?
(Assignee)

Comment 66

6 years ago
Created attachment 504074 [details] [diff] [review]
1.9.1 patch
Attachment #504074 - Flags: approval1.9.1.18?
(Assignee)

Updated

6 years ago
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
(Assignee)

Comment 67

6 years ago
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.)
(Assignee)

Comment 68

6 years ago
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
(Assignee)

Comment 69

6 years ago
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
(Assignee)

Comment 70

6 years ago
(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
(Assignee)

Updated

6 years ago
OS: Linux → All
Hardware: x86_64 → All
The crash-test doesn't require Firebug or any special privs, right? (doesn't look like a chrome test.)
blocking1.9.1: ? → .18+
blocking1.9.2: ? → .15+
status1.9.1: --- → wanted
status1.9.2: --- → wanted
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
Attachment #504073 - Flags: approval1.9.2.15? → approval1.9.2.15+
Attachment #504074 - Flags: approval1.9.1.18? → approval1.9.1.18+
(Assignee)

Comment 73

6 years ago
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/472fdb1e2bda
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/ed900f665114
status1.9.1: wanted → .18-fixed
status1.9.2: wanted → .15-fixed
(Assignee)

Comment 74

6 years ago
(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.
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.
Group: core-security
You need to log in before you can comment on or make changes to this bug.