Skip to content

Commit d60309d

Browse files
author
Filip Pizlo
committed
Put g_gigacageBasePtr into its own page and make it read-only
https://bugs.webkit.org/show_bug.cgi?id=174972 Reviewed by Michael Saboff. Source/bmalloc: This puts the gigacage base pointers into their own page and makes that page read-only. * bmalloc/Gigacage.cpp: (Gigacage::ensureGigacage): (Gigacage::disablePrimitiveGigacage): (Gigacage::addPrimitiveDisableCallback): * bmalloc/Gigacage.h: (Gigacage::basePtr): (Gigacage::basePtrs): Source/JavaScriptCore: C++ code doesn't have to know about this change. That includes C++ code that generates JIT code. But the offline assembler now needs to know about how to load from offsets of global variables. This turned out to be easy to support by extending the existing expression support. * llint/LowLevelInterpreter64.asm: * offlineasm/ast.rb: * offlineasm/parser.rb: * offlineasm/transform.rb: * offlineasm/x86.rb: Canonical link: https://commits.webkit.org/193838@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@222549 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 71f4678 commit d60309d

9 files changed

Lines changed: 120 additions & 29 deletions

File tree

‎Source/JavaScriptCore/ChangeLog‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,21 @@
1+
2017-09-26 Filip Pizlo <fpizlo@apple.com>
2+
3+
Put g_gigacageBasePtr into its own page and make it read-only
4+
https://bugs.webkit.org/show_bug.cgi?id=174972
5+
6+
Reviewed by Michael Saboff.
7+
8+
C++ code doesn't have to know about this change. That includes C++ code that generates JIT code.
9+
10+
But the offline assembler now needs to know about how to load from offsets of global variables.
11+
This turned out to be easy to support by extending the existing expression support.
12+
13+
* llint/LowLevelInterpreter64.asm:
14+
* offlineasm/ast.rb:
15+
* offlineasm/parser.rb:
16+
* offlineasm/transform.rb:
17+
* offlineasm/x86.rb:
18+
119
2017-09-26 Commit Queue <commit-queue@webkit.org>
220

321
Unreviewed, rolling out r222518.

‎Source/JavaScriptCore/llint/LowLevelInterpreter64.asm‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1209,7 +1209,7 @@ _llint_op_is_object:
12091209

12101210
macro loadPropertyAtVariableOffset(propertyOffsetAsInt, objectAndStorage, value)
12111211
bilt propertyOffsetAsInt, firstOutOfLineOffset, .isInline
1212-
loadCaged(_g_jsValueGigacageBasePtr, constexpr JSVALUE_GIGACAGE_MASK, JSObject::m_butterfly[objectAndStorage], objectAndStorage, value)
1212+
loadCaged(_g_gigacageBasePtrs + Gigacage::BasePtrs::jsValue, constexpr JSVALUE_GIGACAGE_MASK, JSObject::m_butterfly[objectAndStorage], objectAndStorage, value)
12131213
negi propertyOffsetAsInt
12141214
sxi2q propertyOffsetAsInt, propertyOffsetAsInt
12151215
jmp .ready
@@ -1222,7 +1222,7 @@ end
12221222

