From: Yao Qi <qiyaoltc@gmail.com>
To: gdb-patches@sourceware.org
Subject: [PATCH] Initialize target description early in IPA
Date: Thu, 07 Dec 2017 10:01:00 -0000 [thread overview]
Message-ID: <1512640910-26090-1-git-send-email-yao.qi@linaro.org> (raw)
Target descriptions are allocated lazily, that is fine in GDBserver,
but it is not safe to call malloc in gdb_collect in IPA, because we
can set a fast tracepoint in malloc, and when the tracepoint is hit,
gdb_collect/malloc is called, deadlock or memory corruption may be
triggered.
#0 0xf7cfc200 in malloc ()
#1 0xf7efdc07 in operator new(unsigned int) ()
#2 0xf7ef7636 in allocate_target_description() ()
#3 0xf7efcbe1 in i386_create_target_description(unsigned long long, bool) ()
#4 0xf7efb474 in i386_linux_read_description(unsigned long long) ()
#5 0xf7efb190 in get_ipa_tdesc(int) ()
#6 0xf7ef9baa in gdb_collect ()
The fix is to initialize all target descriptions earlier, when the
IPA is loaded. In order to guarantee malloc is not called in IPA
in gdb_collect, I change the test to set a breakpoint on malloc, if
IPA gdb_collect calls malloc, program will hit the breakpoint, and
test fail.
continue
Continuing.
Thread 1 "" hit Breakpoint 5, 0xf7cfc200 in malloc ()
(gdb) FAIL: gdb.trace/ftrace.exp: advance through tracing
gdb/gdbserver:
2017-12-07 Yao Qi <yao.qi@linaro.org>
* linux-aarch64-ipa.c (initialize_low_tracepoint): Call
aarch64_linux_read_description.
* linux-amd64-ipa.c (idx2mask): New array.
(get_ipa_tdesc): Move idx2mask out.
(initialize_low_tracepoint): Initialize target descriptions.
* linux-i386-ipa.c (idx2mask): New array.
(get_ipa_tdesc): Move idx2mask out.
(initialize_low_tracepoint): Initialize target descriptions.
gdb/testsuite:
2017-12-07 Yao Qi <yao.qi@linaro.org>
* gdb.trace/ftrace.exp (run_trace_experiment): Set breakpoint on
malloc and catch syscall.
---
gdb/gdbserver/linux-aarch64-ipa.c | 1 +
gdb/gdbserver/linux-amd64-ipa.c | 32 +++++++++++++++++++++-----------
gdb/gdbserver/linux-i386-ipa.c | 25 +++++++++++++------------
gdb/testsuite/gdb.trace/ftrace.exp | 33 ++++++++++++++++++++++++++++++---
4 files changed, 65 insertions(+), 26 deletions(-)
diff --git a/gdb/gdbserver/linux-aarch64-ipa.c b/gdb/gdbserver/linux-aarch64-ipa.c
index b240100..c39b7ec 100644
--- a/gdb/gdbserver/linux-aarch64-ipa.c
+++ b/gdb/gdbserver/linux-aarch64-ipa.c
@@ -204,4 +204,5 @@ alloc_jump_pad_buffer (size_t size)
void
initialize_low_tracepoint (void)
{
+ aarch64_linux_read_description ();
}
diff --git a/gdb/gdbserver/linux-amd64-ipa.c b/gdb/gdbserver/linux-amd64-ipa.c
index 85d0d45..5209dab 100644
--- a/gdb/gdbserver/linux-amd64-ipa.c
+++ b/gdb/gdbserver/linux-amd64-ipa.c
@@ -169,6 +169,19 @@ supply_static_tracepoint_registers (struct regcache *regcache,
#endif /* HAVE_UST */
+#if !defined __ILP32__
+/* Map the tdesc index to xcr0 mask. */
+uint64_t idx2mask[X86_TDESC_LAST] = {
+ X86_XSTATE_X87_MASK,
+ X86_XSTATE_SSE_MASK,
+ X86_XSTATE_AVX_MASK,
+ X86_XSTATE_MPX_MASK,
+ X86_XSTATE_AVX_MPX_MASK,
+ X86_XSTATE_AVX_AVX512_MASK,
+ X86_XSTATE_AVX_MPX_AVX512_PKU_MASK,
+};
+#endif
+
/* Return target_desc to use for IPA, given the tdesc index passed by
gdbserver. */
@@ -194,17 +207,6 @@ get_ipa_tdesc (int idx)
break;
}
#else
- /* Map the tdesc index to xcr0 mask. */
- uint64_t idx2mask[X86_TDESC_LAST] = {
- X86_XSTATE_X87_MASK,
- X86_XSTATE_SSE_MASK,
- X86_XSTATE_AVX_MASK,
- X86_XSTATE_MPX_MASK,
- X86_XSTATE_AVX_MPX_MASK,
- X86_XSTATE_AVX_AVX512_MASK,
- X86_XSTATE_AVX_MPX_AVX512_PKU_MASK,
- };
-
return amd64_linux_read_description (idx2mask[idx], false);
#endif
@@ -276,4 +278,12 @@ alloc_jump_pad_buffer (size_t size)
void
initialize_low_tracepoint (void)
{
+#if defined __ILP32__
+ amd64_linux_read_description (X86_XSTATE_SSE_MASK, true);
+ amd64_linux_read_description (X86_XSTATE_AVX_MASK, true);
+ amd64_linux_read_description (X86_XSTATE_AVX_AVX512_MASK, true);
+#else
+ for (auto i = 0; i < X86_TDESC_LAST; i++)
+ amd64_linux_read_description (idx2mask[i], false);
+#endif
}
diff --git a/gdb/gdbserver/linux-i386-ipa.c b/gdb/gdbserver/linux-i386-ipa.c
index 785a63e..8ff91b8 100644
--- a/gdb/gdbserver/linux-i386-ipa.c
+++ b/gdb/gdbserver/linux-i386-ipa.c
@@ -245,6 +245,17 @@ initialize_fast_tracepoint_trampoline_buffer (void)
}
}
+/* Map the tdesc index to xcr0 mask. */
+uint64_t idx2mask[X86_TDESC_LAST] = {
+ X86_XSTATE_X87_MASK,
+ X86_XSTATE_SSE_MASK,
+ X86_XSTATE_AVX_MASK,
+ X86_XSTATE_MPX_MASK,
+ X86_XSTATE_AVX_MPX_MASK,
+ X86_XSTATE_AVX_AVX512_MASK,
+ X86_XSTATE_AVX_MPX_AVX512_PKU_MASK,
+};
+
/* Return target_desc to use for IPA, given the tdesc index passed by
gdbserver. */
@@ -256,18 +267,6 @@ get_ipa_tdesc (int idx)
internal_error (__FILE__, __LINE__,
"unknown ipa tdesc index: %d", idx);
}
-
- /* Map the tdesc index to xcr0 mask. */
- uint64_t idx2mask[X86_TDESC_LAST] = {
- X86_XSTATE_X87_MASK,
- X86_XSTATE_SSE_MASK,
- X86_XSTATE_AVX_MASK,
- X86_XSTATE_MPX_MASK,
- X86_XSTATE_AVX_MPX_MASK,
- X86_XSTATE_AVX_AVX512_MASK,
- X86_XSTATE_AVX_MPX_AVX512_PKU_MASK,
- };
-
return i386_linux_read_description (idx2mask[idx]);
}
@@ -290,4 +289,6 @@ void
initialize_low_tracepoint (void)
{
initialize_fast_tracepoint_trampoline_buffer ();
+ for (auto i = 0; i < X86_TDESC_LAST; i++)
+ i386_linux_read_description (idx2mask[i]);
}
diff --git a/gdb/testsuite/gdb.trace/ftrace.exp b/gdb/testsuite/gdb.trace/ftrace.exp
index b862680..6f24256 100644
--- a/gdb/testsuite/gdb.trace/ftrace.exp
+++ b/gdb/testsuite/gdb.trace/ftrace.exp
@@ -63,9 +63,36 @@ proc run_trace_experiment {} {
gdb_test_no_output "tstart" "start trace experiment"
- gdb_test "continue" \
- ".*Breakpoint \[0-9\]+, end .*" \
- "advance through tracing"
+ # Fast tracepoint can be set in signal handler, so gdb_collect in
+ # IPA shouldn't call any non-async-signal-safe functions. It is
+ # impractical to list all non-async-signal-safe functions, and set
+ # breakpoints on them, so choose malloc only in this test.
+ gdb_test "b -q malloc"
+
+ # Performance-wise, gdb_collect in IPA shouldn't call any syscall
+ # in order to keep fast tracepoint fast enough.
+ global gdb_prompt
+ set test "catch syscall"
+ gdb_test_multiple $test $test {
+ -re "The feature \'catch syscall\' is not supported.*\r\n$gdb_prompt $" {
+ }
+ -re ".*$gdb_prompt $" {
+ pass $test
+ }
+ }
+
+ global decimal
+ set test "advance through tracing"
+ gdb_test_multiple "continue" $test {
+ -re "Thread 2 .* hit Catchpoint $decimal \\(call to syscall .*\\).*\r\n$gdb_prompt $" {
+ # IPA starts a helper thread, which calls accept. Ignore it.
+ send_gdb "continue\n"
+ exp_continue
+ }
+ -re "Breakpoint \[0-9\]+, end .*$gdb_prompt $" {
+ pass $test
+ }
+ }
gdb_test "tstatus" ".*Trace .*" "check on trace status"
--
1.9.1
next reply other threads:[~2017-12-07 10:01 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-07 10:01 Yao Qi [this message]
2017-12-07 11:30 ` Pedro Alves
2017-12-07 17:08 ` Yao Qi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1512640910-26090-1-git-send-email-yao.qi@linaro.org \
--to=qiyaoltc@gmail.com \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).