Skip to content

Commit 38301e7

Browse files
gsathyaCommit Bot
gsathya
authored and
Commit Bot
committed
Revert "Reland "[runtime] Move Context::native_context to the map""
This reverts commit c7c47c6. Reason for revert: breaks TSAN https://ci.chromium.org/p/v8/builders/ci/V8%20Linux64%20TSAN/28738 Original change's description: > Reland "[runtime] Move Context::native_context to the map" > > This is a reland of f05bae1 > > Previously I presumed that the context read from a frame in the profiler was > a valid context. Turns out that on non-intel we're not guaranteed that the > frame is properly set up. In the case we looked at, the profiler took a > sample right before writing the frame marker indicating a builtin frame, > causing the "context" pointer from that frame to be a bytecode array. Since > we'll read random garbage on the stack as a possible context pointer, I made > the code reading the native context from it a little more defensive. > > Bug: v8:9860 > > Original change's description: > > [runtime] Move Context::native_context to the map > > > > Remove the native context slot from contexts by making context maps > > native-context-specific. Now we require 2 loads to go from a context to the > > native context, but we have 1 field fewer to store when creating contexts. > > > > Change-Id: I3c0d7c50c94060c4129db684f46a567de6f30e8d > > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1859629 > > Commit-Queue: Toon Verwaest <verwaest@chromium.org> > > Reviewed-by: Igor Sheludko <ishell@chromium.org> > > Reviewed-by: Peter Marshall <petermarshall@chromium.org> > > Reviewed-by: Maya Lekova <mslekova@chromium.org> > > Reviewed-by: Georg Neis <neis@chromium.org> > > Reviewed-by: Ulan Degenbaev <ulan@chromium.org> > > Reviewed-by: Toon Verwaest <verwaest@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#64296} > > Change-Id: If9461e9b21d35a260d71c79d7f95e518cc429e09 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1864930 > Reviewed-by: Ulan Degenbaev <ulan@chromium.org> > Reviewed-by: Peter Marshall <petermarshall@chromium.org> > Reviewed-by: Igor Sheludko <ishell@chromium.org> > Reviewed-by: Georg Neis <neis@chromium.org> > Commit-Queue: Toon Verwaest <verwaest@chromium.org> > Auto-Submit: Toon Verwaest <verwaest@chromium.org> > Cr-Commit-Position: refs/heads/master@{#64314} TBR=ulan@chromium.org,neis@chromium.org,petermarshall@chromium.org,ishell@chromium.org,verwaest@chromium.org,mslekova@chromium.org,victorgomes@google.com Change-Id: I4f9edc62ea6f9f5857619ff0ad1a63cab4b33cc3 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: v8:9860 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1864937 Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org> Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org> Cr-Commit-Position: refs/heads/master@{#64316}
1 parent 76bc9a8 commit 38301e7

73 files changed

