Project

General

Profile

Actions

Bug #12537

closed

Fiddle::TestPointer#test_to_s and test_to_str destroy literal string data

Added by ngoto (Naohisa Goto) over 7 years ago. Updated over 7 years ago.

Status:
Closed
Assignee:
-
Target version:
-
[ruby-dev:49700]

Description

r55547 以降、rubyciの32ビット環境では、以下のFailureが出ています。

  1) Failure:
Fiddle::TestPointer#test_plus [/extdisk/chkbuild/chkbuild/tmp/build/20160701T080500Z/ruby/test/fiddle/test_pointer.rb:70]:
<"lo world"> expected but was
<"lo">.

  2) Failure:
Psych::Visitors::TestEmitter#test_sequence [/extdisk/chkbuild/chkbuild/tmp/build/20160701T080500Z/ruby/test/psych/visitors/test_emitter.rb:110]:
Expected /- hello/ to match "---\n- ! \"hello\\0world\"\n".

  3) Failure:
Psych::Visitors::TestYAMLTree#test_yaml_tree_can_take_an_emitter [/extdisk/chkbuild/chkbuild/tmp/build/20160701T080500Z/ruby/test/psych/visitors/test_yaml_tree.rb:27]:
Expected /hello\x00world/ to match "--- !binary |-\n  aGVsbG8Ad29ybGQ=\n".

(中略)

  7) Failure:
TestPsych#test_parse_file [/extdisk/chkbuild/chkbuild/tmp/build/20160701T080500Z/ruby/test/psych/test_psych.rb:157]:
Expected: "hello\u0000world"
  Actual: "hello world"

  8) Failure:
TestPsych#test_load_file [/extdisk/chkbuild/chkbuild/tmp/build/20160701T080500Z/ruby/test/psych/test_psych.rb:143]:
Expected: "hello\u0000world"
  Actual: "hello world"

  9) Failure:
TestRDocMarkupIndentedParagraph#test_text [/extdisk/chkbuild/chkbuild/tmp/build/20160701T080500Z/ruby/test/rdoc/test_rdoc_markup_indented_paragraph.rb:42]:
Expected: "hello\u0000world"
  Actual: "hello world"

 10) Failure:
TestRDocMarkupParagraph#test_text [/extdisk/chkbuild/chkbuild/tmp/build/20160701T080500Z/ruby/test/rdoc/test_rdoc_markup_paragraph.rb:21]:
Expected: "hello\u0000world"
  Actual: "hello world"

 11) Failure:
TestRDocMarkupToJoinedParagraph#test_accept_paragraph [/extdisk/chkbuild/chkbuild/tmp/build/20160701T080500Z/ruby/test/rdoc/test_rdoc_markup_to_joined_paragraph.rb:19]:
Expected: [para: "hello\u0000world"]
  Actual: [para: "hello world"]

 12) Failure:
TestRDocMarkupToJoinedParagraph#test_accept_paragraph_break [/extdisk/chkbuild/chkbuild/tmp/build/20160701T080500Z/ruby/test/rdoc/test_rdoc_markup_to_joined_paragraph.rb:29]:
--- expected
+++ actual
@@ -1 +1 @@
-[para: "hello\u0000world", [break], "everyone"]
+[para: "hello world", [break], "everyone"]


 13) Failure:
TestSprintf#test_named_default [/extdisk/chkbuild/chkbuild/tmp/build/20160701T080500Z/ruby/test/ruby/test_sprintf.rb:428]:
<"hello\u0000world"> expected but was
<"hello world">.

 14) Failure:
TestString#test_prepend [/extdisk/chkbuild/chkbuild/tmp/build/20160701T080500Z/ruby/test/ruby/test_string.rb:2307]:
<"hello\u0000world"> expected but was
<"hello world">.

 15) Failure:
TestString2#test_prepend [/extdisk/chkbuild/chkbuild/tmp/build/20160701T080500Z/ruby/test/ruby/test_string.rb:2307]:
<"hello\u0000world"> expected but was
<"hello world">.

原因のひとつは、r55547 にて、CのAPIでのencodingを指定しない文字列作成時には、末尾の'\0'用の領域を(UTF-32が来るかもしれないので)4バイトの可能性を考慮するようにしたため、従来よりEMBED可能な文字列長が3バイト短くなる場合が出てきたためですが、(これ自体は後で可能な限りは修正したいと思います)

もうひとつの原因は、下記のように Fiddle::TestPointer#test_to_s および test_to_str 内でメモリを破壊しているせいのようです。

test/fiddle/test_pointer.rb 内の Fiddle::TestPointer#test_to_str および test_to_s には、以下のような記述が存在します。

    def test_to_str
      str = "hello world"
      ptr = Pointer[str]
###(snip)
      ptr[5] = 0
      assert_equal "hello\0world", ptr.to_str
    end

    def test_to_s
      str = "hello world"
      ptr = Pointer[str]
###(snip)
      ptr[5] = 0
      assert_equal 'hello', ptr.to_s
    end

両メソッド内にて、ptr[5] = 0 のようにメモリを直接書き換えています。

しかし、最近のRubyでは、同一内容の文字列リテラルは1個に集約され、別オブジェクトであっても可能な限り同一メモリ空間を参照するようになっているため、
上記のように Fiddle::Pointer#[]= を使ってメモリの内容を(Rubyインタプリタの正規の手段を踏まずに)直接書き換えてしまうと、同じ文字列リテラル "hello world" を使用する他の全ての箇所に影響が及び、(Fiddle::TestPointerとまったく無関係な)様々な場所で文字列比較が失敗することになります。

r55547より前で大丈夫だったのは、"hello world"がちょうど11バイトと32ビット環境でのEMBED文字列の最大長と同じであり、EMBED文字列としてstruct RStringに埋め込まれたものをFiddle::Pointerで書き換えていたため、大元のリテラル格納場所のメモリには影響が及ばなかったためです。(64ビット環境では、EMBED可能な長さはもっと長いため今でも大丈夫)

Fiddleは、その内容を理解した人のみが使うべき達人専用モジュールであるのは確かですが、
Rubyインタプリタの内部データ構造に過剰に依存するのも面倒ですので、
このテストは、他と完全に独立していることが確実に保証された文字列を使用するように書き換えたいと思います。
(たとえば、Marshal.dumpしてからloadするなど?)

Actions #1

Updated by ngoto (Naohisa Goto) over 7 years ago

  • Status changed from Open to Closed

Applied in changeset r55554.


  • test/fiddle/test_pointer.rb (test_to_str, test_to_s, test_aref_aset):
    Attempt to use independent strings for destructive tests that
    directly modify values on memory by using Fiddle::Pointer.
    [Bug #12537] [ruby-dev:49700]

Updated by nagachika (Tomoyuki Chikanaga) over 7 years ago

  • Backport changed from 2.1: UNKNOWN, 2.2: UNKNOWN, 2.3: UNKNOWN to 2.1: WONTFIX, 2.2: REQUIRED, 2.3: REQUIRED

Updated by nagachika (Tomoyuki Chikanaga) over 7 years ago

  • Backport changed from 2.1: WONTFIX, 2.2: REQUIRED, 2.3: REQUIRED to 2.1: WONTFIX, 2.2: REQUIRED, 2.3: DONE

ruby_2_3 r55989 merged revision(s) 55554.

Updated by usa (Usaku NAKAMURA) over 7 years ago

  • Backport changed from 2.1: WONTFIX, 2.2: REQUIRED, 2.3: DONE to 2.1: WONTFIX, 2.2: DONE, 2.3: DONE

ruby_2_2 r56302 merged revision(s) 55554.

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0