From 4cf1dca26437b6020eadf250ab4768a1a63f8b4e Mon Sep 17 00:00:00 2001 From: James Roseborough Date: Sun, 15 Mar 2015 21:32:34 +0000 Subject: [PATCH] Fix aliasing issue for some multiple assignments from varargs return values --- README.html | 3 +- src/core/org/luaj/vm2/LuaClosure.java | 13 ++++---- src/core/org/luaj/vm2/LuaValue.java | 17 ++++++---- src/core/org/luaj/vm2/Varargs.java | 39 ++++++++++++++++++++-- test/junit/org/luaj/vm2/FragmentsTest.java | 8 +++++ 5 files changed, 63 insertions(+), 17 deletions(-) diff --git a/README.html b/README.html index 6f5ac567..1ed986fe 100644 --- a/README.html +++ b/README.html @@ -974,7 +974,8 @@ Files are no longer hosted at LuaForge.
  • collectgarbage() now behaves same as collectgarbage("collect") (fixes issue #41).
  • Allow access to Java inner classes using lua field syntax (fixes issue #40).
  • List keyeq() and keyindex() methods as abstract on LuaTable.Entry (issue #37).
    • -
  • Fix return value for table.remove() and table.insert() (issue #39)
    • +
  • Fix return value for table.remove() and table.insert() (fixes issue #39)
    • +
  • Fix aliasing issue for some multiple assignments from varargs return values (fixes issue #38)
    • diff --git a/src/core/org/luaj/vm2/LuaClosure.java b/src/core/org/luaj/vm2/LuaClosure.java index 7bb3eef5..bec787c2 100644 --- a/src/core/org/luaj/vm2/LuaClosure.java +++ b/src/core/org/luaj/vm2/LuaClosure.java @@ -363,16 +363,15 @@ public class LuaClosure extends LuaFunction { default: b = i>>>23; c = (i>>14)&0x1ff; - v = b>0? - varargsOf(stack,a+1,b-1): // exact arg count - varargsOf(stack, a+1, top-v.narg()-(a+1), v); // from prev top - v = stack[a].invoke(v); + v = stack[a].invoke(b>0? + varargsOf(stack, a+1, b-1): // exact arg count + varargsOf(stack, a+1, top-v.narg()-(a+1), v)); // from prev top if ( c > 0 ) { - while ( --c > 0 ) - stack[a+c-1] = v.arg(c); - v = NONE; // TODO: necessary? + v.copyto(stack, a, c-1); + v = NONE; } else { top = a + v.narg(); + v = v.dealias(); } continue; } diff --git a/src/core/org/luaj/vm2/LuaValue.java b/src/core/org/luaj/vm2/LuaValue.java index 8700d6b1..c787dbb6 100644 --- a/src/core/org/luaj/vm2/LuaValue.java +++ b/src/core/org/luaj/vm2/LuaValue.java @@ -3405,7 +3405,8 @@ public class LuaValue extends Varargs { public static Varargs varargsOf(final LuaValue[] v,Varargs r) { switch ( v.length ) { case 0: return r; - case 1: return new Varargs.PairVarargs(v[0],r); + case 1: return r.narg()>0? new Varargs.PairVarargs(v[0],r): v[0]; + case 2: return r.narg()>0? new Varargs.ArrayVarargs(v,r): new Varargs.PairVarargs(v[0],v[1]); default: return new Varargs.ArrayVarargs(v,r); } } @@ -3424,11 +3425,14 @@ public class LuaValue extends Varargs { case 0: return NONE; case 1: return v[offset]; case 2: return new Varargs.PairVarargs(v[offset+0],v[offset+1]); - default: return new Varargs.ArrayPartVarargs(v,offset,length); + default: return new Varargs.ArrayPartVarargs(v, offset, length, NONE); } } /** Construct a {@link Varargs} around an array of {@link LuaValue}s. + * + * Caller must ensure that array contents are not mutated after this call + * or undefined behavior will result. * * @param v The array of {@link LuaValue}s * @param offset number of initial values to skip in the array @@ -3438,10 +3442,11 @@ public class LuaValue extends Varargs { * @see LuaValue#varargsOf(LuaValue[], Varargs) * @see LuaValue#varargsOf(LuaValue[], int, int) */ - public static Varargs varargsOf(final LuaValue[] v, final int offset, final int length,Varargs more) { + public static Varargs varargsOf(final LuaValue[] v, final int offset, final int length, Varargs more) { switch ( length ) { case 0: return more; - case 1: return new Varargs.PairVarargs(v[offset],more); + case 1: return more.narg()>0? new Varargs.PairVarargs(v[offset],more): v[offset]; + case 2: return more.narg()>0? new Varargs.ArrayPartVarargs(v,offset,length,more): new Varargs.PairVarargs(v[offset],v[offset+1]); default: return new Varargs.ArrayPartVarargs(v,offset,length,more); } } @@ -3477,7 +3482,7 @@ public class LuaValue extends Varargs { public static Varargs varargsOf(LuaValue v1,LuaValue v2,Varargs v3) { switch ( v3.narg() ) { case 0: return new Varargs.PairVarargs(v1,v2); - default: return new Varargs.ArrayVarargs(new LuaValue[] {v1,v2},v3); + default: return new Varargs.ArrayPartVarargs(new LuaValue[]{v1,v2}, 0, 2, v3); } } @@ -3540,7 +3545,7 @@ public class LuaValue extends Varargs { public LuaValue arg1() { return NIL; } public String tojstring() { return "none"; } public Varargs subargs(final int start) { return start > 0? this: argerror(1, "start must be > 0"); } - + void copyto(LuaValue[] dest, int offset, int length) { for(;length>0; length--) dest[offset++] = NIL; } } /** diff --git a/src/core/org/luaj/vm2/Varargs.java b/src/core/org/luaj/vm2/Varargs.java index 437b8625..c1d932a1 100644 --- a/src/core/org/luaj/vm2/Varargs.java +++ b/src/core/org/luaj/vm2/Varargs.java @@ -606,9 +606,6 @@ public abstract class Varargs { ArrayVarargs(LuaValue[] v, Varargs r) { this.v = v; this.r = r ; - for (int i = 0; i < v.length; ++i) - if (v[i] == null) - throw new IllegalArgumentException("nulls in array"); } public LuaValue arg(int i) { return i < 1 ? LuaValue.NIL: i <= v.length? v[i - 1]: r.arg(i-v.length); @@ -626,6 +623,11 @@ public abstract class Varargs { return r.subargs(start - v.length); return LuaValue.varargsOf(v, start - 1, v.length - (start - 1), r); } + void copyto(LuaValue[] dest, int offset, int length) { + int n = Math.min(v.length, length); + System.arraycopy(v, 0, dest, offset, n); + r.copyto(dest, offset + n, length - n); + } } /** Varargs implemenation backed by an array of LuaValues @@ -685,5 +687,36 @@ public abstract class Varargs { return more.subargs(start - length); return LuaValue.varargsOf(v, offset + start - 1, length - (start - 1), more); } + void copyto(LuaValue[] dest, int offset, int length) { + int n = Math.min(this.v.length, length); + System.arraycopy(this.v, this.offset, dest, offset, n); + more.copyto(dest, offset + n, length - n); + } + } + + /** Copy values in a varargs into a destination array. + * Internal utility method not intended to be called directly from user code. + * @return Varargs containing same values, but flattened. + */ + void copyto(LuaValue[] dest, int offset, int length) { + for (int i=0; i