Skip to content

Commit 96c1015

Browse files
author
Filip Pizlo
committed
All of the different ArrayBuffer::data's should be CagedPtr<>
https://bugs.webkit.org/show_bug.cgi?id=175515 Reviewed by Michael Saboff. Source/JavaScriptCore: This straightforwardly implements what the title says. * runtime/ArrayBuffer.cpp: (JSC::SharedArrayBufferContents::~SharedArrayBufferContents): (JSC::ArrayBufferContents::destroy): (JSC::ArrayBufferContents::tryAllocate): (JSC::ArrayBufferContents::makeShared): (JSC::ArrayBufferContents::copyTo): (JSC::ArrayBuffer::createFromBytes): (JSC::ArrayBuffer::transferTo): * runtime/ArrayBuffer.h: (JSC::SharedArrayBufferContents::data const): (JSC::ArrayBufferContents::data const): (JSC::ArrayBuffer::data): (JSC::ArrayBuffer::data const): * runtime/ArrayBufferView.h: (JSC::ArrayBufferView::baseAddress const): * runtime/CagedBarrierPtr.h: Added a specialization so that CagedBarrierPtr<Gigacage::Foo, void> is valid. * runtime/DataView.h: (JSC::DataView::get): (JSC::DataView::set): * runtime/JSArrayBufferView.cpp: (JSC::JSArrayBufferView::ConstructionContext::ConstructionContext): * runtime/JSArrayBufferView.h: (JSC::JSArrayBufferView::ConstructionContext::vector const): (JSC::JSArrayBufferView::vector const): * runtime/JSGenericTypedArrayViewInlines.h: (JSC::JSGenericTypedArrayView<Adaptor>::visitChildren): Source/WTF: Added a specialization so that CagedPtr<void> is valid. * wtf/CagedPtr.h: Canonical link: https://commits.webkit.org/192841@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@221439 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 0bef02b commit 96c1015

12 files changed

Lines changed: 165 additions & 38 deletions

JSTests/stress/dont-reserve-huge-capacity-lexer.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
//@ skip if ($architecture != "x86-64") or $memoryLimited
1+
//@ if ($architecture != "x86-64") or $memoryLimited then skip else runDefault end
22

33
var fe="f";
44
try

Source/JavaScriptCore/ChangeLog

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,39 @@
1+
2017-08-31 Filip Pizlo <[email protected]>
2+
3+
All of the different ArrayBuffer::data's should be CagedPtr<>
4+
https://bugs.webkit.org/show_bug.cgi?id=175515
5+
6+
Reviewed by Michael Saboff.
7+
8+
This straightforwardly implements what the title says.
9+
10+
* runtime/ArrayBuffer.cpp:
11+
(JSC::SharedArrayBufferContents::~SharedArrayBufferContents):
12+
(JSC::ArrayBufferContents::destroy):
13+
(JSC::ArrayBufferContents::tryAllocate):
14+
(JSC::ArrayBufferContents::makeShared):
15+
(JSC::ArrayBufferContents::copyTo):
16+
(JSC::ArrayBuffer::createFromBytes):
17+
(JSC::ArrayBuffer::transferTo):
18+
* runtime/ArrayBuffer.h:
19+
(JSC::SharedArrayBufferContents::data const):
20+
(JSC::ArrayBufferContents::data const):
21+
(JSC::ArrayBuffer::data):
22+
(JSC::ArrayBuffer::data const):
23+
* runtime/ArrayBufferView.h:
24+
(JSC::ArrayBufferView::baseAddress const):
25+
* runtime/CagedBarrierPtr.h: Added a specialization so that CagedBarrierPtr<Gigacage::Foo, void> is valid.
26+
* runtime/DataView.h:
27+
(JSC::DataView::get):
28+
(JSC::DataView::set):
29+
* runtime/JSArrayBufferView.cpp:
30+
(JSC::JSArrayBufferView::ConstructionContext::ConstructionContext):
31+
* runtime/JSArrayBufferView.h:
32+
(JSC::JSArrayBufferView::ConstructionContext::vector const):
33+
(JSC::JSArrayBufferView::vector const):
34+
* runtime/JSGenericTypedArrayViewInlines.h:
35+
(JSC::JSGenericTypedArrayView<Adaptor>::visitChildren):
36+
137
2017-08-22 Filip Pizlo <[email protected]>
238

339
Strings need to be in some kind of gigacage

