oracle / truffleruby

A high performance implementation of the Ruby programming language, built on GraalVM.
https://www.graalvm.org/ruby/
Other
3.02k stars 185 forks source link

Loofah test suite failing on truffleruby-head #2649

Closed flavorjones closed 2 years ago

flavorjones commented 2 years ago

Sometime between these two versions:

the test suite for Loofah started failing, mostly with this error message:

Loofah::HTML::DocumentFragment::bad scrub method#test_0001_raise a ScrubberNotFound exception:
ArgumentError: not enough arguments for required
    /home/runner/.rubies/truffleruby-head/lib/truffle/truffle/cext.rb:1091:in `rb_exc_raise'
    exception.c:29:in `rb_exc_raise'
    /home/runner/.rubies/truffleruby-head/lib/cext/include/truffleruby/truffleruby.h:234:in `rb_html_document_s_new'
    /home/runner/.rubies/truffleruby-head/lib/truffle/truffle/cext_ruby.rb:[41](https://github.com/flavorjones/loofah/runs/6211940003?check_suite_focus=true#step:4:41):in `new'
    /home/runner/work/loofah/loofah/lib/loofah/html/document_fragment.rb:19:in `parse'
    /home/runner/work/loofah/loofah/test/unit/test_scrubbers.rb:8:in `test_0001_raise a ScrubberNotFound exception'

If I have some time this morning I'll try to git-bisect it (it's about 27 commits from 0b02c3cb..a6d2e0bb).

flavorjones commented 2 years ago

Git bisect reveals 3a8f5196e6a51dc6143ba3d480a984d1cb1e09e5 is the first bad commit.

Tagging @aardvark179 for awareness.

commit 3a8f5196e6a51dc6143ba3d480a984d1cb1e09e5
Author: Duncan MacGregor <duncan.macgregor@oracle.com>
Date:   Thu Feb 24 13:17:21 2022 +0000

    Refactor C processing of Ruby args for constant argument descriptors.

 lib/cext/include/truffleruby/truffleruby.h | 92 ++++++++++++------------------
 src/main/c/cext/args.c                     | 44 ++++++++++++++
 2 files changed, 81 insertions(+), 55 deletions(-)
aardvark179 commented 2 years ago

Thanks for that, I'll take a look.

flavorjones commented 2 years ago

Reproduced this on the Nokogiri test suite (which Loofah depends on and subclasses heavily) using 22.2.0-dev-dc3fc7ef:

ArgumentError: not enough arguments for required
    /home/flavorjones/code/oss/truffleruby-ws/graal/sdk/mxbuild/linux-amd64/GRAALVM_67F33FC705_JAVA11/graalvm-67f33fc705-java11-22.2.0-dev/languages/ruby/lib/truffle/truffle/cext.rb:1091:in `rb_exc_raise'
    exception.c:29:in `rb_exc_raise'
    /home/flavorjones/code/oss/truffleruby-ws/graal/sdk/mxbuild/linux-amd64/GRAALVM_67F33FC705_JAVA11/graalvm-67f33fc705-java11-22.2.0-dev/languages/ruby/lib/cext/include/truffleruby/truffleruby.h:234:in `new'
    /home/flavorjones/code/oss/truffleruby-ws/graal/sdk/mxbuild/linux-amd64/GRAALVM_67F33FC705_JAVA11/graalvm-67f33fc705-java11-22.2.0-dev/languages/ruby/lib/truffle/truffle/cext_ruby.rb:41:in `new'
    /home/flavorjones/code/oss/nokogiri/lib/nokogiri/xml/builder.rb:312:in `initialize'
    /home/flavorjones/code/oss/nokogiri/test/namespaces/test_namespaces_in_builder_doc.rb:10:in `setup'
flavorjones commented 2 years ago

Smallest possible repro?

../truffleruby-ws/truffleruby/bin/jt ruby -Ilib -rnokogiri -S irb
Using Interpreted TruffleRuby: mxbuild/truffleruby-jvm
$ /home/flavorjones/code/oss/truffleruby-ws/truffleruby/mxbuild/truffleruby-jvm/languages/ruby/bin/ruby \
  --experimental-options \
  --core-load-path=/home/flavorjones/code/oss/truffleruby-ws/truffleruby/src/main/ruby/truffleruby \
  -Ilib \
  -rnokogiri \
  -S \
  irb

