8065585: Change ShouldNotReachHere() to never return

[Stefan Karlsson]

Hi Goetz,

On 2015-04-15 16:54, Lindenmaier, Goetz wrote:
> Hi Stefan,
>
> Thanks for doing this fix, I would appreciate this cleanup a lot.
> I just built it on aix.  So far, the attribute unfortunately does not
> suppress the warnings.  I'll investgate this some more ...

Thanks for verifying the patch.

What warnings are you getting? Are you running with -Wuninitialized?

Thanks,
StefanK


>
> Best regards,
>    Goetz.
>
> -----Original Message-----
> From: hotspot-dev [mailto:hotspot-dev-bounces at openjdk.java.net] On Behalf Of Stefan Karlsson
> Sent: Mittwoch, 15. April 2015 12:49
> To: hotspot-dev Source Developers
> Subject: RFR: 8065585: Change ShouldNotReachHere() to never return
>
> 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