1234567: [REDO] JDK-8245121 (bf) XBuffer.put(Xbuffer src) can give unexpected result when storage overlaps

[Paul Sandoz]

Grrr… I should have realized this. When I added mismatch support I probably ran into similar issues with this outlier of an implementation.

For mismatch I punted on the optimization, because the underlying storage of string might be in compact form, rather than in the same layout as char[]. I am sure its possible to support but I thought the effort was questionable. The same reasoning can apply here too.

My inclination here is to generate conditional code that only applies for CharBuffer, and for the other implementations place in an assert on the conditions of null base and direct being true.

The CharBuffer implementation can use arc.charRegionOrder() returning null to drop into the sequential loop (same trick used for mismatch).

Paul.


> On Jun 1, 2020, at 2:28 PM, Brian Burkhalter <brian.burkhalter at oracle.com> wrote:
> 
> In the patch for JDK-8245121 [1] I overlooked the case for XBuffer.putXBuffer(src) when src is neither direct nor heap [2]. In this case the base (hb) could be null and there is no memory address either. The patch is modified from that for JDK-8245121 to detect whether src is not direct and has a null base in which case the old loopy put is used. This case presently occurs only when src is a StringCharBuffer. The corrected patch is at [3]. Note that as StringCharBuffer is read-only, the reverse copy direction cannot occur, i.e., ‘this' is not direct and has a null base. Note that the patch [3] redoes the patch for [1] with this correction included.
> 
> I don’t know whether it might be better for the StringCharBuffer source case to get a handle to the internal String values array and use that directly. This would be a read-only access.
> 
> Regression tests are in progress.
> 
> Thanks,
> 
> Brian
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8245121
> [2] https://bugs.openjdk.java.net/browse/JDK-8246282
> [3] http://cr.openjdk.java.net/~bpb/8246282/webrev.00/