summaryrefslogtreecommitdiff
path: root/gc.c
diff options
context:
space:
mode:
authorPeter Zhu <peter@peterzhu.ca>2023-02-24 09:20:14 -0500
committerPeter Zhu <peter@peterzhu.ca>2023-02-24 14:10:09 -0500
commit3e098224077e8c43a1d8c2070b26ffdfda422780 (patch)
treeac701b8c89d90f3e6cd632ce22d0713d149ba945 /gc.c
parentd2631c427ee723f6136ac1e08dd3c9c5b04c6725 (diff)
Fix incorrect line numbers in GC hook
If the previous instruction is not a leaf instruction, then the PC was incremented before the instruction was ran (meaning the currently executing instruction is actually the previous instruction), so we should not increment the PC otherwise we will calculate the source line for the next instruction. This bug can be reproduced in the following script: ``` require "objspace" ObjectSpace.trace_object_allocations_start a = 1.0 / 0.0 p [ObjectSpace.allocation_sourceline(a), ObjectSpace.allocation_sourcefile(a)] ``` Which outputs: [4, "test.rb"] This is incorrect because the object was allocated on line 10 and not line 4. The behaviour is correct when we use a leaf instruction (e.g. if we replaced `1.0 / 0.0` with `"hello"`), then the output is: [10, "test.rb"]. [Bug #19456]
Notes
Notes: Merged: https://github.com/ruby/ruby/pull/7357
Diffstat (limited to 'gc.c')
-rw-r--r--gc.c21
1 files changed, 19 insertions, 2 deletions
diff --git a/gc.c b/gc.c
index 75770d0c2f..dcd248f8aa 100644
--- a/gc.c
+++ b/gc.c
@@ -100,6 +100,7 @@
#include "id_table.h"
#include "internal.h"
#include "internal/class.h"
+#include "internal/compile.h"
#include "internal/complex.h"
#include "internal/cont.h"
#include "internal/error.h"
@@ -2485,8 +2486,24 @@ gc_event_hook_body(rb_execution_context_t *ec, rb_objspace_t *objspace, const rb
{
const VALUE *pc = ec->cfp->pc;
if (pc && VM_FRAME_RUBYFRAME_P(ec->cfp)) {
- /* increment PC because source line is calculated with PC-1 */
- ec->cfp->pc++;
+ int prev_opcode = rb_vm_insn_addr2opcode((void *)*ec->cfp->iseq->body->iseq_encoded);
+ for (const VALUE *insn = ec->cfp->iseq->body->iseq_encoded; insn < pc; insn += rb_insn_len(prev_opcode)) {
+ prev_opcode = rb_vm_insn_addr2opcode((void *)*insn);
+ }
+
+ /* If the previous instruction is a leaf instruction, then the PC is
+ * the currently executing instruction. We should increment the PC
+ * because the source line is calculated with PC-1 in calc_pos.
+ *
+ * If the previous instruction is not a leaf instruction, then the PC
+ * was incremented before the instruction was ran (meaning the
+ * currently executing instruction is actually the previous
+ * instruction), so we should not increment the PC otherwise we will
+ * calculate the source line for the next instruction.
+ */
+ if (rb_insns_leaf_p(prev_opcode)) {
+ ec->cfp->pc++;
+ }
}
EXEC_EVENT_HOOK(ec, event, ec->cfp->self, 0, 0, 0, data);
ec->cfp->pc = pc;