kinst: add kinst_excluded()

[christos]

Summary

Exclude functions that are not safe-to-trace.

[christos]

No longer search for push %rbp, just exclude the exception handlers.
This way we can trace safe functions that do not push %rbp, such as
cpu_switch().

[christos]

Surround amd64 exception handlers in an ifdef.

[christos]

Exclude cpu_switch().

[christos]

Add RISC-V exception handlers.

[christos]

Remove unused KINST_PUSHL_RBP define.

[christos]

Exclude unsafe arm64 exception handlers.

[christos]

return 1 not 0

[markj]

So we should certainly exclude dtrace_* like FBT does. Same for unwind_frame(). I'd suggest posting that as its own patch, since it's needed independent of whether we remove the push %rbp check.

Right now we still have the check for push %rbp at the beginning of each function, which covers most of the rest. Where exactly did we land on that? I know that there are traceable functions where push %rbp is not the first instruction, but perhaps there's another way to handle them?

[christos]

In D39229#916539, @markj wrote:

Right now we still have the check for push %rbp at the beginning of each function, which covers most of the rest. Where exactly did we land on that? I know that there are traceable functions where push %rbp is not the first instruction, but perhaps there's another way to handle them?

I think it's better to not check for push %rbp at all, so we do not ignore safe to trace functions that just have push %rbp after the first instruction, or do not have it at all but are safe to trace, and instead exclude functions that are known to be unsafe-to-trace. I tested this patch with
quite a lot of functions that do not have push %rbp (https://github.com/christosmarg/omitrbp) and haven't gotten any crash so far with this exclusion list.

[christos]

In D39229#916539, @markj wrote:

I'd suggest posting that as its own patch, since it's needed independent of whether we remove the push %rbp check.

I'm not sure these 2 changes are unrelated. Basically the push %rbp check was our "exclusion" mechanism, now replaced by kinst_excluded().

[markj]

In D39229#916821, @christos wrote:

In D39229#916539, @markj wrote:

I'd suggest posting that as its own patch, since it's needed independent of whether we remove the push %rbp check.

I'm not sure these 2 changes are unrelated. Basically the push %rbp check was our "exclusion" mechanism, now replaced by kinst_excluded().

The "push %rbp" check isn't sufficient though. Today you can do this: dtrace -n 'kinst::dtrace_probe:0'. This doesn't work very well.

I would like to at least fix that problem before reconsidering the push %rbp check.

[christos]

In D39229#916822, @markj wrote:

The "push %rbp" check isn't sufficient though. Today you can do this: dtrace -n 'kinst::dtrace_probe:0'. This doesn't work very well.

I would like to at least fix that problem before reconsidering the push %rbp check.

Okay, I can split it into two patches.

[christos]

Get rid of the 'push rbp' removal in this patch.

[christos]

Remove amd64 exception handlers.

[christos]

Add kinst_md_excluded() as well.

[markj]

sys/cddl/dev/kinst/kinst.c

87

This is handled by the push rbp check. Only amd64 has that check currently, but I think riscv should perform that as well, see my comments in that review.

[christos]

Remove cpu_switch() from kinst_excluded().

[markj]

Approved

[christos]

Closed by commit 9c80ad6839cd30ecfeff2fb946d86888815da600, but had to do it manually because the commit message ate up the 'D' from 'Differential Revision'...