RFR JDK-8021954 VM SIGSEGV during classloading on MacOS; hs_err_pid file produced
Lois Foltan
lois.foltan at oracle.com
Thu Aug 29 06:15:08 PDT 2013
On 8/29/2013 7:13 AM, David Holmes wrote:
> On 29/08/2013 1:53 AM, Coleen Phillimore wrote:
>> On 8/27/2013 8:46 PM, David Holmes wrote:
>>> Thanks for the detailed explanation. It is frustrating that we keep
>>> having to revisit these allocation routines.
>>>
>>> However it still seems to me that we don't need NO_EXCEPT on most of
>>> our operator new definitions because they will never return NULL. This
>>> is trivially so for things like:
>>>
>>> void* StackObj::operator new(size_t size) NOEXCEPT {
>>> ShouldNotCallThis(); return 0; }
>>> void* _ValueObj::operator new(size_t size) NOEXCEPT {
>>> ShouldNotCallThis(); return 0; }
>>>
>>> but also for anything that invokes vm_exit_out_of_memory directly or
>>> indirectly.
>>
>> I don't think this is a good idea at all. We want the constructor
>> prologue to always check for null before calling the constructor. We
>> will never throw C++ exceptions for these so all operator new's are
>> NOEXCEPT. These trivial examples would return zero rather than
>> throwing if ShouldNotCallThis() is ifdef'ed out in product. So, this
>> example is either trivially wrong without NOEXCEPT or correct without
>> NOEXCEPT. It is safer code to make them all NOEXCEPT.
>
> If the allocator can never return a NULL value then checking for NULL
> is completely redundant.
Hi David,
We do have allocators that can return a NULL value, like
MetaSpace::allocate(), Chunk's operator new(), etc. Both demonstrating
the return of NULL in the two bug reports.
> We don't check our internal allocation routines for NULL returns where
> we know they abort on failure, so how is this any different ?
Actually we have been checking NULL, it's just that the C++ compiler's
have been implicitly checking NULL for us. Solaris C++ generates by
default NULL detection constructor prologue code for user-defined class
member operator new() even in a throw situation. For gcc we specify
-fcheck-new. So whether or not we explicitly direct the compiler to
check via an empty "throw()" specification or not, it's still being
done. So why should we check now? Because we have a C++ compiler,
clang, that adheres to the standard and doesn't support -fcheck-new. It
wants to be explicitly told.
> AFAICS ShouldNotCallThis() is not ifdef'd out but if it were then yes
> this would fail - as would ifdef'ing out a vm_exit_out_of_memory in
> the other allocation routines.
>> So really the only question is whether NOEXCEPT is better than
>> throw(). throw() looks nicer because it's not a macro. NOEXCEPT may
>> be required for the future if that's required to conditionally add the
>> noexcept keyword.
>>
>> Maybe we should use throw() for now and worry about the noexcept keyword
>> if that becomes an issue.
>
> It would be less ugly to use throw() but more intrusive in that it
> affects all the compilers. Plus I don't know what the benefit would be
> of moving from throw() to noexcept if they both mean the same thing?
I believe the c++11 standard obsolesces "throw()" in favor of noexcept
keyword. However, I can't imagine that a compiler would completely
obsolesce it, I can image there will be a mode to still support "throw()".
Lois
>
>> David, where are you removing spurious throw() expressions and why?
>
> ./share/vm/code/nmethod.cpp:void* nmethod::operator new(size_t size,
> int nmethod_size) throw () {
>
> See:
>
> http://mail.openjdk.java.net/pipermail/hotspot-dev/2013-August/010526.html
>
>
> and subsequent replies. The presence of throw() was considered an
> error at the time but now it seems not. I don't know if it has been
> removed in hotspot-compiler, or perhaps just in the staging repos for
> the ppc64 changes.
>
> Cheers,
> David
> ------
>
>
>
>> Thanks,
>> Coleen
>>
>>>
>>> David
>>> -----
>>>
>>> On 27/08/2013 10:44 PM, Lois Foltan wrote:
>>>>
>>>> On 8/27/2013 7:31 AM, David Holmes wrote:
>>>>> On 27/08/2013 6:01 PM, Mikael Gerdin wrote:
>>>>>> On 2013-08-27 04:41, David Holmes wrote:
>>>>>>> On 27/08/2013 4:36 AM, Lois Foltan wrote:
>>>>>>>> Please review the following fix:
>>>>>>>>
>>>>>>>> Internal webrev:
>>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~coleenp/bug_jdk8021954/
>>>>>>>>
>>>>>>>> Bug: VM SIGSEGV during classloading on MacOS; hs_err_pid file
>>>>>>>> produced &
>>>>>>>> runtime/6878713/Test6878713.sh fails on mac
>>>>>>>>
>>>>>>>> bug links at:
>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8021954
>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8022140
>>>>>>>>
>>>>>>>> Summary of fix:
>>>>>>>> On MacOS, currently Hotspot is built specifying the
>>>>>>>> -fcheck-new
>>>>>>>> command line option to the llvm-g++ compiler.
>>>>>>>> The -fcheck-new option directs the compiler to "check that
>>>>>>>> the
>>>>>>>> pointer returned by |operator new| is non-null
>>>>>>>> before attempting to modify the storage allocated." The
>>>>>>>> clang++
>>>>>>>> compiler does not support the
>>>>>>>> -fcheck-new option. To obtain similiar functionality when
>>>>>>>> building Hotspot with clang++, empty exception
>>>>>>>> throw() specifications must be added to all user-defined
>>>>>>>> operator
>>>>>>>> new()'s.
>>>>>>>
>>>>>>> We just spotted something related in the PPC64 port and were going
>>>>>>> the
>>>>>>> other way or removing these "spurious" throw() declarations.
>>>>>>>
>>>>>>> But this seems really ugly - is there really a need to specialize
>>>>>>> this
>>>>>>> for clang and use NOEXCEPT to hide it? Shouldn't all C++ compilers
>>>>>>> honour the nothrow() semantics?
>>>>>>>
>>>>>>> That said I thought we already handled this using the "const
>>>>>>> std::nothrow_t& nothrow_constant" where needed? Our other
>>>>>>> operator new
>>>>>>> implementations shouldn't return NULL but will abort.
>>>>>>>
>>>>>>> So why do we need -fcheck-new or this empty throw() ?
>>>>>>
>>>>>> I guess that if the C++ compiler knows that operator new() will
>>>>>> throw an
>>>>>> exception if an allocation fails then it doesn't need to emit
>>>>>> code to
>>>>>> explicitly avoid calling the constructor.
>>>>>
>>>>> Yes that is what happens, but why do _we_ need it when ...
>>>>>
>>>>>> If we declare operator new() with an empty throw() we will also
>>>>>> inform
>>>>>> the compiler that new can't throw an exception and therefore the
>>>>>> compiler needs to explicitly handle failed allocations.
>>>>>
>>>>> ... we have two kinds of operator new implementations (or at least we
>>>>> _should_ only have two kinds):
>>>>>
>>>>> 1. Will abort on OOM (either ResourceArea exhaustion or Stack
>>>>> exhaustion) and so never returns NULL
>>>>> 2. Can return NULL and uses std::nothrow to indicate that
>>>>>
>>>>> So we should not need this if the nothrow is acting as I think it
>>>>> should. Have we missed places where nothrow is needed? Do we have
>>>>> errant operator new definitions unrelated to the allocation base
>>>>> classes?
>>>> Hi David,
>>>>
>>>> I think there may be some confusion about what std::nothrow_t is
>>>> actually doing. It provides a mechanism to overload operator new &
>>>> delete in order to direct the compiler to prefer a function definition
>>>> that also should contain an empty "throw()" specification. Take for
>>>> example the standard definition of the global ::operator new(). It
>>>> actually looks like:
>>>>
>>>> * void * operator new
>>>> <a00960.html#a1414bcdb34c39ce82fc84bc8d5287966>
>>>> (std::size_t) throw (std::bad_alloc)
>>>> * void * operator new
>>>> <a00960.html#a940b606c7824b4d5dd121199c277d629>
>>>> (std::size_t, const std::nothrow_t &) throw ()
>>>>
>>>> When the std::nothrow_t parameter is passed to the global ::operator
>>>> new() the invoked function is the one that also contains an empty
>>>> throw() specification. It is due to this empty throw() specification
>>>> that directs or triggers the C++ compiler to generate the detection of
>>>> NULL constructor prologue. Specifying only the std::nothrow_t
>>>> parameter to a class member user-defined ::operator new() does not
>>>> indicate to a C++ compiler that the function does not throw an
>>>> exception. According to the c++98 standard, a function that will
>>>> throw
>>>> no exceptions should be delcared with an empty "throw()" list
>>>> specification. When first experimenting with a fix, I did just try to
>>>> overload Hotspot's user-defined operator new() with the std:nothrow_t
>>>> parameter. As expected, the mere presence of this parameter did not
>>>> trigger the clang++ compiler to generate NULL detection constructor
>>>> prologue code. Unfortunately, probably due to legacy code, some C++
>>>> compilers behave differently. For example, I did confirm with Solaris
>>>> C++ development, the Solaris C++ by default always generates
>>>> constructor
>>>> prologue code that detects NULL for class member user-defined operator
>>>> new() even in a throw situation. G++ provides the -fcheck-new command
>>>> line option. It is due to these differences of behavior or Hotspot's
>>>> build specification of -fcheck-new that has led us to believe that
>>>> std::nothrow_t was providing something that it is not.
>>>>
>>>> That said, I would also prefer what you propose, deciding on two kinds
>>>> of operator new()'s that can be defined. I did examine all of the
>>>> user-defined operator new()'s within the code and I determined that
>>>> only
>>>> a very small number possibly could not return NULL. Given this,
>>>> factored in with the historical reliance on -fcheck-new for the g++
>>>> compilers, I decided the safe route for JDK 8 was to introduce the
>>>> NOEXCEPT macro.
>>>>
>>>> Lois
>>>>>
>>>>> David
>>>>> -----
>>>>>
>>>>>> The g++ man page states:
>>>>>>
>>>>>> -fcheck-new
>>>>>> Check that the pointer returned by "operator new" is
>>>>>> non-null before attempting to modify the storage allocated. This
>>>>>> check
>>>>>> is normally unnecessary because the C++ standard specifies that
>>>>>> "operator new" will only return 0 if it is declared
>>>>>> throw(),
>>>>>> in which case the compiler will always check the return value even
>>>>>> without this option. In all other cases, when "operator new"
>>>>>> has a non-empty exception specification, memory
>>>>>> exhaustion
>>>>>> is signalled by throwing "std::bad_alloc". See also new (nothrow).
>>>>>>
>>>>>> /Mikael
>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>> David
>>>>>>>
>>>>>>>> Tests:
>>>>>>>>
>>>>>>>> Solaris: built fastdebug & product images
>>>>>>>> Linux: built fastdebug & product images
>>>>>>>> MacOS: built fastdebug & product images using llvm-g++ -
>>>>>>>> ran
>>>>>>>> JTREG
>>>>>>>> built fastdebug & product images using
>>>>>>>> clang++ -
>>>>>>>> ran JTREG, JCK vm & lang, vm.quick.testlist (in progress)
>>>>>>>> Windows: built fastdebug & product images with VS2010
>>>>>>>>
>>>>
>>
More information about the hotspot-runtime-dev
mailing list