Bug #3861

Endian bugs in fiddle/dl on sparc64

Added by Jeremy Evans over 4 years ago. Updated over 3 years ago.

[ruby-core:32504]
Status:Closed
Priority:Normal
Assignee:Aaron Patterson
ruby -v:- Backport:

Description

=begin
Running the test suite for 1.9.2 on sparc64 on OpenBSD, the following errors on received:

2) Failure:
test_callback(DL::TestDL)
[/usr/ports/pobj/ruby-1.9.2-p0/ruby-1.9.2-p0/test/dl/test_dl2.rb:144]:
<"aabbfoorz"> expected but was
<"zabrfbooa">.

3) Failure:
test_isdigit(DL::TestFunc)
[/usr/ports/pobj/ruby-1.9.2-p0/ruby-1.9.2-p0/test/dl/test_func.rb:67]:
Expected 0 to be > 0.

4) Failure:
test_qsort1(DL::TestFunc)
[/usr/ports/pobj/ruby-1.9.2-p0/ruby-1.9.2-p0/test/dl/test_func.rb:95]:
<"1349"> expected but was
<"9341">.

5) Failure:
test_qsort2(DL::TestFunc)
[/usr/ports/pobj/ruby-1.9.2-p0/ruby-1.9.2-p0/test/dl/test_func.rb:105]:
<"1349"> expected but was
<"9341">.

6) Failure:
test_isdigit(DL::TestImport)
[/usr/ports/pobj/ruby-1.9.2-p0/ruby-1.9.2-p0/test/dl/test_import.rb:126]:
Expected 0 to be > 0.

It's possible this is an upstream issue with libffi. If so, please let me know and I'll report it to the libffi developers.
=end

fiddle-sparcv9.patch Magnifier (2.13 KB) Naohisa Goto, 08/08/2011 10:37 PM

noname (500 Bytes) Aaron Patterson, 08/09/2011 06:23 AM

noname (500 Bytes) Aaron Patterson, 08/21/2011 04:23 AM

Associated revisions

Revision 32895
Added by Naohisa Goto over 3 years ago

  • ext/fiddle/conversions.c (generic_to_value): ffi_arg and ffi_sarg should be used to handle shorter return value. fix [Bug #3861]
  • ext/fiddle/closure.c (callback): ditto
  • ext/fiddle/conversions.h (fiddle_generic): ditto
  • ext/fiddle/conversions.c (value_to_generic): char, short and int are strictly distinguished on big-endian CPU, e.g. sparc64.

Revision 32895
Added by Naohisa Goto over 3 years ago

  • ext/fiddle/conversions.c (generic_to_value): ffi_arg and ffi_sarg should be used to handle shorter return value. fix [Bug #3861]
  • ext/fiddle/closure.c (callback): ditto
  • ext/fiddle/conversions.h (fiddle_generic): ditto
  • ext/fiddle/conversions.c (value_to_generic): char, short and int are strictly distinguished on big-endian CPU, e.g. sparc64.

History

#1 Updated by Yui NARUSE over 4 years ago

  • Assignee set to Aaron Patterson
  • Status changed from Open to Assigned

=begin

=end

#2 Updated by Aaron Patterson over 4 years ago

=begin
I can't reproduce this on my PPC machine.

Can you provide the output of 'make test-all TESTS=fiddle' and 'make test-all TESTS=dl' on the sparc64 machine?

DL is known to have problems on 64bit machines without Fiddle. I suspect that fiddle was not built on this machine.

Thanks!
=end

#3 Updated by Yui NARUSE over 4 years ago

  • Status changed from Assigned to Feedback
  • Priority changed from Normal to Low

=begin

=end

#4 Updated by Motohiro KOSAKI over 3 years ago

  • Status changed from Feedback to Rejected

I close this as Rejected since no feedback provided.
Please reopen this if it still happens on the latest version of ruby.

#5 Updated by Naohisa Goto over 3 years ago

  • File fiddle-sparcv9.patch added
  • Status changed from Rejected to Open
  • Priority changed from Low to Normal
  • ruby -v changed from ruby 1.9.2p0 (2010-08-18 revision 29036) [sparc64-openbsd4.8] to ruby 1.9.4dev (2011-08-05 trunk 32863) [sparc64-solaris2.10]

This is also reproduced on sparc64 Solaris10 with latest Ruby.
This is not because of endian, nor libffi bug, but a bug of Fiddle.
Patch is attached.

The manpage of ffi_call(3) (http://linux.die.net/man/3/ffi_call ) says:
"rvalue must point to storage that is sizeof(long) or larger. For smaller return value sizes, the ffi_arg or ffi_sarg integral type must be used to hold the return value."
but Fiddle did not use ffi_arg nor ffi_sarg for getting the "rvalue" after ffi_call and closure callback.

Note that value_to_generic and generic_to_value are asymmetric after the patch.
In addition, to avoid potential bug in big-endian architecture, the patch also modifies value_to_generic() to strictly distinguish char, short and int.

#6 Updated by Naohisa Goto over 3 years ago

Patch attached again.
The previous patch contains mistakes and garbage.

#7 Updated by Naohisa Goto over 3 years ago

  • File deleted (fiddle-sparcv9.patch)

#8 Updated by Aaron Patterson over 3 years ago

  • ruby -v changed from ruby 1.9.4dev (2011-08-05 trunk 32863) [sparc64-solaris2.10] to -

On Mon, Aug 08, 2011 at 10:37:53PM +0900, Naohisa Goto wrote:

Issue #3861 has been updated by Naohisa Goto.

File fiddle-sparcv9.patch added

Patch attached again.
The previous patch contains mistakes and garbage.

Please apply these to trunk. I think it should also be applied to the
1.9.3 branch.

Thanks.

--
Aaron Patterson
http://tenderlovemaking.com/

#9 Updated by Naohisa Goto over 3 years ago

  • Status changed from Open to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r32895.
Jeremy, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


  • ext/fiddle/conversions.c (generic_to_value): ffi_arg and ffi_sarg should be used to handle shorter return value. fix [Bug #3861]
  • ext/fiddle/closure.c (callback): ditto
  • ext/fiddle/conversions.h (fiddle_generic): ditto
  • ext/fiddle/conversions.c (value_to_generic): char, short and int are strictly distinguished on big-endian CPU, e.g. sparc64.

#10 Updated by Naohisa Goto over 3 years ago

  • Target version set to 1.9.3
  • Status changed from Closed to Open

re-open this issue for the reminder whether it is backported to 1.9.3 or not.

#11 Updated by Motohiro KOSAKI over 3 years ago

  • Status changed from Open to Assigned

Aaron, I think it's ok to backport. But I'd respect your opinion. Please decide it.

Thanks.

#12 Updated by Aaron Patterson over 3 years ago

On Sat, Aug 20, 2011 at 07:11:23PM +0900, Motohiro KOSAKI wrote:

Issue #3861 has been updated by Motohiro KOSAKI.

Status changed from Open to Assigned

Aaron, I think it's ok to backport. But I'd respect your opinion. Please decide it.

Yes, I think it's fine. Please commit to the 1.9.3 branch! :-)

--
Aaron Patterson
http://tenderlovemaking.com/

#13 Updated by Naohisa Goto over 3 years ago

  • Status changed from Assigned to Closed

I've just committed with r33015. close the issue now

Also available in: Atom PDF