RFR: 8065585: Change ShouldNotReachHere() to never return (original) (raw)

Stefan Karlsson stefan.karlsson at oracle.com
Wed Apr 15 18:52:22 UTC 2015

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 ATTRIBUTENORETURN static void noreturnfunction() { 213 while (true) { 214 os::nakedshortsleep(10); 215 } 216 } Did you consider using just os::infinitesleep() inside of noreturnfunction()? 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 ismarked() { // 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 TYPEOOP: type = 0; break; case TYPEKLASS: type = 1; break; default: ShouldNotReachHere(); } report(type)

The patch changes the following functions and defines to never return: - fatal(...) - ShouldNotCallThis() - ShouldNotReachHere() - reportvmoutofmemory(...) - vmexitoutofmemory(...) 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

More information about the hotspot-dev mailing list