Ticket #1009 (closed bug: fixed)

Opened 5 years ago

Last modified 5 years ago

examples/shootout/spectralnorm.pir shows a segfault in the continuation PMC set_pointer() VTABLE

Reported by: darbelo Owned by: chromatic
Priority: normal Milestone:
Component: none Version: trunk
Severity: medium Keywords:
Cc: Language:
Patch status: Platform: openbsd

Description

As the summary states, examples/shootout/spectralnorm.pir shows a segfault in the continuation PMC set_pointer() VTABLE. So far I've only seen this happen on OpenBSD amd64 and the failure does not show up in all runs of the program.

The cause for the segfault is an invalid 'pos' pointer, which chromatic stated on IRC points to a problem in in the exit op, specifically line 889 or 890 of src/ops/core.ops

For reference here is the backtrace:

#0  0x000000020f7149ac in Parrot_Continuation_set_pointer (interp=0x20b6b8c00, pmc=0x2014a1540, 
    value=0x2014a4000) at continuation.pmc:190
190             if (pos && (*pos == PARROT_OP_get_results_pc))
(gdb) bt
#0  0x000000020f7149ac in Parrot_Continuation_set_pointer (interp=0x20b6b8c00, pmc=0x2014a1540, 
    value=0x2014a4000) at continuation.pmc:190
#1  0x000000020f63c046 in new_ret_continuation_pmc (interp=0x20b6b8c00, address=0x2014a4000)
    at src/sub.c:68
#2  0x000000020f53f11d in Parrot_exit_ic (cur_opcode=0x2014a3ff0, interp=0x20b6b8c00) at core.ops:890
#3  0x000000020f634df6 in runops_slow_core (interp=0x20b6b8c00, runcore=0x206772c00, pc=0x2014a3ff0)
    at src/runcore/cores.c:955
#4  0x000000020f6332b2 in runops_int (interp=0x20b6b8c00, offset=210) at src/runcore/main.c:640
#5  0x000000020f5edd53 in runops (interp=0x20b6b8c00, offs=210) at src/call/ops.c:119
#6  0x000000020f5ee120 in runops_args (interp=0x20b6b8c00, sub=0x200d574d0, obj=0x2075525e0, 
    meth_unused=0x0, sig=0x20f90e9e2 "vP", ap=0x7f7ffffbd1b0) at src/call/ops.c:269
#7  0x000000020f5ee3f5 in Parrot_runops_fromc_args (interp=0x20b6b8c00, sub=0x200d574d0, 
    sig=0x20f90e9e2 "vP") at src/call/ops.c:338
#8  0x000000020f5c7cf3 in Parrot_runcode (interp=0x20b6b8c00, argc=1, argv=0x7f7ffffbd408)
    at src/embed.c:852
#9  0x000000020f7e4c6e in imcc_run_pbc (interp=0x20b6b8c00, obj_file=0, output_file=0x0, argc=1, 
    argv=0x7f7ffffbd408) at compilers/imcc/main.c:797
#10 0x000000020f7e5836 in imcc_run (interp=0x20b6b8c00, 
    sourcefile=0x7f7ffffbd8b7 "examples/shootout/spectralnorm.pir", argc=1, argv=0x7f7ffffbd408)
    at compilers/imcc/main.c:1088
#11 0x0000000000400eb5 in main (argc=1, argv=0x7f7ffffbd408) at src/main.c:60
(gdb) 

Attachments

exit_null.patch Download (1.4 KB) - added by NotFound 5 years ago.

Change History

  Changed 5 years ago by bacek

Ok, minimal example is

.sub main
   exit 0
.end

op exit tries to build retcontinuation for next-after-exit op using expr NEXT();. Problem is there is no "next" opcode. Workaround for this problem is just remove exit 0 from example. Proper solution - someone (read "chromatic") can patch IMCC to put "noop" after "exit".

-- Bacek

follow-up: ↓ 4   Changed 5 years ago by bacek

  • owner set to chromatic

Oookey. I've committed my version of fix for IMCC in r41397. Reassign to chromatic for reviewing fix.

Changed 5 years ago by NotFound

  Changed 5 years ago by NotFound

A more generic fix is to better check address at Continuation.set_pointer before dereferencing. The attached patch does that and partially reverts r41397. Can someone test it in OpenBSD amd64?

in reply to: ↑ 2   Changed 5 years ago by darbelo

  • status changed from new to closed
  • resolution set to fixed

Replying to bacek:

Oookey. I've committed my version of fix for IMCC in r41397. Reassign to chromatic for reviewing fix.

After r41397 I'm no longer seeing segfaults on this test. Closing ticket as fixed.

  Changed 5 years ago by chromatic

  • status changed from closed to reopened
  • resolution fixed deleted

Hang on -- I think NotFound's approach is much better. Rather than papering over the "reading outside a segment of memory" problem by extending memory by an op, let's check instead for "can we read outside this section of memory safely?"

follow-up: ↓ 7   Changed 5 years ago by bacek

I've committed fixed version of NotFound's patch at r41404.

-- Bacek.

in reply to: ↑ 6   Changed 5 years ago by darbelo

  • status changed from reopened to closed
  • resolution set to fixed

Replying to bacek:

I've committed fixed version of NotFound's patch at r41404. -- Bacek.

Tested after r41404 and the segfaults have not returned. With the proper fix in place, I'm re-closing this ticket.

Note: See TracTickets for help on using tickets.