Commit 69bb1100 authored by antonm@chromium.org's avatar antonm@chromium.org

Fix the case of no words to copy.

CopyWords cannot actually copy zero words---it'd clobber destiantion with
the first word of source.

Add an ASSERT to check this condition plus update array builtins to verify
for amount of copied data when necessary.

TBR=vitalyr@chromium.org

Review URL: http://codereview.chromium.org/1559004

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@4313 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
parent 17eece57
...@@ -268,6 +268,7 @@ static void CopyElements(AssertNoAllocation* no_gc, ...@@ -268,6 +268,7 @@ static void CopyElements(AssertNoAllocation* no_gc,
int src_index, int src_index,
int len) { int len) {
ASSERT(dst != src); // Use MoveElements instead. ASSERT(dst != src); // Use MoveElements instead.
ASSERT(len > 0);
CopyWords(dst->data_start() + dst_index, CopyWords(dst->data_start() + dst_index,
src->data_start() + src_index, src->data_start() + src_index,
len); len);
...@@ -390,7 +391,9 @@ BUILTIN(ArrayPush) { ...@@ -390,7 +391,9 @@ BUILTIN(ArrayPush) {
FixedArray* new_elms = FixedArray::cast(obj); FixedArray* new_elms = FixedArray::cast(obj);
AssertNoAllocation no_gc; AssertNoAllocation no_gc;
CopyElements(&no_gc, new_elms, 0, elms, 0, len); if (len > 0) {
CopyElements(&no_gc, new_elms, 0, elms, 0, len);
}
FillWithHoles(new_elms, new_length, capacity); FillWithHoles(new_elms, new_length, capacity);
elms = new_elms; elms = new_elms;
...@@ -537,7 +540,9 @@ BUILTIN(ArrayUnshift) { ...@@ -537,7 +540,9 @@ BUILTIN(ArrayUnshift) {
FixedArray* new_elms = FixedArray::cast(obj); FixedArray* new_elms = FixedArray::cast(obj);
AssertNoAllocation no_gc; AssertNoAllocation no_gc;
CopyElements(&no_gc, new_elms, to_add, elms, 0, len); if (len > 0) {
CopyElements(&no_gc, new_elms, to_add, elms, 0, len);
}
FillWithHoles(new_elms, new_length, capacity); FillWithHoles(new_elms, new_length, capacity);
elms = new_elms; elms = new_elms;
...@@ -734,11 +739,16 @@ BUILTIN(ArraySplice) { ...@@ -734,11 +739,16 @@ BUILTIN(ArraySplice) {
AssertNoAllocation no_gc; AssertNoAllocation no_gc;
// Copy the part before actual_start as is. // Copy the part before actual_start as is.
CopyElements(&no_gc, new_elms, 0, elms, 0, actual_start); if (actual_start > 0) {
CopyElements(&no_gc, CopyElements(&no_gc, new_elms, 0, elms, 0, actual_start);
new_elms, actual_start + item_count, }
elms, actual_start + actual_delete_count, const int to_copy = len - actual_delete_count - actual_start;
(len - actual_delete_count - actual_start)); if (to_copy > 0) {
CopyElements(&no_gc,
new_elms, actual_start + item_count,
elms, actual_start + actual_delete_count,
to_copy);
}
FillWithHoles(new_elms, new_length, capacity); FillWithHoles(new_elms, new_length, capacity);
elms = new_elms; elms = new_elms;
...@@ -812,10 +822,12 @@ BUILTIN(ArrayConcat) { ...@@ -812,10 +822,12 @@ BUILTIN(ArrayConcat) {
int start_pos = 0; int start_pos = 0;
for (int i = 0; i < n_arguments; i++) { for (int i = 0; i < n_arguments; i++) {
JSArray* array = JSArray::cast(args[i]); JSArray* array = JSArray::cast(args[i]);
FixedArray* elms = FixedArray::cast(array->elements());
int len = Smi::cast(array->length())->value(); int len = Smi::cast(array->length())->value();
CopyElements(&no_gc, result_elms, start_pos, elms, 0, len); if (len > 0) {
start_pos += len; FixedArray* elms = FixedArray::cast(array->elements());
CopyElements(&no_gc, result_elms, start_pos, elms, 0, len);
start_pos += len;
}
} }
ASSERT(start_pos == result_len); ASSERT(start_pos == result_len);
......
...@@ -600,6 +600,8 @@ static inline void MemsetPointer(T** dest, T* value, int counter) { ...@@ -600,6 +600,8 @@ static inline void MemsetPointer(T** dest, T* value, int counter) {
// Copies data from |src| to |dst|. The data spans MUST not overlap. // Copies data from |src| to |dst|. The data spans MUST not overlap.
inline void CopyWords(Object** dst, Object** src, int num_words) { inline void CopyWords(Object** dst, Object** src, int num_words) {
ASSERT(Min(dst, src) + num_words <= Max(dst, src)); ASSERT(Min(dst, src) + num_words <= Max(dst, src));
ASSERT(num_words > 0);
// Use block copying memcpy if the segment we're copying is // Use block copying memcpy if the segment we're copying is
// enough to justify the extra call/setup overhead. // enough to justify the extra call/setup overhead.
static const int kBlockCopyLimit = 16; static const int kBlockCopyLimit = 16;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment