<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[209407] trunk/Source/WebKit2</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/209407">209407</a></dd>
<dt>Author</dt> <dd>andersca@apple.com</dd>
<dt>Date</dt> <dd>2016-12-06 11:20:19 -0800 (Tue, 06 Dec 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>Don't memcpy out of line data
https://bugs.webkit.org/show_bug.cgi?id=165434
Reviewed by Sam Weinig.
Change the Decoder constructor to take a buffer deallocator parameter. If the buffer deallocator is null, the
data will be copied as before. Otherwise, the memory will be adopted by the Decoder object, and will be deallocated
by invoking the data deallocator.
* Platform/IPC/Decoder.cpp:
(IPC::copyBuffer):
Add a new helper.
(IPC::Decoder::Decoder):
Copy the buffer if the deallocator is null.
(IPC::Decoder::~Decoder):
Invoke the deallocator or call fastFree if it is null.
(IPC::Decoder::unwrapForTesting):
Update constructor.
(IPC::roundUpToAlignment):
(IPC::Decoder::alignBufferPosition):
(IPC::Decoder::decodeVariableLengthByteArray):
(IPC::decodeValueFromBuffer):
Change all these to deal with const pointers.
* Platform/IPC/Decoder.h:
Add new members.
* Platform/IPC/mac/ConnectionMac.mm:
(IPC::createMessageDecoder):
When we have out of line data, pass a deallocator that calls vm_deallocate, instead of copying the data
and then immediately throwing the original away.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebKit2ChangeLog">trunk/Source/WebKit2/ChangeLog</a></li>
<li><a href="#trunkSourceWebKit2PlatformIPCDecodercpp">trunk/Source/WebKit2/Platform/IPC/Decoder.cpp</a></li>
<li><a href="#trunkSourceWebKit2PlatformIPCDecoderh">trunk/Source/WebKit2/Platform/IPC/Decoder.h</a></li>
<li><a href="#trunkSourceWebKit2PlatformIPCmacConnectionMacmm">trunk/Source/WebKit2/Platform/IPC/mac/ConnectionMac.mm</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebKit2ChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/ChangeLog (209406 => 209407)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/ChangeLog 2016-12-06 19:07:56 UTC (rev 209406)
+++ trunk/Source/WebKit2/ChangeLog 2016-12-06 19:20:19 UTC (rev 209407)
</span><span class="lines">@@ -1,3 +1,41 @@
</span><ins>+2016-12-05 Anders Carlsson <andersca@apple.com>
+
+ Don't memcpy out of line data
+ https://bugs.webkit.org/show_bug.cgi?id=165434
+
+ Reviewed by Sam Weinig.
+
+ Change the Decoder constructor to take a buffer deallocator parameter. If the buffer deallocator is null, the
+ data will be copied as before. Otherwise, the memory will be adopted by the Decoder object, and will be deallocated
+ by invoking the data deallocator.
+
+ * Platform/IPC/Decoder.cpp:
+ (IPC::copyBuffer):
+ Add a new helper.
+
+ (IPC::Decoder::Decoder):
+ Copy the buffer if the deallocator is null.
+
+ (IPC::Decoder::~Decoder):
+ Invoke the deallocator or call fastFree if it is null.
+
+ (IPC::Decoder::unwrapForTesting):
+ Update constructor.
+
+ (IPC::roundUpToAlignment):
+ (IPC::Decoder::alignBufferPosition):
+ (IPC::Decoder::decodeVariableLengthByteArray):
+ (IPC::decodeValueFromBuffer):
+ Change all these to deal with const pointers.
+
+ * Platform/IPC/Decoder.h:
+ Add new members.
+
+ * Platform/IPC/mac/ConnectionMac.mm:
+ (IPC::createMessageDecoder):
+ When we have out of line data, pass a deallocator that calls vm_deallocate, instead of copying the data
+ and then immediately throwing the original away.
+
</ins><span class="cx"> 2016-12-06 Tim Horton <timothy_horton@apple.com>
</span><span class="cx">
</span><span class="cx"> Almost half-second stall scrolling apple.com because of synchronous getPositionInformation
</span></span></pre></div>
<a id="trunkSourceWebKit2PlatformIPCDecodercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/Platform/IPC/Decoder.cpp (209406 => 209407)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/Platform/IPC/Decoder.cpp 2016-12-06 19:07:56 UTC (rev 209406)
+++ trunk/Source/WebKit2/Platform/IPC/Decoder.cpp 2016-12-06 19:20:19 UTC (rev 209407)
</span><span class="lines">@@ -36,12 +36,23 @@
</span><span class="cx">
</span><span class="cx"> namespace IPC {
</span><span class="cx">
</span><del>-Decoder::Decoder(const DataReference& buffer, Vector<Attachment> attachments)
</del><ins>+static const uint8_t* copyBuffer(const uint8_t* buffer, size_t bufferSize)
</ins><span class="cx"> {
</span><del>- initialize(buffer.data(), buffer.size());
</del><ins>+ auto bufferCopy = static_cast<uint8_t*>(fastMalloc(bufferSize));
+ memcpy(bufferCopy, buffer, bufferSize);
</ins><span class="cx">
</span><del>- m_attachments = WTFMove(attachments);
</del><ins>+ return bufferCopy;
+}
</ins><span class="cx">
</span><ins>+Decoder::Decoder(const uint8_t* buffer, size_t bufferSize, void (*bufferDeallocator)(const uint8_t*, size_t), Vector<Attachment> attachments)
+ : m_buffer { bufferDeallocator ? buffer : copyBuffer(buffer, bufferSize) }
+ , m_bufferPos { m_buffer }
+ , m_bufferEnd { m_buffer + bufferSize }
+ , m_bufferDeallocator { bufferDeallocator }
+ , m_attachments { WTFMove(attachments) }
+{
+ ASSERT(!(reinterpret_cast<uintptr_t>(m_buffer) % alignof(uint64_t)));
+
</ins><span class="cx"> if (!decode(m_messageFlags))
</span><span class="cx"> return;
</span><span class="cx">
</span><span class="lines">@@ -58,7 +69,12 @@
</span><span class="cx"> Decoder::~Decoder()
</span><span class="cx"> {
</span><span class="cx"> ASSERT(m_buffer);
</span><del>- fastFree(m_buffer);
</del><ins>+
+ if (m_bufferDeallocator)
+ m_bufferDeallocator(m_buffer, m_bufferEnd - m_buffer);
+ else
+ fastFree(const_cast<uint8_t*>(m_buffer));
+
</ins><span class="cx"> // FIXME: We need to dispose of the mach ports in cases of failure.
</span><span class="cx">
</span><span class="cx"> #if HAVE(QOS_CLASSES)
</span><span class="lines">@@ -103,10 +119,10 @@
</span><span class="cx"> if (!decoder.decode(wrappedMessage))
</span><span class="cx"> return nullptr;
</span><span class="cx">
</span><del>- return std::make_unique<Decoder>(wrappedMessage, WTFMove(attachments));
</del><ins>+ return std::make_unique<Decoder>(wrappedMessage.data(), wrappedMessage.size(), nullptr, WTFMove(attachments));
</ins><span class="cx"> }
</span><span class="cx">
</span><del>-static inline uint8_t* roundUpToAlignment(uint8_t* ptr, unsigned alignment)
</del><ins>+static inline const uint8_t* roundUpToAlignment(const uint8_t* ptr, unsigned alignment)
</ins><span class="cx"> {
</span><span class="cx"> // Assert that the alignment is a power of 2.
</span><span class="cx"> ASSERT(alignment && !(alignment & (alignment - 1)));
</span><span class="lines">@@ -115,17 +131,6 @@
</span><span class="cx"> return reinterpret_cast<uint8_t*>((reinterpret_cast<uintptr_t>(ptr) + alignmentMask) & ~alignmentMask);
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void Decoder::initialize(const uint8_t* buffer, size_t bufferSize)
-{
- m_buffer = static_cast<uint8_t*>(fastMalloc(bufferSize));
-
- ASSERT(!(reinterpret_cast<uintptr_t>(m_buffer) % alignof(uint64_t)));
-
- m_bufferPos = m_buffer;
- m_bufferEnd = m_buffer + bufferSize;
- memcpy(m_buffer, buffer, bufferSize);
-}
-
</del><span class="cx"> static inline bool alignedBufferIsLargeEnoughToContain(const uint8_t* alignedPosition, const uint8_t* bufferEnd, size_t size)
</span><span class="cx"> {
</span><span class="cx"> return bufferEnd >= alignedPosition && static_cast<size_t>(bufferEnd - alignedPosition) >= size;
</span><span class="lines">@@ -133,7 +138,7 @@
</span><span class="cx">
</span><span class="cx"> bool Decoder::alignBufferPosition(unsigned alignment, size_t size)
</span><span class="cx"> {
</span><del>- uint8_t* alignedPosition = roundUpToAlignment(m_bufferPos, alignment);
</del><ins>+ const uint8_t* alignedPosition = roundUpToAlignment(m_bufferPos, alignment);
</ins><span class="cx"> if (!alignedBufferIsLargeEnoughToContain(alignedPosition, m_bufferEnd, size)) {
</span><span class="cx"> // We've walked off the end of this buffer.
</span><span class="cx"> markInvalid();
</span><span class="lines">@@ -169,7 +174,7 @@
</span><span class="cx"> if (!alignBufferPosition(1, size))
</span><span class="cx"> return false;
</span><span class="cx">
</span><del>- uint8_t* data = m_bufferPos;
</del><ins>+ const uint8_t* data = m_bufferPos;
</ins><span class="cx"> m_bufferPos += size;
</span><span class="cx">
</span><span class="cx"> dataReference = DataReference(data, size);
</span><span class="lines">@@ -177,7 +182,7 @@
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> template<typename Type>
</span><del>-static void decodeValueFromBuffer(Type& value, uint8_t*& bufferPosition)
</del><ins>+static void decodeValueFromBuffer(Type& value, const uint8_t*& bufferPosition)
</ins><span class="cx"> {
</span><span class="cx"> memcpy(&value, bufferPosition, sizeof(value));
</span><span class="cx"> bufferPosition += sizeof(Type);
</span></span></pre></div>
<a id="trunkSourceWebKit2PlatformIPCDecoderh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/Platform/IPC/Decoder.h (209406 => 209407)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/Platform/IPC/Decoder.h 2016-12-06 19:07:56 UTC (rev 209406)
+++ trunk/Source/WebKit2/Platform/IPC/Decoder.h 2016-12-06 19:20:19 UTC (rev 209407)
</span><span class="lines">@@ -43,9 +43,12 @@
</span><span class="cx"> class Decoder {
</span><span class="cx"> WTF_MAKE_FAST_ALLOCATED;
</span><span class="cx"> public:
</span><del>- Decoder(const DataReference& buffer, Vector<Attachment>);
</del><ins>+ Decoder(const uint8_t* buffer, size_t bufferSize, void (*bufferDeallocator)(const uint8_t*, size_t), Vector<Attachment>);
</ins><span class="cx"> ~Decoder();
</span><span class="cx">
</span><ins>+ Decoder(const Decoder&) = delete;
+ Decoder(Decoder&&) = delete;
+
</ins><span class="cx"> StringReference messageReceiverName() const { return m_messageReceiverName; }
</span><span class="cx"> StringReference messageName() const { return m_messageName; }
</span><span class="cx"> uint64_t destinationID() const { return m_destinationID; }
</span><span class="lines">@@ -130,16 +133,14 @@
</span><span class="cx">
</span><span class="cx"> static const bool isIPCDecoder = true;
</span><span class="cx">
</span><del>-protected:
- void initialize(const uint8_t* buffer, size_t bufferSize);
-
</del><ins>+private:
</ins><span class="cx"> bool alignBufferPosition(unsigned alignment, size_t);
</span><span class="cx"> bool bufferIsLargeEnoughToContain(unsigned alignment, size_t) const;
</span><span class="cx">
</span><del>-private:
- uint8_t* m_buffer;
- uint8_t* m_bufferPos;
- uint8_t* m_bufferEnd;
</del><ins>+ const uint8_t* m_buffer;
+ const uint8_t* m_bufferPos;
+ const uint8_t* m_bufferEnd;
+ void (*m_bufferDeallocator)(const uint8_t*, size_t);
</ins><span class="cx">
</span><span class="cx"> Vector<Attachment> m_attachments;
</span><span class="cx">
</span><span class="lines">@@ -149,7 +150,6 @@
</span><span class="cx">
</span><span class="cx"> uint64_t m_destinationID;
</span><span class="cx">
</span><del>-
</del><span class="cx"> #if PLATFORM(MAC)
</span><span class="cx"> std::unique_ptr<ImportanceAssertion> m_importanceAssertion;
</span><span class="cx"> #endif
</span></span></pre></div>
<a id="trunkSourceWebKit2PlatformIPCmacConnectionMacmm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/Platform/IPC/mac/ConnectionMac.mm (209406 => 209407)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/Platform/IPC/mac/ConnectionMac.mm 2016-12-06 19:07:56 UTC (rev 209406)
+++ trunk/Source/WebKit2/Platform/IPC/mac/ConnectionMac.mm 2016-12-06 19:20:19 UTC (rev 209407)
</span><span class="lines">@@ -400,7 +400,7 @@
</span><span class="cx"> uint8_t* body = reinterpret_cast<uint8_t*>(header + 1);
</span><span class="cx"> size_t bodySize = header->msgh_size - sizeof(mach_msg_header_t);
</span><span class="cx">
</span><del>- return std::make_unique<Decoder>(DataReference(body, bodySize), Vector<Attachment>());
</del><ins>+ return std::make_unique<Decoder>(body, bodySize, nullptr, Vector<Attachment> { });
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> bool messageBodyIsOOL = header->msgh_id & MessageBodyIsOutOfLine;
</span><span class="lines">@@ -439,10 +439,10 @@
</span><span class="cx"> uint8_t* messageBody = static_cast<uint8_t*>(descriptor->out_of_line.address);
</span><span class="cx"> size_t messageBodySize = descriptor->out_of_line.size;
</span><span class="cx">
</span><del>- auto decoder = std::make_unique<Decoder>(DataReference(messageBody, messageBodySize), WTFMove(attachments));
</del><ins>+ auto decoder = std::make_unique<Decoder>(messageBody, messageBodySize, [](const uint8_t* buffer, size_t length) {
+ vm_deallocate(mach_task_self(), reinterpret_cast<vm_address_t>(buffer), length);
+ }, WTFMove(attachments));
</ins><span class="cx">
</span><del>- vm_deallocate(mach_task_self(), reinterpret_cast<vm_address_t>(descriptor->out_of_line.address), descriptor->out_of_line.size);
-
</del><span class="cx"> return decoder;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -449,7 +449,7 @@
</span><span class="cx"> uint8_t* messageBody = descriptorData;
</span><span class="cx"> size_t messageBodySize = header->msgh_size - (descriptorData - reinterpret_cast<uint8_t*>(header));
</span><span class="cx">
</span><del>- return std::make_unique<Decoder>(DataReference(messageBody, messageBodySize), attachments);
</del><ins>+ return std::make_unique<Decoder>(messageBody, messageBodySize, nullptr, attachments);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> // The receive buffer size should always include the maximum trailer size.
</span></span></pre>
</div>
</div>
</body>
</html>