[webkit-reviews] review granted: [Bug 115782] Crash properly on iOS : [Attachment 201117] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Benjamin Poulain
<benjamin at webkit.org>'s request for review:
Bug 115782: Crash properly on iOS
https://bugs.webkit.org/show_bug.cgi?id=115782

Attachment 201117: Patch
https://bugs.webkit.org/attachment.cgi?id=201117&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=201117&action=review


This changes things so that all crashes happen inside a function. The old code
went out of its way to crash at the call site, not inside a function. I think
the reason we wanted that was to make debugging easier. Not sure if that is
important any more. Seems a little risky to change, though.

I’m assuming the Windows build failures are due to changes in the exported
symbols that are needed.

> Source/WTF/wtf/Assertions.cpp:333
> +static void invokeCrashHook()
>  {
>      if (globalHook)
>	   globalHook();
>  }

Why is this still a function? Why not just call the hook directly in WTFCrash?

> Source/WTF/wtf/Assertions.cpp:339
> +    (*(int *)(uintptr_t)0xbbadbeef = 0);

There are extra unneeded parentheses here.

> Source/WTF/wtf/Assertions.cpp:344
> +    (*(int *)(uintptr_t)0xbbadbeef = 0);
> +#if COMPILER(CLANG)
> +    __builtin_trap();
> +#else
> +    ((void(*)())0)();
> +#endif

Really could use a why comment explaining the bbadbeef thing.