It jumped up to #52 top browser crasher in 18.0.1 and #47 in 19.0b3. It has the same abort message as bug 814007, bug 802152, bug 776473, bug 767343. It's correlated to recent versions of crossrider extensions: mozalloc_abort(char const* const) | NS_DebugBreak_P | AppendUTF8toUTF16(nsACString_internal const&, nsAString_internal&)|EXCEPTION_BREAKPOINT (212 crashes) 30% (63/212) vs. 0% (623/176971) crossriderapp4479@crossrider.com (Giant Savings) 0% (0/212) vs. 0% (1/176971) 0.81.12 0% (0/212) vs. 0% (3/176971) 0.83.20 0% (0/212) vs. 0% (1/176971) 0.85.42 0% (0/212) vs. 0% (1/176971) 0.85.43 30% (63/212) vs. 0% (617/176971) 0.86.44 13% (27/212) vs. 0% (117/176971) crossriderapp4639@crossrider.com (SavingsApp) 0% (0/212) vs. 0% (2/176971) 0.84.35 13% (27/212) vs. 0% (115/176971) 0.86.44 16% (34/212) vs. 4% (7440/176971) plugin@yontoo.com (1.20.00) Signature mozalloc_abort(char const* const) | NS_DebugBreak_P | AppendUTF8toUTF16(nsACString_internal const&, nsAString_internal&) More Reports Search UUID 69aadc97-4b29-4455-8d70-c12902130130 Date Processed 2013-01-30 13:13:52 Uptime 6953 Install Age 1.5 weeks since version was first installed. Install Time 2013-01-19 19:09:20 Product Firefox Version 18.0.1 Build ID 20130116073211 Release Channel release OS Windows NT OS Version 5.1.2600 Service Pack 3 Build Architecture x86 Build Architecture Info GenuineIntel family 6 model 23 stepping 6 Crash Reason EXCEPTION_BREAKPOINT Crash Address 0xfe1988 App Notes AdapterVendorID: 0x10de, AdapterDeviceID: 0x06e4, AdapterSubsysID: 10911019, AdapterDriverVersion: 6.14.12.6658 D3D10 Layers? D3D10 Layers- D3D9 Layers? D3D9 Layers+ xpcom_runtime_abort(###!!! ABORT: OOM: file e:\builds\moz2_slave\rel-m-rel-w32-bld\build\obj-firefox\dist\include\nsTSubstring.h, line 529) Processor Notes sp-processor10.phx1.mozilla.com_20265:2008 EMCheckCompatibility True Adapter Vendor ID 0x10de Adapter Device ID 0x06e4 Total Virtual Memory 2147352576 Available Virtual Memory 762540032 System Memory Use Percentage 99 Available Page File 2080358400 Available Physical Memory 16498688 Frame Module Signature Source 0 mozalloc.dll mozalloc_abort memory/mozalloc/mozalloc_abort.cpp:23 1 xul.dll NS_DebugBreak_P xpcom/base/nsDebugImpl.cpp:410 2 xul.dll AppendUTF8toUTF16 xpcom/string/src/nsReadableUtils.cpp:194 3 xul.dll xul.dll@0xd46ef0 4 xul.dll XPCConvert::NativeData2JS js/xpconnect/src/xpcprivate.h:3348 5 xul.dll nsMemory::Clone obj-firefox/xpcom/build/nsMemory.cpp:31 6 xul.dll XPCConvert::JSData2Native js/xpconnect/src/XPCConvert.cpp:459 7 xul.dll XPCWrappedNative::CallMethod js/xpconnect/src/XPCWrappedNative.cpp:2383 8 xul.dll XPC_WN_CallMethod js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1469 9 mozjs.dll js::mjit::JaegerShot js/src/methodjit/MethodJIT.cpp:1122 10 mozjs.dll js::Interpret js/src/jsinterp.cpp:2529 11 mozjs.dll js::RunScript js/src/jsinterp.cpp:324 12 mozjs.dll UncachedInlineCall js/src/methodjit/InvokeHelpers.cpp:363 13 mozjs.dll js::mjit::stubs::UncachedCallHelper js/src/methodjit/InvokeHelpers.cpp:451 14 mozjs.dll js::mjit::stubs::UncachedLoweredCall js/src/methodjit/InvokeHelpers.cpp:414 15 mozjs.dll js::mjit::stubs::CompileFunction js/src/methodjit/InvokeHelpers.cpp:244 16 mozjs.dll js::mjit::JaegerShot js/src/methodjit/MethodJIT.cpp:1122 17 mozjs.dll js::Interpret js/src/jsinterp.cpp:2529 ... More reports at: https://crash-stats.mozilla.com/report/list?signature=mozalloc_abort%28char+const*+const%29+|+NS_DebugBreak_P+|+AppendUTF8toUTF16%28nsACString_internal+const%26%2C+nsAString_internal%26%29

