On Thu, Jun 13, 2019 at 03:00:55PM -0700, Alexei Starovoitov wrote:
> > @@ -392,8 +402,16 @@ bool unwind_next_frame(struct unwind_state *state)
> > * calls and calls to noreturn functions.
> > */
> > orc = orc_find(state->signal ? state->ip : state->ip - 1);
> > - if (!orc)
> > - goto err;
> > + if (!orc) {
> > + /*
> > + * As a fallback, try to assume this code uses a frame pointer.
> > + * This is useful for generated code, like BPF, which ORC
> > + * doesn't know about. This is just a guess, so the rest of
> > + * the unwind is no longer considered reliable.
> > + */
> > + orc = &orc_fp_entry;
> > + state->error = true;
>
> That seems fragile.
I don't think so. The unwinder has sanity checks to make sure it
doesn't go off the rails. And it works just fine. The beauty is that
it should work for all generated code (not just BPF).
> Can't we populate orc_unwind tables after JIT ?
As I mentioned it would introduce a lot more complexity. For each JIT
function, BPF would have to tell ORC the following:
- where the BPF function lives
- how big the stack frame is
- where RBP and other callee-saved regs are on the stack
There's a lot more fragility lurking there, compared to the above.
Not to mention the unwinder would need BPF-specific knowledge, unless we
created some generic abstraction for generated code to register their
functions (which we have actually considered in the past). But the
above approach is much simpler: just have all generated code use frame
pointers.
--
Josh
