Unexpected Assertion Behaviour - Radar 21826180

Apologies for the lack of posts, full time work and family time among other things have been keeping me from finishing off Swift performance stuff. The new Swift 2.0 features look really good but again I haven't got stuck in yet. This is a quick post about an issue concerning me since I noticed it when reading Erica Sadun's post about assertions. I actually thought it couldn't be correct when I first read it so unexpected was it but when I checked the documentation it matched exactly what Erica said.

Assertions and Unchecked Builds

I use assertions for things that shouldn't happen but sometimes might. For example corrupt or unprocessable network data where I write code to safely handle the case but I also want to know instantly and start debugging the issue (whether it is in the app or the server). If you do the same and have assertions which could happen then you must not build in -Ounchecked (or in Swift 2.0 with Disable Safety Checks enabled) otherwise in the event of assertions not being true you will be in undefined behaviour.

[UPDATE - Additional]

This can be demonstrated with the following code which can run with any version of Swift (I've tested 1.2 and 2.0 that is Xcode 6.4 and 7.0 beta3). The resulting return values when run with -Ounchecked or Disable Safety Checks show that the compiler assumes the assertionFailure will not be called and it optimises whole blocks of code away.

Note that I've added some complexity (e.g. the random number) to try to ensure that the function isn't entirely optimised away. The use of addition of numbers is to avoid any function calls or other added complexities if I needed to drop into the assembly.

In my testing of unchecked builds assert(false) does get handled as a no-op (the behaviour I want) although the documentation suggests it should be undefined but paths with assertionFailure on them are optimised away (as documented but not as expedted.

How Unexpected is this? [Added in Update]

Based on some discussions on Twitter there is some surprise that asserts and assertionFailure paths can be removed by the compiler rather than just being no operations. In other languages that I'm aware of asserts are halts in debug builds and no-ops otherwise. I'm not aware of any where build settings can make the asserts change the behaviour of surrounding code. The context which many people bring to Swift includes expectations of behavour of assert that are broken in unchecked builds.

Why don't I have a problem with precondition? [Added in Update]

I'm OK with the behaviour of the precondition and preconditionFailure functions which is similar in unchecked mode for two reasons.

Firstly precondition is a stronger statement that the condition will not occur, they halt in release builds rather than no-op so they should only be used when the developer believes the condition can never happen. Assertions can be used for things that shouldn't happen

Secondly I don't have as strong an instinct about how they behave from other languages as I do with assert so I would be more likely to properly study the documentation than with assert where I think I know what it will do. I suspect others are similar.

Preconditions are as much documentation as a command to be run and using them as compiler hints in unsafe builds feels OK to me.

General Advice about Build Settings

My general advice would be not to use the unsafe build settings (-Ounchecked in Swift 1 and SWIFT_DISABLE_SAFETY_CHECKS anyway as the performance gains are small compared with normal release (-O and especially with whole module optimisation). I haven't done much performance testing on Swift 2.0 yet. But the risks of undefined behaviour in the event of error cases and limited gains make the safe builds the choice for me except in outright performance experiments. If you do use the unchecked build modes try to limit it to performance critical code compiled as a separate module from any code handling network or other untrusted data.

Radar 21826180:

The security risk is to potential apps written and published using builds with Disable Safety Checks enabled.

I am completely happy with the precondition behaviour which is a appropriate however I’m concerned about the assert and assertionFailure behaviour with safety checks disabled.

Currently the docs say (note that these use the out of date Swift 1 build terminology which I've raised a separate radar for:21825875):

/// Traditional C-style assert with an optional message.

/// * In -Ounchecked builds, `condition` is not evaluated, but the
///   optimizer may assume that it *would* evaluate to `true`. Failure
///   to satisfy that assumption in -Ounchecked builds is a serious
///   programming error.

I use asserts for events that are unexpected and I want to be strongly and unavoidably informed about as a developer. Some of these could actually happen in production and in many cases I write code to handle the event even with the assertion in place. One example is that I place them in some areas handling incoming data (network or file) where in debug I want to be violently informed but in Release I will want to safely handle (possibly by ignoring the data). To be clear on these occasions an error has occurred but it is possibly outside the app and in the data source (possibly a remote server or device controlled by a third party).

Particularly in these cases handling external data if the build was made with disable safety checks not just not continuing on assertions but the compiler assuming the assertion condition is true we risk entering undefined behaviour while processing insecure data from 3rd parties and I’m very uncomfortable with that scenario.

I request that for the Disable Safety Checks builds the assertion behaviour should be the same as for Release builds. No check but no assumption of result, simple no-op.

If the user does want the current behaviour they could combine assertions and preconditions (writing their own function performing both if they wished).


2 responses
I think you might be over describing what is happening. The compiler *can* optimize stuff away. However, depending on the code, you might get a crash if the code path goes that direction. See: https://gist.github.com/owensd/0724d2e66f49c682... But yeah, this behavior is really weird. I don't understand how it is safe for the compiler to simply ignore the branches above as the `assertionFailure()` is not marked as `@noreturn` anymore. That, regardless of all the other shenanigans, seems like a serious bug to me.
It is definitely unsafe but I don't think it is weird and it is actually documented. The compiler is free to make whatever optimisation and changes it likes provided that the behaviour on paths with no invalid assertions are changed. The compiler in unchecked / disable safety checks mode is free to crash, execute arbitrary code or continue with a safe no op on any path with an assertionFailure, assert(false), preconditionFailure or precondition(false). The documentation is reasonably clear that it is a "serious programming error" and makes no statements about the behaviour. -Joseph