8205118: CodeStrings::copy() assertion caused by -XX:+VerifyOops -XX:+PrintStubCode

[Stuart Monteith]

Hi,
  Looks ok to me, although I'm in the Aleksey camp as far as using a
getter rather than accessing the field directly. If I'm to pick a nit,
I'd add the missing space (that admittedly was never there):

From
 1155     *ps = new CodeString(n->string(),n->_offset);
to
 1155     *ps = new CodeString(n->string(), n->_offset);

Thanks,
    Stuart

On 18 June 2018 at 18:07, Aleksey Shipilev <shade at redhat.com> wrote:
> On 06/18/2018 07:02 PM, Andrew Haley wrote:
>> My recent patch to re-enable the printing of code comments in
>> PrintStubCode revealed a latent bug in CodeStrings::copy().
>> VerifyOops uses CodeStrings to hold its assertion strings, and these
>> are distinguished from code comments by an offset of -1.  (Presumably
>> to make sure they're not interpreted as code comments by the
>> disassembler.)  Unfortunately, CodeStrings::copy() triggers an
>> assertion failure when it sees any of the assertion strings.
>>
>> The best fix, IMO, is to correct CodeStrings::copy(): it shouldn't
>> fail whatever the code strings are.
>>
>> http://cr.openjdk.java.net/~aph/8205118-1/
>
> This fix looks like a typo :)
>
> Better make it explicit, e.g.
>
>   intptr_t     offset() const { assert(_offset >= 0, "offset for non comment?"); return offset_raw();  }
>   intptr_t offset_raw() const { return _offset;  }
>
> ...
>
>   *ps = new CodeString(n->string(),n->offset_raw());
>
> -Aleksey
>