Title: [206151] trunk/Source
Revision
206151
Author
jbed...@apple.com
Date
2016-09-20 10:14:00 -0700 (Tue, 20 Sep 2016)

Log Message

Undefined behavior: Left shift negative number
https://bugs.webkit.org/show_bug.cgi?id=161866

Reviewed by Keith Miller.

Left shifting a negative number is undefined behavior in C/C++, although most implementations do define it. Explicitly clarifying the intended behavior due to shifting negative number in some cases.

Source/_javascript_Core:

* dfg/DFGAbstractHeap.h:
(JSC::DFG::AbstractHeap::encode): Explicitly cast signed integer for left shift.

Source/WTF:

* wtf/text/Base64.cpp:
(WTF::base64EncodeInternal): Changed signed character to unsigned when shifting.
(WTF::base64Encode): Ditto.
(WTF::base64URLEncode): Ditto.
(WTF::base64DecodeInternal): Ditto.
* wtf/text/Base64.h: Ditto.
(WTF::SignedOrUnsignedCharVectorAdapter): Rebuilt to stop using union as a bitwise_cast.
(WTF::ConstSignedOrUnsignedCharVectorAdapter): Ditto.

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (206150 => 206151)


--- trunk/Source/_javascript_Core/ChangeLog	2016-09-20 15:58:37 UTC (rev 206150)
+++ trunk/Source/_javascript_Core/ChangeLog	2016-09-20 17:14:00 UTC (rev 206151)
@@ -1,3 +1,15 @@
+2016-09-20  Jonathan Bedard  <jbed...@apple.com>
+
+        Undefined behavior: Left shift negative number
+        https://bugs.webkit.org/show_bug.cgi?id=161866
+
+        Reviewed by Keith Miller.
+
+        Left shifting a negative number is undefined behavior in C/C++, although most implementations do define it. Explicitly clarifying the intended behavior due to shifting negative number in some cases.
+
+        * dfg/DFGAbstractHeap.h:
+        (JSC::DFG::AbstractHeap::encode): Explicitly cast signed integer for left shift.
+
 2016-09-20  Saam Barati  <sbar...@apple.com>
 
         Unreviewed fix for 32-bit DFG x86 implementation of HasOwnProperty.

Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractHeap.h (206150 => 206151)


--- trunk/Source/_javascript_Core/dfg/DFGAbstractHeap.h	2016-09-20 15:58:37 UTC (rev 206150)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractHeap.h	2016-09-20 17:14:00 UTC (rev 206151)
@@ -298,7 +298,7 @@
     {
         int64_t kindAsInt = static_cast<int64_t>(kind);
         ASSERT(kindAsInt < (1 << topShift));
-        return kindAsInt | (payload.isTop() << topShift) | (payload.valueImpl() << valueShift);
+        return kindAsInt | (static_cast<uint64_t>(payload.isTop()) << topShift) | (bitwise_cast<uint64_t>(payload.valueImpl()) << valueShift);
     }
     
     // The layout of the value is:

Modified: trunk/Source/WTF/ChangeLog (206150 => 206151)


--- trunk/Source/WTF/ChangeLog	2016-09-20 15:58:37 UTC (rev 206150)
+++ trunk/Source/WTF/ChangeLog	2016-09-20 17:14:00 UTC (rev 206151)
@@ -1,3 +1,21 @@
+2016-09-20  Jonathan Bedard  <jbed...@apple.com>
+
+        Undefined behavior: Left shift negative number
+        https://bugs.webkit.org/show_bug.cgi?id=161866
+
+        Reviewed by Keith Miller.
+
+        Left shifting a negative number is undefined behavior in C/C++, although most implementations do define it. Explicitly clarifying the intended behavior due to shifting negative number in some cases.
+
+        * wtf/text/Base64.cpp:
+        (WTF::base64EncodeInternal): Changed signed character to unsigned when shifting.
+        (WTF::base64Encode): Ditto.
+        (WTF::base64URLEncode): Ditto.
+        (WTF::base64DecodeInternal): Ditto.
+        * wtf/text/Base64.h: Ditto.
+        (WTF::SignedOrUnsignedCharVectorAdapter): Rebuilt to stop using union as a bitwise_cast.
+        (WTF::ConstSignedOrUnsignedCharVectorAdapter): Ditto.
+
 2016-09-19  Daniel Bates  <daba...@apple.com>
 
         Remove ENABLE(TEXT_AUTOSIZING) automatic text size adjustment code

Modified: trunk/Source/WTF/wtf/text/Base64.cpp (206150 => 206151)


--- trunk/Source/WTF/wtf/text/Base64.cpp	2016-09-20 15:58:37 UTC (rev 206150)
+++ trunk/Source/WTF/wtf/text/Base64.cpp	2016-09-20 17:14:00 UTC (rev 206151)
@@ -92,7 +92,7 @@
     0x31, 0x32, 0x33, nonAlphabet, nonAlphabet, nonAlphabet, nonAlphabet, nonAlphabet
 };
 
