Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH] [gdb/record] Fix syscall exit recording for arm
@ 2026-03-10 14:36 Tom de Vries
  2026-03-26 10:03 ` Luis
  0 siblings, 1 reply; 4+ messages in thread
From: Tom de Vries @ 2026-03-10 14:36 UTC (permalink / raw)
  To: gdb-patches

[ Submitted earlier [1] with $subject: "[PATCH] [gdb/record] Fix syscall
recording for arm". ]

On arm-linux, I run into:
...
(gdb) continue^M
Continuing.^M
The next instruction is syscall exit_group.  It will make the program exit.  \
  Do you want to stop the program?([y] or n) yes^M
Process record does not support instruction 0xdf00 at address 0xf7e8f984.^M
Process record: failed to record execution log.^M
^M
Program stopped.^M
__libc_do_syscall () at libc-do-syscall.S:46^M
warning: 46     libc-do-syscall.S: No such file or directory^M
(gdb) FAIL: gdb.reverse/sigall-reverse.exp: continue to signal exit
...

The problem is this bit of code here in decode_insn:
...
      ret = thumb2_record_decode_insn_handler (arm_record);

      if (ret != ARM_RECORD_SUCCESS)
       {
         arm_record_unsupported_insn (arm_record);
         ret = -1;
       }
...
where ret == 1 is mapped to -1.

The 1 is returned by arm_linux_syscall_record and is meant to be interpreted
using this categorization:
- res  < 0: Process record: failed to record execution log.
- res == 0: No failure.
- res  > 0: Process record: inferior program stopped.

But the port interprets 1 as ARM_RECORD_FAILURE:
...
enum arm_record_result
{
  ARM_RECORD_SUCCESS = 0,
  ARM_RECORD_FAILURE = 1
};
...

We could fix this confusion this by:
- adding an ARM_RECORD_UNKNOWN = 2, and
- applying translations at the appropriate points, translating:
  - ARM_RECORD_UNKNOWN into 1 and vice versa,
  - ARM_RECORD_FAILURE into -1 and vice versa,
  similar to what we did for aarch64 and loongarch.

But it seems easier to adopt a go-with-the-flow approach, defining
ARM_RECORD_FAILURE as -1, freeing up the 1 for ARM_RECORD_UNKNOWN = 1.

Then the aforementioned FAIL is fixed by simply doing:
...
      if (ret == ARM_RECORD_FAILURE)
       arm_record_unsupported_insn (arm_record);
...

Tested on arm-linux.

Reviewed-By: Guinevere Larsen <guinevere@redhat.com>

[1] https://sourceware.org/pipermail/gdb-patches/2026-February/225372.html
---
 gdb/arm-tdep.c | 43 +++++++++++++++++++++++++++----------------
 1 file changed, 27 insertions(+), 16 deletions(-)

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index dabae0aec0f..7dfde6649d9 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -11195,10 +11195,30 @@ sbo_sbz (uint32_t insn, uint32_t bit_num, uint32_t len, uint32_t sbo)
   return 1;
 }
 
+/* The record infrastructure supports the following result values:
+   1. res  < 0: Process record: failed to record execution log.
+   2. res == 0: No failure.
+   3. res  > 0: Process record: inferior program stopped.
+
+   For aarch64, we have two distinct failure values:
+   - AARCH64_RECORD_FAILURE:
+     Process record: failed to record execution log.
+   - AARCH64_RECORD_UNSUPPORTED:
+     Process record does not support instruction $hex at address $hex.
+     Process record: failed to record execution log.
+
+   For some reason for arm we don't have an UNSUPPORTED enum value, and
+   instead treat ARM_RECORD_FAILURE like an UNSUPPORTED enum value.  */
+
 enum arm_record_result
 {
+  /* Process record does not support instruction $hex at address $hex.
+     Process record: failed to record execution log.  */
+  ARM_RECORD_FAILURE = -1,
+  /* No failure.  */
   ARM_RECORD_SUCCESS = 0,
-  ARM_RECORD_FAILURE = 1
+  /* Process record: inferior program stopped.  */
+  ARM_RECORD_UNKNOWN = 1,
 };
 
 enum arm_record_strx_t
@@ -14576,11 +14596,8 @@ decode_insn (abstract_instruction_reader &reader,
 	     then we need not decode it anymore.  */
 	  ret = arm_handle_insn[insn_id] (arm_record);
 	}
-      if (ret != ARM_RECORD_SUCCESS)
-	{
-	  arm_record_unsupported_insn (arm_record);
-	  ret = -1;
-	}
+      if (ret == ARM_RECORD_FAILURE)
+	arm_record_unsupported_insn (arm_record);
     }
   else if (THUMB_RECORD == record_type)
     {
@@ -14588,11 +14605,8 @@ decode_insn (abstract_instruction_reader &reader,
       arm_record->cond = -1;
       insn_id = bits (arm_record->arm_insn, 13, 15);
       ret = thumb_handle_insn[insn_id] (arm_record);
-      if (ret != ARM_RECORD_SUCCESS)
-	{
-	  arm_record_unsupported_insn (arm_record);
-	  ret = -1;
-	}
+      if (ret == ARM_RECORD_FAILURE)
+	arm_record_unsupported_insn (arm_record);
     }
   else if (THUMB2_RECORD == record_type)
     {
@@ -14605,11 +14619,8 @@ decode_insn (abstract_instruction_reader &reader,
 
       ret = thumb2_record_decode_insn_handler (arm_record);
 
-      if (ret != ARM_RECORD_SUCCESS)
-	{
-	  arm_record_unsupported_insn (arm_record);
-	  ret = -1;
-	}
+      if (ret == ARM_RECORD_FAILURE)
+	arm_record_unsupported_insn (arm_record);
     }
   else
     {

base-commit: 1add703e09f0f8d073cde4af9d11cd59996e9763
-- 
2.51.0


^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-03-27  1:00 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-03-10 14:36 [PATCH] [gdb/record] Fix syscall exit recording for arm Tom de Vries
2026-03-26 10:03 ` Luis
2026-03-26 14:14   ` Tom de Vries
2026-03-27  0:59     ` Luis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox