Skip to content

Commit

Permalink
Merge r230264 - JSArray::appendMemcpy seems to be missing a barrier
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=184290

Reviewed by Mark Lam.

If you write to an array that may contain pointers and you didn't just allocate it, then you need to
barrier right after.

I don't know if this is really a bug - it's possible that all callers of appendMemcpy do things that
obviate the need for this barrier. But these barriers are cheap, so we should do them if in doubt.

* runtime/JSArray.cpp:
(JSC::JSArray::appendMemcpy):
  • Loading branch information
Filip Pizlo authored and carlosgcampos committed Apr 9, 2018
1 parent 4f64525 commit f963d0c
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 1 deletion.
16 changes: 16 additions & 0 deletions Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,19 @@
2018-04-03 Filip Pizlo <fpizlo@apple.com>

JSArray::appendMemcpy seems to be missing a barrier
https://bugs.webkit.org/show_bug.cgi?id=184290

Reviewed by Mark Lam.

If you write to an array that may contain pointers and you didn't just allocate it, then you need to
barrier right after.

I don't know if this is really a bug - it's possible that all callers of appendMemcpy do things that
obviate the need for this barrier. But these barriers are cheap, so we should do them if in doubt.

* runtime/JSArray.cpp:
(JSC::JSArray::appendMemcpy):

2018-03-31 Filip Pizlo <fpizlo@apple.com>

JSC crash in JIT code with for-of loop and Array/Set iterators
Expand Down
4 changes: 3 additions & 1 deletion Source/JavaScriptCore/runtime/JSArray.cpp
Expand Up @@ -554,8 +554,10 @@ bool JSArray::appendMemcpy(ExecState* exec, VM& vm, unsigned startIndex, JSC::JS
}
} else if (type == ArrayWithDouble)
memcpy(butterfly()->contiguousDouble().data() + startIndex, otherArray->butterfly()->contiguousDouble().data(), sizeof(JSValue) * otherLength);
else
else {
memcpy(butterfly()->contiguous().data() + startIndex, otherArray->butterfly()->contiguous().data(), sizeof(JSValue) * otherLength);
vm.heap.writeBarrier(this);
}

return true;
}
Expand Down

0 comments on commit f963d0c

Please sign in to comment.