I wrote about this in a reply to a learn thread, but realized I should really post it here on its own.

A user posted seemingly innocuous code that caused a segmentation fault here, and also asked about it on Discord. We worked through trying to figure out the issue, and it turns out it was a memory corruption (the seg fault was deep inside the garbage collector).

But not one that I would have expected. It was an out-of-bounds access passed into a std.bitmanip.BitArray, which corrupted GC metadata, and caused a later allocation to fail. Like any reasonable array type, BitArray uses asserts to enforce bounds. And he did NOT turn off bounds checks.

So what happened? Phobos is compiled in release mode, even if your code is not. So bounds checks are disabled based on the type.

If BitArray was a template, it might have bounds checks enabled. But maybe not, if the compiler detected it was already instantiated inside Phobos and decided it didn't need to emit the code for it. This kind of Russian Roulette seems prone to cause hours of debugging when it could give you an answer instantly.

How do we fix such a thing? The easiest thing I can think of is to ship an assert-enabled version of Phobos, and use that when requested. This at least allows a quick mechanism to test your code with bounds checks (and other asserts) enabled. LDC actually already has this, see https://d.godbolt.org/z/feETE8W78 (courtesy of Max Haughton)

But this feels awkward. If you build your code with bounds checks, you would like the libraries you use to have them also. And a compiler switch isn't going to help you when you are using other libs.

Now consider that BitArray, being quite old, uses contracts to enforce its bounds checks. However, D runs the contracts inside the function itself, instead of at the call site. But this seems wrong to me, as the contracts are checking the caller's code, not the callee's.

Would it be a reasonable thing to change contracts to be called by the caller instead of the callee? Is that something that could make its way into D, such that the input checking of functions that are compiled for release still can run when you compile your code in non-release mode?

-Steve

On Monday, 18 October 2021 at 13:04:47 UTC, Steven Schveighoffer wrote:

Isn't this how contracts should work in general? You test the preconditions before calling the function, hoping that the optimizer will resolve them at compiletime?

On Monday, 18 October 2021 at 13:04:47 UTC, Steven Schveighoffer wrote:

The first step is to just have an assert-enabled version at least.

On Monday, 18 October 2021 at 13:04:47 UTC, Steven Schveighoffer wrote:

Learn from Delphi. It implement long time ago.
It always deploy library in release (default) and debug. The compiler has option to compile/use Debug Library (Use Debug DCUs). So for D, just deploy ...lib\debug and add compiler option to use it

Happy coding!

On Monday, 18 October 2021 at 13:04:47 UTC, Steven Schveighoffer wrote:

My radical idea (which I also brought up on Discord) is that we should enable all contract checks and asserts in release mode by default, and tell programmers to use debug assert(...) if they want a particular check to be removed in release builds.

Of course, if you really wanted to disable contract checks or assertions at build time, you would still be able to do so with -check=assert=off and -check=[in|out|invariant]=off. But requiring an explicit opt-in would make this much less of a footgun than it currently is.

On Monday, 18 October 2021 at 23:29:09 UTC, rikki cattermole wrote:

Nim has the same division, with the opposite default, in assert (erased in release builds) and doAssert (retained in release builds). D can simulate a bunch of different kinds of asserts by abusing dmd flags and different places that asserts can appear in - function bodies, contract bodies, unittests, and debug statements, version blocks, but what D's missing is the convention of using these asserts differently.

On Monday, 18 October 2021 at 23:37:51 UTC, jfondren wrote:

My guess is that if we changed the behavior of -release, D programmers would learn the new convention pretty quickly.

Even if they didn't, though, the change is fail-safe--it will not break any code that isn't already broken in debug builds. So as compiler changes go, this one is relatively low-risk.