Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Anonymous lvar #5035

Merged
merged 2 commits into from
Nov 7, 2021
Merged

Anonymous lvar #5035

merged 2 commits into from
Nov 7, 2021

Conversation

nobu
Copy link
Member

@nobu nobu commented Oct 27, 2021

No description provided.

@ko1 ko1 mentioned this pull request Oct 27, 2021
@casperisfine
Copy link
Contributor

Could we pull the test/ruby/test_iseq.rb changes from #4961 too?

@casperisfine
Copy link
Contributor

Could we pull the test/ruby/test_iseq.rb changes from #4961 too?

I tested locally they pass. 👍

@nobu
Copy link
Member Author

nobu commented Oct 27, 2021

Could we pull the test/ruby/test_iseq.rb changes from #4961 too?

Haven't they been included in dc8d349?

@casperisfine
Copy link
Contributor

Haven't they been included in dc8d349?

🤦 yes, yes they did.

@casperisfine
Copy link
Contributor

So I tried this branch against our app and it crashed inside error_highlight:

<internal:ast>:67: [BUG] Segmentation fault at 0x0000000000000008  ETA: 02:17:23
ruby 3.1.0dev (2021-10-27T14:49:12Z shopify 19f45dd2bc) [x86_64-linux]
 
-- Control frame information -----------------------------------------------
c:0164 p:0003 s:1107 e:001106 METHOD <internal:ast>:67
c:0163 p:0045 s:1100 e:001099 METHOD /usr/local/ruby/lib/ruby/3.1.0/error_highlight/core_ext.rb:19
c:0162 p:0004 s:1089 e:001088 METHOD /usr/local/ruby/lib/ruby/3.1.0/did_you_mean/core_ext/name_error.rb:15 [FINISH]
c:0161 p:---- s:1083 e:001082 CFUNC  :inspect
c:0160 p:0115 s:1079 e:001075 METHOD /app/lib/exception_reporter.rb:33
...
c:0011 p:0008 s:0058 e:000057 METHOD /tmp/bundle/ruby/3.1.0/gems/minitest-5.14.4/lib/minitest.rb:1029
c:0010 p:0017 s:0051 e:000050 BLOCK  /tmp/bundle/ruby/3.1.0/gems/ci-queue-0.21.0/lib/minitest/queue.rb:177
c:0009 p:0007 s:0048 e:000047 METHOD /tmp/bundle/ruby/3.1.0/gems/ci-queue-0.21.0/lib/minitest/queue.rb:168
c:0008 p:0004 s:0042 e:000041 METHOD /tmp/bundle/ruby/3.1.0/gems/ci-queue-0.21.0/lib/minitest/queue.rb:176
c:0007 p:0005 s:0038 e:000037 BLOCK  /tmp/bundle/ruby/3.1.0/gems/ci-queue-0.21.0/lib/minitest/queue.rb:231
c:0006 p:0030 s:0031 e:000030 METHOD /tmp/bundle/ruby/3.1.0/gems/ci-queue-0.21.0/lib/ci/queue/redis/worker.rb:52
c:0005 p:0006 s:0026 e:000025 METHOD /tmp/bundle/ruby/3.1.0/gems/ci-queue-0.21.0/lib/minitest/queue.rb:230
c:0004 p:0012 s:0020 e:000019 METHOD /tmp/bundle/ruby/3.1.0/gems/ci-queue-0.21.0/lib/minitest/queue.rb:215
c:0003 p:0142 s:0015 e:000014 METHOD /tmp/bundle/ruby/3.1.0/gems/minitest-5.14.4/lib/minitest.rb:141
c:0002 p:0073 s:0008 E:000f90 BLOCK  /tmp/bundle/ruby/3.1.0/gems/minitest-5.14.4/lib/minitest.rb:68 [FINISH]
c:0001 p:0000 s:0003 E:002220 (none) [FINISH]
 
