Adding clone3 and rseq system call support in gdb record mode

Message ID d19ebfb8-c2c3-49ff-acd4-5bd9372f4a98@gmail.com
State New
Headers
Series Adding clone3 and rseq system call support in gdb record mode |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 fail Patch failed to apply
linaro-tcwg-bot/tcwg_gdb_build--master-arm fail Patch failed to apply

Commit Message

Navin P May 14, 2024, 2:53 p.m. UTC
  Hi,

     Please review the patch for adding clone3 and rseq system call in 
gdb record mode.This starts new lwp process for threads. Without this 
change gdb stops execution saying unsupported systemcall.

     I've created 2 new entries for clone3 and rseq system call in the 
below files.


Regards,
Navin


---
  gdb/amd64-linux-tdep.c | 8 ++++++++
  gdb/amd64-linux-tdep.h | 6 +++++-
  gdb/linux-record.c     | 6 ++++++
  gdb/linux-record.h     | 2 ++
  4 files changed, 21 insertions(+), 1 deletion(-)

--
  

Comments

Guinevere Larsen May 15, 2024, 6:26 p.m. UTC | #1
On 5/14/24 11:53, Navin P wrote:
> Hi,
>
>     Please review the patch for adding clone3 and rseq system call in 
> gdb record mode.This starts new lwp process for threads. Without this 
> change gdb stops execution saying unsupported systemcall.
>
>     I've created 2 new entries for clone3 and rseq system call in the 
> below files.

Hi!

Thank you for your interest in the recording subsystem, and this patch! 
I also looked over your change request in the github mirror.

When committing code to GDB nowadays, we like to have very descriptive 
commit message, and usually somewhat independent of the one-line 
summary. If I were the one making this exact commit, I would have 
something like this:

         gdb, record: Add support for clone3 and rseq syscalls on amd64

         This patch introduces support for recording through the linux 
syscalls clone3 and rseq in the amd64             target. This commit 
opts to not record anything beyond the syscall instruction itself, 
following what is         done by the clone syscall.

(I don't know how much you know of git, but if you use just "git commit" 
it will open your editor and make life easier for multi-line commit 
messages :) sorry if you already knew this).

However, I am doing some investigation on my end and I see that rax 
should also be recorded when calling clone3. I haven't double checked 
clone yet, it might be incorrect too.

I checked a couple of memory-looking registers values, and the top of 
the stack, and saw nothing, but it would be nice if we had exact 
reasoning (quoting the patch or code, in case you know something). This 
is not a hard requirement, though, just a nice to have in case you have 
kernel knowledge I lack :)

On the rseq syscall, everything looks alright as far as I understand it, 
but the problem is that GDB can't single step through rseq regions of 
code, since every time it does, the inferior gets de-scheduled and we're 
back at the first instruction. syscall support should be added anyway, 
but I do wonder if we can identify that an rseq region will be recorded, 
and warn the user that they've hit an infinite loop.

I would also like to see a simple testcase for this patch. Please avoid 
using libc stuff if possible, because that's subject to change without 
warning for us, but just have a simple program that spawns one thread 
with clone3 directly, sets up an rseq region, and waits for the thread 
to finish, and the test case dumps the register state at the start, 
records to the end, runs back to the start and compares the register state.
  

Patch

diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
index c52b0436872..545c880f02d 100644
--- a/gdb/amd64-linux-tdep.c
+++ b/gdb/amd64-linux-tdep.c
@@ -1435,6 +1435,14 @@  amd64_canonicalize_syscall (enum amd64_syscall 
syscall_number)
    case amd64_x32_sys_move_pages:
      return gdb_sys_move_pages;

+  case amd64_sys_clone3:
+  case amd64_x32_sys_clone3:
+    return gdb_sys_clone3;
+
+  case amd64_sys_rseq:
+  case amd64_x32_sys_rseq:
+    return gdb_sys_rseq;
+
    default:
      return gdb_sys_no_syscall;
    }
diff --git a/gdb/amd64-linux-tdep.h b/gdb/amd64-linux-tdep.h
index 2003dcda78f..7aae17d9926 100644
--- a/gdb/amd64-linux-tdep.h
+++ b/gdb/amd64-linux-tdep.h
@@ -321,7 +321,9 @@  enum amd64_syscall {
    amd64_sys_vmsplice = 278,
    amd64_sys_move_pages = 279,
    amd64_sys_pipe2 = 293,
-  amd64_sys_getrandom = 318
+  amd64_sys_getrandom = 318,
+  amd64_sys_clone3 = 435,
+  amd64_sys_rseq = 334,
  };

  /* Enum that defines the syscall identifiers for x32 linux.
@@ -571,6 +573,8 @@  enum amd64_x32_syscall {
    amd64_x32_sys_splice = (amd64_x32_syscall_bit + 275),
    amd64_x32_sys_tee = (amd64_x32_syscall_bit + 276),
    amd64_x32_sys_sync_file_range = (amd64_x32_syscall_bit + 277),
+  amd64_x32_sys_rseq = (amd64_x32_syscall_bit + 334),
+  amd64_x32_sys_clone3 = (amd64_x32_syscall_bit + 435),
    amd64_x32_sys_rt_sigaction = (amd64_x32_syscall_bit + 512),
    amd64_x32_sys_rt_sigreturn = (amd64_x32_syscall_bit + 513),
    amd64_x32_sys_ioctl = (amd64_x32_syscall_bit + 514),
diff --git a/gdb/linux-record.c b/gdb/linux-record.c
index 549ea1bd713..051e8560165 100644
--- a/gdb/linux-record.c
+++ b/gdb/linux-record.c
@@ -2047,6 +2047,12 @@  Do you want to stop the program?"),
      case gdb_sys_inotify_init1:
        break;

+    case gdb_sys_clone3:
+      break;
+
+    case gdb_sys_rseq:
+      break;
+
      default:
        gdb_printf (gdb_stderr,
            _("Process record and replay target doesn't "
diff --git a/gdb/linux-record.h b/gdb/linux-record.h
index 962cedc3d34..c75c5b80c7b 100644
--- a/gdb/linux-record.h
+++ b/gdb/linux-record.h
@@ -541,6 +541,8 @@  enum gdb_syscall {
    gdb_sys_msgctl = 531,
    gdb_sys_semtimedop = 532,
    gdb_sys_newfstatat = 540,
+  gdb_sys_clone3 = 541,
+  gdb_sys_rseq = 542,
  };

  /* Record a linux syscall.  */