https://github.com/delcypher updated https://github.com/llvm/llvm-project/pull/165996
>From c5ea9f5e3ac056a2681851b77c830fbc93a01edd Mon Sep 17 00:00:00 2001 From: Dan Liew <[email protected]> Date: Sat, 1 Nov 2025 02:19:20 -0700 Subject: [PATCH] [Clang][LLDB] Refactor trap reason demangling out of LLDB and into Clang This patch refactors the trap reason demangling logic in `lldb_private::VerboseTrapFrameRecognizer::RecognizeFrame` into a new public function `clang::CodeGen::DemangleTrapReasonInDebugInfo`. There are two reasons for doing this: 1. In a future patch the logic for demangling needs to be used somewhere else in LLDB and thus the logic needs refactoring to avoid duplicating code. 2. The logic for demangling shouldn't really be in LLDB anyway because it's a Clang implementation detail and thus the logic really belongs inside Clang, not LLDB. Unit tests have been added for the new function that demonstrate how to use the new API. The function names recognized by VerboseTrapFrameRecognizer are identical to before. However, this patch isn't NFC because: * The `lldbTarget` library now links against `clangCodeGen` which it didn't previously. * The LLDB logging output is a little different now. The previous code tried to log failures for an invalid regex pattern and for the `Regex::match` API not returning the correct number of matches. These failure conditions are unreachable via unit testing so they have been made assertions failures inside the `DemangleTrapReasonInDebugInfo` implementation instead of trying to log them in LLDB. rdar://163230807 --- clang/include/clang/CodeGen/ModuleBuilder.h | 17 +++++ clang/lib/CodeGen/ModuleBuilder.cpp | 29 ++++++++ clang/unittests/CodeGen/CMakeLists.txt | 1 + .../CodeGen/DemangleTrapReasonInDebugInfo.cpp | 67 +++++++++++++++++++ lldb/source/Target/CMakeLists.txt | 3 + .../Target/VerboseTrapFrameRecognizer.cpp | 31 ++------- 6 files changed, 123 insertions(+), 25 deletions(-) create mode 100644 clang/unittests/CodeGen/DemangleTrapReasonInDebugInfo.cpp diff --git a/clang/include/clang/CodeGen/ModuleBuilder.h b/clang/include/clang/CodeGen/ModuleBuilder.h index f1b8229edd362..4298ba06c472e 100644 --- a/clang/include/clang/CodeGen/ModuleBuilder.h +++ b/clang/include/clang/CodeGen/ModuleBuilder.h @@ -120,6 +120,23 @@ CodeGenerator *CreateLLVMCodeGen(DiagnosticsEngine &Diags, llvm::LLVMContext &C, CoverageSourceInfo *CoverageInfo = nullptr); +namespace CodeGen { +/// Demangle the artificial function name (\param FuncName) used to encode trap +/// reasons used in debug info for traps (e.g. __builtin_verbose_trap). See +/// `CGDebugInfo::CreateTrapFailureMessageFor`. +/// +/// \param FuncName - The function name to demangle. +/// +/// \return A std::optional. If demangling succeeds the optional will contain +/// a pair of StringRefs where the first field is the trap category and the +/// second is the trap message. These can both be empty. If demangling fails the +/// optional will not contain a value. Note the returned StringRefs if non-empty +/// point into the underlying storage for \param FuncName and thus have the same +/// lifetime. +std::optional<std::pair<StringRef, StringRef>> +DemangleTrapReasonInDebugInfo(StringRef FuncName); +} // namespace CodeGen + } // end namespace clang #endif diff --git a/clang/lib/CodeGen/ModuleBuilder.cpp b/clang/lib/CodeGen/ModuleBuilder.cpp index 96f3f6221e20f..8ec8aef311656 100644 --- a/clang/lib/CodeGen/ModuleBuilder.cpp +++ b/clang/lib/CodeGen/ModuleBuilder.cpp @@ -23,6 +23,7 @@ #include "llvm/IR/DataLayout.h" #include "llvm/IR/LLVMContext.h" #include "llvm/IR/Module.h" +#include "llvm/Support/FormatVariadic.h" #include "llvm/Support/VirtualFileSystem.h" #include <memory> @@ -378,3 +379,31 @@ clang::CreateLLVMCodeGen(DiagnosticsEngine &Diags, llvm::StringRef ModuleName, HeaderSearchOpts, PreprocessorOpts, CGO, C, CoverageInfo); } + +namespace clang { +namespace CodeGen { +std::optional<std::pair<StringRef, StringRef>> +DemangleTrapReasonInDebugInfo(StringRef FuncName) { + static auto TrapRegex = + llvm::Regex(llvm::formatv("^{0}\\$(.*)\\$(.*)$", ClangTrapPrefix).str()); + llvm::SmallVector<llvm::StringRef, 3> Matches; + std::string *ErrorPtr = nullptr; +#ifndef NDEBUG + std::string Error; + ErrorPtr = &Error; +#endif + if (!TrapRegex.match(FuncName, &Matches, ErrorPtr)) { + assert(ErrorPtr && ErrorPtr->empty() && "Invalid regex pattern"); + return {}; + } + + if (Matches.size() != 3) { + assert(0 && "Expected 3 matches from Regex::match"); + return {}; + } + + // Returns { Trap Category, Trap Message } + return std::make_pair(Matches[1], Matches[2]); +} +} // namespace CodeGen +} // namespace clang diff --git a/clang/unittests/CodeGen/CMakeLists.txt b/clang/unittests/CodeGen/CMakeLists.txt index f5bcecb0b08a3..d4efb2230a054 100644 --- a/clang/unittests/CodeGen/CMakeLists.txt +++ b/clang/unittests/CodeGen/CMakeLists.txt @@ -1,6 +1,7 @@ add_clang_unittest(ClangCodeGenTests BufferSourceTest.cpp CodeGenExternalTest.cpp + DemangleTrapReasonInDebugInfo.cpp TBAAMetadataTest.cpp CheckTargetFeaturesTest.cpp CLANG_LIBS diff --git a/clang/unittests/CodeGen/DemangleTrapReasonInDebugInfo.cpp b/clang/unittests/CodeGen/DemangleTrapReasonInDebugInfo.cpp new file mode 100644 index 0000000000000..17bfe17c31d65 --- /dev/null +++ b/clang/unittests/CodeGen/DemangleTrapReasonInDebugInfo.cpp @@ -0,0 +1,67 @@ +//=== unittests/CodeGen/DemangleTrapReasonInDebugInfo.cpp -----------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "clang/CodeGen/ModuleBuilder.h" +#include "llvm/ADT/StringRef.h" +#include "gtest/gtest.h" + +using namespace clang::CodeGen; + +void CheckValidCommon(llvm::StringRef FuncName, const char *ExpectedCategory, + const char *ExpectedMessage) { + auto MaybeTrapReason = DemangleTrapReasonInDebugInfo(FuncName); + ASSERT_TRUE(MaybeTrapReason.has_value()); + auto [Category, Message] = MaybeTrapReason.value(); + ASSERT_STREQ(Category.str().c_str(), ExpectedCategory); + ASSERT_STREQ(Message.str().c_str(), ExpectedMessage); +} + +void CheckInvalidCommon(llvm::StringRef FuncName) { + auto MaybeTrapReason = DemangleTrapReasonInDebugInfo(FuncName); + ASSERT_TRUE(!MaybeTrapReason.has_value()); +} + +TEST(DemangleTrapReasonInDebugInfo, Valid) { + std::string FuncName(ClangTrapPrefix); + FuncName += "$trap category$trap message"; + CheckValidCommon(FuncName, "trap category", "trap message"); +} + +TEST(DemangleTrapReasonInDebugInfo, ValidEmptyCategory) { + std::string FuncName(ClangTrapPrefix); + FuncName += "$$trap message"; + CheckValidCommon(FuncName, "", "trap message"); +} + +TEST(DemangleTrapReasonInDebugInfo, ValidEmptyMessage) { + std::string FuncName(ClangTrapPrefix); + FuncName += "$trap category$"; + CheckValidCommon(FuncName, "trap category", ""); +} + +TEST(DemangleTrapReasonInDebugInfo, ValidAllEmpty) { + // `__builtin_verbose_trap` actually allows this + // currently. However, we should probably disallow this in Sema because having + // an empty category and message completely defeats the point of using the + // builtin (#165981). + std::string FuncName(ClangTrapPrefix); + FuncName += "$$"; + CheckValidCommon(FuncName, "", ""); +} + +TEST(DemangleTrapReasonInDebugInfo, InvalidOnlyPrefix) { + std::string FuncName(ClangTrapPrefix); + CheckInvalidCommon(FuncName); +} + +TEST(DemangleTrapReasonInDebugInfo, Invalid) { + std::string FuncName("foo"); + CheckInvalidCommon(FuncName); +} + +TEST(DemangleTrapReasonInDebugInfo, InvalidEmpty) { CheckInvalidCommon(""); } diff --git a/lldb/source/Target/CMakeLists.txt b/lldb/source/Target/CMakeLists.txt index b7788e80eecac..ba8f11878899e 100644 --- a/lldb/source/Target/CMakeLists.txt +++ b/lldb/source/Target/CMakeLists.txt @@ -97,6 +97,9 @@ add_lldb_library(lldbTarget lldbUtility lldbValueObject lldbPluginProcessUtility + + CLANG_LIBS + clangCodeGen ) add_dependencies(lldbTarget diff --git a/lldb/source/Target/VerboseTrapFrameRecognizer.cpp b/lldb/source/Target/VerboseTrapFrameRecognizer.cpp index 03ab58b8c59a9..be3769191dc9f 100644 --- a/lldb/source/Target/VerboseTrapFrameRecognizer.cpp +++ b/lldb/source/Target/VerboseTrapFrameRecognizer.cpp @@ -95,33 +95,14 @@ VerboseTrapFrameRecognizer::RecognizeFrame(lldb::StackFrameSP frame_sp) { if (func_name.empty()) return {}; - static auto trap_regex = - llvm::Regex(llvm::formatv("^{0}\\$(.*)\\$(.*)$", ClangTrapPrefix).str()); - SmallVector<llvm::StringRef, 3> matches; - std::string regex_err_msg; - if (!trap_regex.match(func_name, &matches, ®ex_err_msg)) { - LLDB_LOGF(GetLog(LLDBLog::Unwind), - "Failed to parse match trap regex for '%s': %s", func_name.data(), - regex_err_msg.c_str()); - - return {}; - } - - // For `__clang_trap_msg$category$message$` we expect 3 matches: - // 1. entire string - // 2. category - // 3. message - if (matches.size() != 3) { - LLDB_LOGF(GetLog(LLDBLog::Unwind), - "Unexpected function name format. Expected '<trap prefix>$<trap " - "category>$<trap message>'$ but got: '%s'.", - func_name.data()); - + auto MaybeTrapReason = + clang::CodeGen::DemangleTrapReasonInDebugInfo(func_name); + if (!MaybeTrapReason.has_value()) { + LLDB_LOGF(GetLog(LLDBLog::Unwind), "Failed to demangle '%s' as trap reason", + func_name.str().c_str()); return {}; } - - auto category = matches[1]; - auto message = matches[2]; + auto [category, message] = MaybeTrapReason.value(); std::string stop_reason = category.empty() ? "<empty category>" : category.str(); _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
