Closed
Bug 717762
Opened 13 years ago
Closed 12 years ago
Proliferate js_memcpy and PodCopy
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla12
People
(Reporter: cdleary, Assigned: cdleary)
References
(Depends on 1 open bug)
Details
Attachments
(1 file, 1 obsolete file)
40.93 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
I think it'd be nice to have a js_memcpy(dst, src, len) that asserts that src does not fall in the range [dst, dst + len) and use it throughout the engine. This passes locally, pushing to try.
Attachment #588188 -
Flags: review?(luke)
Comment 1•13 years ago
|
||
Comment on attachment 588188 [details] [diff] [review] Make js_memcpy and use it in a bunch of places. >+static JS_ALWAYS_INLINE void * >+js_memcpy(void *dst, const void *src, size_t len) >+{ >+ /* Check that src is not in [dst, dst + len). */ >+ JS_ASSERT(!(dst <= src && src < (char *) dst + len)); >+ return memcpy(dst, src, len); >+} I think non-overlapping-ness wants an even stronger assert, say, like the one in PodCopy: >@@ -316,17 +324,17 @@ PodCopy(T *dst, const T *src, size_t nel > /* Cannot find portable word-sized abs(). */ > JS_ASSERT_IF(dst >= src, size_t(dst - src) >= nelem); > JS_ASSERT_IF(src >= dst, size_t(src - dst) >= nelem); Second, it looks like a lot (but not all) of these memcpy uses could be PodCopy instead (avoiding the sizeof(T) computation). Could you switch them since you are touching all these lines anyway?
Assignee | ||
Comment 2•13 years ago
|
||
Now with PodAssign and a bunch of PodCopy's!
Attachment #588188 -
Attachment is obsolete: true
Attachment #588188 -
Flags: review?(luke)
Attachment #588229 -
Flags: review?(luke)
Comment 3•13 years ago
|
||
Comment on attachment 588229 [details] [diff] [review] Make js_memcpy and use it in a bunch of places. r+ with more PodCopy/PodAssign in TradeGuts and JSScript::NewScriptFromEmitter (if possible) and with verification that the generated asm isn't any worse.
Attachment #588229 -
Flags: review?(luke) → review+
Assignee | ||
Comment 4•13 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/b36175bbda47
Target Milestone: --- → mozilla12
Assignee | ||
Updated•13 years ago
|
Summary: Make js_memcpy for bounds checking → Proliferate js_memcpy and PodCopy
Assignee | ||
Comment 5•13 years ago
|
||
Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/4dd329d75054
Target Milestone: mozilla12 → ---
Assignee | ||
Comment 6•12 years ago
|
||
Take 2: https://hg.mozilla.org/integration/mozilla-inbound/rev/7736d47f8fab
Target Milestone: --- → mozilla12
Comment 7•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7736d47f8fab
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•