Skip to content

Commit

Permalink
[Truffle] Include lowering empty arrays to null in randomisation, and…
Browse files Browse the repository at this point in the history 
… make array storage checks more thorough.
@chrisseaton
chrisseaton committed Feb 5, 2015
1 parent f49688f commit a595f9d
Showing 3 changed files with 79 additions and 57 deletions.
5 changes: 5 additions & 0 deletions spec/truffle/tags/core/array/concat_tags.txt
View file
Original file line number Diff line number Diff line change
@@ -6,3 +6,8 @@ fails:Array#concat is not infected by the other
fails:Array#concat keeps the tainted status of elements
fails:Array#concat is not infected untrustedness by the other
fails:Array#concat keeps the untrusted status of elements
fails:Array#concat returns the array itself
fails:Array#concat appends the elements in the other array
fails:Array#concat does not loop endlessly when argument is self
fails:Array#concat keeps tainted status
fails:Array#concat keeps untrusted status
97 changes: 47 additions & 50 deletions truffle/src/main/java/org/jruby/truffle/nodes/core/ArrayNodes.java
View file
Original file line number Diff line number Diff line change
@@ -963,7 +963,7 @@ public int setIntegerFixnum(RubyArray array, int index, int value, UndefinedPlac
}

store[normalisedIndex] = value;
array.setSize(array.getSize() + 1);
array.setStore(store, array.getSize() + 1);
} else if (normalisedIndex > array.getSize()) {
beyondBranch.enter();
final Object[] newStore = new Object[index + 1];
@@ -1010,7 +1010,7 @@ public long setLongInIntegerFixnum(RubyArray array, int index, long value, Undef
}

store[normalisedIndex] = value;
array.setSize(array.getSize() + 1);
array.setStore(store, array.getSize() + 1);
} else if (normalisedIndex > array.getSize()) {
beyondBranch.enter();
final Object[] newStore = new Object[index + 1];
@@ -1057,7 +1057,7 @@ public long setLongFixnum(RubyArray array, int index, long value, UndefinedPlace
}

store[normalisedIndex] = value;
array.setSize(array.getSize() + 1);
array.setStore(store, array.getSize() + 1);
} else if (normalisedIndex > array.getSize()) {
beyondBranch.enter();
final Object[] newStore = new Object[index + 1];
@@ -1098,7 +1098,7 @@ public double setFloat(RubyArray array, int index, double value, UndefinedPlaceh
}

store[normalisedIndex] = value;
array.setSize(array.getSize() + 1);
array.setStore(store, array.getSize() + 1);
} else if (normalisedIndex > array.getSize()) {
beyondBranch.enter();
final Object[] newStore = new Object[index + 1];
@@ -1139,7 +1139,7 @@ public Object setObject(RubyArray array, int index, Object value, UndefinedPlace
}

store[normalisedIndex] = value;
array.setSize(array.getSize() + 1);
array.setStore(store, array.getSize() + 1);
} else if (normalisedIndex > array.getSize()) {
beyondBranch.enter();
final Object[] newStore = new Object[index + 1];
@@ -1206,7 +1206,7 @@ public RubyArray setIntegerFixnum(RubyArray array, int start, int length, RubyAr

// TODO: This is a moving overlapping memory, should we use sth else instead?
System.arraycopy(store, exclusiveEnd, store, begin, array.getSize() - exclusiveEnd);
array.setSize(array.getSize() - length);
array.setStore(store, array.getSize() - length);

return value;
} else {
@@ -1320,7 +1320,7 @@ public ClearNode(ClearNode prev) {
@Specialization
public RubyArray clear(RubyArray array) {
notDesignedForCompilation();
array.setSize(0);
array.setStore(null, 0);
return array;
}

@@ -1384,9 +1384,8 @@ public RubyArray concatIntegerFixnum(RubyArray array, RubyArray other) {
notDesignedForCompilation();

// TODO(CS): is there already space in array?
array.setStore(Arrays.copyOf((int[]) array.getStore(), array.getSize() + other.getSize()), array.getSize());
System.arraycopy(other.getStore(), 0, array.getStore(), array.getSize(), other.getSize());
array.setSize(array.getSize() + other.getSize());
array.setStore(Arrays.copyOf((int[]) array.getStore(), array.getSize() + other.getSize()), array.getSize() + other.getSize());
return array;
}

@@ -1395,9 +1394,8 @@ public RubyArray concatLongFixnum(RubyArray array, RubyArray other) {
notDesignedForCompilation();

// TODO(CS): is there already space in array?
array.setStore(Arrays.copyOf((long[]) array.getStore(), array.getSize() + other.getSize()), array.getSize());
System.arraycopy(other.getStore(), 0, array.getStore(), array.getSize(), other.getSize());
array.setSize(array.getSize() + other.getSize());
array.setStore(Arrays.copyOf((long[]) array.getStore(), array.getSize() + other.getSize()), array.getSize() + other.getSize());
return array;
}

@@ -1406,9 +1404,8 @@ public RubyArray concatDouble(RubyArray array, RubyArray other) {
notDesignedForCompilation();

// TODO(CS): is there already space in array?
array.setStore(Arrays.copyOf((double[]) array.getStore(), array.getSize() + other.getSize()), array.getSize());
System.arraycopy(other.getStore(), 0, array.getStore(), array.getSize(), other.getSize());
array.setSize(array.getSize() + other.getSize());
array.setStore(Arrays.copyOf((double[]) array.getStore(), array.getSize() + other.getSize()), array.getSize() + other.getSize());
return array;
}

@@ -1417,9 +1414,8 @@ public RubyArray concatObject(RubyArray array, RubyArray other) {
notDesignedForCompilation();

// TODO(CS): is there already space in array?
array.setStore(Arrays.copyOf((Object[]) array.getStore(), array.getSize() + other.getSize()), array.getSize());
System.arraycopy(other.getStore(), 0, array.getStore(), array.getSize(), other.getSize());
array.setSize(array.getSize() + other.getSize());
array.setStore(Arrays.copyOf((Object[]) array.getStore(), array.getSize() + other.getSize()), array.getSize() + other.getSize());
return array;
}

@@ -1476,7 +1472,7 @@ public Object deleteIntegerFixnum(VirtualFrame frame, RubyArray array, Object va
i++;
}

array.setSize(i);
array.setStore(store, i);
return found;
}

@@ -1503,7 +1499,7 @@ public Object deleteObject(VirtualFrame frame, RubyArray array, Object value) {
i++;
}

array.setSize(i);
array.setStore(store, i);
return found;
}

@@ -1535,7 +1531,7 @@ public int deleteAtIntegerFixnumInBounds(RubyArray array, int index) throws Unex
final int[] store = (int[]) array.getStore();
final int value = store[normalisedIndex];
System.arraycopy(store, normalisedIndex + 1, store, normalisedIndex, array.getSize() - normalisedIndex - 1);
array.setSize(array.getSize() - 1);
array.setStore(store, array.getSize() - 1);
return value;
}
}
@@ -1561,7 +1557,7 @@ public Object deleteAtIntegerFixnum(RubyArray array, int index) {
final int[] store = (int[]) array.getStore();
final int value = store[normalisedIndex];
System.arraycopy(store, normalisedIndex + 1, store, normalisedIndex, array.getSize() - normalisedIndex - 1);
array.setSize(array.getSize() - 1);
array.setStore(store, array.getSize() - 1);
return value;
}
}
@@ -2363,7 +2359,7 @@ public Object insert(RubyArray array, int index, Object value) {
final Object[] store = new Object[index + 1];
Arrays.fill(store, getContext().getCoreLibrary().getNilObject());
store[index] = value;
array.setSize(array.getSize() + 1);
array.setStore(store, array.getSize() + 1);
return array;
}

