* [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
* Re: [PATCH] [gdb/record] Fix syscall exit recording for arm
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
0 siblings, 1 reply; 4+ messages in thread
From: Luis @ 2026-03-26 10:03 UTC (permalink / raw)
To: Tom de Vries, gdb-patches
Hi Tom,
Sorry, I was out on time off.
On 10/03/2026 14:36, Tom de Vries wrote:
> [ 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. */
> +
Probably historical reasons.
> 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
The patch looks fine for me.
Approved-By: Luis Machado <luis.machado.foss@gmail.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] [gdb/record] Fix syscall exit recording for arm
2026-03-26 10:03 ` Luis
@ 2026-03-26 14:14 ` Tom de Vries
2026-03-27 0:59 ` Luis
0 siblings, 1 reply; 4+ messages in thread
From: Tom de Vries @ 2026-03-26 14:14 UTC (permalink / raw)
To: Luis, gdb-patches
On 3/26/26 11:03 AM, Luis wrote:
> The patch looks fine for me.
>
> Approved-By: Luis Machado <luis.machado.foss@gmail.com>
Hi Luis,
thanks for the review.
Unfortunately I've already pushed it, so I'm not able to credit you in
the commit.
Thanks,
- Tom
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] [gdb/record] Fix syscall exit recording for arm
2026-03-26 14:14 ` Tom de Vries
@ 2026-03-27 0:59 ` Luis
0 siblings, 0 replies; 4+ messages in thread
From: Luis @ 2026-03-27 0:59 UTC (permalink / raw)
To: Tom de Vries, gdb-patches
On 26/03/2026 14:14, Tom de Vries wrote:
> On 3/26/26 11:03 AM, Luis wrote:
>> The patch looks fine for me.
>>
>> Approved-By: Luis Machado <luis.machado.foss@gmail.com>
>
>
> Hi Luis,
>
> thanks for the review.
>
> Unfortunately I've already pushed it, so I'm not able to credit you in
> the commit.
Oh well, no worries.
^ 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