12231223
macro storePropertyAtVariableOffset(propertyOffsetAsInt, objectAndStorage, value, scratch)
12241224
bilt propertyOffsetAsInt, firstOutOfLineOffset, .isInline
1225-
loadCaged(_g_jsValueGigacageBasePtr, constexpr JSVALUE_GIGACAGE_MASK, JSObject::m_butterfly[objectAndStorage], objectAndStorage, scratch)
1225+
loadCaged(_g_gigacageBasePtrs + Gigacage::BasePtrs::jsValue, constexpr JSVALUE_GIGACAGE_MASK, JSObject::m_butterfly[objectAndStorage], objectAndStorage, scratch)
12261226
negi propertyOffsetAsInt
12271227
sxi2q propertyOffsetAsInt, propertyOffsetAsInt
12281228
jmp .ready
@@ -1298,7 +1298,7 @@ _llint_op_get_array_length:
12981298
btiz t2, IsArray, .opGetArrayLengthSlow
12991299
btiz t2, IndexingShapeMask, .opGetArrayLengthSlow
13001300
loadisFromInstruction(1, t1)
1301-
loadCaged(_g_jsValueGigacageBasePtr, constexpr JSVALUE_GIGACAGE_MASK, JSObject::m_butterfly[t3], t0, t2)
1301+
loadCaged(_g_gigacageBasePtrs + Gigacage::BasePtrs::jsValue, constexpr JSVALUE_GIGACAGE_MASK, JSObject::m_butterfly[t3], t0, t2)
13021302
loadi -sizeof IndexingHeader + IndexingHeader::u.lengths.publicLength[t0], t0
13031303
bilt t0, 0, .opGetArrayLengthSlow
13041304
orq tagTypeNumber, t0
@@ -1481,7 +1481,7 @@ _llint_op_get_by_val:
14811481
loadisFromInstruction(3, t3)
14821482
loadConstantOrVariableInt32(t3, t1, .opGetByValSlow)
14831483
sxi2q t1, t1
1484-
loadCaged(_g_jsValueGigacageBasePtr, constexpr JSVALUE_GIGACAGE_MASK, JSObject::m_butterfly[t0], t3, t5)
1484+
loadCaged(_g_gigacageBasePtrs + Gigacage::BasePtrs::jsValue, constexpr JSVALUE_GIGACAGE_MASK, JSObject::m_butterfly[t0], t3, t5)
14851485
andi IndexingShapeMask, t2
14861486
bieq t2, Int32Shape, .opGetByValIsContiguous
14871487
bineq t2, ContiguousShape, .opGetByValNotContiguous
@@ -1528,7 +1528,7 @@ _llint_op_get_by_val:
15281528
bia t2, LastArrayType - FirstArrayType, .opGetByValSlow
15291529

15301530
# Sweet, now we know that we have a typed array. Do some basic things now.
1531-
loadCaged(_g_primitiveGigacageBasePtr, constexpr PRIMITIVE_GIGACAGE_MASK, JSArrayBufferView::m_vector[t0], t3, t5)
1531+
loadCaged(_g_gigacageBasePtrs + Gigacage::BasePtrs::primitive, constexpr PRIMITIVE_GIGACAGE_MASK, JSArrayBufferView::m_vector[t0], t3, t5)
15321532
biaeq t1, JSArrayBufferView::m_length[t0], .opGetByValSlow
15331533

