Fwd: Re: RFR: 8012902: remove use of global operator new - take 2

Yumin Qi yumin.qi at oracle.com
Tue May 14 21:36:08 PDT 2013


This is second approval from David for this bug (already integrated into 
RT_Baseline today, 5/14/2013, PDT). Thanks, David.

There is a issue with third party code calling operator new of jvm, such 
as currently on OS X plartform. A new bug filed by David: 
https://jbs.oracle.com/bugs/browse/JDK-8014326 against this issue --- 
all globals exported (external) from jvm on Darwin.

In this bug changes, ALLOW_OPERATOR_NEW_USAGE is defined for Darwin, 
under which operator new and new[] are allowed to be called from third 
party code.  In case more platforms discovered with this issue, we need 
to define this variable in make file to workaround it.

Thanks
Yumin


-------- Original Message --------
Subject: 	Re: RFR: 8012902: remove use of global operator new - take 2
Date: 	Tue, 14 May 2013 15:39:39 +1000
From: 	David Holmes <david.holmes at oracle.com>
Organization: 	Oracle Corporation
To: 	Yumin Qi <yumin.qi at oracle.com>
CC: 	Coleen Phillimore <coleen.phillimore at oracle.com>



Hi Yumin,

Current webrev is fine for this iteration :)

Thanks,
David

