Closed Bug 1495244 Opened 3 years ago Closed 7 months ago

Crash in icu_62::DecimalFormat::setGroupingUsed

Categories

(Core :: JavaScript: Internationalization API, defect, P3)

63 Branch
defect

Tracking

()

RESOLVED WORKSFORME
Tracking Status
firefox-esr60 --- unaffected
firefox62 --- unaffected
firefox63 --- wontfix
firefox64 --- wontfix
firefox65 --- wontfix
firefox66 --- fix-optional

People

(Reporter: philipp, Unassigned)

References

(Depends on 1 open bug)

Details

(Keywords: crash, csectype-nullptr, regression)

Crash Data

This bug was filed from the Socorro interface and is
report bp-2e83d66f-5161-4839-a53e-527490180929.
=============================================================

Top 10 frames of crashing thread:

0 xul.dll icu_62::DecimalFormat::setGroupingUsed intl/icu/source/i18n/decimfmt.cpp:350
1 xul.dll static void icu_62::fixNumberFormatForDates intl/icu/source/i18n/smpdtfmt.cpp:257
2 xul.dll void icu_62::SimpleDateFormat::initialize intl/icu/source/i18n/smpdtfmt.cpp:864
3 xul.dll void icu_62::SimpleDateFormat::construct intl/icu/source/i18n/smpdtfmt.cpp:839
4 xul.dll icu_62::SimpleDateFormat::SimpleDateFormat intl/icu/source/i18n/smpdtfmt.cpp:499
5 xul.dll static class icu_62::DateFormat* icu_62::DateFormat::create intl/icu/source/i18n/datefmt.cpp:526
6 xul.dll icu_62::DateFormat::createDateInstance intl/icu/source/i18n/datefmt.cpp:402
7 xul.dll void icu_62::DateTimePatternGenerator::addICUPatterns intl/icu/source/i18n/dtptngen.cpp:664
8 xul.dll void icu_62::DateTimePatternGenerator::initData intl/icu/source/i18n/dtptngen.cpp:480
9 xul.dll void icu_62::DateTimePatternGenerator::DateTimePatternGenerator intl/icu/source/i18n/dtptngen.cpp:350

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

this is a cross-platform but low volume crash signature appearing in firefox 63, apparently related to the icu update in bug 1466471.
these crashes are mostly taking place in the content crashes and skew towards 32bit installations.
Andre, can you please take a look at this?
Flags: needinfo?(andrebargull)
ecx is 0x00000000, so most likely a null-pointer crash; probably the OOM issue reported at https://unicode-org.atlassian.net/browse/ICU-13847.
Flags: needinfo?(andrebargull)
Is there anything actionable here for 64/65?
Flags: needinfo?(jwalden+bmo)
Crash Signature: [@ icu_62::DecimalFormat::setGroupingUsed] → [@ icu_62::DecimalFormat::setGroupingUsed] [@ icu_63::DecimalFormat::setGroupingUsed]

The upstream ticket doesn't seem to be going anywhere. Is there any chance we could hack around this on our end for now?

Flags: needinfo?(andrebargull)

To make this reproducible for me, I've built a shell with JS_SetICUMemoryFunctions setting ICU's allocation functions to use js_malloc and friends, which allows to create an oomTest function test case:

oomTest(function() {
    for (var i = 0; i < 10; ++i) {
        new Intl.DateTimeFormat("de").format(0)
    }
})

And I got the crash from comment #0 right away. \o/

Added some fixes to catch the case when fields is nullptr when created from here and that should be it, right? Rebuild ICU and reran the oomTest test to ensure the newly added workarounds did indeed fix this crash. Nope, next crash happened, now fields->formatter created here is nullptr. Added more workarounds, which, by the way, involved making the DecimalFormat class non-functional, because not all users of fields->formatter have a way to report an error. And again, rebuild ICU and reran the oomTest. Next crash because Calendar::fZone created here is nullptr, but all Calendar users expect this field is never null.

After that crash I stopped playing whack-a-mole with ICU. /o\

So I don't think we can (successfully) hack around this without changing dozens of places in ICU. :-/

Flags: needinfo?(andrebargull)

OK, thanks for trying :(. Doesn't sound like this bug is actionable until upstream gets around to fixing it.

I've looked into this sort of problem before -- André could fill people in on the details if it mattered -- and reached the same conclusion about lots of places in ICU requiring a fix, and that fix being too complicated to take anywhere but upstream first.

I don't think this is so critical as to prioritize over other work, but we always could if OOM issues that will mostly manifest on 32-bit are worth caring about super-strongly. It's hard for me to see why we would think that.

Depends on: 1552900

There was a very low volume of crashes in the last 6 months and all of them happened on ~FX67, not the latest Firefox versions.
Closing this as resolved:worksforme, please do re-open it if this crash will appear on the latest version of Firefox.

Status: NEW → RESOLVED
Closed: 7 months ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.