[ruby-core:20962] [Bug #633] dl segfaults on x86_64-linux systems

Issue #633 has been updated by Takehiro Kubo.


A patch again which I previously attached excluding changeset r21110
and including a few new issues.

Index: ext/dl/test/test_base.rb
===================================================================
--- ext/dl/test/test_base.rb	(revision 21112)
+++ ext/dl/test/test_base.rb	(working copy)
@@ -6,8 +6,17 @@
   LIBC_SO = "cygwin1.dll"
   LIBM_SO = "cygwin1.dll"
 when /linux/
-  LIBC_SO = "/lib/libc.so.6"
-  LIBM_SO = "/lib/libm.so.6"
+  libdir = '/lib'
+  case [0].pack('L!').size
+  when 4
+    # 32-bit ruby
+    libdir = '/lib32' if File.directory? '/lib32'
+  when 8
+    # 64-bit ruby
+    libdir = '/lib64' if File.directory? '/lib64'
+  end
+  LIBC_SO = File.join(libdir, "libc.so.6")
+  LIBM_SO = File.join(libdir, "libm.so.6")
 when /mingw/, /mswin32/
   LIBC_SO = "msvcrt.dll"
   LIBM_SO = "msvcrt.dll"
Index: ext/dl/test/test_import.rb
===================================================================
--- ext/dl/test/test_import.rb	(revision 21112)
+++ ext/dl/test/test_import.rb	(working copy)
@@ -11,10 +11,10 @@
 
     extern "void *strcpy(char*, char*)"
     extern "int isdigit(int)"
-    extern "float atof(string)"
+    extern "double atof(string)"
     extern "unsigned long strtoul(char*, char **, int)"
-    extern "int qsort(void*, int, int, void*)"
-    extern "void fprintf(FILE*, char*)"
+    extern "int qsort(void*, unsigned long, unsigned long, void*)"
+    extern "int fprintf(FILE*, char*)"
     extern "int gettimeofday(timeval*, timezone*)" rescue nil
 
     QsortCallback = bind("void *qsort_callback(void*, void*)", :temp)
Index: ext/dl/test/test_func.rb
===================================================================
--- ext/dl/test/test_func.rb	(revision 21112)
+++ ext/dl/test/test_func.rb	(working copy)
@@ -24,7 +24,7 @@
     end
 
     def test_atof()
-      f = Function.new(CFunc.new(@libc['atof'], TYPE_FLOAT, 'atof'),
+      f = Function.new(CFunc.new(@libc['atof'], TYPE_DOUBLE, 'atof'),
                        [TYPE_VOIDP])
       r = f.call("12.34")
       assert_match(12.00..13.00, r)
Index: ext/dl/dl.h
===================================================================
--- ext/dl/dl.h	(revision 21112)
+++ ext/dl/dl.h	(working copy)
@@ -50,29 +50,65 @@
     stack[15],stack[16],stack[17],stack[18],stack[19]
 
 #define DLSTACK_PROTO0 
-#define DLSTACK_PROTO1 DLSTACK_TYPE
-#define DLSTACK_PROTO2 DLSTACK_PROTO1, DLSTACK_TYPE
-#define DLSTACK_PROTO3 DLSTACK_PROTO2, DLSTACK_TYPE
-#define DLSTACK_PROTO4 DLSTACK_PROTO3, DLSTACK_TYPE
-#define DLSTACK_PROTO4 DLSTACK_PROTO3, DLSTACK_TYPE
-#define DLSTACK_PROTO5 DLSTACK_PROTO4, DLSTACK_TYPE
-#define DLSTACK_PROTO6 DLSTACK_PROTO5, DLSTACK_TYPE
-#define DLSTACK_PROTO7 DLSTACK_PROTO6, DLSTACK_TYPE
-#define DLSTACK_PROTO8 DLSTACK_PROTO7, DLSTACK_TYPE
-#define DLSTACK_PROTO9 DLSTACK_PROTO8, DLSTACK_TYPE
-#define DLSTACK_PROTO10 DLSTACK_PROTO9, DLSTACK_TYPE
-#define DLSTACK_PROTO11 DLSTACK_PROTO10, DLSTACK_TYPE
-#define DLSTACK_PROTO12 DLSTACK_PROTO11, DLSTACK_TYPE
-#define DLSTACK_PROTO13 DLSTACK_PROTO12, DLSTACK_TYPE
-#define DLSTACK_PROTO14 DLSTACK_PROTO13, DLSTACK_TYPE
-#define DLSTACK_PROTO14 DLSTACK_PROTO13, DLSTACK_TYPE
-#define DLSTACK_PROTO15 DLSTACK_PROTO14, DLSTACK_TYPE
-#define DLSTACK_PROTO16 DLSTACK_PROTO15, DLSTACK_TYPE
-#define DLSTACK_PROTO17 DLSTACK_PROTO16, DLSTACK_TYPE
-#define DLSTACK_PROTO18 DLSTACK_PROTO17, DLSTACK_TYPE
-#define DLSTACK_PROTO19 DLSTACK_PROTO18, DLSTACK_TYPE
-#define DLSTACK_PROTO20 DLSTACK_PROTO19, DLSTACK_TYPE
+#define DLSTACK_PROTO1_ DLSTACK_TYPE
+#define DLSTACK_PROTO2_ DLSTACK_PROTO1_, DLSTACK_TYPE
+#define DLSTACK_PROTO3_ DLSTACK_PROTO2_, DLSTACK_TYPE
+#define DLSTACK_PROTO4_ DLSTACK_PROTO3_, DLSTACK_TYPE
+#define DLSTACK_PROTO4_ DLSTACK_PROTO3_, DLSTACK_TYPE
+#define DLSTACK_PROTO5_ DLSTACK_PROTO4_, DLSTACK_TYPE
+#define DLSTACK_PROTO6_ DLSTACK_PROTO5_, DLSTACK_TYPE
+#define DLSTACK_PROTO7_ DLSTACK_PROTO6_, DLSTACK_TYPE
+#define DLSTACK_PROTO8_ DLSTACK_PROTO7_, DLSTACK_TYPE
+#define DLSTACK_PROTO9_ DLSTACK_PROTO8_, DLSTACK_TYPE
+#define DLSTACK_PROTO10_ DLSTACK_PROTO9_, DLSTACK_TYPE
+#define DLSTACK_PROTO11_ DLSTACK_PROTO10_, DLSTACK_TYPE
+#define DLSTACK_PROTO12_ DLSTACK_PROTO11_, DLSTACK_TYPE
+#define DLSTACK_PROTO13_ DLSTACK_PROTO12_, DLSTACK_TYPE
+#define DLSTACK_PROTO14_ DLSTACK_PROTO13_, DLSTACK_TYPE
+#define DLSTACK_PROTO14_ DLSTACK_PROTO13_, DLSTACK_TYPE
+#define DLSTACK_PROTO15_ DLSTACK_PROTO14_, DLSTACK_TYPE
+#define DLSTACK_PROTO16_ DLSTACK_PROTO15_, DLSTACK_TYPE
+#define DLSTACK_PROTO17_ DLSTACK_PROTO16_, DLSTACK_TYPE
+#define DLSTACK_PROTO18_ DLSTACK_PROTO17_, DLSTACK_TYPE
+#define DLSTACK_PROTO19_ DLSTACK_PROTO18_, DLSTACK_TYPE
+#define DLSTACK_PROTO20_ DLSTACK_PROTO19_, DLSTACK_TYPE
 
+/*
+ * Add ",..." as the last argument.
+ * This is required for variable argument functions such
+ * as fprintf() on x86_64-linux.
+ *
+ * http://refspecs.linuxfoundation.org/elf/x86_64-abi-0.95.pdf
+ * page 19:
+ *
+ *   For calls that may call functions that use varargs or stdargs
+ *   (prototype-less calls or calls to functions containing ellipsis
+ *   (...) in the declaration) %al is used as hidden argument to
+ *   specify the number of SSE registers used.
+ */
+#define DLSTACK_PROTO1 DLSTACK_PROTO1_, ...
+#define DLSTACK_PROTO2 DLSTACK_PROTO2_, ...
+#define DLSTACK_PROTO3 DLSTACK_PROTO3_, ...
+#define DLSTACK_PROTO4 DLSTACK_PROTO4_, ...
+#define DLSTACK_PROTO4 DLSTACK_PROTO4_, ...
+#define DLSTACK_PROTO5 DLSTACK_PROTO5_, ...
+#define DLSTACK_PROTO6 DLSTACK_PROTO6_, ...
+#define DLSTACK_PROTO7 DLSTACK_PROTO7_, ...
+#define DLSTACK_PROTO8 DLSTACK_PROTO8_, ...
+#define DLSTACK_PROTO9 DLSTACK_PROTO9_, ...
+#define DLSTACK_PROTO10 DLSTACK_PROTO10_, ...
+#define DLSTACK_PROTO11 DLSTACK_PROTO11_, ...
+#define DLSTACK_PROTO12 DLSTACK_PROTO12_, ...
+#define DLSTACK_PROTO13 DLSTACK_PROTO13_, ...
+#define DLSTACK_PROTO14 DLSTACK_PROTO14_, ...
+#define DLSTACK_PROTO14 DLSTACK_PROTO14_, ...
+#define DLSTACK_PROTO15 DLSTACK_PROTO15_, ...
+#define DLSTACK_PROTO16 DLSTACK_PROTO16_, ...
+#define DLSTACK_PROTO17 DLSTACK_PROTO17_, ...
+#define DLSTACK_PROTO18 DLSTACK_PROTO18_, ...
+#define DLSTACK_PROTO19 DLSTACK_PROTO19_, ...
+#define DLSTACK_PROTO20 DLSTACK_PROTO20_, ...
+
 #define DLSTACK_ARGS0(stack)
 #define DLSTACK_ARGS1(stack) stack[0]
 #define DLSTACK_ARGS2(stack) DLSTACK_ARGS1(stack), stack[1]
Index: ext/dl/lib/dl/stack.rb
===================================================================
--- ext/dl/lib/dl/stack.rb	(revision 21112)
+++ ext/dl/lib/dl/stack.rb	(working copy)
@@ -121,20 +121,26 @@
       @template = ""
       addr      = 0
       types.each{|t|
-        orig_addr = addr
-        addr = align(orig_addr, ALIGN_MAP[t])
-        d = addr - orig_addr
-        if( d > 0 )
-          @template << "x#{d}"
-        end
+        addr = add_padding(addr, ALIGN_MAP[t])
         @template << PACK_MAP[t]
         addr += SIZE_MAP[t]
       }
+      addr = add_padding(addr, ALIGN_MAP[SIZEOF_VOIDP])
       if( addr % SIZEOF_VOIDP == 0 )
         @size = addr / SIZEOF_VOIDP
       else
         @size = (addr / SIZEOF_VOIDP) + 1
       end
     end
+
+    def add_padding(addr, align)
+      orig_addr = addr
+      addr = align(orig_addr, align)
+      d = addr - orig_addr
+      if( d > 0 )
+        @template << "x#{d}"
+      end
+      addr
+    end
   end
 end


- ext/dl/test/test_base.rb

  /lib/libc.so is i386 binary on x86_64 redhat.

  redhat-based x86_64 linux distributions:
    /lib    - 32-bit libraries
    /lib64  - 64-bit libraries

  debian-based x86_64 linux distributions:
    /lib    - 64-bit libraries
    /lib32  - 32-bit libraries
    /lib64  - symbolic link to /lib

  This will work on the following combinations.
   - i386 ruby on i386 linux
   - i386 ruby on redhat-based x86_64 linux
   - i386 ruby on debian-based x86_64 linux
   - x86_64 ruby on redhat-based x86_64 linux
   - x86_64 ruby on debian-based x86_64 linux

- ext/dl/test/test_import.rb

  atof()'s return value is double.
  The test at test_import.rb:133 fails on x86_64 linux without this
  fix.

  The types of qsort's second and third argument are size_t.
  But DL::Importer cannot handle size_t. So I replaced them
  to unsigned long. It happens to work on 64-bit little-endian
  binary, but not on 64-bit big-endian binary without this fix.

  fprintf()'s return value is int.
  I don't know what difference is made by this change, but
  it will be safe.

- ext/dl/test/test_func.rb

  atof()'s return value is double.
  The test at test_func.rb:30 fails on x86_64 linux without this fix.

- ext/dl/dl.h

  The process may be dumped by segv at test_import.rb:67 without this
  fix. It depends on the value of %al register at cfunc.c:276.
  The reason is described in a comment of the patch.

- ext/dl/lib/dl/stack.rb

  The process may be dumped by segv at test_func.rb:18 without this
  fix. If the last argument's size is less than SIZEOF_VOIDP, the
  value is deleted by .unpack('l!*') at stack.rb:24.

----------------------------------------
http://redmine.ruby-lang.org/issues/show/633

----------------------------------------
http://redmine.ruby-lang.org