D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 3407 - [tdpl] Compiling with -safe -release must keep all bound checks
Summary: [tdpl] Compiling with -safe -release must keep all bound checks
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D2
Hardware: Other Linux
: P2 normal
Assignee: No Owner
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-10-15 17:30 UTC by Andrei Alexandrescu
Modified: 2015-06-09 05:14 UTC (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Andrei Alexandrescu 2009-10-15 17:30:43 UTC
The following program completes successfully when compiled with -safe -release:

void main() {
    auto a = new int[2];
    auto b = a[3];
}

When the -safe flag is present, all bound checks must be present.
Comment 1 Don 2009-10-30 06:45:47 UTC
I don't understand the reasoning behind this. The primary reason you use -release is to turn off bounds checking, because bounds checking is *really* slow.
This change would basically make -safe and -release incompatible in practice: why on earth would you use both?
Up to now, -safe has been entirely about compile-time checks. Here, you're making it insert runtime checks as well, so that compile-time safety incurs a run-time penalty. I don't like that at all.
Comment 2 Andrei Alexandrescu 2009-10-30 07:55:56 UTC
(In reply to comment #1)
> I don't understand the reasoning behind this. The primary reason you use
> -release is to turn off bounds checking, because bounds checking is *really*
> slow.
> This change would basically make -safe and -release incompatible in practice:
> why on earth would you use both?
> Up to now, -safe has been entirely about compile-time checks. Here, you're
> making it insert runtime checks as well, so that compile-time safety incurs a
> run-time penalty. I don't like that at all.

To me the charters of -safe and -release are different:

-safe: "Do whatever the hell it takes to make sure my program never has a memory error"

-release: "I've tested and debugged my programs, eliminate runtime design checks"

Compiling with -safe implies array bounds checks stay, come hell or high water. Compiling with -release means the contracts and assert are removed. Those can't cause memory errors. Only non-safe builds will remove array bounds checks.
Comment 3 Don 2009-10-30 08:38:34 UTC
> -safe: "Do whatever the hell it takes to make sure my program never has a
> memory error"
> 
> -release: "I've tested and debugged my programs, eliminate runtime design
> checks"
> 
> Compiling with -safe implies array bounds checks stay, come hell or high water.
> Compiling with -release means the contracts and assert are removed. Those can't
> cause memory errors. Only non-safe builds will remove array bounds checks.

The current -safe guarantees that you never have to debug memory corruption problems, which can be notoriously difficult to track down. I would use it 100% of the time. But I'd always have bounds checking off in production code, it hurts performance too much.

Actually, I think what this issue is, is that you need an option to have contracts and asserts turned off, while leaving bounds checking on.

  compile   runtime
   safe    bounds contracts
    Y       Y          Y
    Y       N          N   <---- I want to keep this option
    Y       Y          N   <---- You're saying we need this option

I think -release is a misnomer, and should be split in two.
(Actually I think since we have module(system), we'll probably be able make -safe mandatory, eventually).
Comment 4 Andrei Alexandrescu 2009-10-30 08:51:31 UTC
(In reply to comment #3)
> > -safe: "Do whatever the hell it takes to make sure my program never has a
> > memory error"
> > 
> > -release: "I've tested and debugged my programs, eliminate runtime design
> > checks"
> > 
> > Compiling with -safe implies array bounds checks stay, come hell or high water.
> > Compiling with -release means the contracts and assert are removed. Those can't
> > cause memory errors. Only non-safe builds will remove array bounds checks.
> 
> The current -safe guarantees that you never have to debug memory corruption
> problems, which can be notoriously difficult to track down.

Yes. Well, that's the intent - the feature is not finished.

> I would use it 100%
> of the time. But I'd always have bounds checking off in production code, it
> hurts performance too much.

What you want is impossible as far as we know. Safety and no bounds checking cannot be done with the current technology, or at least not in a way that's not onerous (e.g. a 12-hour analysis of a 10,000 lines program).

> Actually, I think what this issue is, is that you need an option to have
> contracts and asserts turned off, while leaving bounds checking on.
> 
>   compile   runtime
>    safe    bounds contracts
>     Y       Y          Y
>     Y       N          N   <---- I want to keep this option
>     Y       Y          N   <---- You're saying we need this option
> 
> I think -release is a misnomer, and should be split in two.

I agree. You may want to make a proposal, and quick. At any rate, today "-safe" is the best-defined notion we have and I think it would be difficult to convince people that -safe and no bounds checking make sense together. 

BTW, there's nothing special about compile-time vs. run-time in "safe". Memory safety asks for a combo solution. Oftentimes a runtime check could be hoisted to compilation time.

> (Actually I think since we have module(system), we'll probably be able make
> -safe mandatory, eventually).

That would be great!
Comment 5 David Simcha 2009-10-30 10:00:46 UTC
http://en.wikipedia.org/wiki/Bounds-checking_elimination

Not sure if DMD does any of this yet, or if the implementation of bounds checking is naive.  It may eventually be possible to optimize bounds checking enough that it can be left on in all but the most performance critical code.  After all, Java gets away w/ this somehow.
Comment 6 Leandro Lucarella 2009-10-30 15:23:19 UTC
LDC already have all the checks splat in different options:

$ ldc --help
[...]
  -enable-asserts                              - (*) Enable assertions
  -enable-boundscheck                          - (*) Enable array bounds checks
  -enable-contracts                            - (*) Enable function pre- and post-conditions
  -enable-invariants                           - (*) Enable invariants
  -enable-postconditions                       - (*) Enable function postconditions
  -enable-preconditions                        - (*) Enable function preconditions
  -enable-inlining                             - (*) Enable function inlining in -O<N>
[...]
Options marked with (*) also have a -disable-FOO variant with inverted
meaning.
$ 

It will be very nice if this switches are included in the reference implementation too.
Comment 7 Andrei Alexandrescu 2009-10-30 17:21:41 UTC
(In reply to comment #6)
> LDC already have all the checks splat in different options:
> 
> $ ldc --help
> [...]
>   -enable-asserts                              - (*) Enable assertions
>   -enable-boundscheck                          - (*) Enable array bounds checks
>   -enable-contracts                            - (*) Enable function pre- and
> post-conditions
>   -enable-invariants                           - (*) Enable invariants
>   -enable-postconditions                       - (*) Enable function
> postconditions
>   -enable-preconditions                        - (*) Enable function
> preconditions
>   -enable-inlining                             - (*) Enable function inlining
> in -O<N>
> [...]
> Options marked with (*) also have a -disable-FOO variant with inverted
> meaning.
> $ 
> 
> It will be very nice if this switches are included in the reference
> implementation too.

I think this is too much configurability that does not reflect principles, only mechanism.

DbC means assert, precondition, postcondition, and invariant are enabled. I don't think it's wise to enable or disable those independently.
Comment 8 Leandro Lucarella 2009-10-30 17:45:01 UTC
(In reply to comment #7)
> (In reply to comment #6)
> > LDC already have all the checks splat in different options:
> > 
> > $ ldc --help
> > [...]
> >   -enable-asserts                              - (*) Enable assertions
> >   -enable-boundscheck                          - (*) Enable array bounds checks
> >   -enable-contracts                            - (*) Enable function pre- and
> > post-conditions
> >   -enable-invariants                           - (*) Enable invariants
> >   -enable-postconditions                       - (*) Enable function
> > postconditions
> >   -enable-preconditions                        - (*) Enable function
> > preconditions
> >   -enable-inlining                             - (*) Enable function inlining
> > in -O<N>
> > [...]
> > Options marked with (*) also have a -disable-FOO variant with inverted
> > meaning.
> > $ 
> > 
> > It will be very nice if this switches are included in the reference
> > implementation too.
> 
> I think this is too much configurability that does not reflect principles, only
> mechanism.
> 
> DbC means assert, precondition, postcondition, and invariant are enabled. I
> don't think it's wise to enable or disable those independently.

I don't agree about not providing the option because "it's not wise". Let the user decide what is wise or not.

That said, I can agree about having that kind of granularity might be not very useful (there are very very few situations where you might want to enable preconditions and not postconditions, for example).
Comment 9 anonymous4 2009-11-06 03:09:21 UTC
I usually do (unimportant) debug asserts as debug asserts so they disappear if no -debug switch was supplied... well I don't use contracts. And use simple asserts for critical sequrity checks, I basically see no point in -release switch. "I tested and debugged my programs" is too brave assertion for a programmer.
Comment 10 Walter Bright 2009-11-21 02:53:58 UTC
Will remove -safe switch and add -noboundscheck switch. @safe functions always get bounds checked unless -noboundscheck is on. @trusted and @system functions only get bounds checked when -release and -noboundscheck are off.
Comment 11 Leandro Lucarella 2009-11-21 14:49:14 UTC
Related SVN commit: http://www.dsource.org/projects/dmd/changeset/258
Comment 12 Andrei Alexandrescu 2009-11-22 17:33:37 UTC
(In reply to comment #10)
> Will remove -safe switch and add -noboundscheck switch. @safe functions always
> get bounds checked unless -noboundscheck is on. @trusted and @system functions
> only get bounds checked when -release and -noboundscheck are off.

Would -no-bounds-check be more readable? Just asking.
Comment 13 Walter Bright 2009-11-22 19:21:29 UTC
To me that always looks like a mistake where there are multiple switches someone forgot to put spaces in between.
Comment 14 Walter Bright 2009-12-06 00:53:25 UTC
Fixed dmd 2.037