Fix unsafe memory load/store from the argument encoder/decoder affecting ARM

https://bugs.webkit.org/show_bug.cgi?id=125674

Patch by Benjamin Poulain <bpoulain@apple.com> on 2013-12-12
Reviewed by Darin Adler.

Depending on the CPU and CPU config, load and store may have to be aligned.
The argument buffer has no particular alignment which can cause problems.

In this case, on ARMv7, strd and ldrd can have alignment restriction on 16 bytes.
The code encoding double and 64 bits integers was causing bugs.

To avoid problems, the encoders/decoders are modified to just use memcpy. The compiler optimizes
it away for the right instructions (clang uses two ldr/str in the case of 64bits values on ARMv7).

* Platform/CoreIPC/ArgumentDecoder.cpp:
(CoreIPC::decodeValueFromBuffer):
(CoreIPC::ArgumentDecoder::decode):
* Platform/CoreIPC/ArgumentEncoder.cpp:
(CoreIPC::copyValueToBuffer):
(CoreIPC::ArgumentEncoder::encode):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@160529 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Task-number: QTBUG-44740
Change-Id: I9bd448cbfc524c62bdf4bfaad52fa194d8159726
Reviewed-by: Julien Brianceau <jbriance@cisco.com>
Reviewed-by: Andras Becsi <andras.becsi@theqtcompany.com>

2 files changed, 34 insertions, 39 deletions

diff --git a/Source/WebKit2/Platform/CoreIPC/ArgumentDecoder.cpp b/Source/WebKit2/Platform/CoreIPC/ArgumentDecoder.cpp
index e23d28a2c..73da9c914 100644
--- a/Source/WebKit2/Platform/CoreIPC/ArgumentDecoder.cpp
+++ b/Source/WebKit2/Platform/CoreIPC/ArgumentDecoder.cpp
@@ -129,13 +129,19 @@ bool ArgumentDecoder::decodeVariableLengthByteArray(DataReference& dataReference
     return true;
 }
 
+template<typename Type>
+static void decodeValueFromBuffer(Type& value, uint8_t*& bufferPosition)
+{
+    memcpy(&value, bufferPosition, sizeof(value));
+    bufferPosition += sizeof(Type);
+}
+
 bool ArgumentDecoder::decode(bool& result)
 {
     if (!alignBufferPosition(sizeof(result), sizeof(result)))
         return false;
     
-    result = *reinterpret_cast<bool*>(m_bufferPos);
-    m_bufferPos += sizeof(result);
+    decodeValueFromBuffer(result, m_bufferPos);
     return true;
 }
 
@@ -144,8 +150,7 @@ bool ArgumentDecoder::decode(uint8_t& result)
     if (!alignBufferPosition(sizeof(result), sizeof(result)))
         return false;
 
-    result = *reinterpret_cast<uint8_t*>(m_bufferPos);
-    m_bufferPos += sizeof(result);
+    decodeValueFromBuffer(result, m_bufferPos);
     return true;
 }
 
@@ -154,8 +159,7 @@ bool ArgumentDecoder::decode(uint16_t& result)
     if (!alignBufferPosition(sizeof(result), sizeof(result)))
         return false;
 
-    result = *reinterpret_cast_ptr<uint16_t*>(m_bufferPos);
-    m_bufferPos += sizeof(result);
+    decodeValueFromBuffer(result, m_bufferPos);
     return true;
 }
 
@@ -163,9 +167,8 @@ bool ArgumentDecoder::decode(uint32_t& result)
 {
     if (!alignBufferPosition(sizeof(result), sizeof(result)))
         return false;
-    
-    result = *reinterpret_cast_ptr<uint32_t*>(m_bufferPos);
-    m_bufferPos += sizeof(result);
+
+    decodeValueFromBuffer(result, m_bufferPos);
     return true;
 }
 
@@ -174,8 +177,7 @@ bool ArgumentDecoder::decode(uint64_t& result)
     if (!alignBufferPosition(sizeof(result), sizeof(result)))
         return false;
     
-    result = *reinterpret_cast_ptr<uint64_t*>(m_bufferPos);
-    m_bufferPos += sizeof(result);
+    decodeValueFromBuffer(result, m_bufferPos);
     return true;
 }
 
@@ -184,8 +186,7 @@ bool ArgumentDecoder::decode(int32_t& result)
     if (!alignBufferPosition(sizeof(result), sizeof(result)))
         return false;
     
-    result = *reinterpret_cast_ptr<uint32_t*>(m_bufferPos);
-    m_bufferPos += sizeof(result);
+    decodeValueFromBuffer(result, m_bufferPos);
     return true;
 }
 
