[PATCH] Provide a simple backtrace in the samba AD DC also

Andrew Bartlett abartlet at samba.org
Tue Apr 10 05:00:32 UTC 2018


smbd has for a long time been able to produce a simple internal
backtrace.

This patch allows 'samba' to do the same, by bringing that code into
lib/util.  An un-used IRIX mode, lost since the move to waf, was
removed to allow this to happen.  (That mode used become_root()). 

This is important because gdb_backtrace as a panic action is not
compatible with gitlab CI, so without this we have no way to tell where
the fault is. 

The cstyle.py failures are all in the moved code:

ad84ecfd836 s3-lib: Remove support for libexc for IRIX backtraces
2469ab654a3 lib/util: Log PANIC before calling pacic action just like
s3
212ae4db4bb lib/util: Move log_stack_trace() to common code
  :lib/util/fault.c:
      178  Comment line does not have * aligned with top
      179  Comment line does not have * aligned with top
      180  Comment line does not have * aligned with top
      181  Garbage before * in comment line
      181  Comment is 3+ lines but is not formatted as block comment
      197  Preprocessor statement in function body
      201  Comment is 3+ lines but is not formatted as block comment
      227  Space before close parenthesis
      234  Space after cast operator (or inline if/while body)
      242  Space after cast operator (or inline if/while body)
      252  Preprocessor statement in function body
      273  Preprocessor statement in function body
      275  Preprocessor statement in function body
19dba766ad6 lib/util: Call log_stack_trace() in smb_panic_default()

Please review and push,

Andrew Bartlett
-- 
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team         https://samba.org
Samba Development and Support, Catalyst IT   
https://catalyst.net.nz/services/samba



-------------- next part --------------
From ad84ecfd836f07bbfe050fe203d352490c85decc Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Tue, 10 Apr 2018 15:54:10 +1200
Subject: [PATCH 1/4] s3-lib: Remove support for libexc for IRIX backtraces

IRIX is long dead, and this code needs become_root() which is not in
the top level code.

Additionally, the check for libexc never made it into waf, so this
has been dead code since Samba 4.1.

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source3/lib/util.c | 42 ------------------------------------------
 source3/wscript    |  2 +-
 2 files changed, 1 insertion(+), 43 deletions(-)

diff --git a/source3/lib/util.c b/source3/lib/util.c
index ae9fe71c974..98c47b3df9f 100644
--- a/source3/lib/util.c
+++ b/source3/lib/util.c
@@ -852,10 +852,6 @@ void smb_panic_s3(const char *why)
 #include <execinfo.h>
 #endif
 
-#ifdef HAVE_LIBEXC_H
-#include <libexc.h>
-#endif
-
 void log_stack_trace(void)
 {
 #ifdef HAVE_LIBUNWIND
@@ -934,44 +930,6 @@ libunwind_failed:
 		/* Leak the backtrace_strings, rather than risk what free() might do */
 	}
 
-#elif HAVE_LIBEXC
-
-	/* The IRIX libexc library provides an API for unwinding the stack. See
-	 * libexc(3) for details. Apparantly trace_back_stack leaks memory, but
-	 * since we are about to abort anyway, it hardly matters.
-	 */
-
-#define NAMESIZE 32 /* Arbitrary */
-
-	__uint64_t	addrs[BACKTRACE_STACK_SIZE];
-	char *      	names[BACKTRACE_STACK_SIZE];
-	char		namebuf[BACKTRACE_STACK_SIZE * NAMESIZE];
-
-	int		i;
-	int		levels;
-
-	ZERO_ARRAY(addrs);
-	ZERO_ARRAY(names);
-	ZERO_ARRAY(namebuf);
-
-	/* We need to be root so we can open our /proc entry to walk
-	 * our stack. It also helps when we want to dump core.
-	 */
-	become_root();
-
-	for (i = 0; i < BACKTRACE_STACK_SIZE; i++) {
-		names[i] = namebuf + (i * NAMESIZE);
-	}
-
-	levels = trace_back_stack(0, addrs, names,
-			BACKTRACE_STACK_SIZE, NAMESIZE - 1);
-
-	DEBUG(0, ("BACKTRACE: %d stack frames:\n", levels));
-	for (i = 0; i < levels; i++) {
-		DEBUGADD(0, (" #%d 0x%llx %s\n", i, addrs[i], names[i]));
-	}
-#undef NAMESIZE
-
 #else
 	DEBUG(0, ("unable to produce a stack trace on this platform\n"));
 #endif
diff --git a/source3/wscript b/source3/wscript
index 826877d3dfd..8d658b2f53e 100644
--- a/source3/wscript
+++ b/source3/wscript
@@ -96,7 +96,7 @@ def configure(conf):
     # We crash without vfs_default
     required_static_modules.extend(TO_LIST('vfs_default'))
 
-    conf.CHECK_HEADERS('execinfo.h libexc.h libunwind.h netdb.h')
+    conf.CHECK_HEADERS('execinfo.h libunwind.h netdb.h')
     conf.CHECK_HEADERS('linux/falloc.h linux/ioctl.h')
 
     conf.CHECK_FUNCS('getcwd fchown chmod fchmod mknod')
-- 
2.11.0


From 2469ab654a3c39099ec6423acb5927182849a3b1 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Tue, 10 Apr 2018 16:06:12 +1200
Subject: [PATCH 2/4] lib/util: Log PANIC before calling pacic action just like
 s3

This is like the changes made in s3 by
4fa555980070d78b39711ef21d77628d26055bc2

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 lib/util/fault.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/util/fault.c b/lib/util/fault.c
index 54d84711742..887c7d6c238 100644
--- a/lib/util/fault.c
+++ b/lib/util/fault.c
@@ -123,6 +123,9 @@ _PUBLIC_ const char *panic_action = NULL;
 static void smb_panic_default(const char *why) _NORETURN_;
 static void smb_panic_default(const char *why)
 {
+	DEBUG(0,("PANIC (pid %llu): %s\n",
+		    (unsigned long long)getpid(), why));
+
 #if defined(HAVE_PRCTL) && defined(PR_SET_PTRACER)
 	/*
 	 * Make sure all children can attach a debugger.
@@ -148,7 +151,6 @@ static void smb_panic_default(const char *why)
 					  WEXITSTATUS(result)));
 		}
 	}
-	DEBUG(0,("PANIC: %s\n", why));
 
 #ifdef SIGABRT
 	CatchSignal(SIGABRT, SIG_DFL);
-- 
2.11.0


From 212ae4db4bb433de5e3a70fcb797c6dc21f4d795 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Tue, 10 Apr 2018 16:35:07 +1200
Subject: [PATCH 3/4] lib/util: Move log_stack_trace() to common code

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 lib/util/fault.c           | 104 +++++++++++++++++++++++++++++++++++++++++++++
 lib/util/fault.h           |   1 +
 lib/util/wscript_configure |   1 +
 source3/include/local.h    |   3 --
 source3/include/proto.h    |   1 -
 source3/lib/util.c         |  97 ------------------------------------------
 source3/wscript            |   2 +-
 7 files changed, 107 insertions(+), 102 deletions(-)

diff --git a/lib/util/fault.c b/lib/util/fault.c
index 887c7d6c238..af06c9484db 100644
--- a/lib/util/fault.c
+++ b/lib/util/fault.c
@@ -3,6 +3,7 @@
    Critical Fault handling
    Copyright (C) Andrew Tridgell 1992-1998
    Copyright (C) Tim Prouty 2009
+   Copyright (C) James Peach 2006
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -170,3 +171,106 @@ _PUBLIC_ void smb_panic(const char *why)
 	}
 	smb_panic_default(why);
 }
+
+
+
+/*******************************************************************
+ Print a backtrace of the stack to the debug log. This function
+ DELIBERATELY LEAKS MEMORY. The expectation is that you should
+ exit shortly after calling it.
+********************************************************************/
+
+/* Buffer size to use when printing backtraces */
+#define BACKTRACE_STACK_SIZE 64
+
+
+#ifdef HAVE_LIBUNWIND_H
+#include <libunwind.h>
+#endif
+
+#ifdef HAVE_EXECINFO_H
+#include <execinfo.h>
+#endif
+
+void log_stack_trace(void)
+{
+#ifdef HAVE_LIBUNWIND
+	/* Try to use libunwind before any other technique since on ia64
+	 * libunwind correctly walks the stack in more circumstances than
+	 * backtrace.
+	 */
+	unw_cursor_t cursor;
+	unw_context_t uc;
+	unsigned i = 0;
+
+	char procname[256];
+	unw_word_t ip, sp, off;
+
+	procname[sizeof(procname) - 1] = '\0';
+
+	if (unw_getcontext(&uc) != 0) {
+		goto libunwind_failed;
+	}
+
+	if (unw_init_local(&cursor, &uc) != 0) {
+		goto libunwind_failed;
+	}
+
+	DEBUG(0, ("BACKTRACE:\n"));
+
+	do {
+	    ip = sp = 0;
+	    unw_get_reg(&cursor, UNW_REG_IP, &ip);
+	    unw_get_reg(&cursor, UNW_REG_SP, &sp);
+
+	    switch (unw_get_proc_name(&cursor,
+			procname, sizeof(procname) - 1, &off) ) {
+	    case 0:
+		    /* Name found. */
+	    case -UNW_ENOMEM:
+		    /* Name truncated. */
+		    DEBUGADD(0, (" #%u %s + %#llx [ip=%#llx] [sp=%#llx]\n",
+			    i, procname, (long long)off,
+			    (long long)ip, (long long) sp));
+		    break;
+	    default:
+	    /* case -UNW_ENOINFO: */
+	    /* case -UNW_EUNSPEC: */
+		    /* No symbol name found. */
+		    DEBUGADD(0, (" #%u %s [ip=%#llx] [sp=%#llx]\n",
+			    i, "<unknown symbol>",
+			    (long long)ip, (long long) sp));
+	    }
+	    ++i;
+	} while (unw_step(&cursor) > 0);
+
+	return;
+
+libunwind_failed:
+	DEBUG(0, ("unable to produce a stack trace with libunwind\n"));
+
+#elif HAVE_BACKTRACE_SYMBOLS
+	void *backtrace_stack[BACKTRACE_STACK_SIZE];
+	size_t backtrace_size;
+	char **backtrace_strings;
+
+	/* get the backtrace (stack frames) */
+	backtrace_size = backtrace(backtrace_stack,BACKTRACE_STACK_SIZE);
+	backtrace_strings = backtrace_symbols(backtrace_stack, backtrace_size);
+
+	DEBUG(0, ("BACKTRACE: %lu stack frames:\n",
+		  (unsigned long)backtrace_size));
+
+	if (backtrace_strings) {
+		int i;
+
+		for (i = 0; i < backtrace_size; i++)
+			DEBUGADD(0, (" #%u %s\n", i, backtrace_strings[i]));
+
+		/* Leak the backtrace_strings, rather than risk what free() might do */
+	}
+
+#else
+	DEBUG(0, ("unable to produce a stack trace on this platform\n"));
+#endif
+}
diff --git a/lib/util/fault.h b/lib/util/fault.h
index 0ac6cb9ceb5..dfa339b7650 100644
--- a/lib/util/fault.h
+++ b/lib/util/fault.h
@@ -53,5 +53,6 @@ void fault_setup(void);
 void fault_setup_disable(void);
 _NORETURN_ void smb_panic(const char *reason);
 
+void log_stack_trace(void);
 
 #endif /* _SAMBA_FAULT_H_ */
diff --git a/lib/util/wscript_configure b/lib/util/wscript_configure
index 8e5a59c8480..2a8dbef699b 100644
--- a/lib/util/wscript_configure
+++ b/lib/util/wscript_configure
@@ -6,6 +6,7 @@ if Options.options.disable_fault_handling:
 
 # backtrace could be in libexecinfo or in libc
 conf.CHECK_FUNCS_IN('backtrace backtrace_symbols', 'execinfo', checklibc=True, headers='execinfo.h')
+conf.CHECK_HEADERS('execinfo.h libunwind.h')
 
 conf.CHECK_STRUCTURE_MEMBER('struct statvfs', 'f_frsize', define='HAVE_FRSIZE', headers='sys/statvfs.h')
 
diff --git a/source3/include/local.h b/source3/include/local.h
index 7f97d4ece9c..c2be1ff3b7f 100644
--- a/source3/include/local.h
+++ b/source3/include/local.h
@@ -172,9 +172,6 @@
 /* Number in seconds for winbindd to wait for the mutex. Make this 2 * smbd wait time. */
 #define WINBIND_SERVER_MUTEX_WAIT_TIME (( ((NUM_CLI_AUTH_CONNECT_RETRIES) * ((CLI_AUTH_TIMEOUT)/1000)) + 5)*2)
 
-/* Buffer size to use when printing backtraces */
-#define BACKTRACE_STACK_SIZE 64
-
 /* size of listen() backlog in smbd */
 #define SMBD_LISTEN_BACKLOG 50
 
diff --git a/source3/include/proto.h b/source3/include/proto.h
index fa87407ff24..2bc5ab2f532 100644
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -356,7 +356,6 @@ char *gidtoname(gid_t gid);
 uid_t nametouid(const char *name);
 gid_t nametogid(const char *name);
 void smb_panic_s3(const char *why);
-void log_stack_trace(void);
 const char *readdirname(DIR *p);
 bool is_in_path(const char *name, name_compare_entry *namelist, bool case_sensitive);
 void set_namearray(name_compare_entry **ppname_array, const char *namelist);
diff --git a/source3/lib/util.c b/source3/lib/util.c
index 98c47b3df9f..5f786f95d3e 100644
--- a/source3/lib/util.c
+++ b/source3/lib/util.c
@@ -839,103 +839,6 @@ void smb_panic_s3(const char *why)
 }
 
 /*******************************************************************
- Print a backtrace of the stack to the debug log. This function
- DELIBERATELY LEAKS MEMORY. The expectation is that you should
- exit shortly after calling it.
-********************************************************************/
-
-#ifdef HAVE_LIBUNWIND_H
-#include <libunwind.h>
-#endif
-
-#ifdef HAVE_EXECINFO_H
-#include <execinfo.h>
-#endif
-
-void log_stack_trace(void)
-{
-#ifdef HAVE_LIBUNWIND
-	/* Try to use libunwind before any other technique since on ia64
-	 * libunwind correctly walks the stack in more circumstances than
-	 * backtrace.
-	 */ 
-	unw_cursor_t cursor;
-	unw_context_t uc;
-	unsigned i = 0;
-
-	char procname[256];
-	unw_word_t ip, sp, off;
-
-	procname[sizeof(procname) - 1] = '\0';
-
-	if (unw_getcontext(&uc) != 0) {
-		goto libunwind_failed;
-	}
-
-	if (unw_init_local(&cursor, &uc) != 0) {
-		goto libunwind_failed;
-	}
-
-	DEBUG(0, ("BACKTRACE:\n"));
-
-	do {
-	    ip = sp = 0;
-	    unw_get_reg(&cursor, UNW_REG_IP, &ip);
-	    unw_get_reg(&cursor, UNW_REG_SP, &sp);
-
-	    switch (unw_get_proc_name(&cursor,
-			procname, sizeof(procname) - 1, &off) ) {
-	    case 0:
-		    /* Name found. */
-	    case -UNW_ENOMEM:
-		    /* Name truncated. */
-		    DEBUGADD(0, (" #%u %s + %#llx [ip=%#llx] [sp=%#llx]\n",
-			    i, procname, (long long)off,
-			    (long long)ip, (long long) sp));
-		    break;
-	    default:
-	    /* case -UNW_ENOINFO: */
-	    /* case -UNW_EUNSPEC: */
-		    /* No symbol name found. */
-		    DEBUGADD(0, (" #%u %s [ip=%#llx] [sp=%#llx]\n",
-			    i, "<unknown symbol>",
-			    (long long)ip, (long long) sp));
-	    }
-	    ++i;
-	} while (unw_step(&cursor) > 0);
-
-	return;
-
-libunwind_failed:
-	DEBUG(0, ("unable to produce a stack trace with libunwind\n"));
-
-#elif HAVE_BACKTRACE_SYMBOLS
-	void *backtrace_stack[BACKTRACE_STACK_SIZE];
-	size_t backtrace_size;
-	char **backtrace_strings;
-
-	/* get the backtrace (stack frames) */
-	backtrace_size = backtrace(backtrace_stack,BACKTRACE_STACK_SIZE);
-	backtrace_strings = backtrace_symbols(backtrace_stack, backtrace_size);
-
-	DEBUG(0, ("BACKTRACE: %lu stack frames:\n", 
-		  (unsigned long)backtrace_size));
-
-	if (backtrace_strings) {
-		int i;
-
-		for (i = 0; i < backtrace_size; i++)
-			DEBUGADD(0, (" #%u %s\n", i, backtrace_strings[i]));
-
-		/* Leak the backtrace_strings, rather than risk what free() might do */
-	}
-
-#else
-	DEBUG(0, ("unable to produce a stack trace on this platform\n"));
-#endif
-}
-
-/*******************************************************************
   A readdir wrapper which just returns the file name.
  ********************************************************************/
 
