This is an automated email from the ASF dual-hosted git repository. tqchen pushed a commit to branch debug-symbol in repository https://gitbox.apache.org/repos/asf/tvm.git
commit 3e43fb2a0648601c49db385ef8c8f408f55598be Author: tqchen <[email protected]> AuthorDate: Sun Aug 24 13:24:15 2025 -0400 Tracback handling --- ffi/include/tvm/ffi/c_api.h | 4 +-- ffi/include/tvm/ffi/error.h | 15 +++++----- ffi/scripts/run_tests.sh | 4 +-- ffi/src/ffi/error.cc | 3 +- ffi/src/ffi/extra/testing.cc | 2 +- ffi/src/ffi/traceback.cc | 65 +++++++++++++++++++++++++------------------- ffi/src/ffi/traceback.h | 23 ++++++++++------ ffi/src/ffi/traceback_win.cc | 35 +++++++++++++----------- src/tir/schedule/error.h | 3 +- 9 files changed, 87 insertions(+), 67 deletions(-) diff --git a/ffi/include/tvm/ffi/c_api.h b/ffi/include/tvm/ffi/c_api.h index 39b7de69fa..d54ef0ea49 100644 --- a/ffi/include/tvm/ffi/c_api.h +++ b/ffi/include/tvm/ffi/c_api.h @@ -838,8 +838,8 @@ TVM_FFI_DLL const TVMFFITypeAttrColumn* TVMFFIGetTypeAttrColumn(const TVMFFIByte * \param func The current function * \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); diff --git a/ffi/include/tvm/ffi/error.h b/ffi/include/tvm/ffi/error.h index 4430cb121a..5e5ceac667 100644 --- a/ffi/include/tvm/ffi/error.h +++ b/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), \ + 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), true) \ + .stream() // Glog style checks with TVM_FFI prefix // NOTE: we explicitly avoid glog style generic macros (LOG/CHECK) in tvm ffi diff --git a/ffi/scripts/run_tests.sh b/ffi/scripts/run_tests.sh index 8fc9eb95d0..7fe292a12c 100755 --- a/ffi/scripts/run_tests.sh +++ b/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 diff --git a/ffi/src/ffi/error.cc b/ffi/src/ffi/error.cc index 9fd81c4789..1ed0476203 100644 --- a/ffi/src/ffi/error.cc +++ b/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)); } void TVMFFIErrorSetRaised(TVMFFIObjectHandle error) { diff --git a/ffi/src/ffi/extra/testing.cc b/ffi/src/ffi/extra/testing.cc index 3d27d5ccb6..e3efba6350 100644 --- a/ffi/src/ffi/extra/testing.cc +++ b/ffi/src/ffi/extra/testing.cc @@ -56,7 +56,7 @@ class TestObjectDerived : public TestObjectBase { }; void TestRaiseError(String kind, String msg) { - throw ffi::Error(kind, msg, TVM_FFI_TRACEBACK_HERE); + throw ffi::Error(kind, msg, TVMFFITraceback(__FILE__, __LINE__, TVM_FFI_FUNC_SIG)); } void TestApply(Function f, PackedArgs args, Any* ret) { f.CallPacked(args, ret); } diff --git a/ffi/src/ffi/traceback.cc b/ffi/src/ffi/traceback.cc index 90d02121f0..debe5c5b07 100644 --- a/ffi/src/ffi/traceback.cc +++ b/ffi/src/ffi/traceback.cc @@ -45,7 +45,6 @@ namespace tvm { namespace ffi { namespace { - void BacktraceCreateErrorCallback(void*, const char* msg, int) { std::cerr << "Could not initialize backtrace state: " << msg << std::endl; } @@ -96,6 +95,8 @@ int BacktraceFullCallback(void* data, uintptr_t pc, const char* filename, int li backtrace_syminfo(_bt_state, pc, BacktraceSyminfoCallback, BacktraceErrorCallback, &symbol_str); } symbol = symbol_str.data(); + // TVMFFITraceback function itself does not count + if (IsTracebackFunction(filename, symbol)) return 0; if (stack_trace->ExceedTracebackLimit()) { return 1; @@ -103,35 +104,53 @@ int BacktraceFullCallback(void* data, uintptr_t pc, const char* filename, int li if (ShouldStopTraceback(filename, symbol)) { return 1; } + // skip extra frames + if (stack_trace->skip_frame_count > 0) { + stack_trace->skip_frame_count--; + return 0; + } if (ShouldExcludeFrame(filename, symbol)) { return 0; } stack_trace->Append(filename, symbol, lineno); return 0; } +} // namespace +} // namespace ffi +} // namespace tvm -std::string Traceback() { - TracebackStorage traceback; - - if (_bt_state == nullptr) { - return ""; +const TVMFFIByteArray* TVMFFITraceback(const char* filename, int lineno, const char* func) { + // We collapse the traceback into a single function + // to simplify the traceback detection handling (since we need to detect TVMFFITraceback) + static thread_local std::string traceback_str; + static thread_local TVMFFIByteArray traceback_array; + // pass in current line as here so last line of traceback is always accurate + tvm::ffi::TracebackStorage traceback; + if (filename != nullptr && func != nullptr) { + traceback.skip_frame_count = 1; + traceback.Append(filename, func, lineno); } // libbacktrace eats memory if run on multiple threads at the same time, so we guard against it - { + if (tvm::ffi::_bt_state != nullptr) { static std::mutex m; std::lock_guard<std::mutex> lock(m); - backtrace_full(_bt_state, 0, BacktraceFullCallback, BacktraceErrorCallback, &traceback); + backtrace_full(tvm::ffi::_bt_state, 0, tvm::ffi::BacktraceFullCallback, + tvm::ffi::BacktraceErrorCallback, &traceback); } - return traceback.GetTraceback(); + traceback_str = traceback.GetTraceback(); + traceback_array.data = traceback_str.data(); + traceback_array.size = traceback_str.size(); + return &traceback_array; } #if TVM_FFI_BACKTRACE_ON_SEGFAULT -void backtrace_handler(int sig) { +void TVMFFISegFaultHandler(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 FFI encountered a Segfault !!!!!!!\n" << Traceback() << std::endl; - + const TVMFFIByteArray* traceback = TVMFFITraceback(nullptr, 0, nullptr); + std::cerr << "!!!!!!! Segfault encountered !!!!!!!\n" + << std::string(traceback->data, traceback->size) << std::endl; // Re-raise signal with default handler struct sigaction act; std::memset(&act, 0, sizeof(struct sigaction)); @@ -141,31 +160,21 @@ void backtrace_handler(int sig) { raise(sig); } -__attribute__((constructor)) void install_signal_handler(void) { +__attribute__((constructor)) void TVMFFIInstallSignalHandler(void) { // this may override already installed signal handlers - std::signal(SIGSEGV, backtrace_handler); + std::signal(SIGSEGV, TVMFFISegFaultHandler); } #endif // TVM_FFI_BACKTRACE_ON_SEGFAULT -} // namespace -} // namespace ffi -} // namespace tvm - -const TVMFFIByteArray* TVMFFITraceback(const char*, int, const char*) { - static thread_local std::string traceback_str; - static thread_local TVMFFIByteArray traceback_array; - traceback_str = ::tvm::ffi::Traceback(); - traceback_array.data = traceback_str.data(); - traceback_array.size = traceback_str.size(); - return &traceback_array; -} #else // fallback implementation simply print out the last trace const TVMFFIByteArray* TVMFFITraceback(const char* filename, int lineno, const char* func) { static thread_local std::string traceback_str; static thread_local TVMFFIByteArray traceback_array; std::ostringstream traceback_stream; - // python style backtrace - traceback_stream << " File \"" << filename << "\", line " << lineno << ", in " << func << '\n'; + if (filename != nullptr && func != nullptr) { + // python style backtrace + traceback_stream << " File \"" << filename << "\", line " << lineno << ", in " << func << '\n'; + } traceback_str = traceback_stream.str(); traceback_array.data = traceback_str.data(); traceback_array.size = traceback_str.size(); diff --git a/ffi/src/ffi/traceback.h b/ffi/src/ffi/traceback.h index 47b91e16b0..7a8ac1e7b8 100644 --- a/ffi/src/ffi/traceback.h +++ b/ffi/src/ffi/traceback.h @@ -48,6 +48,18 @@ inline int32_t GetTracebackLimit() { #pragma warning(pop) #endif +bool IsTracebackFunction(const char*, const char* symbol) { + if (symbol != nullptr) { + if (strncmp(symbol, "TVMFFITraceback", 15) == 0) { + return true; + } + if (strncmp(symbol, "TVMFFIErrorSetRaisedFromCStr", 28) == 0) { + return true; + } + } + return false; +} + /*! * \brief List frame patterns that should be excluded as they contain less information */ @@ -66,12 +78,6 @@ inline bool ShouldExcludeFrame(const char* filename, const char* symbol) { if (strstr(filename, "include/tvm/ffi/any.h")) { return true; } - if (strstr(filename, "include/tvm/runtime/logging.h")) { - return true; - } - if (strstr(filename, "src/ffi/traceback.cc")) { - return true; - } // C++ stdlib frames if (strstr(filename, "include/c++/")) { return true; @@ -84,9 +90,6 @@ inline bool ShouldExcludeFrame(const char* filename, const char* symbol) { return true; } } - if (strncmp(symbol, "TVMFFIErrorSetRaisedFromCStr", 28) == 0) { - return true; - } // libffi.so stack frames. These may also show up as numeric // addresses with no symbol name. This could be improved in the // future by using dladdr() to check whether an address is contained @@ -126,6 +129,8 @@ struct TracebackStorage { std::vector<std::string> lines; /*! \brief Maximum size of the traceback. */ size_t max_frame_size = GetTracebackLimit(); + /*! \brief Number of frames to skip. */ + size_t skip_frame_count = 0; void Append(const char* filename, const char* func, int lineno) { // skip frames with empty filename diff --git a/ffi/src/ffi/traceback_win.cc b/ffi/src/ffi/traceback_win.cc index 8278de1d77..1b7d2ee71c 100644 --- a/ffi/src/ffi/traceback_win.cc +++ b/ffi/src/ffi/traceback_win.cc @@ -36,12 +36,17 @@ #include "./traceback.h" -namespace tvm { -namespace ffi { -namespace { +const TVMFFIByteArray* TVMFFITraceback(const char* filename, int lineno, const char* func) { + static thread_local std::string traceback_str; + static thread_local TVMFFIByteArray traceback_array; + + // pass in current line as here so last line of traceback is always accurate + tvm::ffi::TracebackStorage traceback; + if (filename != nullptr && func != nullptr) { + traceback.skip_frame_count = 1; + traceback.Append(filename, func, lineno); + } -std::string Traceback() { - TracebackStorage traceback; HANDLE process = GetCurrentProcess(); HANDLE thread = GetCurrentThread(); @@ -106,26 +111,24 @@ std::string Traceback() { if (SymFromAddr(process, stack.AddrPC.Offset, &displacement, symbol_info)) { symbol = symbol_info->Name; } - + if (IsTracebackFunction(filename, symbol)) { + continue; + } if (ShouldStopTraceback(filename, symbol)) { break; } + // skip extra frames + if (traceback.skip_frame_count > 0) { + traceback.skip_frame_count--; + continue; + } if (ShouldExcludeFrame(filename, symbol)) { continue; } traceback.Append(filename, symbol, lineno); } SymCleanup(process); - return traceback.GetTraceback(); -} -} // namespace -} // namespace ffi -} // namespace tvm - -const TVMFFIByteArray* TVMFFITraceback(const char* filename, int lineno, const char* func) { - static thread_local std::string traceback_str; - static thread_local TVMFFIByteArray traceback_array; - traceback_str = ::tvm::ffi::Traceback(); + traceback_str = traceback.GetTraceback(); traceback_array.data = traceback_str.data(); traceback_array.size = traceback_str.size(); return &traceback_array; diff --git a/src/tir/schedule/error.h b/src/tir/schedule/error.h index f579a54bbc..0c185bea34 100644 --- a/src/tir/schedule/error.h +++ b/src/tir/schedule/error.h @@ -30,7 +30,8 @@ namespace tir { class ScheduleError : public tvm::runtime::Error { public: /*! \brief Base constructor */ - ScheduleError() : tvm::runtime::Error("ScheduleError", "", TVM_FFI_TRACEBACK_HERE) {} + ScheduleError() + : tvm::runtime::Error("ScheduleError", "", TVMFFITraceback(nullptr, 0, nullptr)) {} /*! \brief The error occurred in this IRModule */ virtual IRModule mod() const = 0; /*! \brief The locations of interest that we want to point out */