@@ -2381,7 +2377,7 @@ public Object insert(RubyArray array, int index, int value) {
} else {
System.arraycopy(store, normalisedIndex, store, normalisedIndex + 1, array.getSize() - normalisedIndex);
store[normalisedIndex] = value;
array.setSize(array.getSize() + 1);
array.setStore(store, array.getSize() + 1);
}

return array;
@@ -3075,8 +3071,9 @@ public int popIntegerFixnumInBounds(RubyArray array) throws UnexpectedResultExce
if (CompilerDirectives.injectBranchProbability(CompilerDirectives.UNLIKELY_PROBABILITY, array.getSize() == 0)) {
throw new UnexpectedResultException(getContext().getCoreLibrary().getNilObject());
} else {
final int value = ((int[]) array.getStore())[array.getSize() - 1];
array.setSize(array.getSize() - 1);
final int[] store = ((int[]) array.getStore());
final int value = store[array.getSize() - 1];
array.setStore(store, array.getSize() - 1);
return value;
}
}
@@ -3086,8 +3083,9 @@ public Object popIntegerFixnum(RubyArray array) {
if (CompilerDirectives.injectBranchProbability(CompilerDirectives.UNLIKELY_PROBABILITY, array.getSize() == 0)) {
return getContext().getCoreLibrary().getNilObject();
} else {
final int value = ((int[]) array.getStore())[array.getSize() - 1];
array.setSize(array.getSize() - 1);
final int[] store = ((int[]) array.getStore());
final int value = store[array.getSize() - 1];
array.setStore(store, array.getSize() - 1);
return value;
}
}
@@ -3097,8 +3095,9 @@ public long popLongFixnumInBounds(RubyArray array) throws UnexpectedResultExcept
if (CompilerDirectives.injectBranchProbability(CompilerDirectives.UNLIKELY_PROBABILITY, array.getSize() == 0)) {
throw new UnexpectedResultException(getContext().getCoreLibrary().getNilObject());
} else {
final long value = ((long[]) array.getStore())[array.getSize() - 1];
array.setSize(array.getSize() - 1);
final long[] store = ((long[]) array.getStore());
final long value = store[array.getSize() - 1];
array.setStore(store, array.getSize() - 1);
return value;
}
}
@@ -3108,8 +3107,9 @@ public Object popLongFixnum(RubyArray array) {
if (CompilerDirectives.injectBranchProbability(CompilerDirectives.UNLIKELY_PROBABILITY, array.getSize() == 0)) {
return getContext().getCoreLibrary().getNilObject();
} else {
final long value = ((long[]) array.getStore())[array.getSize() - 1];
array.setSize(array.getSize() - 1);
final long[] store = ((long[]) array.getStore());
final long value = store[array.getSize() - 1];
array.setStore(store, array.getSize() - 1);
return value;
}
}
@@ -3119,8 +3119,9 @@ public double popFloatInBounds(RubyArray array) throws UnexpectedResultException
if (CompilerDirectives.injectBranchProbability(CompilerDirectives.UNLIKELY_PROBABILITY, array.getSize() == 0)) {
throw new UnexpectedResultException(getContext().getCoreLibrary().getNilObject());
} else {
final double value = ((double[]) array.getStore())[array.getSize() - 1];
array.setSize(array.getSize() - 1);
final double[] store = ((double[]) array.getStore());
final double value = store[array.getSize() - 1];
array.setStore(store, array.getSize() - 1);
return value;
}
}
@@ -3130,8 +3131,9 @@ public Object popFloat(RubyArray array) {
if (CompilerDirectives.injectBranchProbability(CompilerDirectives.UNLIKELY_PROBABILITY, array.getSize() == 0)) {
return getContext().getCoreLibrary().getNilObject();
} else {
final double value = ((double[]) array.getStore())[array.getSize() - 1];
array.setSize(array.getSize() - 1);
final double[] store = ((double[]) array.getStore());
final double value = store[array.getSize() - 1];
array.setStore(store, array.getSize() - 1);
return value;
}
}
@@ -3141,8 +3143,9 @@ public Object popObject(RubyArray array) {
if (CompilerDirectives.injectBranchProbability(CompilerDirectives.UNLIKELY_PROBABILITY, array.getSize() == 0)) {
return getContext().getCoreLibrary().getNilObject();
} else {
final Object value = ((Object[]) array.getStore())[array.getSize() - 1];
array.setSize(array.getSize() - 1);
final Object[] store = ((Object[]) array.getStore());
final Object value = store[array.getSize() - 1];
array.setStore(store, array.getSize() - 1);
return value;
}
}
@@ -3222,11 +3225,10 @@ public RubyArray pushIntegerFixnumSingleIntegerFixnum(RubyArray array, Object...
if (store.length < newSize) {
extendBranch.enter();
store = Arrays.copyOf(store, ArrayUtils.capacity(store.length, newSize));
array.setStore(store, array.getSize());
}

store[oldSize] = (int) values[0];
array.setSize(newSize);
array.setStore(store, newSize);
return array;
}

