Re: [PATCH] [gdb/tdep] Backport i386_canonicalize_syscall rewrite to gdb-16-branch

[Guinevere Larsen <guinevere@redhat.com>]

On 3/13/25 10:06 AM, Tom de Vries wrote:
> On 3/13/25 13:40, Guinevere Larsen wrote:
>> On 3/13/25 6:53 AM, Tom de Vries wrote:
>>> Commit fbfb29b304e ("[gdb/tdep] Rewrite i386_canonicalize_syscall") 
>>> fixes
>>> PR32770, which reproduces on the gdb-16-branch, but the commit is 
>>> not ideal
>>> for backporting because it completely rewrites 
>>> i386_canonicalize_syscall.
>>>
>>> Instead, this is a version of the patch that adds a single line 
>>> entry for each
>>> syscall value for which i386_canonicalize_syscall gives a different 
>>> result
>>> with and without the patch.
>>>
>>> Consequently, the two versions give identical results.  I've checked 
>>> this for
>>> syscalls 0 to 466.
>>>
>>> Tested on x86_64-linux with target board unix/-m32, on top of 
>>> gdb-16- branch.
>>>
>>> PR tdep/32770
>>> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32770
>>> ---
>>
>> Hi!
>>
>> I looked over all the special cases, and it mostly looks ok. There 
>> are quite a few syscalls where we have an older or similar enough 
>> version, like execveat and execve, or faccessat and faccessat2, but I 
>> can see from the accept4 patch why you wouldn't have done that... 
>> however, for the semtimedop_time64 syscall, you just converted to the 
>> semtimedop version. What is special about that one?
>>
>
> Good question.
>
> This was one of the syscalls I came across while reviewing the 
> gdb_syscall enum values >= 500.  And since the recording support 
> doesn't actually record the timeout parameter, the time64 part of 
> semtimedop_time64 seemed irrelevant.
>
> So, what was special here was that gdb_sys_semtimedop was supported, 
> but there was no corresponding mapping from i386.
>
> For the execveat case, gdb_sys_execveat is missing.  That makes it 
> similar to the accept4 case.

I see... so other syscalls ending with _time64 where GDB wouldn't record 
the time64 portion could just be swapped for the regular version?

I'm asking because I noticed a couple, but I didn't double check what 
exactly they recorded, but it could be a quick improvement in that case.

The reason I ask for execveat, is that we don't actually record 
anything, so for GDB it would be the same as execve, which we do 
support. I'm basically trying to narrow down when to include a gdb_sys 
version, and when I can return a close-enough syscall...

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

>
> I hope that answers your question.
>
> Thanks,
> - Tom
>