[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20241113172118.6db57c42@gandalf.local.home>
Date: Wed, 13 Nov 2024 17:21:18 -0500
From: Steven Rostedt <rostedt@...dmis.org>
To: Josh Poimboeuf <jpoimboe@...nel.org>
Cc: Jens Remus <jremus@...ux.ibm.com>, x86@...nel.org, Peter Zijlstra
<peterz@...radead.org>, Ingo Molnar <mingo@...nel.org>, Arnaldo Carvalho de
Melo <acme@...nel.org>, linux-kernel@...r.kernel.org, Indu Bhagat
<indu.bhagat@...cle.com>, Mark Rutland <mark.rutland@....com>, Alexander
Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa
<jolsa@...nel.org>, Namhyung Kim <namhyung@...nel.org>, Ian Rogers
<irogers@...gle.com>, Adrian Hunter <adrian.hunter@...el.com>,
linux-perf-users@...r.kernel.org, Mark Brown <broonie@...nel.org>,
linux-toolchains@...r.kernel.org, Jordan Rome <jordalgo@...a.com>, Sam
James <sam@...too.org>, linux-trace-kernel@...r.kernel.org, Andrii Nakryiko
<andrii.nakryiko@...il.com>, Mathieu Desnoyers
<mathieu.desnoyers@...icios.com>, Florian Weimer <fweimer@...hat.com>, Andy
Lutomirski <luto@...nel.org>, Heiko Carstens <hca@...ux.ibm.com>, Vasily
Gorbik <gor@...ux.ibm.com>
Subject: Re: [PATCH v3 09/19] unwind: Introduce sframe user space unwinding
[ This reply fixes the linux-trace-kernel email :-p ]
On Wed, 13 Nov 2024 17:13:26 -0500
Steven Rostedt <rostedt@...dmis.org> wrote:
> BTW, the following changes were needed to make it work for me:
>
> -- Steve
>
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index 434c548f0837..64cc3c1188ca 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -842,7 +842,8 @@ static int load_elf_binary(struct linux_binprm *bprm)
> int first_pt_load = 1;
> unsigned long error;
> struct elf_phdr *elf_ppnt, *elf_phdata, *interp_elf_phdata = NULL;
> - struct elf_phdr *elf_property_phdata = NULL, *sframe_phdr = NULL;
> + struct elf_phdr *elf_property_phdata = NULL;
> + unsigned long sframe_vaddr = 0;
Could not just save the pointer to the sframe phd, as it gets freed before we need it.
> unsigned long elf_brk;
> int retval, i;
> unsigned long elf_entry;
> @@ -951,7 +952,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
> break;
>
> case PT_GNU_SFRAME:
> - sframe_phdr = elf_ppnt;
> + sframe_vaddr = elf_ppnt->p_vaddr;
> break;
>
> case PT_LOPROC ... PT_HIPROC:
> @@ -1344,8 +1345,8 @@ static int load_elf_binary(struct linux_binprm *bprm)
> task_pid_nr(current), retval);
> }
>
> - if (sframe_phdr)
> - sframe_add_section(load_bias + sframe_phdr->p_vaddr,
> + if (sframe_vaddr)
> + sframe_add_section(load_bias + sframe_vaddr,
> start_code, end_code);
>
> regs = current_pt_regs();
> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
> index 933e47696e29..ca4ef0b72772 100644
> --- a/kernel/unwind/sframe.c
> +++ b/kernel/unwind/sframe.c
> @@ -73,15 +73,15 @@ static int find_fde(struct sframe_section *sec, unsigned long ip,
> struct sframe_fde *fde)
> {
> struct sframe_fde __user *first, *last, *found = NULL;
> - u32 ip_off, func_off_low = 0, func_off_high = -1;
> + s32 ip_off, func_off_low = INT_MIN, func_off_high = INT_MAX;
The ip_off is a signed it. I wrote a program to dump out the sframe section
of files, and I had:
ffffed88: (1020) size: 16 off: 146 num: 2 info: 1 rep:16
ffffed98: (1030) size: 336 off: 154 num: 2 info:17 rep:16
ffffefe1: (1279) size: 113 off: 0 num: 4 info: 0 rep: 0
fffff052: (12ea) size: 54 off: 15 num: 3 info: 0 rep: 0
fffff088: (1320) size: 167 off: 26 num: 3 info: 0 rep: 0
fffff12f: (13c7) size: 167 off: 37 num: 4 info: 0 rep: 0
fffff1d6: (146e) size: 167 off: 52 num: 4 info: 0 rep: 0
fffff27d: (1515) size: 22 off: 67 num: 4 info: 0 rep: 0
fffff293: (152b) size: 141 off: 82 num: 4 info: 0 rep: 0
fffff320: (15b8) size: 81 off: 97 num: 4 info: 0 rep: 0
fffff371: (1609) size: 671 off: 112 num: 4 info: 1 rep: 0
fffff610: (18a8) size: 171 off: 131 num: 4 info: 0 rep: 0
The above turns was created by a loop of:
fde = (void *)sframes + sizeof(*sframes) + sframes->sfh_fdeoff;
for (s = 0; s < sframes->sfh_num_fdes; s++, fde++) {
printf("\t%x: (%lx) size:%8u off:%8u num:%8u info:%2u rep:%2u\n",
fde->sfde_func_start_address,
fde->sfde_func_start_address + shdr->sh_offset,
fde->sfde_func_size,
fde->sfde_func_start_fre_off,
fde->sfde_func_num_fres,
fde->sfde_func_info,
fde->sfde_func_rep_size);
}
As you can see, all the ip_off are negative.
>
> ip_off = ip - sec->sframe_addr;
>
> first = (void __user *)sec->fdes_addr;
> - last = first + sec->fdes_nr;
> + last = first + sec->fdes_nr - 1;
The above was mentioned before.
> while (first <= last) {
> struct sframe_fde __user *mid;
> - u32 func_off;
> + s32 func_off;
>
> mid = first + ((last - first) / 2);
>
> diff --git a/kernel/unwind/user.c b/kernel/unwind/user.c
> index 11aadfade005..d9cd820150c5 100644
> --- a/kernel/unwind/user.c
> +++ b/kernel/unwind/user.c
> @@ -97,7 +97,7 @@ int unwind_user_start(struct unwind_user_state *state)
>
> if (current_has_sframe())
> state->type = UNWIND_USER_TYPE_SFRAME;
> - else if (IS_ENABLED(CONFIG_UNWIND_USER_FP))
> + else if (IS_ENABLED(CONFIG_HAVE_UNWIND_USER_FP))
This was mentioned too.
> state->type = UNWIND_USER_TYPE_FP;
> else
> state->type = UNWIND_USER_TYPE_NONE;
> @@ -138,7 +138,7 @@ int unwind_user(struct unwind_stacktrace *trace, unsigned int max_entries)
> static u64 ctx_to_cookie(u64 cpu, u64 ctx)
> {
> BUILD_BUG_ON(NR_CPUS > 65535);
> - return (ctx & ((1UL << 48) - 1)) | cpu;
> + return (ctx & ((1UL << 48) - 1)) | (cpu << 48);
And so was this.
-- Steve
> }
>
> /*
Powered by blists - more mailing lists