[vmkit-commits] [vmkit] r145527 - /vmkit/trunk/lib/j3/ClassLib/ArrayCopy.inc

[Nicolas Geoffray]

Hi Will,

On Wed, Nov 30, 2011 at 11:12 PM, Will Dietz <wdietz2 at illinois.edu> wrote:

> Author: wdietz2
> Date: Wed Nov 30 16:12:56 2011
> New Revision: 145527
>
> URL: http://llvm.org/viewvc/llvm-project?rev=145527&view=rev
> Log:
> ArrayCopy: handle overlapping arrays of Objects
>
> Modified:
>    vmkit/trunk/lib/j3/ClassLib/ArrayCopy.inc
>
> Modified: vmkit/trunk/lib/j3/ClassLib/ArrayCopy.inc
> URL:
> http://llvm.org/viewvc/llvm-project/vmkit/trunk/lib/j3/ClassLib/ArrayCopy.inc?rev=145527&r1=145526&r2=145527&view=diff
>
> ==============================================================================
> --- vmkit/trunk/lib/j3/ClassLib/ArrayCopy.inc (original)
> +++ vmkit/trunk/lib/j3/ClassLib/ArrayCopy.inc Wed Nov 30 16:12:56 2011
> @@ -62,17 +62,30 @@
>   }
>
>   if (!(dstType->isPrimitive())) {
> +    // Scan to ensure element compatibility, recording the first item
> +    // that requires an exception be thrown.
> +    int badEl = -1;
>     for (int i = 0; i < len; i++) {
>       cur = ArrayObject::getElement((ArrayObject*)src, i + sstart);
>       if (cur) {
>         if (!(JavaObject::getClass(cur)->isAssignableFrom(dstType))) {
> -          th->throwException(vm->CreateArrayStoreException(
> -              (JavaVirtualTable*)dst->getVirtualTable()));
> -          UNREACHABLE();
> +          badEl = i;
> +          break;
>         }
>       }
> -      ArrayObject::setElement((ArrayObject*)dst, cur, i + dstart);
>     }
> +
> +    // Copy over the non-conflicting elements, handling overlaps
> +    void* ptrDst =
> (void*)((int64_t)ArrayObject::getElements((ArrayObject*)dst) +
> dstart*sizeof(void*));
> +    void* ptrSrc =
> (void*)((int64_t)ArrayObject::getElements((ArrayObject*)src) +
> sstart*sizeof(void*));
> +    int copyLen = badEl == -1 ? len : badEl - 1;
> +    if (copyLen > 0)
> +      memmove(ptrDst, ptrSrc, copyLen * sizeof(void*));
>

Please do not do a memmove on ArrayObject. It is fine to do a memmove on
primitive arrays, but definitely not on arrays of objects. You need to
execute a write barrier for each copied element.

To fix overlaps, I guess you need to check the start indices for both
arrays, and see which one starts first to know in which side you need to
iterate (from end to start or from start to end).

Nicolas


> +
> +    // TODO: Record the conflicting types in the exception message?
> +    if (badEl != -1)
> +      vm->arrayStoreException();
> +
>   } else {
>     uint32 logSize = dstType->asPrimitiveClass()->logSize;
>     void* ptrDst = (void*)((int64_t)JavaArray::getElements(dst) + (dstart
> << logSize));
>
>
> _______________________________________________
> vmkit-commits mailing list
> vmkit-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/vmkit-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/vmkit-commits/attachments/20111130/f828e1b7/attachment.html>