15341534
# Now bisect through the various types. Note that we can treat Uint8ArrayType and
@@ -1619,7 +1619,7 @@ macro putByVal(slowPath)
16191619
loadisFromInstruction(2, t0)
16201620
loadConstantOrVariableInt32(t0, t3, .opPutByValSlow)
16211621
sxi2q t3, t3
1622-
loadCaged(_g_jsValueGigacageBasePtr, constexpr JSVALUE_GIGACAGE_MASK, JSObject::m_butterfly[t1], t0, t5)
1622+
loadCaged(_g_gigacageBasePtrs + Gigacage::BasePtrs::jsValue, constexpr JSVALUE_GIGACAGE_MASK, JSObject::m_butterfly[t1], t0, t5)
16231623
andi IndexingShapeMask, t2
16241624
bineq t2, Int32Shape, .opPutByValNotInt32
16251625
contiguousPutByVal(

‎Source/JavaScriptCore/offlineasm/ast.rb‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1110,10 +1110,18 @@ def dump
11101110

11111111
class LabelReference < Node
11121112
attr_reader :label
1113+
attr_accessor :offset
11131114

11141115
def initialize(codeOrigin, label)
11151116
super(codeOrigin)
11161117
@label = label
1118+
@offset = 0
1119+
end
1120+
1121+
def plusOffset(additionalOffset)
1122+
result = LabelReference.new(codeOrigin, label)
1123+
result.offset = @offset + additionalOffset
1124+
result
11171125
end
11181126

11191127
def children

‎Source/JavaScriptCore/offlineasm/parser.rb‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -516,7 +516,7 @@ def parseExpressionMul
516516
end
517517

518518
def couldBeExpression
519-
@tokens[@idx] == "-" or @tokens[@idx] == "~" or @tokens[@idx] == "sizeof" or @tokens[@idx] == "constexpr" or isInteger(@tokens[@idx]) or isString(@tokens[@idx]) or isVariable(@tokens[@idx]) or @tokens[@idx] == "("
519+
@tokens[@idx] == "-" or @tokens[@idx] == "~" or @tokens[@idx] == "sizeof" or @tokens[@idx] == "constexpr" or isInteger(@tokens[@idx]) or isString(@tokens[@idx]) or isVariable(@tokens[@idx]) or isLabel(@tokens[@idx]) or @tokens[@idx] == "("
520520
end
521521

522522
def parseExpressionAdd
@@ -574,10 +574,6 @@ def parseOperand(comment)
574574
end
575575
elsif @tokens[@idx] == "["
576576
parseAddress(Immediate.new(@tokens[@idx].codeOrigin, 0))
577-
elsif isLabel @tokens[@idx]
578-
result = LabelReference.new(@tokens[@idx].codeOrigin, Label.forName(@tokens[@idx].codeOrigin, @tokens[@idx].string))
579-
@idx += 1
580-
result
581577
elsif isLocalLabel @tokens[@idx]
582578
result = LocalLabelReference.new(@tokens[@idx].codeOrigin, LocalLabel.forName(@tokens[@idx].codeOrigin, @tokens[@idx].string))
583579
@idx += 1

‎Source/JavaScriptCore/offlineasm/transform.rb‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,10 @@ class AddImmediates
312312
def fold
313313
@left = @left.fold
314314
@right = @right.fold
315+
316+
return right.plusOffset(@left.value) if @left.is_a? Immediate and @right.is_a? LabelReference
317+
return left.plusOffset(@right.value) if @left.is_a? LabelReference and @right.is_a? Immediate
318+
315319
return self unless @left.is_a? Immediate
316320
return self unless @right.is_a? Immediate
317321
Immediate.new(codeOrigin, @left.value + @right.value)
@@ -322,6 +326,9 @@ class SubImmediates
322326
def fold
323327
@left = @left.fold
324328
@right = @right.fold
329+
330+
return left.plusOffset(-@right.value) if @left.is_a? LabelReference and @right.is_a? Immediate
331+
325332
return self unless @left.is_a? Immediate
326333
return self unless @right.is_a? Immediate
327334
Immediate.new(codeOrigin, @left.value - @right.value)

‎Source/JavaScriptCore/offlineasm/x86.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,7 @@ def x86LoadOperand(kind, dst)
466466
# FIXME: Implement this on platforms that aren't Mach-O.
467467
# https://bugs.webkit.org/show_bug.cgi?id=175104
468468
$asm.puts "movq #{asmLabel}@GOTPCREL(%rip), #{dst.x86Operand(:ptr)}"
469-
"(#{dst.x86Operand(kind)})"
469+
"#{offset}(#{dst.x86Operand(kind)})"
470470
end
471471
end
472472

‎Source/bmalloc/ChangeLog‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,20 @@
1+
2017-09-26 Filip Pizlo <fpizlo@apple.com>
2+
3+
Put g_gigacageBasePtr into its own page and make it read-only
4+
https://bugs.webkit.org/show_bug.cgi?id=174972
5+
6+
Reviewed by Michael Saboff.
7+
8+
This puts the gigacage base pointers into their own page and makes that page read-only.
9+
10+
* bmalloc/Gigacage.cpp:
11+
(Gigacage::ensureGigacage):
12+
(Gigacage::disablePrimitiveGigacage):
13+
(Gigacage::addPrimitiveDisableCallback):
14+
* bmalloc/Gigacage.h:
15+
(Gigacage::basePtr):
16+
(Gigacage::basePtrs):
17+
118
2017-09-04 Adrian Perez de Castro <aperez@igalia.com>
219

320
Unreviewed build fix for Clang with libc++

‎Source/bmalloc/bmalloc/Gigacage.cpp‎

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,17 +33,43 @@
3333
#include <cstdio>
3434
#include <mutex>
3535

36-
// FIXME: Ask dyld to put this in its own page, and mprotect the page after we ensure the gigacage.
37-
// https://bugs.webkit.org/show_bug.cgi?id=174972
38-
void* g_primitiveGigacageBasePtr;
39-
void* g_jsValueGigacageBasePtr;
40-
void* g_stringGigacageBasePtr;
36+
char g_gigacageBasePtrs[GIGACAGE_BASE_PTRS_SIZE] __attribute__((aligned(GIGACAGE_BASE_PTRS_SIZE)));
4137

4238
using namespace bmalloc;
4339

4440
namespace Gigacage {
4541

46-
static bool s_isDisablingPrimitiveGigacageDisabled;
42+
namespace {
43+
44+
bool s_isDisablingPrimitiveGigacageDisabled;
45+
46+
void protectGigacageBasePtrs()
47+
{
48+
uintptr_t basePtrs = reinterpret_cast<uintptr_t>(g_gigacageBasePtrs);
49+
// We might only get page size alignment, but that's also the minimum we need.
50+
RELEASE_BASSERT(!(basePtrs & (vmPageSize() - 1)));
51+
mprotect(g_gigacageBasePtrs, GIGACAGE_BASE_PTRS_SIZE, PROT_READ);
52+
}
53+
54+
void unprotectGigacageBasePtrs()
55+
{
56+
mprotect(g_gigacageBasePtrs, GIGACAGE_BASE_PTRS_SIZE, PROT_READ | PROT_WRITE);
57+
}
58+
59+
class UnprotectGigacageBasePtrsScope {
60+
public:
61+
UnprotectGigacageBasePtrsScope()
62+
{
63+
unprotectGigacageBasePtrs();
64+
}
65+
66+
~UnprotectGigacageBasePtrsScope()
67+
{
68+
protectGigacageBasePtrs();
69+
}
70+
};
71+
72+
} // anonymous namespce
4773

4874
struct Callback {
4975
Callback() { }
@@ -86,14 +112,16 @@ void ensureGigacage()
86112

87113
vmDeallocatePhysicalPages(basePtr(kind), totalSize(kind));
88114
});
115+
116+
protectGigacageBasePtrs();
89117
});
90118
#endif // GIGACAGE_ENABLED
91119
}
92120

