Bug 12183 – using std.algorithm.sort makes valgrind abort

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) [...]

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?

Yah, casting to double is a fine workaround. Please don't change default stability.

(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.

As noted in the pull request, floating point isn't even needed for this. Can do the same thing with integers.

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

(In reply to Mathias LANG from comment #0) > 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. This is actually wrong, valgrind does support those instructions it just doesn't run them at full 80-bit precision. The aborts are caused by obscure forms of the instructions that have not been implemented. Chances are the workaround can now be removed.

(In reply to yebblies from comment #6) > Chances are the workaround can now be removed. They fixed it by avoiding floating point altogether, the solution should be superior than the prior code.

(In reply to safety0ff.bugz from comment #7) > > They fixed it by avoiding floating point altogether, the solution should be > superior than the prior code. Even better.