Round 2 RFR (S) 6424123: JVM crashes on failed 'strdup' call.

[Zhengyu Gu]

Hi David,

On 6/9/2014 11:50 PM, David Holmes wrote:
> I am not sure what needs to be checked?
>
> The result of calling os::strdup - else we haven't fixed the original 
> bug. Unless you are saying that all uses of os::strdup do check the 
> return value somewhere in the call chain?
>
Thanks for the clarification. That is what the main purpose of this 
change. As I mentioned in earlier email, I intended to replace the call 
sites that
do not check return values with the new function that handles null 
return values.


>>
>> To determine whether uses os::strdup() or DuplicationString(), should be
>> the same way to determine whether uses os::malloc() or AllocateHeap(),
>> in my opinion.
>
> AllocateHeap should only be being used as the implementation for 
> CHeapObj and related types, it is not a general purpose allocation 
> interface. So I don't see that the analogy holds. I'd rather see only 
> os::strdup used with all the clients doing the null check (somewhere 
> in the stack) and handling it appropriately.
>
That is not true. AllocateHeap() is widely used, all NEW_C_HEAP_xxx 
macros are wrappers of AllocateHeap(). According to comments, the macros
are primary functions for allocating memory.

// The following macros and function should be used to allocate memory
// directly in the resource area or in the C-heap, The _OBJ variants
// of the NEW/FREE_C_HEAP macros are used for alloc/dealloc simple
// objects which are not inherited from CHeapObj, note constructor and
// destructor are not called. The preferable way to allocate objects
// is using the new operator.
//
// WARNING: The array variant must only be used for a homogenous array
// where all objects are of the exact type specified. If subtypes are
// stored in the array then must pay attention to calling destructors
// at needed.
//
//   NEW_RESOURCE_ARRAY(type, size)
//   NEW_RESOURCE_OBJ(type)
//   NEW_C_HEAP_ARRAY(type, size)
//   NEW_C_HEAP_OBJ(type, memflags)
//   FREE_C_HEAP_ARRAY(type, old, memflags)
//   FREE_C_HEAP_OBJ(objname, type, memflags)
//   char* AllocateHeap(size_t size, const char* name);
//   void  FreeHeap(void* p);
//

A quick grep shows that hotspot repo has about 40 instances of 
os::malloc(), but close to 400 instances NEW_C_HEAP_ARRAY* alone.

Thanks,

-Zhengyu