2022-09-07 11:24:42

by Qing Zhang

[permalink] [raw]
Subject: [PATCH v3 10/10] LoongArch/ftrace: Fix unwind state when option func_stack_trace

Ftrace plays like function head exception, prologue analysis will stop soon
because PC is at entry.

90000000004c5a54 <callee>:
90000000004c5a54: 03400000 andi $zero, $zero, 0x0 ==>move t0, ra
90000000004c5a58: 03400000 andi $zero, $zero, 0x0 ==>bl callsite
==>90000000004c5a5c: 02fcc063 addi.d $sp, $sp, -208(0xf30)
...

When encountering ftrace_call, save trace function ra at PT_ERA, save parent ra
at PT_R1, At this time, pc is the position after the two nops of callee. There is
no conventional prologue operation between this position and the function entry,
so we need to resetthe first flag to make the caller continue to unwind.

testing method:
echo path_openat > ./set_ftrace_filter
echo 1 > ./options/func_stack_trace
echo function > ./current_tracer

Signed-off-by: Qing Zhang <[email protected]>
---
arch/loongarch/include/asm/unwind.h | 2 +-
arch/loongarch/kernel/unwind_prologue.c | 33 +++++++++++++++++++++----
2 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/arch/loongarch/include/asm/unwind.h b/arch/loongarch/include/asm/unwind.h
index f66b07c3e6a1..f2b52b9ea93d 100644
--- a/arch/loongarch/include/asm/unwind.h
+++ b/arch/loongarch/include/asm/unwind.h
@@ -20,7 +20,7 @@ struct unwind_state {
char type; /* UNWINDER_XXX */
struct stack_info stack_info;
struct task_struct *task;
- bool first, error;
+ bool first, error, is_ftrace;
int graph_idx;
unsigned long sp, pc, ra;
};
diff --git a/arch/loongarch/kernel/unwind_prologue.c b/arch/loongarch/kernel/unwind_prologue.c
index 03f8b31a90cc..0bb19d3ae8ba 100644
--- a/arch/loongarch/kernel/unwind_prologue.c
+++ b/arch/loongarch/kernel/unwind_prologue.c
@@ -14,9 +14,7 @@ unsigned long unwind_get_return_address(struct unwind_state *state)

if (unwind_done(state))
return 0;
- else if (state->type)
- return state->pc;
- else if (state->first)
+ else if (state->type || state->first)
return state->pc;

return *(unsigned long *)(state->sp);
@@ -42,16 +40,39 @@ static bool unwind_by_guess(struct unwind_state *state)
return false;
}

+static void unwind_state_fixup(struct unwind_state *state)
+{
+ static unsigned long ftrace_case = (unsigned long)ftrace_call + 4;
+
+ if (state->pc == ftrace_case)
+ state->is_ftrace = true;
+}
+
static bool unwind_by_prologue(struct unwind_state *state)
{
struct stack_info *info = &state->stack_info;
union loongarch_instruction *ip, *ip_end;
unsigned long frame_size = 0, frame_ra = -1;
unsigned long size, offset, pc = state->pc;
+ struct pt_regs *regs;

if (state->sp >= info->end || state->sp < info->begin)
return false;

+ if (state->is_ftrace) {
+ /*
+ * As we meet ftrace_regs_entry, reset first flag like first doing
+ * tracing, Prologue analysis will stop soon because PC is at entry.
+ */
+ regs = (struct pt_regs *)state->sp;
+ state->pc = regs->csr_era;
+ state->ra = regs->regs[1];
+ state->sp = regs->regs[3];
+ state->first = true;
+ state->is_ftrace = false;
+ return true;
+ }
+
if (!kallsyms_lookup_size_offset(pc, &size, &offset))
return false;

@@ -97,7 +118,7 @@ static bool unwind_by_prologue(struct unwind_state *state)

state->pc = *(unsigned long *)(state->sp + frame_ra);
state->sp = state->sp + frame_size;
- return !!__kernel_text_address(state->pc);
+ goto out;

first:
state->first = false;
@@ -106,7 +127,9 @@ static bool unwind_by_prologue(struct unwind_state *state)

state->pc = state->ra;

- return !!__kernel_text_address(state->ra);
+out:
+ unwind_state_fixup(state);
+ return !!__kernel_text_address(state->pc);
}

