oracle / truffleruby

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

File.directory? calls to_i in RC15, but not in RC14 #1665

Closed rbotafogo closed 5 years ago

rbotafogo commented 5 years ago

The following code

Rscript --jvm --polyglot -e 'eval.polyglot("ruby", "\
$LOAD_PATH.unshift %q(/home/rbotafogo/desenv/galaaz/lib)\
require(%q(galaaz))\
class RbChunk\
end\
RbChunk.new\
RbChunk.instance_eval(%q(File.directory?(%q(.))))
")'

When executed in RC14 returns the expected result '[1] TRUE', since '.' is a directory and Ruby 'true' becomes R vector with 1 element 'TRUE'.

Attaching package: ‘rlang’

The following object is masked from ‘package:formula.tools’:

    env

Attaching package: ‘purrr’

The following objects are masked from ‘package:rlang’:

    %@%, as_function, flatten, flatten_chr, flatten_dbl, flatten_int,
    flatten_lgl, flatten_raw, invoke, list_along, modify, prepend,
    splice

[1] TRUE

Now, when ran against RC15 results in:

Attaching package: ‘rlang’

The following object is masked from ‘package:formula.tools’:

    env

Attaching package: ‘purrr’

The following objects are masked from ‘package:rlang’:

    %@%, as_function, flatten, flatten_chr, flatten_dbl, flatten_int,
    flatten_lgl, flatten_raw, invoke, list_along, modify, prepend,
    splice

Error in polyglot evaluation : Method to_i not found in R environment (NoMethodError)
    at <ruby> 
R::Support.eval(/home/rbotafogo/desenv/galaaz/lib/R_interface/rsupport.rb:89:3503-3580)
    at <ruby> 

R::Support.exec_function_name(/home/rbotafogo/desenv/galaaz/lib/R_interface/
      rsupport.rb:269:10237-10276)
        at <ruby> 

 R::Object#method_missing(/home/rbotafogo/desenv/galaaz/lib/R_interface/
      robject.rb:183:7683-7741)
 at <ruby> <top (required)>((eval):1:0-21)
 at <ruby> null(Unknown)

Note that although Galaaz was required, there is no call to anything in Galaaz. Class RbChunk was locally defined and is an empty class. Then I evaluate File.directory?('.') in the scope of RbChunk. I can't see why 'method_missing' is called with R::Object, since I'm giving a String '.' to File.directory?

I might have done something strange, but I really can't see it.

Adding the following code at the beginning of method_missing:

def method_missing(symbol, *args, &block)
      puts "******* method_missing"
      puts caller
      puts self
      puts symbol
      puts args
 ...
end

shows:

