8065585: Change ShouldNotReachHere() to never return

[Lindenmaier, Goetz]

Hi Stefan, 

the IBM docu says that __notreturn__ affects only warnings about
uninitialized variables, not about missing returns.
It also says that registers are not stored on a call.
http://publibfp.boulder.ibm.com/epubs/pdf/c1473260.pdf page 193.

Sure I could suppress the warning, but I guess that's not the point
of this change.

For the compiler flags and warnings I get see the compile output below.

I had to fix two precompiled header issues in the build, but then got it
through.  The "return value" warnings are the only ones I see.

Best regards,
  Goetz

bash-4.3$ xlC_r -qversion
IBM XL C/C++ for AIX, V12.1 (5765-J02, 5725-C72)
Version: 12.01.0000.0008
bash-4.3$ xlC_r -DAIX -DPPC64 -DASSERT -DDEBUG  -I/sapmnt/home1/d045726/oJ/8065585-hs-gc/src/share/vm/prims -I/sapmnt/home1/d045726/oJ/8065585-hs-gc/src/share/vm -I/sapmnt/home1/d045726/oJ/8065585-hs-gc/src/share/vm/precompiled -I/sapmnt/home1/d045726/oJ/8065585-hs-gc/src/cpu/ppc/vm -I/sapmnt/home1/d045726/oJ/8065585-hs-gc/src/os_cpu/aix_ppc/vm -I/sapmnt/home1/d045726/oJ/8065585-hs-gc/src/os/aix/vm -I/sapmnt/home1/d045726/oJ/8065585-hs-gc/src/os/posix/vm -I../generated -DHOTSPOT_BUILD_USER="\"d045726\"" -DHOTSPOT_LIB_ARCH=\"ppc64\" -DHOTSPOT_VM_DISTRO="\"OpenJDK\""  -DTARGET_OS_FAMILY_aix -DTARGET_ARCH_ppc -DTARGET_ARCH_MODEL_ppc_64 -DTARGET_OS_ARCH_aix_ppc -DTARGET_OS_ARCH_MODEL_aix_ppc_64 -DTARGET_COMPILER_xlc -DCOMPILER2 -qpic=large -qnortti -qnoeh -D_REENTRANT -D__STDC_FORMAT_MACROS -q64 -g -q64 -qlanglvl=c99vla -qlanglvl=noredefmac -qsuppress=1540-0198 -qsuppress=1540-1090 -qsuppress=1500-010 -qsuppress=1540-1639 -qsuppress=1540-1088      -c -qmakedep=gcc -MF ../generated/dependencies/ad_ppc_64.o.d -o ad_ppc_64.o ../generated/adfiles/ad_ppc_64.cpp
"/sapmnt/home1/d045726/oJ/8065585-hs-gc/src/share/vm/gc_implementation/g1/ptrQueue.hpp", line 246.3: 1540-1101 (W) A return value of type "bool" is expected.
"/sapmnt/home1/d045726/oJ/8065585-hs-gc/src/share/vm/gc_implementation/shared/gcWhen.hpp", line 45.3: 1540-1101 (W) A return value of type "const char *" is expected.
"/sapmnt/home1/d045726/oJ/8065585-hs-gc/src/share/vm/gc_interface/gcName.hpp", line 58.3: 1540-1101 (W) A return value of type "const char *" is expected.
"/sapmnt/home1/d045726/oJ/8065585-hs-gc/src/share/vm/gc_implementation/g1/g1YCTypes.hpp", line 48.3: 1540-1101 (W) A return value of type "const char *" is expected.


-----Original Message-----
From: Stefan Karlsson [mailto:stefan.karlsson at oracle.com] 
Sent: Mittwoch, 15. April 2015 16:59
To: Lindenmaier, Goetz; hotspot-dev Source Developers
Subject: Re: 8065585: Change ShouldNotReachHere() to never return

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