diff --git a/source3/wscript b/source3/wscript
index 8d658b2f53e..ab64e80214e 100644
--- a/source3/wscript
+++ b/source3/wscript
@@ -96,7 +96,7 @@ def configure(conf):
     # We crash without vfs_default
     required_static_modules.extend(TO_LIST('vfs_default'))
 
-    conf.CHECK_HEADERS('execinfo.h libunwind.h netdb.h')
+    conf.CHECK_HEADERS('netdb.h')
     conf.CHECK_HEADERS('linux/falloc.h linux/ioctl.h')
 
     conf.CHECK_FUNCS('getcwd fchown chmod fchmod mknod')
-- 
2.11.0


From 19dba766ad65916fe329db77f6bdd8dc59d882a2 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Tue, 10 Apr 2018 16:37:45 +1200
Subject: [PATCH 4/4] lib/util: Call log_stack_trace() in smb_panic_default()

This matches the AD DC with the behaviour in smbd.

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 lib/util/fault.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/util/fault.c b/lib/util/fault.c
index af06c9484db..77415e1134d 100644
--- a/lib/util/fault.c
+++ b/lib/util/fault.c
@@ -126,6 +126,7 @@ static void smb_panic_default(const char *why)
 {
 	DEBUG(0,("PANIC (pid %llu): %s\n",
 		    (unsigned long long)getpid(), why));
+	log_stack_trace();
 
 #if defined(HAVE_PRCTL) && defined(PR_SET_PTRACER)
 	/*
-- 
2.11.0



More information about the samba-technical mailing list