`name` is used via `RSTRING_PTR` within rb_str_catf, which may allocate
and thus potentially trigger GC. Although `name` is still referenced
by a local variable, the compiler might optimize away the reference
before the GC sees it, especially under aggressive optimization or when
debugging tools like ASAN are used.
This patch adds an explicit `RB_GC_GUARD` to ensure `name` is kept alive
until after the last use.
While it's not certain this is the root cause of the following observed
use-after-poison ASAN error, I believe this fix is indeed needed and
hopefully a likely candidate for preventing the error.
```
==1960369==ERROR: AddressSanitizer: use-after-poison on address 0x7ec6a00f1d88 at pc 0x5fb5bcafcf2e bp 0x7ffcc1178cb0 sp 0x7ffcc1178470
READ of size 61 at 0x7ec6a00f1d88 thread T0
#0 0x5fb5bcafcf2d in __asan_memcpy (/tmp/ruby/build/trunk_asan/ruby+0x204f2d) (BuildId: 6d92c84a27b87cfd253c38eeb552593f215ffb3d)
#1 0x5fb5bcde1fa5 in memcpy /usr/include/x86_64-linux-gnu/bits/string_fortified.h:29:10
#2 0x5fb5bcde1fa5 in ruby_nonempty_memcpy /tmp/ruby/src/trunk_asan/include/ruby/internal/memory.h:758:16
#3 0x5fb5bcde1fa5 in ruby__sfvwrite /tmp/ruby/src/trunk_asan/sprintf.c:1083:9
#4 0x5fb5bcde1521 in BSD__sprint /tmp/ruby/src/trunk_asan/vsnprintf.c:318:8
#5 0x5fb5bcde0fbc in BSD_vfprintf /tmp/ruby/src/trunk_asan/vsnprintf.c:1215:3
#6 0x5fb5bcdde4b1 in ruby_vsprintf0 /tmp/ruby/src/trunk_asan/sprintf.c:1164:5
#7 0x5fb5bcddd648 in rb_str_vcatf /tmp/ruby/src/trunk_asan/sprintf.c🔢5
#8 0x5fb5bcddd648 in rb_str_catf /tmp/ruby/src/trunk_asan/sprintf.c:1245:11
#9 0x5fb5bcf97c67 in location_format /tmp/ruby/src/trunk_asan/vm_backtrace.c:462:9
#10 0x5fb5bcf97c67 in location_to_str /tmp/ruby/src/trunk_asan/vm_backtrace.c:493:12
#11 0x5fb5bcf90a37 in location_to_str_dmyarg /tmp/ruby/src/trunk_asan/vm_backtrace.c:795:12
#12 0x5fb5bcf90a37 in backtrace_collect /tmp/ruby/src/trunk_asan/vm_backtrace.c:786:28
#13 0x5fb5bcf90a37 in backtrace_to_str_ary /tmp/ruby/src/trunk_asan/vm_backtrace.c:804:9
#14 0x5fb5bcf90a37 in rb_backtrace_to_str_ary /tmp/ruby/src/trunk_asan/vm_backtrace.c:816:9
#15 0x5fb5bd335b25 in exc_backtrace /tmp/ruby/src/trunk_asan/error.c:1904:15
#16 0x5fb5bd335b25 in rb_get_backtrace /tmp/ruby/src/trunk_asan/error.c:1924:16
```
https://ci.rvm.jp/results/trunk_asan@ruby-sp1/5810304
Follow up to fix 3b7373fd00.
In that commit, the line number in the first frame was overwritten after
the whole backtrace was created. There was a problem that the line
number was overwritten even if the location was backpatched.
Instead, this commit uses first_lineno if the frame is
VM_FRAME_MAGIC_DUMMY when generating the backtrace.
Before the patch:
```
$ ./miniruby -e '[1, 2].inject(:tap)'
-e:in '<main>': wrong number of arguments (given 1, expected 0) (ArgumentError)
from -e:1:in 'Enumerable#inject'
from -e:1:in '<main>'
```
After the patch:
```
$ ./miniruby -e '[1, 2].inject(:tap)'
-e:1:in '<main>': wrong number of arguments (given 1, expected 0) (ArgumentError)
from -e:1:in 'Enumerable#inject'
from -e:1:in '<main>'
```
This changeset suppresses backtrace locations like
`<internal:array>:211` as much as possible.
Before the patch:
```
$ ruby -e '[1].fetch_values(42)'
<internal:array>:211:in 'Array#fetch': index 42 outside of array bounds: -1...1 (IndexError)
from <internal:array>:211:in 'block in Array#fetch_values'
from <internal:array>:211:in 'Array#map!'
from <internal:array>:211:in 'Array#fetch_values'
from -e:1:in '<main>'
```
After the patch:
```
$ ./miniruby -e '[1].fetch_values(42)'
-e:1:in 'Array#fetch_values': index 42 outside of array bounds: -1...1 (IndexError)
from -e:1:in '<main>'
```
Specifically:
* The special backtrace handling of BUILTIN_ATTR_C_TRACE is now always
applied to frames with `<internal:...>`.
* When multiple consecutive internal frames appear, all but the bottom
(caller-side) frame are removed.
[Misc #20968]
In preparation for using it to update not only cfunc frames but also
internal frames, the function (and related variable names) are chagned.
I felt that the word "backpatch" is more appropriate than the more
general verb "update" here.
Previously, a user of the debug inspector API was supposed to use
`rb_debug_inspector_backtrace_locations` to get an array of
`Thread::Backtrace::Location`, and then used its index to get more
information from `rb_debug_inspector _frame_binding_get(index)`, etc.
However, `rb_debug_inspector_backtrace_locations` returned an array of
backtraces excluding rescue/ensure frames. On the other hand,
`rb_debug_inspector_frame_binding_get(index)` interprets index with
rescue/ensure frames. This led to inconsistency of the index, and it was
very difficult to correctly use the debug inspector API.
This is a minimal fix for the issue by making
`rb_debug_inspector_backtrace_locations` return a raw backtrace
including rescue/ensure frames.
Previously, Ruby displayed backtraces for each thread on deadlock. However, it has not been shown since Ruby 3.0.
It should display the backtrace for debugging.
Co-authored-by: Jeremy Evans <code@jeremyevans.net>
* YJIT: Replace Array#each only when YJIT is enabled
* Add comments about BUILTIN_ATTR_C_TRACE
* Make Ruby Array#each available with --yjit as well
* Fix all paths that expect a C location
* Use method_basic_definition_p to detect patches
* Copy a comment about C_TRACE flag to compilers
* Rephrase a comment about add_yjit_hook
* Give METHOD_ENTRY_BASIC flag to Array#each
* Add --yjit-c-builtin option
* Allow inconsistent source_location in test-spec
* Refactor a check of BUILTIN_ATTR_C_TRACE
* Set METHOD_ENTRY_BASIC without touching vm->running
With verbopse mode (-w), the interpreter shows a warning if
a block is passed to a method which does not use the given block.
Warning on:
* the invoked method is written in C
* the invoked method is not `initialize`
* not invoked with `super`
* the first time on the call-site with the invoked method
(`obj.foo{}` will be warned once if `foo` is same method)
[Feature #15554]
`Primitive.attr! :use_block` is introduced to declare that primitive
functions (written in C) will use passed block.
For minitest, test needs some tweak, so use
ea9caafc07
for `test-bundled-gems`.
Using rb_gc_mark_movable and a reference update function, we can make
frame infos movable in memory, and avoid pinning frame info backtraces.
```
require "objspace"
exceptions = []
GC.disable
50_000.times do
begin
raise "some exception"
rescue => exception
exception.backtrace_locations
exceptions << exception
end
end
GC.enable
GC.compact
p ObjectSpace.dump_all(output: :string).lines.grep(/"pinned":true/).count
```
Co-authored-by: Peter Zhu <peter@peterzhu.ca>
[Feature #13557]
Setting the backtrace with an array of strings is lossy. The resulting
exception will return nil on `#backtrace_locations`.
By accepting an array of `Backtrace::Location` instance, we can rebuild
a `Backtrace` instance and have a fully functioning Exception.
Co-Authored-By: Étienne Barrié <etienne.barrie@gmail.com>
This `st_table` is used to both mark and pin classes
defined from the C API. But `vm->mark_object_ary` already
does both much more efficiently.
Currently a Ruby process starts with 252 rooted classes,
which uses `7224B` in an `st_table` or `2016B` in an `RArray`.
So a baseline of 5kB saved, but since `mark_object_ary` is
preallocated with `1024` slots but only use `405` of them,
it's a net `7kB` save.
`vm->mark_object_ary` is also being refactored.
Prior to this changes, `mark_object_ary` was a regular `RArray`, but
since this allows for references to be moved, it was marked a second
time from `rb_vm_mark()` to pin these objects.
This has the detrimental effect of marking these references on every
minors even though it's a mostly append only list.
But using a custom TypedData we can save from having to mark
all the references on minor GC runs.
Addtionally, immediate values are now ignored and not appended
to `vm->mark_object_ary` as it's just wasted space.
This frees FL_USER0 on both T_MODULE and T_CLASS.
Note: prior to this, FL_SINGLETON was never set on T_MODULE,
so checking for `FL_SINGLETON` without first checking that
`FL_TYPE` was `T_CLASS` was valid. That's no longer the case.
```
test.rb:1:in 'Object#toplevel_meth': unhandled exception
from test.rb:4:in 'Foo.class_meth'
from test.rb:6:in 'Foo#instance_meth'
from test.rb:11:in 'singleton_meth'
from test.rb:13:in '<main>'
```
[Feature #19117]
Instead of having iseq and cfunc separately, this change lets
Thread::Backtrace::Location have them together as
rb_callable_method_entry_t.
This is a refactoring, but also a preparation for implementing
[Feature #19117].
When using M:N threads, EC is set to NULL in the shared native thread
when nothing is scheduled. This previously caused a segfault when we try
to examine the EC.
Returning 0 instead means we may miss profiling information, but a
profiler relying on this isn't thread aware anyways, and observing that
"nothing" is running is probably correct.
Fixes [Bug #20017]
Co-authored-by: Dustin Brown <dbrown9@gmail.com>
rb_backtrace_t is 32B, so it fits well in a 80B slot.
There is some unused spaces but given Backtrace objects are
rarely held onto it should be inconsequential and avoid
the malloc churn.
Co-Authored-By: Étienne Barrié <etienne.barrie@gmail.com>
The struct is 16B, so they will use the 80B size pool, so on paper it
wastes 80 - 32 - 16 = 52B, however most malloc implementations will
either pad sizes or use an extra 16B for each segment, so in practice
the waste isn't that big. Also `Backtrace::Location` are rarely held
on for long, so avoiding the malloc churn help performance.
Co-Authored-By: Étienne Barrié <etienne.barrie@gmail.com>
Add a new API rb_profile_thread_frames(), which is essentialy a
per-thread version of rb_profile_frames().
While the original rb_profile_frames() always returns results about the
current active thread obtained by GET_EC(), this new API takes a Thread
to be profiled as an argument.
This should come in handy when profiling I/O-bound programs such as
webapps, since this new API allows us to learn about Threads performing
I/O (which do not have the GVL).
Profiling worker threads (such as Sidekiq workers) may be another
application.
Implements [Feature #10602]
Co-authored-by: Mike Perham <mike@perham.net>
Frames pushed by YJIT have an unreliable PC. The PC could be garbage,
and if we try to read the line number with a garbage PC, then the
program can crash.
This commit returns line 0 for programs where there is a `jit_return`
function. If `jit_return` has been set then this frame was pushed by
the JIT, and we cannot trust the PC.
Here is a debugger session for a program that crashed due to a broken
PC:
```
(lldb) p ruby_current_vm_ptr->ractor.main_thread->ec->cfp->iseq->body->iseq_encoded
(VALUE *) $0 = 0x0000000118a30e00
(lldb) p/x ruby_current_vm_ptr->ractor.main_thread->ec->cfp->pc
(const VALUE *) $1 = 0x0000600000b02d00
(lldb) p/x ruby_current_vm_ptr->ractor.main_thread->ec->cfp->jit_return
(void *) $2 = 0x000000010622942c
```
You can see the PC is completely out of range, but there is a
`jit_return` pointer so we can avoid this crash.
According to the C99 specification section 7.20.3.2 paragraph 2:
> If ptr is a null pointer, no action occurs.
So we do not need to check that the pointer is a null pointer.
Right now the attached object is stored as an instance variable
and all the call sites that either get or set it have to know how it's
stored.
It's preferable to hide this implementation detail behind accessors
so that it is easier to change how it's stored.
Backtrace objects hold references to:
* iseqs - via the captured locations
* strary - a lazily allocated array of strings
* locary - a lazily allocated array of backtrace locations
Co-authored-by: Adam Hess <HParker@github.com>
StackProf uses a signal handler to call `rb_profile_frames`. Signals
are delivered to threads randomly, and can be delivered after the thread
has been created but before the CFP has been established on the EC.
This commit returns early if there is no CFP to use.
Here is some info from the core files we are seeing. Here you can see
the CFP on the current EC is 0x0:
```
(gdb) p ruby_current_ec
$20 = (struct rb_execution_context_struct *) 0x7f3481301b50
(gdb) p ruby_current_ec->cfp
$21 = (rb_control_frame_t *) 0x0
```
Here is where VM_FRAME_CFRAME_P gets a 0x0 CFP:
```
6 VM_FRAME_CFRAME_P (cfp=0x0) at vm_core.h:1350
7 VM_FRAME_RUBYFRAME_P (cfp=<optimized out>) at vm_core.h:1350
8 rb_profile_frames (start=0, limit=2048, buff=0x7f3493809590, lines=0x7f349380d590) at vm_backtrace.c:1587
```
Down the stack we can see this is happening after thread creation:
```
19 0x00007f3495bf9420 in <signal handler called> () at /lib/x86_64-linux-gnu/libpthread.so.0
20 0x000055d531574e55 in thread_start_func_2 (th=<optimized out>, stack_start=<optimized out>) at thread.c:676
21 0x000055d531575b31 in thread_start_func_1 (th_ptr=<optimized out>) at thread_pthread.c:1170
22 0x00007f3495bed609 in start_thread (arg=<optimized out>) at pthread_create.c:477
23 0x00007f3495b12133 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
```
The following new debug context APIs are for implementing debugger's
`next` (step over) and similar functionality.
* `rb_debug_inspector_frame_depth(dc, index)` returns `index`-th
frame's depth.
* `rb_debug_inspector_current_depth()` returns current frame depth.
The frame depth is not related to the frame index because debug
context API skips some special frames but proposed `_depth()` APIs
returns the count of all frames (raw depth).
This patch pushes dummy frames when loading code for the
profiling purpose.
The following methods push a dummy frame:
* `Kernel#require`
* `Kernel#load`
* `RubyVM::InstructionSequence.compile_file`
* `RubyVM::InstructionSequence.load_from_binary`
https://bugs.ruby-lang.org/issues/18559
The `rb_profile_frames` API did not skip the two dummy frames that
each thread has at its beginning. This was unlike `backtrace_each` and
`rb_ec_parcial_backtrace_object`, which do skip them.
This does not seem to be a problem for non-main thread frames,
because both `VM_FRAME_RUBYFRAME_P(cfp)` and
`rb_vm_frame_method_entry(cfp)` are NULL for them.
BUT, on the main thread `VM_FRAME_RUBYFRAME_P(cfp)` was true
and thus the dummy thread was still included in the output of
`rb_profile_frames`.
I've now made `rb_profile_frames` skip this extra frame (like
`backtrace_each` and friends), as well as add a test that asserts
the size and contents of `rb_profile_frames`.
Fixes [Bug #18907] (<https://bugs.ruby-lang.org/issues/18907>)
Use ISEQ_BODY macro to get the rb_iseq_constant_body of the ISeq. Using
this macro will make it easier for us to change the allocation strategy
of rb_iseq_constant_body when using Variable Width Allocation.
All frames should be either iseq frames or cfunc frames. Use a
VM assert instead of a conditional to check for a cfunc frame if
the current frame is not an iseq frame.
Fixes this compiler warning:
warning: 'loc' may be used uninitialized in this function [-Wmaybe-uninitialized]
bt_yield_loc(loc - cfunc_counter, cfunc_counter, btobj);
This method takes a block and yields Thread::Backtrace::Location
objects to the block. It does not take arguments, and always
starts at the default frame that caller_locations would start at.
Implements [Feature #16663]
