[gdb/record] Fix return value for svc in aarch64_record_branch_except_sys

The Linaro CI reported the following regression in test-case
gdb.reverse/sigall-reverse.exp on aarch64-linux:
...
(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 0xd4000001 at address $hex.^M
Process record: failed to record execution log.^M
^M
Program stopped.^M
__GI__exit (status=status@entry=0) at ../sysdeps/unix/sysv/linux/_exit.c:30^M
30            INLINE_SYSCALL (exit_group, 1, status);^M
(gdb) FAIL: gdb.reverse/sigall-reverse.exp: continue to signal exit
...
due to commit 0cb5fde010a ("[gdb/testsuite] Fix some incorrect uses of
decimal").

[ Note that 0xd4000001 is "svc #0", the syscall instruction. ]

The test fails because it expects "Process record: inferior program stopped":
...
    gdb_test "continue" "Process record: inferior program stopped.*" \
       "continue to signal exit" \
        "The next instruction is syscall exit_group.* program...y. or n. " \
        "yes"
...
instead of "Process record: failed to record execution log".

The problem is caused by the fact that there are two result value domains:
- one for generic record (res < 0, res == 0, and res > 0) and
- one for aarch64 record (AARCH64_RECORD_SUCCESS = 0, AARCH64_RECORD_UNSUPPORTED = 1
  and AARCH64_RECORD_UNKNOWN = 2, and an unnamed -1).

When returning here from aarch64_record_branch_except_sys:
...
             return tdep->aarch64_syscall_record (aarch64_insn_r->regcache,
                                                  svc_number);
...
we transfer from the generic record domain to the aarch64 record domain, converting:
- a value 1, meaning "Process record: inferior program stopped", into
- a value AARCH64_RECORD_UNSUPPORTED, meaning
  "Process record does not support instruction $hex at address $hex.
   Process record: failed to record execution log.".

[ Then again, the transfer of domains might be in aarch64_linux_syscall_record.
It's kind of hard to say, given that typing is not used to distinguish between
the two domains. ]

Fix this by:
- introducing a conversion function to_aarch64_record_result, and
- using it in aarch64_record_branch_except_sys.

Likewise, add a from_aarch64_record_result and use it in
aarch64_process_record.

Also add some documentation to enum aarch64_record_result, and add the missing
-1 value.

AFAICT, the loongarch port needs the same fix, but I can't test that so I
think it's better to only deal with aarch64 in this patch.

Tested on aarch64-linux.

Approved-By: Luis Machado <luis.machado.foss@gmail.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33913

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index f1bdce453db27dffeb42f10fcd44a408f706afb2..b6d6d70cb56c78fb029565bd635c8881e6f11d30 100644 (file)
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -5125,13 +5125,65 @@ struct aarch64_mem_r
   uint64_t addr;   /* Memory address.  */
 };
 
+/* 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 distinguish one additional value, so we use an enum with 4
+   values.  */
+
 enum aarch64_record_result
 {
-  AARCH64_RECORD_SUCCESS,
-  AARCH64_RECORD_UNSUPPORTED,
-  AARCH64_RECORD_UNKNOWN
+  /* Process record: failed to record execution log.  */
+  AARCH64_RECORD_FAILURE = -1,
+  /* No failure.  */
+  AARCH64_RECORD_SUCCESS = 0,
+  /* Process record does not support instruction $hex at address $hex.
+     Process record: failed to record execution log.  */
+  AARCH64_RECORD_UNSUPPORTED = 1,
+  /* Process record: inferior program stopped.  */
+  AARCH64_RECORD_UNKNOWN = 2
 };
 
+/* Convert from aarch64_record_result.  */
+
+static inline int
+from_aarch64_record_result (int val)
+{
+  if (val == AARCH64_RECORD_FAILURE)
+    return -1;
+
+  if (val == AARCH64_RECORD_SUCCESS)
+    return 0;
+
+  if (val == AARCH64_RECORD_UNSUPPORTED)
+    {
+      /* After printing the "does not support" message, this is handled the
+        same as the AARCH64_RECORD_FAILURE case.  */
+      return -1;
+    }
+
+  if (val == AARCH64_RECORD_UNKNOWN)
+    return 1;
+
+  gdb_assert_not_reached ();
+}
+
+/* Convert to aarch64_record_result.  */
+
+static inline int
+to_aarch64_record_result (int val)
+{
+  if (val > 0)
+    return AARCH64_RECORD_UNKNOWN;
+
+  if (val < 0)
+    return AARCH64_RECORD_FAILURE;
+
+  return AARCH64_RECORD_SUCCESS;
+}
+
 struct aarch64_insn_decode_record
 {
   struct gdbarch *gdbarch;
@@ -5293,8 +5345,10 @@ aarch64_record_branch_except_sys (aarch64_insn_decode_record *aarch64_insn_r)
 
              regcache_raw_read_unsigned (aarch64_insn_r->regcache, 8,
                                          &svc_number);
-             return tdep->aarch64_syscall_record (aarch64_insn_r->regcache,
-                                                  svc_number);
+             int res
+               = tdep->aarch64_syscall_record (aarch64_insn_r->regcache,
+                                               svc_number);
+             return to_aarch64_record_result (res);
            }
          else
            return AARCH64_RECORD_UNSUPPORTED;
@@ -6128,14 +6182,12 @@ aarch64_process_record (struct gdbarch *gdbarch, struct regcache *regcache,
 
   ret = aarch64_record_decode_insn_handler (&aarch64_record);
   if (ret == AARCH64_RECORD_UNSUPPORTED)
-    {
-      gdb_printf (gdb_stderr,
-                 _("Process record does not support instruction "
-                   "0x%0x at address %s.\n"),
-                 aarch64_record.aarch64_insn,
-                 paddress (gdbarch, insn_addr));
-      ret = -1;
-    }
+    gdb_printf (gdb_stderr,
+               _("Process record does not support instruction "
+                 "0x%0x at address %s.\n"),
+               aarch64_record.aarch64_insn,
+               paddress (gdbarch, insn_addr));
+  ret = from_aarch64_record_result (ret);
 
   if (0 == ret)
     {