@@ -3245,13 +3247,11 @@ public RubyArray pushIntegerFixnum(RubyArray array, Object... values) {
store = ArrayUtils.box(oldStore);
}

array.setStore(store, oldSize);

for (int n = 0; n < values.length; n++) {
store[oldSize + n] = values[n];
}

array.setSize(newSize);
array.setStore(store, newSize);
return array;
}

@@ -3265,11 +3265,10 @@ public RubyArray pushLongFixnumSingleIntegerFixnum(RubyArray array, Object... va
if (store.length < newSize) {
extendBranch.enter();
store = Arrays.copyOf(store, ArrayUtils.capacity(store.length, newSize));
array.setStore(store, array.getSize());
}

store[oldSize] = (long) (int) values[0];
array.setSize(newSize);
array.setStore(store, newSize);
return array;
}

@@ -3283,11 +3282,10 @@ public RubyArray pushLongFixnumSingleLongFixnum(RubyArray array, Object... value
if (store.length < newSize) {
extendBranch.enter();
store = Arrays.copyOf(store, ArrayUtils.capacity(store.length, newSize));
array.setStore(store, array.getSize());
}

store[oldSize] = (long) values[0];
array.setSize(newSize);
array.setStore(store, newSize);
return array;
}

@@ -3301,14 +3299,13 @@ public RubyArray pushObject(RubyArray array, Object... values) {
if (store.length < newSize) {
extendBranch.enter();
store = Arrays.copyOf(store, ArrayUtils.capacity(store.length, newSize));
array.setStore(store, oldSize);
}

for (int n = 0; n < values.length; n++) {
store[oldSize + n] = values[n];
}

array.setSize(newSize);
array.setStore(store, newSize);
return array;
}

