mattgallagher / CwlPreconditionTesting

A Mach exception handler that allows Swift precondition failures to be caught and tested.
ISC License
175 stars 46 forks source link

alternative to `setjmp` under new Swift releases #7

Closed aaroncrespo closed 7 years ago

aaroncrespo commented 7 years ago

Swift 3.1 (Xcode 8.3beta2) has a small source breaking change that unfortunately affects this project: setjmp is no longer available. Found it when trying to compile Quick and Nimble

modocache commented 7 years ago

Ah, this was my change: https://github.com/apple/swift/pull/6513

As stated on the pull request, functions that return twice were never really supported in Swift; the compiler and runtime weren't capable of making the guarantees you'd expect from Swift: guaranteed variable initialization and deinitialization, for example.

mattgallagher commented 7 years ago

This code is inherently doing things that Swift doesn't support (catching precondition failures). The fact that it will leak memory and potentially create other problems on the stack is a known issue – that's why this code is for testing only.

I don't know what the fix here will be but it's likely to be scurrilous and dirty.

modocache commented 7 years ago

In light of the discussion on the original pull request, as to whether this was to be considered a source-breaking change or not, I'd be interested to hear what the Swift team thinks. What do you think of tagging a few of them on this thread?

aaroncrespo commented 7 years ago

@jrose-apple, @jckarter thoughts on any alternatives? It's probably okay to say that we shouldn't be doing this even in code that is only run in support of testing.

Just tagging in light of

your code should compile in 3.1 if it compiles in 3.0 --- paraphrasing.

jrose-apple commented 7 years ago

I'm not sure that goal ever applied to code that we considered to not work. You can always hack around these things with a tiny static inline C function, though, and the result is as unsupported as anything else you can do in C that violates Swift's rules.

jckarter commented 7 years ago

A more principled alternative, if it's possible, might be to run the code you want to longjmp out of in its own thread, so that its context isn't interwoven with the outer code's.

jckarter commented 7 years ago

e.g.:

import Darwin

func threadsetjmp(_ body: () -> Int) -> Int {
  return withoutActuallyEscaping(body) { body in
    var body2 = body
    return withUnsafeMutableBytes(of: &body2) { rawBody in

      var thread: pthread_t?
      let errno = pthread_create(&thread, nil, { rawContext in
        let body = rawContext.load(as: (() -> Int).self)
        let result = body()
        return UnsafeMutableRawPointer(bitPattern: result)
      }, rawBody.baseAddress)
      assert(errno == 0)

      var rawResult: UnsafeMutableRawPointer?
      let errno2 = pthread_join(thread!, &rawResult)
      assert(errno2 == 0)
      return Int(bitPattern: rawResult)
    }
  }
}

func threadlongjmp(_ result: Int) -> Never {
  // pthread_exit still leaks since Swift emits no unwind info, but doesn't
  // have the local semantics problems of setjmp/longjmp...
  pthread_exit(UnsafeMutableRawPointer(bitPattern: result))
}

print(threadsetjmp {
  threadlongjmp(1738)
})
johnno1962 commented 7 years ago

My vote is to reinstate it. The code seemed to be working so I’d resist the temptation to save people from themselves in this case.

jckarter commented 7 years ago

It's undefined behavior. We have no way to guarantee that arbitrary changes to Swift or LLVM won't break what "seems to work" with the exact versions of everything that ship today.

mattgallagher commented 7 years ago

I think I've reached a good solution. Just to let everyone know, I tried two different solutions to this problem.

The first version involved calling pthread_kill(pthread_self(), SIGILL), and from the signal handler, saving the state of the thread so that if a subsequent SIGILL is received (i.e. from a precondition failure) the stack could be restored to this stored state (minus a couple of stack frames to skip over __pthread_kill and pthread_kill).

I call this version "The Worst of All Possible Worlds". You can view it here. I think this version will haunt me in my dreams tonight.

So... I think I like the @jckarter approach much better – nuke the whole stack from orbit, it's the only way to be sure. You can view that here. It's much cleaner, doesn't make assumptions about the structure of the stack and doesn't rely on global variables either.

Given the issues with withoutActuallyEscaping (and the fact that I'd like to support Swift 3.0 for a little longer) I've changed the function to @escaping for now.

mattgallagher commented 7 years ago

As for whether or not to keep setjmp – my vote is: not unless you've got a better use case than this. I don't need it.

jckarter commented 7 years ago

@mattgallagher What issues with withoutActuallyEscaping are you referring to?

mattgallagher commented 7 years ago

@jckarter Sorry, I think I misread the "FIXME" comment in your code. I thought you were talking about a bug of some kind associated with the param passed through withoutActuallyEscaping. Nevermind.

In any case, I'll wait until after Swift 3.1 is part of Xcode non-beta before incorporating withoutActuallyEscaping

jckarter commented 7 years ago

Sounds good, just wanted to make sure there wasn't a latent bug. The FIXME refers to a miscompile in the pass-inout-by-pointer of a function, nothing specific to withoutActuallyEscaping.

jckarter commented 7 years ago

I totally misdiagnosed the problem—the real problem was that pthread_create escaped the address of &body2 to the spawned thread. Not even compiler nerds can get lifetimes right, I guess. It should be:

func threadsetjmp(_ body: () -> Int) -> Int {
  return withoutActuallyEscaping(body) { body in
    var body2 = body
    return withUnsafeMutableBytes(of: &body2) { rawBody in

      var thread: pthread_t?
      let errno = pthread_create(&thread, nil, { rawContext in
        let body = rawContext.load(as: (() -> Int).self)
        let result = body()
        return UnsafeMutableRawPointer(bitPattern: result)
      }, rawBody.baseAddress)
      assert(errno == 0)

      var rawResult: UnsafeMutableRawPointer?
      let errno2 = pthread_join(thread!, &rawResult)
      assert(errno2 == 0)
      return Int(bitPattern: rawResult)
    }
  }
}