-- Ruby level backtrace information ----------------------------------------
/tmp/bundle/ruby/3.1.0/gems/minitest-5.14.4/lib/minitest.rb:68:in `block in autorun'
/tmp/bundle/ruby/3.1.0/gems/minitest-5.14.4/lib/minitest.rb:141:in `run'
....
/app/lib/exception_reporter.rb:33:in `notify'
/app/lib/exception_reporter.rb:33:in `inspect'
/usr/local/ruby/lib/ruby/3.1.0/did_you_mean/core_ext/name_error.rb:15:in `to_s'
/usr/local/ruby/lib/ruby/3.1.0/error_highlight/core_ext.rb:19:in `to_s'
<internal:ast>:67:in `of'
 
-- Machine register context ------------------------------------------------
 RIP: 0x00005647bcbd60f4 RBP: 0x0000000000000000 RSP: 0x00007ffef1b77458
 RAX: 0x0000000000000000 RBX: 0x00007fa8f5d60280 RCX: 0x00007fa8fa67ad68
 RDX: 0x00007fa90906f000 RDI: 0x0000000000000008 RSI: 0x0000000000000000
  R8: 0x0000000000000008  R9: 0x00007fa90917a000 R10: 0x00007fa90906f000
 R11: 0x00007fa909006440 R12: 0x0000000000000008 R13: 0x00007fa8f5d60280
 R14: 0x0000000000000008 R15: 0x00005647bcbdb020 EFL: 0x0000000000010246
 
-- C level backtrace information -------------------------------------------
/usr/local/ruby/bin/real-ruby(rb_print_backtrace+0x11) [0x5647bcb9cd69] vm_dump.c:759
/usr/local/ruby/bin/real-ruby(rb_vm_bugreport) vm_dump.c:1045
/usr/local/ruby/bin/real-ruby(rb_bug_for_fatal_signal+0xec) [0x5647bc99701c] error.c:820
/usr/local/ruby/bin/real-ruby(sigsegv+0x4d) [0x5647bcaf21ad] signal.c:964
/lib/x86_64-linux-gnu/libpthread.so.0(__restore_rt+0x0) [0x7fa909bcf3c0]
/usr/local/ruby/bin/real-ruby(RB_FL_TEST_RAW+0x0) [0x5647bcbd60f4] array.c:1684
/usr/local/ruby/bin/real-ruby(RB_FL_ANY_RAW) ./include/ruby/internal/fl_type.h:558
/usr/local/ruby/bin/real-ruby(rb_array_len) ./include/ruby/internal/core/rarray.h:302
/usr/local/ruby/bin/real-ruby(rb_ary_entry_internal) internal/array.h:53
/usr/local/ruby/bin/real-ruby(rb_ary_entry) array.c:1685
/usr/local/ruby/bin/real-ruby(lex_array+0x20) [0x5647bcbdb040] ast.c:131
/usr/local/ruby/bin/real-ruby(lex_getline+0x6) [0x5647bca62012] parse.y:6414
/usr/local/ruby/bin/real-ruby(nextline) parse.y:6582
/usr/local/ruby/bin/real-ruby(nextc+0x19) [0x5647bca8c4a8] parse.y:6624
/usr/local/ruby/bin/real-ruby(parser_prepare) parse.y:8419
/usr/local/ruby/bin/real-ruby(yycompile0) parse.y:6318
/usr/local/ruby/bin/real-ruby(rb_suppress_tracing+0x114) [0x5647bcba2434] vm_trace.c:450
/usr/local/ruby/bin/real-ruby(yycompile+0x46) [0x5647bca6c6c2] parse.y:6373
/usr/local/ruby/bin/real-ruby(rb_parser_compile_generic) parse.y:6499
/usr/local/ruby/bin/real-ruby(ast_parse_done+0x0) [0x5647bcbdd1e8] ast.c:149
/usr/local/ruby/bin/real-ruby(ast_s_of) ast.c:150
/usr/local/ruby/bin/real-ruby(vm_invoke_builtin_delegate+0x31) [0x5647bcb921eb] vm_insnhelper.c:5720
/usr/local/ruby/bin/real-ruby(vm_exec_core) insns.def:1471
/usr/local/ruby/bin/real-ruby(rb_vm_exec+0xd3) [0x5647bcb80703] vm.c:2189
/usr/local/ruby/bin/real-ruby(vm_call0_body+0x528) [0x5647bcb887b8] vm_eval.c:192
/usr/local/ruby/bin/real-ruby(rb_funcallv_scope+0x1aa) [0x5647bcb8c47a] vm_eval.c:86
/usr/local/ruby/bin/real-ruby(RB_IMMEDIATE_P+0x0) [0x5647bcb0e6e7] string.c:1655
/usr/local/ruby/bin/real-ruby(RB_SPECIAL_CONST_P) ./include/ruby/internal/special_consts.h:262
/usr/local/ruby/bin/real-ruby(rbimpl_RB_TYPE_P_fastpath) ./include/ruby/internal/value_type.h:348
/usr/local/ruby/bin/real-ruby(RB_TYPE_P) ./include/ruby/internal/value_type.h:378
/usr/local/ruby/bin/real-ruby(rb_obj_as_string_result) string.c:1662
/usr/local/ruby/bin/real-ruby(rb_obj_as_string) string.c:1656
/usr/local/ruby/bin/real-ruby(rb_obj_as_string) string.c:1648
/usr/local/ruby/bin/real-ruby(exc_inspect+0x55) [0x5647bc995ed5] error.c:1339
/usr/local/ruby/bin/real-ruby(vm_call_cfunc_with_frame+0x12b) [0x5647bcb74e3b] vm_insnhelper.c:3025
/usr/local/ruby/bin/real-ruby(vm_call_method_each_type+0x79) [0x5647bcb81b59] vm_insnhelper.c:3541
/usr/local/ruby/bin/real-ruby(vm_call_method+0xb4) [0x5647bcb82494] vm_insnhelper.c:3665
/usr/local/ruby/bin/real-ruby(vm_sendish+0xc) [0x5647bcb8f6ef] vm_insnhelper.c:4651
/usr/local/ruby/bin/real-ruby(vm_exec_core) insns.def:777
/usr/local/ruby/bin/real-ruby(rb_vm_exec+0x9ad) [0x5647bcb80fdd] vm.c:2198
/usr/local/ruby/bin/real-ruby(vm_call0_body+0x528) [0x5647bcb887b8] vm_eval.c:192
/usr/local/ruby/bin/real-ruby(rb_funcallv_scope+0x1aa) [0x5647bcb8c47a] vm_eval.c:86
/tmp/bundle/ruby/3.1.0/bundler/gems/liquid-c-d38956d76b58/lib/liquid_c.so(vm_render_rescue+0x125) [0x7fa90137aca5] vm.c:527
/usr/local/ruby/bin/real-ruby(rb_vrescue2+0x2a7) [0x5647bc9a1867] eval.c:936
/usr/local/ruby/bin/real-ruby(rb_rescue2+0x8e) [0x5647bc9a195e] eval.c:884
/tmp/bundle/ruby/3.1.0/bundler/gems/liquid-c-d38956d76b58/lib/liquid_c.so(liquid_vm_render+0xc5) [0x7fa90137b105] vm.c:550
/tmp/bundle/ruby/3.1.0/bundler/gems/liquid-c-d38956d76b58/lib/liquid_c.so(block_body_render_to_output_buffer+0xe0) [0x7fa9013740d0] block.c:341
[0x5647bd265484]

master work fine though (as long as we disable iseq compilation)

@casperisfine
Copy link
Contributor

The following is broken though:

def foo
end
p RubyVM::AbstractSyntaxTree.of(method(:foo))

def foo(*)
end
p RubyVM::AbstractSyntaxTree.of(method(:foo))

def foo(**)
end
p RubyVM::AbstractSyntaxTree.of(method(:foo))

def foo(*, **)
end
p RubyVM::AbstractSyntaxTree.of(method(:foo))

RubyVM::InstructionSequence.load_from_binary(RubyVM::InstructionSequence.compile_file(__FILE__).to_binary).eval
$ ./ruby /tmp/ast.rb 
#<RubyVM::AbstractSyntaxTree::Node:SCOPE@1:0-2:3>
#<RubyVM::AbstractSyntaxTree::Node:SCOPE@5:0-6:3>
#<RubyVM::AbstractSyntaxTree::Node:SCOPE@9:0-10:3>
#<RubyVM::AbstractSyntaxTree::Node:SCOPE@13:0-14:3>
<internal:ast>:67:in `of': wrong argument type nil (expected Array) (TypeError)
        from /tmp/ast.rb:3:in `<main>'
        from /tmp/ast.rb:17:in `eval'
        from /tmp/ast.rb:17:in `<main>'

@nobu
Copy link
Member Author

nobu commented Oct 28, 2021

As it crashed also on the current master, seems a different bug.
I'll investigate it more.

@nobu
Copy link
Member Author

nobu commented Oct 31, 2021

The explanation in the commit log feels inaccurate or insufficient now.
The ID may be called “unregistered” rather than “unnamed”.

nobu and others added 2 commits November 7, 2021 01:37
```ruby
def foo(*); ->{ super }; end
```

This code makes anonymous parameters which is not registered as an
ID.  The problem is that when Ractors try to scan `getlocal`
instructions, it puts the Symbol corresponding to the parameter
in to a hash.  Since it is not registered, we end up with a
strange exception.  This commit wraps the unregistered ID in an
internal ID so that we get the same exception for `...` as `*`.

Co-Authored-By: Aaron Patterson <tenderlove@ruby-lang.org>
Co-Authored-By: John Hawthorn <john@hawthorn.email>
@nobu nobu merged commit ec657f4 into ruby:master Nov 7, 2021
@nobu nobu deleted the anonymous-lvar branch November 7, 2021 03:40
@mame
Copy link
Member

mame commented Nov 7, 2021

Just a quick note: this change seems not to work correctly on Solaris.

http://rubyci.s3.amazonaws.com/solaris10-gcc/ruby-master/log/20211107T120003Z.fail.html.gz

#1287 test_ractor.rb:1505:in `<top (required)>': 
     def foo(*); ->{ super }; end
     begin
       Ractor.make_shareable(foo)
     rescue Ractor::IsolationError
       "ok"
     end
  #=> "#<Proc:0xff1c4ef4 bootstraptest.tmp.rb:2 (lambda)>" (expected "ok")  
#1288 test_ractor.rb:1514:in `<top (required)>': 
     def foo(**); ->{ super }; end
     begin
       Ractor.make_shareable(foo)
     rescue Ractor::IsolationError
       "ok"
     end
  #=> "#<Proc:0xff1c4ef4 bootstraptest.tmp.rb:2 (lambda)>" (expected "ok")  
#1290 test_ractor.rb:1532:in `<top (required)>': 
     def foo((x), (y)); ->{ super }; end
     begin
       Ractor.make_shareable(foo([], []))
     rescue Ractor::IsolationError
       "ok"
     end
  #=> "#<Proc:0xff1c4eb8 bootstraptest.tmp.rb:2 (lambda)>" (expected "ok")  
FAIL 3/1639 tests failed

@mame
Copy link
Member

mame commented Nov 7, 2021

debian-i386 also failed with the same errors. Maybe this change does not work on 32 bit machines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants