Fix aliasing issue for some multiple assignments from varargs return values

README.html

@@ -974,7 +974,8 @@ Files are no longer hosted at LuaForge.
 <li>collectgarbage() now behaves same as collectgarbage("collect") (fixes issue #41).</li>
 <li>Allow access to Java inner classes using lua field syntax (fixes issue #40).</li>
 <li>List keyeq() and keyindex() methods as abstract on LuaTable.Entry (issue #37).</li>
-<li>Fix return value for table.remove() and table.insert() (issue #39)</li>
+<li>Fix return value for table.remove() and table.insert() (fixes issue #39)</li>
+<li>Fix aliasing issue for some multiple assignments from varargs return values (fixes issue #38)</li>
 
 </ul></td></tr>
 </table></td></tr></table>

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? 
+						v = stack[a].invoke(b>0? 
-							varargsOf(stack,a+1,b-1): // exact arg count
+							varargsOf(stack, a+1, b-1): // exact arg count
-							varargsOf(stack, a+1, top-v.narg()-(a+1), v); // from prev top 
+							varargsOf(stack, a+1, top-v.narg()-(a+1), v));  // from prev top 
-						v = stack[a].invoke(v);
 						if ( c > 0 ) {
-							while ( --c > 0 )
+							v.copyto(stack, a, c-1);
-								stack[a+c-1] = v.arg(c);
+							v = NONE;
-							v = NONE; // TODO: necessary?
 						} else {
 							top = a + v.narg();
+							v = v.dealias();
 						}
 						continue;
 					}

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; }
 	}
 	
 	/**

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<length; ++i)
+			dest[offset+i] = arg(i+1);
+	}
+
+	/** Return Varargs that cannot be using a shared array for the storage, and is flattened.
+	 * Internal utility method not intended to be called directly from user code.
+	 * @return Varargs containing same values, but flattened and with a new array if needed.
+	 */
+	Varargs dealias() {
+		int n = narg();
+		switch (n) {
+		case 0: return LuaValue.NONE;
+		case 1: return arg1();
+		case 2: return new PairVarargs(arg1(), arg(2));
+		default:
+			LuaValue[] v = new LuaValue[n];
+			copyto(v, 0, n);
+			return new ArrayVarargs(v, LuaValue.NONE);
+		}
 	}
 }

test/junit/org/luaj/vm2/FragmentsTest.java

@@ -600,5 +600,13 @@ public class FragmentsTest extends TestSuite {
 		public void testTypeOfTableRemoveReturnValue() {
 			runFragment(LuaValue.valueOf("nil"), "local k = table.remove({ }) return type(k)");
 		}
+		public void testVarargBugReport() {
+			runFragment(LuaValue.varargsOf(new LuaValue[] { 
+					LuaValue.valueOf(1), LuaValue.valueOf(2), LuaValue.valueOf(3) }),
+					"local i = function(...) return ... end\n"
+					+ "local v1, v2, v3 = i(1, 2, 3)\n"
+					+ "return v1, v2, v3");
+			
+		}
 	}
 }