Source/JavaScriptCore/runtime/ArrayBuffer.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ SharedArrayBufferContents::SharedArrayBufferContents(void* data, ArrayBufferDest
4141

4242
SharedArrayBufferContents::~SharedArrayBufferContents()
4343
{
44-
m_destructor(m_data);
44+
m_destructor(m_data.getMayBeNull());
4545
}
4646

4747
ArrayBufferContents::ArrayBufferContents()
@@ -81,7 +81,7 @@ void ArrayBufferContents::clear()
8181

8282
void ArrayBufferContents::destroy()
8383
{
84-
m_destructor(m_data);
84+
m_destructor(m_data.getMayBeNull());
8585
}
8686

8787
void ArrayBufferContents::reset()
@@ -113,15 +113,15 @@ void ArrayBufferContents::tryAllocate(unsigned numElements, unsigned elementByte
113113
}
114114

115115
if (policy == ZeroInitialize)
116-
memset(m_data, 0, size);
116+
memset(m_data.get(), 0, size);
117117

118118
m_sizeInBytes = numElements * elementByteSize;
119119
m_destructor = [] (void* p) { Gigacage::free(Gigacage::Primitive, p); };
120120
}
121121

122122
void ArrayBufferContents::makeShared()
123123
{
124-
m_shared = adoptRef(new SharedArrayBufferContents(m_data, WTFMove(m_destructor)));
124+
m_shared = adoptRef(new SharedArrayBufferContents(m_data.getMayBeNull(), WTFMove(m_destructor)));
125125
m_destructor = [] (void*) { };
126126
}
127127

@@ -141,7 +141,7 @@ void ArrayBufferContents::copyTo(ArrayBufferContents& other)
141141
other.tryAllocate(m_sizeInBytes, sizeof(char), ArrayBufferContents::DontInitialize);
142142
if (!other.m_data)
143143
return;
144-
memcpy(other.m_data, m_data, m_sizeInBytes);
144+
memcpy(other.m_data.get(), m_data.get(), m_sizeInBytes);
145145
other.m_sizeInBytes = m_sizeInBytes;
146146
}
147147

