RFR: 8065585: Change ShouldNotReachHere() to never return

[Per Liden]

Hi Stefan,

Nice cleanup! Some comments below.

On 2015-04-15 12:49, Stefan Karlsson wrote:
> Hi,
>
> Today the it's possible for the code to return out from
> ShouldNotReachHere() calls. This sometimes forces us to add return
> statements and assignments to parts of the code where it they don't make
> sense. By telling the compiler that ShouldNotReachHere is a dead end, we
> can get rid of these unnecessary constructs.
>
> For example:
>
> 1) We could get rid of return statements after ShouldNotReachHere():
>
> bool is_marked() {
>    // actual code here
>
>    // Execution path that "should not" happen.
>    ShouldNotReachHere();
>    return false;
> }
>
> 2) The following code will actually use an uninitialized value today.
> The compiler will find this if we turn on -Wuninitialized. But if we
> change ShouldNotReachHere() to not return,  the compiler will stop
> complaining because report(type) will never be called with an
> uninitialized value:
>
> int type;
> switch (value) {
>    case TYPE_OOP: type = 0; break;
>    case TYPE_KLASS: type = 1; break;
>    default: ShouldNotReachHere();
> }
> report(type)
>
>
> The patch changes the following functions and defines to never return:
> - fatal(...)
> - ShouldNotCallThis()
> - ShouldNotReachHere()
> - report_vm_out_of_memory(...)
> - vm_exit_out_of_memory(...)
>
> but leaves the following unchanged:
> - Unimplemented()
> - Untested(...)
> - guarantee(...)
>
> We might want to change the the behavior of Unimplemented() and
> Untested(...), but they are used a lot in compiler code, so I've not
> changed them for this patch. There has been request to leave
> guarantee(...) unchanged so that they can be turned off in production code.

guarantee() should not have this attribute as it should be able to return.

>
> I had to change some instance of ShouldNotReachHere() in destructors,
> because the VS C++ compiler complained about potential memory leaks.
>
> http://cr.openjdk.java.net/~stefank/8065585/webrev.01/
> https://bugs.openjdk.java.net/browse/JDK-8065585

Looks good, just a few minor suggestions:

- NORETURN_ATTRIBUTE -> ATTRIBUTE_NORETURN, to better match the existing 
ATTRIBUTE_PRINTF?

- Make noreturn_function() static and move it into debug.cpp?

cheers,
/Per