Skip to content

Commit 8f99034

Browse files
eregoneregon
committed
Remove duplication in rb_define_method() and rework rb_debug_inspector_backtrace_locations()
1 parent 0f68db7 commit 8f99034

File tree

8 files changed

+65
-82
lines changed

8 files changed

+65
-82
lines changed

‎lib/cext/include/truffleruby/truffleruby-abi-version.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,6 @@
1010
// $RUBY_VERSION must be the same as TruffleRuby.LANGUAGE_VERSION.
1111
// $ABI_NUMBER starts at 1 and is incremented for every ABI-incompatible change.
1212

13-
#define TRUFFLERUBY_ABI_VERSION "3.2.2.5"
13+
#define TRUFFLERUBY_ABI_VERSION "3.2.2.6"
1414

1515
#endif

‎lib/truffle/truffle/cext.rb

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2112,18 +2112,26 @@ def rb_imemo_tmpbuf_set_ptr(obj, ptr)
21122112
obj.pointer = ptr
21132113
end
21142114

2115-
def rb_debug_inspector_open_contexts
2116-
Truffle::Debug.get_frame_bindings.map do |binding|
2115+
RB_THREAD_CURRENT_BACKTRACE_LOCATIONS_OMIT = NFI ? 4 : 5
2116+
2117+
def rb_debug_inspector_open_contexts_and_backtrace
2118+
contexts = Truffle::Debug.get_frame_bindings.map do |binding|
21172119
if binding
21182120
[binding.receiver, Primitive.class(binding.receiver), binding]
21192121
else
21202122
[nil, nil, nil]
21212123
end
21222124
end.freeze
2123-
end
21242125

2125-
def rb_thread_current_backtrace_locations
2126-
Thread.current.backtrace_locations(4)
2126+
backtrace = Thread.current.backtrace_locations(RB_THREAD_CURRENT_BACKTRACE_LOCATIONS_OMIT)
2127+
2128+
if contexts.size != backtrace.size
2129+
STDERR.puts '', 'contexts:', contexts.map { |_,_,binding| binding.source_location.join(':') }
2130+
STDERR.puts '', 'backtrace:', backtrace, ''
2131+
raise 'debug_inspector contexts and backtrace lengths are not equal'
2132+
end
2133+
2134+
[contexts, backtrace]
21272135
end
21282136

21292137
def rb_syserr_new(errno, mesg)

‎lib/truffle/truffle/cext_ruby.rb

Lines changed: 17 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -20,49 +20,26 @@ def rb_define_method(mod, name, function, argc)
2020
raise ArgumentError, "arity out of range: #{argc} for -2..15"
2121
end
2222

