RFR(S/M): 8199472: Fix non-PCH build after JDK-8199319

coleen.phillimore at oracle.com coleen.phillimore at oracle.com
Mon Mar 12 19:42:55 UTC 2018

Hi this looks good except:


Can you move this a function in instanceKlass.cpp and would this 
eliminate the changes that add include instanceKlass.inline.hpp ?

If Stefan is not still online, I'll sponsor this for you.

I have a follow-on related change 
https://bugs.openjdk.java.net/browse/JDK-8199263 which is quickly 
expanding due to transitive includes that I hope you can help me test 
out (when I get it to compile on solaris).


On 3/12/18 3:34 PM, Volker Simonis wrote:
> Hi,
> can I please have a review and a sponsor for the following fix:
> http://cr.openjdk.java.net/~simonis/webrevs/2018/8199472/
> https://bugs.openjdk.java.net/browse/JDK-8199472
> The number changes files is "M" but the fix is actually "S" :)
> Here come the gory details:
> Change "8199319: Remove handles.inline.hpp include from
> reflectionUtils.hpp" breaks the non-PCH build (at least on Ubuntu
> 16.04 with gcc 5.4.0). If you configure with
> "--disable-precompiled-headers" you will get a whole lot of undefined
> reference for "Handle::Handle(Thread*, oopDesc*)" - see bug report.
> It seems that newer versions of GCC (and possibly other compilers as
> well) don't emit any code for inline functions if these functions can
> be inlined at all potential call sites.
> The problem in this special case is that "Handle::Handle(Thread*,
> oopDesc*)" is not declared "inline" in "handles.hpp", but its
> definition in "handles.inline.hpp" is declared "inline". This leads to
> a situation, where compilation units which only include "handles.hpp"
> will emit a call to "Handle::Handle(Thread*, oopDesc*)" while
> compilation units which include "handles.inline.hpp" will try to
> inline "Handle::Handle(Thread*, oopDesc*)". If all the inlining
> attempts are successful, no instance of "Handle::Handle(Thread*,
> oopDesc*)" will be generated in any of the object files. This will
> lead to the link errors listed in the .
> The quick fix for this issue is to include "handles.inline.hpp" into
> all the compilation units with undefined references (listed below).
> The correct fix (realized in this RFR) is to declare
> "Handle::Handle(Thread*, oopDesc*)" inline in "handles.hpp". This will
> lead to warnings (which are treated as errors) if the inline
> definition is not available at a call site and will avoid linking
> error due to compiler optimizations. Unfortunately this requires a
> whole lot of follow-up changes, because "handles.hpp" defines some
> derived classes of "Handle" which all have implicitly inline
> constructors which all reference the base class
> "Handle::Handle(Thread*, oopDesc*)" constructor. So the constructors
> of the derived classes have to be explicitly declared inline in
> "handles.hpp" and their implementation has to be moved to
> "handles.inline.hpp". This change again triggers other changes for all
> files which relayed on the derived Handle classes having inline
> constructors...
> Thank you and best regards,
> Volker

More information about the hotspot-dev mailing list