* Josh Poimboeuf <jpoim...@redhat.com> wrote:

> +#ifndef _UNDWARF_TYPES_H
> +#define _UNDWARF_TYPES_H
> +
> +/*
> + * The UNDWARF_REG_* registers are base registers which are used to find 
> other
> + * registers on the stack.
> + *
> + * The CFA (call frame address) is the value of the stack pointer on the
> + * previous frame, i.e. the caller's SP before it called the callee.
> + *
> + * The CFA is usually based on SP, unless a frame pointer has been saved, in
> + * which case it's based on BP.
> + *
> + * BP is usually either based on CFA or is undefined (meaning its value 
> didn't
> + * change for the current frame).
> + *
> + * So the CFA base is usually either SP or BP, and the FP base is usually 
> either
> + * CFA or undefined.  The rest of the base registers are needed for special
> + * cases like entry code and gcc aligned stacks.
> + */
> +#define UNDWARF_REG_UNDEFINED                0
> +#define UNDWARF_REG_CFA                      1
> +#define UNDWARF_REG_DX                       2
> +#define UNDWARF_REG_DI                       3
> +#define UNDWARF_REG_BP                       4
> +#define UNDWARF_REG_SP                       5
> +#define UNDWARF_REG_R10                      6
> +#define UNDWARF_REG_R13                      7
> +#define UNDWARF_REG_BP_INDIRECT              8
> +#define UNDWARF_REG_SP_INDIRECT              9
> +#define UNDWARF_REG_MAX                      15
> +
> +/*
> + * UNDWARF_TYPE_CFA: Indicates that cfa_reg+cfa_offset points to the caller's
> + * stack pointer (aka the CFA in DWARF terms).  Used for all callable
> + * functions, i.e.  all C code and all callable asm functions.
> + *
> + * UNDWARF_TYPE_REGS: Used in entry code to indicate that cfa_reg+cfa_offset
> + * points to a fully populated pt_regs from a syscall, interrupt, or 
> exception.
> + *
> + * UNDWARF_TYPE_REGS_IRET: Used in entry code to indicate that
> + * cfa_reg+cfa_offset points to the iret return frame.
> + */
> +#define UNDWARF_TYPE_CFA             0
> +#define UNDWARF_TYPE_REGS            1
> +#define UNDWARF_TYPE_REGS_IRET               2
> +
> +/*
> + * This struct contains a simplified version of the DWARF Call Frame
> + * Information standard.  It contains only the necessary parts of the real
> + * DWARF, simplified for ease of access by the in-kernel unwinder.  It tells
> + * the unwinder how to find the previous SP and BP (and sometimes entry regs)
> + * on the stack for a given code address (IP).  Each instance of the struct
> + * corresponds to one or more code locations.
> + */
> +struct undwarf {
> +     short cfa_offset;
> +     short bp_offset;
> +     unsigned cfa_reg:4;
> +     unsigned bp_reg:4;
> +     unsigned type:2;
> +};

I never know straight away what 'CFA' stands for - could we please use natural 
names, i.e. something like:

struct undwarf {
        u16             sp_offset;
        u16             bp_offset;
        unsigned        sp_reg:4;
        unsigned        bp_reg:4;
        unsigned        type:2;
};

...

struct unwind_hint {
        u32             ip;
        u16             sp_offset;
        u8              sp_reg;
        u8              type;
};

?

Also note the slightly cleaner vertical alignment, plus the conversion to more 
stable data types: I believe various bits of tooling (perf and so) will 
eventually 
learn about undwarf, so having a well defined cross-arch data structure is 
probably of advantage.

Since we are not bound by DWARF anymore, we might as well use readable names 
and 
such?

Plus, shouldn't we use __packed for 'struct undwarf' to minimize the 
structure's 
size (to 6 bytes AFAICS?) - or is optimal packing of the main undwarf array 
already guaranteed on every platform with this layout?

Thanks,

        Ingo

Reply via email to