23-
if NFI
24-
wrapper = RB_DEFINE_METHOD_WRAPPERS[argc]
25-
method_body = Truffle::Graal.copy_captured_locals -> *args, &block do
26-
if argc == -1 # (int argc, VALUE *argv, VALUE obj)
27-
args = [function, args.size, Truffle::CExt.RARRAY_PTR(args), Primitive.cext_wrap(self)]
28-
elsif argc == -2 # (VALUE obj, VALUE rubyArrayArgs)
29-
args = [function, Primitive.cext_wrap(self), Primitive.cext_wrap(args)]
30-
elsif argc >= 0 # (VALUE obj); (VALUE obj, VALUE arg1); (VALUE obj, VALUE arg1, VALUE arg2); ...
31-
if args.size != argc
32-
raise ArgumentError, "wrong number of arguments (given #{args.size}, expected #{argc})"
33-
end
34-
args = [function, Primitive.cext_wrap(self), *args.map! { |arg| Primitive.cext_wrap(arg) }]
23+
wrapper = RB_DEFINE_METHOD_WRAPPERS[argc]
24+
method_body = Truffle::Graal.copy_captured_locals -> *args, &block do
25+
if argc == -1 # (int argc, VALUE *argv, VALUE obj)
26+
args = [function, args.size, Truffle::CExt.RARRAY_PTR(args), Primitive.cext_wrap(self)]
27+
elsif argc == -2 # (VALUE obj, VALUE rubyArrayArgs)
28+
args = [function, Primitive.cext_wrap(self), Primitive.cext_wrap(args)]
29+
elsif argc >= 0 # (VALUE obj); (VALUE obj, VALUE arg1); (VALUE obj, VALUE arg1, VALUE arg2); ...
30+
if args.size != argc
31+
raise ArgumentError, "wrong number of arguments (given #{args.size}, expected #{argc})"
3532
end
36-
37-
exc = $!
38-
Primitive.thread_set_exception(nil)
39-
# We must set block argument if given here so that the
40-
# `rb_block_*` functions will be able to find it by walking the stack.
41-
res = Primitive.call_with_c_mutex_and_frame_and_unwrap(wrapper, args, Primitive.caller_special_variables_if_available, block)
42-
Primitive.thread_set_exception(exc)
43-
res
33+
args = [function, Primitive.cext_wrap(self), *args.map! { |arg| Primitive.cext_wrap(arg) }]
4434
end
45-
else
46-
method_body = Truffle::Graal.copy_captured_locals -> *args, &block do
47-
if argc == -1 # (int argc, VALUE *argv, VALUE obj)
48-
args = [args.size, Truffle::CExt.RARRAY_PTR(args), Primitive.cext_wrap(self)]
49-
elsif argc == -2 # (VALUE obj, VALUE rubyArrayArgs)
50-
args = [Primitive.cext_wrap(self), Primitive.cext_wrap(args)]
51-
elsif argc >= 0 # (VALUE obj); (VALUE obj, VALUE arg1); (VALUE obj, VALUE arg1, VALUE arg2); ...
52-
if args.size != argc
53-
raise ArgumentError, "wrong number of arguments (given #{args.size}, expected #{argc})"
54-
end
55-
args = [Primitive.cext_wrap(self), *args.map! { |arg| Primitive.cext_wrap(arg) }]
56-
end
5735

58-
exc = $!
59-
Primitive.thread_set_exception(nil)
60-
# We must set block argument if given here so that the
61-
# `rb_block_*` functions will be able to find it by walking the stack.
62-
res = Primitive.call_with_c_mutex_and_frame_and_unwrap(function, args, Primitive.caller_special_variables_if_available, block)
63-
Primitive.thread_set_exception(exc)
64-
res
65-
end
36+
exc = $!
37+
Primitive.thread_set_exception(nil)
38+
# We must set block argument if given here so that the
39+
# `rb_block_*` functions will be able to find it by walking the stack.
40+
res = Primitive.call_with_c_mutex_and_frame_and_unwrap(wrapper, args, Primitive.caller_special_variables_if_available, block)
41+
Primitive.thread_set_exception(exc)
42+
res
6643
end
6744

6845
# Even if the argc is -2, the arity number

‎spec/ruby/optional/capi/debug_spec.rb

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
describe "rb_debug_inspector_frame_self_get" do
1818
it "returns self" do
1919
@o.rb_debug_inspector_frame_self_get(0).should == @o
20+
@o.rb_debug_inspector_frame_self_get(1).should == self
2021
end
2122
end
2223

@@ -35,10 +36,14 @@
3536
end
3637

3738
it "matches the locations in rb_debug_inspector_backtrace_locations" do
38-
frames = @o.rb_debug_inspector_open(42);
39+
frames = @o.rb_debug_inspector_open(42)
3940
frames.each do |_s, _klass, binding, _iseq, backtrace_location|
4041
if binding
41-
"#{backtrace_location.path}:#{backtrace_location.lineno}".should == "#{binding.source_location[0]}:#{binding.source_location[1]}"
42+
binding.source_location.should == [backtrace_location.path, backtrace_location.lineno]
43+
method_name = binding.eval('__method__')
44+
if method_name
45+
method_name.should == backtrace_location.base_label.to_sym
46+
end
4247
end
4348
end
4449
end

