RFR: 8065585: Change ShouldNotReachHere() to never return

[Stefan Karlsson]

On 2015-04-15 20:16, Jon Masamitsu wrote:
> 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.

I did consider it once, but rejected it for some reason that I can't 
remember at this point. I could probably get rid of the 
noreturn_function(), tag infinit_sleep with ATTRIBUTE_NORETURN, and use 
it instead.

Thanks,
StefanK

>
> 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
>