RFR(S): 8039805: Fix the signature of the global new/delete operators in allocation.cpp

[Lois Foltan]

Hi Volker,

Overall I understand the motivation for this change but have some concerns.

make/aix/makefiles/vm.make
No comments.

make/bsd/makefiles/vm.make
No comments, but I do share Vladimir Kozlov's concern that this change 
to remove the definition of ALLOW_OPERATOR_NEW_USAGE may need to be 
considered further by the runtime group.

src/os/aiz/vm/os_aix.cpp
No comments.

src/os/aix/vm/porting_aix.cpp
No comments.

src/share/vm/memory/allocation.cpp
I would prefer not to remove the empty "throw()" specifications. For 
consistency reasons, any user-defined operator new within the JVM should 
be defined with the empty "throw()" specification in order to ensure 
that the issues encountered within JDK-8021954 do not resurface.  I 
think this type of consistency is important whether in production or 
debug mode to avoid confusion.  We are still a long way off before 
adopting the C++11/14 standard.  The intent was that in the future, when 
C++11 is adopted, all "throw()" specifications would be changed to use 
the "nothrow" keyword.  Since JDK-8021954 addressed issues with the 
clang++ compiler on MacOS, I am curious to know if you completed any 
testing of the JVM built with clang++?

Thanks,
Lois


On 4/9/2014 12:34 PM, Volker Simonis wrote:
> Hi,
>
> could you please review and sponsor the following small change:
>
> http://cr.openjdk.java.net/~simonis/webrevs/8039805/
> https://bugs.openjdk.java.net/browse/JDK-8039805
>
> which fixes the signature of the global new/delete operators in allocation.cpp
>
> For non-product builds allocation.cpp defines global new/delete
> operators which shut down the VM if they get called at runtime. The
> rational behind this is that the these global operators should never
> be used in HotSpot.
>
> Unfortunately, the signature of some of these operators doesn't
> conform to the C++ standard which confuses some C++ compilers. For a
> more detailed explanation of the C++ background of this issue see the
> new comments in allcoation.cpp and the end of this mail.
>
> This change also replaces the asserts in the operators with guarantees
> because the code may also be active in not-product (aka. 'optimized')
> builds.
>
> Finally, the webrev fixes two places in the AIX-port which used the
> global new operator. After the change we can now remove the definition
> of ALLOW_OPERATOR_NEW_USAGE from aix/makefiles/vm.make.
>
> I have also removed ALLOW_OPERATOR_NEW_USAGE from
> bsd/makefiles/vm.make and the corresponding comments in allcoation.cpp
> which state that on Mac OS X the global new/delete operators from the
> HotSpot cause problems together with usages of these operators from
> the class library such as the ones from
> Java_sun_security_ec_ECKeyPairGenerator_generateECKeyPair. I couldn’t
> observe any such problems but if anybody has some concrete concerns
> I'm ready to remove this part from the webrev.
>
> I've build and tested these changes on Linux/x86_64, Linux/ppc64,
> Solaris/Sparc, Windows/x86_64, MacOS X and AIX/ppc64. I've especially
> run the regression tests from sun/security/ec which exercise the
> method Java_sun_security_ec_ECKeyPairGenerator_generateECKeyPair which
> was mentioned to cause problems in conjunction with the globally
> defined new/delete operators from the HotSpot but couldn't see any
> issues, neither in the slowdebug nor in the optimized build.
>
> Following some C++ background regarding the global new/delete operators:
>
> In C++98/03 the throwing new operators are defined with the following signature:
>
> void* operator new(std::size_tsize) throw(std::bad_alloc);
> void* operator new[](std::size_tsize) throw(std::bad_alloc);
>
> while all the other (non-throwing) new and delete operators are
> defined with an empty throw clause (i.e. "operator delete(void* p)
> throw()") which means that they do not throw any exceptions (see
> section 18.4 of the C++ standard
> http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2005/n1905.pdf).
>
> In the new C++11/14 standard
> (http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3797.pdf),
> the signature of the throwing new operators was changed by completely
> omitting the throw clause (which effectively means they could throw
> any exception) while all the other new/delete operators where changed
> to have a 'nothrow' clause instead of an empty throw clause.
>
> Unfortunately, the support for exception specifications among C++
> compilers is still very fragile. While some more strict compilers like
> AIX xlC or HP aCC reject to override the default throwing new operator
> with a user operator with an empty throw() clause, the MS Visual C++
> compiler warns for every non-empty throw clause like
> throw(std::bad_alloc) that it will ignore the exception specification
> (see http://msdn.microsoft.com/en-us/library/sa28fef8.aspx).
>
> I've therefore changed the operator definitions such that they
> correctly work with all currently supported compilers and in way which
> should be upwards compatible with C++11/14.
>
> Please notice that I'm aware of the discussion around "8021954: VM
> SIGSEGV during classloading on MacOS; hs_err_pid file produced"
> (http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2013-August/thread.html#8924,
> http://hg.openjdk.java.net/hsx/hotspot-comp/hotspot/rev/9758d9f36299)
> which introduced empty throw() clauses on all user defined new
> operators. But I think the rational used for that change doesn't apply
> here, because these global, user user defined new operators changed in
> this webrev aren't meant to be really used. There only task is to
> override the default, global operators (and therefore they need to
> have the right signature) and to shut the VM down if they get called.
>
> Thank you and best regards,
> Volker