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

Tom de Vries
2025-03-13T15:35:48Z

On 3/13/25 16:20, Guinevere Larsen wrote:
> 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...

I think the goal is to maximize the same-name mappings, so, support
execveat like this:
...
diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
index 3fb1c1765e4..dcb973fd4c7 100644
--- a/gdb/i386-linux-tdep.c
+++ b/gdb/i386-linux-tdep.c
@@ -758,7 +758,7 @@ i386_canonicalize_syscall (int syscall)
SYSCALL_MAP (getrandom);
UNSUPPORTED_SYSCALL_MAP (memfd_create);
UNSUPPORTED_SYSCALL_MAP (bpf);
- UNSUPPORTED_SYSCALL_MAP (execveat);
+ SYSCALL_MAP (execveat);
SYSCALL_MAP (socket);
SYSCALL_MAP (socketpair);
SYSCALL_MAP (bind);
diff --git a/gdb/linux-record.c b/gdb/linux-record.c
index 0b2709b30b2..c442d13a9bc 100644
--- a/gdb/linux-record.c
+++ b/gdb/linux-record.c
@@ -289,6 +289,7 @@ record_linux_system_call (enum gdb_syscall syscall,
case gdb_sys_link:
case gdb_sys_unlink:
case gdb_sys_execve:
+ case gdb_sys_execveat:
case gdb_sys_chdir:
break;

diff --git a/gdb/linux-record.h b/gdb/linux-record.h
index 6d6ba073089..e8b33f81013 100644
--- a/gdb/linux-record.h
+++ b/gdb/linux-record.h
@@ -542,6 +542,7 @@ enum gdb_syscall {
gdb_sys_msgctl = 531,
gdb_sys_semtimedop = 532,
gdb_sys_accept4 = 533,
+ gdb_sys_execveat = 534,
gdb_sys_newfstatat = 540,
};

diff --git a/gdb/loongarch-linux-tdep.c b/gdb/loongarch-linux-tdep.c
index 2168ce642b9..b9ba265f4f8 100644
--- a/gdb/loongarch-linux-tdep.c
+++ b/gdb/loongarch-linux-tdep.c
@@ -851,7 +851,7 @@ loongarch_canonicalize_syscall (enum
loongarch_syscall syscall_number)
SYSCALL_MAP (getrandom);
UNSUPPORTED_SYSCALL_MAP (memfd_create);
UNSUPPORTED_SYSCALL_MAP (bpf);
- UNSUPPORTED_SYSCALL_MAP (execveat);
+ SYSCALL_MAP (execveat);
UNSUPPORTED_SYSCALL_MAP (userfaultfd);
UNSUPPORTED_SYSCALL_MAP (membarrier);
UNSUPPORTED_SYSCALL_MAP (mlock2);
...

Thanks,
- Tom