[FFI] Improve traceback setups

This PR improves traceback related setups

Author: tqchen

ffi/CMakeLists.txt

@@ -126,6 +126,8 @@ if (MSVC)
   target_link_libraries(tvm_ffi_objs PRIVATE DbgHelp.lib)
   target_link_libraries(tvm_ffi_shared PRIVATE DbgHelp.lib)
   target_link_libraries(tvm_ffi_static PRIVATE DbgHelp.lib)
+  # produce pdb file
+  target_link_options(tvm_ffi_shared PRIVATE /DEBUG)
 endif ()
 
 # expose the headers as public dependencies
@@ -141,24 +143,21 @@ if (NOT ${PROJECT_NAME} STREQUAL ${CMAKE_PROJECT_NAME})
   return()
 endif()
 
-option(TVM_FFI_USE_RELEASE_FLAGS "Set O3 with RelWithDebInfo, useful to get lineno" OFF)
+option(TVM_FFI_ATTACH_DEBUG_SYMBOLS "Attach debug symbols even in release mode" OFF)
 option(TVM_FFI_BUILD_TESTS "Adding test targets." OFF)
 
-# related options
-if(CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang")
-  if (TVM_FFI_USE_RELEASE_FLAGS)
-    message(STATUS "Targeting global CXX flags to -O3 for optimized build")
-    target_compile_options(tvm_ffi_objs PRIVATE -O3 -g)
-    target_compile_options(tvm_ffi_shared PRIVATE -O3)
-    target_compile_options(tvm_ffi_static PRIVATE -O3)
+if (TVM_FFI_ATTACH_DEBUG_SYMBOLS)
+  if(CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang")
+    target_compile_options(tvm_ffi_objs PRIVATE -g1)
   endif()
 endif()
 
-
 include(cmake/Utils/CxxWarning.cmake)
 include(cmake/Utils/Sanitizer.cmake)
 
-tvm_ffi_add_cxx_warning(tvm_ffi_objs)
+# remap the file name to the source directory so we can see the
+# exact file name in traceback relative to the project source root
+tvm_ffi_add_prefix_map(tvm_ffi_objs ${CMAKE_SOURCE_DIR})
 
 ########## Adding cpp tests ##########
 
@@ -169,21 +168,7 @@ if (TVM_FFI_BUILD_TESTS)
   message(STATUS "Enable Testing")
   include(cmake/Utils/AddGoogleTest.cmake)
   add_subdirectory(tests/cpp/)
-endif()
-
-########## Install the related for normal cmake library ##########
-
-install(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/include/tvm/ffi/ DESTINATION include/tvm/ffi/)
-install(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/3rdparty/dlpack/include/ DESTINATION include/)
-
-if (WIN32)
-  # on windows, install by target is more robust as sometimes target may
-  # append subfolders lib/Release in msbuild
-  install(TARGETS tvm_ffi_shared tvm_ffi_static DESTINATION lib)
-else()
-  # on mac and other platforms, install by dir helps to also copy
-  # .dSYM over for debugging symbols
-  install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/lib/ DESTINATION lib)
+  tvm_ffi_add_cxx_warning(tvm_ffi_objs)
 endif()
 
 ########## Adding python module ##########
@@ -208,9 +193,12 @@ if (TVM_FFI_BUILD_PYTHON_MODULE)
     ${CMAKE_CURRENT_SOURCE_DIR}/python/tvm_ffi/cython/object.pxi
     ${CMAKE_CURRENT_SOURCE_DIR}/python/tvm_ffi/cython/string.pxi
   )
