Thread (2 messages) flat view 2 messages, 1 author, 2024-11-13

Re: [PATCH v3 09/19] unwind: Introduce sframe user space unwinding

From: Steven Rostedt <rostedt@goodmis.org>
Date: 2024-11-13 22:21:01
Also in: linux-perf-users, linux-toolchains, lkml

[ This reply fixes the linux-trace-kernel email :-p ]

On Wed, 13 Nov 2024 17:13:26 -0500
Steven Rostedt [off-list ref] wrote:
quoted hunk
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.
quoted hunk
 	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.
quoted hunk
 	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.
quoted hunk
 		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
 }
 
 /*
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help