Changeset 222058 in webkit for trunk/Source/JavaScriptCore/assembler/ProbeContext.h

Timestamp:

Sep 14, 2017, 4:08:54 PM (8 years ago)

Author:

[email protected]

Message:

AddressSanitizer: stack-buffer-underflow in JSC::Probe::Page::Page
​https://bugs.webkit.org/show_bug.cgi?id=176874
<rdar://problem/34436415>

Reviewed by Saam Barati.

Source/JavaScriptCore:

1. Make Probe::Stack play nice with ASan by:

1. using a local memcpy implementation that suppresses ASan on ASan builds. We don't want to use std:memcpy() which validates stack memory because we are intentionally copying stack memory beyond the current frame.

2. changing Stack::s_chunkSize to equal sizeof(uintptr_t) on ASan builds. This ensures that Page::flushWrites() only writes stack memory that was modified by a probe. The probes should only modify stack memory that belongs to JSC stack data structures. We don't want to inadvertently modify adjacent words that may belong to ASan (which may happen if s_chunkSize is larger than sizeof(uintptr_t)).

3. fixing a bug in Page dirtyBits management for when the size of the value to write is greater than s_chunkSize. The fix in generic, but in practice, this currently only manifests on 32-bit ASan builds because sizeof(uintptr_t) and s_chunkSize are 32-bit, and we may write 64-bit values.

4. making Page::m_dirtyBits 64 bits always. This maximizes the number of s_chunksPerPage we can have even on ASan builds.

2. Fixed the bottom most Probe::Context and Probe::Stack get/set methods to use std::memcpy to avoid strict aliasing issues.

3. Optimized the implementation of Page::physicalAddressFor().

4. Optimized the implementation of Stack::set() in the recording of the low watermark. We just record the lowest raw pointer now, and only compute the alignment to its chuck boundary later when the low watermark is requested.

5. Changed a value in testmasm to make the test less vulnerable to rounding issues.

No new test needed because this is already covered by testmasm with ASan enabled.

• assembler/ProbeContext.h:

(JSC::Probe::CPUState::gpr const):
(JSC::Probe::CPUState::spr const):
(JSC::Probe::Context::gpr):
(JSC::Probe::Context::spr):
(JSC::Probe::Context::fpr):
(JSC::Probe::Context::gprName):
(JSC::Probe::Context::sprName):
(JSC::Probe::Context::fprName):
(JSC::Probe::Context::gpr const):
(JSC::Probe::Context::spr const):
(JSC::Probe::Context::fpr const):
(JSC::Probe::Context::pc):
(JSC::Probe::Context::fp):
(JSC::Probe::Context::sp):
(JSC::Probe:: const): Deleted.

• assembler/ProbeStack.cpp:

(JSC::Probe::copyStackPage):
(JSC::Probe::Page::Page):
(JSC::Probe::Page::flushWrites):

• assembler/ProbeStack.h:

(JSC::Probe::Page::get):
(JSC::Probe::Page::set):
(JSC::Probe::Page::dirtyBitFor):
(JSC::Probe::Page::physicalAddressFor):
(JSC::Probe::Stack::lowWatermark):
(JSC::Probe::Stack::get):
(JSC::Probe::Stack::set):

• assembler/testmasm.cpp:

(JSC::testProbeModifiesStackValues):

Source/WTF:

Added a convenience version of roundUpToMultipleOf() so that it can be applied to
pointers without the client having to cast explicitly.

• wtf/StdLibExtras.h:

(WTF::roundUpToMultipleOf):

File:

• trunk/Source/JavaScriptCore/assembler/ProbeContext.h (modified) (3 diffs)

trunk/Source/JavaScriptCore/assembler/ProbeContext.h

@@ -46 +46 @@
     inline double& fpr(FPRegisterID);
 
-    template<typename T, typename std::enable_if<std::is_integral<T>::value>::type* = nullptr>
-    T gpr(RegisterID) const;
-    template<typename T, typename std::enable_if<std::is_pointer<T>::value>::type* = nullptr>
-    T gpr(RegisterID) const;
-    template<typename T, typename std::enable_if<std::is_integral<T>::value>::type* = nullptr>
-    T spr(SPRegisterID) const;
-    template<typename T, typename std::enable_if<std::is_pointer<T>::value>::type* = nullptr>
-    T spr(SPRegisterID) const;
+    template<typename T> T gpr(RegisterID) const;
+    template<typename T> T spr(SPRegisterID) const;
     template<typename T> T fpr(FPRegisterID) const;
 