+  # set working directory to source so we can see the exact file name in traceback
+  # relatived to the project source root
   add_custom_command(
       OUTPUT ${core_cpp}
       COMMAND ${Python_EXECUTABLE} -m cython --cplus ${core_pyx} -o ${core_cpp}
+      WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
       COMMENT "Transpiling ${core_pyx} to ${core_cpp}"
       DEPENDS ${cython_sources}
       VERBATIM
@@ -247,3 +235,18 @@ if (TVM_FFI_BUILD_PYTHON_MODULE)
   install(FILES ${CMAKE_CURRENT_SOURCE_DIR}/CMakeLists.txt DESTINATION .)
   install(FILES ${CMAKE_CURRENT_SOURCE_DIR}/cmake/tvm_ffi-config.cmake DESTINATION lib/cmake/tvm_ffi/)
 endif()
+
+########## Install the related for normal cmake library ##########
+
+install(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/include/tvm/ffi/ DESTINATION include/tvm/ffi/)
+install(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/3rdparty/dlpack/include/ DESTINATION include/)
+install(TARGETS tvm_ffi_shared  DESTINATION lib)
+# ship additional dSYM files for debugging symbols on if available
+if (APPLE)
+  install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/lib/ DESTINATION lib FILES_MATCHING PATTERN "*.dSYM")
+endif()
+
+if (NOT TVM_FFI_BUILD_PYTHON_MODULE)
+  # when building wheel, we do not ship static as we already ships source and dll
+  install(TARGETS tvm_ffi_static DESTINATION lib)
+endif()

ffi/cmake/Utils/Library.cmake