@@ -198,7 +198,7 @@ Ref<ArrayBuffer> ArrayBuffer::createAdopted(const void* data, unsigned byteLengt
198198
// - WebAssembly. Wasm should allocate from the cage.
199199
Ref<ArrayBuffer> ArrayBuffer::createFromBytes(const void* data, unsigned byteLength, ArrayBufferDestructorFunction&& destructor)
200200
{
201-
if (data && byteLength && !Gigacage::isCaged(Gigacage::Primitive, data))
201+
if (data && !Gigacage::isCaged(Gigacage::Primitive, data))
202202
Gigacage::disablePrimitiveGigacage();
203203

204204
ArrayBufferContents contents(const_cast<void*>(data), byteLength, WTFMove(destructor));
@@ -322,7 +322,7 @@ bool ArrayBuffer::transferTo(VM& vm, ArrayBufferContents& result)
322322
Ref<ArrayBuffer> protect(*this);
323323

324324
if (!m_contents.m_data) {
325-
result.m_data = 0;
325+
result.m_data = nullptr;
326326
return false;
327327
}
328328

Source/JavaScriptCore/runtime/ArrayBuffer.h

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2009, 2013, 2016 Apple Inc. All rights reserved.
2+
* Copyright (C) 2009-2017 Apple Inc. All rights reserved.
33
*
44
* Redistribution and use in source and binary forms, with or without
55
* modification, are permitted provided that the following conditions
@@ -28,6 +28,7 @@
2828
#include "ArrayBufferSharingMode.h"
2929
#include "GCIncomingRefCounted.h"
3030
#include "Weak.h"
31+
#include <wtf/CagedPtr.h>
3132
#include <wtf/Function.h>
3233
#include <wtf/StdLibExtras.h>
3334
#include <wtf/ThreadSafeRefCounted.h>
@@ -47,12 +48,10 @@ class SharedArrayBufferContents : public ThreadSafeRefCounted<SharedArrayBufferC
4748
SharedArrayBufferContents(void* data, ArrayBufferDestructorFunction&&);
4849
~SharedArrayBufferContents();
4950

50-
void* data() const { return m_data; }
51+
void* data() const { return m_data.getMayBeNull(); }
5152

5253
private:
53-
// FIXME: This should be CagedPtr<>.
54-
// https://bugs.webkit.org/show_bug.cgi?id=175515
55-
void* m_data;
54+
CagedPtr<Gigacage::Primitive, void> m_data;
5655
ArrayBufferDestructorFunction m_destructor;
5756
};
5857

@@ -70,7 +69,7 @@ class ArrayBufferContents {
7069

7170
explicit operator bool() { return !!m_data; }
7271

73-
void* data() const { return m_data; }
72+
void* data() const { return m_data.getMayBeNull(); }
7473
unsigned sizeInBytes() const { return m_sizeInBytes; }
7574

7675
bool isShared() const { return m_shared; }
@@ -97,9 +96,7 @@ class ArrayBufferContents {
9796

9897
ArrayBufferDestructorFunction m_destructor;
9998
RefPtr<SharedArrayBufferContents> m_shared;
100-
// FIXME: This should be CagedPtr<>.
101-
// https://bugs.webkit.org/show_bug.cgi?id=175515
102-
void* m_data;
99+
CagedPtr<Gigacage::Primitive, void> m_data;
103100
unsigned m_sizeInBytes;
104101
};
105102

@@ -185,12 +182,12 @@ int ArrayBuffer::clampValue(int x, int left, int right)
185182

186183
void* ArrayBuffer::data()
187184
{
188-
return m_contents.m_data;
185+
return m_contents.m_data.getMayBeNull();
189186
}
190187

191188
const void* ArrayBuffer::data() const
192189
{
193-
return m_contents.m_data;
190+
return m_contents.m_data.getMayBeNull();
194191
}
195192

196193
unsigned ArrayBuffer::byteLength() const

Source/JavaScriptCore/runtime/ArrayBufferView.h

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2009, 2013, 2016 Apple Inc. All rights reserved.
2+
* Copyright (C) 2009-2017 Apple Inc. All rights reserved.
33
*
44
* Redistribution and use in source and binary forms, with or without
55
* modification, are permitted provided that the following conditions
@@ -72,7 +72,7 @@ class ArrayBufferView : public RefCounted<ArrayBufferView> {
7272
{
7373
if (isNeutered())
7474
return 0;
75-
return m_baseAddress;
75+
return m_baseAddress.getMayBeNull();
7676
}
7777

7878
void* data() const { return baseAddress(); }
@@ -147,9 +147,7 @@ class ArrayBufferView : public RefCounted<ArrayBufferView> {
147147
}
148148

149149
// This is the address of the ArrayBuffer's storage, plus the byte offset.
150-
// FIXME: This should be CagedPtr<>.
151-
// https://bugs.webkit.org/show_bug.cgi?id=175515
152-
void* m_baseAddress;
150+
CagedPtr<Gigacage::Primitive, void> m_baseAddress;
153151

154152
unsigned m_byteOffset : 31;
155153
bool m_isNeuterable : 1;

Source/JavaScriptCore/runtime/CagedBarrierPtr.h

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,4 +88,51 @@ class CagedBarrierPtr {
8888
AuxiliaryBarrier<CagedPtr<kind, T>> m_barrier;
8989
};
9090

91+
template<Gigacage::Kind passedKind>
92+
class CagedBarrierPtr<passedKind, void> {
93+
public:
94+
static constexpr Gigacage::Kind kind = passedKind;
95+
typedef void Type;
96+
97+
CagedBarrierPtr() { }
98+
99+
template<typename U>
100+
CagedBarrierPtr(VM& vm, JSCell* cell, U&& value)
101+
{
102+
m_barrier.set(vm, cell, std::forward<U>(value));
103+
}
104+
105+
void clear() { m_barrier.clear(); }
106+
107+
template<typename U>
108+
void set(VM& vm, JSCell* cell, U&& value)
109+
{
110+
m_barrier.set(vm, cell, std::forward<U>(value));
111+
}
112+
113+
void* get() const { return m_barrier.get().get(); }
114+
void* getMayBeNull() const { return m_barrier.get().getMayBeNull(); }
115+
116+
bool operator==(const CagedBarrierPtr& other) const
117+
{
118+
return getMayBeNull() == other.getMayBeNull();
119+
}
120+
121+
bool operator!=(const CagedBarrierPtr& other) const
122+
{
123+
return !(*this == other);
124+
}
125+
126+
explicit operator bool() const
127+
{
128+
return *this != CagedBarrierPtr();
129+
}
130+
131+
template<typename U>
132+
void setWithoutBarrier(U&& value) { m_barrier.setWithoutBarrier(std::forward<U>(value)); }
133+
134+
private:
135+
AuxiliaryBarrier<CagedPtr<kind, void>> m_barrier;
136+
};
137+
91138
} // namespace JSC

Source/JavaScriptCore/runtime/DataView.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2013 Apple Inc. All rights reserved.
2+
* Copyright (C) 2013-2017 Apple Inc. All rights reserved.
33
*
44
* Redistribution and use in source and binary forms, with or without
55
* modification, are permitted provided that the following conditions
@@ -62,7 +62,7 @@ class DataView : public ArrayBufferView {
6262
} else
6363
ASSERT_WITH_SECURITY_IMPLICATION(offset + sizeof(T) <= byteLength());
6464
return flipBytesIfLittleEndian(
65-
*reinterpret_cast<T*>(static_cast<uint8_t*>(m_baseAddress) + offset),
65+
*reinterpret_cast<T*>(static_cast<uint8_t*>(m_baseAddress.get()) + offset),
6666
littleEndian);
6767
}
6868

@@ -86,7 +86,7 @@ class DataView : public ArrayBufferView {
8686
*status = true;
8787
} else
8888
ASSERT_WITH_SECURITY_IMPLICATION(offset + sizeof(T) <= byteLength());
89-
*reinterpret_cast<T*>(static_cast<uint8_t*>(m_baseAddress) + offset) =
89+
*reinterpret_cast<T*>(static_cast<uint8_t*>(m_baseAddress.get()) + offset) =
9090
flipBytesIfLittleEndian(value, littleEndian);
9191
}
9292

