Commit 799fa7b0 authored by Santiago Aboy Solanes's avatar Santiago Aboy Solanes Committed by V8 LUCI CQ

[object] Set/Get JSFunction::prototype_or_initial_map atomically

Maps set on the JSFunction were done so in a non-atomic way, which meant
that we were failing to have a synchronization point and the read/writes
could be reordered.

This started happening after a previous CL[1] moved some methods from
relaxed to non-atomic, which triggered TSAN (see v8:11696).

[1]: https://chromium-review.googlesource.com/c/v8/v8/+/2843359

Bug: v8:7790, v8:11696
Change-Id: I8472ff8b63d391376ee2f1dcf0a8b4fd7cecfcd1
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2851893Reviewed-by: 's avatarGeorg Neis <neis@chromium.org>
Reviewed-by: 's avatarRoss McIlroy <rmcilroy@chromium.org>
Reviewed-by: 's avatarMichael Lippautz <mlippautz@chromium.org>
Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74357}
parent e75736ae
...@@ -3614,7 +3614,8 @@ Handle<JSFunction> Factory::JSFunctionBuilder::BuildRaw(Handle<Code> code) { ...@@ -3614,7 +3614,8 @@ Handle<JSFunction> Factory::JSFunctionBuilder::BuildRaw(Handle<Code> code) {
function.set_code(*code, kReleaseStore, mode); function.set_code(*code, kReleaseStore, mode);
if (function.has_prototype_slot()) { if (function.has_prototype_slot()) {
function.set_prototype_or_initial_map( function.set_prototype_or_initial_map(
ReadOnlyRoots(isolate).the_hole_value(), SKIP_WRITE_BARRIER); ReadOnlyRoots(isolate).the_hole_value(), kReleaseStore,
SKIP_WRITE_BARRIER);
} }
// Potentially body initialization. // Potentially body initialization.
......
...@@ -1638,8 +1638,8 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object, ...@@ -1638,8 +1638,8 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
JSFunction::kSizeWithPrototype, 0, prototype, JSFunction::kSizeWithPrototype, 0, prototype,
Builtins::kFunctionConstructor); Builtins::kFunctionConstructor);
// Function instances are sloppy by default. // Function instances are sloppy by default.
function_fun->set_prototype_or_initial_map( function_fun->set_prototype_or_initial_map(*isolate_->sloppy_function_map(),
*isolate_->sloppy_function_map()); kReleaseStore);
function_fun->shared().DontAdaptArguments(); function_fun->shared().DontAdaptArguments();
function_fun->shared().set_length(1); function_fun->shared().set_length(1);
InstallWithIntrinsicDefaultProto(isolate_, function_fun, InstallWithIntrinsicDefaultProto(isolate_, function_fun,
...@@ -4168,7 +4168,7 @@ void Genesis::InitializeIteratorFunctions() { ...@@ -4168,7 +4168,7 @@ void Genesis::InitializeIteratorFunctions() {
JSFunction::kSizeWithPrototype, 0, generator_function_prototype, JSFunction::kSizeWithPrototype, 0, generator_function_prototype,
Builtins::kGeneratorFunctionConstructor); Builtins::kGeneratorFunctionConstructor);
generator_function_function->set_prototype_or_initial_map( generator_function_function->set_prototype_or_initial_map(
native_context->generator_function_map()); native_context->generator_function_map(), kReleaseStore);
generator_function_function->shared().DontAdaptArguments(); generator_function_function->shared().DontAdaptArguments();
generator_function_function->shared().set_length(1); generator_function_function->shared().set_length(1);
InstallWithIntrinsicDefaultProto( InstallWithIntrinsicDefaultProto(
...@@ -4197,7 +4197,7 @@ void Genesis::InitializeIteratorFunctions() { ...@@ -4197,7 +4197,7 @@ void Genesis::InitializeIteratorFunctions() {
JSFunction::kSizeWithPrototype, 0, async_generator_function_prototype, JSFunction::kSizeWithPrototype, 0, async_generator_function_prototype,
Builtins::kAsyncGeneratorFunctionConstructor); Builtins::kAsyncGeneratorFunctionConstructor);
async_generator_function_function->set_prototype_or_initial_map( async_generator_function_function->set_prototype_or_initial_map(
native_context->async_generator_function_map()); native_context->async_generator_function_map(), kReleaseStore);
async_generator_function_function->shared().DontAdaptArguments(); async_generator_function_function->shared().DontAdaptArguments();
async_generator_function_function->shared().set_length(1); async_generator_function_function->shared().set_length(1);
InstallWithIntrinsicDefaultProto( InstallWithIntrinsicDefaultProto(
...@@ -4298,7 +4298,7 @@ void Genesis::InitializeIteratorFunctions() { ...@@ -4298,7 +4298,7 @@ void Genesis::InitializeIteratorFunctions() {
JSFunction::kSizeWithPrototype, 0, async_function_prototype, JSFunction::kSizeWithPrototype, 0, async_function_prototype,
Builtins::kAsyncFunctionConstructor); Builtins::kAsyncFunctionConstructor);
async_function_constructor->set_prototype_or_initial_map( async_function_constructor->set_prototype_or_initial_map(
native_context->async_function_map()); native_context->async_function_map(), kReleaseStore);
async_function_constructor->shared().DontAdaptArguments(); async_function_constructor->shared().DontAdaptArguments();
async_function_constructor->shared().set_length(1); async_function_constructor->shared().set_length(1);
native_context->set_async_function_constructor(*async_function_constructor); native_context->set_async_function_constructor(*async_function_constructor);
......
...@@ -206,27 +206,28 @@ void JSFunction::set_context(HeapObject value, WriteBarrierMode mode) { ...@@ -206,27 +206,28 @@ void JSFunction::set_context(HeapObject value, WriteBarrierMode mode) {
CONDITIONAL_WRITE_BARRIER(*this, kContextOffset, value, mode); CONDITIONAL_WRITE_BARRIER(*this, kContextOffset, value, mode);
} }
ACCESSORS_CHECKED(JSFunction, prototype_or_initial_map, HeapObject, RELEASE_ACQUIRE_ACCESSORS_CHECKED(JSFunction, prototype_or_initial_map,
kPrototypeOrInitialMapOffset, map().has_prototype_slot()) HeapObject, kPrototypeOrInitialMapOffset,
map().has_prototype_slot())
DEF_GETTER(JSFunction, has_prototype_slot, bool) { DEF_GETTER(JSFunction, has_prototype_slot, bool) {
return map(cage_base).has_prototype_slot(); return map(cage_base).has_prototype_slot();
} }
DEF_GETTER(JSFunction, initial_map, Map) { DEF_GETTER(JSFunction, initial_map, Map) {
return Map::cast(prototype_or_initial_map(cage_base)); return Map::cast(prototype_or_initial_map(cage_base, kAcquireLoad));
} }
DEF_GETTER(JSFunction, has_initial_map, bool) { DEF_GETTER(JSFunction, has_initial_map, bool) {
DCHECK(has_prototype_slot(cage_base)); DCHECK(has_prototype_slot(cage_base));
return prototype_or_initial_map(cage_base).IsMap(cage_base); return prototype_or_initial_map(cage_base, kAcquireLoad).IsMap(cage_base);
} }
DEF_GETTER(JSFunction, has_instance_prototype, bool) { DEF_GETTER(JSFunction, has_instance_prototype, bool) {
DCHECK(has_prototype_slot(cage_base)); DCHECK(has_prototype_slot(cage_base));
return has_initial_map(cage_base) || return has_initial_map(cage_base) ||
!prototype_or_initial_map(cage_base).IsTheHole( !prototype_or_initial_map(cage_base, kAcquireLoad)
GetReadOnlyRoots(cage_base)); .IsTheHole(GetReadOnlyRoots(cage_base));
} }
DEF_GETTER(JSFunction, has_prototype, bool) { DEF_GETTER(JSFunction, has_prototype, bool) {
...@@ -251,7 +252,7 @@ DEF_GETTER(JSFunction, instance_prototype, HeapObject) { ...@@ -251,7 +252,7 @@ DEF_GETTER(JSFunction, instance_prototype, HeapObject) {
return initial_map(cage_base).prototype(cage_base); return initial_map(cage_base).prototype(cage_base);
// When there is no initial map and the prototype is a JSReceiver, the // When there is no initial map and the prototype is a JSReceiver, the
// initial map field is used for the prototype field. // initial map field is used for the prototype field.
return HeapObject::cast(prototype_or_initial_map(cage_base)); return HeapObject::cast(prototype_or_initial_map(cage_base, kAcquireLoad));
} }
DEF_GETTER(JSFunction, prototype, Object) { DEF_GETTER(JSFunction, prototype, Object) {
......
...@@ -400,7 +400,7 @@ void SetInstancePrototype(Isolate* isolate, Handle<JSFunction> function, ...@@ -400,7 +400,7 @@ void SetInstancePrototype(Isolate* isolate, Handle<JSFunction> function,
// Put the value in the initial map field until an initial map is needed. // Put the value in the initial map field until an initial map is needed.
// At that point, a new initial map is created and the prototype is put // At that point, a new initial map is created and the prototype is put
// into the initial map where it belongs. // into the initial map where it belongs.
function->set_prototype_or_initial_map(*value); function->set_prototype_or_initial_map(*value, kReleaseStore);
} else { } else {
Handle<Map> new_map = Handle<Map> new_map =
Map::Copy(isolate, initial_map, "SetInstancePrototype"); Map::Copy(isolate, initial_map, "SetInstancePrototype");
...@@ -425,7 +425,7 @@ void SetInstancePrototype(Isolate* isolate, Handle<JSFunction> function, ...@@ -425,7 +425,7 @@ void SetInstancePrototype(Isolate* isolate, Handle<JSFunction> function,
// Put the value in the initial map field until an initial map is // Put the value in the initial map field until an initial map is
// needed. At that point, a new initial map is created and the // needed. At that point, a new initial map is created and the
// prototype is put into the initial map where it belongs. // prototype is put into the initial map where it belongs.
function->set_prototype_or_initial_map(*value); function->set_prototype_or_initial_map(*value, kReleaseStore);
if (value->IsJSObject()) { if (value->IsJSObject()) {
// Optimize as prototype to detach it from its transition tree. // Optimize as prototype to detach it from its transition tree.
JSObject::OptimizeAsPrototype(Handle<JSObject>::cast(value)); JSObject::OptimizeAsPrototype(Handle<JSObject>::cast(value));
...@@ -488,7 +488,7 @@ void JSFunction::SetInitialMap(Isolate* isolate, Handle<JSFunction> function, ...@@ -488,7 +488,7 @@ void JSFunction::SetInitialMap(Isolate* isolate, Handle<JSFunction> function,
Map::SetPrototype(isolate, map, prototype); Map::SetPrototype(isolate, map, prototype);
} }
map->SetConstructor(*constructor); map->SetConstructor(*constructor);
function->set_prototype_or_initial_map(*map); function->set_prototype_or_initial_map(*map, kReleaseStore);
if (FLAG_log_maps) { if (FLAG_log_maps) {
LOG(isolate, MapEvent("InitialMap", Handle<Map>(), map, "", LOG(isolate, MapEvent("InitialMap", Handle<Map>(), map, "",
SharedFunctionInfo::DebugName( SharedFunctionInfo::DebugName(
......
...@@ -55,7 +55,7 @@ class JSBoundFunction ...@@ -55,7 +55,7 @@ class JSBoundFunction
class JSFunction : public JSFunctionOrBoundFunction { class JSFunction : public JSFunctionOrBoundFunction {
public: public:
// [prototype_or_initial_map]: // [prototype_or_initial_map]:
DECL_ACCESSORS(prototype_or_initial_map, HeapObject) DECL_RELEASE_ACQUIRE_ACCESSORS(prototype_or_initial_map, HeapObject)
// [shared]: The information about the function that // [shared]: The information about the function that
// can be shared by instances. // can be shared by instances.
......
...@@ -889,7 +889,7 @@ void V8HeapExplorer::ExtractJSObjectReferences(HeapEntry* entry, ...@@ -889,7 +889,7 @@ void V8HeapExplorer::ExtractJSObjectReferences(HeapEntry* entry,
} else if (obj.IsJSFunction()) { } else if (obj.IsJSFunction()) {
JSFunction js_fun = JSFunction::cast(js_obj); JSFunction js_fun = JSFunction::cast(js_obj);
if (js_fun.has_prototype_slot()) { if (js_fun.has_prototype_slot()) {
Object proto_or_map = js_fun.prototype_or_initial_map(); Object proto_or_map = js_fun.prototype_or_initial_map(kAcquireLoad);
if (!proto_or_map.IsTheHole(isolate)) { if (!proto_or_map.IsTheHole(isolate)) {
if (!proto_or_map.IsMap()) { if (!proto_or_map.IsMap()) {
SetPropertyReference(entry, roots.prototype_string(), proto_or_map, SetPropertyReference(entry, roots.prototype_string(), proto_or_map,
......
...@@ -497,7 +497,7 @@ bool InitClassPrototype(Isolate* isolate, ...@@ -497,7 +497,7 @@ bool InitClassPrototype(Isolate* isolate,
map = Map::CopyDropDescriptors(isolate, map); map = Map::CopyDropDescriptors(isolate, map);
map->set_is_prototype_map(true); map->set_is_prototype_map(true);
Map::SetPrototype(isolate, map, prototype_parent); Map::SetPrototype(isolate, map, prototype_parent);
constructor->set_prototype_or_initial_map(*prototype); constructor->set_prototype_or_initial_map(*prototype, kReleaseStore);
map->SetConstructor(*constructor); map->SetConstructor(*constructor);
Handle<FixedArray> computed_properties( Handle<FixedArray> computed_properties(
class_boilerplate->instance_computed_properties(), isolate); class_boilerplate->instance_computed_properties(), isolate);
......
...@@ -3070,7 +3070,8 @@ TEST(NewPromiseCapability) { ...@@ -3070,7 +3070,8 @@ TEST(NewPromiseCapability) {
CHECK(result->promise().IsJSObject()); CHECK(result->promise().IsJSObject());
Handle<JSObject> promise(JSObject::cast(result->promise()), isolate); Handle<JSObject> promise(JSObject::cast(result->promise()), isolate);
CHECK_EQ(constructor_fn->prototype_or_initial_map(), promise->map()); CHECK_EQ(constructor_fn->prototype_or_initial_map(kAcquireLoad),
promise->map());
CHECK(result->resolve().IsJSFunction()); CHECK(result->resolve().IsJSFunction());
CHECK(result->reject().IsJSFunction()); CHECK(result->reject().IsJSFunction());
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment