Date: Mon, 20 Mar 2023 18:16:50 GMT From: Mark Johnston <markj@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: c3179891f897 - main - kerneldump: Inline dump_savectx() into its callers Message-ID: <202303201816.32KIGopb035591@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=c3179891f897d840f578a5139839fcacb587c96d commit c3179891f897d840f578a5139839fcacb587c96d Author: Mark Johnston <markj@FreeBSD.org> AuthorDate: 2023-03-20 18:16:00 +0000 Commit: Mark Johnston <markj@FreeBSD.org> CommitDate: 2023-03-20 18:16:28 +0000 kerneldump: Inline dump_savectx() into its callers The callers of dump_savectx() (i.e., doadump() and livedump_start()) subsequently call dumpsys()/minidumpsys(), which dump the calling thread's stack when writing the dump. If dump_savectx() gets its own stack frame, that frame might be clobbered when its caller later calls dumpsys()/minidumpsys(), making it difficult for debuggers to unwind the stack. Fix this by making dump_savectx() a macro, so that savectx() is always called directly by the function which subsequently calls dumpsys()/minidumpsys(). This fixes stack unwinding for the panicking thread from arm64 minidumps. The same happened to work on amd64, but kgdb reports the dump_savectx() calls as coming from dumpsys(), so in that case it appears to work by accident. Fixes: c9114f9f86f9 ("Add new vnode dumper to support live minidumps") Reviewed by: mhorne, jhb MFC after: 3 days Differential Revision: https://reviews.freebsd.org/D39151 --- sys/kern/kern_shutdown.c | 15 ++------------- sys/kern/kern_vnodedumper.c | 1 + sys/sys/conf.h | 16 +++++++++++++++- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/sys/kern/kern_shutdown.c b/sys/kern/kern_shutdown.c index 6f912369268a..70de6b691fd3 100644 --- a/sys/kern/kern_shutdown.c +++ b/sys/kern/kern_shutdown.c @@ -244,8 +244,8 @@ MTX_SYSINIT(dumper_configs, &dumpconf_list_lk, "dumper config list", MTX_DEF); static TAILQ_HEAD(dumpconflist, dumperinfo) dumper_configs = TAILQ_HEAD_INITIALIZER(dumper_configs); -/* Context information for dump-debuggers. */ -static struct pcb dumppcb; /* Registers. */ +/* Context information for dump-debuggers, saved by the dump_savectx() macro. */ +struct pcb dumppcb; /* Registers. */ lwpid_t dumptid; /* Thread ID. */ static struct cdevsw reroot_cdevsw = { @@ -392,17 +392,6 @@ print_uptime(void) printf("%lds\n", (long)ts.tv_sec); } -/* - * Set up a context that can be extracted from the dump. - */ -void -dump_savectx(void) -{ - - savectx(&dumppcb); - dumptid = curthread->td_tid; -} - int doadump(boolean_t textdump) { diff --git a/sys/kern/kern_vnodedumper.c b/sys/kern/kern_vnodedumper.c index 26154af20372..0104369b9d67 100644 --- a/sys/kern/kern_vnodedumper.c +++ b/sys/kern/kern_vnodedumper.c @@ -44,6 +44,7 @@ #include <sys/sysctl.h> #include <sys/vnode.h> +#include <machine/pcb.h> #include <machine/vmparam.h> static dumper_start_t vnode_dumper_start; diff --git a/sys/sys/conf.h b/sys/sys/conf.h index ad6ffc31dc2a..f1d4df674e6c 100644 --- a/sys/sys/conf.h +++ b/sys/sys/conf.h @@ -360,7 +360,21 @@ struct dumperinfo { extern int dumping; /* system is dumping */ -void dump_savectx(void); +/* + * Save registers for later extraction from a kernel dump. + * + * This must be inlined into the caller, which in turn must be the function that + * calls (mini)dumpsys(). Otherwise, the saved frame pointer will reference a + * stack frame that may be clobbered by subsequent function calls. + */ +#define dump_savectx() do { \ + extern struct pcb dumppcb; \ + extern lwpid_t dumptid; \ + \ + savectx(&dumppcb); \ + dumptid = curthread->td_tid; \ +} while (0) + int doadump(boolean_t); struct diocskerneldump_arg; int dumper_create(const struct dumperinfo *di_template, const char *devname,
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202303201816.32KIGopb035591>