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