www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 12183] New: using std.algorithm.sort makes valgrind abort

reply d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12183

           Summary: using std.algorithm.sort makes valgrind abort
           Product: D
           Version: D2
          Platform: x86_64
        OS/Version: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Phobos
        AssignedTo: nobody puremagic.com
        ReportedBy: pro.mathias.lang gmail.com
        Depends on: 10054


--- Comment #0 from Mathias LANG <pro.mathias.lang gmail.com> 2014-02-16
06:32:06 PST ---
Hi,
Valgrind currently doesn't support 80-bits x87 operation, and is unlikely to do
so (http://www.valgrind.org/docs/manual/manual-core.html#manual-core.limits).
So everytime it'll encounter one, it'll choke on it and abort.

By default, std.algorithm.sort will use the unstable SwapStrategy to sort,
which will emit an x87 80 bits instruction (there's a cast(real)r.lengh). So by
default, we will by slightly more optimized, but one of the most popular
debugging tool on linux will stop to work, and force users (possibly newbies)
to look into valgrind's implementation details to understand why it's not
working.

That's why I think we should change the default from unstable to stable. I'll
submit a PR soon.

NB: Example of code:

import std.algorithm, std.string;
void    main()
{
    string  names = "alex|jake|mat";
    auto    sn = names.split("|").sort();
}

Result in:
geod Barsoom:~$ valgrind ./test
==19592== Memcheck, a memory error detector
==19592== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
==19592== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
==19592== Command: ./test
==19592== 
Boom ?
vex amd64->IR: unhandled instruction bytes: 0x48 0xDF 0x6D 0xF0 0xEB 0x1A 0x48
0xB9
vex amd64->IR:   REX=1 REX.W=1 REX.R=0 REX.X=0 REX.B=0
vex amd64->IR:   VEX=0 VEX.L=0 VEX.nVVVV=0x0 ESC=NONE
vex amd64->IR:   PFX.66=0 PFX.F2=0 PFX.F3=0
==19592== valgrind: Unrecognised instruction at address 0x4326dd.
==19592==    at 0x4326DD:
_D3std9algorithm62__T4sortVAyaa5_61203c2062VE3std9algorithm12SwapStrategy0TAAyaZ4sortFAAyaZS3std5range39__T11SortedRangeTAAyaVAyaa5_61203c2062Z11SortedRange
(in /home/geod/test)
[...]

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Feb 16 2014
next sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12183


monarchdodra gmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |monarchdodra gmail.com
          Component|Phobos                      |DMD


--- Comment #1 from monarchdodra gmail.com 2014-02-16 07:58:40 PST ---
Re-assigning as DMD issue. You can't expect library code to work around such an
issue.

BTW, I think your fix is wrong. For staters, changing default behavior is
unacceptable.

It would be much better to simply change the cast to "double": In this case,
there is absolutely nothing that warrants using "real" level precision: "depth"
is nothing more than a rough estimate to begin with. If anything, I'd think
passing around an 80 bit real to be less efficient than a standard double. Why
don't you go with that fix?

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Feb 16 2014
prev sibling next sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12183


Andrei Alexandrescu <andrei erdani.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |andrei erdani.com


--- Comment #2 from Andrei Alexandrescu <andrei erdani.com> 2014-02-26 18:21:45
PST ---
Yah, casting to double is a fine workaround. Please don't change default
stability.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Feb 26 2014
prev sibling next sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12183


Walter Bright <bugzilla digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bugzilla digitalmars.com
          Component|DMD                         |Phobos


--- Comment #3 from Walter Bright <bugzilla digitalmars.com> 2014-03-02
10:54:39 PST ---
(In reply to comment #1)
 Re-assigning as DMD issue. You can't expect library code to work around such an
 issue.

Bugs in valgrind are not a dmd issue. Putting a workaround in Phobos is more appropriate. -- Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Mar 02 2014
prev sibling next sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12183



--- Comment #4 from Walter Bright <bugzilla digitalmars.com> 2014-03-02
16:59:34 PST ---
As noted in the pull request, floating point isn't even needed for this. Can do
the same thing with integers.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Mar 02 2014
prev sibling next sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12183



--- Comment #5 from github-bugzilla puremagic.com 2014-03-11 16:22:41 PDT ---
Commits pushed to master at https://github.com/D-Programming-Language/phobos

https://github.com/D-Programming-Language/phobos/commit/4ed10be8bc620b0fce7ad2eed1ba7c86e5c079ea
Fix issue 12183 : Avoid using floating point in std.algorithm.sort.

- Prevent valgrind from aborting (some x87 FP operation are not supported).
- Can be slower on machines that have a soft floating point ABI.

https://github.com/D-Programming-Language/phobos/commit/1fa56c676d870de8f7944ce4af93fcb1d6aebefa
Merge pull request #1946 from Geod24/make-valgrind-happy

Fix issue 12183 : Remove use of floating point in std.algorithm.quickSortImpl

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Mar 11 2014
prev sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12183


Andrei Alexandrescu <andrei erdani.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED


-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Mar 12 2014