Re: [PATCH v3 07/19] unwind: Add user space unwinding API

Jens Remus <[email protected]>
Newsgroups org.kernel.vger.linux-toolchains,org.kernel.vger.linux-kernel,org.kernel.vger.linux-perf-users
Organization IBM Deutschland Research & Development GmbH
Message-ID <[email protected]>
On 28.10.2024 22:47, Josh Poimboeuf wrote:
> Introduce a user space unwinder API which provides a generic way to
> unwind user stacks.

...

> diff --git a/kernel/unwind/user.c b/kernel/unwind/user.c

...

> +int unwind_user_next(struct unwind_user_state *state)
> +{
> +	struct unwind_user_frame _frame;
> +	struct unwind_user_frame *frame = &_frame;
> +	unsigned long prev_ip, cfa, fp, ra = 0;
> +
> +	if (state->done)
> +		return -EINVAL;
> +
> +	prev_ip = state->ip;
> +
> +	switch (state->type) {
> +	case UNWIND_USER_TYPE_FP:
> +		frame = &fp_frame;
> +		break;
> +	default:
> +		BUG();
> +	}
> +
> +	cfa = (frame->use_fp ? state->fp : state->sp) + frame->cfa_off;
> +
> +	if (frame->ra_off && get_user(ra, (unsigned long __user *)(cfa + frame->ra_off)))
> +		goto the_end;
> +
> +	if (ra == prev_ip)
> +		goto the_end;

This seems too restrictive to me, as it effectively prevents
unwinding from recursive functions, e.g. Glibc internal merge sort
msort_with_tmp():

$ perf record -F 9999 --call-graph fp /usr/bin/objdump -wdWF /usr/bin/objdump
$ perf script
...
objdump    8314 236064.515562:     100010 task-clock:ppp:
                  100630a compare_symbols+0x2a (/usr/bin/objdump)
              3ffb9e58e7c msort_with_tmp.part.0+0x15c (/usr/lib64/libc.so.6)
              3ffb9e58d76 msort_with_tmp.part.0+0x56 (/usr/lib64/libc.so.6)
[unwinding unexpectedly stops]

Would it be an option to only stop unwinding if both the IP and SP do
not change?

if (sp == prev_sp && ra == prev_ra)
	gote the_end;

> +
> +	if (frame->fp_off && get_user(fp, (unsigned long __user *)(cfa + frame->fp_off)))
> +		goto the_end;
> +
> +	state->sp = cfa;
> +	state->ip = ra;
> +	if (frame->fp_off)
> +		state->fp = fp;
> +
> +	return 0;
> +
> +the_end:
> +	state->done = true;
> +	return -EINVAL;
> +}

...

Thanks and regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303) and z/VSE Support
+49-7031-16-1128 Office
[email protected]

IBM

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Böblingen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.