Skip to content

Enable import of _Concurrency by default on Linux #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

drexin
Copy link

@drexin drexin commented Apr 14, 2021

No description provided.

@DougGregor
Copy link
Owner

Thank you!

@DougGregor DougGregor merged this pull request into DougGregor:import-concurrency-by-default-take-2 Apr 14, 2021
DougGregor pushed a commit that referenced this pull request Jun 28, 2021
* Synchronize both versions of actor_counters.swift test

* Synchronize on Job address

Make sure to synchronize on Job address (AsyncTasks are Jobs, but not
all Jobs are AsyncTasks).

* Add fprintf debug output for TSan acquire/release

* Add tsan_release edge on task creation

without this, we are getting false data races between when a task
is created and immediately scheduled on a different thread.

False positive for `Sanitizers/tsan/actor_counters.swift` test:
```
WARNING: ThreadSanitizer: data race (pid=81452)
  Read of size 8 at 0x7b2000000560 by thread T5:
    #0 Counter.next() <null>:2 (a.out:x86_64+0x1000047f8)
    #1 (1) suspend resume partial function for worker(identity:counters:numIterations:) <null>:2 (a.out:x86_64+0x100005961)
    #2 swift::runJobInEstablishedExecutorContext(swift::Job*) <null>:2 (libswift_Concurrency.dylib:x86_64+0x280ef)

  Previous write of size 8 at 0x7b2000000560 by main thread:
    #0 Counter.init(maxCount:) <null>:2 (a.out:x86_64+0x1000046af)
    #1 Counter.__allocating_init(maxCount:) <null>:2 (a.out:x86_64+0x100004619)
    #2 runTest(numCounters:numWorkers:numIterations:) <null>:2 (a.out:x86_64+0x100006d2e)
    #3 swift::runJobInEstablishedExecutorContext(swift::Job*) <null>:2 (libswift_Concurrency.dylib:x86_64+0x280ef)
    #4 main <null>:2 (a.out:x86_64+0x10000a175)
```

New edge with this change:
```
[4357150208] allocate task 0x7b3800000000, parent = 0x0
[4357150208] creating task 0x7b3800000000 with parent 0x0
[4357150208] tsan_release on 0x7b3800000000    <<< new release edge
[139088221442048] tsan_acquire on 0x7b3800000000
[139088221442048] trying to switch from executor 0x0 to 0x7ff85e2d9a00
[139088221442048] switch failed, task 0x7b3800000000 enqueued on executor 0x7ff85e2d9a00
[139088221442048] enqueue job 0x7b3800000000 on executor 0x7ff85e2d9a00
[139088221442048] tsan_release on 0x7b3800000000
[139088221442048] tsan_release on 0x7b3800000000
[4357150208] tsan_acquire on 0x7b3800000000
counters: 1, workers: 1, iterations: 1
[4357150208] allocate task 0x7b3c00000000, parent = 0x0
[4357150208] creating task 0x7b3c00000000 with parent 0x0
[4357150208] tsan_release on 0x7b3c00000000    <<< new release edge
[139088221442048] tsan_acquire on 0x7b3c00000000
[4357150208] task 0x7b3800000000 waiting on task 0x7b3c00000000, going to sleep
[4357150208] tsan_release on 0x7b3800000000
[4357150208] tsan_release on 0x7b3800000000
[139088221442048] getting current executor 0x0
[139088221442048] tsan_release on 0x7b3c00000000
...
```

rdar://78932849

* Add static_cast<Job *>()

* Move TSan release edge to swift_task_enqueueGlobal()

Move the TSan release edge from `swift_task_create_commonImpl()` to
`swift_task_enqueueGlobalImpl()`.  Task creation itself is not an event
that needs synchronization, but rather that task creation "happens
before" execution of that task on another thread.