93121
void disablePrimitiveGigacage()
94122
{
95123
ensureGigacage();
96-
if (!g_primitiveGigacageBasePtr) {
124+
if (!basePtrs().primitive) {
97125
// It was never enabled. That means that we never even saved any callbacks. Or, we had already disabled
98126
// it before, and already called the callbacks.
99127
return;
@@ -104,13 +132,14 @@ void disablePrimitiveGigacage()
104132
for (Callback& callback : callbacks.callbacks)
105133
callback.function(callback.argument);
106134
callbacks.callbacks.shrink(0);
107-
g_primitiveGigacageBasePtr = nullptr;
135+
UnprotectGigacageBasePtrsScope unprotectScope;
136+
basePtrs().primitive = nullptr;
108137
}
109138

110139
void addPrimitiveDisableCallback(void (*function)(void*), void* argument)
111140
{
112141
ensureGigacage();
113-
if (!g_primitiveGigacageBasePtr) {
142+
if (!basePtrs().primitive) {
114143
// It was already disabled or we were never able to enable it.
115144
function(argument);
116145
return;

‎Source/bmalloc/bmalloc/Gigacage.h‎

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,18 @@
5757
#define GIGACAGE_ENABLED 0
5858
#endif
5959

60-
extern "C" BEXPORT void* g_primitiveGigacageBasePtr;
61-
extern "C" BEXPORT void* g_jsValueGigacageBasePtr;
62-
extern "C" BEXPORT void* g_stringGigacageBasePtr;
60+
#define GIGACAGE_BASE_PTRS_SIZE 8192
61+
62+
extern "C" BEXPORT char g_gigacageBasePtrs[GIGACAGE_BASE_PTRS_SIZE] __attribute__((aligned(GIGACAGE_BASE_PTRS_SIZE)));
6363

6464
namespace Gigacage {
6565

66+
struct BasePtrs {
67+
void* primitive;
68+
void* jsValue;
69+
void* string;
70+
};
71+
6672
enum Kind {
6773
Primitive,
6874
JSValue,
@@ -97,18 +103,28 @@ BINLINE const char* name(Kind kind)
97103
return nullptr;
98104
}
99105

100-
BINLINE void*& basePtr(Kind kind)
106+
BINLINE void*& basePtr(BasePtrs& basePtrs, Kind kind)
101107
{
102108
switch (kind) {
103109
case Primitive:
104-
return g_primitiveGigacageBasePtr;
110+
return basePtrs.primitive;
105111
case JSValue:
106-
return g_jsValueGigacageBasePtr;
112+
return basePtrs.jsValue;
107113
case String:
108-
return g_stringGigacageBasePtr;
114+
return basePtrs.string;
109115
}
110116
BCRASH();
111-
return g_primitiveGigacageBasePtr;
117+
return basePtrs.primitive;
118+
}
119+
120+
BINLINE BasePtrs& basePtrs()
121+
{
122+
return *reinterpret_cast<BasePtrs*>(g_gigacageBasePtrs);
123+
}
124+
125+
BINLINE void*& basePtr(Kind kind)
126+
{
127+
return basePtr(basePtrs(), kind);
112128
}
113129

114130
BINLINE size_t size(Kind kind)

0 commit comments

Comments
 (0)