D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 17650 - [REG v2.075.0 b1-b4] std.getopt range violation
Summary: [REG v2.075.0 b1-b4] std.getopt range violation
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: phobos (show other issues)
Version: D2
Hardware: x86 Mac OS X
: P1 regression
Assignee: No Owner
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-07-14 07:31 UTC by Jon Degenhardt
Modified: 2018-01-05 13:30 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Jon Degenhardt 2017-07-14 07:31:34 UTC
The unit test below passes in 2.074.1 but fails in 2.075.0 beta-1 and beta-4

===== getopt_test.d =====
import std.getopt;

unittest // Dashes
{
    auto args = ["program", "-m", "-5", "-n", "-50", "-c", "-"];

    int m;
    int n;
    char c;

    getopt(
        args,
        "m|mm", "integer", &m,
        "n|nn", "integer", &n,
        "c|cc", "character", &c,
        );

    assert(m == -5);
    assert(n == -50);
    assert(c == '-');
}

(The above adopted from tsv utilities unit tests here: https://github.com/eBay/tsv-utils-dlang/blob/master/common/src/getopt_inorder.d#L302)

With 2.074.1 (succeeds):

$ ./dmd2.074.1  --version
DMD64 D Compiler v2.074.1
Copyright (c) 1999-2017 by Digital Mars written by Walter Bright

$ ./dmd2.074.1  -unittest -main -run getopt_test.d
$

With 2.075.0 beta-4 (error):

$ dmd --version
DMD64 D Compiler v2.075.0-b4
Copyright (c) 1999-2017 by Digital Mars written by Walter Bright

$ dmd -unittest -main -run getopt_test.d
core.exception.RangeError@std/getopt.d(1112): Range violation
----------------
4   dmd_runceaLDU                       0x000000010f02a06a _d_arrayboundsp + 110
5   dmd_runceaLDU                       0x000000010f046f03 @safe bool std.getopt.optMatch(immutable(char)[], immutable(char)[], ref immutable(char)[], std.getopt.configuration) + 639
6   dmd_runceaLDU                       0x000000010f01558d @safe bool std.getopt.handleOption!(int*).handleOption(immutable(char)[], int*, ref immutable(char)[][], ref std.getopt.configuration, bool) + 985
7   dmd_runceaLDU                       0x000000010f014a88 @safe void std.getopt.getoptImpl!(immutable(char)[], immutable(char)[], int*, immutable(char)[], immutable(char)[], int*, immutable(char)[], immutable(char)[], char*).getoptImpl(ref immutable(char)[][], ref std.getopt.configuration, ref std.getopt.GetoptResult, ref std.getopt.GetOptException, void[][immutable(char)[]], void[][immutable(char)[]], immutable(char)[], immutable(char)[], int*, immutable(char)[], immutable(char)[], int*, immutable(char)[], immutable(char)[], char*) + 1164
8   dmd_runceaLDU                       0x000000010f014349 @safe std.getopt.GetoptResult std.getopt.getopt!(immutable(char)[], immutable(char)[], int*, immutable(char)[], immutable(char)[], int*, immutable(char)[], immutable(char)[], char*).getopt(ref immutable(char)[][], immutable(char)[], immutable(char)[], int*, immutable(char)[], immutable(char)[], int*, immutable(char)[], immutable(char)[], char*) + 189
9   dmd_runceaLDU                       0x000000010f011961 void getopt_test.__unittestL3_1() + 301
10  dmd_runceaLDU                       0x000000010f011808 void getopt_test.__modtest() + 8
11  dmd_runceaLDU                       0x000000010f02a8d0 int core.runtime.runModuleUnitTests().__foreachbody2(object.ModuleInfo*) + 44
12  dmd_runceaLDU                       0x000000010f02096e int object.ModuleInfo.opApply(scope int delegate(object.ModuleInfo*)).__lambda2(immutable(object.ModuleInfo*)) + 34
13  dmd_runceaLDU                       0x000000010f042205 int rt.minfo.moduleinfos_apply(scope int delegate(immutable(object.ModuleInfo*))).__foreachbody2(ref rt.sections_osx_x86_64.SectionGroup) + 85
14  dmd_runceaLDU                       0x000000010f042190 int rt.minfo.moduleinfos_apply(scope int delegate(immutable(object.ModuleInfo*))) + 32
15  dmd_runceaLDU                       0x000000010f020945 int object.ModuleInfo.opApply(scope int delegate(object.ModuleInfo*)) + 33
16  dmd_runceaLDU                       0x000000010f02a7ba runModuleUnitTests + 126
17  dmd_runceaLDU                       0x000000010f03a9aa void rt.dmain2._d_run_main(int, char**, extern (C) int function(char[][])*).runAll() + 22
18  dmd_runceaLDU                       0x000000010f03a943 void rt.dmain2._d_run_main(int, char**, extern (C) int function(char[][])*).tryExec(scope void delegate()) + 31
19  dmd_runceaLDU                       0x000000010f03a8ae _d_run_main + 458
20  dmd_runceaLDU                       0x000000010f01182f main + 15
21  libdyld.dylib                       0x00007fffc7550234 start + 0
22  ???                                 0x0000000000000000 0x0 + 0
Comment 1 Martin Nowak 2017-07-14 13:19:35 UTC
We'll address this with 2.075.1 to not delay the release even further.
Comment 2 Vladimir Panteleev 2017-07-15 05:28:04 UTC
Introduced in https://github.com/dlang/phobos/pull/5351
Comment 3 Vladimir Panteleev 2017-07-15 05:30:51 UTC
Interesting - looking at the PR, this doesn't really seem like a regression, rather that the addition of the @safe attribute exposed an out-of-bounds array access that was always there. Feel free to reclassify, Jon.
Comment 4 Vladimir Panteleev 2017-07-15 05:35:05 UTC
Actually, I believe we do count a breakage as a regression if something breaks for the end-user, regardless of what is going on under the hood. E.g. it's possible that for supported architectures, the range violation was completely benign.
Comment 5 Jon Degenhardt 2017-07-15 06:23:27 UTC
Wow. So I introduced this, initiated by trying add new unit tests. And my own unit tests caught it, but unfortunately, not the unit tests I added to Phobos. How ironic. Thanks for investigating Vladimir.
Comment 6 ZombineDev 2017-07-15 08:12:41 UTC
The error doesn't make sense to me. @safe adds bounds checks where there are none only when building with -release. I.e. if we're not building with -release, there shouldn't be any change in behavior - bounds checking should already be present (-boundscheck defaults to 'on' in that case). Am I missing something?
Comment 7 Jon Degenhardt 2017-07-15 15:31:06 UTC
(In reply to ZombineDev from comment #6)
> The error doesn't make sense to me. @safe adds bounds checks where there are
> none only when building with -release. I.e. if we're not building with
> -release, there shouldn't be any change in behavior - bounds checking should
> already be present (-boundscheck defaults to 'on' in that case). Am I
> missing something?

There was already an existing range violation, one that didn't have a unit test in the std.getopt module. Adding @safe to the function turned on bounds checking in debug mode. I had a unit test in my own code (not std.geopt) that triggered the out-of-bounds case. This unit test didn't get run until I actually ran my own unit tests with the new code.

The out-of-bounds case occurs when there is an command line argument that is a single dash. e.g.

   $ myprogram -x a  # okay
   $ myprogram -x -  # out-of-bounds

In the single dash case there's an attempt to access one character past it. Kind of understandable there wasn't a unit test for this originally. If you want to see the sequence that led to adding @safe read https://github.com/dlang/phobos/pull/5347.
Comment 8 Jon Degenhardt 2017-07-15 19:54:35 UTC
I'm preparing a PR to fix the underlying issue. A concern for the general release is that a single hyphen is often used to represent standard input in command line args. e.g.

$ cat file.txt | my_d_program --file - > out.txt

This will trigger the bounds check error if my_d_program is compiled in safe mode.
Comment 9 Jon Degenhardt 2017-07-15 20:31:59 UTC
https://github.com/dlang/phobos/pull/5612
Comment 10 github-bugzilla 2017-07-15 22:51:00 UTC
Commits pushed to master at https://github.com/dlang/phobos

https://github.com/dlang/phobos/commit/296184f5419e1a7f8748688606950e747338f8f1
Fix issue 17650: std.getopt range violation when option value is a hyphen.

https://github.com/dlang/phobos/commit/603e406b60cedd199b7bb2b6f9167438ec452307
Merge pull request #5612 from jondegenhardt/issue-17650-getopt-range-violation

Issue 17650: std.getopt range violation when option value is a hyphen
merged-on-behalf-of: Andrei Alexandrescu <andralex@users.noreply.github.com>
Comment 11 github-bugzilla 2017-07-16 01:36:42 UTC
Commit pushed to stable at https://github.com/dlang/phobos

https://github.com/dlang/phobos/commit/7a0ff521f296fef66455a84f11864b9cb046c43a
Fix issue 17650: std.getopt range violation when option value is a hyphen.
Comment 12 github-bugzilla 2017-07-19 14:17:47 UTC
Commit pushed to master at https://github.com/dlang/phobos

https://github.com/dlang/phobos/commit/7a0ff521f296fef66455a84f11864b9cb046c43a
Fix issue 17650: std.getopt range violation when option value is a hyphen.
Comment 13 github-bugzilla 2017-08-16 13:24:07 UTC
Commits pushed to stable at https://github.com/dlang/phobos

https://github.com/dlang/phobos/commit/296184f5419e1a7f8748688606950e747338f8f1
Fix issue 17650: std.getopt range violation when option value is a hyphen.

https://github.com/dlang/phobos/commit/603e406b60cedd199b7bb2b6f9167438ec452307
Merge pull request #5612 from jondegenhardt/issue-17650-getopt-range-violation
Comment 14 github-bugzilla 2018-01-05 13:30:25 UTC
Commits pushed to dmd-cxx at https://github.com/dlang/phobos

https://github.com/dlang/phobos/commit/296184f5419e1a7f8748688606950e747338f8f1
Fix issue 17650: std.getopt range violation when option value is a hyphen.

https://github.com/dlang/phobos/commit/603e406b60cedd199b7bb2b6f9167438ec452307
Merge pull request #5612 from jondegenhardt/issue-17650-getopt-range-violation

https://github.com/dlang/phobos/commit/7a0ff521f296fef66455a84f11864b9cb046c43a
Fix issue 17650: std.getopt range violation when option value is a hyphen.