RFR: 8065585: Change ShouldNotReachHere() to never return

[Jon Masamitsu]

http://cr.openjdk.java.net/~stefank/8065585/webrev.02/src/share/vm/utilities/debug.cpp.frames.html

>   210 // A function that never returns. Tells the compilers
>   211 // that the control flow stops at the call of this function.
>   212 ATTRIBUTE_NORETURN static void noreturn_function() {
>   213   while (true) {
>   214     os::naked_short_sleep(10);
>   215   }
>   216 }

Did you consider using just os::infinite_sleep()
inside of noreturn_function()?   It doesn't do a
short sleep but I don't know what using the
short sleep vs. a longer sleep is meant to do.

Not done yet.

Jon


On 4/15/2015 3:49 AM, 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.
>
> 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
>
> Thanks,
> StefanK