ba7a5575a46d95cfe6cac053851c5f7e9c1a6323
[WebKit-https.git] / Source / WTF / ChangeLog
1 2016-09-28  Mark Lam  <mark.lam@apple.com>
2
3         Re-enable StringView life-cycle checking.
4         https://bugs.webkit.org/show_bug.cgi?id=160384
5         <rdar://problem/28479434>
6
7         Reviewed by Saam Barati.
8
9         * wtf/text/StringView.h:
10
11 2016-09-28  Filip Pizlo  <fpizlo@apple.com>
12
13         The write barrier should be down with TSO
14         https://bugs.webkit.org/show_bug.cgi?id=162316
15
16         Reviewed by Geoffrey Garen.
17         
18         Added clearRange(), which quickly clears a range of bits. This turned out to be useful for
19         a DFG optimization pass.
20
21         * wtf/FastBitVector.cpp:
22         (WTF::FastBitVector::clearRange):
23         * wtf/FastBitVector.h:
24
25 2016-09-28  Mark Lam  <mark.lam@apple.com>
26
27         Fix race condition in StringView's UnderlyingString lifecycle management.
28         https://bugs.webkit.org/show_bug.cgi?id=162702
29
30         Reviewed by Geoffrey Garen.
31
32         There 2 relevant functions at play:
33
34         void StringView::setUnderlyingString(const StringImpl* string)
35         {
36             UnderlyingString* underlyingString;
37             if (!string)
38                 underlyingString = nullptr;
39             else {
40                 std::lock_guard<StaticLock> lock(underlyingStringsMutex);
41                 auto result = underlyingStrings().add(string, nullptr);
42                 if (result.isNewEntry)
43                     result.iterator->value = new UnderlyingString(*string);
44                 else
45                     ++result.iterator->value->refCount;
46                 underlyingString = result.iterator->value; // Point P2.
47             }
48             adoptUnderlyingString(underlyingString); // Point P5.
49         }
50
51         ... and ...
52
53         void StringView::adoptUnderlyingString(UnderlyingString* underlyingString)
54         {
55             if (m_underlyingString) {
56                 // Point P0.
57                 if (!--m_underlyingString->refCount) {
58                     if (m_underlyingString->isValid) { // Point P1.
59                         std::lock_guard<StaticLock> lock(underlyingStringsMutex);
60                         underlyingStrings().remove(&m_underlyingString->string); // Point P3.
61                     }
62                     delete m_underlyingString; // Point P4.
63                 }
64             }
65             m_underlyingString = underlyingString;
66         }
67
68         Imagine the following scenario:
69
70         1. Thread T1 has been using an UnderlyingString U1, and is now done with it.
71            T1 runs up to point P1 in adoptUnderlyingString(), and is blocked waiting for
72            the underlyingStringsMutex (which is currently being held by Thread T2).
73         2. Context switch to Thread T2.
74            T2 wants to use UnderlyingString U1, and runs up to point P2 in setUnderlyingString()
75            and releases the underlyingStringsMutex.
76            Note: T2 thinks it has successfully refCounted U1, and therefore U1 is safe to use.
77         3. Context switch to Thread T1.
78            T1 acquires the underlyingStringsMutex, and proceeds to remove it from the
79            underlyingStrings() map (see Point P3).  It thinks it has successfully done so
80            and proceeds to delete U1 (see Point P4).
81         4. Context switch to Thread T2.
82            T2 proceeds to use U1 (see Point P5 in setUnderlyingString()).
83            Note: U1 has already been freed.  This is a use after free.
84
85         The fix is to acquire the underlyingStringsMutex at Point P0 in adoptUnderlyingString()
86         instead of after P1.  This ensures that the decrementing of the UnderlyingString
87         refCount and its removal from the underlyingStrings() map is done as an atomic unit.
88
89         Note: If you look in StringView.cpp, you see another setUnderlyingString() which
90         takes a StringView otherString.  This version of setUnderlyingString() can only
91         be called from within the same thread that created the other StringView.  As a
92         result, here, we are guaranteed that the UnderlyingString refCount is never zero,
93         and there's no other threat of another thread trying to delete the UnderlyingString
94         while we adopt it.  Hence, we don't need to acquire the underlyingStringsMutex
95         here.
96
97         This race condition was found when running layout tests fetch/fetch-worker-crash.html
98         and storage/indexeddb/modern/opendatabase-versions.html when CHECK_STRINGVIEW_LIFETIME
99         is enabled.  This issue resulted in those tests crashing due to a use-after-free.
100
101         * wtf/text/StringView.cpp:
102         (WTF::StringView::adoptUnderlyingString):
103         (WTF::StringView::setUnderlyingString):
104
105 2016-09-28  Brent Fulgham  <bfulgham@apple.com>
106
107         Correct 'safeCast' implementation
108         https://bugs.webkit.org/show_bug.cgi?id=162679
109         <rdar://problem/28518189>
110
111         Reviewed by Zalan Bujtas.
112
113         * wtf/StdLibExtras.h:
114         (WTF::safeCast): Use a RELEASE_ASSERT.
115
116 2016-09-27  Don Olmstead  <don.olmstead@am.sony.com>
117
118         [CMake] Add HAVE_LOCALTIME_R definition
119         https://bugs.webkit.org/show_bug.cgi?id=162636
120
121         Reviewed by Alex Christensen.
122
123         * wtf/DateMath.cpp:
124         (WTF::getLocalTime):
125         * wtf/GregorianDateTime.cpp:
126         (WTF::GregorianDateTime::setToCurrentLocalTime):
127         * wtf/Platform.h:
128
129 2016-09-27  JF Bastien  <jfbastien@apple.com>
130
131         Speed up Heap::isMarkedConcurrently
132         https://bugs.webkit.org/show_bug.cgi?id=162095
133
134         Reviewed by Filip Pizlo.
135
136         Heap::isMarkedConcurrently had a load-load fence which is expensive on weak memory ISAs such as ARM.
137
138         This patch is fairly perf-neutral overall, but the GC's instrumentation reports:
139           GC:Eden is 93% average runtime after change
140           GC:Full is 76% average runtime after change
141
142         The fence was there because:
143          1. If the read of m_markingVersion in MarkedBlock::areMarksStale isn't what's expected then;
144          2. The read of m_marks in MarkedBlock::isMarked needs to observe the value that was stored *before* m_markingVersion was stored.
145
146         This ordering isn't guaranteed on ARM, which has a weak memory model.
147
148         There are 3 ways to guarantee this ordering:
149          A. Use a barrier instruction.
150          B. Use a load-acquire (new in ARMv8).
151          C. use ARM's address dependency rule, which C++ calls memory_order_consume.
152
153         In general:
154          A. is slow but orders all of memory in an intuitive manner.
155          B. is faster-ish and has the same property-ish.
156          C. should be faster still, but *only orders dependent loads*. This last part is critical! Consume isn't an all-out replacement for acquire (acquire is rather a superset of consume).
157
158         ARM explains the address dependency rule in their document "barrier litmus tests and cookbook":
159
160         > *Resolving by the use of barriers and address dependency*
161         >
162         > There is a rule within the ARM architecture that:
163         > Where the value returned by a read is used to compute the virtual address of a subsequent read or write (this is known as an address dependency), then these two memory accesses will be observed in program order. An address dependency exists even if the value read by the first read has no effect in changing the virtual address (as might be the case if the value returned is masked off before it is used, or if it had no effect on changing a predicted address value).
164         > This restriction applies only when the data value returned from one read is used as a data value to calculate the address of a subsequent read or write. This does not apply if the data value returned from one read is used to determine the condition code flags, and the values of the flags are used for condition code evaluation to determine the address of a subsequent reads, either through conditional execution or the evaluation of a branch. This is known as a control dependency.
165         > Where both a control and address dependency exist, the ordering behaviour is consistent with the address dependency.
166
167         C++'s memory_order_consume is unfortunately unimplemented by C++ compilers, and maybe unimplementable as spec'd. I'm working with interested folks in the committee to fix this situation: http://wg21.link/p0190r2
168
169         * wtf/Atomics.h:
170         (WTF::zeroWithConsumeDependency): a 0 which carries a dependency
171         (WTF::consumeLoad): pixie magic
172
173 2016-09-27  JF Bastien  <jfbastien@apple.com>
174
175         Atomics.h on Windows: remove seq_cst hack
176         https://bugs.webkit.org/show_bug.cgi?id=162022
177
178         Reviewed by Mark Lam.
179
180         No need to force access to seq_cst, always inlining fixes the MSVC warning.
181
182         * wtf/Atomics.h:
183         (WTF::Atomic::compareExchangeWeak): remove seq_cst hack
184         (WTF::Atomic::compareExchangeStrong): remove seq_cst hack
185         (WTF::Atomic::exchangeAndAdd): remove seq_cst hack
186         (WTF::Atomic::exchange): remove seq_cst hack
187
188 2016-09-27  Don Olmstead  <don.olmstead@am.sony.com>
189
190         [CMake] Use CMake to determine HAVE_* defines
191         https://bugs.webkit.org/show_bug.cgi?id=162368
192
193         Reviewed by Alex Christensen.
194
195         * wtf/Platform.h:
196
197 2016-09-20  Anders Carlsson  <andersca@apple.com>
198
199         PlatformEvent::m_modifiers should be an OptionSet
200         https://bugs.webkit.org/show_bug.cgi?id=162326
201
202         Reviewed by Daniel Bates.
203
204         * wtf/OptionSet.h:
205         (WTF::OptionSet::operator!=):
206         (WTF::OptionSet::operator-):
207
208 2016-09-27  Jer Noble  <jer.noble@apple.com>
209
210         Remove deprecated ENCRYPTED_MEDIA implementation.
211         https://bugs.webkit.org/show_bug.cgi?id=161010
212
213         Reviewed by Eric Carlson.
214
215         Remove ENABLE_ENCRYPTED_MEDIA.
216
217         * wtf/FeatureDefines.h:
218
219 2016-09-27  Youenn Fablet  <youenn@apple.com>
220
221         [Fetch API] Use Ref<const T> in FetchBody::m_data variant
222         https://bugs.webkit.org/show_bug.cgi?id=162599
223
224         Reviewed by Alex Christensen.
225
226         Enabling to use DeferrableRefCounted<const T> by making m_refCount mutable.
227
228         * wtf/DeferrableRefCounted.h:
229         (WTF::DeferrableRefCountedBase::ref):
230         (WTF::DeferrableRefCountedBase::derefBase):
231         (WTF::DeferrableRefCounted::deref):
232
233 2016-09-26  Daniel Bates  <dabates@apple.com>
234
235         Rename IOS_TEXT_AUTOSIZING to TEXT_AUTOSIZING
236         https://bugs.webkit.org/show_bug.cgi?id=162365
237
238         Reviewed by Simon Fraser.
239
240         * wtf/FeatureDefines.h:
241
242 2016-09-26  Benjamin Poulain  <benjamin@webkit.org>
243
244         [JSC] Shrink the Math inline caches some more
245         https://bugs.webkit.org/show_bug.cgi?id=162485
246
247         Reviewed by Saam Barati.
248
249         * wtf/Bag.h:
250         Don't copy the arguments before initializing the nodes.
251
252 2016-09-26  Michael Catanzaro  <mcatanzaro@igalia.com>
253
254         std::unique_ptr deleter functions should not check if pointer is null
255         https://bugs.webkit.org/show_bug.cgi?id=162558
256
257         Reviewed by Alex Christensen.
258
259         std::unique_ptr already does this before calling the deleter.
260
261         * wtf/efl/UniquePtrEfl.h:
262         * wtf/glib/GUniquePtr.h:
263
264 == Rolled over to ChangeLog-2016-09-26 ==