Lines changed: 1068 additions & 1042 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎include/v8-internal.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ class Internals {
146146
static const int kFixedArrayHeaderSize = 2 * kApiTaggedSize;
147147
static const int kEmbedderDataArrayHeaderSize = 2 * kApiTaggedSize;
148148
static const int kEmbedderDataSlotSize = kApiSystemPointerSize;
149-
static const int kNativeContextEmbedderDataOffset = 6 * kApiTaggedSize;
149+
static const int kNativeContextEmbedderDataOffset = 7 * kApiTaggedSize;
150150
static const int kFullStringRepresentationMask = 0x0f;
151151
static const int kStringEncodingMask = 0x8;
152152
static const int kExternalTwoByteRepresentationTag = 0x02;

‎src/builtins/base.tq‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ extern class Context extends HeapObject {
117117
scope_info: ScopeInfo;
118118
previous: Object;
119119
extension: Object;
120+
native_context: Object;
120121
}
121122
extern class AwaitContext extends Context generates 'TNode<Context>';
122123
extern class BlockContext extends Context generates 'TNode<Context>';
@@ -302,7 +303,7 @@ extern class Map extends HeapObject {
302303
@ifnot(TAGGED_SIZE_8_BYTES) optional_padding: void;
303304

304305
prototype: HeapObject;
305-
constructor_or_back_pointer_or_native_context: Object;
306+
constructor_or_back_pointer: Object;
306307
instance_descriptors: DescriptorArray;
307308
@if(V8_DOUBLE_FIELDS_UNBOXING) layout_descriptor: LayoutDescriptor;
308309
@ifnot(V8_DOUBLE_FIELDS_UNBOXING) layout_descriptor: void;

‎src/builtins/builtins-async-gen.cc‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,7 @@ TNode<Object> AsyncBuiltinsAssembler::AwaitOld(
4545
TNode<Context> closure_context = UncheckedCast<Context>(base);
4646
{
4747
// Initialize the await context, storing the {generator} as extension.
48-
TNode<Map> map = CAST(
49-
LoadContextElement(native_context, Context::AWAIT_CONTEXT_MAP_INDEX));
50-
StoreMapNoWriteBarrier(closure_context, map);
48+
StoreMapNoWriteBarrier(closure_context, RootIndex::kAwaitContextMap);
5149
StoreObjectFieldNoWriteBarrier(closure_context, Context::kLengthOffset,
5250
SmiConstant(Context::MIN_CONTEXT_SLOTS));
5351
TNode<Object> const empty_scope_info =
@@ -58,6 +56,8 @@ TNode<Object> AsyncBuiltinsAssembler::AwaitOld(
5856
native_context);
5957
StoreContextElementNoWriteBarrier(closure_context, Context::EXTENSION_INDEX,
6058
generator);
59+
StoreContextElementNoWriteBarrier(
60+
closure_context, Context::NATIVE_CONTEXT_INDEX, native_context);
6161
}
6262

6363
// Let promiseCapability be ! NewPromiseCapability(%Promise%).
@@ -138,9 +138,7 @@ TNode<Object> AsyncBuiltinsAssembler::AwaitOptimized(
138138
TNode<Context> closure_context = UncheckedCast<Context>(base);
139139
{
140140
// Initialize the await context, storing the {generator} as extension.
141-
TNode<Map> map = CAST(
142-
LoadContextElement(native_context, Context::AWAIT_CONTEXT_MAP_INDEX));
143-
StoreMapNoWriteBarrier(closure_context, map);
141+
StoreMapNoWriteBarrier(closure_context, RootIndex::kAwaitContextMap);
144142
StoreObjectFieldNoWriteBarrier(closure_context, Context::kLengthOffset,
145143
SmiConstant(Context::MIN_CONTEXT_SLOTS));
146144
TNode<Object> const empty_scope_info =
@@ -151,6 +149,8 @@ TNode<Object> AsyncBuiltinsAssembler::AwaitOptimized(
151149
native_context);
152150
StoreContextElementNoWriteBarrier(closure_context, Context::EXTENSION_INDEX,
153151
generator);
152+
StoreContextElementNoWriteBarrier(
153+
closure_context, Context::NATIVE_CONTEXT_INDEX, native_context);
154154
}
155155

156156
// Initialize resolve handler

‎src/builtins/builtins-call-gen.cc‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -412,9 +412,8 @@ TNode<JSReceiver> CallOrConstructBuiltinsAssembler::GetCompatibleReceiver(
412412
// {var_template} variable), and see if that is a HeapObject.
413413
// If it's a Smi then it is non-instance prototype on some
414414
// initial map, which cannot be the case for API instances.
415-
TNode<Object> constructor =
416-
LoadObjectField(var_template.value(),
417-
Map::kConstructorOrBackPointerOrNativeContextOffset);
415+
TNode<Object> constructor = LoadObjectField(
416+
var_template.value(), Map::kConstructorOrBackPointerOffset);
418417
GotoIf(TaggedIsSmi(constructor), &holder_next);
419418

420419
// Now there are three cases for {constructor} that we care

‎src/builtins/builtins-constructor-gen.cc‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -196,8 +196,8 @@ TNode<JSObject> ConstructorBuiltinsAssembler::EmitFastNewObject(
196196

197197
// Fall back to runtime if the target differs from the new target's
198198
// initial map constructor.
199-
TNode<Object> new_target_constructor = LoadObjectField(
200-
initial_map, Map::kConstructorOrBackPointerOrNativeContextOffset);
199+
TNode<Object> new_target_constructor =
200+
LoadObjectField(initial_map, Map::kConstructorOrBackPointerOffset);
201201
GotoIf(TaggedNotEqual(target, new_target_constructor), call_runtime);
202202

203203
TVARIABLE(HeapObject, properties);
@@ -230,21 +230,19 @@ TNode<Context> ConstructorBuiltinsAssembler::EmitFastNewFunctionContext(
230230
TNode<Context> function_context =
231231
UncheckedCast<Context>(AllocateInNewSpace(size));
232232

233-
TNode<NativeContext> native_context = LoadNativeContext(context);
234-
Context::Field index;
233+
RootIndex context_type;
235234
switch (scope_type) {
236235
case EVAL_SCOPE:
237-
index = Context::EVAL_CONTEXT_MAP_INDEX;
236+
context_type = RootIndex::kEvalContextMap;
238237
break;
239238
case FUNCTION_SCOPE:
240-
index = Context::FUNCTION_CONTEXT_MAP_INDEX;
239+
context_type = RootIndex::kFunctionContextMap;
241240
break;
242241
default:
243242
UNREACHABLE();
244243
}
245-
TNode<Map> map = CAST(LoadContextElement(native_context, index));
246244
// Set up the header.
247-
StoreMapNoWriteBarrier(function_context, map);
245+
StoreMapNoWriteBarrier(function_context, context_type);
248246
TNode<IntPtrT> min_context_slots = IntPtrConstant(Context::MIN_CONTEXT_SLOTS);
249247
// TODO(ishell): for now, length also includes MIN_CONTEXT_SLOTS.
250248
TNode<IntPtrT> length = IntPtrAdd(slots_intptr, min_context_slots);
@@ -256,6 +254,9 @@ TNode<Context> ConstructorBuiltinsAssembler::EmitFastNewFunctionContext(
256254
context);
257255
StoreObjectFieldNoWriteBarrier(function_context, Context::kExtensionOffset,
258256
TheHoleConstant());
257+
TNode<NativeContext> native_context = LoadNativeContext(context);
258+
StoreObjectFieldNoWriteBarrier(function_context,
259+
Context::kNativeContextOffset, native_context);
259260

260261
// Initialize the varrest of the slots to undefined.
261262
TNode<Oddball> undefined = UndefinedConstant();

‎src/builtins/builtins-proxy-gen.cc‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,9 +119,7 @@ Node* ProxiesCodeStubAssembler::CreateProxyRevokeFunctionContext(
119119
Node* proxy, Node* native_context) {
120120
TNode<HeapObject> const context =
121121
Allocate(FixedArray::SizeFor(kProxyContextLength));
122-
TNode<Map> map = CAST(
123-
LoadContextElement(native_context, Context::FUNCTION_CONTEXT_MAP_INDEX));
124-
StoreMapNoWriteBarrier(context, map);
122+
StoreMapNoWriteBarrier(context, RootIndex::kFunctionContextMap);
125123
InitializeFunctionContext(native_context, context, kProxyContextLength);
126124
StoreContextElementNoWriteBarrier(CAST(context), kProxySlot, proxy);
127125
return context;

‎src/builtins/ia32/builtins-ia32.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2354,7 +2354,7 @@ void Builtins::Generate_Call(MacroAssembler* masm, ConvertReceiverMode mode) {
23542354
// Overwrite the original receiver with the (original) target.
23552355
__ mov(Operand(esp, eax, times_system_pointer_size, kSystemPointerSize), edi);
23562356
// Let the "call_as_function_delegate" take care of the rest.
2357-
__ LoadNativeContextSlot(edi, Context::CALL_AS_FUNCTION_DELEGATE_INDEX);
2357+
__ LoadGlobalFunction(Context::CALL_AS_FUNCTION_DELEGATE_INDEX, edi);
23582358
__ Jump(masm->isolate()->builtins()->CallFunction(
23592359
ConvertReceiverMode::kNotNullOrUndefined),
23602360
RelocInfo::CODE_TARGET);
@@ -2474,7 +2474,7 @@ void Builtins::Generate_Construct(MacroAssembler* masm) {
24742474
__ mov(Operand(esp, eax, times_system_pointer_size, kSystemPointerSize),
24752475
edi);
24762476
// Let the "call_as_constructor_delegate" take care of the rest.
2477-
__ LoadNativeContextSlot(edi, Context::CALL_AS_CONSTRUCTOR_DELEGATE_INDEX);
2477+
__ LoadGlobalFunction(Context::CALL_AS_CONSTRUCTOR_DELEGATE_INDEX, edi);
24782478
__ Jump(masm->isolate()->builtins()->CallFunction(),
24792479
RelocInfo::CODE_TARGET);
24802480
}

‎src/codegen/arm/macro-assembler-arm.cc‎

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1775,7 +1775,7 @@ void MacroAssembler::CompareObjectType(Register object, Register map,
17751775
UseScratchRegisterScope temps(this);
17761776
const Register temp = type_reg == no_reg ? temps.Acquire() : type_reg;
17771777

1778-
LoadMap(map, object);
1778+
ldr(map, FieldMemOperand(object, HeapObject::kMapOffset));
17791779
CompareInstanceType(map, temp, type);
17801780
}
17811781

@@ -2010,19 +2010,13 @@ void TurboAssembler::Abort(AbortReason reason) {
20102010
// will not return here
20112011
}
20122012

2013-
void MacroAssembler::LoadMap(Register destination, Register object) {
2014-
ldr(destination, FieldMemOperand(object, HeapObject::kMapOffset));
2015-
}
2016-
20172013
void MacroAssembler::LoadGlobalProxy(Register dst) {
20182014
LoadNativeContextSlot(Context::GLOBAL_PROXY_INDEX, dst);
20192015
}
20202016

20212017
void MacroAssembler::LoadNativeContextSlot(int index, Register dst) {
2022-
LoadMap(dst, cp);
2023-
ldr(dst, FieldMemOperand(
2024-
dst, Map::kConstructorOrBackPointerOrNativeContextOffset));
2025-
ldr(dst, MemOperand(dst, Context::SlotOffset(index)));
2018+
ldr(dst, NativeContextMemOperand());
2019+
ldr(dst, ContextMemOperand(dst, index));
20262020
}
20272021

20282022
void TurboAssembler::InitializeRootRegister() {
@@ -2084,7 +2078,7 @@ void MacroAssembler::AssertConstructor(Register object) {
20842078
tst(object, Operand(kSmiTagMask));
20852079
Check(ne, AbortReason::kOperandIsASmiAndNotAConstructor);
20862080
push(object);
2087-
LoadMap(object, object);
2081+
ldr(object, FieldMemOperand(object, HeapObject::kMapOffset));
20882082
ldrb(object, FieldMemOperand(object, Map::kBitFieldOffset));
20892083
tst(object, Operand(Map::IsConstructorBit::kMask));
20902084
pop(object);
@@ -2124,7 +2118,7 @@ void MacroAssembler::AssertGeneratorObject(Register object) {
21242118
// Load map
21252119
Register map = object;
21262120
push(object);
2127-
LoadMap(map, object);
2121+
ldr(map, FieldMemOperand(object, HeapObject::kMapOffset));
21282122

21292123
// Check if JSGeneratorObject
21302124
Label do_check;
@@ -2152,7 +2146,7 @@ void MacroAssembler::AssertUndefinedOrAllocationSite(Register object,
21522146
AssertNotSmi(object);
21532147
CompareRoot(object, RootIndex::kUndefinedValue);
21542148
b(eq, &done_checking);
2155-
LoadMap(scratch, object);
2149+
ldr(scratch, FieldMemOperand(object, HeapObject::kMapOffset));
21562150
CompareInstanceType(scratch, scratch, ALLOCATION_SITE_TYPE);
21572151
Assert(eq, AbortReason::kExpectedUndefinedOrCell);
21582152
bind(&done_checking);

‎src/codegen/arm/macro-assembler-arm.h‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -620,8 +620,6 @@ class V8_EXPORT_PRIVATE MacroAssembler : public TurboAssembler {
620620
void LeaveExitFrame(bool save_doubles, Register argument_count,
621621
bool argument_count_is_length = false);
622622

623-
void LoadMap(Register destination, Register object);
624-
625623
// Load the global proxy from the current context.
626624
void LoadGlobalProxy(Register dst);
627625

@@ -807,6 +805,17 @@ class V8_EXPORT_PRIVATE MacroAssembler : public TurboAssembler {
807805
DISALLOW_IMPLICIT_CONSTRUCTORS(MacroAssembler);
808806
};
809807

808+
// -----------------------------------------------------------------------------
809+
// Static helper functions.
810+
811+
inline MemOperand ContextMemOperand(Register context, int index = 0) {
812+
return MemOperand(context, Context::SlotOffset(index));
813+
}
814+
815+
inline MemOperand NativeContextMemOperand() {
816+
return ContextMemOperand(cp, Context::NATIVE_CONTEXT_INDEX);
817+
}
818+
810819
#define ACCESS_MASM(masm) masm->
811820

812821
} // namespace internal

‎src/codegen/arm64/macro-assembler-arm64.cc‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1534,7 +1534,8 @@ void MacroAssembler::AssertConstructor(Register object) {
15341534
UseScratchRegisterScope temps(this);
15351535
Register temp = temps.AcquireX();
15361536

1537-
LoadMap(temp, object);
1537+
LoadTaggedPointerField(temp,
1538+
FieldMemOperand(object, HeapObject::kMapOffset));
15381539
Ldrb(temp, FieldMemOperand(temp, Map::kBitFieldOffset));
15391540
Tst(temp, Operand(Map::IsConstructorBit::kMask));
15401541

@@ -1573,7 +1574,7 @@ void MacroAssembler::AssertGeneratorObject(Register object) {
15731574
// Load map
15741575
UseScratchRegisterScope temps(this);
15751576
Register temp = temps.AcquireX();
1576-
LoadMap(temp, object);
1577+
LoadTaggedPointerField(temp, FieldMemOperand(object, HeapObject::kMapOffset));
15771578

15781579
Label do_check;
15791580
// Load instance type and check if JSGeneratorObject
@@ -1599,7 +1600,8 @@ void MacroAssembler::AssertUndefinedOrAllocationSite(Register object) {
15991600
Label done_checking;
16001601
AssertNotSmi(object);
16011602
JumpIfRoot(object, RootIndex::kUndefinedValue, &done_checking);
1602-
LoadMap(scratch, object);
1603+
LoadTaggedPointerField(scratch,
1604+
FieldMemOperand(object, HeapObject::kMapOffset));
16031605
CompareInstanceType(scratch, scratch, ALLOCATION_SITE_TYPE);
16041606
Assert(eq, AbortReason::kExpectedUndefinedOrCell);
16051607
Bind(&done_checking);
@@ -2618,14 +2620,10 @@ void MacroAssembler::JumpIfObjectType(Register object, Register map,
26182620
// Sets condition flags based on comparison, and returns type in type_reg.
26192621
void MacroAssembler::CompareObjectType(Register object, Register map,
26202622
Register type_reg, InstanceType type) {
2621-
LoadMap(map, object);
2623+
LoadTaggedPointerField(map, FieldMemOperand(object, HeapObject::kMapOffset));
26222624
CompareInstanceType(map, type_reg, type);
26232625
}
26242626

2625-
void MacroAssembler::LoadMap(Register dst, Register object) {
2626-
LoadTaggedPointerField(dst, FieldMemOperand(object, HeapObject::kMapOffset));
2627-
}
2628-
26292627
// Sets condition flags based on comparison, and returns type in type_reg.
26302628
void MacroAssembler::CompareInstanceType(Register map, Register type_reg,
26312629
InstanceType type) {
@@ -3079,11 +3077,8 @@ void TurboAssembler::Abort(AbortReason reason) {
30793077
}
30803078

30813079
void MacroAssembler::LoadNativeContextSlot(int index, Register dst) {
3082-
LoadMap(dst, cp);
3083-
LoadTaggedPointerField(
3084-
dst, FieldMemOperand(
3085-
dst, Map::kConstructorOrBackPointerOrNativeContextOffset));
3086-
LoadTaggedPointerField(dst, MemOperand(dst, Context::SlotOffset(index)));
3080+
LoadTaggedPointerField(dst, NativeContextMemOperand());
3081+
LoadTaggedPointerField(dst, ContextMemOperand(dst, index));
30873082
}
30883083

30893084
// This is the main Printf implementation. All other Printf variants call
@@ -3339,6 +3334,14 @@ CPURegister UseScratchRegisterScope::AcquireNextAvailable(
33393334
return result;
33403335
}
33413336

3337+
MemOperand ContextMemOperand(Register context, int index) {
3338+
return MemOperand(context, Context::SlotOffset(index));
3339+
}
3340+
3341+
MemOperand NativeContextMemOperand() {
3342+
return ContextMemOperand(cp, Context::NATIVE_CONTEXT_INDEX);
3343+
}
3344+
33423345
void TurboAssembler::ComputeCodeStartAddress(const Register& rd) {
33433346
// We can use adr to load a pc relative location.
33443347
adr(rd, -pc_offset());

0 commit comments

Comments
 (0)