‎src/main/c/cext/debug.c

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,10 @@ VALUE rb_debug_inspector_backtrace_locations(const rb_debug_inspector_t *dc) {
2323

2424
VALUE rb_debug_inspector_open(rb_debug_inspector_func_t func, void *data) {
2525
rb_debug_inspector_t dbg_context;
26-
VALUE backtrace = RUBY_CEXT_INVOKE("rb_thread_current_backtrace_locations");
27-
dbg_context.contexts = RUBY_CEXT_INVOKE("rb_debug_inspector_open_contexts");
28-
if (RARRAY_LENINT(backtrace) != RARRAY_LENINT(dbg_context.contexts)) {
29-
rb_raise(rb_eRuntimeError, "debug_inspector contexts and backtrace lengths are not equal");
30-
}
26+
VALUE contexts_and_backtrace = RUBY_CEXT_INVOKE("rb_debug_inspector_open_contexts_and_backtrace");
27+
VALUE contexts = RARRAY_AREF(contexts_and_backtrace, 0);
28+
VALUE backtrace = RARRAY_AREF(contexts_and_backtrace, 1);
29+
dbg_context.contexts = contexts;
3130
dbg_context.backtrace = backtrace;
3231
dbg_context.backtrace_size = RARRAY_LEN(backtrace);
3332
return (*func)(&dbg_context, data);

‎src/main/java/org/truffleruby/debug/TruffleDebugNodes.java

Lines changed: 13 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,13 @@
1313
import java.util.ArrayDeque;
1414
import java.util.ArrayList;
1515
import java.util.Collections;
16-
import java.util.Deque;
1716
import java.util.HashMap;
1817
import java.util.List;
1918
import java.util.Map;
2019

2120
import com.oracle.truffle.api.TruffleStackTrace;
2221
import com.oracle.truffle.api.TruffleStackTraceElement;
2322
import com.oracle.truffle.api.exception.AbstractTruffleException;
24-
import com.oracle.truffle.api.frame.Frame;
2523
import com.oracle.truffle.api.frame.FrameInstance.FrameAccess;
2624
import com.oracle.truffle.api.frame.MaterializedFrame;
2725
import com.oracle.truffle.api.interop.ArityException;
@@ -67,6 +65,7 @@
6765
import org.truffleruby.interop.BoxedValue;
6866
import org.truffleruby.interop.FromJavaStringNode;
6967
import org.truffleruby.interop.ToJavaStringNode;
68+
import org.truffleruby.language.CallStackManager;
7069
import org.truffleruby.language.ImmutableRubyObject;
7170
import org.truffleruby.core.string.ImmutableRubyString;
7271
import org.truffleruby.language.Nil;
@@ -76,7 +75,6 @@
7675
import org.truffleruby.language.arguments.NoKeywordArgumentsDescriptor;
7776
import org.truffleruby.language.arguments.RubyArguments;
7877
import org.truffleruby.language.backtrace.BacktraceFormatter;
79-
import org.truffleruby.language.backtrace.InternalRootNode;
8078
import org.truffleruby.language.control.RaiseException;
8179
import org.truffleruby.language.library.RubyStringLibrary;
8280
import org.truffleruby.language.methods.DeclarationContext;
@@ -1315,23 +1313,19 @@ public abstract static class IterateFrameBindingsNode extends CoreMethodArrayArg
13151313
@TruffleBoundary
13161314
@Specialization
13171315
RubyArray getFrameBindings() {
1318-
Deque<Pair<MaterializedFrame, SourceSection>> stack = new ArrayDeque<>();
1316+
var stack = new ArrayDeque<Pair<MaterializedFrame, SourceSection>>();
1317+
13191318
getContext().getCallStack().iterateFrameBindings(5, frameInstance -> {
13201319
final RootNode rootNode = ((RootCallTarget) frameInstance.getCallTarget()).getRootNode();
1321-
if (rootNode instanceof RubyRootNode) {
1322-
final Frame frame = frameInstance.getFrame(FrameAccess.MATERIALIZE);
1323-
final SourceSection sourceSection;
1324-
if (frameInstance.getCallNode() != null &&
1325-
BacktraceFormatter
1326-
.isAvailable(frameInstance.getCallNode().getEncapsulatingSourceSection())) {
1327-
sourceSection = frameInstance.getCallNode().getEncapsulatingSourceSection();
1328-
} else {
1329-
sourceSection = null;
1330-
}
1331-
stack.push(Pair.create(frame.materialize(), sourceSection));
1332-
} else if (!(rootNode instanceof InternalRootNode) &&
1333-
frameInstance.getCallNode().getEncapsulatingSourceSection() != null) {
1334-
stack.push(Pair.create(null, null));
1320+
var callNode = frameInstance.getCallNode();
1321+
var encapsulatingSourceSection = callNode == null ? null : callNode.getEncapsulatingSourceSection();
1322+
if (rootNode instanceof RubyRootNode && BacktraceFormatter.isAvailable(encapsulatingSourceSection)) {
1323+
final MaterializedFrame frame = frameInstance.getFrame(FrameAccess.MATERIALIZE).materialize();
1324+
assert frame.getFrameDescriptor().getDefaultValue() == nil;
1325+
assert CallStackManager.isRubyFrame(frame);
1326+
stack.push(Pair.create(frame, encapsulatingSourceSection));
1327+
} else {
1328+
stack.push(Pair.empty());
13351329
}
13361330
return null;
13371331
});
@@ -1354,8 +1348,7 @@ RubyArray getFrameBindings() {
13541348
} else {
13551349
sourceSection = lastAvailableSourceSection;
13561350
}
1357-
final RubyBinding binding = BindingNodes
1358-
.createBinding(getContext(), getLanguage(), frame, sourceSection);
1351+
RubyBinding binding = BindingNodes.createBinding(getContext(), getLanguage(), frame, sourceSection);
13591352
frameBindings.add(binding);
13601353
} else {
13611354
frameBindings.add(nil);

‎src/main/java/org/truffleruby/language/CallStackManager.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -149,17 +149,17 @@ private Frame getCallerFrame(Predicate<FrameInstance> filter, FrameAccess frameA
149149
return iterateFrames(1, filter, f -> f.getFrame(frameAccess));
150150
}
151151

152+
/** Does the same filtering as {@link Backtrace#getStackTrace(Throwable)} */
152153
@TruffleBoundary
153154
public void iterateFrameBindings(int skip, Function<FrameInstance, Object> action) {
154155
iterateFrames(skip, frameInstance -> {
155-
if (ignoreFrame(frameInstance.getCallNode(), (RootCallTarget) frameInstance.getCallTarget())) {
156+
Node callNode = frameInstance.getCallNode();
157+
var callTarget = (RootCallTarget) frameInstance.getCallTarget();
158+
if (ignoreFrame(callNode, callTarget)) {
156159
return false;
157160
}
158-
final RootNode rootNode = ((RootCallTarget) frameInstance.getCallTarget()).getRootNode();
159-
if (rootNode instanceof RubyRootNode || frameInstance.getCallNode() != null) {
160-
return true;
161-
}
162-
return false;
161+
final RootNode rootNode = callTarget.getRootNode();
162+
return rootNode instanceof RubyRootNode || callNode != null;
163163
}, action);
164164
}
165165

‎src/main/java/org/truffleruby/language/backtrace/Backtrace.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -214,10 +214,11 @@ private TruffleStackTraceElement[] getStackTrace(Throwable truffleException) {
214214
int retainedCount = 0;
215215
for (TruffleStackTraceElement stackTraceElement : fullStackTrace) {
216216
assert processedCount != 0 || stackTraceElement.getLocation() == location;
217-
final Node callNode = stackTraceElement.getLocation();
218217
++processedCount;
219218

220-
if (callStackManager.ignoreFrame(callNode, stackTraceElement.getTarget())) {
219+
Node callNode = stackTraceElement.getLocation();
220+
var callTarget = stackTraceElement.getTarget();
221+
if (callStackManager.ignoreFrame(callNode, callTarget)) {
221222
continue;
222223
}
223224

@@ -228,7 +229,7 @@ private TruffleStackTraceElement[] getStackTrace(Throwable truffleException) {
228229

229230
// TODO (eregon, 4 Feb 2019): we should not ignore foreign frames without a
230231
// call node, but print info based on the methodName and CallTarget.
231-
final RootNode rootNode = stackTraceElement.getTarget().getRootNode();
232+
final RootNode rootNode = callTarget.getRootNode();
232233
if (rootNode instanceof RubyRootNode || callNode != null) {
233234
stackTraceList.add(stackTraceElement);
234235
}

0 commit comments

Comments
 (0)