This edge is usually added when the task is scheduled via
`swift_task_enqueue()` (which then usually calls
`swift_task_enqueueGlobal()`).  However, not all task scheduling goes
through the `swift_task_enqueue()` funnel as some places call the more
specific `swift_task_enqueueGlobal()` directly.  So let's annotate this
function (duplicate edges aren't harmful) to ensure we cover all
schedule events, including newly-created tasks (our original problem
here).

rdar://78932849

Co-authored-by: Julian Lettner <[email protected]>
DougGregor pushed a commit that referenced this pull request Oct 31, 2021
DougGregor pushed a commit that referenced this pull request Dec 4, 2021
Case #1. Literal zero = natural alignment
   %1 = integer_literal $Builtin.Int64, 0
   %2 = builtin "uncheckedAssertAlignment"
	(%0 : $Builtin.RawPointer, %1 : $Builtin.Int64) : $Builtin.RawPointer
   %3 = pointer_to_address %2 : $Builtin.RawPointer to [align=1] $*Int

   Erases the `pointer_to_address` `[align=]` attribute:

Case #2. Literal nonzero = forced alignment.

   %1 = integer_literal $Builtin.Int64, 16
   %2 = builtin "uncheckedAssertAlignment"
	(%0 : $Builtin.RawPointer, %1 : $Builtin.Int64) : $Builtin.RawPointer
   %3 = pointer_to_address %2 : $Builtin.RawPointer to [align=1] $*Int

   Promotes the `pointer_to_address` `[align=]` attribute to a higher value.

Case #3. Folded dynamic alignment

   %1 = builtin "alignof"<T>(%0 : $@thin T.Type) : $Builtin.Word
   %2 = builtin "uncheckedAssertAlignment"
	(%0 : $Builtin.RawPointer, %1 : $Builtin.Int64) : $Builtin.RawPointer
   %3 = pointer_to_address %2 : $Builtin.RawPointer to [align=1] $*T

   Erases the `pointer_to_address` `[align=]` attribute.
DougGregor pushed a commit that referenced this pull request Sep 22, 2023
# This is the 1st commit message:

utils: update the build-windows-toolchain.bat to extract the toolchain

Fetch a prebuilt toolchain to build the toolchain.  This is required to
enable the macro support on Windows.

# The commit message #2 will be skipped:

# build: build SwiftSyntax before the toolchain build
#
# Perform a build of Swift Syntax prior to the build of the toolchain so
# that we can enable the early swift syntax parser builds.  This is a
# prerequisite for enabling macros on Windows.

# The commit message #3 will be skipped:

# # This is a combination of 5 commits.
# # This is the 1st commit message:
#
# build: wire up the early swift-syntax build to the build
#
# This enables the early swift syntax build to get us macro support on
# Windows.
#
# # The commit message #2 will be skipped:
#
# # Update build-windows-toolchain.bat
#
# # The commit message #3 will be skipped:
#
# # Update build-windows-toolchain.bat
#
# # The commit message #4 will be skipped:
#
# # Update build-windows-toolchain.bat
#
# # The commit message swiftlang#5 will be skipped:
#
# # Update build-windows-toolchain.bat
DougGregor pushed a commit that referenced this pull request Jan 19, 2024
Co-authored-by: Karoy Lorentey <[email protected]>
DougGregor pushed a commit that referenced this pull request Apr 28, 2024
DougGregor pushed a commit that referenced this pull request Jul 15, 2024
This inserts a suitably named function into the stack trace whenever
a dynamic cast failure involves a NULL source or target type.
Very often, crash logs include backtraces with function names but
no log output; with this change, such a backtrace might look like
the following -- note `TARGET_TYPE_NULL` in the function name
here to mark the missing type information:

```
 frame #0: __pthread_kill + 8
 frame #1: pthread_kill + 288
 frame #2: abort + 128
 frame #3: swift::fatalErrorv()
 frame #4: swift::fatalError()
 frame swiftlang#5: swift_dynamicCastFailure_TARGET_TYPE_NULL()
 frame swiftlang#6: swift::swift_dynamicCastFailure()
 frame swiftlang#7: ::swift_dynamicCast()
```

Resolves rdar://130630157
DougGregor pushed a commit that referenced this pull request Dec 13, 2024
Two are fixes needed in most of the `RawSpan` and `Span` initializers. For example:

```
    let baseAddress = buffer.baseAddress
    let span = RawSpan(_unchecked: baseAddress, byteCount: buffer.count)
    // As a trivial value, 'baseAddress' does not formally depend on the
    // lifetime of 'buffer'. Make the dependence explicit.
    self = _overrideLifetime(span, borrowing: buffer)
```

Fix #1. baseAddress needs to be a variable

`span` has a lifetime dependence on `baseAddress` via its
initializer. Therefore, the lifetime of `baseAddress` needs to include the call
to `_overrideLifetime`. The override sets the lifetime dependency of its result,
not its argument. It's argument still needs to be non-escaping when it is passed
in.

Alternatives:

- Make the RawSpan initializer `@_unsafeNonescapableResult`.

  Any occurrence of `@_unsafeNonescapableResult` actually signals a bug. We never
  want to expose this annotation.

  In addition to being gross, it would totally disable enforcement of the
  initialized span. But we really don't want to side-step `_overrideLifetime`
  where it makes sense. We want the library author to explicitly indicate that
  they understand exactly which dependence is unsafe. And we do want to
  eventually expose the `_overrideLifetime` API, which needs to be well
  understood, supported, and tested.

- Add lifetime annotations to a bunch of `UnsafePointer`-family APIs so the
  compiler can see that the resulting pointer is derived from self, where self is
  an incoming `Unsafe[Buffer]Pointer`. This would create a massive lifetime
  annotation burden on the `UnsafePointer`-family APIs, which don't really have
  anything to do with lifetime dependence. It makes more sense for the author of
  `Span`-like APIs to reason about pointer lifetimes.

Fix #2. `_overrideLifetime` changes the lifetime dependency of span to be on an
incoming argument rather than a local variable.

This makes it legal to escape the function (by assigning it to self). Remember
that self is implicitly returned, so the `@lifetime(borrow buffer)` tells the
compiler that `self` is valid within `buffer`'s borrow scope.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants