There are some non-trivial snags for this proposal. A lot of
assertions rely on other code gated on #ifdef DEBUG or
DebugOnly<...> to avoid defining data members or storing values
in non-debug builds. We could replace those with more
fine-grained macros, of course, but particularly in the case of
data members, we'd probably also take a significant memory usage
regression.
There's also the snag of NS_ASSERTION macros which are... weird.
This is nothing we can't deal with, of course, but it will
probably require a lot of manual work if we want to do it
correctly.
On Mon, Sep 17, 2018 at 02:38:14PM -0400, Randell Jesup wrote:
There are quite a few things that may be caught by assertions by
developers before committing, especially sec issues - but most
developers don't run debug builds as their main local test builds, or
for local browsing use, because they are almost unusably slow. And
turning on compiler optimization doesn't actually help much here - the
problem is mostly debug-only assertions and code that's debug only, such
as bug 1441052 ("Don't do full grey-node checking in local debug
builds").
These added checks may be useful for CI tests, but they make the builds
unusable, so the vast majority of assertions don't run in the builds our
developers are using. So enabling most of the existing MOZ_ASSERTs for
local opt builds (minus the worst performance-limiting ones) would help
developers catch bugs before landing them (and perhaps shipping
them). It's a lot like using ASAN builds to browse - but with much less
perf degradation on hopes.
Even better, if we can make the overall hit to perf low enough (1%?), we
could enable it for some/all Nightly users/builds. Or make "early
nightly" (and developer builds) enable them, and late nightly and beta
not.
If we moved some of the most expensive checks to an alternative macro
(MOZ_ASSERT_DEBUG()), we could (selectively?) enable MOZ_ASSERT checks
in some opt builds. Alternatively we could promote some MOZ_ASSERTs to
MOZ_ASSERT_NIGHTLY() (or MOZ_DIAGNOSTIC_ASSERT), which would assert in
(all) debug builds, and in nightly opt builds - and maybe promote some
to MOZ_ASSERT_RELEASE if we can take the perf hit, and they're in an
important spot.
And as a stepping-stone to the above, having local opt builds enable
(most) MOZ_ASSERTs (excluding really expensive ones, like bug 1441052)
even if the hit is larger (5, 10%) would also increase the likelihood
that we'll catch things before we land them, or before they get to beta.
(Note: --enable-release would turn this local-build behavior off - and
anyone doing speed tests/profiling/etc needs that already, and probably
we should have a specific enable/disable like
--disable-extra-assertions). This wouldn't be all that hard - most of
the work would be finding "expensive" assertions like bug 1441052.
(and perhaps we should not continue to overload --enable-release for "I
want to benchmark/profile this build")
Enabling most MOZ_ASSERTs in automation tests on opt builds would also
slightly increase our odds of finding problems - though this may not
help much, as the same tests are being run in debug builds. The only
advantage would be races may be more likely in the faster opt builds.
It might be worth trying once we have this for a few weeks, and see if
it helps or not.
Note: I've discussed this concept with various people including bz in
the past, and mostly have had agreement that this would be useful - thus
my filing of bug 1441052. If we agree this is worth doing, we should
file bugs on it and see what the effort to get this would be, and if we
could ship some of this to users. Much like nightly-asan, this would
shake out bugs we'll never find from crash-stats reports, and which can
lead to critical sec bugs, and turn them into frequently-actionable
bugs. If needed this can be enabled incrementally once the build/config
infra and macros are in place; there are several options for that.
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
