Bug #12509: Using qsort_s in mingw-w64 causes failures

• Status: Closed
• Assignee: nobu (Nobuyoshi Nakada)
• Target version: -
• ruby -v: ruby 2.4.0dev (2016-09-14 trunk 56160) [x64-mingw32]
• Backport: 2.1: UNKNOWN, 2.2: UNKNOWN, 2.3: UNKNOWN

[moritat (Tsuyoshi Morita)]

I got failures related to 'qsort_s'.
The following failures will disappear when I delete AC_CHECK_FUNCS(qsort_s) code in configure.

  1) Failure:
REXMLTests::TestXPathText#test_ancestors [C:/msys64/home/tsuyoshi/ruby-2.4.0-preview1/test/rexml/xpath/test_text.rb:72]:
Failed assertion, no message given.

  2) Failure:
RSS::TestMaker09#test_items [C:/msys64/home/tsuyoshi/ruby-2.4.0-preview1/test/rss/test_maker_0.9.rb:336]:
<"TITLE0"> expected but was
<"TITLE1">.

  3) Failure:
RSS::TestMaker09#test_items_with_new_api_since_018 [C:/msys64/home/tsuyoshi/ruby-2.4.0-preview1/test/rss/test_maker_0.9.rb:336]:
<"TITLE0"> expected but was
<"TITLE1">.

  4) Failure:
RSS::TestMaker10#test_items [C:/msys64/home/tsuyoshi/ruby-2.4.0-preview1/test/rss/test_maker_1.0.rb:288]:
<"http://hoge.com/0"> expected but was
<"http://hoge.com/1">.

  5) Failure:
RSS::TestMaker10#test_items_with_new_api_since_018 [C:/msys64/home/tsuyoshi/ruby-2.4.0-preview1/test/rss/test_maker_1.0.rb:288]:
<"http://hoge.com/0"> expected but was
<"http://hoge.com/1">.

  6) Failure:
RSS::TestMaker20#test_items [C:/msys64/home/tsuyoshi/ruby-2.4.0-preview1/test/rss/test_maker_2.0.rb:400]:
<"TITLE0"> expected but was
<"TITLE1">.

  7) Failure:
RSS::TestMaker20#test_items_with_new_api_since_018 [C:/msys64/home/tsuyoshi/ruby-2.4.0-preview1/test/rss/test_maker_2.0.rb:400]:
<"TITLE0"> expected but was
<"TITLE1">.

  8) Failure:
TestArray#test_sort_bang_with_freeze [C:/msys64/home/tsuyoshi/ruby-2.4.0-preview1/test/ruby/test_array.rb:1470]:
frozen during comparison.
[RuntimeError] exception expected, not.
Class: <ArgumentError>
Message: <"comparison of Object with Object failed">
---Backtrace---
C:/msys64/home/tsuyoshi/ruby-2.4.0-preview1/test/ruby/test_array.rb:1470:in `sort!'
C:/msys64/home/tsuyoshi/ruby-2.4.0-preview1/test/ruby/test_array.rb:1470:in `block in test_sort_bang_with_freeze'
C:/msys64/home/tsuyoshi/ruby-2.4.0-preview1/test/lib/test/unit/assertions.rb:74:in `assert_raise'
C:/msys64/home/tsuyoshi/ruby-2.4.0-preview1/test/ruby/test_array.rb:1470:in `test_sort_bang_with_freeze'
C:/msys64/home/tsuyoshi/ruby-2.4.0-preview1/test/lib/test/unit.rb:1027:in `run_test'
---------------

  9) Failure:
TestGemRequestSetLockfile#test_to_s_gem_dependency_non_default [C:/msys64/home/tsuyoshi/ruby-2.4.0-preview1/test/rubygems/test_gem_request_set_lockfile.rb:239]:
--- expected
+++ actual
@@ -10,5 +10,5 @@
 
 DEPENDENCIES
   a
-  b
+  b (>= 1)
 "


 10) Failure:
TestGemResolver#test_raises_dependency_error [C:/msys64/home/tsuyoshi/ruby-2.4.0-preview1/test/rubygems/test_gem_resolver.rb:425]:
--- expected
+++ actual
@@ -1,2 +1,2 @@
-[Gem::Dependency.new("c", Gem::Requirement.new(["= 2"]), :runtime),
- Gem::Dependency.new("c", Gem::Requirement.new(["= 1"]), :runtime)]
+[Gem::Dependency.new("c", Gem::Requirement.new(["= 1"]), :runtime),
+ Gem::Dependency.new("c", Gem::Requirement.new(["= 2"]), :runtime)]


 11) Failure:
TestGemResolver#test_raises_when_possibles_are_exhausted [C:/msys64/home/tsuyoshi/ruby-2.4.0-preview1/test/rubygems/test_gem_resolver.rb:524]:
Expected: "c-2"
  Actual: "c-1"

$ gcc -v
Using built-in specs.
COLLECT_GCC=C:\msys64\mingw64\bin\gcc.exe
COLLECT_LTO_WRAPPER=C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/5.4.0/lto-wrapper.exe
Target: x86_64-w64-mingw32
Configured with: ../gcc-5.4.0/configure --prefix=/mingw64 --with-local-prefix=/mingw64/local --build=x86_64-w64-mingw32 --host=x86_64-w64-mingw32 --target=x86_64-w64-mingw32 --with-native-system-header-dir=/mingw64/x86_64-w64-mingw32/include --libexecdir=/mingw64/lib --with-gxx-include-dir=/mingw64/include/c++/5.4.0 --enable-bootstrap --with-arch=x86-64 --with-tune=generic --enable-languages=c,lto,c++,objc,obj-c++,fortran,ada --enable-shared --enable-static --enable-libatomic --enable-threads=posix --enable-graphite --enable-fully-dynamic-string --enable-libstdcxx-time=yes --disable-libstdcxx-pch --disable-libstdcxx-debug --enable-version-specific-runtime-libs --disable-isl-version-check --enable-lto --enable-libgomp --disable-multilib --enable-checking=release --disable-rpath --disable-win32-registry --disable-nls --disable-werror --disable-symvers --with-libiconv --with-system-zlib --with-gmp=/mingw64 --with-mpfr=/mingw64 --with-mpc=/mingw64 --with-isl=/mingw64 --with-pkgversion='Rev1, Built by MSYS2 project' --with-bugurl=https://sourceforge.net/projects/msys2 --with-gnu-as --with-gnu-ld
Thread model: posix
gcc version 5.4.0 (Rev1, Built by MSYS2 project)

[moritat (Tsuyoshi Morita)]

• File configure.in.patch configure.in.patch added

I confirmed that the failures 1) to 8) are same as
VS2010 with '#define HAVE_QSORT_S'.
I suppose 9) to 11) are skipped for VS2010.

Since mingw* uses the msvcrt runtime, I propose
disabling use of qsort_s for mingw*, too.

[moritat (Tsuyoshi Morita)]

• ruby -v changed from ruby 2.4.0preview1 (2016-06-20 trunk 55466) [x64-mingw32] to ruby 2.4.0dev (2016-09-14 trunk 56160) [x64-mingw32]

Could you take in the attached patch before 2.4.0 release ?
These keep fail in the current trunk version.

[shyouhei (Shyouhei Urabe)]

• Status changed from Open to Assigned
• Assignee set to nobu (Nobuyoshi Nakada)

[nobu (Nobuyoshi Nakada)]

• Status changed from Assigned to Closed

Applied in changeset r56410.

---

test_array.rb: do not assume stable sort

• test/ruby/test_array.rb (test_sort_bang_with_freeze): make a
  clone to copy a <=> singleton method, instead of dup. which
  element will be called is not predictable.
  [ruby-core:76088] [Bug #12509]

[nobu (Nobuyoshi Nakada)]

This is because Array#sort and Array#sort_by may not be stable.
I fixed test_array.rb and reported to rubygems, but uncertain the rest, whether rexml and rss expect stable sort, or the orders are undefined and the tests have bugs.

A patch to fix the tests.

diff --git i/test/rexml/xpath/test_text.rb w/test/rexml/xpath/test_text.rb
index cea3b05..7222388 100644
--- i/test/rexml/xpath/test_text.rb
+++ w/test/rexml/xpath/test_text.rb
@@ -69,7 +69,9 @@
       assert_equal(1, nodes.size, "<b> has one element ancestor")
       nodes = @doc.get_elements('//b/ancestor::node()')
       assert_equal(2, nodes.size, "<b> has two node ancestors")
-      assert_kind_of REXML::Document, nodes[1]
+      nodes.sort_by!(&:name)
+      assert_kind_of REXML::Document, nodes[0]
+      assert_kind_of REXML::Element, nodes[1]
     end
   end
 end
diff --git i/test/rss/test_maker_0.9.rb w/test/rss/test_maker_0.9.rb
index 64d04bc..d07a724 100644
--- i/test/rss/test_maker_0.9.rb
+++ w/test/rss/test_maker_0.9.rb
@@ -315,6 +315,7 @@
       assert_equal(link, item.link)
       assert_nil(item.description)
 
+      pubDate = Time.now
 
       item_size = 5
       rss = RSS::Maker.make("0.91") do |maker|
@@ -325,6 +326,7 @@
             _item.title = "#{title}#{i}"
             _item.link = "#{link}#{i}"
             _item.description = "#{description}#{i}"
+            _item.date = pubDate - i
           end
         end
         maker.items.do_sort = true
diff --git i/test/rss/test_maker_1.0.rb w/test/rss/test_maker_1.0.rb
index c8f9977..f3c0e50 100644
--- i/test/rss/test_maker_1.0.rb
+++ w/test/rss/test_maker_1.0.rb
@@ -269,6 +269,7 @@
       assert_equal(link, item.link)
       assert_nil(item.description)
 
+      pubDate = Time.now
 
       item_size = 5
       rss = RSS::Maker.make("1.0") do |maker|
@@ -279,6 +280,7 @@
             _item.title = "#{title}#{i}"
             _item.link = "#{link}#{i}"
             _item.description = "#{description}#{i}"
+            _item.date = pubDate - i
           end
         end
         maker.items.do_sort = true
diff --git i/test/rss/test_maker_2.0.rb w/test/rss/test_maker_2.0.rb
index 8528611..f6d83f0 100644
--- i/test/rss/test_maker_2.0.rb
+++ w/test/rss/test_maker_2.0.rb
@@ -390,7 +390,7 @@
             item.description = "#{description}#{i}"
             item.author = "#{author}#{i}"
             item.comments = "#{comments}#{i}"
-            item.date = pubDate
+            item.date = pubDate - i
           end
         end
         maker.items.do_sort = true
@@ -402,8 +402,8 @@
         assert_equal("#{description}#{i}", item.description)
         assert_equal("#{author}#{i}", item.author)
         assert_equal("#{comments}#{i}", item.comments)
-        assert_equal(pubDate, item.pubDate)
-        assert_equal(pubDate, item.date)
+        assert_equal(pubDate - i, item.pubDate)
+        assert_equal(pubDate - i, item.date)
       end
 
       rss = RSS::Maker.make("2.0") do |maker|

[nobu (Nobuyoshi Nakada)]

Or a patch for stable sort.

diff --git i/lib/rexml/xpath_parser.rb w/lib/rexml/xpath_parser.rb
index 181b2b6..7ae6495 100644
--- i/lib/rexml/xpath_parser.rb
+++ w/lib/rexml/xpath_parser.rb
@@ -503,9 +503,9 @@
           node_idx << np.parent.index( np )
           np = np.parent
         end
-        new_arry << [ node_idx.reverse, node ]
+        new_arry << [ node_idx.reverse, new_arry.size, node ]
       }
-      new_arry.sort{ |s1, s2| s1[0] <=> s2[0] }.collect{ |s| s[1] }
+      new_arry.sort.collect(&:last)
     end
 
 
diff --git i/lib/rss/maker/base.rb w/lib/rss/maker/base.rb
index bc4ca84..8c00375 100644
--- i/lib/rss/maker/base.rb
+++ w/lib/rss/maker/base.rb
@@ -690,13 +690,15 @@
       private
       def sort_if_need
         if @do_sort.respond_to?(:call)
-          @items.sort do |x, y|
-            @do_sort.call(x, y)
-          end
+          @items.map.with_index.sort do |(x, xi), (y, yi)|
+            c = @do_sort.call(x, y)
+            c == 0 ? xi <=> yi : c
+          end.map(&:first)
         elsif @do_sort
-          @items.sort do |x, y|
-            y <=> x
-          end
+          @items.map.with_index.sort do |(x, xi), (y, yi)|
+            c = y <=> x
+            c == 0 ? xi <=> yi : c
+          end.map(&:first)
         else
           @items
         end