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

[Vladimir Kozlov]

I updated webrev

http://cr.openjdk.java.net/~kvn/6973963/webrev.02

I added 'virtual' to ~ResourceObj() to avoid any surprises as Dave pointed.

I save/restore allocation type around Copy::fill_to_bytes() in ~CodeBuffer() to solve my problem.

Vladimir

David Holmes wrote:
> 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