void unwind_start(struct unwind_state *state, struct task_struct *task,
--
2.20.1


2022-09-07 11:29:34

by Jinyang He

[permalink] [raw]
Subject: Re: [PATCH v3 10/10] LoongArch/ftrace: Fix unwind state when option func_stack_trace

On 2022/9/7 18:06, Qing Zhang wrote:

> Ftrace plays like function head exception, prologue analysis will stop soon
> because PC is at entry.
>
> 90000000004c5a54 <callee>:
> 90000000004c5a54: 03400000 andi $zero, $zero, 0x0 ==>move t0, ra
> 90000000004c5a58: 03400000 andi $zero, $zero, 0x0 ==>bl callsite
> ==>90000000004c5a5c: 02fcc063 addi.d $sp, $sp, -208(0xf30)
> ...
>
> When encountering ftrace_call, save trace function ra at PT_ERA, save parent ra
> at PT_R1, At this time, pc is the position after the two nops of callee. There is
> no conventional prologue operation between this position and the function entry,
> so we need to resetthe first flag to make the caller continue to unwind.
>
> testing method:
> echo path_openat > ./set_ftrace_filter
> echo 1 > ./options/func_stack_trace
> echo function > ./current_tracer
>
> Signed-off-by: Qing Zhang <[email protected]>
> ---
> arch/loongarch/include/asm/unwind.h | 2 +-
> arch/loongarch/kernel/unwind_prologue.c | 33 +++++++++++++++++++++----
> 2 files changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/arch/loongarch/include/asm/unwind.h b/arch/loongarch/include/asm/unwind.h
> index f66b07c3e6a1..f2b52b9ea93d 100644
> --- a/arch/loongarch/include/asm/unwind.h
> +++ b/arch/loongarch/include/asm/unwind.h
> @@ -20,7 +20,7 @@ struct unwind_state {
> char type; /* UNWINDER_XXX */
> struct stack_info stack_info;
> struct task_struct *task;
> - bool first, error;
> + bool first, error, is_ftrace;
> int graph_idx;
> unsigned long sp, pc, ra;
> };
> diff --git a/arch/loongarch/kernel/unwind_prologue.c b/arch/loongarch/kernel/unwind_prologue.c
> index 03f8b31a90cc..0bb19d3ae8ba 100644
> --- a/arch/loongarch/kernel/unwind_prologue.c
> +++ b/arch/loongarch/kernel/unwind_prologue.c
> @@ -14,9 +14,7 @@ unsigned long unwind_get_return_address(struct unwind_state *state)
>
> if (unwind_done(state))
> return 0;
> - else if (state->type)
> - return state->pc;
> - else if (state->first)
> + else if (state->type || state->first)
> return state->pc;
>
> return *(unsigned long *)(state->sp);
> @@ -42,16 +40,39 @@ static bool unwind_by_guess(struct unwind_state *state)
> return false;
> }
>
> +static void unwind_state_fixup(struct unwind_state *state)
> +{
> + static unsigned long ftrace_case = (unsigned long)ftrace_call + 4;
> +
> + if (state->pc == ftrace_case)
> + state->is_ftrace = true;
> +}
> +
> static bool unwind_by_prologue(struct unwind_state *state)
> {
> struct stack_info *info = &state->stack_info;
> union loongarch_instruction *ip, *ip_end;
> unsigned long frame_size = 0, frame_ra = -1;
> unsigned long size, offset, pc = state->pc;
> + struct pt_regs *regs;
>
> if (state->sp >= info->end || state->sp < info->begin)
> return false;
>
> + if (state->is_ftrace) {
> + /*
> + * As we meet ftrace_regs_entry, reset first flag like first doing
> + * tracing, Prologue analysis will stop soon because PC is at entry.
> + */
> + regs = (struct pt_regs *)state->sp;
> + state->pc = regs->csr_era;
> + state->ra = regs->regs[1];
> + state->sp = regs->regs[3];
> + state->first = true;
> + state->is_ftrace = false;
> + return true;
> + }
> +
> if (!kallsyms_lookup_size_offset(pc, &size, &offset))
> return false;
>
> @@ -97,7 +118,7 @@ static bool unwind_by_prologue(struct unwind_state *state)
>
> state->pc = *(unsigned long *)(state->sp + frame_ra);
> state->sp = state->sp + frame_size;
> - return !!__kernel_text_address(state->pc);
> + goto out;
>
> first:
> state->first = false;
> @@ -106,7 +127,9 @@ static bool unwind_by_prologue(struct unwind_state *state)
>
> state->pc = state->ra;
>
> - return !!__kernel_text_address(state->ra);
> +out:
> + unwind_state_fixup(state);
> + return !!__kernel_text_address(state->pc);
> }
>
> void unwind_start(struct unwind_state *state, struct task_struct *task,

Hi, Qing

Additional 'Reported-by: Jeff Xie <[email protected]>' is welcome.

For this patch, Tested-by: Jinyang He <[email protected]>.


Thanks,

Jinyang

2022-09-07 14:40:12

by Jeff Xie

[permalink] [raw]
Subject: Re: [PATCH v3 10/10] LoongArch/ftrace: Fix unwind state when option func_stack_trace

Hi Qing,

On Wed, Sep 7, 2022 at 7:56 PM Qing Zhang <[email protected]> wrote:
>
> Ftrace plays like function head exception, prologue analysis will stop soon
> because PC is at entry.
>
> 90000000004c5a54 <callee>:
> 90000000004c5a54: 03400000 andi $zero, $zero, 0x0 ==>move t0, ra
> 90000000004c5a58: 03400000 andi $zero, $zero, 0x0 ==>bl callsite
> ==>90000000004c5a5c: 02fcc063 addi.d $sp, $sp, -208(0xf30)
> ...
>
> When encountering ftrace_call, save trace function ra at PT_ERA, save parent ra
> at PT_R1, At this time, pc is the position after the two nops of callee. There is
> no conventional prologue operation between this position and the function entry,
> so we need to resetthe first flag to make the caller continue to unwind.
>
> testing method:
> echo path_openat > ./set_ftrace_filter
> echo 1 > ./options/func_stack_trace
> echo function > ./current_tracer
>
> Signed-off-by: Qing Zhang <[email protected]>
> ---
> arch/loongarch/include/asm/unwind.h | 2 +-
> arch/loongarch/kernel/unwind_prologue.c | 33 +++++++++++++++++++++----
> 2 files changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/arch/loongarch/include/asm/unwind.h b/arch/loongarch/include/asm/unwind.h
> index f66b07c3e6a1..f2b52b9ea93d 100644
> --- a/arch/loongarch/include/asm/unwind.h
> +++ b/arch/loongarch/include/asm/unwind.h
> @@ -20,7 +20,7 @@ struct unwind_state {
> char type; /* UNWINDER_XXX */
> struct stack_info stack_info;
> struct task_struct *task;
> - bool first, error;
> + bool first, error, is_ftrace;
> int graph_idx;
> unsigned long sp, pc, ra;
> };
> diff --git a/arch/loongarch/kernel/unwind_prologue.c b/arch/loongarch/kernel/unwind_prologue.c
> index 03f8b31a90cc..0bb19d3ae8ba 100644
> --- a/arch/loongarch/kernel/unwind_prologue.c
> +++ b/arch/loongarch/kernel/unwind_prologue.c
> @@ -14,9 +14,7 @@ unsigned long unwind_get_return_address(struct unwind_state *state)
>
> if (unwind_done(state))
> return 0;
> - else if (state->type)
> - return state->pc;
> - else if (state->first)
> + else if (state->type || state->first)
> return state->pc;
>
> return *(unsigned long *)(state->sp);
> @@ -42,16 +40,39 @@ static bool unwind_by_guess(struct unwind_state *state)
> return false;
> }
>
> +static void unwind_state_fixup(struct unwind_state *state)
> +{
> + static unsigned long ftrace_case = (unsigned long)ftrace_call + 4;
> +
> + if (state->pc == ftrace_case)
> + state->is_ftrace = true;
> +}
> +
> static bool unwind_by_prologue(struct unwind_state *state)
> {
> struct stack_info *info = &state->stack_info;
> union loongarch_instruction *ip, *ip_end;
> unsigned long frame_size = 0, frame_ra = -1;
> unsigned long size, offset, pc = state->pc;
> + struct pt_regs *regs;
>
> if (state->sp >= info->end || state->sp < info->begin)
> return false;
>
> + if (state->is_ftrace) {
> + /*
> + * As we meet ftrace_regs_entry, reset first flag like first doing
> + * tracing, Prologue analysis will stop soon because PC is at entry.
> + */
> + regs = (struct pt_regs *)state->sp;
> + state->pc = regs->csr_era;
> + state->ra = regs->regs[1];
> + state->sp = regs->regs[3];
> + state->first = true;
> + state->is_ftrace = false;
> + return true;
> + }
> +
> if (!kallsyms_lookup_size_offset(pc, &size, &offset))
> return false;
>
> @@ -97,7 +118,7 @@ static bool unwind_by_prologue(struct unwind_state *state)
>
> state->pc = *(unsigned long *)(state->sp + frame_ra);
> state->sp = state->sp + frame_size;
> - return !!__kernel_text_address(state->pc);
> + goto out;
>
> first:
> state->first = false;
> @@ -106,7 +127,9 @@ static bool unwind_by_prologue(struct unwind_state *state)
>
> state->pc = state->ra;
>
> - return !!__kernel_text_address(state->ra);
> +out:
> + unwind_state_fixup(state);
> + return !!__kernel_text_address(state->pc);
> }
>
> void unwind_start(struct unwind_state *state, struct task_struct *task,
> --
> 2.20.1
>

The patch looks good.

Tested-by: Jeff Xie <[email protected]>

--
Thanks,
JeffXie

2022-09-08 01:57:11

by Qing Zhang

[permalink] [raw]
Subject: Re: [PATCH v3 10/10] LoongArch/ftrace: Fix unwind state when option func_stack_trace

Hi, Jinyang

On 2022/9/7 下午6:40, Jinyang He wrote:
> On 2022/9/7 18:06, Qing Zhang wrote:
>
>> Ftrace plays like function head exception, prologue analysis will stop
>> soon
>> because PC is at entry.
>>
>>     90000000004c5a54 <callee>:
>>     90000000004c5a54:    03400000  andi   $zero, $zero, 0x0   ==>move
>> t0, ra
>>     90000000004c5a58:    03400000  andi   $zero, $zero, 0x0   ==>bl
>> callsite
>> ==>90000000004c5a5c:    02fcc063  addi.d $sp, $sp, -208(0xf30)
>>     ...
>>
>> When encountering ftrace_call, save trace function ra at PT_ERA, save
>> parent ra
>> at PT_R1, At this time, pc is the position after the two nops of
>> callee. There is
>> no conventional prologue operation between this position and the
>> function entry,
>> so we need to resetthe first flag to make the caller continue to unwind.
>>
>> testing method:
>>    echo path_openat > ./set_ftrace_filter
>>    echo 1 > ./options/func_stack_trace
>>    echo function > ./current_tracer
>>
>> Signed-off-by: Qing Zhang <[email protected]>
>> ---
>>   arch/loongarch/include/asm/unwind.h     |  2 +-
>>   arch/loongarch/kernel/unwind_prologue.c | 33 +++++++++++++++++++++----
>>   2 files changed, 29 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/loongarch/include/asm/unwind.h
>> b/arch/loongarch/include/asm/unwind.h
>> index f66b07c3e6a1..f2b52b9ea93d 100644
>> --- a/arch/loongarch/include/asm/unwind.h
>> +++ b/arch/loongarch/include/asm/unwind.h
>> @@ -20,7 +20,7 @@ struct unwind_state {
>>       char type; /* UNWINDER_XXX */
>>       struct stack_info stack_info;
>>       struct task_struct *task;
>> -    bool first, error;
>> +    bool first, error, is_ftrace;
>>       int graph_idx;
>>       unsigned long sp, pc, ra;
>>   };
>> diff --git a/arch/loongarch/kernel/unwind_prologue.c
>> b/arch/loongarch/kernel/unwind_prologue.c
>> index 03f8b31a90cc..0bb19d3ae8ba 100644
>> --- a/arch/loongarch/kernel/unwind_prologue.c
>> +++ b/arch/loongarch/kernel/unwind_prologue.c
>> @@ -14,9 +14,7 @@ unsigned long unwind_get_return_address(struct
>> unwind_state *state)
>>       if (unwind_done(state))
>>           return 0;
>> -    else if (state->type)
>> -        return state->pc;
>> -    else if (state->first)
>> +    else if (state->type || state->first)
>>           return state->pc;
>>       return *(unsigned long *)(state->sp);
>> @@ -42,16 +40,39 @@ static bool unwind_by_guess(struct unwind_state
>> *state)
>>       return false;
>>   }
>> +static void unwind_state_fixup(struct unwind_state *state)
>> +{
>> +    static unsigned long ftrace_case = (unsigned long)ftrace_call + 4;
>> +
>> +    if (state->pc == ftrace_case)
>> +        state->is_ftrace = true;
>> +}
>> +
>>   static bool unwind_by_prologue(struct unwind_state *state)
>>   {
>>       struct stack_info *info = &state->stack_info;
>>       union loongarch_instruction *ip, *ip_end;
>>       unsigned long frame_size = 0, frame_ra = -1;
>>       unsigned long size, offset, pc = state->pc;
>> +    struct pt_regs *regs;
>>       if (state->sp >= info->end || state->sp < info->begin)
>>           return false;
>> +    if (state->is_ftrace) {
>> +        /*
>> +         * As we meet ftrace_regs_entry, reset first flag like first
>> doing
>> +         * tracing, Prologue analysis will stop soon because PC is at
>> entry.
>> +         */
>> +        regs = (struct pt_regs *)state->sp;
>> +        state->pc = regs->csr_era;
>> +        state->ra = regs->regs[1];
>> +        state->sp = regs->regs[3];
>> +        state->first = true;
>> +        state->is_ftrace = false;
>> +        return true;
>> +    }
>> +
>>       if (!kallsyms_lookup_size_offset(pc, &size, &offset))
>>           return false;
>> @@ -97,7 +118,7 @@ static bool unwind_by_prologue(struct unwind_state
>> *state)
>>       state->pc = *(unsigned long *)(state->sp + frame_ra);
>>       state->sp = state->sp + frame_size;
>> -    return !!__kernel_text_address(state->pc);
>> +    goto out;
>>   first:
>>       state->first = false;
>> @@ -106,7 +127,9 @@ static bool unwind_by_prologue(struct unwind_state
>> *state)
>>       state->pc = state->ra;
>> -    return !!__kernel_text_address(state->ra);
>> +out:
>> +    unwind_state_fixup(state);
>> +    return !!__kernel_text_address(state->pc);
>>   }
>>   void unwind_start(struct unwind_state *state, struct task_struct *task,
>
> Hi, Qing
>
> Additional 'Reported-by: Jeff Xie <[email protected]>' is welcome.
>
> For this patch, Tested-by: Jinyang He <[email protected]>.
>
ok, I will add relevant Reported and Tested tags, wait for other
people's comments on the series before this. ping Steven manually. :)

Thanks,
-Qing
>
> Thanks,
>
> Jinyang
>