From: Stephen Hemminger <stephen@networkplumber.org>
To: dev@dpdk.org
Cc: "Stephen Hemminger" <stephen@networkplumber.org>,
"Morten Brørup" <mb@smartsharesystems.com>,
"Bruce Richardson" <bruce.richardson@intel.com>
Subject: [PATCH v4] rte_dump_stack: make in async signal safe
Date: Thu, 14 Apr 2022 13:19:40 -0700 [thread overview]
Message-ID: <20220414201940.266711-1-stephen@networkplumber.org> (raw)
In-Reply-To: <20220129011039.264377-1-stephen@networkplumber.org>
rte_dump_stack() needs to be usable in situations when a bug is
encountered and from signal handlers (such as SEGV).
Glibc backtrace_symbols() calls malloc which makes it
dangerous in a signal handler that is handling errors that maybe
due to memory corruption. Additionally, rte_log() is unsafe because
syslog() is not signal safe; printf() is also documented as
not being safe.
This version formats message and uses writev for each line in a manner
similar to what glibc version of backtrace_symbols_fd() does. The
FreeBSD version of backtrace_symbols_fd() is not signal safe.
Sample output:
0: ./build/app/dpdk-testpmd (rte_dump_stack+0x2b) [560a6e9c002b]
1: ./build/app/dpdk-testpmd (main+0xad) [560a6decd5ad]
2: /lib/x86_64-linux-gnu/libc.so.6 (__libc_start_main+0xcd) [7fd43d3e27fd]
3: ./build/app/dpdk-testpmd (_start+0x2a) [560a6e83628a]
Bugzilla ID: 929
Acked-by: Morten Brørup <mb@smartsharesystems.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
v4
- fix whitespace report from checkpatch
v3
- merge previous two patches into one
common Linux/FreeBSD code.
- rewrite the code to not use functions which are not
documented to be signal safe.
lib/eal/freebsd/eal_debug.c | 43 -------------
lib/eal/freebsd/meson.build | 1 -
lib/eal/include/rte_debug.h | 2 +-
lib/eal/linux/eal_debug.c | 38 -----------
lib/eal/linux/meson.build | 1 -
lib/eal/unix/eal_debug.c | 123 ++++++++++++++++++++++++++++++++++++
lib/eal/unix/meson.build | 1 +
7 files changed, 125 insertions(+), 84 deletions(-)
delete mode 100644 lib/eal/freebsd/eal_debug.c
delete mode 100644 lib/eal/linux/eal_debug.c
create mode 100644 lib/eal/unix/eal_debug.c
diff --git a/lib/eal/freebsd/eal_debug.c b/lib/eal/freebsd/eal_debug.c
deleted file mode 100644
index 64dab4e0da24..000000000000
--- a/lib/eal/freebsd/eal_debug.c
+++ /dev/null
@@ -1,43 +0,0 @@
-/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2010-2014 Intel Corporation
- */
-
-#ifdef RTE_BACKTRACE
-#include <execinfo.h>
-#endif
-#include <stdarg.h>
-#include <signal.h>
-#include <stdlib.h>
-#include <stdio.h>
-#include <stdint.h>
-
-#include <rte_log.h>
-#include <rte_debug.h>
-#include <rte_common.h>
-#include <rte_eal.h>
-
-#define BACKTRACE_SIZE 256
-
-/* dump the stack of the calling core */
-void rte_dump_stack(void)
-{
-#ifdef RTE_BACKTRACE
- void *func[BACKTRACE_SIZE];
- char **symb = NULL;
- int size;
-
- size = backtrace(func, BACKTRACE_SIZE);
- symb = backtrace_symbols(func, size);
-
- if (symb == NULL)
- return;
-
- while (size > 0) {
- rte_log(RTE_LOG_ERR, RTE_LOGTYPE_EAL,
- "%d: [%s]\n", size, symb[size - 1]);
- size --;
- }
-
- free(symb);
-#endif /* RTE_BACKTRACE */
-}
diff --git a/lib/eal/freebsd/meson.build b/lib/eal/freebsd/meson.build
index 398ceab71d03..85cca5a096ca 100644
--- a/lib/eal/freebsd/meson.build
+++ b/lib/eal/freebsd/meson.build
@@ -7,7 +7,6 @@ sources += files(
'eal.c',
'eal_alarm.c',
'eal_cpuflags.c',
- 'eal_debug.c',
'eal_dev.c',
'eal_hugepage_info.c',
'eal_interrupts.c',
diff --git a/lib/eal/include/rte_debug.h b/lib/eal/include/rte_debug.h
index c4bc71ce28f5..2c4b94a7c9bf 100644
--- a/lib/eal/include/rte_debug.h
+++ b/lib/eal/include/rte_debug.h
@@ -22,7 +22,7 @@ extern "C" {
#endif
/**
- * Dump the stack of the calling core to the console.
+ * Dump the stack of the calling core to the standard error.
*/
void rte_dump_stack(void);
diff --git a/lib/eal/linux/eal_debug.c b/lib/eal/linux/eal_debug.c
deleted file mode 100644
index b0ecf5a9dcde..000000000000
--- a/lib/eal/linux/eal_debug.c
+++ /dev/null
@@ -1,38 +0,0 @@
-/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2010-2014 Intel Corporation
- */
-
-#ifdef RTE_BACKTRACE
-#include <execinfo.h>
-#endif
-#include <stdlib.h>
-#include <stdio.h>
-
-#include <rte_log.h>
-#include <rte_debug.h>
-
-#define BACKTRACE_SIZE 256
-
-/* dump the stack of the calling core */
-void rte_dump_stack(void)
-{
-#ifdef RTE_BACKTRACE
- void *func[BACKTRACE_SIZE];
- char **symb = NULL;
- int size;
-
- size = backtrace(func, BACKTRACE_SIZE);
- symb = backtrace_symbols(func, size);
-
- if (symb == NULL)
- return;
-
- while (size > 0) {
- rte_log(RTE_LOG_ERR, RTE_LOGTYPE_EAL,
- "%d: [%s]\n", size, symb[size - 1]);
- size --;
- }
-
- free(symb);
-#endif /* RTE_BACKTRACE */
-}
diff --git a/lib/eal/linux/meson.build b/lib/eal/linux/meson.build
index 65f2ac6b4798..3cccfa36c0a4 100644
--- a/lib/eal/linux/meson.build
+++ b/lib/eal/linux/meson.build
@@ -7,7 +7,6 @@ sources += files(
'eal.c',
'eal_alarm.c',
'eal_cpuflags.c',
- 'eal_debug.c',
'eal_dev.c',
'eal_hugepage_info.c',
'eal_interrupts.c',
diff --git a/lib/eal/unix/eal_debug.c b/lib/eal/unix/eal_debug.c
new file mode 100644
index 000000000000..dea7372af2f8
--- /dev/null
+++ b/lib/eal/unix/eal_debug.c
@@ -0,0 +1,123 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2010-2014 Intel Corporation
+ */
+
+#include <rte_debug.h>
+
+
+#ifdef RTE_BACKTRACE
+
+#include <dlfcn.h>
+#include <execinfo.h>
+#include <string.h>
+#include <sys/uio.h>
+#include <unistd.h>
+
+#define BACKTRACE_SIZE 256
+
+/*
+ * Convert number to string and return start of string.
+ * Note: string does not start at beginning of buffer.
+ */
+static char *safe_itoa(long val, char *buf, size_t len, unsigned int radix)
+{
+ char *bp = buf + len;
+ static const char hexdigit[] = "0123456789abcdef";
+
+ *--bp = '\0'; /* Null terminate the string */
+ do {
+ /* if buffer is not big enough, then truncate */
+ if (bp == buf)
+ return bp;
+
+ *--bp = hexdigit[val % radix];
+ val /= radix;
+ } while (val != 0);
+
+ return bp;
+}
+
+
+/* Dump the stack of the calling core
+ *
+ * To be safe in signal handler requires limiting what functions are
+ * used in this code since may be called from inside libc or
+ * when malloc poll is corrupt.
+ *
+ * Most of libc is therefore not safe, include RTE_LOG (calls syslog);
+ * backtrace_symbols (calls malloc), etc.
+ */
+void rte_dump_stack(void)
+{
+ void *func[BACKTRACE_SIZE];
+ Dl_info info;
+ char buf1[8], buf2[32], buf3[32], buf4[32];
+ struct iovec iov[10];
+ int i, size;
+
+ size = backtrace(func, BACKTRACE_SIZE);
+
+ for (i = 0; i < size; i++) {
+ struct iovec *io = iov;
+ char *str;
+ uintptr_t base;
+ long offset;
+ void *pc = func[i];
+
+/* Macro to put string onto set of iovecs
+ * cast is to suppress warnings about lose of const qualifier
+ */
+#define PUSH_IOV(io, str) { \
+ (io)->iov_base = (char *)(uintptr_t)str; \
+ (io)->iov_len = strlen(str); \
+ ++io; }
+
+ /* output stack frame number */
+ str = safe_itoa(i, buf1, sizeof(buf1), 10);
+ PUSH_IOV(io, str); /* iov[0] */
+ PUSH_IOV(io, ": "); /* iov[1] */
+
+ /* Lookup the symbol information */
+ if (dladdr(pc, &info) == 0) {
+ PUSH_IOV(io, "?? [");
+ } else {
+ const char *fname;
+
+ if (info.dli_fname && *info.dli_fname)
+ fname = info.dli_fname;
+ else
+ fname = "(vdso)";
+ PUSH_IOV(io, fname); /* iov[2] */
+ PUSH_IOV(io, " ("); /* iov[3] */
+
+ if (info.dli_saddr != NULL) {
+ PUSH_IOV(io, info.dli_sname); /* iov[4] */
+ base = (uintptr_t)info.dli_saddr;
+ } else {
+ str = safe_itoa((unsigned long)info.dli_fbase,
+ buf3, sizeof(buf3), 16);
+ PUSH_IOV(io, str);
+ base = (uintptr_t)info.dli_fbase;
+ }
+
+ PUSH_IOV(io, "+0x"); /* iov[5] */
+
+ offset = (uintptr_t)pc - base;
+ str = safe_itoa(offset, buf4, sizeof(buf4), 16);
+ PUSH_IOV(io, str); /* iov[6] */
+
+ PUSH_IOV(io, ") ["); /* iov[7] */
+ }
+
+ str = safe_itoa((unsigned long)pc, buf2, sizeof(buf2), 16);
+ PUSH_IOV(io, str); /* iov[8] */
+ PUSH_IOV(io, "]\n"); /* iov[9] */
+
+ if (writev(STDERR_FILENO, iov, io - iov) < 0)
+ break;
+ }
+}
+#else
+/* stub if not enabled */
+void rte_dump_stack(void) { }
+#endif /* RTE_BACKTRACE */
diff --git a/lib/eal/unix/meson.build b/lib/eal/unix/meson.build
index 781505ca9061..cc7d67dd321d 100644
--- a/lib/eal/unix/meson.build
+++ b/lib/eal/unix/meson.build
@@ -2,6 +2,7 @@
# Copyright(c) 2020 Dmitry Kozlyuk
sources += files(
+ 'eal_debug.c',
'eal_file.c',
'eal_filesystem.c',
'eal_firmware.c',
--
2.35.1
next prev parent reply other threads:[~2022-04-14 20:19 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-29 1:10 [RFC] eal_debug: do not use malloc in rte_dump_stack Stephen Hemminger
2022-01-29 8:25 ` Morten Brørup
2022-02-12 18:44 ` [PATCH v2 0/2] rte_dump_stack: improvements Stephen Hemminger
2022-02-12 18:44 ` [PATCH v2 1/2] eal_debug: do not use malloc in rte_dump_stack Stephen Hemminger
2022-02-13 11:41 ` Thomas Monjalon
2022-03-17 23:13 ` Stephen Hemminger
2022-02-12 18:44 ` [PATCH v2 2/2] eal: common rte_dump_stack for both Linux and FreeBSD Stephen Hemminger
2022-02-14 11:10 ` [PATCH v2 0/2] rte_dump_stack: improvements Morten Brørup
2022-02-14 11:51 ` Bruce Richardson
2022-04-07 12:45 ` David Marchand
2022-04-07 23:06 ` Stephen Hemminger
2022-04-14 19:41 ` [PATCH v3] rte_dump_stack: make in async signal safe Stephen Hemminger
2022-04-14 20:19 ` Stephen Hemminger [this message]
2022-06-23 7:51 ` [PATCH v4] " David Marchand
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=20220414201940.266711-1-stephen@networkplumber.org \
--to=stephen@networkplumber.org \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=mb@smartsharesystems.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.