Source/JavaScriptCore/runtime/JSArrayBufferView.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ JSArrayBufferView::ConstructionContext::ConstructionContext(
7777
m_mode = FastTypedArray;
7878

7979
if (mode == ZeroFill) {
80-
uint64_t* asWords = static_cast<uint64_t*>(m_vector);
80+
uint64_t* asWords = static_cast<uint64_t*>(m_vector.get());
8181
for (unsigned i = size / sizeof(uint64_t); i--;)
8282
asWords[i] = 0;
8383
}
@@ -94,7 +94,7 @@ JSArrayBufferView::ConstructionContext::ConstructionContext(
9494
if (!m_vector)
9595
return;
9696
if (mode == ZeroFill)
97-
memset(m_vector, 0, size);
97+
memset(m_vector.get(), 0, size);
9898

9999
vm.heap.reportExtraMemoryAllocated(static_cast<size_t>(length) * elementSize);
100100

Source/JavaScriptCore/runtime/JSArrayBufferView.h

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -133,16 +133,14 @@ class JSArrayBufferView : public JSNonFinalObject {
133133
bool operator!() const { return !m_structure; }
134134

135135
Structure* structure() const { return m_structure; }
136-
void* vector() const { return m_vector; }
136+
void* vector() const { return m_vector.getMayBeNull(); }
137137
uint32_t length() const { return m_length; }
138138
TypedArrayMode mode() const { return m_mode; }
139139
Butterfly* butterfly() const { return m_butterfly; }
140140

141141
private:
142142
Structure* m_structure;
143-
// FIXME: This should be CagedPtr<>.
144-
// https://bugs.webkit.org/show_bug.cgi?id=175515
145-
void* m_vector;
143+
CagedPtr<Gigacage::Primitive, void> m_vector;
146144
uint32_t m_length;
147145
TypedArrayMode m_mode;
148146
Butterfly* m_butterfly;
@@ -169,7 +167,7 @@ class JSArrayBufferView : public JSNonFinalObject {
169167
bool isNeutered() { return hasArrayBuffer() && !vector(); }
170168
void neuter();
171169

172-
void* vector() const { return m_vector.get(); }
170+
void* vector() const { return m_vector.getMayBeNull(); }
173171

174172
unsigned byteOffset();
175173
unsigned length() const { return m_length; }
@@ -192,9 +190,7 @@ class JSArrayBufferView : public JSNonFinalObject {
192190

193191
static String toStringName(const JSObject*, ExecState*);
194192

195-
// FIXME: This should be CagedBarrierPtr<>.
196-
// https://bugs.webkit.org/show_bug.cgi?id=175515
197-
AuxiliaryBarrier<void*> m_vector;
193+
CagedBarrierPtr<Gigacage::Primitive, void> m_vector;
198194
uint32_t m_length;
199195
TypedArrayMode m_mode;
200196
};

Source/JavaScriptCore/runtime/JSGenericTypedArrayViewInlines.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -517,7 +517,7 @@ void JSGenericTypedArrayView<Adaptor>::visitChildren(JSCell* cell, SlotVisitor&
517517

518518
switch (thisObject->m_mode) {
519519
case FastTypedArray: {
520-
if (void* vector = thisObject->m_vector.get())
520+
if (void* vector = thisObject->m_vector.getMayBeNull())
521521
visitor.markAuxiliary(vector);
522522
break;
523523
}

0 commit comments

Comments
 (0)