On 14/05/2013 3:35 PM, Yumin Qi wrote:
>
> On 5/13/2013 10:29 PM, David Holmes wrote:
>> On 14/05/2013 3:25 PM, Yumin Qi wrote:
>>> HI, David
>>>
>>>    I took your diff from https://jbs.oracle.com/bugs/browse/JDK-8014326,
>>> add diff to gcc.make and map-version-<flavor>.
>>>    It works fine.
>>> test/sun/management/jmxremote/bootstrap/RmiRegistrySslTest.sh tested
>>> pass w/o crash.
>>>
>>>    Question, since this bug may be a rootcause of multiple crashes,
>>> could you promote the priority and push asap?
>>
>> This is still being investigated. I have no way to confirm if this is
>> indeed the cause of random crashes - but I have someone testing it out
>> for a few days.
>>
>>> If so I will remove
>>> ALLOW_OPERATOR_NEW_USAGE def from allocation.cpp and make file. Or do
>>> you think my current diff is OK for push?
>>
>> I think you will want to push and we will cleanup later. Even if the
>> linkage is the problem I don't think the mapfile solution will be
>> acceptable for JDK8, so it will take more time to derive a full
>> solution and test it. Plus in that case  will need to hand it off to
>> someone in runtime I expect.
>>
> So my understanding is currently, the webrev of 8012902 should keep
> ALLOW_OPERATOR_NEW_USAGE? Maybe more platforms have the same problem so
> keep the def is a good solution. Are you OK for current webrev?
>
> http://cr.openjdk.java.net/~minqi/8012902/webrev2
> <http://cr.openjdk.java.net/%7Eminqi/8012902/webrev2>
>
> Thanks
> Yumin
>
>> Thanks,
>> David
>>
>>>
>>> Thanks
>>> Yumin
>>>
>>>
>>>
>>>
>>> On 5/7/2013 5:39 PM, David Holmes wrote:
>>>> On 8/05/2013 10:33 AM, Yumin Qi wrote:
>>>>> Thanks!
>>>>>
>>>>>   suppose -fvisibility=hidden should hide all the variables
>>>>> (functions)
>>>>> from client, but I added this to CFLAGS, it still showed exported:
>>>>
>>>> There's another fix coming down the pipeline that corrects the
>>>> JNI_EXPORT definition, after which -fvisibility=hidden should work.
>>>> But was also add a bunch of symbols for SA access and that probably
>>>> means we will still need to use the mapfiles (which is what I'm trying
>>>> to get working).
>>>>
>>>> David
>>>>
>>>>> Compiling
>>>>> /Users/btravers/yqi/ws/8012902/src/share/vm/memory/allocation.cpp
>>>>> rm -f allocation.o
>>>>> llvm-g++ -D_ALLBSD_SOURCE -D_GNU_SOURCE -DAMD64 -DASSERT -I.
>>>>> -I/Users/btravers/yqi/ws/8012902/src/closed/share/vm/prims
>>>>> -I/Users/btravers/yqi/ws/8012902/src/share/vm/prims
>>>>> -I/Users/btravers/yqi/ws/8012902/src/closed/share/vm
>>>>> -I/Users/btravers/yqi/ws/8012902/src/share/vm
>>>>> -I/Users/btravers/yqi/ws/8012902/src/share/vm/precompiled
>>>>> -I/Users/btravers/yqi/ws/8012902/src/closed/cpu/x86/vm
>>>>> -I/Users/btravers/yqi/ws/8012902/src/cpu/x86/vm
>>>>> -I/Users/btravers/yqi/ws/8012902/src/os_cpu/bsd_x86/vm
>>>>> -I/Users/btravers/yqi/ws/8012902/src/os/bsd/vm
>>>>> -I/Users/btravers/yqi/ws/8012902/src/closed/os/posix/vm
>>>>> -I/Users/btravers/yqi/ws/8012902/src/os/posix/vm -I../generated
>>>>> -DHOTSPOT_RELEASE_VERSION="\"25.0-b31-internal\""
>>>>> -DHOTSPOT_BUILD_TARGET="\"fastdebug\""
>>>>> -DHOTSPOT_BUILD_USER="\"btravers\"" -DHOTSPOT_LIB_ARCH=\"amd64\"
>>>>> -DHOTSPOT_VM_DISTRO="\"Java HotSpot(TM)\"" -DTARGET_OS_FAMILY_bsd
>>>>> -DTARGET_ARCH_x86 -DTARGET_ARCH_MODEL_x86_64 -DTARGET_OS_ARCH_bsd_x86
>>>>> -DTARGET_OS_ARCH_MODEL_bsd_x86_64 -DTARGET_COMPILER_gcc -DCOMPILER2
>>>>> -DCOMPILER1 -fPIC -fno-rtti -fno-exceptions -pthread -fcheck-new -m64
>>>>> -pipe -fno-strict-aliasing -DMAC_OS_X_VERSION_MAX_ALLOWED=1070
>>>>> -mmacosx-version-min=10.7.0 -Os -DVM_LITTLE_ENDIAN -D_LP64=1
>>>>> -fno-omit-frame-pointer -DINCLUDE_TRACE -Werror -Wpointer-arith
>>>>> -Wconversion -Wsign-compare -Wundef -DDTRACE_ENABLED -D_XOPEN_SOURCE
>>>>> -D_DARWIN_C_SOURCE -fvisibility=hidden -c -fpch-deps -MMD -MP -MF
>>>>> ../generated/dependencies/allocation.o.d -o allocation.o
>>>>> /Users/btravers/yqi/ws/8012902/src/share/vm/memory/allocation.cpp
>>>>>
>>>>> note that -fvisibility=hidden
>>>>> by default, it will export everything, but with
>>>>> -fvisibility=hidden, it
>>>>> should shed those which don't have 'default' attributes from client.
>>>>>
>>>>> btravers$ nm -gm libjvm.dylib | grep Znam
>>>>> 000000000010ac5f (__TEXT,__text) external __Znam
>>>>> Still exported!
>>>>>
>>>>> I am looking forward to hearing your solutions!
>>>>>
>>>>> Thanks
>>>>> Yumin
>>>>>
>>>>>
>>>>> On 5/7/2013 5:25 PM, David Holmes wrote:
>>>>>> Okay ....
>>>>>>
>>>>>> 1. I'm fine with KlassHandle etc not being changed at this time to
>>>>>> extend an allocation base class. That will be done as a follow up CR.
>>>>>>
>>>>>> 2. The asserting version of global new operators is fine - I too
>>>>>> missed the fact they were non-product and so the assert will always
>>>>>> fire.
>>>>>>
>>>>>> 3. I'm following up on the OSX symbol export problem separately. When
>>>>>> that gets fixed I'll update the ALLOW_OPERATOR_NEW_USAGE build
>>>>>> setting
>>>>>> and code comments.
>>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>>
>>>>>> On 8/05/2013 5:25 AM, Yumin Qi wrote:
>>>>>>>
>>>>>>> On 5/7/2013 10:59 AM, Coleen Phillimore wrote:
>>>>>>>>
>>>>>>>> On 05/07/2013 01:53 PM, Yumin Qi wrote:
>>>>>>>>> off alias.
>>>>>>>>>
>>>>>>>>> On 5/6/2013 11:26 PM, David Holmes wrote:
>>>>>>>>>> Hi Yumin,
>>>>>>>>>>
>>>>>>>>>> First thanks for persevering here. :)
>>>>>>>>
>>>>>>>> Yes, you should get an award!
>>>>>>>>
>>>>>>> Thanks, I want award!
>>>>>>>>>>
>>>>>>>>>> I still have a few queries/concerns:
>>>>>>>>>>
>>>>>>>>>> Why doesn't KlassHandle extend one of the allocation types?
>>>>>>>>> I don't know either, it has a long history to the hotspot origin.
>>>>>>>>>
>>>>>>>>> oopHandles(intanceHandle, arrayHandle, objArrayHandle,
>>>>>>>>> typeArrayHandle) : public Handle : public _valueobj (On linux,
>>>>>>>>> it is
>>>>>>>>> empty)
>>>>>>>>> metaHandles (methodHandle, constantPoolHandle): nothing
>>>>>>>>> KlassHandle: nothing
>>>>>>>>> instanceKlassHandle: KlassHandle : nothing
>>>>>>>>>
>>>>>>>>> There should be a uniform for all those handles in future I think.
>>>>>>>>
>>>>>>>> Yes, I'm afraid of changing this now.  These Handles have always
>>>>>>>> been
>>>>>>>> special.   When I rewrote them for permgen elimination I forgot to
>>>>>>>> give them a StackObj allocation class but I probably should
>>>>>>>> have.   I
>>>>>>>> think we should not fix this now though because it might be risky.
>>>>>>> I guess you mean _ValueObj here? HandleMark is StackObj but not
>>>>>>> handles.
>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> I also thought we could now have HandleMark extend StackObj as
>>>>>>>>>> desired. But note that by adding the operator new overrides to
>>>>>>>>>> the
>>>>>>>>>> class, rather than using NEW_C_HEAP+placement-new at the call
>>>>>>>>>> site,
>>>>>>>>>> people can incorrectly define additional C-heap instances beyond
>>>>>>>>>> the
>>>>>>>>>> one special case that we had to handle. :(
>>>>>>>>>>
>>>>>>>>>> I don't understand why these overrides:
>>>>>>>>>>
>>>>>>>>>> ! #ifndef ALLOW_OPERATOR_NEW_USAGE
>>>>>>>>>>   void* operator new(size_t size){
>>>>>>>>>> !   assert(false, "Should not call global operator new");
>>>>>>>>>> !   return 0;
>>>>>>>>>>   }
>>>>>>>>>>
>>>>>>>>>> use assert instead of guarantee? On product builds we will simply
>>>>>>>>>> return 0 and have a null pointer that will lead to a crash if not
>>>>>>>>>> checked (and chances are that code incorrectly using the
>>>>>>>>>> global new
>>>>>>>>>> is not expecting to have to check the return value).
>>>>>>>>>>
>>>>>>>>> I think it should return AllocateHeap here, so it still works for
>>>>>>>>> product?
>>>>>>>>
>>>>>>>> This whole block is protected by #ifndef PRODUCT, which I didn't
>>>>>>>> realize when I freaked out when it used to be ShouldNotReachHere,
>>>>>>>> which is defined in product.
>>>>>>>>
>>>>>>>> Maybe ShouldNotReachHere() is better (as long as this is not
>>>>>>>> product!)
>>>>>>> Both ShouldNotReachHere and ShouldNotCallThis are in product, also
>>>>>>> seen
>>>>>>> in debug. Should I use  ShouldNotCallThis instead?
>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>>> I'm still concerned that the JDK library uses the libjvm's
>>>>>>>>>> operator
>>>>>>>>>> new. This indicates some kind of linkage error to me. I'm
>>>>>>>>>> trying to
>>>>>>>>>> investigate this further.
>>>>>>>>>>
>>>>>>>>> Have seen your next email, it is surprising to find dylib
>>>>>>>>> export all
>>>>>>>>> to outside!
>>>>>>>>>> Fianlly where do we stand with all the other classes that don't
>>>>>>>>>> extend an allocation base type? Will we file a follow up CR to
>>>>>>>>>> convert them all?
>>>>>>>>>>
>>>>>>>>> I would file one for this.
>>>>>>>>
>>>>>>>> We can fix these on an ongoing basis.   Maybe file 1 RFE to look
>>>>>>>> for
>>>>>>>> the rest, but this is lower priority than the ones that called
>>>>>>>> global
>>>>>>>> operator new.   I maintain that this is a good incremental
>>>>>>>> improvement
>>>>>>>> but we have to cut it off somewhere.
>>>>>>>>
>>>>>>> OK,  file a bug with low priority then.
>>>>>>>
>>>>>>> Thanks
>>>>>>> Yumin
>>>>>>>> Thanks,
>>>>>>>> Coleen
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks
>>>>>>>>> Yumin
>>>>>>>>>> Thanks,
>>>>>>>>>> David
>>>>>>>>>>
>>>>>>>>>> On 4/05/2013 3:15 AM, Yumin Qi wrote:
>>>>>>>>>>> Sorry  the link is not right, the right link:
>>>>>>>>>>> http://cr.openjdk.java.net/~minqi/8012902/webrev2/
>>>>>>>>>>>
>>>>>>>>>>> Thanks
>>>>>>>>>>> Yumin
>>>>>>>>>>> On 5/3/2013 10:08 AM, Yumin Qi wrote:
>>>>>>>>>>>> Coleen,
>>>>>>>>>>>>
>>>>>>>>>>>>   Thanks, new webrev:
>>>>>>>>>>>> http://cr.openjdk.java.net/~minqi/8012902/webrev2
>>>>>>>>>>>>
>>>>>>>>>>>> Yumin
>>>>>>>>>>>>
>>>>>>>>>>>> On 5/2/2013 2:41 PM, Coleen Phillimore wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>> I think I hit send too soon.
>>>>>>>>>>>>>
>>>>>>>>>>>>> http://cr.openjdk.java.net/~minqi/8012902/webrev1/src/share/vm/runtim/unhandledOops.hpp.cdiff.html
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> I think this should not be mtOther, but mtThread since this
>>>>>>>>>>>>> belongs
>>>>>>>>>>>>> to the thread.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Other than these small things, I think it looks good.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>> Coleen
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 05/01/2013 11:20 AM, Yumin Qi wrote:
>>>>>>>>>>>>>> Hi,  the link in fact directs to old webrev, you need to
>>>>>>>>>>>>>> copy the
>>>>>>>>>>>>>> text in browser to open the new link, or follow
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> http://cr.openjdk.java.net/~minqi/8012902/webrev1/
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks
>>>>>>>>>>>>>> Yumin
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 5/1/2013 7:56 AM, Yumin Qi wrote:
>>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>   This revised version replace CATCH_OPERATOR_NEW_USAGE with
>>>>>>>>>>>>>>> ALLOW_OPERATOR_NEW_USAGE.  For platforms on which
>>>>>>>>>>>>>>> operator new
>>>>>>>>>>>>>>> called from other source other than jvm, define this
>>>>>>>>>>>>>>> macro to
>>>>>>>>>>>>>>> enable global operator new instead.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>   Also fixed KlassHandle creation in ciReplay.cpp, that
>>>>>>>>>>>>>>> still
>>>>>>>>>>>>>>> use
>>>>>>>>>>>>>>> global operator new.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~minqi/8012902/webrev1/
>>>>>>>>>>>>>>> <http://cr.openjdk.java.net/%7Eminqi/8012902/webrev/>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>   Tested: vm.quick.testlist, jtreg, runThese, JPRT
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Thanks
>>>>>>>>>>>>>>> Yumin
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> *
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> *
>>>>>>>>>>>>>>> On 4/29/2013 10:08 AM, Yumin Qi wrote:
>>>>>>>>>>>>>>>> Coleen and David,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On 4/29/2013 6:36 AM, Coleen Phillimore wrote:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> David,
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> I think Yumin has some additional operator new[] calls
>>>>>>>>>>>>>>>>> that he
>>>>>>>>>>>>>>>>> hasn't fixed yet so I'm expecting another webrev. I agree
>>>>>>>>>>>>>>>>> it's
>>>>>>>>>>>>>>>>> a more complicated change than originally thought, but
>>>>>>>>>>>>>>>>> it's
>>>>>>>>>>>>>>>>> progressing so I don't think quitting now is a good
>>>>>>>>>>>>>>>>> idea.  The
>>>>>>>>>>>>>>>>> main change helps resolve a fundamental problem that has
>>>>>>>>>>>>>>>>> recently
>>>>>>>>>>>>>>>>> broken the VM.  Yumin's change makes it harder to do this.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Yes, I think if we stop here, this one will not be
>>>>>>>>>>>>>>>> revisited
>>>>>>>>>>>>>>>> for
>>>>>>>>>>>>>>>> long time --- more dangerous code will be added then. To
>>>>>>>>>>>>>>>> prevent
>>>>>>>>>>>>>>>> this from happening, better to stop them as soon as we can.
>>>>>>>>>>>>>>>>> On 04/29/2013 09:00 AM, David Holmes wrote:
>>>>>>>>>>>>>>>>>> Hi Yumin,
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> I think we need to pull back on this until we can
>>>>>>>>>>>>>>>>>> address the
>>>>>>>>>>>>>>>>>> broader issues:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> a) there are a number of classes that don't obey the
>>>>>>>>>>>>>>>>>> rules
>>>>>>>>>>>>>>>>>> about
>>>>>>>>>>>>>>>>>> extending one of the allocation types
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> This will always be the case.   This shouldn't be a
>>>>>>>>>>>>>>>>> blocker.
>>>>>>>>>>>>>>>> Those classes (most of them) are used as stack obj,
>>>>>>>>>>>>>>>> currently did
>>>>>>>>>>>>>>>> not find any used as heap obj. For VALUE_OBJ_CLASS_SPEC,
>>>>>>>>>>>>>>>> since it
>>>>>>>>>>>>>>>> is empty on linux, every class which take it as parent will
>>>>>>>>>>>>>>>> come
>>>>>>>>>>>>>>>> from nothing that is similar to those classes not obey the
>>>>>>>>>>>>>>>> rules
>>>>>>>>>>>>>>>> --- This is why I asked if we should make it _ValueObj on
>>>>>>>>>>>>>>>> linux
>>>>>>>>>>>>>>>> but you think that will add more bytes to the objects.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> b) adding additional operator new/new[] for explicit
>>>>>>>>>>>>>>>>>> C-Heap
>>>>>>>>>>>>>>>>>> usage conflicts with the use of the existing
>>>>>>>>>>>>>>>>>> macros/functions
>>>>>>>>>>>>>>>>>> documented in allocation.hpp (I still think I prefer
>>>>>>>>>>>>>>>>>> NEW_C_HEAP_OBJ + global placement new to invoke the
>>>>>>>>>>>>>>>>>> correct
>>>>>>>>>>>>>>>>>> constructor). If you stick with your approach then the
>>>>>>>>>>>>>>>>>> documentation in allocation.hpp needs rewriting.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> I think the documentation in allocation.hpp describes
>>>>>>>>>>>>>>>>> how we
>>>>>>>>>>>>>>>>> want
>>>>>>>>>>>>>>>>> this to work.   The exceptional cases should be documented
>>>>>>>>>>>>>>>>> where
>>>>>>>>>>>>>>>>> they exist.   I don't really have an opinion whether
>>>>>>>>>>>>>>>>> NEW_C_HEAP_OBJ vs. adding new and new[] to the exceptional
>>>>>>>>>>>>>>>>> classes is better.  Both have their pros and cons.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Using macro and calling constructors need carefulness,
>>>>>>>>>>>>>>>> which
>>>>>>>>>>>>>>>> caused too much concern, so if the use case is simple, I
>>>>>>>>>>>>>>>> would
>>>>>>>>>>>>>>>> like to use macros, but if it is complex, implementing
>>>>>>>>>>>>>>>> operator
>>>>>>>>>>>>>>>> new is preferable I think.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> c) there seem to be other global array allocations still
>>>>>>>>>>>>>>>>>> lurking
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Yes, the ones we know about should be fixed. And do due
>>>>>>>>>>>>>>>>> diligence to find them all.  The purpose of the assert
>>>>>>>>>>>>>>>>> is to
>>>>>>>>>>>>>>>>> find
>>>>>>>>>>>>>>>>> any that might leak in after this exercise.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> I found one more case using nm on linux. Do you know
>>>>>>>>>>>>>>>> what it
>>>>>>>>>>>>>>>> will
>>>>>>>>>>>>>>>> be on solaris? I tried to code a small program, but
>>>>>>>>>>>>>>>> could not
>>>>>>>>>>>>>>>> locate 'new' in the output. For shared code, linux will
>>>>>>>>>>>>>>>> output
>>>>>>>>>>>>>>>> all
>>>>>>>>>>>>>>>> the unsettled operator new, what I am concerning here is
>>>>>>>>>>>>>>>> some
>>>>>>>>>>>>>>>> platform specific code.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> d) the effect of the hotspot global operator new on the
>>>>>>>>>>>>>>>>>> other
>>>>>>>>>>>>>>>>>> libraries needs to be better understood and dealt with.
>>>>>>>>>>>>>>>>>> If I
>>>>>>>>>>>>>>>>>> understand your fix as it stands you will abort in
>>>>>>>>>>>>>>>>>> product
>>>>>>>>>>>>>>>>>> mode,
>>>>>>>>>>>>>>>>>> and warn in debug - yet we know this problem exists so
>>>>>>>>>>>>>>>>>> this
>>>>>>>>>>>>>>>>>> will
>>>>>>>>>>>>>>>>>> simply force an abort. I would not expect to see the
>>>>>>>>>>>>>>>>>> ShouldNotReachHere() variants.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> I think the logic is reversed in the new code. #ifndef
>>>>>>>>>>>>>>>>> CATCH_OPERATOR_NEW_USAGE should just revert to the global
>>>>>>>>>>>>>>>>> operators new/new[]/delete/delete[], ie be empty. The code
>>>>>>>>>>>>>>>>> under CATCH_* should assert and return AllocateHeap() in
>>>>>>>>>>>>>>>>> product
>>>>>>>>>>>>>>>>> quietly. ShouldNotReachHere() gives a fatal error in
>>>>>>>>>>>>>>>>> product
>>>>>>>>>>>>>>>>> mode
>>>>>>>>>>>>>>>>> too, so it should be avoided.
>>>>>>>>>>>>>>>> Yes, the logic here now is on macosx (currently I did not
>>>>>>>>>>>>>>>> find
>>>>>>>>>>>>>>>> any
>>>>>>>>>>>>>>>> other platform the global operator new switched to jvm
>>>>>>>>>>>>>>>> 'new'),
>>>>>>>>>>>>>>>> only gives warnings for the operator first time called, and
>>>>>>>>>>>>>>>> return
>>>>>>>>>>>>>>>> AllocateHeap, no stop here.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> BTW, when I tried to test on Windows to find if they will
>>>>>>>>>>>>>>>> fail on
>>>>>>>>>>>>>>>> new[] (a lot of new[] used in awt, swing etc), the demo
>>>>>>>>>>>>>>>> did not
>>>>>>>>>>>>>>>> crash on new[] but there is a failure in awt code,
>>>>>>>>>>>>>>>> Hashtable.cpp
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> This comes not only my fastdebug version, but all other
>>>>>>>>>>>>>>>> debug
>>>>>>>>>>>>>>>> versions on Windows. It is an awt related error. It happens
>>>>>>>>>>>>>>>> on my
>>>>>>>>>>>>>>>> desktop whenever you type characters in input area of the
>>>>>>>>>>>>>>>> testing
>>>>>>>>>>>>>>>> program interface.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Thanks
>>>>>>>>>>>>>>>> Yumin
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>
>>>
>



-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20130514/997632bd/attachment-0001.html 


More information about the hotspot-runtime-dev mailing list