Raising a RuntimeError when calling Kernel#binding from a C/non-ruby frame. #3449
Conversation
oracle-contributor-agreement
bot
added
the
OCA Verified
| @@ -334,6 +337,18 @@ RubyBinding binding(Frame callerFrame, Object self, Object[] rubyArgs, RootCallT | |||
| @Cached( | |||
| value = "getAdoptedNode(this).getEncapsulatingSourceSection()", | |||
| allowUncached = true, neverDefault = false) SourceSection sourceSection) { | |||
|
|
|||
| final MaterializedFrame materializedCallerFrame = Truffle.getRuntime() | |||
There was a problem hiding this comment.
This is what the materializedCallerFrame looks like in the debugger:
Compared to when called from Ruby:
| @@ -334,6 +337,18 @@ RubyBinding binding(Frame callerFrame, Object self, Object[] rubyArgs, RootCallT | |||
| @Cached( | |||
| value = "getAdoptedNode(this).getEncapsulatingSourceSection()", | |||
| allowUncached = true, neverDefault = false) SourceSection sourceSection) { | |||
|
|
|||
| final MaterializedFrame materializedCallerFrame = Truffle.getRuntime() | |||
| .iterateFrames(f -> f.getFrame(FrameInstance.FrameAccess.MATERIALIZE).materialize(), 1); | |||
There was a problem hiding this comment.
We definitely can't do iterateFrames in Kernel#binding.
Kernel#binding is very well optimized currently and iterateFrames would completely ruin its performance.
One way to detect C extension methods quickly is #3436 (comment).
But also I don't think this matters. We do have currently a Ruby frame for C extension methods, so it's only fair to return a Binding for that case.
The aim of this new exception is to not give an empty/unconnected binding for situations where there isn't a real binding.
For instance method(:binding).call is such a case, because Method#call is implemented in C/Java and so there is no way to get a proper Binding for Method#call, hence there it's clearer to raise an exception.
As mentioned in #3436 (comment), we should raise the new exception for method(:binding).call and add a spec for that.
And let's leave C extension methods (& rb_funcall) alone, i.e. let them return a Binding as they do, because there is a real Ruby frame in there.
There was a problem hiding this comment.
Feel free to try #3436 (comment) in this PR.
I tried it locally and test fast passed so maybe it's OK, it would need a full CI run to know better.
It's too risky for 24.0 but we can always merge it for 24.1.
Ruby 3.2 compatibility.
Addresses "Kernel#binding raises RuntimeError if called from a non-Ruby frame
(such as a method defined in C)." defined in #3039
CRuby: https://bugs.ruby-lang.org/issues/18487
Used the same stack walking as seen in https://github.com/Shopify/truffleruby/blob/90bfaa00401778f11849dbca05eb1bd008271c1b/src/main/java/org/truffleruby/language/arguments/ReadCallerVariablesNode.java#L46