[Fwd: Request for reviews (M): 6973963: SEGV in ciBlock::start_bci() with EA]

[David Holmes]

Vladimir Kozlov said the following on 08/03/10 19:33:
> On 8/2/10 11:23 PM, David Holmes wrote:
>> src/share/vm/asm/codeBuffer.cpp
>>
>> + void CodeBuffer::operator delete(void* p) {
>> + ResourceObj::operator delete(p);
>> #ifdef ASSERT
>> ! Copy::fill_to_bytes(p, sizeof(CodeBuffer), badResourceValue);
>> #endif
>> }
>>
>> You moved the copy from the destructor to operator delete, but now you 
>> access p after you have deleted it.
> 
> Yes, you are right. But CodeBuffer is never allocated on C heap and 
> ResourceObj::delete(p)
> should be called only for allocation on C_HEAP. So this code should 
> never be executed,

I'm confused. CodeBuffer is a StackObj, not a ResourceObj, so why would 
you call ResourceObj::delete in the first place ??

> I will replace it with ShouldNotCallThis() in CodeBuffer::delete(void* p).
> Which leaves the original problem: I need to find how to zap CodeBuffer 
> after ResourceObj destructor
> which is called after CodeBuffer destructor.

Again I'm missing something - what is the connection between CodeBuffer 
and ResourceObj ?

> Note: the problem here is not CodeBuffer but its field: OopRecorder  
> _default_oop_recorder;

Explain that to me off-list if you like, I'm not familiar with this part 
of the code.

>> src/share/vm/memory/allocation.hpp
>>
>> ! ~ResourceObj();
>>
>> You've added a destructor, but subclasses define their own destructors 
>> - so doesn't this need to be virtual?
> 
> Constructor and destructor are special methods. Super class's default 
> destructor are always called from
> subclass's destructor. The only case when you need to declare it as 
> virtual if subclass object is assigned to
> local/field of super class type and compiler does not know what the 
> original subclass was.
> 
> ResourceObj *f = new OopRecorder();
> ...
> delete f;
> 
> We don't use such constructions.

Ok. Just wanted to check. In another codebase we got bitten by a cleanup 
method in the superclass that did "delete this;" which did not invoke 
the destructor for the dynamic type of 'this' because the destructor was 
not virtual.

Cheers,
David