>> Nokogiri::XML::Document.new
array.c:82:in `rb_ary_entry': Message not supported. (Polyglot::ForeignException)
    from /home/flavorjones/code/oss/nokogiri/ext/nokogiri/xml_document.c:397:in `new'
    from /home/flavorjones/code/oss/truffleruby-ws/graal/sdk/mxbuild/linux-amd64/GRAALVM_67F33FC705_JAVA11/graalvm-67f33fc705-java11-22.2.0-dev/languages/ruby/lib/truffle/truffle/cext_ruby.rb:41:in `<unknown>'
    from /home/flavorjones/code/oss/truffleruby-ws/graal/sdk/mxbuild/linux-amd64/GRAALVM_67F33FC705_JAVA11/graalvm-67f33fc705-java11-22.2.0-dev/languages/ruby/lib/truffle/truffle/cext_ruby.rb:41:in `Nokogiri::XML::Document.new'
    from (irb):1:in `<top (required)>'
    from <internal:core> core/kernel.rb:407:in `Kernel#loop'
    from <internal:core> core/throw_catch.rb:36:in `Kernel#catch'
    from <internal:core> core/throw_catch.rb:36:in `Kernel#catch'
    from /home/flavorjones/code/oss/truffleruby-ws/graal/sdk/mxbuild/linux-amd64/GRAALVM_67F33FC705_JAVA11/graalvm-67f33fc705-java11-22.2.0-dev/languages/ruby/lib/gems/gems/irb-1.3.5/exe/irb:11:in `<top (required)>'
    from <internal:core> core/kernel.rb:376:in `Truffle::KernelOperations.load'
    from <internal:core> core/kernel.rb:376:in `Kernel#load'
    from /home/flavorjones/code/oss/truffleruby-ws/graal/sdk/mxbuild/linux-amd64/GRAALVM_67F33FC705_JAVA11/graalvm-67f33fc705-java11-22.2.0-dev/languages/ruby/bin/irb:42:in `<main>'

>> Nokogiri::XML::Document.new
/home/flavorjones/code/oss/truffleruby-ws/graal/sdk/mxbuild/linux-amd64/GRAALVM_67F33FC705_JAVA11/graalvm-67f33fc705-java11-22.2.0-dev/languages/ruby/lib/truffle/truffle/cext.rb:1091:in `rb_exc_raise': not enough arguments for required (ArgumentError)
    from exception.c:29:in `rb_exc_raise'
    from /home/flavorjones/code/oss/truffleruby-ws/graal/sdk/mxbuild/linux-amd64/GRAALVM_67F33FC705_JAVA11/graalvm-67f33fc705-java11-22.2.0-dev/languages/ruby/lib/cext/include/truffleruby/truffleruby.h:234:in `new'
    from /home/flavorjones/code/oss/truffleruby-ws/graal/sdk/mxbuild/linux-amd64/GRAALVM_67F33FC705_JAVA11/graalvm-67f33fc705-java11-22.2.0-dev/languages/ruby/lib/truffle/truffle/cext_ruby.rb:41:in `new'
    from (irb):3:in `<top (required)>'
    from <internal:core> core/kernel.rb:407:in `loop'
    from <internal:core> core/throw_catch.rb:36:in `catch'
    from <internal:core> core/throw_catch.rb:36:in `catch'
    from /home/flavorjones/code/oss/truffleruby-ws/graal/sdk/mxbuild/linux-amd64/GRAALVM_67F33FC705_JAVA11/graalvm-67f33fc705-java11-22.2.0-dev/languages/ruby/lib/gems/gems/irb-1.3.5/exe/irb:11:in `<top (required)>'
    from <internal:core> core/kernel.rb:376:in `load'
    from <internal:core> core/kernel.rb:376:in `load'
    from /home/flavorjones/code/oss/truffleruby-ws/graal/sdk/mxbuild/linux-amd64/GRAALVM_67F33FC705_JAVA11/graalvm-67f33fc705-java11-22.2.0-dev/languages/ruby/bin/irb:42:in `<main>'

Note that the exception raised is different during the first method call; the second method call demonstrates the exception that shows up in the test suites.

aardvark179 commented 2 years ago

Looks like this has uncovered an entirely separate bug. One of the argument counts appears to have been accidentally overwritten by VALUE, the post argument count has some marker bits I recognise. I'll track down what's doing this.

aardvark179 commented 2 years ago

Found the problem. Testing a fix in CI at the moment.

aardvark179 commented 2 years ago

The fix for this has now been merged to master (https://github.com/oracle/truffleruby/commit/4abd9b2475eb83ccbb39b6742c6c730170ab69fc).

flavorjones commented 2 years ago

Thank you!