If you compile and run the following code, what happens?

void main()
{
   assert(0, "error message");
}

answer: it depends. On OSX, if you compile this like so:

dmd testassert.d
./testassert

You get this message + stack trace:

core.exception.AssertError@testassert.d(3): error message

Not bad. But assert(0) is special in that it is always enabled, even in release mode. So let's try that:

dmd -release testassert.d
./testassert
Segmentation fault: 11

WAT. The explanation is, assert(0) is translated in release mode to a HLT instruction. on X86, this results in a segfault. But a seg fault is tremendously less useful. Let's say you are running a 10k line program, and you see this. Compared with seeing the assert message and stack trace, this is going to cause hours of extra debugging.

Why do we do this? I'm really not sure. Note that "error message" is essentially not used if we have a seg fault. Throwing an assert error shouldn't cause any issues with stack unwinding performance, since this can be done inside a nothrow function. And when you throw an AssertError, it shouldn't be caught anyway.

Why can't assert(0) throw an assert error in release mode instead of segfaulting? What would it cost to do this instead?

-Steve

On Monday, 3 August 2015 at 14:34:52 UTC, Steven Schveighoffer wrote:
> Why do we do this?

Because all asserts must be completely removed in -release

Yet assert(0) effectively mean "unreachable code" (it is actually defined that way in spec) and thus it is possible to ensure extra "free" bit of safety by crashing the app.

On 8/3/15 11:18 AM, Dicebot wrote:
> On Monday, 3 August 2015 at 14:34:52 UTC, Steven Schveighoffer wrote:
>> Why do we do this?
>
> Because all asserts must be completely removed in -release

1. They aren't removed, they are replaced with a nearly useless segfault.
2. If we are going to put something in there instead of "assert", why not just throw an error?

Effectively:

assert(0, msg)

becomes a fancy way of writing (in any mode, release or otherwise):

throw new AssertError(msg);

This is actually the way I thought it was done.

-Steve

On Monday, 3 August 2015 at 15:50:56 UTC, Steven Schveighoffer wrote:
> On 8/3/15 11:18 AM, Dicebot wrote:
>> On Monday, 3 August 2015 at 14:34:52 UTC, Steven Schveighoffer wrote:
>>> Why do we do this?
>>
>> Because all asserts must be completely removed in -release
>
> 1. They aren't removed, they are replaced with a nearly useless segfault.
> 2. If we are going to put something in there instead of "assert", why not just throw an error?
>
> Effectively:
>
> assert(0, msg)
>
> becomes a fancy way of writing (in any mode, release or otherwise):
>
> throw new AssertError(msg);
>
> This is actually the way I thought it was done.
>
> -Steve

Now, they are completely removed. There is effectively no AssertError present in -release (it is defined but compiler is free to assume it never happens). I'd expect any reasonable compiler to not even emit stack unwinding code for functions with assert(0) (and no other throwables are present).

assert(0) is effectively same as gcc __builtin_unreachable with all consequences for optimization - with only difference that latter won't even insert HLT but just continue executing corrupted program.

General advice  - simply don't ever use -release unless you are _very_ sure about program correctness (to the point of 100% test coverage and previous successful debug runs)

On 8/3/15 11:57 AM, Dicebot wrote:
> On Monday, 3 August 2015 at 15:50:56 UTC, Steven Schveighoffer wrote:
>> On 8/3/15 11:18 AM, Dicebot wrote:
>>> On Monday, 3 August 2015 at 14:34:52 UTC, Steven Schveighoffer wrote:
>>>> Why do we do this?
>>>
>>> Because all asserts must be completely removed in -release
>>
>> 1. They aren't removed, they are replaced with a nearly useless segfault.
>> 2. If we are going to put something in there instead of "assert", why
>> not just throw an error?
>>
>> Effectively:
>>
>> assert(0, msg)
>>
>> becomes a fancy way of writing (in any mode, release or otherwise):
>>
>> throw new AssertError(msg);
>>
>> This is actually the way I thought it was done.
>>
> Now, they are completely removed. There is effectively no AssertError
> present in -release (it is defined but compiler is free to assume it
> never happens). I'd expect any reasonable compiler to not even emit
> stack unwinding code for functions with assert(0) (and no other
> throwables are present).

I actually don't care if stack is unwound (it's not guaranteed by language anyway). Just segfault is not useful at all to anyone who doesn't have core dump enabled, and even if they do enable it, it's not trivial to get at the real cause of the error. I'd settle for calling:

