Re: [Valgrind-developers] extra syscall_cancel frames

[Adhemerval Z. N. <adh...@li...>]

On 20/03/25 19:10, Florian Weimer wrote:
> * Mark Wielaard:
> 
>>> I think __internal_syscall_cancel should get inlined into
>>> __syscall_cancel.
>>
>> It isn't, I double checked with gdb and there are always two extra
>> frames on top of the call stack.
> 
> Oh, right, it's not static.

And we need __always_inline to proper inline it. I will send a patch
to change it.

> 
>>> And this commit fixed it:
>>>
>>> commit 89b53077d2a58f00e7debdfe58afabe953dac60d
>>> Author: Adhemerval Zanella <adh...@li...>
>>> Date:   Tue Jun 25 16:17:44 2024 -0300
>>>
>>>     nptl: Fix Race conditions in pthread cancellation [BZ#12683]
>>
>> Interesting, so this is actually in 2.41? I should try the fedora 42
>> beta then. Do you happen to know whether people/distros have
>> backported this to earlier releases?
> 
> We're probably going to backport it all the way to Fedora 40/RHEL 10,
> but perhaps not before Fedora 40 goes out of support.
> 
>> I think these extra __*syscall*cancel* frames are somewhat confusing
>> to the user and messes up existing suppressions. They also cause
>> trouble for the valgrind regtests.
>>
>> I think the solution for valgrind is to just skip the top (two) frames
>> if they match the __*syscall*cancel* symbol address ranges. And we
>> only need to do that when we are creating a backtrace from a valgrind
>> syscall wrapper.
>>
>> Looking at the glibc symtab I see four function symbol matching that
>> pattern:
>>
>>  2140: 0000000000079840     51 FUNC    LOCAL  DEFAULT        4 __syscall_cancel_arch
>>  3561: 000000000006daf0     64 FUNC    LOCAL  DEFAULT        4 __syscall_cancel
>>  3700: 000000000006da60    140 FUNC    LOCAL  DEFAULT        4 __internal_syscall_cancel
>>  4566: 000000000006da00     87 FUNC    LOCAL  DEFAULT        4 __syscall_do_cancel
>>
>> Can we rely on those names (and assume there are only 4) or is it
>> better to be flexible and just create a dynamic array for any glibc
>> local function that matches the __*syscall*cancel* pattern?
> 
> I don't think __syscall_do_cancel will need suppressions?  It just
> starts unwinding.
> 
> The names should remain fairly stable.  We should add comments to the
> sources mentioning the valgrind dependency.  Adhemerval, what do you
> think?

It should be reasonable, I will add then on the path as well.  Although
the names are an implementation details, it should be ok add this
constraint.

> 
> Does it matter for valgrind's purposes that those aren't dynamic
> symbols?  In Fedora, we try to accommodate valgrind and similar tools
> and preserve the static symbol table, but not all distributions do this.
> 
> Thanks,
> Florian
>