One of the changes in bug 824224 was to remove the MOZ_NORETURN annotation on mozalloc_abort(). This triggers this build warning, w/ GCC 4.7.2 on Ubuntu 12.10: > mozalloc_abort.cpp: In function ‘void abort()’: > mozalloc_abort.cpp:40:1: warning: ‘noreturn’ function does return [enabled by default] ...because a system-header declares "abort() as noreturn, and we're providing our own abort() implementation, in which we call some other method (mozalloc_abort), and as far as the compiler knows, mozalloc_abort() *will* return, so abort()looks like it can also return. We can fix this by adding back the MOZ_NORETURN annotation on mozalloc_abort. Per bug 824224 comment 48, this change might decrease the quality of our crash-stacks, but we don't know that for sure, so I propose we do it. (and then, if it does break crash-stacks, we can revert it and maybe just disable this warning in this directory)
Created attachment 700713 [details] [diff] [review] fix v1 This patch leaves the directory warning-free, too (at least on Linux; I'll verify on other platforms), so the patch marks it as FAIL_ON_WARNINGS = 1 to keep it that way. :)
I'm pretty sure this helped the crash stacks to be better on arm, so i would at least make it conditional.
Sounds good to me. What's the recommended way to check for that?
Looks like some existing code uses #if !defined(__arm__) -- I'll add that.
Created attachment 700733 [details] [diff] [review] fix v1 again (forgot to qref, sorry)
Created attachment 700754 [details] [diff] [review] fix v2: now with #if !defined(__arm__) (sorry, forgot to qref to pick up the !defined(__arm__) change)
Created attachment 700786 [details] [diff] [review] fix v3: now w/out FAIL_ON_WARNINGS so that Android builds can still warn about this ...and of course, our Android builds spam the same "warning: ‘noreturn’ function does return", so we can't mark this FAIL_ON_WARNINGS if we're exempting arm builds from the fix. So: Here's the patch again, w/out the FAIL_ON_WARNINGS annotation.