On 09/22/2016 04:28 AM, Nicholas Nethercote wrote:
I want to highlight a nice case where converting a normal assertion
into a release assertion was a win. In bug 1159244 Michael Layzell did
this in nsTArray::ElementAt(), to implement a form of always-on array
bounds checking. See
https://bugzilla.mozilla.org/show_bug.cgi?id=1159244#c55 for
discussion of how this is finding real bugs in the wild. (As well as
identifying new bugs, it's also helping understand existing crash
reports, e.g. see bug 1291082 where the crash signature changed.)
Independently of the release/debug assertion, I want to raise something that
I see in the patch above, and in a patch made to investigate some rare
crashes in the JS engine.
The patch from the bug above adds instrumentation to attach a dynamically
computed reason string (snprintf). In the JS engine Jan De Mooij did
something similar by dumping content on the stack between 2 magic numbers,
in order to include this information in crash reports.
What I see is a recurring pattern where the classical MOZ_CRASH / MOZ_ASSERT
might not be ergonomic enough. I think we should add a way to annotate our
crashes (even in debug builds) with some state, without going in the
internals of MOZ_CRASHs or requiring privileged access to the mini-dump.
Maybe something like:
MOZ_ASSERT(foo < 0x1000, MOZ_REASON("Unexpected value: %x", foo));
which could be converted into a lambda doing the snprintf into a
pre-reserved space.
--
Nicolas B. Pierron
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
