[webkit-dev] Why does RELEASE_ASSERT not have an error message?

[Filip Pizlo]

Geoff,

If you find a case of code that looks like this:

if (!things) {
   dataLog(...);
   RELEASE_ASSERT_NOT_REACHED();
}

and this code blames to fpizlo at apple.com, then I can tell you why I wrote it that way.  It was because I had crashed on some assert in that function, and I needed to figure out which one.

Here's a great example that I remember vividly in Heap.cpp:

    if (m_collectorBelievesThatTheWorldIsStopped) {
        dataLog("FATAL: world already stopped.\n");
        RELEASE_ASSERT_NOT_REACHED();
    }

This code *should* say RELEASE_ASSERT(!m_collectorBelievesThatTheWOrldIsStopped), but I had to change that sensible-looking assert to this ugly code because of trap coalescing.  Before the change, the crash site made zero sense - it was a different assert in a function inlined into a function that was inlined into this one.  It sent me on a wild goose chase trying to figure out bugs in that other function.  Yuck!

At this point, writing RELEASE_ASSERTS in this awful way has become a kind of cargo-cult experience for me because of the frequency with which the debugger gives me false information if I use the normal RELEASE_ASSERT.  I have to assume that when I crash at a RELEASE_ASSERT that there is a good chance that the crash site is bogus.

Note that whenever I am forced to write such a verbose four-line assert, I usually take that as an opportunity to print more diagnostic information.  But from I can remember, this is rarely the reason why I write such code.  I'm writing it to get accurate stacks!  So, I don't buy the argument that this four-line code is great because it's printing additional diagnostics.


> On Feb 23, 2017, at 2:39 PM, Geoffrey Garen <ggaren at apple.com> wrote:
> 
> Not all functions suffer from this problem. Few enough functions suffer from this problem that I haven’t felt an urgent need to address it.

I don't know what fraction of functions suffer from this problem, but enough of them do that it is causing me to have to write ugly hacks to get around it.  Tools only have to fail like 5% of the time to become untrustworthy.  I don't want my stack traces to come with a 95% confidence interval!

I'd like to find a way to stop writing code like this:

if (!things) {
    dataLog("things");
    RELEASE_ASSERT_NOT_REACHED();
}

What do you propose?  Note that "just use RELEASE_ASSERT" is not a great answer, because it is wrong enough times that when I do get a RELEASE_ASSERT crash, the very first thing I do is convert it to the code above to make sure I'm not in the 5% case.  That's quite bad when such a crash shows up in telemetry - we can't even be sure that we know where we crashed in that case.

-Filip