@@ -86 +80 @@
 }
 
-template<typename T, typename std::enable_if<std::is_integral<T>::value>::type*>
+template<typename T>
 T CPUState::gpr(RegisterID id) const
 {
     CPUState* cpu = const_cast<CPUState*>(this);
-    return static_cast<T>(cpu->gpr(id));
-}
-
-template<typename T, typename std::enable_if<std::is_pointer<T>::value>::type*>
-T CPUState::gpr(RegisterID id) const
-{
-    CPUState* cpu = const_cast<CPUState*>(this);
-    return reinterpret_cast<T>(cpu->gpr(id));
-}
-
-template<typename T, typename std::enable_if<std::is_integral<T>::value>::type*>
+    auto& from = cpu->gpr(id);
+    typename std::remove_const<T>::type to { };
+    std::memcpy(&to, &from, sizeof(to)); // Use std::memcpy to avoid strict aliasing issues.
+    return to;
+}
+
+template<typename T>
 T CPUState::spr(SPRegisterID id) const
 {
     CPUState* cpu = const_cast<CPUState*>(this);
-    return static_cast<T>(cpu->spr(id));
-}
-
-template<typename T, typename std::enable_if<std::is_pointer<T>::value>::type*>
-T CPUState::spr(SPRegisterID id) const
-{
-    CPUState* cpu = const_cast<CPUState*>(this);
-    return reinterpret_cast<T>(cpu->spr(id));
+    auto& from = cpu->spr(id);
+    typename std::remove_const<T>::type to { };
+    std::memcpy(&to, &from, sizeof(to)); // Use std::memcpy to avoid strict aliasing issues.
+    return to;
 }
 
@@ -211 +197 @@
     { }
 
-    uintptr_t& gpr(RegisterID id) { return m_state->cpu.gpr(id); }
-    uintptr_t& spr(SPRegisterID id) { return m_state->cpu.spr(id); }
-    double& fpr(FPRegisterID id) { return m_state->cpu.fpr(id); }
-    const char* gprName(RegisterID id) { return m_state->cpu.gprName(id); }
-    const char* sprName(SPRegisterID id) { return m_state->cpu.sprName(id); }
-    const char* fprName(FPRegisterID id) { return m_state->cpu.fprName(id); }
-
-    void*& pc() { return m_state->cpu.pc(); }
-    void*& fp() { return m_state->cpu.fp(); }
-    void*& sp() { return m_state->cpu.sp(); }
-
-    template<typename T> T pc() { return m_state->cpu.pc<T>(); }
-    template<typename T> T fp() { return m_state->cpu.fp<T>(); }
-    template<typename T> T sp() { return m_state->cpu.sp<T>(); }
+    uintptr_t& gpr(RegisterID id) { return cpu.gpr(id); }
+    uintptr_t& spr(SPRegisterID id) { return cpu.spr(id); }
+    double& fpr(FPRegisterID id) { return cpu.fpr(id); }
+    const char* gprName(RegisterID id) { return cpu.gprName(id); }
+    const char* sprName(SPRegisterID id) { return cpu.sprName(id); }
+    const char* fprName(FPRegisterID id) { return cpu.fprName(id); }
+
+    template<typename T> T gpr(RegisterID id) const { return cpu.gpr<T>(id); }
+    template<typename T> T spr(SPRegisterID id) const { return cpu.spr<T>(id); }
+    template<typename T> T fpr(FPRegisterID id) const { return cpu.fpr<T>(id); }
+
+    void*& pc() { return cpu.pc(); }
+    void*& fp() { return cpu.fp(); }
+    void*& sp() { return cpu.sp(); }
+
+    template<typename T> T pc() { return cpu.pc<T>(); }
+    template<typename T> T fp() { return cpu.fp<T>(); }
+    template<typename T> T sp() { return cpu.sp<T>(); }
 
     Stack& stack()