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

[David Holmes]

I think we should just delete the whole ALLOW_OPERATOR_NEW block. We 
fixed the problem of it being called outside the VM under 8014326.

David

On 10/04/2014 12:48 PM, David Holmes wrote:
> Hi Volker,
>
> Need more time to consider this in full but from the existing code:
>
>   689 // On certain platforms, such as Mac OS X (Darwin), in debug
> version, new is being called
>   690 // from jdk source and causing data corruption. Such as
>   691 //  Java_sun_security_ec_ECKeyPairGenerator_generateECKeyPair
>   692 // define ALLOW_OPERATOR_NEW_USAGE for platform on which global
> operator new allowed.
>
> we actually fixed that by using the mapfiles to ensure the hotspot
> operator new was not visible externally. The existence of
> ALLOW_OPERATOR_NEW_USAGE wasn't even raised at the time :(
>
> https://bugs.openjdk.java.net/browse/JDK-8014326
>
> David
>
> On 10/04/2014 2:34 AM, 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
>>