__onAssertZero(__FILE__, __LINE__, message);

Which can do whatever we want, print a message, do HLT, throw AssertError, etc.

> assert(0) is effectively same as gcc __builtin_unreachable with all
> consequences for optimization - with only difference that latter won't
> even insert HLT but just continue executing corrupted program.

No reason this would change. Compiler can still consider this unreachable code for optimization purposes.

-Steve

On 8/3/15 11:59 AM, Dicebot wrote:
> General advice  - simply don't ever use -release unless you are _very_
> sure about program correctness (to the point of 100% test coverage and
> previous successful debug runs)

So in other words, only release code that has no bugs. Got it ;)

-Steve

On 8/3/2015 8:50 AM, Steven Schveighoffer wrote:
> 1. They aren't removed, they are replaced with a nearly useless segfault.

Not useless at all:

1. The program does not continue running after it has failed. (Please, let's not restart that debate.
2. Running it under a debugger, the location of the fault will be identified.


> 2. If we are going to put something in there instead of "assert", why not just
> throw an error?

Because they are expensive.

To keep asserts in all their glory in released code, do not use the -release switch.

On 8/3/15 6:54 PM, Walter Bright wrote:
> On 8/3/2015 8:50 AM, Steven Schveighoffer wrote:
>> 1. They aren't removed, they are replaced with a nearly useless segfault.
>
> Not useless at all:
>
> 1. The program does not continue running after it has failed. (Please,
> let's not restart that debate.

You can run some code that is fresh, i.e. don't *continue* running failing code, but it's ok, for example, to run a signal handler, or call a pure function. My thought is that you should print the file location and exit (perhaps with segfault).

> 2. Running it under a debugger, the location of the fault will be
> identified.

Yeah, the real problem is when you are not running under a debugger. If you have an intermittent bug, or one that is in the field, a stack trace of the problem is worth 100x more than a segfault and trying to talk a customer through setting up a debugger, or even setting up their system to do a core dump for you to debug. They aren't always interested in that enough to care.

But also, a segfault has a specific meaning. It means, you tried to access memory you don't have access to. This clearly is not that, and it sends the wrong message -- memory corruption. This is not memory corruption, it's a program error.

>> 2. If we are going to put something in there instead of "assert", why
>> not just
>> throw an error?
>
> Because they are expensive.

They are expensive when? 2 microseconds before the program ends? At that point, I'd rather you spend as much resources telling me what went wrong as possible. More info the better.

Now, if it's expensive to include the throw vs. not including it (I don't know), then that's a fair point. If it means an extra few instructions to set up the throw (which isn't run at all when the assert(0) line isn't run), that's not convincing.

> To keep asserts in all their glory in released code, do not use the
> -release switch.

OK, this brings up another debate. The thing that triggered all this is an issue with core.time, see issue https://issues.dlang.org/show_bug.cgi?id=14863

Essentially, we wrote code to get all the clock information at startup on a posix system that supports clock_gettime, which is immutable and can be read easily at startup. However, how we handled the case where a clock could not be fetched is to assert(0, "don't support clock " ~ clockname).

The clear expectation was that the message will be printed (along with file/line number), and we can go fix the issue.

On Linux 2.6.32 - a supported LTS release I guess - one of these clocks is not supported. This means running a simple "hello world" program crashes at startup in a segfault, not a nice informative message.

So what is the right answer here? We want an assert to trigger for this, but the only assert that stays in is assert(0). However, that's useless if all someone sees is "segfuault". "some message" never gets printed, because druntime is compiled in release mode. I'm actually quite thrilled that someone figured this all out -- one person analyzed his core dump to narrow down the function, and happened to include his linux kernel version, and another person had the arcane knowledge that some clock wasn't supported in that version.

Is there a better mechanism we should be using in druntime that is clearly going to be compiled in release mode? Should we just throw an assert error directly? Clearly the code is not "corrupted" at this point, it's just an environmental issue. But we don't want to continue execution. What is the right call?

At the very least, assert(0, "message") should be a compiler error, the message is unused information.

-Steve

On 08/03/2015 04:57 PM, Steven Schveighoffer wrote:

> At the very least, assert(0, "message") should be a compiler error, the
> message is unused information.

Agreed.

How about dumping the message to stderr as a best effort if the message is a literal? Hm... On the other hand, undefined behavior means that even trying that can cause harm like radiating a human with too much medical radiation. :(

Perhaps, if it is a complicated expression like calling format(), then it should be an error?

Ali