Re: [RFC PATCH v2 11/20] objtool: arm64: Walk instructions and compute CFI for each instruction

Chen Zhongjin
2022-05-24T13:45:29Z

Hi,

On 2022/5/24 8:16, madvenka@linux.microsoft.com wrote:
> From: "Madhavan T. Venkataraman" <madvenka@linux.microsoft.com>
>
> Implement arch_initial_func_cfi_state() to initialize the CFI for a
> function.
>
> Add code to fpv_decode() to walk the instructions in every function and
> compute the CFI information for each instruction.
>
> Implement special handling for cases like jump tables.
>
> Signed-off-by: Madhavan T. Venkataraman <madvenka@linux.microsoft.com>
> ---
> tools/objtool/arch/arm64/decode.c | 15 +++
> tools/objtool/fpv.c | 204 ++++++++++++++++++++++++++++++
> 2 files changed, 219 insertions(+)
...
> +static void update_cfi_state(struct cfi_state *cfi, struct stack_op *op)
> +{
> + struct cfi_reg *cfa = &cfi->cfa;
> + struct cfi_reg *regs = cfi->regs;
> +
> + if (op->src.reg == CFI_SP) {
> + if (op->dest.reg == CFI_SP)
> + cfa->offset -= op->src.offset;
> + else
> + regs[CFI_FP].offset = -cfa->offset + op->src.offset;
Seems wrong here, we don't have any op->src.offset for [mov x29, sp] so here we
get: fp->offset = -cfa->offset. The dumped info also proves this.

> + case UNWIND_HINT_TYPE_CALL:
> + /* Normal call */
> + frame->cfa += orc->sp_offset;
> + fp = frame->cfa + orc->fp_offset;
> + break;
Obviously this is not conform to the reliability check because we get
frame->cfa == fp here.

IIUC your sp_offset equals to stack length, and fp_offset is offset from next
x29 to next CFA. So maybe here we should have
regs[CFI_FP].offset = regs[CFI_SP].offset for [mov x29, sp].

Anyway, in original objtool sp_offset and fp_offset both represents the offset
from CFA to REGs. I think it's better not spoiling their original meaning and
just extending.