Bug #9172

siphash faulty on arm little endian with word align - missing code

Added by Alban Browaeys over 1 year ago. Updated over 1 year ago.

[ruby-core:58658]
Status:Closed
Priority:Normal
Assignee:Nobuyoshi Nakada
ruby -v:ruby 1.9.3p448 (2013-06-27 revision 41675) [arm-linux-eabihf] Backport:1.9.3: UNKNOWN, 2.0.0: UNKNOWN

Description

siphash.c check for "little endian+unaligned word access" and
"bigendian". But my arch : armv7-a cortex a9 is little endian word
aligned. This discard the loop that reduce the input below 8 bytes and a
case when the leftover is four bytes.
https://github.com/ruby/ruby/commit/525cb66467ff22a50f2e6bf307924459d38cd592#diff-41728f3441b348d39ebae01b3a5694a0R409
and
https://github.com/ruby/ruby/commit/525cb66467ff22a50f2e6bf307924459d38cd592#diff-41728f3441b348d39ebae01b3a5694a0R446

I found this issue via bundler as it uses the hash of an array (the
collision is "sub-setter".hash and "discoverer".hash has they both share
all other fields (platform and version both at 0.0.2).
I reported it back then as public
https://github.com/bundler/bundler/issues/2724

extracts:

  • testcase: "collision.rb" # on arm (odroid u2) #$ ruby ~/collision.rb #708079652 #708079652

require 'rubygems'

p ["discoverer", Gem::Version.new("0.0.2"), "ruby"].hash
p ["sub-setter", Gem::Version.new("0.0.2"), "ruby"].hash

NB: For now I rebuild with those check for UNALIGNED_WORD_ACCESS removed and
at least I get way less conflict . This is no proper fix though.

Associated revisions

Revision 43928
Added by Nobuyoshi Nakada over 1 year ago

siphash.c: fix missing condition

  • siphash.c (sip_hash24): fix for aligned word access little endian platforms. [Bug #9172]

Revision 43928
Added by Nobuyoshi Nakada over 1 year ago

siphash.c: fix missing condition

  • siphash.c (sip_hash24): fix for aligned word access little endian platforms. [Bug #9172]

History

#1 Updated by Zachary Scott over 1 year ago

  • Status changed from Open to Assigned
  • Assignee set to Nobuyoshi Nakada

nobu what do you think?

what is "collision.rb"?

#2 Updated by Yusuke Endoh over 1 year ago

I confirmed the issue on my BeagleBone Black.

$ ./miniruby -ve 'p ["discoverer".hash, "sub-setter".hash]'
ruby 2.1.0dev (2013-11-28 trunk 43540) [armv7l-linux-eabihf]
[-51053038, -51053038]

I think this is a bug of the siphash-c upstream:

https://github.com/emboss/siphash-c/blob/master/src/siphash.c#L450

The bug was introduced by nobu, interestingly.

https://github.com/emboss/siphash-c/commit/99ff3a058ffe3acc25dbadea6c053a74b73559e2

Yusuke Endoh mame@tsg.ne.jp

#3 Updated by Nobuyoshi Nakada over 1 year ago

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

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


siphash.c: fix missing condition

  • siphash.c (sip_hash24): fix for aligned word access little endian platforms. [Bug #9172]

Also available in: Atom PDF