[GitHub] [tvm] driazati commented on a diff in pull request #12959: [LOGGING] Optionally print backtrace on segfault

[GitBox <gi...@apache.org>]

driazati commented on code in PR #12959:
URL: https://github.com/apache/tvm/pull/12959#discussion_r984994872


##########
src/runtime/logging.cc:
##########
@@ -117,6 +121,21 @@ int BacktraceFullCallback(void* data, uintptr_t pc, const char* filename, int li
   }
   return 0;
 }
+
+#if TVM_BACKTRACE_ON_SEGFAULT
+void backtrace_handler(int sig) {
+  // Technically we shouldn't do any allocation in a signal handler, but
+  // Backtrace may allocate. What's the worst it could do? We're already
+  // crashing.
+  std::cerr << "!!!!!!! TVM encountered a Segfault !!!!!!!\n" << Backtrace() << std::endl;
+  exit(1);
+}
+
+__attribute__((constructor)) void install_signal_handler(void) {
+  // this may override already install signal handlers

Review Comment:
   ```suggestion
     // this may override already installed signal handlers
   ```



##########
tests/scripts/task_config_build_cpu.sh:
##########
@@ -55,6 +55,7 @@ echo set\(USE_CMSISNN OFF\) >> config.cmake
 echo set\(USE_VITIS_AI ON\) >> config.cmake
 echo set\(USE_VERILATOR ON\) >> config.cmake
 echo set\(USE_LIBBACKTRACE ON\) >> config.cmake
+echo set\(BACKTRACE_ON_SEGFAULT ON\) >> config.cmake

Review Comment:
   What do you think about adding it to every CI config (i.e. all the `task_*` files)? We often get crashes in the cortexm and hexagon builds too



##########
src/runtime/logging.cc:
##########
@@ -117,6 +121,21 @@ int BacktraceFullCallback(void* data, uintptr_t pc, const char* filename, int li
   }
   return 0;
 }
+
+#if TVM_BACKTRACE_ON_SEGFAULT
+void backtrace_handler(int sig) {
+  // Technically we shouldn't do any allocation in a signal handler, but
+  // Backtrace may allocate. What's the worst it could do? We're already
+  // crashing.
+  std::cerr << "!!!!!!! TVM encountered a Segfault !!!!!!!\n" << Backtrace() << std::endl;
+  exit(1);

Review Comment:
   we can at least call back to the default signal handler with this to get the standard `terminated by signal SIGSEGV` text
   
   ```suggestion
   
     struct sigaction act;
     memset(&act, 0, sizeof(struct sigaction));
     act.sa_flags = SA_RESETHAND;
     act.sa_handler = SIG_DFL;
     sigaction(sig, &act, NULL);
     raise(sig);
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org