Last Comment Bug 602115 - Crash [@ txExecutionState::popTemplateRule]
: Crash [@ txExecutionState::popTemplateRule]
Status: RESOLVED FIXED
: [sg:critical?]
: crash, testcase, verified1.9.1, verified1.9.2
Product: Core
Classification: Components
Component: XSLT
: Trunk
: x86 Mac OS X
: -- critical (vote)
: ---
Assigned To: Jonas Sicking (:sicking)
: xslt
:
:
: 326633 637226
  Show dependency treegraph
 
Reported: 2010-10-05 17:53 PDT by Jesse Ruderman
Modified: 2011-05-23 17:56 PDT (History)
8 users (show)
See Also:
Crash Signature:
[@ txExecutionState::popTemplateRule]
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  final+
  ---
  .14+
  .14-fixed
  .17+
  .17-fixed


Attachments
testcase (crashes Firefox when loaded) (265 bytes, text/html)
2010-10-05 17:53 PDT, Jesse Ruderman
no flags Details
stack trace (9.37 KB, text/plain)
2010-10-05 17:55 PDT, Jesse Ruderman
no flags Details
Patch to fix (8.72 KB, patch)
2010-11-24 00:46 PST, Jonas Sicking (:sicking)
peterv: review+
Details | Diff | Splinter Review
branch patch (8.70 KB, patch)
2011-01-19 00:17 PST, Jonas Sicking (:sicking)
no flags Details | Diff | Splinter Review
branch patch (8.70 KB, patch)
2011-01-19 00:22 PST, Jonas Sicking (:sicking)
dveditz: approval1.9.2.14+
dveditz: approval1.9.1.17+
Details | Diff | Splinter Review
1.8 version (3.26 KB, patch)
2011-01-28 07:43 PST, Martin Stránský
no flags Details | Diff | Splinter Review

Summon comment box

Description Jesse Ruderman 2010-10-05 17:53:38 PDT
Created attachment 481111 [details]
testcase (crashes Firefox when loaded)
Comment 1 Jesse Ruderman 2010-10-05 17:55:39 PDT
Created attachment 481112 [details]
stack trace
Comment 2 Jesse Ruderman 2010-10-05 17:56:24 PDT
Crash address 0xfffffffffffffff8 is kinda scary.
Comment 3 Boris Zbarsky (:bz) 2010-10-05 21:16:10 PDT
439         NS_IF_RELEASE(mTemplateRules[mTemplateRuleCount].mModeLocalName);

mTemplateRuleCount is -1.

The popTemplateRule call is coming from end()... but we never called pushTemplateRule, because we bailed out from init() like so:

169     NS_ENSURE_TRUE(document, NS_ERROR_FAILURE);

and the caller in txMozillaXSLTProcessor::TransformToDoc didn't check the rv and pressed on.
Comment 4 Damon Sicore (:damons) 2010-10-19 14:06:11 PDT
Jonas, progress here?
Comment 5 Johnny Stenback (:jst, jst@mozilla.com) 2010-11-09 14:22:00 PST
Jonas?
Comment 6 Jonas Sicking (:sicking) 2010-11-23 11:22:57 PST
I'll start looking at this today
Comment 7 Jonas Sicking (:sicking) 2010-11-24 00:46:45 PST
Created attachment 492934 [details] [review]
Patch to fix

Our error handling here is way busted. We even ignore errors generated from txExecutionState.init.

This patch makes us honor that error, and skips the template rule cleanup when the transformation failed for any reason. Instead template rules are properly owned by an nsTArray which simplifies things a lot.
Comment 8 Peter Van der Beken [:peterv] 2010-12-06 09:15:09 PST
Comment on attachment 492934 [details] [review]
Patch to fix

>+  var docType = document.implementation.createDocumentType(undefined, '', '');
>+  var doc = document.implementation.createDocument('', '', null);
>+  var xp = new XSLTProcessor;
>+  xp.importStylesheet(doc);
>+  xp.transformToDocument(docType);
>+}
>+catch (ex) {}
>+
>+try {
>+  var docType = document.implementation.createDocumentType(undefined, '', '');
>+  var doc = document.implementation.createDocument('', '', null);
>+  var xp = new XSLTProcessor;

Don't think you need the |var|s again here.

> nsresult

This should be void now, it only returns NS_OK.

> txExecutionState::pushTemplateRule(txStylesheet::ImportFrame* aFrame,
Comment 9 Jonas Sicking (:sicking) 2010-12-13 14:30:53 PST
Checked in http://hg.mozilla.org/mozilla-central/rev/7127fb5bc918
Comment 10 Daniel Veditz 2011-01-06 16:10:17 PST
This crashes 3.6.x too
Comment 11 Daniel Veditz 2011-01-07 10:12:37 PST
Blocking branch releases, please work up a back-ported patch if necessary
Comment 12 Daniel Veditz 2011-01-19 00:01:19 PST
Comment on attachment 492934 [details] [review]
Patch to fix

>-    TemplateRule* mTemplateRules;
>-    PRInt32 mTemplateRulesBufferSize;
>-    PRInt32 mTemplateRuleCount;
>+    AutoInfallibleTArray<TemplateRule, 10> mTemplateRules;

Neither branch has AutoInfallibleTArray. Did this compile in your tree?
Comment 13 Jonas Sicking (:sicking) 2011-01-19 00:12:07 PST
No, that was the only change I had to make (which also meant returning OOM as needed)
Comment 14 Jonas Sicking (:sicking) 2011-01-19 00:17:50 PST
Created attachment 505007 [details] [review]
branch patch
Comment 15 Jonas Sicking (:sicking) 2011-01-19 00:22:20 PST
Created attachment 505009 [details] [review]
branch patch

forgot to refresh
Comment 16 Daniel Veditz 2011-01-19 09:55:07 PST
Comment on attachment 505009 [details] [review]
branch patch

Approved for 1.9.2.14 and 1.9.1.17, a=dveditz
Comment 18 Jonas Sicking (:sicking) 2011-01-19 10:15:19 PST
Somehow missed part of the patch when moving it to the 1.9.1 branch. Followup landed (this is in the approved patch above):

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/7f3448d65808
Comment 19 Al Billings [:abillings] 2011-01-20 17:14:06 PST
Verified fixed for 1.9.2 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.14pre) Gecko/20110120 Namoroka/3.6.14pre. Verified crash in 1.9.2.13.

Verified fixed for 1.9.1 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.1.17pre) Gecko/20110120 Shiretoko/3.5.17pre. Verified crash in 1.9.1.16.
Comment 20 Martin Stránský 2011-01-28 07:43:43 PST
Created attachment 507857 [details] [review]
1.8 version
Comment 21 Daniel Veditz 2011-05-23 17:56:44 PDT
*** Bug 637226 has been marked as a duplicate of this bug. ***

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