Re: Converting assertions into release assertions

[Julian Seward]

> [assertions expensive, etc]

(random suggestion):  Why don't we profile the cost of all assertions,
or at least the number of times each is evaluated, across some some test
workload.

Then we'd have a quantitative basis on which to say "we can enable the
following set of assertions at the cost of a 1% perf hit, and the
following extras if we are prepared to take a 5% hit".

Right now it seems to me that we know assertions are expensive, but we
don't have a systematic way to evaluate tradeoffs between performance and
extra safety.

J

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Converting assertions into release assertions

[Michael Layzell]

We already have implemented that static analysis but haven't enabled it
yet, because static analysis doesn't run on windows machines:
https://bugzilla.mozilla.org/show_bug.cgi?id=1223932

On Fri, Sep 23, 2016 at 12:51 PM, Bobby Holley 
wrote:

> On Fri, Sep 23, 2016 at 9:30 AM, Ted Mielczarek 
> wrote:
>
> > On Fri, Sep 23, 2016, at 10:20 AM, Ehsan Akhgari wrote:
> > > On 2016-09-23 8:49 AM, Gijs Kruitbosch wrote:
> > > > Then this enables me to answer Ehsan's question. These are the builds
> > > > I've recently tried using (e.g. when debugging intermittents in VMs
> > > > because then I don't need to set up too much of a build env) and
> > they're
> > > > still very slow.
> > >
> > > Unfortunately I don't think we can do debug builds that are faster than
> > > that.  It's possible that the debugging checks that we perform in the
> > > cases you're trying to use the browser in are just prohibitively too
> > > expensive.  :(
> >
> > If someone had time, it might be interesting to profile something that
> > feels slow in an --enable-debug --enable-optimize build and see if
> > anything stands out. These builds will always be a little slower than
> > the builds we ship (because they're not PGOed), but it would be nice if
> > they were at least usable. Making them faster would also make our debug
> > test runs in automation faster, which is a nice win! (We spend *a lot*
> > of CPU time running debug tests in automation.)
> >
>
> Nathan looked at this a few years ago:
> https://bugzilla.mozilla.org/show_bug.cgi?id=972135
>
> There's some low-hanging fruit for anyone with static analysis savvy.
>
>
> >
> > -Ted
> >
> > ___
> > dev-platform mailing list
> > dev-platform@lists.mozilla.org
> > https://lists.mozilla.org/listinfo/dev-platform
> >
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Converting assertions into release assertions

[Bobby Holley]

On Fri, Sep 23, 2016 at 9:30 AM, Ted Mielczarek  wrote:

> On Fri, Sep 23, 2016, at 10:20 AM, Ehsan Akhgari wrote:
> > On 2016-09-23 8:49 AM, Gijs Kruitbosch wrote:
> > > Then this enables me to answer Ehsan's question. These are the builds
> > > I've recently tried using (e.g. when debugging intermittents in VMs
> > > because then I don't need to set up too much of a build env) and
> they're
> > > still very slow.
> >
> > Unfortunately I don't think we can do debug builds that are faster than
> > that.  It's possible that the debugging checks that we perform in the
> > cases you're trying to use the browser in are just prohibitively too
> > expensive.  :(
>
> If someone had time, it might be interesting to profile something that
> feels slow in an --enable-debug --enable-optimize build and see if
> anything stands out. These builds will always be a little slower than
> the builds we ship (because they're not PGOed), but it would be nice if
> they were at least usable. Making them faster would also make our debug
> test runs in automation faster, which is a nice win! (We spend *a lot*
> of CPU time running debug tests in automation.)
>

Nathan looked at this a few years ago:
https://bugzilla.mozilla.org/show_bug.cgi?id=972135

There's some low-hanging fruit for anyone with static analysis savvy.


>
> -Ted
>
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Converting assertions into release assertions

[Ted Mielczarek]

On Fri, Sep 23, 2016, at 10:20 AM, Ehsan Akhgari wrote:
> On 2016-09-23 8:49 AM, Gijs Kruitbosch wrote:
> > Then this enables me to answer Ehsan's question. These are the builds
> > I've recently tried using (e.g. when debugging intermittents in VMs
> > because then I don't need to set up too much of a build env) and they're
> > still very slow.
> 
> Unfortunately I don't think we can do debug builds that are faster than
> that.  It's possible that the debugging checks that we perform in the
> cases you're trying to use the browser in are just prohibitively too
> expensive.  :(

If someone had time, it might be interesting to profile something that
feels slow in an --enable-debug --enable-optimize build and see if
anything stands out. These builds will always be a little slower than
the builds we ship (because they're not PGOed), but it would be nice if
they were at least usable. Making them faster would also make our debug
test runs in automation faster, which is a nice win! (We spend *a lot*
of CPU time running debug tests in automation.)

-Ted

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Converting assertions into release assertions

[Ehsan Akhgari]

On 2016-09-23 8:49 AM, Gijs Kruitbosch wrote:
> On 23/09/2016 11:20, Ted Mielczarek wrote:
>> On Fri, Sep 23, 2016, at 05:58 AM, Panos Astithas wrote:
>>> I used to do that in the past, but nowadays artifact builds have changed
>>> the cost-benefit trade-off so very few people bother AFAIK, when not
>>> touching C++ code. If we could get artifact builds to use --enable-debug
>>> and --enable-optimize (or have an option to do so) that could change the
>>> calculus.
>>
>> The debug builds we do in automation are already built with optimization
>> (we changed this a few years ago to speed up running tests), so it ought
>> to be straightforward to make debug artifact builds work by fetching
>> those bits.
>>
>> -Ted
> 
> Then this enables me to answer Ehsan's question. These are the builds
> I've recently tried using (e.g. when debugging intermittents in VMs
> because then I don't need to set up too much of a build env) and they're
> still very slow.

Unfortunately I don't think we can do debug builds that are faster than
that.  It's possible that the debugging checks that we perform in the
cases you're trying to use the browser in are just prohibitively too
expensive.  :(
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Converting assertions into release assertions

[Ehsan Akhgari]

On 2016-09-23 8:49 AM, Gijs Kruitbosch wrote:
> On 23/09/2016 11:20, Ted Mielczarek wrote:
>> On Fri, Sep 23, 2016, at 05:58 AM, Panos Astithas wrote:
>>> I used to do that in the past, but nowadays artifact builds have changed
>>> the cost-benefit trade-off so very few people bother AFAIK, when not
>>> touching C++ code. If we could get artifact builds to use --enable-debug
>>> and --enable-optimize (or have an option to do so) that could change the
>>> calculus.
>>
>> The debug builds we do in automation are already built with optimization
>> (we changed this a few years ago to speed up running tests), so it ought
>> to be straightforward to make debug artifact builds work by fetching
>> those bits.
>>
>> -Ted
> 
> Then this enables me to answer Ehsan's question. These are the builds
> I've recently tried using (e.g. when debugging intermittents in VMs
> because then I don't need to set up too much of a build env) and they're
> still very slow.

Unfortunately I don't think we can do debug builds that are faster than
that.  It's possible that the debugging checks that we perform in the
cases you're trying to use the browser in are just prohibitively too
expensive.  :(
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Converting assertions into release assertions

[Paolo Amadini]

On 9/23/2016 1:49 PM, Gijs Kruitbosch wrote:

Then this enables me to answer Ehsan's question. These are the builds
I've recently tried using (e.g. when debugging intermittents in VMs
because then I don't need to set up too much of a build env) and they're
still very slow.


Some time ago I ventilated the idea of having a build mode that enables
debug for everything except the JavaScript engine itself, but the
conclusion was that there wasn't a strong need for it as we could just
use the non-debug optimized build for front-end development.

Paolo
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Converting assertions into release assertions

[Gijs Kruitbosch]

On 23/09/2016 11:20, Ted Mielczarek wrote:

On Fri, Sep 23, 2016, at 05:58 AM, Panos Astithas wrote:

I used to do that in the past, but nowadays artifact builds have changed
the cost-benefit trade-off so very few people bother AFAIK, when not
touching C++ code. If we could get artifact builds to use --enable-debug
and --enable-optimize (or have an option to do so) that could change the
calculus.


The debug builds we do in automation are already built with optimization
(we changed this a few years ago to speed up running tests), so it ought
to be straightforward to make debug artifact builds work by fetching
those bits.

-Ted


Then this enables me to answer Ehsan's question. These are the builds 
I've recently tried using (e.g. when debugging intermittents in VMs 
because then I don't need to set up too much of a build env) and they're 
still very slow.


You can use debug builds from artifact mode using the MOZ_DEBUG switch 
in your mozconfig (see 
https://bugzilla.mozilla.org/show_bug.cgi?id=1253697 - annoyingly, last 
I tried, just --enable-debug isn't good enough to download debug bits, 
see also the patch that landed for that bug).


~ Gijs

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Converting assertions into release assertions

[Ted Mielczarek]

On Fri, Sep 23, 2016, at 05:58 AM, Panos Astithas wrote:
> I used to do that in the past, but nowadays artifact builds have changed
> the cost-benefit trade-off so very few people bother AFAIK, when not
> touching C++ code. If we could get artifact builds to use --enable-debug
> and --enable-optimize (or have an option to do so) that could change the
> calculus.

The debug builds we do in automation are already built with optimization
(we changed this a few years ago to speed up running tests), so it ought
to be straightforward to make debug artifact builds work by fetching
those bits.

-Ted
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Converting assertions into release assertions

[Panos Astithas]

On Thu, Sep 22, 2016 at 6:07 PM, Ehsan Akhgari 
wrote:

> On 2016-09-22 9:07 AM, Gijs Kruitbosch wrote:
> > On 22/09/2016 05:28, Nicholas Nethercote wrote:
> >> Greetings,
> >>
> >> Assertions, such as MOZ_ASSERT, are great. But they only run in debug
> >> builds.
> >>
> >> Release assertions, such as MOZ_RELEASE_ASSERT, run in all builds.
> >>
> >> 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.)
> >>
> >> Obviously we can't convert every normal assertion in the codebase into
> >> a release assertion. But it might be worth thinking about which normal
> >> assertions are good candidates for conversion. Good candidates include
> >> any assertion where the consequence of failure is dangerous, e.g.
> >> might cause memory access violations.
> >>
> >> Nick
> >
> > Yes please. This + diagnostic assert also helps frontend people who
> > build and run opt builds (because debug builds are too slow to be
> > usable, especially when combined with the browser toolbox (JS
> > debugging)). Right now I miss some of these and then only find out when
> > the tests that I did run go orange on try and/or inbound/autoland, and
> > then I have to locally change the relevant C++ so I can test in my opt
> > build (or resign myself to doing a separate clobber debug build
> somewhere).
>
> What exact debug configuration is too slow for you?  People who want to
> debug C++ generally turn optimizations off, but for front-end devs, I
> think building with --enable-debug and --enable-optimize should give you
> an optimized build with the debug facilities turned on, which should be
> much faster.  Although it is not going to be as fast as a
> --disable-debug --enable-optimize build, there's a lot of value in
> Mozilla developers running builds with debug checks turned on, so that
> we can get good bug reports when an assertion fires when working on a
> front-end feature, etc.
>

I used to do that in the past, but nowadays artifact builds have changed
the cost-benefit trade-off so very few people bother AFAIK, when not
touching C++ code. If we could get artifact builds to use --enable-debug
and --enable-optimize (or have an option to do so) that could change the
calculus.

Panos
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Converting assertions into release assertions

[Julian Seward]

On 22/09/16 17:07, Ehsan Akhgari wrote:

> What exact debug configuration is too slow for you?  People who want to
> debug C++ generally turn optimizations off, but for front-end devs, I
> think building with --enable-debug and --enable-optimize should give you
> an optimized build with the debug facilities turned on, which should be
> much faster.

At least with recent-ish gccs on Linux, --enable-optimize="-Og -g" provides
quite significant optimization but with the compiler making efforts to
retain good debuggability of the generated code.  At least with GDB it
is quite feasible to navigate around in -Og compiled code.  I use -Og a
lot; it's a good compromise.

J

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Converting assertions into release assertions

[Ehsan Akhgari]

On 2016-09-22 9:07 AM, Gijs Kruitbosch wrote:
> On 22/09/2016 05:28, Nicholas Nethercote wrote:
>> Greetings,
>>
>> Assertions, such as MOZ_ASSERT, are great. But they only run in debug
>> builds.
>>
>> Release assertions, such as MOZ_RELEASE_ASSERT, run in all builds.
>>
>> 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.)
>>
>> Obviously we can't convert every normal assertion in the codebase into
>> a release assertion. But it might be worth thinking about which normal
>> assertions are good candidates for conversion. Good candidates include
>> any assertion where the consequence of failure is dangerous, e.g.
>> might cause memory access violations.
>>
>> Nick
> 
> Yes please. This + diagnostic assert also helps frontend people who
> build and run opt builds (because debug builds are too slow to be
> usable, especially when combined with the browser toolbox (JS
> debugging)). Right now I miss some of these and then only find out when
> the tests that I did run go orange on try and/or inbound/autoland, and
> then I have to locally change the relevant C++ so I can test in my opt
> build (or resign myself to doing a separate clobber debug build somewhere).

What exact debug configuration is too slow for you?  People who want to
debug C++ generally turn optimizations off, but for front-end devs, I
think building with --enable-debug and --enable-optimize should give you
an optimized build with the debug facilities turned on, which should be
much faster.  Although it is not going to be as fast as a
--disable-debug --enable-optimize build, there's a lot of value in
Mozilla developers running builds with debug checks turned on, so that
we can get good bug reports when an assertion fires when working on a
front-end feature, etc.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Converting assertions into release assertions

[Benjamin Smedberg]

Definitely explore this!

I want us to be very careful/deliberate about the privacy consequences of
this, though. Any values which could be user data need to be tightly
controlled, in the same manner we control access to the minidumps
themselves. So I don't think we should be too generic about this.

As data steward I'd like to be kept in the loop and do a data review of any
changes here.

--BDS

On Thu, Sep 22, 2016 at 9:28 AM, Nicolas B. Pierron <
nicolas.b.pier...@mozilla.com> wrote:

> 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
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Converting assertions into release assertions

[Nicolas B. Pierron]

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

Re: Converting assertions into release assertions

[Gijs Kruitbosch]

On 22/09/2016 05:28, Nicholas Nethercote wrote:

Greetings,

Assertions, such as MOZ_ASSERT, are great. But they only run in debug builds.

Release assertions, such as MOZ_RELEASE_ASSERT, run in all builds.

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.)

Obviously we can't convert every normal assertion in the codebase into
a release assertion. But it might be worth thinking about which normal
assertions are good candidates for conversion. Good candidates include
any assertion where the consequence of failure is dangerous, e.g.
might cause memory access violations.

Nick


Yes please. This + diagnostic assert also helps frontend people who 
build and run opt builds (because debug builds are too slow to be 
usable, especially when combined with the browser toolbox (JS 
debugging)). Right now I miss some of these and then only find out when 
the tests that I did run go orange on try and/or inbound/autoland, and 
then I have to locally change the relevant C++ so I can test in my opt 
build (or resign myself to doing a separate clobber debug build somewhere).


~ Gijs
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Converting assertions into release assertions

[Wayne]

On 9/22/2016 12:28 AM, Nicholas Nethercote wrote:

Greetings,

Assertions, such as MOZ_ASSERT, are great. But they only run in debug builds.

Release assertions, such as MOZ_RELEASE_ASSERT, run in all builds.

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.)

Obviously we can't convert every normal assertion in the codebase into
a release assertion. But it might be worth thinking about which normal
assertions are good candidates for conversion. Good candidates include
any assertion where the consequence of failure is dangerous, e.g.
might cause memory access violations.

Nick


For grins I ran the socorro query from the bug report against 
Thunderbird 
https://crash-stats.mozilla.com/search/?moz_crash_reason=~ElementAt=Thunderbird&_sort=-date&_facets=signature&_facets=moz_crash_reason&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform#facet-signature


___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Converting assertions into release assertions

[Patrick McManus]

+1 on MOZ_DIAGNOSTIC_ASSERT - its been very useful to me as well.

On Thu, Sep 22, 2016 at 6:40 AM, Bobby Holley  wrote:

> There's also MOZ_DIAGNOSTIC_ASSERT, which is fatal in pre-release builds
> but not release ones. It can be a good compromise to find bugs in the wild
> when the performance cost is probably negligible but you're still not quite
> comfortable shipping it on release. I added it last year while working on
> stability for the media stack, and found it very useful.
>
> bholley
>
> On Wed, Sep 21, 2016 at 9:28 PM, Nicholas Nethercote <
> n.netherc...@gmail.com
> > wrote:
>
> > Greetings,
> >
> > Assertions, such as MOZ_ASSERT, are great. But they only run in debug
> > builds.
> >
> > Release assertions, such as MOZ_RELEASE_ASSERT, run in all builds.
> >
> > 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.)
> >
> > Obviously we can't convert every normal assertion in the codebase into
> > a release assertion. But it might be worth thinking about which normal
> > assertions are good candidates for conversion. Good candidates include
> > any assertion where the consequence of failure is dangerous, e.g.
> > might cause memory access violations.
> >
> > Nick
> > ___
> > dev-platform mailing list
> > dev-platform@lists.mozilla.org
> > https://lists.mozilla.org/listinfo/dev-platform
> >
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Converting assertions into release assertions

[Bobby Holley]

There's also MOZ_DIAGNOSTIC_ASSERT, which is fatal in pre-release builds
but not release ones. It can be a good compromise to find bugs in the wild
when the performance cost is probably negligible but you're still not quite
comfortable shipping it on release. I added it last year while working on
stability for the media stack, and found it very useful.

bholley

On Wed, Sep 21, 2016 at 9:28 PM, Nicholas Nethercote  wrote:

> Greetings,
>
> Assertions, such as MOZ_ASSERT, are great. But they only run in debug
> builds.
>
> Release assertions, such as MOZ_RELEASE_ASSERT, run in all builds.
>
> 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.)
>
> Obviously we can't convert every normal assertion in the codebase into
> a release assertion. But it might be worth thinking about which normal
> assertions are good candidates for conversion. Good candidates include
> any assertion where the consequence of failure is dangerous, e.g.
> might cause memory access violations.
>
> Nick
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform