[swift-evolution] Asserts should not cause undefined behaviour

[Kevin Ballard]

On Fri, Jan 1, 2016, at 11:58 PM, Kevin Ballard wrote:
> On Fri, Jan 1, 2016, at 11:25 PM, Chris Lattner via swift-evolution wrote:
> > > On Dec 31, 2015, at 1:56 PM, Dave Abrahams <dabrahams at apple.com> wrote:
> > >>> 2) Adding asserts to code should not make the code more dangerous whatever the build. Assuming the truth of the assert may lead to runtime safety checks being skipped and undefined behaviour when a no-op would be a safe behaviour.
> > >> 
> > >> This only affects code built with -Ounchecked, which is definitely not a safe mode to build your code.  The intention of this mode is that you can use it to get a performance boost, if you believe your code to be sufficiently tested.  This mode, which isn’t the default in any way, intentionally takes the guard rails off to get better performance.
> > >> 
> > >> If you don’t like that model, don’t use this mode.
> > > 
> > > Let’s just consider -O; I think I understand Joseph’s objection here, and it seems valid.
> > 
> > Ah, good point.
> > 
> > > Normally in -O, we say that if you stay in the “safe subset” of Swift code, you never get undefined behavior, even if there’s a bug in your code.  You might get *unpredictable* behavior of course, but presumably guaranteeing no undefined behavior rules out large classes of problems, including many security holes.  Now suppose you decide to be responsible and add some asserts to help you catch bugs during development.  Hopefully they help you catch all the bugs, but what if they don’t?  All of a sudden, if you still have a bug when you ship, you now have undefined behavior.  As much as I’m a fan of assertions having optimization benefits, It seems a little perverse that using them could make shipping code less secure.
> > 
> > Yes, I agree.  -O should not imply undefined behavior in the case of an assert() predicate being dynamically false.
> > 
> > It sounds like we just need a documentation update here?
> 
> assert() and assertionFailure() are already explicitly documented as doing nothing in -O builds, and only making the assumption that the condition is always true/function is never called in -Ounchecked builds.
> 
> As far as I can tell, the only actual inaccuracy in the documentation is the fact that assert() claims that the optimizer may assume the condition is always true in -Ounchecked when it doesn't seem to actually do that (because it doesn't do anything at all when not using -Onone). Both assert() and assertionFailure() do nothing in -O, which matches the documentation.

Having said that, it occurs to me that maybe the optimizer is actually specially detecting the call to assert() under -Ounchecked, but from the source of assert() there's nothing there to indicate that it actually makes any assumptions under -Ounchecked.

-Kevin Ballard