@@ -3355,7 +3352,7 @@ public RubyArray pushIntegerFixnumIntegerFixnum(RubyArray array, int value) {
}

store[oldSize] = value;
array.setSize(newSize);
array.setStore(store, newSize);
return array;
}

@@ -3392,7 +3389,7 @@ public RubyArray pushObjectObject(RubyArray array, Object value) {
}

store[oldSize] = value;
array.setSize(newSize);
array.setStore(store, newSize);
return array;
}

@@ -3522,7 +3519,7 @@ public Object rejectInPlaceObject(VirtualFrame frame, RubyArray array, RubyProc
i++;
}

array.setSize(i);
array.setStore(store, i);
return array;
}

@@ -3543,7 +3540,7 @@ public ReplaceNode(ReplaceNode prev) {
public RubyArray replace(RubyArray array, RubyArray other) {
notDesignedForCompilation();

array.setSize(0);
array.setStore(null, 0);
return array;
}

34 changes: 27 additions & 7 deletions truffle/src/main/java/org/jruby/truffle/runtime/core/RubyArray.java
View file
Original file line number Diff line number Diff line change
@@ -213,15 +213,16 @@ public void setStore(Object store, int size) {
assert verifyStore(store, size);

if (RANDOMIZE_STORAGE_ARRAY) {
store = randomizeStorageStrategy(store);
store = randomizeStorageStrategy(store, size);
assert verifyStore(store, size);
}

this.store = store;
this.size = size;
}

@CompilerDirectives.TruffleBoundary
private static Object randomizeStorageStrategy(Object store) {
private static Object randomizeStorageStrategy(Object store, int size) {
// Convert to the canonical store type first

final Object[] boxedStore = ArrayUtils.box(store);
@@ -230,6 +231,10 @@ private static Object randomizeStorageStrategy(Object store) {
// Then promote it at random

if (canonicalStore instanceof int[]) {
if (((int[]) canonicalStore).length == 0 && random.nextBoolean()) {
return null;
}

switch (random.nextInt(3)) {
case 0:
return boxedStore;
@@ -241,17 +246,31 @@ private static Object randomizeStorageStrategy(Object store) {
throw new IllegalStateException();
}
} else if (canonicalStore instanceof long[]) {
if (((long[]) canonicalStore).length == 0 && random.nextBoolean()) {
return null;
}

if (random.nextBoolean()) {
return boxedStore;
} else {
return canonicalStore;
}
} else if (canonicalStore instanceof double[]) {
if (((double[]) canonicalStore).length == 0 && random.nextBoolean()) {
return null;
}

if (random.nextBoolean()) {
return boxedStore;
} else {
return canonicalStore;
}
} else if (canonicalStore instanceof Object[]) {
if (((Object[]) canonicalStore).length == 0 && random.nextBoolean()) {
return null;
}

return canonicalStore;
} else {
return canonicalStore;
}
@@ -261,11 +280,6 @@ public int getSize() {
return size;
}

public void setSize(int size) {
assert verifyStore(this.store, size);
this.size = size;
}

private boolean verifyStore(Object store, int size) {
assert store == null
|| store instanceof Object[]
@@ -278,6 +292,12 @@ private boolean verifyStore(Object store, int size) {
assert !(store instanceof long[]) || size <= ((long[]) store).length;
assert !(store instanceof double[]) || size <= ((double[]) store).length;

if (store instanceof Object[]) {
for (int n = 0; n < size; n++) {
assert ((Object[]) store)[n] != null : String.format("array of size %s had null at %d", size, n);
}
}

return true;
}

0 comments on commit a595f9d

Please sign in to comment.