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

[Jens Remus]

On 09.12.2024 21:54, Josh Poimboeuf wrote:
> On Fri, Dec 06, 2024 at 11:29:21AM +0100, Jens Remus wrote:
> > On 28.10.2024 22:47, Josh Poimboeuf wrote:
> > > +	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;
>
> Good point, I've already fixed that for the next version (not yet
> posted).  I believe the only thing we really need to check here is that
> the unwind is heading in the right direction:
>
> if (cfa <= state->sp)
> 	goto the_end;

Assuming the x86 definition of the CFA (CFA == SP at call site) this
translates into:

if (sp <= state->sp)
	goto the_end;

That won't work for architectures that pass the return address in a
register instead of on the stack, such as s390. At least in the
topmost frame the unwound SP may be unchanged. For instance when in
the function prologue or when in a leaf function.

One of my patches for s390 support introduces a state->first flag,
indicating whether it is the topmost user space frame. Using that
your check could be extended to:

if ((state->first && sp < state->sp) || (!state->first && sp <= state->sp))
	goto the_end;

Which could be simplified to:

if (sp <= state->sp - state->first)
	goto the_end;

Btw. neither would work for architectures with an upwards-growing
stack, such as hppa. Not sure if that needs to be considered.

Regards,
Jens
--
Jens Remus
Linux on Z Development (D3303) and z/VSE Support
+49-7031-16-1128 Office
jremus@xxxxxxxxxx

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/