RFR/advice: 8054292 : Code comments leak in fastdebug builds

[Vladimir Kozlov]

On 8/14/14 10:51 AM, David Chase wrote:
> Correction — we do NOT copy strings out, we instead copy the pointer,
> hence the need to zero the strings in the CodeBuffer after a copy-out
> if we reclaim non-zeroes strings in the destructor.
>
>> You added _code.free_strings() in C1 but you missed the same case in C2 when compilation is bailed out and strings are not copied to nmethod.
>
>> In scratch_emit_size() you should free strings instead of setting NULL since they are not copied at that place.
>
> I think I will modify the code to do the reclamation in the destructor, because otherwise
> I’ll lose track of all the places where the strings go uncopied (and if I don’t, the next guy
> will).  There’s many more failure paths than success paths, so it is easier to track success
> and zero the pointer after copying, and I can be pretty sure that I have executed the
> success paths (but not the failure paths).

Yes, if you always zero after a copy (it is only one place free_strings(), right?).
And do not zero in other places (to not create dangling pointers) this should work.

>
> Or are you saying that safest is simply to deep-copy always  and always free strings
> in the destructor and never set them NULL?

I meant it would be simplest and clean solution from language point of view but you will pay performance and memory for 
that. So I prefer your suggestion above.

>
>> CodeBuffer::free_strings() should set _Strings to NULL after freeing them.
>
> A side-effect of free_strings is that _Strings.isNull becomes true.
> I’ll comment this.
>
>> Indention is wrong in new code - should be 2 spaces.
>
> Will fix.
>
>> And no spaces around arrow:  "cb -> “.
>
> Fixed.
>
> I’ll come around with another webrev after another round of testing.
>

Thanks,
Vladimir