From ed21f1eb5d3fb28a90efb8ff1618bf4bea23a775 Mon Sep 17 00:00:00 2001 From: Jan de Mooij Date: Fri, 4 May 2018 17:29:05 +0200 Subject: [PATCH] Bug 1458456 part 8 - Fix remaining TSan races. r=jonco --- js/src/gc/Tracer.cpp | 21 +++++++++++++++------ js/src/vm/JSCompartment.cpp | 13 +++++++++++++ js/src/vm/JSCompartment.h | 4 +--- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/js/src/gc/Tracer.cpp b/js/src/gc/Tracer.cpp index dc622e364576..0838ebea03d5 100644 --- a/js/src/gc/Tracer.cpp +++ b/js/src/gc/Tracer.cpp @@ -64,24 +64,33 @@ template <> Value DoCallback(JS::CallbackTracer* trc, Value* vp, const char* name) { - *vp = DispatchTyped(DoCallbackFunctor(), *vp, trc, name); - return *vp; + // Only update *vp if the value changed, to avoid TSan false positives for + // template objects when using DumpHeapTracer or UbiNode tracers while Ion + // compiling off-thread. + Value v = DispatchTyped(DoCallbackFunctor(), *vp, trc, name); + if (*vp != v) + *vp = v; + return v; } template <> jsid DoCallback(JS::CallbackTracer* trc, jsid* idp, const char* name) { - *idp = DispatchTyped(DoCallbackFunctor(), *idp, trc, name); - return *idp; + jsid id = DispatchTyped(DoCallbackFunctor(), *idp, trc, name); + if (*idp != id) + *idp = id; + return id; } template <> TaggedProto DoCallback(JS::CallbackTracer* trc, TaggedProto* protop, const char* name) { - *protop = DispatchTyped(DoCallbackFunctor(), *protop, trc, name); - return *protop; + TaggedProto proto = DispatchTyped(DoCallbackFunctor(), *protop, trc, name); + if (*protop != proto) + *protop = proto; + return proto; } void diff --git a/js/src/vm/JSCompartment.cpp b/js/src/vm/JSCompartment.cpp index d63256d77526..e85e77ab1072 100644 --- a/js/src/vm/JSCompartment.cpp +++ b/js/src/vm/JSCompartment.cpp @@ -1032,6 +1032,19 @@ JSCompartment::setAllocationMetadataBuilder(const js::AllocationMetadataBuilder allocationMetadataBuilder = builder; } +void +JSCompartment::forgetAllocationMetadataBuilder() +{ + // Unlike setAllocationMetadataBuilder, we don't have to discard all JIT + // code here (code is still valid, just a bit slower because it doesn't do + // inline GC allocations when a metadata builder is present), but we do want + // to cancel off-thread Ion compilations to avoid races when Ion calls + // hasAllocationMetadataBuilder off-thread. + CancelOffThreadIonCompile(this); + + allocationMetadataBuilder = nullptr; +} + void JSCompartment::clearObjectMetadata() { diff --git a/js/src/vm/JSCompartment.h b/js/src/vm/JSCompartment.h index 52c2ef1f620f..b8124674960e 100644 --- a/js/src/vm/JSCompartment.h +++ b/js/src/vm/JSCompartment.h @@ -956,9 +956,7 @@ struct JSCompartment return allocationMetadataBuilder; } void setAllocationMetadataBuilder(const js::AllocationMetadataBuilder* builder); - void forgetAllocationMetadataBuilder() { - allocationMetadataBuilder = nullptr; - } + void forgetAllocationMetadataBuilder(); void setNewObjectMetadata(JSContext* cx, JS::HandleObject obj); void clearObjectMetadata(); const void* addressOfMetadataBuilder() const {