It's now #18 top browser crasher in 18.0.1, #6 in 19.0b4, #24 in 20.0a2, and #37 in 21.0a1. It's still correlated to Giant Savings and other known malware. * 18.0.1: 49% (552/1133) vs. 1% (1506/204664) crossriderapp4479@crossrider.com (Giant Savings) 0% (1/1133) vs. 0% (2/204664) 0.81.12 0% (1/1133) vs. 0% (5/204664) 0.81.13 0% (1/1133) vs. 0% (2/204664) 0.83.28 0% (1/1133) vs. 0% (2/204664) 0.85.42 41% (460/1133) vs. 1% (1260/204664) 0.86.44 5% (61/1133) vs. 0% (169/204664) 0.87.66 2% (27/1133) vs. 0% (57/204664) 0.88.67 19% (216/1133) vs. 4% (8375/204664) plugin@yontoo.com (1.20.00) 15% (167/1133) vs. 0% (408/204664) crossriderapp4639@crossrider.com (SavingsApp) 0% (2/1133) vs. 0% (2/204664) 0.82.14 15% (165/1133) vs. 0% (405/204664) 0.86.44 16% (182/1133) vs. 3% (7053/204664) ffxtlbr@babylon.com 0% (2/1133) vs. 0% (20/204664) 1.1.3 7% (79/1133) vs. 1% (1689/204664) 1.1.9 2% (24/1133) vs. 1% (1283/204664) 1.2.0 7% (77/1133) vs. 2% (4044/204664) 1.5.0 13% (147/1133) vs. 3% (5345/204664) ffxtlbr@funmoods.com 5% (61/1133) vs. 0% (985/204664) 1.5.0 8% (86/1133) vs. 2% (4360/204664) 1.5.1 * 19.0 Beta: 43% (73/168) vs. 0% (210/56681) crossriderapp4479@crossrider.com (Giant Savings) 2% (4/168) vs. 0% (4/56681) 0.83.33 37% (62/168) vs. 0% (176/56681) 0.86.44 4% (6/168) vs. 0% (20/56681) 0.87.66 1% (1/168) vs. 0% (10/56681) 0.88.67 26% (43/168) vs. 4% (2530/56681) ffxtlbr@babylon.com 7% (12/168) vs. 1% (436/56681) 1.1.9 2% (3/168) vs. 1% (307/56681) 1.2.0 17% (28/168) vs. 3% (1777/56681) 1.5.0 17% (29/168) vs. 5% (2632/56681) plugin@yontoo.com (1.20.00)

Jorge - how do you feel about blocking these malware add-ons? Have we done so in the past?

