Improve diagnosis of SEGV failures in tests

commandline.c

@@ -50,6 +50,10 @@ int cli_usage() {
   return -1;
 }
 
+/* Remember the name by which this program was invoked.
+*/
+const char *exec_argv0 = NULL;
+
 /* Data structures for accumulating output of a single JNI call.
 */
 
@@ -160,7 +164,7 @@ JNIEXPORT jint JNICALL Java_org_servalproject_servald_ServalD_rawCommand(JNIEnv
     outv_current = outv_buffer;
     // Execute the command.
     jni_env = env;
-    status = parseCommandLine(argc, argv);
+    status = parseCommandLine(NULL, argc, argv);
     jni_env = NULL;
   }
   // Release argv Java string buffers.
@@ -205,9 +209,10 @@ static void complainCommandLine(const char *prefix, int argc, const char *const
   WHYF("%s%s%s", prefix, strbuf_str(b), strbuf_overrun(b) ? "..." : "");
 }
 
-/* args[] excludes command name (unless hardlinks are used to use first words 
-   of command sequences as alternate names of the command. */
-int parseCommandLine(int argc, const char *const *args)
+/* The argc and argv arguments must be passed verbatim from main(argc, argv), so argv[0] is path to
+   executable.
+*/
+int parseCommandLine(const char *argv0, int argc, const char *const *args)
 {
   int i;
   int ambiguous=0;
@@ -216,6 +221,7 @@ int parseCommandLine(int argc, const char *const *args)
   fd_clearstats();
 
   IN();
+  exec_argv0 = argv0;
   for(i=0;command_line_options[i].function;i++)
     {
       int j;
@@ -579,6 +585,11 @@ int app_server_start(int argc, const char *const *argv, struct command_line_opti
   if (cli_arg(argc, argv, o, "instance path", &thisinstancepath, cli_absolute_path, NULL) == -1
    || cli_arg(argc, argv, o, "exec path", &execpath, cli_absolute_path, NULL) == -1)
     return -1;
+  if (execpath == NULL) {
+    if (jni_env)
+      return WHY("Must supply <exec path> argument when invoked via JNI");
+    execpath = exec_argv0;
+  }
   /* Create the instance directory if it does not yet exist */
   if (create_serval_instance_dir() == -1)
     return -1;
@@ -593,7 +604,9 @@ int app_server_start(int argc, const char *const *argv, struct command_line_opti
   if (pid < 0)
     return -1;
   int ret = -1;
-  if (pid > 0) {
+  // If the pidfile identifies this process, it probably means we are re-spawning after a SEGV, so
+  // go ahead and do the fork/exec.
+  if (pid > 0 && pid != getpid()) {
     INFOF("Server already running (pid=%d)", pid);
     ret = 10;
   } else {
@@ -622,7 +635,10 @@ int app_server_start(int argc, const char *const *argv, struct command_line_opti
 	  _exit(WHY_perror("open"));
 	if (setsid() == -1)
 	  _exit(WHY_perror("setsid"));
-	(void)chdir("/");
+	const char *dir = getenv("SERVALD_SERVER_CHDIR");
+	if (!dir)
+	  dir = confValueGet("server.chdir", "/");
+	(void)chdir(dir);
 	(void)dup2(fd, 0);
 	(void)dup2(fd, 1);
 	(void)dup2(fd, 2);

main.c

@@ -37,7 +37,7 @@ int main(int argc, char **argv)
   if (argc > 1 && argv[1][0] == '-')
     status = parseOldCommandLine(argc, argv);
   else
-    status = parseCommandLine(argc - 1, (const char*const*)&argv[1]);
+    status = parseCommandLine(argv[0], argc - 1, (const char*const*)&argv[1]);
 #if defined WIN32
   WSACleanup();
 #endif

serval.h

@@ -1165,7 +1165,7 @@ typedef struct dna_identity_status {
   int uniqueDidAndName;
 } dna_identity_status;
 
-int parseCommandLine(int argc, const char *const *argv);
+int parseCommandLine(const char *argv0, int argc, const char *const *argv);
 
 int parseOldCommandLine(int argc, char **argv);
 int parseAssignment(unsigned char *text, int *var_id, unsigned char *value, int *value_len);

servalwrap.c

@@ -25,6 +25,5 @@ int main(int argc,char **argv)
  void *h = dlopen("/data/data/org.servalproject/lib/libserval.so",RTLD_LAZY);
  int (*servalmain)(int,const char *const*) = dlsym(h,"parseCommandLine");
  if (!servalmain) return fprintf(stderr,"Could not load libserval.so\n");
- return (*servalmain)(argc - 1, (const char*const*)&argv[1]);
-
+ return (*servalmain)(argv[0], argc - 1, (const char*const*)&argv[1]);
 }

server.c

@@ -25,11 +25,13 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 
 #include "serval.h"
 #include "strbuf.h"
+#include "strbuf_helpers.h"
 
 #define PIDFILE_NAME	  "servald.pid"
 #define STOPFILE_NAME	  "servald.stop"
 
-char *exec_args[128];
+#define EXEC_NARGS 20
+char *exec_args[EXEC_NARGS + 1];
 int exec_argc = 0;
 
 int serverMode=0;
@@ -151,9 +153,9 @@ int server_pid()
 void server_save_argv(int argc, const char *const *argv)
 {
     /* Save our argv[] to use for relaunching */
-    for (exec_argc = 0; exec_argc != argc; ++exec_argc)
+    for (exec_argc = 0; exec_argc < argc && exec_argc < EXEC_NARGS; ++exec_argc)
       exec_args[exec_argc] = strdup(argv[exec_argc]);
-    exec_args[exec_argc] = 0;
+    exec_args[exec_argc] = NULL;
 }
 
 int server(char *backing_file)
@@ -433,14 +435,19 @@ void signal_handler(int signal)
       return;
   }
   /* oops - caught a bad signal -- exec() ourselves fresh */
-  INFO("Respawning");
-  if (sock>-1) close(sock);
+  if (sock>-1)
+    close(sock);
   int i;
   for(i=0;i<overlay_interface_count;i++)
     if (overlay_interfaces[i].alarm.poll.fd>-1)
       close(overlay_interfaces[i].alarm.poll.fd);
-  execv(exec_args[0],exec_args);
+  strbuf b = strbuf_alloca(1024);
+  for (i = 0; i < exec_argc; ++i)
+    strbuf_append_shell_quotemeta(strbuf_puts(b, i ? " " : ""), exec_args[i]);
+  INFOF("Respawning %s", strbuf_str(b));
+  execv(exec_args[0], exec_args);
   /* Quit if the exec() fails */
+  WHY_perror("execv");
   exit(-3);
 } 
 

strbuf_helpers.c

@@ -19,6 +19,7 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 
 #include "strbuf_helpers.h"
 #include <poll.h>
+#include <ctype.h>
 
 strbuf strbuf_append_poll_events(strbuf sb, short events)
 {
@@ -61,3 +62,29 @@ strbuf strbuf_append_poll_events(strbuf sb, short events)
     strbuf_putc(sb, '0');
   return sb;
 }
+
+static int is_shellmeta(char c)
+{
+  return !(isalnum(c) || c == '.' || c == '-' || c == '/' || c == ':' || c == '+' || c == '_' || c == ',');
+}
+
+strbuf strbuf_append_shell_quotemeta(strbuf sb, const char *word)
+{
+  const char *p;
+  int hasmeta = 0;
+  for (p = word; *p && !hasmeta; ++p)
+    if (is_shellmeta(*p))
+      hasmeta = 1;
+  if (hasmeta) {
+    strbuf_putc(sb, '\'');
+    for (p = word; *p; ++p)
+      if (*p == '\'')
+	strbuf_puts(sb, "'\\''");
+      else
+	strbuf_putc(sb, *p);
+    strbuf_putc(sb, '\'');
+  } else {
+    strbuf_puts(sb, word);
+  }
+  return sb;
+}

strbuf_helpers.h

@@ -27,4 +27,9 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
 strbuf strbuf_append_poll_events(strbuf sb, short events);
 
+/* Append a string with shell metacharacters and spaces quoted.
+ * @author Andrew Bettison <andrew@servalproject.com>
+ */
+strbuf strbuf_append_shell_quotemeta(strbuf sb, const char *word);
+
 #endif //__STRBUF_HELPERS_H__

testdefs.sh

@@ -78,7 +78,7 @@ setup_servald() {
 #  - executes $servald with the given arguments
 #  - asserts that standard error contains no error messages
 executeOk_servald() {
-   executeOk --executable=$servald "$@"
+   executeOk --core-backtrace --executable=$servald "$@"
    assertStderrGrep --matches=0 --message="stderr of ($executed) contains no error messages" '^ERROR:'
 }
 
@@ -167,7 +167,7 @@ start_servald_server() {
    local -a after_pids
    get_servald_pids before_pids
    tfw_log "# before_pids=$before_pids"
-   SERVALD_LOG_FILE="$instance_servald_log" executeOk $servald start "$@"
+   SERVALD_SERVER_CHDIR="$instance_dir" SERVALD_LOG_FILE="$instance_servald_log" executeOk --core-backtrace $servald start "$@"
    extract_stdout_keyvalue start_instance_path instancepath '.*'
    extract_stdout_keyvalue start_pid pid '[0-9]\+'
    assert [ "$start_instance_path" = "$SERVALINSTANCE_PATH" ]
@@ -218,7 +218,7 @@ stop_servald_server() {
    local -a after_pids
    get_servald_pids before_pids
    tfw_log "# before_pids=$before_pids"
-   execute $servald stop "$@"
+   execute --core-backtrace $servald stop "$@"
    extract_stdout_keyvalue stop_instance_path instancepath '.*'
    assert [ "$stop_instance_path" = "$SERVALINSTANCE_PATH" ]
    if [ -n "$servald_pid" ]; then
@@ -239,6 +239,11 @@ stop_servald_server() {
    fi
    # Append the server log file to the test log.
    [ -s "$instance_servald_log" ] && tfw_cat "$instance_servald_log"
+   # Append a core dump backtrace to the test log.
+   if [ -s "$instance_dir/core" ]; then
+      tfw_core_backtrace "$servald" "$instance_dir/core"
+      rm -f "$instance_dir/core"
+   fi
    # Check there is at least one fewer servald processes running.
    for bpid in ${before_pids[*]}; do
       local isgone=true