Undefined behavior: Left shift negative number
authorjbedard@apple.com <jbedard@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 20 Sep 2016 17:14:00 +0000 (17:14 +0000)
committerjbedard@apple.com <jbedard@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 20 Sep 2016 17:14:00 +0000 (17:14 +0000)
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/JavaScriptCore:

* 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.

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGAbstractHeap.h
Source/WTF/ChangeLog
Source/WTF/wtf/text/Base64.cpp
Source/WTF/wtf/text/Base64.h

index a2ae0fb..059e8bf 100644 (file)
@@ -1,3 +1,15 @@
+2016-09-20  Jonathan Bedard  <jbedard@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  <sbarati@apple.com>
 
         Unreviewed fix for 32-bit DFG x86 implementation of HasOwnProperty.
index afe10c4..5d3b202 100644 (file)
@@ -298,7 +298,7 @@ private:
     {
         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:
index c14656e..46c9a67 100644 (file)
@@ -1,3 +1,21 @@
+2016-09-20  Jonathan Bedard  <jbedard@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  <dabates@apple.com>
 
         Remove ENABLE(TEXT_AUTOSIZING) automatic text size adjustment code
index ffd6faf..714a7ea 100644 (file)
@@ -92,7 +92,7 @@ static const char base64URLDecMap[128] = {
     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 @@ static inline void base64EncodeInternal(const char* data, unsigned len, Vector<c
 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)
index e88ba8a..8205575 100644 (file)
@@ -48,13 +48,62 @@ enum Base64DecodeOptions {
 
 class SignedOrUnsignedCharVectorAdapter {
 public:
-    SignedOrUnsignedCharVectorAdapter(Vector<char>& vector) { m_vector.c = &vector; }
-    SignedOrUnsignedCharVectorAdapter(Vector<uint8_t>& vector) { m_vector.u = &vector; }
-
-    operator Vector<char>&() { return *m_vector.c; }
-    void clear() { m_vector.c->clear(); }
+    SignedOrUnsignedCharVectorAdapter(Vector<char>& vector)
+        : m_isSigned(true)
+    {
+        m_vector.c = &vector;
+    }
+    SignedOrUnsignedCharVectorAdapter(Vector<uint8_t>& vector)
+        : m_isSigned(false)
+    {
+        m_vector.u = &vector;
+    }
+
+    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 @@ private:
 
 class ConstSignedOrUnsignedCharVectorAdapter {
 public:
-    ConstSignedOrUnsignedCharVectorAdapter(const Vector<char>& vector) { m_vector.c = &vector; }
-    ConstSignedOrUnsignedCharVectorAdapter(const Vector<uint8_t>& vector) { 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(); }
+    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;
+    }
+
+    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;