Flip the polarity of the frontend flag controlling whether TSan treats inout
accesses as conceptual writes. It is now on by default. This lets TSan detect
racing mutating methods even when those methods are not themselves instrumented
(such as methods on Standard Library collections).
This behavior can be disabled by passing:
-Xfrontend -disable-tsan-inout-instrumentation
when compiling under TSan.
rdar://problem/31069963
What this routine does is:
1. "Imprints" a cleanup cloner with the managed value.
2. Forwards the managed value.
3. Pop the scope.
4. Use the imprinted cleanup cloner to recreate the managed value in the scope
outside of the current scope.
This just automates the work of:
1. Grabbing the cleanup manager from the SGF.
2. Forming a CleanupLocation from a SILLocation.
Much of the time when one creates a scope, that is the information that one has
available. So lets make it... wait for it... convenient to construct a scope
with this data.
(This re-applies #7736 with an update to the
tsan-inout.swift execution test to handle configurations where
TSan's ignore_interceptors_accesses is enabled by default.)
Add SILGen instrumentation to treat inout accesses as Thread Sanitizer writes.
The goal is to catch races on inout accesses even when there is a not an
llvm-level read/write to a particular address. Ultimately
this will enable TSan to, for example, report racy writes to distinct
stored properties of a common struct as a data race.
This instrumentation is off by default. It can be enabled with the
'enable-experimental-tsan-inout-instrumentation' frontend flag.
The high-level approach is to add a SIL-level builtin that represents a call
to a TSan routine in compiler-rt. Then, when emitting an address for an LValue
as part of an inout expression, we call this builtin for each path component
that represents an LValue. I've added an 'isRValue()' method to PathComponent
that tracks whether a component represents an RValue or an LValue. Right the
only PathComponent that sometimes returns 'true' is ValueComponent().
For now, we're instrumenting only InoutExprs, but in the future it probably
makes sense to instrument all LValue accesses. In this patch I've
added a 'TSanKind' parameter to SILGenFunction::emitAddressOfLValue() and
its helpers to limit instrumentation to inout accesses. I envision that this
parameter will eventually go away.
I am going to be cleaning up some of the code here so that I can move the
creation of a temporary needed for foreign error handling earlier and move the
handling of the error parameter in general into ResultPlanBuilder.
Otherwise, the temporary is cleaned up when I pop a soon to be committed patch
that creates scopes around call sites.
rdar://30955427
This fixes a logic error. Specifically: When we create a formal evaluation scope
in an inout conversion scope, we set it as if it was already popped. In the case
when we pop the scope by hand, this causes a "don't pop this scope twice" assert
to fire.
Add SILGen instrumentation to treat inout accesses as Thread Sanitizer writes.
The goal is to catch races on inout accesses even when there is a not an
llvm-level read/write to a particular address. Ultimately
this will enable TSan to, for example, report racy writes to distinct
stored properties of a common struct as a data race.
This instrumentation is off by default. It can be enabled with the
'enable-experimental-tsan-inout-instrumentation' frontend flag.
The high-level approach is to add a SIL-level builtin that represents a call
to a TSan routine in compiler-rt. Then, when emitting an address for an LValue
as part of an inout expression, we call this builtin for each path component
that represents an LValue. I've added an 'isRValue()' method to PathComponent
that tracks whether a component represents an RValue or an LValue. Right the
only PathComponent that sometimes returns 'true' is ValueComponent().
For now, we're instrumenting only InoutExprs, but in the future it probably
makes sense to instrument all LValue accesses. In this patch I've
added a 'TSanKind' parameter to SILGenFunction::emitAddressOfLValue() and
its helpers to limit instrumentation to inout accesses. I envision that this
parameter will eventually go away.
I added a general description, but more importantly, I added a note about a
quirk in our current implementation that existed before my current changes.
Specifically:
All formal access contain a pointer to a cleanup in the normal cleanup
stack. This is to ensure that when SILGen calls Cleanups.emitBranchAndCleanups
(and other special cleanup code along error edges), writebacks are properly
created. What is key to notice is that all of these cleanup emission types are
non-destructive. Contrast this with normal scope popping. In such a case, the
scope pop is destructive. This means that any pointers from the formal access to
the cleanup stack is now invalid.
rdar://30955427
