* [PATCH 1/3] [gdb/tdep] Add syscall number cache
@ 2023-11-22 9:10 Tom de Vries
2023-11-22 9:10 ` [PATCH 2/3] [gdb/tdep] Add gdbarch_extended_event_to_syscall Tom de Vries
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Tom de Vries @ 2023-11-22 9:10 UTC (permalink / raw)
To: gdb-patches
When running test-case gdb.base/catch-syscall.exp on powerpc64le-linux, we run
into an xfail:
...
(gdb) catch syscall execve^M
Catchpoint 18 (syscall 'execve' [11])^M
(gdb) PASS: gdb.base/catch-syscall.exp: execve: \
catch syscall with arguments (execve)
...
continue^M
Continuing.^M
^M
Catchpoint 18 (call to syscall execve), 0x00007ffff7d7f18c in execve () from \
/lib64/libc.so.6^M
(gdb) PASS: gdb.base/catch-syscall.exp: execve: program has called execve
continue^M
Continuing.^M
process 60484 is executing new program: catch-syscall^M
^M
Breakpoint 17, main (argc=1, argv=0x7fffffffe618) at catch-syscall.c:54^M
54 char buf1[2] = "a";^M
(gdb) XFAIL: gdb.base/catch-syscall.exp: execve: syscall execve has returned
...
The problem is that the catchpoint "(return from syscall execve)" doesn't
trigger.
This is caused by ppc_linux_get_syscall_number returning 0 at execve
syscall-exit-stop, while it should return 11.
This is a problem that was fixed in linux kernel version v5.19, by commit
ec6d0dde71d7 ("powerpc: Enable execve syscall exit tracepoint"), but the
machine I'm running the tests on has v4.18.0.
An approach was discussed in the PR where ppc_linux_get_syscall_number would
try to detect an execve syscall-exit-stop based on the register state, but
that was considered too fragile.
Fix this by caching the syscall number at syscall-enter-stop, and reusing it
at syscall-exit-stop.
This is sufficient to stop triggering the xfail, so remove it.
It's good to point out that this doesn't always eliminate the need to get the
syscall number at a syscall-exit-stop.
The test-case has an example called mid-vfork, where we do:
- catch vfork
- continue
- catch syscall
- continue.
The following things happen:
- the "catch vfork" specifies that we capture the PTRACE_EVENT_VFORK event.
- the first continue runs into the event
- the "catch syscall" specifies that we capture syscall-enter-stop and
syscall-exit-stop events.
- the second continue runs into the syscall-exit-stop. At that point there's
no syscall number value cached, because no corresponding syscall-enter-stop
was observed.
We can address this issue somewhat by translating events into syscalls. A
followup patch in this series use this approach (though not for vfork).
PR tdep/28623
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28623
---
gdb/linux-nat.c | 45 ++++++++++++++++++++++--
gdb/linux-nat.h | 3 ++
gdb/testsuite/gdb.base/catch-syscall.exp | 8 +----
3 files changed, 46 insertions(+), 10 deletions(-)
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 7b0562cf89b..ab6eadd557d 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -1508,6 +1508,17 @@ linux_resume_one_lwp_throw (struct lwp_info *lp, int step,
else
lp->stop_pc = 0;
+ if (catch_syscall_enabled () > 0)
+ {
+ /* Function inf_ptrace_target::resume uses PT_SYSCALL. */
+ }
+ else
+ {
+ /* Function inf_ptrace_target::resume uses PT_CONTINUE.
+ Invalidate syscall_number cache. */
+ lp->syscall_number = -1;
+ }
+
linux_target->low_prepare_to_resume (lp);
linux_target->low_resume (lp->ptid, step, signo);
@@ -1762,7 +1773,26 @@ linux_handle_syscall_trap (struct lwp_info *lp, int stopping)
struct target_waitstatus *ourstatus = &lp->waitstatus;
struct gdbarch *gdbarch = target_thread_architecture (lp->ptid);
thread_info *thread = linux_target->find_thread (lp->ptid);
+
+ enum target_waitkind new_syscall_state
+ = (lp->syscall_state == TARGET_WAITKIND_SYSCALL_ENTRY
+ ? TARGET_WAITKIND_SYSCALL_RETURN
+ : TARGET_WAITKIND_SYSCALL_ENTRY);
+
int syscall_number = (int) gdbarch_get_syscall_number (gdbarch, thread);
+ if (new_syscall_state == TARGET_WAITKIND_SYSCALL_RETURN
+ && lp->syscall_number != -1
+ && lp->syscall_number != syscall_number)
+ {
+ /* Calling gdbarch_get_syscall_number for TARGET_WAITKIND_SYSCALL_RETURN
+ is unreliable on some targets for some syscalls, use the syscall
+ detected at TARGET_WAITKIND_SYSCALL_ENTRY instead. */
+ linux_nat_debug_printf
+ (_("Using syscall number %d supplied by syscall_number cache instead"
+ " of %d supplied by architecture hook"),
+ lp->syscall_number, syscall_number);
+ syscall_number = lp->syscall_number;
+ }
if (stopping)
{
@@ -1801,9 +1831,18 @@ linux_handle_syscall_trap (struct lwp_info *lp, int stopping)
the user could install a new catchpoint for this syscall
between syscall enter/return, and we'll need to know to
report a syscall return if that happens. */
- lp->syscall_state = (lp->syscall_state == TARGET_WAITKIND_SYSCALL_ENTRY
- ? TARGET_WAITKIND_SYSCALL_RETURN
- : TARGET_WAITKIND_SYSCALL_ENTRY);
+ lp->syscall_state = new_syscall_state;
+
+ if (lp->syscall_state == TARGET_WAITKIND_SYSCALL_ENTRY)
+ {
+ /* Save to use in TARGET_WAITKIND_SYSCALL_RETURN. */
+ lp->syscall_number = syscall_number;
+ }
+ else
+ {
+ /* Reset to prevent stale values. */
+ lp->syscall_number = -1;
+ }
if (catch_syscall_enabled ())
{
diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h
index 428bb9f1628..b17037400a3 100644
--- a/gdb/linux-nat.h
+++ b/gdb/linux-nat.h
@@ -277,6 +277,9 @@ struct lwp_info : intrusive_list_node<lwp_info>
- TARGET_WAITKIND_SYSCALL_RETURN */
enum target_waitkind syscall_state;
+ /* Syscall number corresponding to syscall_state. */
+ int syscall_number = -1;
+
/* The processor core this LWP was last seen on. */
int core = -1;
diff --git a/gdb/testsuite/gdb.base/catch-syscall.exp b/gdb/testsuite/gdb.base/catch-syscall.exp
index 0588cb35d87..d8ea466cf00 100644
--- a/gdb/testsuite/gdb.base/catch-syscall.exp
+++ b/gdb/testsuite/gdb.base/catch-syscall.exp
@@ -134,13 +134,7 @@ proc check_return_from_syscall { syscall { pattern "" } } {
return 1
}
-re -wrap ".*Breakpoint $decimal, main .*" {
- # On Powerpc the kernel does not report the returned from
- # syscall as expected by the test. GDB bugzilla 28623.
- if { [istarget "powerpc64*-linux*"] } {
- xfail $thistest
- } else {
- fail $thistest
- }
+ fail $thistest
return 0
}
}
base-commit: 790ce1f70c25129b35e060329cdf2915a6def9fd
--
2.35.3
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/3] [gdb/tdep] Add gdbarch_extended_event_to_syscall
2023-11-22 9:10 [PATCH 1/3] [gdb/tdep] Add syscall number cache Tom de Vries
@ 2023-11-22 9:10 ` Tom de Vries
2023-11-22 9:10 ` [PATCH 3/3] [gdb/tdep] Use ptrace events to get current syscall Tom de Vries
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Tom de Vries @ 2023-11-22 9:10 UTC (permalink / raw)
To: gdb-patches
Add a new gdbarch hook, that translates ptrace events like PTRACE_EVENT_EXEC
into the system call number that triggers the event.
No implementation and no use of the hook yet, this will be added in the
following patch.
---
gdb/arch-utils.c | 8 ++++++++
gdb/arch-utils.h | 3 +++
gdb/gdbarch-gen.h | 7 +++++++
gdb/gdbarch.c | 22 ++++++++++++++++++++++
gdb/gdbarch_components.py | 12 ++++++++++++
5 files changed, 52 insertions(+)
diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index c64584c5760..27ed580b265 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -1492,6 +1492,14 @@ gdbarch_initialized_p (gdbarch *arch)
return arch->initialized_p;
}
+/* See arch-utils.h. */
+
+int
+default_extended_event_to_syscall (struct gdbarch *, int)
+{
+ return -1;
+}
+
void _initialize_gdbarch_utils ();
void
_initialize_gdbarch_utils ()
diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
index e53950c4408..e41d31321a5 100644
--- a/gdb/arch-utils.h
+++ b/gdb/arch-utils.h
@@ -325,4 +325,7 @@ extern enum return_value_convention default_gdbarch_return_value
struct regcache *regcache, struct value **read_value,
const gdb_byte *writebuf);
+/* Default implementation of gdbarch extended_event_to_syscall method. */
+extern int default_extended_event_to_syscall (struct gdbarch *, int);
+
#endif /* ARCH_UTILS_H */
diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h
index 9f468bd1f61..62ad52f5da8 100644
--- a/gdb/gdbarch-gen.h
+++ b/gdb/gdbarch-gen.h
@@ -1283,6 +1283,13 @@ typedef LONGEST (gdbarch_get_syscall_number_ftype) (struct gdbarch *gdbarch, thr
extern LONGEST gdbarch_get_syscall_number (struct gdbarch *gdbarch, thread_info *thread);
extern void set_gdbarch_get_syscall_number (struct gdbarch *gdbarch, gdbarch_get_syscall_number_ftype *get_syscall_number);
+/* Function for the 'catch syscall' feature.
+ Translate extended wait event to architecture-specific system call number. */
+
+typedef int (gdbarch_extended_event_to_syscall_ftype) (struct gdbarch *gdbarch, int event);
+extern int gdbarch_extended_event_to_syscall (struct gdbarch *gdbarch, int event);
+extern void set_gdbarch_extended_event_to_syscall (struct gdbarch *gdbarch, gdbarch_extended_event_to_syscall_ftype *extended_event_to_syscall);
+
/* The filename of the XML syscall for this architecture. */
extern const char * gdbarch_xml_syscall_file (struct gdbarch *gdbarch);
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index ea6e4c647b1..f4661b8b4cd 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -207,6 +207,7 @@ struct gdbarch
gdbarch_get_siginfo_type_ftype *get_siginfo_type = nullptr;
gdbarch_record_special_symbol_ftype *record_special_symbol = nullptr;
gdbarch_get_syscall_number_ftype *get_syscall_number = nullptr;
+ gdbarch_extended_event_to_syscall_ftype *extended_event_to_syscall = default_extended_event_to_syscall;
const char * xml_syscall_file = 0;
struct syscalls_info * syscalls_info = 0;
const char *const * stap_integer_prefixes = 0;
@@ -475,6 +476,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
/* Skip verify of get_siginfo_type, has predicate. */
/* Skip verify of record_special_symbol, has predicate. */
/* Skip verify of get_syscall_number, has predicate. */
+ /* Skip verify of extended_event_to_syscall, invalid_p == 0 */
/* Skip verify of xml_syscall_file, invalid_p == 0 */
/* Skip verify of syscalls_info, invalid_p == 0 */
/* Skip verify of stap_integer_prefixes, invalid_p == 0 */
@@ -1198,6 +1200,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
gdb_printf (file,
"gdbarch_dump: get_syscall_number = <%s>\n",
host_address_to_string (gdbarch->get_syscall_number));
+ gdb_printf (file,
+ "gdbarch_dump: extended_event_to_syscall = <%s>\n",
+ host_address_to_string (gdbarch->extended_event_to_syscall));
gdb_printf (file,
"gdbarch_dump: xml_syscall_file = %s\n",
pstring (gdbarch->xml_syscall_file));
@@ -4512,6 +4517,23 @@ set_gdbarch_get_syscall_number (struct gdbarch *gdbarch,
gdbarch->get_syscall_number = get_syscall_number;
}
+int
+gdbarch_extended_event_to_syscall (struct gdbarch *gdbarch, int event)
+{
+ gdb_assert (gdbarch != NULL);
+ gdb_assert (gdbarch->extended_event_to_syscall != NULL);
+ if (gdbarch_debug >= 2)
+ gdb_printf (gdb_stdlog, "gdbarch_extended_event_to_syscall called\n");
+ return gdbarch->extended_event_to_syscall (gdbarch, event);
+}
+
+void
+set_gdbarch_extended_event_to_syscall (struct gdbarch *gdbarch,
+ gdbarch_extended_event_to_syscall_ftype extended_event_to_syscall)
+{
+ gdbarch->extended_event_to_syscall = extended_event_to_syscall;
+}
+
const char *
gdbarch_xml_syscall_file (struct gdbarch *gdbarch)
{
diff --git a/gdb/gdbarch_components.py b/gdb/gdbarch_components.py
index 694ac366023..0292c91d29f 100644
--- a/gdb/gdbarch_components.py
+++ b/gdb/gdbarch_components.py
@@ -2064,6 +2064,18 @@ Get architecture-specific system calls information from registers.
predicate=True,
)
+Method(
+ comment="""
+Function for the 'catch syscall' feature.
+Translate extended wait event to architecture-specific system call number.
+""",
+ type="int",
+ name="extended_event_to_syscall",
+ params=[("int", "event")],
+ predefault="default_extended_event_to_syscall",
+ invalid=False,
+)
+
Value(
comment="""
The filename of the XML syscall for this architecture.
--
2.35.3
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/3] [gdb/tdep] Use ptrace events to get current syscall
2023-11-22 9:10 [PATCH 1/3] [gdb/tdep] Add syscall number cache Tom de Vries
2023-11-22 9:10 ` [PATCH 2/3] [gdb/tdep] Add gdbarch_extended_event_to_syscall Tom de Vries
@ 2023-11-22 9:10 ` Tom de Vries
2023-11-22 16:16 ` [PATCH 1/3] [gdb/tdep] Add syscall number cache John Baldwin
2023-11-24 21:09 ` Simon Marchi
3 siblings, 0 replies; 8+ messages in thread
From: Tom de Vries @ 2023-11-22 9:10 UTC (permalink / raw)
To: gdb-patches
Add a powerpc implementation of gdbarch_extended_event_to_syscall, for the
moment only mapping PTRACE_EVENT_EXEC to execve.
Use gdbarch_extended_event_to_syscall in linux_handle_extended_wait to
update the cached syscall number.
---
gdb/linux-nat.c | 5 +++++
gdb/ppc-linux-tdep.c | 17 +++++++++++++++++
2 files changed, 22 insertions(+)
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index ab6eadd557d..cbc52cafec3 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -1971,6 +1971,11 @@ linux_handle_extended_wait (struct lwp_info *lp, int status)
you have to be using PTRACE_SEIZE to get that. */
lp->syscall_state = TARGET_WAITKIND_SYSCALL_ENTRY;
+ struct gdbarch *gdbarch = target_thread_architecture (lp->ptid);
+ int syscall_number = gdbarch_extended_event_to_syscall (gdbarch, event);
+ if (syscall_number != -1)
+ lp->syscall_number = syscall_number;
+
if (event == PTRACE_EVENT_FORK || event == PTRACE_EVENT_VFORK
|| event == PTRACE_EVENT_CLONE)
{
diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c
index 24e1b455afd..dffe1489144 100644
--- a/gdb/ppc-linux-tdep.c
+++ b/gdb/ppc-linux-tdep.c
@@ -64,6 +64,7 @@
#include "elf-bfd.h"
#include "producer.h"
#include "target-float.h"
+#include "nat/linux-ptrace.h"
#include "features/rs6000/powerpc-32l.c"
#include "features/rs6000/powerpc-altivec32l.c"
@@ -1373,6 +1374,19 @@ ppc_linux_get_syscall_number (struct gdbarch *gdbarch,
static struct linux_record_tdep ppc_linux_record_tdep;
static struct linux_record_tdep ppc64_linux_record_tdep;
+static int
+ppc_linux_extended_event_to_syscall (struct gdbarch *gdbarch, int event)
+{
+ switch (event)
+ {
+ case PTRACE_EVENT_EXEC:
+ /* Execve syscall. */
+ return 11;
+ }
+
+ return -1;
+}
+
/* ppc_canonicalize_syscall maps from the native PowerPC Linux set of
syscall ids into a canonical set of syscall ids used by process
record. (See arch/powerpc/include/uapi/asm/unistd.h in kernel tree.)
@@ -2173,6 +2187,9 @@ ppc_linux_init_abi (struct gdbarch_info info,
/* Get the syscall number from the arch's register. */
set_gdbarch_get_syscall_number (gdbarch, ppc_linux_get_syscall_number);
+ set_gdbarch_extended_event_to_syscall (gdbarch,
+ ppc_linux_extended_event_to_syscall);
+
/* SystemTap functions. */
set_gdbarch_stap_integer_prefixes (gdbarch, stap_integer_prefixes);
set_gdbarch_stap_register_indirection_prefixes (gdbarch,
--
2.35.3
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] [gdb/tdep] Add syscall number cache
2023-11-22 9:10 [PATCH 1/3] [gdb/tdep] Add syscall number cache Tom de Vries
2023-11-22 9:10 ` [PATCH 2/3] [gdb/tdep] Add gdbarch_extended_event_to_syscall Tom de Vries
2023-11-22 9:10 ` [PATCH 3/3] [gdb/tdep] Use ptrace events to get current syscall Tom de Vries
@ 2023-11-22 16:16 ` John Baldwin
2023-11-24 21:09 ` Simon Marchi
3 siblings, 0 replies; 8+ messages in thread
From: John Baldwin @ 2023-11-22 16:16 UTC (permalink / raw)
To: Tom de Vries, gdb-patches
On 11/22/23 1:10 AM, Tom de Vries wrote:
> When running test-case gdb.base/catch-syscall.exp on powerpc64le-linux, we run
> into an xfail:
> ...
> (gdb) catch syscall execve^M
> Catchpoint 18 (syscall 'execve' [11])^M
> (gdb) PASS: gdb.base/catch-syscall.exp: execve: \
> catch syscall with arguments (execve)
> ...
> continue^M
> Continuing.^M
> ^M
> Catchpoint 18 (call to syscall execve), 0x00007ffff7d7f18c in execve () from \
> /lib64/libc.so.6^M
> (gdb) PASS: gdb.base/catch-syscall.exp: execve: program has called execve
> continue^M
> Continuing.^M
> process 60484 is executing new program: catch-syscall^M
> ^M
> Breakpoint 17, main (argc=1, argv=0x7fffffffe618) at catch-syscall.c:54^M
> 54 char buf1[2] = "a";^M
> (gdb) XFAIL: gdb.base/catch-syscall.exp: execve: syscall execve has returned
> ...
>
> The problem is that the catchpoint "(return from syscall execve)" doesn't
> trigger.
>
> This is caused by ppc_linux_get_syscall_number returning 0 at execve
> syscall-exit-stop, while it should return 11.
>
> This is a problem that was fixed in linux kernel version v5.19, by commit
> ec6d0dde71d7 ("powerpc: Enable execve syscall exit tracepoint"), but the
> machine I'm running the tests on has v4.18.0.
>
> An approach was discussed in the PR where ppc_linux_get_syscall_number would
> try to detect an execve syscall-exit-stop based on the register state, but
> that was considered too fragile.
>
> Fix this by caching the syscall number at syscall-enter-stop, and reusing it
> at syscall-exit-stop.
>
> This is sufficient to stop triggering the xfail, so remove it.
>
> It's good to point out that this doesn't always eliminate the need to get the
> syscall number at a syscall-exit-stop.
>
> The test-case has an example called mid-vfork, where we do:
> - catch vfork
> - continue
> - catch syscall
> - continue.
>
> The following things happen:
> - the "catch vfork" specifies that we capture the PTRACE_EVENT_VFORK event.
> - the first continue runs into the event
> - the "catch syscall" specifies that we capture syscall-enter-stop and
> syscall-exit-stop events.
> - the second continue runs into the syscall-exit-stop. At that point there's
> no syscall number value cached, because no corresponding syscall-enter-stop
> was observed.
>
> We can address this issue somewhat by translating events into syscalls. A
> followup patch in this series use this approach (though not for vfork).
>
> PR tdep/28623
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28623
> ---
> gdb/linux-nat.c | 45 ++++++++++++++++++++++--
> gdb/linux-nat.h | 3 ++
> gdb/testsuite/gdb.base/catch-syscall.exp | 8 +----
> 3 files changed, 46 insertions(+), 10 deletions(-)
>
> diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
> index 7b0562cf89b..ab6eadd557d 100644
> --- a/gdb/linux-nat.c
> +++ b/gdb/linux-nat.c
> @@ -1508,6 +1508,17 @@ linux_resume_one_lwp_throw (struct lwp_info *lp, int step,
> else
> lp->stop_pc = 0;
>
> + if (catch_syscall_enabled () > 0)
> + {
> + /* Function inf_ptrace_target::resume uses PT_SYSCALL. */
> + }
> + else
> + {
> + /* Function inf_ptrace_target::resume uses PT_CONTINUE.
> + Invalidate syscall_number cache. */
> + lp->syscall_number = -1;
> + }
> +
> linux_target->low_prepare_to_resume (lp);
> linux_target->low_resume (lp->ptid, step, signo);
>
> @@ -1762,7 +1773,26 @@ linux_handle_syscall_trap (struct lwp_info *lp, int stopping)
> struct target_waitstatus *ourstatus = &lp->waitstatus;
> struct gdbarch *gdbarch = target_thread_architecture (lp->ptid);
> thread_info *thread = linux_target->find_thread (lp->ptid);
> +
> + enum target_waitkind new_syscall_state
> + = (lp->syscall_state == TARGET_WAITKIND_SYSCALL_ENTRY
> + ? TARGET_WAITKIND_SYSCALL_RETURN
> + : TARGET_WAITKIND_SYSCALL_ENTRY);
> +
> int syscall_number = (int) gdbarch_get_syscall_number (gdbarch, thread);
> + if (new_syscall_state == TARGET_WAITKIND_SYSCALL_RETURN
> + && lp->syscall_number != -1
> + && lp->syscall_number != syscall_number)
> + {
> + /* Calling gdbarch_get_syscall_number for TARGET_WAITKIND_SYSCALL_RETURN
> + is unreliable on some targets for some syscalls, use the syscall
> + detected at TARGET_WAITKIND_SYSCALL_ENTRY instead. */
> + linux_nat_debug_printf
> + (_("Using syscall number %d supplied by syscall_number cache instead"
> + " of %d supplied by architecture hook"),
> + lp->syscall_number, syscall_number);
> + syscall_number = lp->syscall_number;
> + }
>
> if (stopping)
> {
> @@ -1801,9 +1831,18 @@ linux_handle_syscall_trap (struct lwp_info *lp, int stopping)
> the user could install a new catchpoint for this syscall
> between syscall enter/return, and we'll need to know to
> report a syscall return if that happens. */
> - lp->syscall_state = (lp->syscall_state == TARGET_WAITKIND_SYSCALL_ENTRY
> - ? TARGET_WAITKIND_SYSCALL_RETURN
> - : TARGET_WAITKIND_SYSCALL_ENTRY);
> + lp->syscall_state = new_syscall_state;
> +
> + if (lp->syscall_state == TARGET_WAITKIND_SYSCALL_ENTRY)
> + {
> + /* Save to use in TARGET_WAITKIND_SYSCALL_RETURN. */
> + lp->syscall_number = syscall_number;
> + }
> + else
> + {
> + /* Reset to prevent stale values. */
> + lp->syscall_number = -1;
> + }
>
> if (catch_syscall_enabled ())
> {
> diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h
> index 428bb9f1628..b17037400a3 100644
> --- a/gdb/linux-nat.h
> +++ b/gdb/linux-nat.h
> @@ -277,6 +277,9 @@ struct lwp_info : intrusive_list_node<lwp_info>
> - TARGET_WAITKIND_SYSCALL_RETURN */
> enum target_waitkind syscall_state;
>
> + /* Syscall number corresponding to syscall_state. */
> + int syscall_number = -1;
> +
> /* The processor core this LWP was last seen on. */
> int core = -1;
>
> diff --git a/gdb/testsuite/gdb.base/catch-syscall.exp b/gdb/testsuite/gdb.base/catch-syscall.exp
> index 0588cb35d87..d8ea466cf00 100644
> --- a/gdb/testsuite/gdb.base/catch-syscall.exp
> +++ b/gdb/testsuite/gdb.base/catch-syscall.exp
> @@ -134,13 +134,7 @@ proc check_return_from_syscall { syscall { pattern "" } } {
> return 1
> }
> -re -wrap ".*Breakpoint $decimal, main .*" {
> - # On Powerpc the kernel does not report the returned from
> - # syscall as expected by the test. GDB bugzilla 28623.
> - if { [istarget "powerpc64*-linux*"] } {
> - xfail $thistest
> - } else {
> - fail $thistest
> - }
> + fail $thistest
> return 0
> }
> }
>
> base-commit: 790ce1f70c25129b35e060329cdf2915a6def9fd
This looks good to me.
--
John Baldwin
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] [gdb/tdep] Add syscall number cache
2023-11-22 9:10 [PATCH 1/3] [gdb/tdep] Add syscall number cache Tom de Vries
` (2 preceding siblings ...)
2023-11-22 16:16 ` [PATCH 1/3] [gdb/tdep] Add syscall number cache John Baldwin
@ 2023-11-24 21:09 ` Simon Marchi
2023-11-24 22:19 ` Tom de Vries
3 siblings, 1 reply; 8+ messages in thread
From: Simon Marchi @ 2023-11-24 21:09 UTC (permalink / raw)
To: Tom de Vries, gdb-patches
On 11/22/23 04:10, Tom de Vries wrote:
> When running test-case gdb.base/catch-syscall.exp on powerpc64le-linux, we run
> into an xfail:
> ...
> (gdb) catch syscall execve^M
> Catchpoint 18 (syscall 'execve' [11])^M
> (gdb) PASS: gdb.base/catch-syscall.exp: execve: \
> catch syscall with arguments (execve)
> ...
> continue^M
> Continuing.^M
> ^M
> Catchpoint 18 (call to syscall execve), 0x00007ffff7d7f18c in execve () from \
> /lib64/libc.so.6^M
> (gdb) PASS: gdb.base/catch-syscall.exp: execve: program has called execve
> continue^M
> Continuing.^M
> process 60484 is executing new program: catch-syscall^M
> ^M
> Breakpoint 17, main (argc=1, argv=0x7fffffffe618) at catch-syscall.c:54^M
> 54 char buf1[2] = "a";^M
> (gdb) XFAIL: gdb.base/catch-syscall.exp: execve: syscall execve has returned
> ...
>
> The problem is that the catchpoint "(return from syscall execve)" doesn't
> trigger.
>
> This is caused by ppc_linux_get_syscall_number returning 0 at execve
> syscall-exit-stop, while it should return 11.
>
> This is a problem that was fixed in linux kernel version v5.19, by commit
> ec6d0dde71d7 ("powerpc: Enable execve syscall exit tracepoint"), but the
> machine I'm running the tests on has v4.18.0.
>
> An approach was discussed in the PR where ppc_linux_get_syscall_number would
> try to detect an execve syscall-exit-stop based on the register state, but
> that was considered too fragile.
>
> Fix this by caching the syscall number at syscall-enter-stop, and reusing it
> at syscall-exit-stop.
>
> This is sufficient to stop triggering the xfail, so remove it.
>
> It's good to point out that this doesn't always eliminate the need to get the
> syscall number at a syscall-exit-stop.
>
> The test-case has an example called mid-vfork, where we do:
> - catch vfork
> - continue
> - catch syscall
> - continue.
>
> The following things happen:
> - the "catch vfork" specifies that we capture the PTRACE_EVENT_VFORK event.
> - the first continue runs into the event
> - the "catch syscall" specifies that we capture syscall-enter-stop and
> syscall-exit-stop events.
> - the second continue runs into the syscall-exit-stop. At that point there's
> no syscall number value cached, because no corresponding syscall-enter-stop
> was observed.
>
> We can address this issue somewhat by translating events into syscalls. A
> followup patch in this series use this approach (though not for vfork).
>
> PR tdep/28623
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28623
> ---
> gdb/linux-nat.c | 45 ++++++++++++++++++++++--
> gdb/linux-nat.h | 3 ++
> gdb/testsuite/gdb.base/catch-syscall.exp | 8 +----
> 3 files changed, 46 insertions(+), 10 deletions(-)
>
> diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
> index 7b0562cf89b..ab6eadd557d 100644
> --- a/gdb/linux-nat.c
> +++ b/gdb/linux-nat.c
> @@ -1508,6 +1508,17 @@ linux_resume_one_lwp_throw (struct lwp_info *lp, int step,
> else
> lp->stop_pc = 0;
>
> + if (catch_syscall_enabled () > 0)
> + {
> + /* Function inf_ptrace_target::resume uses PT_SYSCALL. */
> + }
> + else
> + {
> + /* Function inf_ptrace_target::resume uses PT_CONTINUE.
> + Invalidate syscall_number cache. */
> + lp->syscall_number = -1;
> + }
> +
> linux_target->low_prepare_to_resume (lp);
> linux_target->low_resume (lp->ptid, step, signo);
>
> @@ -1762,7 +1773,26 @@ linux_handle_syscall_trap (struct lwp_info *lp, int stopping)
> struct target_waitstatus *ourstatus = &lp->waitstatus;
> struct gdbarch *gdbarch = target_thread_architecture (lp->ptid);
> thread_info *thread = linux_target->find_thread (lp->ptid);
> +
> + enum target_waitkind new_syscall_state
> + = (lp->syscall_state == TARGET_WAITKIND_SYSCALL_ENTRY
> + ? TARGET_WAITKIND_SYSCALL_RETURN
> + : TARGET_WAITKIND_SYSCALL_ENTRY);
> +
> int syscall_number = (int) gdbarch_get_syscall_number (gdbarch, thread);
> + if (new_syscall_state == TARGET_WAITKIND_SYSCALL_RETURN
> + && lp->syscall_number != -1
> + && lp->syscall_number != syscall_number)
> + {
> + /* Calling gdbarch_get_syscall_number for TARGET_WAITKIND_SYSCALL_RETURN
> + is unreliable on some targets for some syscalls, use the syscall
> + detected at TARGET_WAITKIND_SYSCALL_ENTRY instead. */
> + linux_nat_debug_printf
> + (_("Using syscall number %d supplied by syscall_number cache instead"
> + " of %d supplied by architecture hook"),
> + lp->syscall_number, syscall_number);
> + syscall_number = lp->syscall_number;
> + }
If we're going to use lp->syscall_number (if it is not -1) and it
disagrees with gdbarch_get_syscall_number, what's the point in calling
gdbarch_get_syscall_number in the first place? Should the logic be:
if (new_syscall_state == TARGET_WAITKIND_SYSCALL_RETURN
&& lp->syscall_number != -1)
// use lp->syscall_number
else
// call gdbarch_get_syscall_number
?
Simon
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] [gdb/tdep] Add syscall number cache
2023-11-24 21:09 ` Simon Marchi
@ 2023-11-24 22:19 ` Tom de Vries
2023-11-27 15:52 ` Simon Marchi
0 siblings, 1 reply; 8+ messages in thread
From: Tom de Vries @ 2023-11-24 22:19 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
On 11/24/23 22:09, Simon Marchi wrote:
> On 11/22/23 04:10, Tom de Vries wrote:
>> When running test-case gdb.base/catch-syscall.exp on powerpc64le-linux, we run
>> into an xfail:
>> ...
>> (gdb) catch syscall execve^M
>> Catchpoint 18 (syscall 'execve' [11])^M
>> (gdb) PASS: gdb.base/catch-syscall.exp: execve: \
>> catch syscall with arguments (execve)
>> ...
>> continue^M
>> Continuing.^M
>> ^M
>> Catchpoint 18 (call to syscall execve), 0x00007ffff7d7f18c in execve () from \
>> /lib64/libc.so.6^M
>> (gdb) PASS: gdb.base/catch-syscall.exp: execve: program has called execve
>> continue^M
>> Continuing.^M
>> process 60484 is executing new program: catch-syscall^M
>> ^M
>> Breakpoint 17, main (argc=1, argv=0x7fffffffe618) at catch-syscall.c:54^M
>> 54 char buf1[2] = "a";^M
>> (gdb) XFAIL: gdb.base/catch-syscall.exp: execve: syscall execve has returned
>> ...
>>
>> The problem is that the catchpoint "(return from syscall execve)" doesn't
>> trigger.
>>
>> This is caused by ppc_linux_get_syscall_number returning 0 at execve
>> syscall-exit-stop, while it should return 11.
>>
>> This is a problem that was fixed in linux kernel version v5.19, by commit
>> ec6d0dde71d7 ("powerpc: Enable execve syscall exit tracepoint"), but the
>> machine I'm running the tests on has v4.18.0.
>>
>> An approach was discussed in the PR where ppc_linux_get_syscall_number would
>> try to detect an execve syscall-exit-stop based on the register state, but
>> that was considered too fragile.
>>
>> Fix this by caching the syscall number at syscall-enter-stop, and reusing it
>> at syscall-exit-stop.
>>
>> This is sufficient to stop triggering the xfail, so remove it.
>>
>> It's good to point out that this doesn't always eliminate the need to get the
>> syscall number at a syscall-exit-stop.
>>
>> The test-case has an example called mid-vfork, where we do:
>> - catch vfork
>> - continue
>> - catch syscall
>> - continue.
>>
>> The following things happen:
>> - the "catch vfork" specifies that we capture the PTRACE_EVENT_VFORK event.
>> - the first continue runs into the event
>> - the "catch syscall" specifies that we capture syscall-enter-stop and
>> syscall-exit-stop events.
>> - the second continue runs into the syscall-exit-stop. At that point there's
>> no syscall number value cached, because no corresponding syscall-enter-stop
>> was observed.
>>
>> We can address this issue somewhat by translating events into syscalls. A
>> followup patch in this series use this approach (though not for vfork).
>>
>> PR tdep/28623
>> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28623
>> ---
>> gdb/linux-nat.c | 45 ++++++++++++++++++++++--
>> gdb/linux-nat.h | 3 ++
>> gdb/testsuite/gdb.base/catch-syscall.exp | 8 +----
>> 3 files changed, 46 insertions(+), 10 deletions(-)
>>
>> diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
>> index 7b0562cf89b..ab6eadd557d 100644
>> --- a/gdb/linux-nat.c
>> +++ b/gdb/linux-nat.c
>> @@ -1508,6 +1508,17 @@ linux_resume_one_lwp_throw (struct lwp_info *lp, int step,
>> else
>> lp->stop_pc = 0;
>>
>> + if (catch_syscall_enabled () > 0)
>> + {
>> + /* Function inf_ptrace_target::resume uses PT_SYSCALL. */
>> + }
>> + else
>> + {
>> + /* Function inf_ptrace_target::resume uses PT_CONTINUE.
>> + Invalidate syscall_number cache. */
>> + lp->syscall_number = -1;
>> + }
>> +
>> linux_target->low_prepare_to_resume (lp);
>> linux_target->low_resume (lp->ptid, step, signo);
>>
>> @@ -1762,7 +1773,26 @@ linux_handle_syscall_trap (struct lwp_info *lp, int stopping)
>> struct target_waitstatus *ourstatus = &lp->waitstatus;
>> struct gdbarch *gdbarch = target_thread_architecture (lp->ptid);
>> thread_info *thread = linux_target->find_thread (lp->ptid);
>> +
>> + enum target_waitkind new_syscall_state
>> + = (lp->syscall_state == TARGET_WAITKIND_SYSCALL_ENTRY
>> + ? TARGET_WAITKIND_SYSCALL_RETURN
>> + : TARGET_WAITKIND_SYSCALL_ENTRY);
>> +
>> int syscall_number = (int) gdbarch_get_syscall_number (gdbarch, thread);
>> + if (new_syscall_state == TARGET_WAITKIND_SYSCALL_RETURN
>> + && lp->syscall_number != -1
>> + && lp->syscall_number != syscall_number)
>> + {
>> + /* Calling gdbarch_get_syscall_number for TARGET_WAITKIND_SYSCALL_RETURN
>> + is unreliable on some targets for some syscalls, use the syscall
>> + detected at TARGET_WAITKIND_SYSCALL_ENTRY instead. */
>> + linux_nat_debug_printf
>> + (_("Using syscall number %d supplied by syscall_number cache instead"
>> + " of %d supplied by architecture hook"),
>> + lp->syscall_number, syscall_number);
>> + syscall_number = lp->syscall_number;
>> + }
>
> If we're going to use lp->syscall_number (if it is not -1) and it
> disagrees with gdbarch_get_syscall_number, what's the point in calling
> gdbarch_get_syscall_number in the first place?
The debug printf that notes the override of gdbarch_get_syscall_number
by the cached syscall number.
> Should the logic be:
>
> if (new_syscall_state == TARGET_WAITKIND_SYSCALL_RETURN
> && lp->syscall_number != -1)
> // use lp->syscall_number
> else
> // call gdbarch_get_syscall_number
>
> ?
If we don't care about that particular type of debug printf, then yes.
Thanks,
- Tom
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] [gdb/tdep] Add syscall number cache
2023-11-24 22:19 ` Tom de Vries
@ 2023-11-27 15:52 ` Simon Marchi
2023-11-27 20:25 ` Tom de Vries
0 siblings, 1 reply; 8+ messages in thread
From: Simon Marchi @ 2023-11-27 15:52 UTC (permalink / raw)
To: Tom de Vries, gdb-patches
On 11/24/23 17:19, Tom de Vries wrote:
>> If we're going to use lp->syscall_number (if it is not -1) and it
>> disagrees with gdbarch_get_syscall_number, what's the point in calling
>> gdbarch_get_syscall_number in the first place?
>
> The debug printf that notes the override of gdbarch_get_syscall_number by the cached syscall number.
>
>> Should the logic be:
>>
>> if (new_syscall_state == TARGET_WAITKIND_SYSCALL_RETURN
>> && lp->syscall_number != -1)
>> // use lp->syscall_number
>> else
>> // call gdbarch_get_syscall_number
>>
>> ?
>
> If we don't care about that particular type of debug printf, then yes.
In my opinion it doesn't add any value to call
gdbarch_get_syscall_number just to be able to make that debug printf.
Simon
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] [gdb/tdep] Add syscall number cache
2023-11-27 15:52 ` Simon Marchi
@ 2023-11-27 20:25 ` Tom de Vries
0 siblings, 0 replies; 8+ messages in thread
From: Tom de Vries @ 2023-11-27 20:25 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
On 11/27/23 16:52, Simon Marchi wrote:
> On 11/24/23 17:19, Tom de Vries wrote:
>>> If we're going to use lp->syscall_number (if it is not -1) and it
>>> disagrees with gdbarch_get_syscall_number, what's the point in calling
>>> gdbarch_get_syscall_number in the first place?
>>
>> The debug printf that notes the override of gdbarch_get_syscall_number by the cached syscall number.
>>
>>> Should the logic be:
>>>
>>> if (new_syscall_state == TARGET_WAITKIND_SYSCALL_RETURN
>>> && lp->syscall_number != -1)
>>> // use lp->syscall_number
>>> else
>>> // call gdbarch_get_syscall_number
>>>
>>> ?
>>
>> If we don't care about that particular type of debug printf, then yes.
>
> In my opinion it doesn't add any value to call
> gdbarch_get_syscall_number just to be able to make that debug printf.
OK, fixed in a v2 (
https://sourceware.org/pipermail/gdb-patches/2023-November/204553.html ).
I've also added another case in linux_handle_syscall_trap of syscall
cache invalidation:
...
@@ -1791,6 +1826,7 @@ linux_handle_syscall_trap (struct lwp_info *lp,
int stopping)
"PTRACE_CONT for SIGSTOP", syscall_number, lp->ptid.lwp ());
lp->syscall_state = TARGET_WAITKIND_IGNORE;
+ lp->syscall_number = -1;
ptrace (PTRACE_CONT, lp->ptid.lwp (), 0, 0);
lp->stopped = 0;
return 1;
...
Thanks,
- Tom
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-11-27 20:25 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-22 9:10 [PATCH 1/3] [gdb/tdep] Add syscall number cache Tom de Vries
2023-11-22 9:10 ` [PATCH 2/3] [gdb/tdep] Add gdbarch_extended_event_to_syscall Tom de Vries
2023-11-22 9:10 ` [PATCH 3/3] [gdb/tdep] Use ptrace events to get current syscall Tom de Vries
2023-11-22 16:16 ` [PATCH 1/3] [gdb/tdep] Add syscall number cache John Baldwin
2023-11-24 21:09 ` Simon Marchi
2023-11-24 22:19 ` Tom de Vries
2023-11-27 15:52 ` Simon Marchi
2023-11-27 20:25 ` Tom de Vries
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).