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

Geoff, If you find a case of code that looks like this: if (!things) { dataLog(...); RELEASE_ASSERT_NOT_REACHED(); } and this code blames to fpi...@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 figur

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

> On Feb 22, 2017, at 12:16 PM, Filip Pizlo wrote: > > >> On Feb 22, 2017, at 11:58 AM, Geoffrey Garen wrote: >> >> I’ve lost countless hours to investigating CrashTracers that would have been >> easy to solve if I had access to register state. > > The current RELEASE_ASSERT means that ever

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

To give an update: I plan to experiment and get some size and perf numbers and report back later. But since this is not a high priority task for me, it may be a while before I get back to this. Mark > On Feb 22, 2017, at 1:25 PM, Ryosuke Niwa wrote: > > On Wed, Feb 22, 2017 at 12:41 PM, Mark

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

On Wed, Feb 22, 2017 at 12:41 PM, Mark Lam wrote: > > I would like to point out that we might be able to get the best of both > worlds. Here’s how we can do it: > > define RELEASE_ASSERT(assertion) do { \ > if (UNLIKELY(!(assertion))) { \ > preserveRegisterState(); \ > WTFRep

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

> On Feb 22, 2017, at 12:46 PM, Filip Pizlo wrote: > >> >> On Feb 22, 2017, at 12:41 PM, Mark Lam > > wrote: >> >>> >>> On Feb 22, 2017, at 12:35 PM, Filip Pizlo >> > wrote: >>> On Feb 22, 2017, at 12:33 PM, Mark Lam >>>

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

> On Feb 22, 2017, at 12:41 PM, Mark Lam wrote: > >> >> On Feb 22, 2017, at 12:35 PM, Filip Pizlo > > wrote: >> >>> >>> On Feb 22, 2017, at 12:33 PM, Mark Lam >> > wrote: >>> >>> On Feb 22, 2017, at 12:16 PM, Filip Pizlo >>>

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

Mark, I know that you keep saying this. I remember you told me this even when I was sitting on a RELEASE_ASSERT that had gotten rage-coalesced. Your reasoning sounds great, but this just isn't what happens in clang. __builtin_trap gets coalesced, as does inline asm. -Filip > On Feb 22, 201

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

> On Feb 22, 2017, at 12:35 PM, Filip Pizlo wrote: > >> >> On Feb 22, 2017, at 12:33 PM, Mark Lam > > wrote: >> >> >>> On Feb 22, 2017, at 12:16 PM, Filip Pizlo >> > wrote: >>> >>> On Feb 22, 2017, at 11:58 AM, Geoffrey Garen >>>

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

For some context, we used to see aggregation of the CRASH() for RELEASE_ASSERT() in the old days before we switched to using the int3 trap. Back then we called a crash() function that never returns. As a result, the C++ compiler was able to coalesce all the calls. With the int3 trap emitted b

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

> On Feb 22, 2017, at 12:33 PM, Mark Lam wrote: > > >> On Feb 22, 2017, at 12:16 PM, Filip Pizlo > > wrote: >> >> >>> On Feb 22, 2017, at 11:58 AM, Geoffrey Garen >> > wrote: >>> >>> I’ve lost countless hours to investigating CrashTracers tha

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

> On Feb 22, 2017, at 12:16 PM, Filip Pizlo wrote: > > >> On Feb 22, 2017, at 11:58 AM, Geoffrey Garen wrote: >> >> I’ve lost countless hours to investigating CrashTracers that would have been >> easy to solve if I had access to register state. > > The current RELEASE_ASSERT means that ever

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

> On Feb 22, 2017, at 12:16 PM, Filip Pizlo wrote: > > >> On Feb 22, 2017, at 11:58 AM, Geoffrey Garen wrote: >> >> I’ve lost countless hours to investigating CrashTracers that would have been >> easy to solve if I had access to register state. > > The current RELEASE_ASSERT means that ever

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

> On Feb 22, 2017, at 12:23 PM, Saam barati wrote: > >> >> On Feb 22, 2017, at 12:16 PM, Filip Pizlo > > wrote: >> >> >>> On Feb 22, 2017, at 11:58 AM, Geoffrey Garen >> > wrote: >>> >>> I’ve lost countless hours to investigating CrashTracers

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

> On Feb 22, 2017, at 11:58 AM, Geoffrey Garen wrote: > > I’ve lost countless hours to investigating CrashTracers that would have been > easy to solve if I had access to register state. The current RELEASE_ASSERT means that every assertion in what the compiler thinks is a function (i.e. some

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

On Wed, 2017-02-22 at 11:58 -0800, Geoffrey Garen wrote: > (2) Change RELEASE_ASSERT to do the normal debug ASSERT thing in > Debug builds. (There’s not much need to preserve register state in > debug builds.) This seems like a more desirable approach. Michael

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

I like this idea. I think even doing what JF suggested could be really nice for suspected crashes. That way the register state would just tell us the reason. If we do log, I think we should use WTFLogAlways instead of dataLog, because I think that does show up in some crash logs (but I could be w

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

I’ve lost countless hours to investigating CrashTracers that would have been easy to solve if I had access to register state. I also want the freedom to add RELEASE_ASSERT without ruining performance due to bad register allocation or making the code too large to inline. For example, hot paths i

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

Do we get the dataLog output in a crash report? It seems useful to have at a minimum some "reason" enum which ends up in a register, so the crash report is somewhat helpful, like we do with JIT code. At that point the release assert might as well capture __LINE__ and other things in a register, so

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

I disagree actually. I've lost countless hours to converting this: RELEASE_ASSERT(blah) into this: if (!blah) { dataLog("Reason why I crashed"); RELEASE_ASSERT_NOT_REACHED(); } Look in the code - you'll find lots of stuff like this. I don't think analyzing register state at crashes is m

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

Oh yeah, I forgot about that. I think the register state is more important for crash analysis, especially if we can make sure that the compiler does not aggregate the int3s. I’ll explore alternatives. > On Feb 21, 2017, at 5:54 PM, Saam barati wrote: > > I thought the main point of moving to

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

I thought the main point of moving to SIGTRAP was to preserve register state? That said, there are probably places where we care more about the message than the registers. - Saam > On Feb 21, 2017, at 5:43 PM, Mark Lam wrote: > > Is there a reason why RELEASE_ASSERT (and friends) does not cal

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

+1 -Filip > On Feb 21, 2017, at 17:43, Mark Lam wrote: > > Is there a reason why RELEASE_ASSERT (and friends) does not call > WTFReportAssertionFailure() to report where the assertion occur? Is this > purely to save memory? svn blame tells me that it has been this way since > the introduct

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

Is there a reason why RELEASE_ASSERT (and friends) does not call WTFReportAssertionFailure() to report where the assertion occur? Is this purely to save memory? svn blame tells me that it has been this way since the introduction of RELEASE_ASSERT in r140577 many years ago. Would anyone object