Attach backtrace signal handling with fl::init() (flashlight#1132)

Summary:
Downstream libs didn't properly initialize signal handling because the backward-cpp signal handler was a static in a C++ file and that could be loaded arbitrarily based on compilation options (static vs shared) and platforms.

Move signal handling init into `fl::init()`, which now calls `initLogging()` which conditionally calls `initBackward()` if compiled to not be a noop.

Also change the name of the directory from `common/backward` to `common/stacktrace`.

Pull Request resolved: flashlight#1132

Test Plan: CI

Reviewed By: bwasti

Differential Revision: D46824199

Pulled By: jacobkahn

fbshipit-source-id: b42efc045ea2528e86d61f2288b4e2d0d2c1863d

.circleci/config.yml

@@ -339,6 +339,7 @@ commands:
               -DFL_USE_CUDNN=$([ "<< parameters.autograd_backend >>" == "cudnn" ] && echo "ON" || echo "OFF") \
               -DFL_USE_ONEDNN=$([ "<< parameters.backend >>" == "onednn" ] || [ "<< parameters.autograd_backend >>" == "onednn" ] && echo "ON" || echo "OFF") \
               -DFL_BUILD_DISTRIBUTED=OFF \
+              -DFL_USE_BACKWARD_CPP=ON \
               -DCMAKE_WINDOWS_EXPORT_ALL_SYMBOLS=ON
             cmake --build build --parallel << parameters.build_parallelism >>
 

.clang-format

@@ -52,7 +52,7 @@ IncludeCategories:
   - Regex:           '.*'
     Priority:        3
 IndentCaseLabels: true
-IndentPPDirectives: BeforeHash
+IndentPPDirectives: None
 IndentWidth:     2
 IndentWrappedFunctionNames: false
 KeepEmptyLinesAtTheStartOfBlocks: false

.github/workflows/build.yml

@@ -33,6 +33,7 @@ jobs:
             -DFL_USE_ONEDNN=${{ (matrix.backend == 'oneDNN' || matrix.autograd_backend == 'oneDNN') && 'ON' || 'OFF'  }} \
             -DFL_USE_TENSOR_STUB=${{ matrix.backend == 'Stub' && 'ON' || 'OFF'  }} \
             -DFL_BUILD_DISTRIBUTED=OFF \
+            -DFL_USE_BACKWARD_CPP=ON \
             -DCMAKE_WINDOWS_EXPORT_ALL_SYMBOLS=ON
       - name: "Build Flashlight"
         run: |
@@ -121,5 +122,4 @@ jobs:
             -DFL_USE_ONEDNN=OFF \
             -DFL_USE_CUDNN=OFF \
             -DFL_BUILD_DISTRIBUTED=OFF \
-            -DFL_BUILD_EXAMPLES=ON \
-            -DFL_BUILD_BACKWARD_CPP=OFF
+            -DFL_BUILD_EXAMPLES=ON

README.md

@@ -433,10 +433,11 @@ Given a simple `project.cpp` file that includes and links to Flashlight:
 #include <flashlight/fl/flashlight.h>
 
 int main() {
- fl::Variable v(fl::full({1}, 1.), true);
- auto result = v + 10;
- std::cout << "Tensor value is " << result.tensor() << std::endl; // 11.000
- return 0;
+  fl::init();
+  fl::Variable v(fl::full({1}, 1.), true);
+  auto result = v + 10;
+  std::cout << "Tensor value is " << result.tensor() << std::endl; // 11.000
+  return 0;
 }
 ```
 

flashlight/fl/common/CMakeLists.txt

@@ -41,15 +41,5 @@ if (NOT MSVC)
   target_link_libraries(flashlight PUBLIC std::filesystem)
 endif()
 
-# backward-cpp
-option(FL_BUILD_BACKWARD_CPP "Build with backward-cpp support" ON)
-if (FL_BUILD_BACKWARD_CPP)
-  find_package(Backward CONFIG)
-  if (NOT Backward_FOUND)
-    message(STATUS "backward-cpp not found - will download and build from source")
-    include(${PROJECT_SOURCE_DIR}/cmake/BuildBackwardCpp.cmake)
-  endif()
-  target_sources(flashlight PRIVATE ${CMAKE_CURRENT_LIST_DIR}/backward/Backward.cpp)
-  # only use include dirs/compiler defs
-  target_link_libraries(flashlight PUBLIC $<BUILD_INTERFACE:Backward::Backward>)
-endif()
+# backward
+include(${CMAKE_CURRENT_LIST_DIR}/stacktrace/CMakeLists.txt)

flashlight/fl/common/Logging.cpp

@@ -12,8 +12,16 @@
 
 #include "flashlight/fl/common/Logging.h"
 #include "flashlight/fl/common/Utils.h"
+#include "flashlight/fl/common/stacktrace/Backward.h"
 
 namespace fl {
+
+void initLogging() {
+  // initialize backward stacktracing. noop if not built with a
+  // backtrace/stacktrace lib
+  detail::initBackward();
+}
+
 LogLevel Logging::maxLoggingLevel_ = DEFAULT_MAX_FL_LOGGING_LEVEL;
 int VerboseLogging::maxLoggingLevel_ = DEFAULT_MAX_VERBOSE_FL_LOGGING_LEVEL;
 

flashlight/fl/common/Logging.h

@@ -8,7 +8,6 @@
 
 #pragma once
 
-#include <signal.h>
 #include <iostream>
 #include <sstream>
 #include <string>
@@ -68,9 +67,13 @@
  * Note that `VLOG(level)` only prints when level is less than or equal to the
  * value set to `VerboseLogging`
  */
-
 namespace fl {
 
+/**
+ * Initialize all logging components including stacktraces and signal handling.
+ */
+void initLogging();
+
 /// \ingroup logging
 enum class LogLevel {
   DISABLED, // use only for when calling setMaxLoggingLevel() or

flashlight/fl/common/backward/Backward.cpp → flashlight/fl/common/stacktrace/Backward.cpp

@@ -34,10 +34,20 @@
 // - g++/clang++ -lunwind
 // #define BACKWARD_HAS_LIBUNWIND 1
 
+#include "flashlight/fl/common/stacktrace/Backward.h"
+
+#if FL_USE_BACKWARD_CPP
+// See https://github.com/bombela/backward-cpp
 #include "backward.hpp"
+#endif
 
-namespace backward {
+namespace fl::detail {
 
-backward::SignalHandling sh;
+void initBackward() {
+  // If not built with backward, this function is a noop
+#if FL_USE_BACKWARD_CPP
+  static ::backward::SignalHandling sh;
+#endif
+}
 
-} // namespace backward
+} // namespace fl::detail

flashlight/fl/common/stacktrace/Backward.h

@@ -0,0 +1,18 @@
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT-style license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+#pragma once
+
+namespace fl::detail {
+
+/**
+ * Initialize stack tracing with backward-cpp. No-op if Flashlight is not built
+ * with backward-cpp.
+ */
+void initBackward();
+
+} // namespace fl::detail

flashlight/fl/common/stacktrace/CMakeLists.txt

@@ -0,0 +1,18 @@
+cmake_minimum_required(VERSION 3.16)
+
+# backward-cpp
+option(FL_USE_BACKWARD_CPP "Build with backward-cpp support" OFF)
+if (FL_USE_BACKWARD_CPP)
+  find_package(Backward CONFIG)
+  if (NOT Backward_FOUND)
+    message(STATUS "backward-cpp not found - will download from source.")
+    include(${PROJECT_SOURCE_DIR}/cmake/BuildBackwardCpp.cmake)
+  endif()
+  add_backward(flashlight) # include dirs and compiler defs
+  # trace demangler libs
+  set_property(TARGET flashlight APPEND PROPERTY
+    INTERFACE_LINK_LIBRARIES "${BACKWARD_LIBRARIES}")
+endif()
+target_sources(flashlight PRIVATE ${CMAKE_CURRENT_LIST_DIR}/Backward.cpp)
+target_compile_definitions(flashlight PUBLIC
+  FL_USE_BACKWARD_CPP=$<BOOL:${FL_USE_BACKWARD_CPP}>)

flashlight/fl/tensor/Init.cpp

@@ -7,6 +7,7 @@
 
 #include <mutex>
 
+#include "flashlight/fl/common/Logging.h"
 #include "flashlight/fl/tensor/DefaultTensorType.h"
 #include "flashlight/fl/tensor/TensorBackend.h"
 
@@ -17,13 +18,17 @@ std::once_flag flInitFlag;
 
 /**
  * Initialize Flashlight. Performs setup, including:
- * - Ensures ArrayFire globals are initialized
- * - Sets the default memory manager (CachingMemoryManager)
+ * - Ensures default tensor backend globals are initialized, including memory
+ *   management, tensor backend state, computation stremas, etc.
+ * - Sets signal handlers helpful for debugging, if enabled.
  *
- * Can only be called once per process. Subsequent calls will be noops.
+ * Body is only run once per process. Subsequent calls will be noops.
  */
 void init() {
-  std::call_once(flInitFlag, []() { defaultTensorBackend(); });
+  std::call_once(flInitFlag, []() {
+    defaultTensorBackend();
+    initLogging();
+  });
 }
 
 } // namespace fl