@@ -193,9 +194,8 @@ bool ArgumentDecoder::decode(int64_t& result)
 {
     if (!alignBufferPosition(sizeof(result), sizeof(result)))
         return false;
-    
-    result = *reinterpret_cast_ptr<uint64_t*>(m_bufferPos);
-    m_bufferPos += sizeof(result);
+
+    decodeValueFromBuffer(result, m_bufferPos);
     return true;
 }
 
@@ -203,9 +203,8 @@ bool ArgumentDecoder::decode(float& result)
 {
     if (!alignBufferPosition(sizeof(result), sizeof(result)))
         return false;
-    
-    result = *reinterpret_cast_ptr<float*>(m_bufferPos);
-    m_bufferPos += sizeof(result);
+
+    decodeValueFromBuffer(result, m_bufferPos);
     return true;
 }
 
@@ -214,8 +213,7 @@ bool ArgumentDecoder::decode(double& result)
     if (!alignBufferPosition(sizeof(result), sizeof(result)))
         return false;
     
-    result = *reinterpret_cast_ptr<double*>(m_bufferPos);
-    m_bufferPos += sizeof(result);
+    decodeValueFromBuffer(result, m_bufferPos);
     return true;
 }
 
diff --git a/Source/WebKit2/Platform/CoreIPC/ArgumentEncoder.cpp b/Source/WebKit2/Platform/CoreIPC/ArgumentEncoder.cpp
index 43246f4b5..f6a065b60 100644
--- a/Source/WebKit2/Platform/CoreIPC/ArgumentEncoder.cpp
+++ b/Source/WebKit2/Platform/CoreIPC/ArgumentEncoder.cpp
@@ -128,67 +128,64 @@ void ArgumentEncoder::encodeVariableLengthByteArray(const DataReference& dataRef
     encodeFixedLengthData(dataReference.data(), dataReference.size(), 1);
 }
 
+template<typename Type>
+static void copyValueToBuffer(Type value, uint8_t* bufferPosition)
+{
+    memcpy(bufferPosition, &value, sizeof(Type));
+}
+
 void ArgumentEncoder::encode(bool n)
 {
     uint8_t* buffer = grow(sizeof(n), sizeof(n));
-    
-    *reinterpret_cast<bool*>(buffer) = n;
+    copyValueToBuffer(n, buffer);
 }
 
 void ArgumentEncoder::encode(uint8_t n)
 {
     uint8_t* buffer = grow(sizeof(n), sizeof(n));
-
-    *reinterpret_cast<uint8_t*>(buffer) = n;
+    copyValueToBuffer(n, buffer);
 }
 
 void ArgumentEncoder::encode(uint16_t n)
 {
     uint8_t* buffer = grow(sizeof(n), sizeof(n));
-
-    *reinterpret_cast_ptr<uint16_t*>(buffer) = n;
+    copyValueToBuffer(n, buffer);
 }
 
 void ArgumentEncoder::encode(uint32_t n)
 {
     uint8_t* buffer = grow(sizeof(n), sizeof(n));
-    
-    *reinterpret_cast_ptr<uint32_t*>(buffer) = n;
+    copyValueToBuffer(n, buffer);
 }
 
 void ArgumentEncoder::encode(uint64_t n)
 {
     uint8_t* buffer = grow(sizeof(n), sizeof(n));
-    
-    *reinterpret_cast_ptr<uint64_t*>(buffer) = n;
+    copyValueToBuffer(n, buffer);
 }
 
 void ArgumentEncoder::encode(int32_t n)
 {
     uint8_t* buffer = grow(sizeof(n), sizeof(n));
-    
-    *reinterpret_cast_ptr<int32_t*>(buffer) = n;
+    copyValueToBuffer(n, buffer);
 }
 
 void ArgumentEncoder::encode(int64_t n)
 {
     uint8_t* buffer = grow(sizeof(n), sizeof(n));
-    
-    *reinterpret_cast_ptr<int64_t*>(buffer) = n;
+    copyValueToBuffer(n, buffer);
 }
 
 void ArgumentEncoder::encode(float n)
 {
     uint8_t* buffer = grow(sizeof(n), sizeof(n));
-
-    *reinterpret_cast_ptr<float*>(buffer) = n;
+    copyValueToBuffer(n, buffer);
 }
 
 void ArgumentEncoder::encode(double n)
 {
     uint8_t* buffer = grow(sizeof(n), sizeof(n));
-
-    *reinterpret_cast_ptr<double*>(buffer) = n;
+    copyValueToBuffer(n, buffer);
 }
 
 void ArgumentEncoder::addAttachment(const Attachment& attachment)