@@ -14,6 +14,14 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
+
+function(tvm_ffi_add_prefix_map target_name prefix_path)
+  # Add prefix map so the path displayed becomes relative to prefix_path
+  if(CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang")
+    target_compile_options(${target_name} PRIVATE "-ffile-prefix-map=${prefix_path}/=")
+  endif()
+endfunction()
+
 function(tvm_ffi_add_apple_dsymutil target_name)
   # running dsymutil on macos to generate debugging symbols for backtraces
   if(APPLE AND TVM_FFI_USE_LIBBACKTRACE)
@@ -36,7 +44,7 @@ function(tvm_ffi_add_msvc_flags target_name)
     target_compile_definitions(${target_name} PUBLIC -D_SCL_SECURE_NO_WARNINGS)
     target_compile_definitions(${target_name} PUBLIC -D_ENABLE_EXTENDED_ALIGNED_STORAGE)
     target_compile_definitions(${target_name} PUBLIC -DNOMINMAX)
-    target_compile_options(${target_name} PRIVATE "/Z7")
+    target_compile_options(${target_name} PRIVATE "/Zi")
   endif()
 endfunction()
 
@@ -77,5 +85,4 @@ function(tvm_ffi_add_target_from_obj target_name obj_target_name)
     endforeach()
   endif()
   tvm_ffi_add_apple_dsymutil(${target_name}_shared)
-  tvm_ffi_add_msvc_flags(${target_name}_shared)
 endfunction()

ffi/include/tvm/ffi/c_api.h

@@ -836,13 +836,15 @@ TVM_FFI_DLL const TVMFFITypeAttrColumn* TVMFFIGetTypeAttrColumn(const TVMFFIByte
  * \param filename The current file name.
  * \param lineno The current line number
  * \param func The current function
+ * \param cross_ffi_boundary Whether the traceback is crossing the ffi boundary
+ *                           or we should stop at the ffi boundary when detected
  * \return The traceback string
  *
- * \note filename func and lino are only used as a backup info, most cases they are not needed.
- *  The return value is set to const char* to be more compatible across dll boundaries.
+ * \note filename/func can be nullptr, then these info are skipped, they are useful
+ *       for cases when debug symbols is not available.
  */
 TVM_FFI_DLL const TVMFFIByteArray* TVMFFITraceback(const char* filename, int lineno,
-                                                   const char* func);
+                                                   const char* func, int cross_ffi_boundary);
 
 /*!
  * \brief Initialize the type info during runtime.

ffi/include/tvm/ffi/error.h

@@ -201,8 +201,6 @@ class ErrorBuilder {
   bool log_before_throw_;
 };
 
-// define traceback here as call into traceback function
-#define TVM_FFI_TRACEBACK_HERE TVMFFITraceback(__FILE__, __LINE__, TVM_FFI_FUNC_SIG)
 }  // namespace details
 
 /*!
@@ -216,9 +214,10 @@ class ErrorBuilder {
  *
  * \endcode
  */
-#define TVM_FFI_THROW(ErrorKind)                                        \
-  ::tvm::ffi::details::ErrorBuilder(#ErrorKind, TVM_FFI_TRACEBACK_HERE, \
-                                    TVM_FFI_ALWAYS_LOG_BEFORE_THROW)    \
+#define TVM_FFI_THROW(ErrorKind)                                                              \
+  ::tvm::ffi::details::ErrorBuilder(#ErrorKind,                                               \
+                                    TVMFFITraceback(__FILE__, __LINE__, TVM_FFI_FUNC_SIG, 0), \
+                                    TVM_FFI_ALWAYS_LOG_BEFORE_THROW)                          \
       .stream()
 
 /*!
@@ -228,8 +227,10 @@ class ErrorBuilder {
  *  cannot be caught, and it is better to have a clear log message.
  *  In most cases, we should use use TVM_FFI_THROW.
  */
-#define TVM_FFI_LOG_AND_THROW(ErrorKind) \
-  ::tvm::ffi::details::ErrorBuilder(#ErrorKind, TVM_FFI_TRACEBACK_HERE, true).stream()
+#define TVM_FFI_LOG_AND_THROW(ErrorKind)                                          \
+  ::tvm::ffi::details::ErrorBuilder(                                              \
+      #ErrorKind, TVMFFITraceback(__FILE__, __LINE__, TVM_FFI_FUNC_SIG, 0), true) \
+      .stream()
 
 // Glog style checks with TVM_FFI prefix
 // NOTE: we explicitly avoid glog style generic macros (LOG/CHECK) in tvm ffi

ffi/pyproject.toml

@@ -62,7 +62,7 @@ build.verbose = true
 cmake.version = "CMakeLists.txt"
 cmake.build-type = "Release"
 cmake.args = [
-  "-DTVM_FFI_USE_RELEASE_FLAGS=ON",
+  "-DTVM_FFI_ATTACH_DEBUG_SYMBOLS=ON",
   "-DTVM_FFI_BUILD_TESTS=OFF",
   "-DTVM_FFI_BUILD_PYTHON_MODULE=ON"
 ]

ffi/python/tvm_ffi/cython/base.pxi

@@ -187,7 +187,8 @@ cdef extern from "tvm/ffi/c_api.h":
     int TVMFFITypeKeyToIndex(TVMFFIByteArray* type_key, int32_t* out_tindex) nogil
     int TVMFFIDataTypeFromString(TVMFFIByteArray* str, DLDataType* out) nogil
     int TVMFFIDataTypeToString(const DLDataType* dtype, TVMFFIAny* out) nogil
-    const TVMFFIByteArray* TVMFFITraceback(const char* filename, int lineno, const char* func) nogil;
+    const TVMFFIByteArray* TVMFFITraceback(
+        const char* filename, int lineno, const char* func, int cross_ffi_boundary) nogil;
     int TVMFFINDArrayFromDLPack(DLManagedTensor* src, int32_t require_alignment,
                                 int32_t require_contiguous, TVMFFIObjectHandle* out) nogil
     int TVMFFINDArrayFromDLPackVersioned(DLManagedTensorVersioned* src,

ffi/python/tvm_ffi/cython/error.pxi

@@ -98,7 +98,7 @@ cdef inline int set_last_ffi_error(error) except -1:
     kind = ERROR_TYPE_TO_NAME.get(type(error), "RuntimeError")
     message = error.__str__()
     py_traceback = _TRACEBACK_TO_STR(error.__traceback__)
-    c_traceback = bytearray_to_str(TVMFFITraceback("<unknown>", 0, "<unknown>"))
+    c_traceback = bytearray_to_str(TVMFFITraceback(NULL, 0, NULL, 0))
 
     # error comes from an exception thrown from C++ side
     if hasattr(error, "__tvm_ffi_error__"):

ffi/scripts/run_tests.sh

@@ -17,10 +17,10 @@
 # under the License.
 set -euxo pipefail
 
-BUILD_TYPE=RelWithDebugInfo
+BUILD_TYPE=Release
 
 rm -rf build/CMakeFiles build/CMakeCache.txt
 cmake -G Ninja -S . -B build  -DTVM_FFI_BUILD_TESTS=ON -DCMAKE_BUILD_TYPE=${BUILD_TYPE} \
-  -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DCMAKE_CXX_COMPILER_LAUNCHER=ccache
+  -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DCMAKE_CXX_COMPILER_LAUNCHER=ccache -DCMAKE_CXX_FLAGS="-O3"
 cmake --build build --parallel 16 --clean-first --config ${BUILD_TYPE} --target tvm_ffi_tests
 GTEST_COLOR=1 ctest -V -C ${BUILD_TYPE} --test-dir build --output-on-failure

ffi/src/ffi/error.cc

@@ -56,7 +56,8 @@ class SafeCallContext {
 
 void TVMFFIErrorSetRaisedFromCStr(const char* kind, const char* message) {
   // NOTE: run traceback here to simplify the depth of tracekback
-  tvm::ffi::SafeCallContext::ThreadLocal()->SetRaisedByCstr(kind, message, TVM_FFI_TRACEBACK_HERE);
+  tvm::ffi::SafeCallContext::ThreadLocal()->SetRaisedByCstr(
+      kind, message, TVMFFITraceback(nullptr, 0, nullptr, 0));
 }
 
 void TVMFFIErrorSetRaised(TVMFFIObjectHandle error) {

ffi/src/ffi/extra/testing.cc

@@ -55,11 +55,16 @@ class TestObjectDerived : public TestObjectBase {
   TVM_FFI_DECLARE_FINAL_OBJECT_INFO(TestObjectDerived, TestObjectBase);
 };
 
-void TestRaiseError(String kind, String msg) {
-  throw ffi::Error(kind, msg, TVM_FFI_TRACEBACK_HERE);
+TVM_FFI_NO_INLINE void TestRaiseError(String kind, String msg) {
+  // keep name and no liner for testing traceback
+  throw ffi::Error(kind, msg, TVMFFITraceback(__FILE__, __LINE__, TVM_FFI_FUNC_SIG, 0));
 }
 
-void TestApply(Function f, PackedArgs args, Any* ret) { f.CallPacked(args, ret); }
+TVM_FFI_NO_INLINE void TestApply(PackedArgs args, Any* ret) {
+  // keep name and no liner for testing traceback
+  auto f = args[0].cast<Function>();
+  f.CallPacked(args.Slice(1), ret);
+}
 
 TVM_FFI_STATIC_INIT_BLOCK({
   namespace refl = tvm::ffi::reflection;
@@ -78,11 +83,7 @@ TVM_FFI_STATIC_INIT_BLOCK({
       .def("testing.test_raise_error", TestRaiseError)
       .def_packed("testing.nop", [](PackedArgs args, Any* ret) { *ret = args[0]; })
       .def_packed("testing.echo", [](PackedArgs args, Any* ret) { *ret = args[0]; })
-      .def_packed("testing.apply",
-                  [](PackedArgs args, Any* ret) {
-                    auto f = args[0].cast<Function>();
-                    TestApply(f, args.Slice(1), ret);
-                  })
+      .def_packed("testing.apply", TestApply)
       .def("testing.run_check_signal",
            [](int nsec) {
              for (int i = 0; i < nsec; ++i) {