[stdlib] relax stride check in UnsafePointer.withMemoryRebound

[glessard]

• The new stride check in UnsafePointer.withMemoryRebound makes less sense when rebinding memory for a single element.
• This skips the stride-matching portion of the _debugPrecondition in withMemoryRebound when count == 1.

Resolves SR-15912.

[glessard]

@swift-ci please test

[atrick]

Thank you

[glessard]

@swift-ci please test

[finagolfin]

@glessard, did you try building and testing NIO with this fix? I tried applying it to my Android CI with the Feb. 22 snapshot, but I still see it trap on Android x86_64. I just tried to reproduce on Fedora x86_64 by checking out the Feb. 22 snapshot and rebuilding the stdlib with this fix applied:

./swift/utils/build-script -RA --build-swift-static-stdlib -j9 --build-swift-tools=0  --install-swift --native-swift-tools-path=/home/butta/swift-DEVELOPMENT-SNAPSHOT-2022-02-22-a-centos8/usr/bin --native-clang-tools-path=/home/butta/swift-DEVELOPMENT-SNAPSHOT-2022-02-22-a-centos8/usr/bin --install-destdir=/home/butta/swift-DEVELOPMENT-SNAPSHOT-2022-02-22-a-centos8/ --skip-build-cmark --skip-build-llvm

That will overwrite the broken stdlib with your fixed one, but it still traps:

.build/x86_64-unknown-linux-gnu/debug/swift-nioPackageTests.xctest NIOPosixTests.BootstrapTest/testBootstrapsCallInitializersOnCorrectEventLoop                                    
Test Suite 'Selected tests' started at 2022-02-27 16:24:01.379
Test Suite 'BootstrapTest' started at 2022-02-27 16:24:01.382
Test Case 'BootstrapTest.testBootstrapsCallInitializersOnCorrectEventLoop' started at 2022-02-27 16:24:01.382
Swift/UnsafePointer.swift:1003: Fatal error

lldb shows NIO failing here on linux x86_64, let me know if you can reproduce.

[stephentyrone]

@buttaface because these functions are _alwaysEmitIntoClient, rebuilding the standard library won't have any effect. You have to rebuild the dependent project and tests.

[finagolfin]

I did, I just left those steps out in my last comment. After rebuilding the stdlib, I run swift package clean and /home/butta/swift-DEVELOPMENT-SNAPSHOT-2022-02-22-a-centos8/usr/bin/swift test on NIO, saw the failing test again, then offered the single test command above.

Are you unable to reproduce?

[glessard]

So far, I cannot reproduce. (my linux vm has not been available this weekend). I also don't understand how the assertion would trip since count == 1. Do the tests run correctly on your built toolchain? The test I added is an approximation of this situation.

[finagolfin]

OK, I will investigate further and let you know. I didn't try running your stdlib test, will try it now.

[finagolfin]

Looked into this more, this new test passes on linux, but maybe _debugPrecondition() doesn't work the way you expect on linux. I tried reverting your earlier pull #39529 that added this condition and that got the NIO tests working again, as did rewriting this pull in this equivalent way:

diff --git a/stdlib/public/core/UnsafePointer.swift b/stdlib/public/core/UnsafePointer.swift
index 558dc3a88a2..3f2221d92e7 100644
--- a/stdlib/public/core/UnsafePointer.swift
+++ b/stdlib/public/core/UnsafePointer.swift
@@ -314,11 +314,13 @@ public struct UnsafePointer<Pointee>: _Pointer {
     _ body: (_ pointer: UnsafePointer<T>) throws -> Result
   ) rethrows -> Result {
     _debugPrecondition(
-      Int(bitPattern: .init(_rawValue)) & (MemoryLayout<T>.alignment-1) == 0 &&
-      MemoryLayout<Pointee>.stride > MemoryLayout<T>.stride
-      ? MemoryLayout<Pointee>.stride % MemoryLayout<T>.stride == 0
-      : MemoryLayout<T>.stride % MemoryLayout<Pointee>.stride == 0
-    )
+      Int(bitPattern: .init(_rawValue)) & (MemoryLayout<T>.alignment-1) == 0) //&&
+    if  ( count != 1 ) {
+    _debugPrecondition(
+        MemoryLayout<Pointee>.stride > MemoryLayout<T>.stride
+        ? MemoryLayout<Pointee>.stride % MemoryLayout<T>.stride == 0
+        : MemoryLayout<T>.stride % MemoryLayout<Pointee>.stride == 0 )
+    }
     let binding = Builtin.bindMemory(_rawValue, count._builtinWordValue, T.self)
     defer { Builtin.rebindMemory(_rawValue, binding) }
     return try body(.init(_rawValue))
@@ -1000,11 +1002,13 @@ public struct UnsafeMutablePointer<Pointee>: _Pointer {
     _ body: (_ pointer: UnsafeMutablePointer<T>) throws -> Result
   ) rethrows -> Result {
     _debugPrecondition(
-      Int(bitPattern: .init(_rawValue)) & (MemoryLayout<T>.alignment-1) == 0 &&
-      MemoryLayout<Pointee>.stride > MemoryLayout<T>.stride
-      ? MemoryLayout<Pointee>.stride % MemoryLayout<T>.stride == 0
-      : MemoryLayout<T>.stride % MemoryLayout<Pointee>.stride == 0
-    )
+      Int(bitPattern: .init(_rawValue)) & (MemoryLayout<T>.alignment-1) == 0 )//&&
+      if ( count != 1 ){
+    _debugPrecondition(
+        MemoryLayout<Pointee>.stride > MemoryLayout<T>.stride
+        ? MemoryLayout<Pointee>.stride % MemoryLayout<T>.stride == 0
+        : MemoryLayout<T>.stride % MemoryLayout<Pointee>.stride == 0 )
+    }
     let binding = Builtin.bindMemory(_rawValue, count._builtinWordValue, T.self)
     defer { Builtin.rebindMemory(_rawValue, binding) }
     return try body(.init(_rawValue))

The fact this doesn't break suggests to me your logical expression isn't being evaluated as you'd expect, ie it still checks the stride if the count is 1 and errors when that last condition is false. Maybe this test doesn't trigger that somehow, someone's going to have to check the codegen on linux.

[stephentyrone]

maybe _debugPrecondition() doesn't work the way you expect

This is not the issue; the issue is that the ternary operator has lower precedence than ||. Guillaume, please add some parentheses.

[stephentyrone]

@buttaface please give #41586 a spin

[finagolfin]

Yep, that fixed it.

[glessard]

Added a fix to the botched test in #41596