Giant Savings (http://giant-savings.com/) displays ad popups (it promises to offer you coupon when shopping online). Other pieces of adware are not enough correlated to blocklist them.

With combined signatures, it's even worse: #10 in 18.0.1, #4 in 19.0b4, and #9 in 20.0a2. More reports also at: https://crash-stats.mozilla.com/report/list?signature=mozalloc_abort%28char+const*+const%29+|+NS_DebugBreak_P+|+AppendUTF16toUTF8%28nsAString_internal+const%26%2C+nsACString_internal%26%29

(In reply to Alex Keybl [:akeybl] from comment #2) > Jorge - how do you feel about blocking these malware add-ons? Have we done > so in the past? It looks like the strongest correlation is Giant Savings. The others are very widely installed, so it's possible they aren't the real cause. Incidentally, see blocklist bug 835664 for Yontoo and bug 804847 for Funmoods. I'll try contacting the Giant Savings and Crossrider people

Crossrider pushed an update that may help with this. We should check the crash stats again in a couple of days and see if there's any change.

The volume of this has been going down since 2013-02-05 data, but we need to keep watching it for a few days longer.

(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #7) > The volume of this has been going down since 2013-02-05 data, but we need to > keep watching it for a few days longer. I don't know where you've got your data but it's still high with no downward tendency, currently #2 top browser crasher in 18.0.2, #4 in 19.0b4 and #3 in 20.0a2 over the last day. (In reply to Jorge Villalobos [:jorgev] from comment #6) > Crossrider pushed an update that may help with this. We should check the > crash stats again in a couple of days and see if there's any change. The latest version of those extensions is still affected: mozalloc_abort(char const* const) | NS_DebugBreak_P | AppendUTF8toUTF16(nsACString_internal const&, nsAString_internal&)|EXCEPTION_BREAKPOINT (1780 crashes) 44% (792/1780) vs. 1% (1465/122260) crossriderapp4479@crossrider.com 0% (0/1780) vs. 0% (2/122260) 0.81.12 0% (1/1780) vs. 0% (2/122260) 0.81.13 0% (2/1780) vs. 0% (2/122260) 0.83.28 0% (0/1780) vs. 0% (1/122260) 0.83.33 0% (3/1780) vs. 0% (3/122260) 0.84.36 0% (8/1780) vs. 0% (15/122260) 0.86.44 1% (9/1780) vs. 0% (20/122260) 0.87.66 1% (16/1780) vs. 0% (33/122260) 0.88.67 42% (753/1780) vs. 1% (1387/122260) 0.88.83 18% (321/1780) vs. 1% (629/122260) crossriderapp4637@crossrider.com 0% (3/1780) vs. 0% (4/122260) 0.83.24 0% (1/1780) vs. 0% (1/122260) 0.83.26 0% (1/1780) vs. 0% (1/122260) 0.85.34 18% (316/1780) vs. 1% (617/122260) 0.87.43 0% (0/1780) vs. 0% (6/122260) 0.88.58 12% (210/1780) vs. 0% (392/122260) crossriderapp4493@crossrider.com 0% (0/1780) vs. 0% (2/122260) 0.81.12 0% (1/1780) vs. 0% (1/122260) 0.85.39 0% (5/1780) vs. 0% (8/122260) 0.85.40 9% (163/1780) vs. 0% (315/122260) 0.88.64 2% (41/1780) vs. 0% (66/122260) 0.88.77

(In reply to Scoobidiver from comment #8) > (In reply to Robert Kaiser (:kairo@mozilla.com) from comment #7) > > The volume of this has been going down since 2013-02-05 data, but we need to > > keep watching it for a few days longer. > I don't know where you've got your data I saw this in terms of crashes per M ADI in my explosiveness reports but the trend didn't hold up, as you also mentioned.

Because we don't know yet if this requires work on our end, leaving it nominated for tracking while more info is gathered.

Benjamin was planning on taking a look to see if we could make any recommendations to the Giant Savings devs. I'd actually like to keep this on the FF19 tracking list in case we feel a blocklist is necessary.

Bah, I wrote a big bug comment here but it apparently got lost. In case it's not clear, the bug here is that we're trying to create a string which is so long that we're failing to allocate memory for it and then aborting. The stack is unfortunately kinda busted above AppendUTF8toUTF16 (it's even worse with MSVC), but I went in using dump_lookup and the most likely caller is http://hg.mozilla.org/releases/mozilla-release/annotate/eecd28b7ba09/modules/libpref/src/nsPrefBranch.cpp#l313 So is it possible that this extension is saving very large amounts of data to the prefservice, or somehow else corrupting prefs or the pref file?

Hmm, nsPrefBranch... This isn't likely to be another casualty of the missed UUID change on nsIPrefBranch, I hope?

That seems pretty unlikely unless crossrider is using a binary component; but I'm pretty sure they are not.

My current theory is that this is actually caused by somebody using the prefserver from an incorrect thread, and is probably only incidentally triggered by extensions asking for prefs at particular times or in certain patterns. This means it would probably be fixed by the things attached to bug 619487. I have no clue how to prove this theory, though, other than wait for those bugs.

I've asked for a copy of the add-on to get QA engaged here for a repro case, given the fact that this remains a top crasher.

Here it is. We're ready for QA now.

(In reply to Scoobidiver from comment #3) > Giant Savings (http://giant-savings.com/) displays ad popups (it promises to > offer you coupon when shopping online). > Other pieces of adware are not enough correlated to blocklist them. I can't figure out how to use this addon. In addons manager it says: "Save big with Giant Savings! Coupons display instantly while you're shopping online!" But I didn't get any notifications on amazon, ebay, sportsdirect and on several other sites resulted from a "shop online coupons" google search. Any thoughts?

My theory from comment 15 is weak because in no case that we know about are we *writing* to the prefservice from another thread; we're only reading from it, which should not cause the pref table to become corrupted.

Any progress on this bug yet? Is there anything more QA can do to assist given that we haven't figured out how this add-on works?

These two patches aren't going to fix anything, but they will save the long or corrupted memory in the minidump.

Comment on attachment 720910 [details] [diff] [review] Make an infallible AeppendUTF8toUTF16 and clean up nsReadableUtils.cpp, rev. 1 r=me, but you don't have any concerns about shoving a string of potentially unbounded length into crash reports? Or is the crash reporter service going to take care of this for us?

Oops wrong patch

Comment on attachment 720910 [details] [diff] [review] Make an infallible AeppendUTF8toUTF16 and clean up nsReadableUtils.cpp, rev. 1 You might want to size-check utf8String.Length, since you're stuffing every byte of it into the minidump. Maybe cap it at a few kb?

Comment on attachment 720920 [details] [diff] [review] Do some custom OOM error reporting in the hopes of catching the relevant memory in the minidump to see what's going on, rev. 1 Review of attachment 720920 [details] [diff] [review]: ----------------------------------------------------------------- I hope this codepath isn't hit very frequently.

https://hg.mozilla.org/integration/mozilla-inbound/rev/917070d58a17 https://hg.mozilla.org/integration/mozilla-inbound/rev/821013d8066d

Comment on attachment 720920 [details] [diff] [review] Do some custom OOM error reporting in the hopes of catching the relevant memory in the minidump to see what's going on, rev. 1 User impact if declined: inability to diagnose crashes Testing completed (on m-c, etc.): just landed, no real way to test except for users who see the bug Risk to taking this patch (and alternatives if risky): This should be an OOM-only codepath change, so it should be safe. Doesn't seem especially risky to me. String or UUID changes made by this patch: None

https://hg.mozilla.org/mozilla-central/rev/917070d58a17 Note that the bug # in Part A was wrong...

https://hg.mozilla.org/releases/mozilla-beta/rev/b0b5a6da2a16 https://hg.mozilla.org/releases/mozilla-beta/rev/6a7999f4cc98 I'm not going to mark status-firefox20:fixed because that's not true.

Note to anyone else doing crash analysis: The patches landed here probably change the crash signatures, possibly to something in pref branch. The crashes do not get fixed by those patches, only hopefully easier to diagnose. We'll have to find out which signatures exactly they move to, and add them to this bug.

(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #32) > We'll have to find out which signatures exactly they move to, and add them to this bug. Found because of the comment in App Notes: https://crash-stats.mozilla.com/report/list?signature=mozalloc_abort%28char+const*+const%29+|+NS_DebugBreak_P+|+nsPrefBranch%3A%3AGetComplexValue%28char+const*%2C+nsID+const%26%2C+void**%29

From one of those reports: https://crash-stats.mozilla.com/report/index/cdbd01fe-66f8-4976-871a-80a1a2130312 "bug836263-size": "8016ef0" That's pretty big (128MB). I opened this dump up and extracted the saved memory region, and it appears to start with a copy of the jquery script: /*! jQuery v1.7.1 jquery.com | jquery.org/license */ (function(a,b){function cy( so, uh...

It's been a couple of releases and we did not get traction here for FF20, wontfixing.

Dropping QAWANTED since we've been unable to find steps to reproduce this crash. Please re-add if the changes landed result in more leads we can follow.

I retested this on the following URLs: www.1-800-bakery.com www.1800flowers.com I get a green Giant Savings widget in the top right on page load which displays some coupons on hover. I tried applying various coupons and playing around with the settings but I was not able to reproduce a crash. I tested with Firefox Nightly 22.0a1.

Some time ago I filed Bug 511135. Whether it has any relevance here I don't know, but I will offer that the frequency of that crash has increased for me in recent months perhaps in the same time frame as this issue would have started on trunk. Like this bug, I don't know why the increase in occurrences. Note: my crashes related to Bug 511135 get reported by breakpad as "Empty ..."

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #34) > From one of those reports: > https://crash-stats.mozilla.com/report/index/cdbd01fe-66f8-4976-871a- > 80a1a2130312 > > "bug836263-size": "8016ef0" > > That's pretty big (128MB). > > I opened this dump up and extracted the saved memory region, and it appears > to start with a copy of the jquery script: > /*! jQuery v1.7.1 jquery.com | jquery.org/license */ > (function(a,b){function cy( > > so, uh... Would it be possible to look into more of these? Partner/QA investigations are not producing anything interesting, so we're really just left with the extra diagnostics we introduced.

At this point, I don't have the bandwidth to look at more of these if I'm going to deal with bug 837835 at all.

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #34) > From one of those reports: > https://crash-stats.mozilla.com/report/index/cdbd01fe-66f8-4976-871a- > 80a1a2130312 > > "bug836263-size": "8016ef0" > > That's pretty big (128MB). > > I opened this dump up and extracted the saved memory region, and it appears > to start with a copy of the jquery script: > /*! jQuery v1.7.1 jquery.com | jquery.org/license */ > (function(a,b){function cy( > > so, uh... I pulled several dumps and they all have this.

Lets get the pref name.

Comment on attachment 732503 [details] [diff] [review] Moar diagnostics I'd like to land all three patches on beta-21 so we can get more insight here.

It looks like the Deals Plugin extension, which goes by crossriderapp4637@crossrider.com in https://crash-stats.mozilla.com/report/index/cdbd01fe-66f8-4976-871a-80a1a2130312 and extension21806@extension21806.com at dealsplugin.com, is the culprit. As far as I can tell, this is a horror show, with a version of the Crossrider framework which fetches a configuration file from https://w9u6a2p6.ssl.hwcdn.net/plugin/apps/21806/manifest/091/ff/manifest.xml, which it uses to fetch another configuration file from http://app-static.crossrider.com/plugin/apps/21806/plugins/091/ff/plugins.json, from which it downloads a list of JS files, which it saves to preferences, and then later executes (incidentally opening a hell of a MITM attack...)

So do we have contacts with the right people? Can we get them to fix it based on the information in comment 44?

Presumably we could just blocklist this version of this extension as well, since it's horribly broken?

Alternately, maybe we could just let these get*Pref methods fail in these cases? JS callers should already be handling exceptions since get*Pref can throw if the pref is unset, right?

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #46) > Presumably we could just blocklist this version of this extension as well, > since it's horribly broken? Unfortunately, as far as I can tell it's the framework that's at fault, so blocklisting one extension probably wouldn't help much.

So, is this problem present in all Crossrider extensions? What is different about the Deals Plugin extension?

This is different than the framework in the add-ons they submit to AMO. I'm not sure if it's a newer version, or they've just been packaging add-ons different when they export them for AMO, but the Crossrider add-ons they've submitted to us do not do this. It's pretty clear that this code is part of the framework, though.

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #47) > Alternately, maybe we could just let these get*Pref methods fail in these > cases? JS callers should already be handling exceptions since get*Pref can > throw if the pref is unset, right? I don't think so. Most extensions use getPrefType to decide which method to call, and the rest tend to just set it in the default branch assume it's set if they expect it to be. Either way, I think throwing rather than crashing is the right way to handle it, at least when the strings we're talking about are this large.

From all I've heard about those add-ons and about what the framework seems to do here, I'm not a friend of them and blocklisting sounds good to me if they are abusing our infrastructure (including the pref service). That said, I think if add-ons do seemingly crazy things like this, we should not crash but fail in a different way if possible. If that makes the add-on not work, that's OK, the user will notice that and uninstall that (and the devs will see what fails and be able to work on correcting it - getting an error/exception on a call from their code makes it way more debuggable from their side, actually). If the whole Firefox instance goes away crashing, that's bad. So, the "throw instead of crash" from comment #51 sounds to me like a good solution to me. :)

(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #52) > From all I've heard about those add-ons and about what the framework seems > to do here, I'm not a friend of them and blocklisting sounds good to me if > they are abusing our infrastructure (including the pref service). I don't think that blocklisting is an option. Even if we used a regexp block for the two known ID patterns above, that wouldn't cover them all. There's also the problem that there are non-foistware add-ons using versions of the Crossrider framework without this issue. It may be necessary to blocklist the add-ons with the highest correlations with this crash signature, though. In any case, I think we should see how the developers respond, and put blocklisting back on the table if it doesn't look like they can address it in a reasonable time frame.

I just forwarded the new information to Crossrider.

I had another look at the framework submitted to AMO and it *is* the same as in this add-on, but the code that fetches remote scripts is prefed off. Scripts are still fetched from within the XPI and stored in preferences, though. In any case, we haven't actually accepted any add-ons using this framework on AMO, so the point is academic.

(In reply to Kris Maglione [:kmag] from comment #53) > It may be necessary to blocklist the add-ons with the highest > correlations with this crash signature, though. Hm. This is now the #13 top crasher in 20 and the comments indicate that the crashes happen often. We should probably do this sooner rather than later.

We discussed this in today's stability meeting shortly, and both me and bsmedberg agreed that we should do someth9ing on our side here. It makes sense to limit the size of prefs being set and error out for huge ones instead of crashing. Can we please get a patch for doing that?

back to khuey

Something like this perhaps?

Comment on attachment 743345 [details] [diff] [review] Possible Patch Yeah, that should work. I was going to say 10k, though... thoughts?

I think how aggressive we should be depends on what branch we want to land this on.

(In reply to Benjamin Smedberg [:bsmedberg] from comment #60) > Yeah, that should work. I was going to say 10k, though... thoughts? kris%nawk '{ m = length($0); if (m > n) n = m } END { print n }' <prefs.js 51723 That's NoScript's CAPS site list. I checked a few other profiles I have floating around, and the smallest is 15K.

(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #61) > I think how aggressive we should be depends on what branch we want to land > this on. Well, I'm happy with this riding the trains, as the original crash has been fixed with add-on updates anyhow. The original crash here seems to be in the range of 128M for the string, see comment #34, and as Kris says in comment #62, NoScript seems to store a rather large (but not crashing) string with 15-51k, so we can either go for the 1MB in the existing patch, go down much further like bsmedberg suggests in comment #60 and message strongly to add-on authors that they need to fix any add-on that uses prefs that potentially are larger than the limit, or go with something in between (say, 100k or similar) and try to avoid having problems with any other add-ons but still limit to lower than an MB. That said, we need to have this messaged in our "add-on-relevant changes in Firefox 23" section anyhow (what flag/keyword to set for that?). Not sure if it should even make relnotes or not.

I support going with a high limit, like 1MB or something in the hundreds of Kb, to minimize add-on breakage.

I don't know if there's anything left to do here, but if there is I'm the wrong owner.

(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #66) > I don't know if there's anything left to do here, but if there is I'm the > wrong owner. I think it should stay assigned to you and get marked FIXED as what we can do on our side has been done. No idea why Scoobidiver has set a "[leave open]" here. IIRC, we discussed in the last channel meetings that we probably should uplift this one to 22 as long as it's still on Aurora, so this is something you still can do here. :)

(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #67) > No idea why Scoobidiver has set a "[leave open]" here. Based on "Do some custom OOM error reporting in the hopes of catching the relevant memory in the minidump to see what's going on, rev. 1" in comment 24. It was not about the patch that landed in comment 65 but nevertheless it's still applicable as there are still crashes: https://crash-stats.mozilla.com/report/list?product=Firefox&version=Firefox%3A23.0a1&signature=mozalloc_abort%28char%20const*%20const%29%20|%20NS_DebugBreak%20|%20nsPrefBranch%3A%3AGetComplexValue%28char%20const*%2C%20nsID%20const%26%2C%20void**%29

(In reply to Scoobidiver from comment #68) > there are still crashes Indeed. Ouch. :(

Well the changes I made only affect writes, not reads, so if you already have giant prefs it won't save you. Also we should probably back out http://hg.mozilla.org/mozilla-central/rev/821013d8066d

(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #70) > Well the changes I made only affect writes, not reads, so if you already > have giant prefs it won't save you. Ah, makes sense. > Also we should probably back out > http://hg.mozilla.org/mozilla-central/rev/821013d8066d Well, I guess it makes sense to crash in nsPrefBranch::GetComplexValue instead of AppendUTFXXtoUTFYY but the debugging code mentioning this bug should probably go away. Benjamin?

(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #71) > (In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #70) > > Well the changes I made only affect writes, not reads, so if you already > > have giant prefs it won't save you. > > Ah, makes sense. That said, I wonder if we should care to clean up somehow as I guess having 128MB+ in prefs might be a significant perf hit when we read prefs as well.

In 21.0b6, it's a new signature: https://crash-stats.mozilla.com/report/list?signature=mozalloc_abort%28char+const*+const%29+|+NS_DebugBreak_P+|+nsAString_internal%3A%3AGetMutableData%28wchar_t**%2C+unsigned+int%29

Jorge typically blogs about addon-compat changes, so no need to relnote unless we know of regressions ahead of release.

Yes, this is already flagged for addon-compat, so I'll take care of it when it's closer to release.

Kyle, can you still request uplift to Aurora 22? IIRC, Jorge said that with the 1MB limit we still should be OK to do this there.

It's currently only #72 browser crasher in 21.0b7 (at least no behind empty dump crashes) while it was #13 in 21.0b6, #6 in 21.0b5, #17 in 21.0b3, #15 in 21.0b2, and #17 in 21.0b1 over the last four weeks.. The Beta working range is: http://hg.mozilla.org/releases/mozilla-beta/pushloghtml?fromchange=c1453860aef9&tochange=0fe6378ac802

Yes, it looks like a fixed add-on has broadly been pushed this week which made this disappear from topcrash views (if you look at the last two days, it's below #150 or so even in release, so any "working range" in terms of our code is a false positive, the fix is on the add-on side).

(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #78) > Yes, it looks like a fixed add-on has broadly been pushed this week A fix for bug 867342 would have avoided my mistake.

(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #78) > Yes, it looks like a fixed add-on has broadly been pushed this week Correlations are now fixed and there's indeed a new version of Giant Saving, 0.91.97, with accounted for about 50% of crashes and that no longer crashes: mozalloc_abort(char const* const) | NS_DebugBreak_P | nsPrefBranch::GetComplexValue(char const*, nsID const&, void**)|EXCEPTION_BREAKPOINT (43 crashes) 49% (21/43) vs. 0% (64/136061) crossriderapp13370@crossrider.com (PhotoMania) 7% (3/43) vs. 0% (8/136061) 0.91.136 42% (18/43) vs. 0% (55/136061) 0.91.142 16% (7/43) vs. 0% (138/136061) crossriderapp4479@crossrider.com (GiantSaving) 7% (3/43) vs. 0% (5/136061) 0.91.90 7% (3/43) vs. 0% (10/136061) 0.91.92 2% (1/43) vs. 0% (3/136061) 0.91.95 0% (0/43) vs. 0% (39/136061) 0.91.96 0% (0/43) vs. 0% (80/136061) 0.91.97 19% (8/43) vs. 3% (4162/136061) plugin@yontoo.com (Yontoo Layers) 19% (8/43) vs. 3% (4121/136061) 1.20.02 12% (5/43) vs. 0% (35/136061) crossriderapp4352@crossrider.com (CouponDropDown) 5% (2/43) vs. 0% (24/136061) 0.91.100 7% (3/43) vs. 0% (9/136061) 0.91.92 0% (0/43) vs. 0% (1/136061) 0.91.94 12% (5/43) vs. 5% (6419/136061) {635abd67-4fe9-1b23-4f01-e679fa7484c1} (Yahoo! Toolbar, https://addons.mozilla.org/addon/2032) 12% (5/43) vs. 4% (6059/136061) 2.5.9.20130409112616 0% (0/43) vs. 0% (124/136061) 2.5.9.20130411104515 0% (0/43) vs. 0% (113/136061) 2.5.9.20130418100420 0% (0/43) vs. 0% (4/136061) 2.6.0.20130418072822 0% (0/43) vs. 0% (8/136061) 3.0.2.20121108061959

Kyle, does anything need doing for the branches?

(In reply to Robert Kaiser (:kairo@mozilla.com) [away until early June] from comment #76) > Kyle, can you still request uplift to Aurora 22? IIRC, Jorge said that with > the 1MB limit we still should be OK to do this there. I don't really feel comfortable with pushing this to 22. I think we should get as much testing as we can on this before releasing.

Setting status flags per comment 82. Kyle, please change if I misunderstood.

Is there anything needed from QA on this now that this is "fixed" in Beta?

Checking the crash stats, I see no crashes on FF 23. I guess we can call this bug verified.

I found my way to this code while researching the RegisterAppMemory function. Apparently this crash still happens in low volume on 40 and 41. Here's an example with size 0x75406f8: bp-92bd627b-8cc0-4f9e-acfa-90f1b2151005. I was unable to find the contents of the string, because the address of utf8String.mData wasn't recorded in the report, and the register containing that value had been clobbered already (oops).