-static inline void base64EncodeInternal(const char* data, unsigned len, Vector<char>& out, Base64EncodePolicy policy, const char (&encodeMap)[64])
+static inline void base64EncodeInternal(const unsigned char* data, unsigned len, Vector<char>& out, Base64EncodePolicy policy, const char (&encodeMap)[64])
 {
     out.clear();
     if (!len)
@@ -160,29 +160,29 @@
 String base64Encode(const void* data, unsigned length, Base64EncodePolicy policy)
 {
     Vector<char> result;
-    base64EncodeInternal(static_cast<const char*>(data), length, result, policy, base64EncMap);
+    base64EncodeInternal(static_cast<const unsigned char*>(data), length, result, policy, base64EncMap);
     return String(result.data(), result.size());
 }
 
 void base64Encode(const void* data, unsigned len, Vector<char>& out, Base64EncodePolicy policy)
 {
-    base64EncodeInternal(static_cast<const char*>(data), len, out, policy, base64EncMap);
+    base64EncodeInternal(static_cast<const unsigned char*>(data), len, out, policy, base64EncMap);
 }
 
 String base64URLEncode(const void* data, unsigned length)
 {
     Vector<char> result;
-    base64EncodeInternal(static_cast<const char*>(data), length, result, Base64URLPolicy, base64URLEncMap);
+    base64EncodeInternal(static_cast<const unsigned char*>(data), length, result, Base64URLPolicy, base64URLEncMap);
     return String(result.data(), result.size());
 }
 
 void base64URLEncode(const void* data, unsigned len, Vector<char>& out)
 {
-    base64EncodeInternal(static_cast<const char*>(data), len, out, Base64URLPolicy, base64URLEncMap);
+    base64EncodeInternal(static_cast<const unsigned char*>(data), len, out, Base64URLPolicy, base64URLEncMap);
 }
 
 template<typename T>
-static inline bool base64DecodeInternal(const T* data, unsigned length, Vector<char>& out, unsigned options, const char (&decodeMap)[128])
+static inline bool base64DecodeInternal(const T* data, unsigned length, SignedOrUnsignedCharVectorAdapter& out, unsigned options, const char (&decodeMap)[128])
 {
     out.clear();
     if (!length)

Modified: trunk/Source/WTF/wtf/text/Base64.h (206150 => 206151)


--- trunk/Source/WTF/wtf/text/Base64.h	2016-09-20 15:58:37 UTC (rev 206150)
+++ trunk/Source/WTF/wtf/text/Base64.h	2016-09-20 17:14:00 UTC (rev 206151)
@@ -48,13 +48,62 @@
 
 class SignedOrUnsignedCharVectorAdapter {
 public:
-    SignedOrUnsignedCharVectorAdapter(Vector<char>& vector) { m_vector.c = &vector; }
-    SignedOrUnsignedCharVectorAdapter(Vector<uint8_t>& vector) { m_vector.u = &vector; }
+    SignedOrUnsignedCharVectorAdapter(Vector<char>& vector)
+        : m_isSigned(true)
+    {
+        m_vector.c = &vector;
+    }
+    SignedOrUnsignedCharVectorAdapter(Vector<uint8_t>& vector)
+        : m_isSigned(false)
+    {
+        m_vector.u = &vector;
+    }
 
-    operator Vector<char>&() { return *m_vector.c; }
-    void clear() { m_vector.c->clear(); }
+    uint8_t* data()
+    {
+        if (m_isSigned)
+            return reinterpret_cast<uint8_t*>(m_vector.c->data());
+        return m_vector.u->data();
+    }
+    
+    size_t size() const
+    {
+        if (m_isSigned)
+            return m_vector.c->size();
+        return m_vector.u->size();
+    }
+    
+    void clear()
+    {
+        if (m_isSigned) {
+            m_vector.c->clear();
+            return;
+        }
+        m_vector.u->clear();
+    }
+    
+    void grow(size_t size)
+    {
+        if (m_isSigned) {
+            m_vector.c->grow(size);
+            return;
+        }
+        m_vector.u->grow(size);
+    }
+    
+    void shrink(size_t size)
+    {
+        if (m_isSigned) {
+            m_vector.c->shrink(size);
+            return;
+        }
+        m_vector.u->shrink(size);
+    }
+    
+    uint8_t& operator[](size_t position) { return data()[position]; }
 
 private:
+    bool m_isSigned;
     union {
         Vector<char>* c;
         Vector<uint8_t>* u;
@@ -63,14 +112,32 @@
 
 class ConstSignedOrUnsignedCharVectorAdapter {
 public:
-    ConstSignedOrUnsignedCharVectorAdapter(const Vector<char>& vector) { m_vector.c = &vector; }
-    ConstSignedOrUnsignedCharVectorAdapter(const Vector<uint8_t>& vector) { m_vector.u = &vector; }
+    ConstSignedOrUnsignedCharVectorAdapter(const Vector<char>& vector)
+        : m_isSigned(false)
+    {
+        m_vector.c = &vector;
+    }
+    ConstSignedOrUnsignedCharVectorAdapter(const Vector<uint8_t>& vector)
+        : m_isSigned(true)
+    {
+        m_vector.u = &vector;
+    }
 
-    operator const Vector<char>&() { return *m_vector.c; }
-    const char* data() const { return m_vector.c->data(); }
-    size_t size() const { return m_vector.c->size(); }
+    const uint8_t* data() const
+    {
+        if (m_isSigned)
+            return reinterpret_cast<const uint8_t*>(m_vector.c->data());
+        return m_vector.u->data();
+    }
+    size_t size() const
+    {
+        if (m_isSigned)
+            return m_vector.c->size();
+        return m_vector.u->size();
+    }
 
 private:
+    bool m_isSigned;
     union {
         const Vector<char>* c;
         const Vector<uint8_t>* u;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to