******* method_missing
["(eval):1:in `attach_function_eagerly'", "(eval):1:in `truffleposix_stat_mode'", "(eval):1:in 
`directory?'", "(eval):1:in `<top (required)>'", "<eval wrapper>:7:in `instance_eval'", "<eval 
wrapper>:7:in `<top (required)>'", "<REPL>:1:in `<repl wrapper>'", "<REPL>:1"]
uint32[[-2:-1]]
to_i
[]

I couldn't reproduce the error without loading Galaaz.

Thanks

eregon commented 5 years ago

Thank you for the report.

I'm confused by the following lines in galaaz master: https://github.com/rbotafogo/galaaz/blob/2e73b3019af705a2a00df0c507bfbabcee877c4a/lib/R_interface/rsupport.rb#L88-L94

    def self.eval(string)
      begin
        Polyglot.eval("R", string)
      rescue RuntimeError
        raise NoMethodError.new("Method #{string} not found in R environment")
      end
    end

Throwing a NoMethodError manually is already unusual, but then assuming it's a NoMethodError without checking what was the RuntimeError seems incorrect in general.

From the error above, it looks like galaaz is trying to Polyglot.eval("R", "to_i")?

rbotafogo commented 5 years ago

Benoit,

You are completely right... this is bad code and incorrect in general. Removing the rescue clause gives the following output:

===== *** method_missing ["(eval):1:in attach_function_eagerly'", "(eval):1:in truffleposix_stat_mode'", "(eval):1:in directory?'", "(eval):1:in<top (required)>'", ":7:in instance_eval'", "<eval wrapper>:7:in <top (required)>'", ":1:in `'", ":1"] uint32[[-2:-1]] to_i [] An internal error occurred. Please report an issue at https://github.com/oracle/fastr including the commands. You can rerun FastR with --R.PrintErrorStacktracesToFile=true to turn on internal errors logging. Please attach the log file to the issue if possible.

And you are also correct that the code is trying to call 'to_i'. Sorry if I'm repeating the question, but this just what I'm wondering, why is to_i being called in RC15 and not in RC14? The code is just calling File.directory?('.') which should be true and I don't see how it could reach method 'eval' in R::Support module.

Not trying to justify the bad code, but this rescue clause was added because for some reason unknown to me, Rspec was implicitly calling to_ary in an R::Object which ended up with the same 'internal error'. In robject.rb I`ve added the following code:

--------------------------------------------------------------------------------------

# @TODO: rspec sometimes calls to_ary when an expected value is false.

I still don't

understand this call. Returning an array with a number inside seems

to make

rspec happy, however this could have other consequences I don't know

of.

--------------------------------------------------------------------------------------

def to_ary
  [1]
end

If you have any clue as why this happens, it would be great!

Thanks...

Em ter, 16 de abr de 2019 às 19:29, Benoit Daloze notifications@github.com escreveu:

Thank you for the report.

I'm confused by the following lines in galaaz master:

https://github.com/rbotafogo/galaaz/blob/2e73b3019af705a2a00df0c507bfbabcee877c4a/lib/R_interface/rsupport.rb#L88-L94

def self.eval(string)
  begin
    Polyglot.eval("R", string)
  rescue RuntimeError
    raise NoMethodError.new("Method #{string} not found in R environment")
  end
end

Throwing a NoMethodError manually is already unusual, but then assuming it's a NoMethodError without checking what was the RuntimeError seems incorrect in general.

From the error above, it looks like galaaz is trying to Polyglot.eval("R", "to_i")?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/oracle/truffleruby/issues/1665#issuecomment-483867702, or mute the thread https://github.com/notifications/unsubscribe-auth/AD0H8e4DPoVdwu0OdsATbZZz3WX1YjdVks5vhk7FgaJpZM4cyjyw .

-- Rodrigo Botafogo

eregon commented 5 years ago

I'm trying to debug this in RC16, the first thing I noticed is the same issue as https://github.com/oracle/truffleruby/issues/1653:

Exception: `TypeError' /home/eregon/.rubies/graalvm-1.0.0-rc16/jre/languages/ruby/lib/mri/rubygems/bundler_version_finder.rb:51:in `coerce_to_failed' - Coercion error: nil.to_str => String failed

But then once I worked around that (with $0=%q(test.R)\ as the first line of the eval), I get the expected:

...
Attaching package: ‘rlang’

The following object is masked from ‘package:formula.tools’:

    env

Attaching package: ‘purrr’

The following objects are masked from ‘package:rlang’:

    %@%, as_function, flatten, flatten_chr, flatten_dbl, flatten_int,
    flatten_lgl, flatten_raw, invoke, list_along, modify, prepend,
    splice

[1] TRUE

That's using the latest Galaaz gem release (0.4.8).

eregon commented 5 years ago

I can reproduce with Galaaz master:

print("Hi")

res <- eval.polyglot("ruby", "\
$0=%q(test.R)\
$LOAD_PATH.unshift %q(/home/eregon/code/galaaz/lib)\
require(%q(galaaz))\
class RbChunk\
end\
RbChunk.new\
RbChunk.instance_eval(%q(File.directory?(%q(.))))
")

print(res)
$ Rscript --jvm --polyglot --ruby.debug --experimental-options --ruby.did-you-mean=false test.R
[1] "Hi"

Attaching package: ‘rlang’

The following object is masked from ‘package:formula.tools’:

    env

Attaching package: ‘purrr’

The following objects are masked from ‘package:rlang’:

    %@%, as_function, flatten, flatten_chr, flatten_dbl, flatten_int,
    flatten_lgl, flatten_raw, invoke, list_along, modify, prepend,
    splice

Exception `RuntimeError' at /home/eregon/code/galaaz/lib/R_interface/rsupport.rb:88:in `eval' - Error: object 'to_i' not found (RError)
Translated to internal error
Exception: `NoMethodError' /home/eregon/code/galaaz/lib/R_interface/rsupport.rb:90:in `eval' - Method to_i not found in R environment
Error in polyglot evaluation : Method to_i not found in R environment (NoMethodError)
    at <ruby> R::Support.eval(/home/eregon/code/galaaz/lib/R_interface/rsupport.rb:90:3566-3643)
    at <ruby> R::Support.exec_function_name(/home/eregon/code/galaaz/lib/R_interface/rsupport.rb:270:10300-10339)
    at <ruby> R::Object#method_missing(/home/eregon/code/galaaz/lib/R_interface/robject.rb:176:7561-7619)
    at <ruby> <top (required)>((eval):1:0-21)
    at <ruby> null(Unknown)
eregon commented 5 years ago

Kernel#puts is redefined in https://github.com/rbotafogo/galaaz/blob/a670116b0251a3c55ad4caac437ebed4e6be04fd/lib/R_interface/ruby_extensions.rb#L24-L31, that's a bit confusing and that's the reason we see the unexpected output for puts caller.

The actual issue is that https://github.com/rbotafogo/galaaz/blob/a670116b0251a3c55ad4caac437ebed4e6be04fd/lib/R_interface/ruby_extensions.rb#L178-L180 redefines Symbol#[] and that's used by our internal implementation of FFI to call truffleposix_stat_mode for File.directory? (nfi_return_type is a Symbol): https://github.com/oracle/truffleruby/blob/04afba28d4ab1bdbfd8d92027f1ccc48b9a50bcb/src/main/ruby/core/posix.rb#L121 Symbol#[] is a standard Ruby method: http://ruby-doc.org/core-2.6.3/Symbol.html#method-i-5B-5D

I can workaround in that code to use String#[] instead like nfi_return_type.to_s[-2..-1].to_i, and that fixes this example. I'll do that change in TruffleRuby. But this shows overriding Symbol#[] might have non-trivial consequences.

eregon commented 5 years ago

Other notes:

It's unfortunate that caller does not show core library files, but this is required for Ruby compatibility.

rbotafogo commented 5 years ago

I redefine Kernel#puts because for some weird reason if I don't do it, then nothing gets printed. Can you try running:

puts R.c(1, 2, 3)

with both puts redefined and not? You'll see that when puts is redefined it does print fine, without it nothing gets printed on the output.

I've realized that there is no need in my code to redefine Symbol#[], so I'm just removing it.... thanks for finding this.

Em dom, 28 de abr de 2019 às 16:52, Benoit Daloze notifications@github.com escreveu:

Kernel#puts is redefined in https://github.com/rbotafogo/galaaz/blob/a670116b0251a3c55ad4caac437ebed4e6be04fd/lib/R_interface/ruby_extensions.rb#L24-L31, that's a bit confusing and that's the reason we see the unexpected output for puts caller.

The actual issue is that https://github.com/rbotafogo/galaaz/blob/a670116b0251a3c55ad4caac437ebed4e6be04fd/lib/R_interface/ruby_extensions.rb#L178-L180 redefines Symbol#[] and that's used by our internal implementation of FFI to call truffleposix_stat_mode for File.directory? (nfi_return_type is a Symbol):

https://github.com/oracle/truffleruby/blob/04afba28d4ab1bdbfd8d92027f1ccc48b9a50bcb/src/main/ruby/core/posix.rb#L121 Symbol#[] is a standard Ruby method: http://ruby-doc.org/core-2.6.3/Symbol.html#method-i-5B-5D

I can workaround in that code to use String#[] instead like nfi_return_type.to_s[-2..-1].to_i, and that fixes this example. I'll do that change in TruffleRuby. But this shows overriding Symbol#[] might have non-trivial consequences.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/oracle/truffleruby/issues/1665#issuecomment-487410190, or mute the thread https://github.com/notifications/unsubscribe-auth/AA6QP4IN4KFROSLUWOW67QLPSX6BBANCNFSM4HGKHSYA .

-- Rodrigo Botafogo

eregon commented 5 years ago
  • To find out which file/line called to_i

Another and probably simpler way is using Truffle::Debug.print_backtrace along with --ruby.backtraces-hide-core-files=false on the command line:

/home/eregon/code/galaaz/lib/R_interface/rsupport.rb:90:in `print_backtrace'
    from /home/eregon/code/galaaz/lib/R_interface/rsupport.rb:90:in `eval'
    from /home/eregon/code/galaaz/lib/R_interface/rsupport.rb:273:in `exec_function_name'
    from /home/eregon/code/galaaz/lib/R_interface/robject.rb:176:in `method_missing'
    from resource:/truffleruby/core/posix.rb:121:in `attach_function_eagerly'
    from resource:/truffleruby/core/posix.rb:90:in `truffleposix_stat_mode'
    from resource:/truffleruby/core/file.rb:372:in `directory?'
        ...
rbotafogo commented 5 years ago

I'm not sure I understand that R::Object#method_missing can just call super. This method has a lot of code to call R when the method is missing, for example, R.c(1, 2, 3) will go through method missing.

Em dom, 28 de abr de 2019 às 17:08, Benoit Daloze notifications@github.com escreveu:

Other notes:

  • R::Object#method_missing can use just super or super(symbol, *args, &block) to trigger a NoMethodError, this will gives better information.
  • To find out which file/line called to_i, the easiest for now is with:

    --ruby.core-load-path=/path/to/truffleruby-checkout-of-the-same-version/src/main/ruby and that gives in this case:

/home/eregon/code/galaaz/lib/R_interface/rsupport.rb:272:in exec_function_name' /home/eregon/code/galaaz/lib/R_interface/robject.rb:176:inmethod_missing' /home/eregon/code/truffleruby-ws/truffleruby/src/main/ruby/core/posix.rb:121:in attach_function_eagerly' /home/eregon/code/truffleruby-ws/truffleruby/src/main/ruby/core/posix.rb:90:intruffleposix_stat_mode' /home/eregon/code/truffleruby-ws/truffleruby/src/main/ruby/core/file.rb:372:in directory?' (eval):1:in<top (required)>'

:8:in `instance_eval' :8:in `' /home/eregon/tmp/test.R:3:in `' /home/eregon/tmp/test.R:1 (truffleruby/src/main/ruby/core/posix.rb:121 being the one calling to_i) It's unfortunate that caller does not show core library files, but this is required for Ruby compatibility. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub , or mute the thread .

-- Rodrigo Botafogo

eregon commented 5 years ago

I redefine Kernel#puts because for some weird reason if I don't do it, then nothing gets printed. Can you try running: puts R.c(1, 2, 3) with both puts redefined and not? You'll see that when puts is redefined it does print fine, without it nothing gets printed on the output.

You are right, I can reproduce, that must be a bug. I'll file a bug.

I'm not sure I understand that R::Object#method_missing can just call super. This method has a lot of code to call R when the method is missing, for example, R.c(1, 2, 3) will go through method missing.

I meant for the to_ary case, so raise NoMethodError if name == "to_ary" can become super(symbol, *args, &block) if symbol == :to_ary which is better for debugging information (the exception knows the method name and the receiver and has a proper message).

eregon commented 5 years ago

@rbotafogo I filed https://github.com/oracle/truffleruby/issues/1679.

eregon commented 5 years ago

I merged a fix to use String#[] instead of Symbol#[] in Truffle::POSIX in 70c7144. It will be in the next release. @rbotafogo Thank you for the report!