Page MenuHomeFreeBSD

dtrace: Blacklist riscv exception handlers for fbt
ClosedPublic

Authored by kp on Dec 24 2020, 11:00 AM.

Details

Summary

We can't safely instrument those exception handlers, so blacklist them.

Test case: dtrace -n :::

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

kp requested review of this revision.Dec 24 2020, 11:00 AM
markj added inline comments.
sys/cddl/dev/fbt/fbt.c
175 ↗(On Diff #81123)

I think you can get away with explicitly specifying cpu_exception_handler and cpu_exception_handler_supervisor.

sys/cddl/dev/fbt/fbt.c
175 ↗(On Diff #81123)

We also need cpu_exception_handler_user. I can change this, but then we've got 4 entries (cpu_exception_handler, cpu_exception_handler_user, cpu_exception_handler_supervisor and do_trap_supervisor) rather than two.

sys/cddl/dev/fbt/fbt.c
175 ↗(On Diff #81123)

Why does cpu_exception_handler_user need to be excluded? If I understand correctly it is called only when handling user-mode exceptions and so shouldn't be callable from dtrace_probe().

sys/cddl/dev/fbt/fbt.c
175 ↗(On Diff #81123)

I don't know. I've tested without and wedge the machine doing dtrace -n :::, with cpu_exception_handler_user added to the exclude list that does work.

Perhaps the problem isn't so much about when the probe gets called as it is the assembly function not being safe to instrument.

sys/cddl/dev/fbt/fbt.c
175 ↗(On Diff #81123)

Perhaps the problem isn't so much about when the probe gets called as it is the assembly function not being safe to instrument.

That seems possible. I noticed that the code which identifies function entry probes on riscv simply looks for a store of ra to sp, ignoring a non-zero offset. The save_registers macro emits "sd ra, (TF_RA)(sp)" which seems like it could be misinterpreted as a function entry probe. I don't have riscv kernel disassembly handy but does a typical function prologue store ra to sp with offset 0? If so we could probably fix part of the problem by making fbt_provide_module_function() stricter.

sys/cddl/dev/fbt/fbt.c
175 ↗(On Diff #81123)

Functions typically start by setting the stack pointer:

ffffffc0000083ba <ck_rhs_map_create>:
{
ffffffc0000083ba:       7139                    addi    sp,sp,-64
        --v;
ffffffc0000083bc:       fc06                    sd      ra,56(sp)
ffffffc0000083be:       f822                    sd      s0,48(sp)
ffffffc0000083c0:       f426                    sd      s1,40(sp)
ffffffc0000083c2:       f04a                    sd      s2,32(sp)
ffffffc0000083c4:       ec4e                    sd      s3,24(sp)
ffffffc0000083c6:       e852                    sd      s4,16(sp)
ffffffc0000083c8:       e456                    sd      s5,8(sp)
ffffffc0000083ca:       e05a                    sd      s6,0(sp)
ffffffc0000083cc:       0080                    addi    s0,sp,64
ffffffc0000083ce:       15fd                    addi    a1,a1,-1
        v |= v >> 1;
ffffffc0000083d0:       0015d61b                srliw   a2,a1,0x1
ffffffc0000083d4:       8dd1                    or      a1,a1,a2
        v |= v >> 2;
sys/cddl/dev/fbt/fbt.c
175 ↗(On Diff #81123)

Yuck. So I guess this is not so easy to fix in FBT. In that case, could you please add a comment explaining why we have to exclude those functions?

Note why we can 't instrument assembly trap handlers.

This revision is now accepted and ready to land.Dec 27 2020, 4:23 PM