TestRubyLiteral::test_float failed with "path changed"

wks commented 5 months ago

TestRubyLiteral::test_float failed with the following stack trace:

/home/wks/code/ruby/gctest/failed_tests/test_float.rb:29:in 'Kernel#eval': SyntaxError#path changed (ArgumentError)
        from /home/wks/code/ruby/gctest/failed_tests/test_float.rb:29:in 'block (3 levels) in Object#test_float'
        from <internal:array>:54:in 'Array#each'
        from /home/wks/code/ruby/gctest/failed_tests/test_float.rb:18:in 'block (2 levels) in Object#test_float'
        from <internal:numeric>:238:in 'Integer#times'
        from /home/wks/code/ruby/gctest/failed_tests/test_float.rb:15:in 'block in Object#test_float'
        from <internal:array>:54:in 'Array#each'
        from /home/wks/code/ruby/gctest/failed_tests/test_float.rb:14:in 'Object#test_float'
        from /home/wks/code/ruby/gctest/failed_tests/test_float.rb:41:in '<main>'

wks commented 5 months ago

Similar errors are observed in other test cases, too. See https://github.com/mmtk/mmtk-ruby/actions/runs/9264075131/job/25483526354?pr=66

    1) Failure:
  TestRubyLiteral#test_string [/home/runner/work/mmtk-ruby/mmtk-ruby/git/ruby/test/ruby/test_literal.rb:71]:
  [ruby-core:39222] eval(?\c\u{��}).
  [SyntaxError] exception expected, not #<ArgumentError: SyntaxError#path changed: "(eval at /home/runner/work/mmtk-ruby/mmtk-ruby/git/ruby/test/ruby/test_literal.rb:71)"->"(eval at /home/runner/work/mmtk-ruby/mmtk-ruby/git/ruby/test/ruby/test_literal.rb:71)">.

    2) Error:
  TestRubyLiteral#test_integer:
  ArgumentError: SyntaxError#path changed: "(eval at /home/runner/work/mmtk-ruby/mmtk-ruby/git/ruby/test/ruby/test_literal.rb:607)"->"(eval at /home/runner/work/mmtk-ruby/mmtk-ruby/git/ruby/test/ruby/test_literal.rb:607)"
      /home/runner/work/mmtk-ruby/mmtk-ruby/git/ruby/test/ruby/test_literal.rb:607:in 'Kernel#eval'
      /home/runner/work/mmtk-ruby/mmtk-ruby/git/ruby/test/ruby/test_literal.rb:607:in 'block (3 levels) in TestRubyLiteral#test_integer'
      <internal:array>:54:in 'Array#each'
      /home/runner/work/mmtk-ruby/mmtk-ruby/git/ruby/test/ruby/test_literal.rb:599:in 'block (2 levels) in TestRubyLiteral#test_integer'
      <internal:numeric>:238:in 'Integer#times'
      /home/runner/work/mmtk-ruby/mmtk-ruby/git/ruby/test/ruby/test_literal.rb:596:in 'block in TestRubyLiteral#test_integer'
      <internal:array>:54:in 'Array#each'
      /home/runner/work/mmtk-ruby/mmtk-ruby/git/ruby/test/ruby/test_literal.rb:595:in 'TestRubyLiteral#test_integer'

My current investigation shows that this is related to GC and the frozen strings table. The root scanning failed to consider some live frozen strings as live, which caused them to be removed form the frozen strings table.

wks commented 5 months ago

Some upstream developers observed the same error. See:

wks commented 5 months ago

The bug I observed was caused by a bug in rb_mmtk_st_update_dedup_table. I introduced that function as an optimisation, a faster way to update hash tables after a copying GC. But I didn't take ENTRY_BASE into account. As a result, after a GC, it cleared the intended element in tab->bins[] but the wrong elemnet in tab->entries. That caused an existing string in the frozen string table to be considered non-existing. That string was inserted twice. That's why we see the old_path and path to be different even though both are interned fstrings. Most applications of frozen strings don't really care if the result of register_fstring is really an existing instance, so the bug remained unnoticed, until the code in syntax_error_with_path did an assertion and observed that they are not the same instance.

Since it is a bug I introduced, the error observed by upstream developers (related to https://github.com/ruby/ruby/commit/bc50f2a3f1d166be3899f32b81bb78f666000592 and https://github.com/ruby/ruby/commit/bc47ca5546a1287fbb052329075293c200d830b0) should be caused by another bug in vanilla CRuby.

mmtk / ruby

TestRubyLiteral::test_float failed with "path changed" #69