[LLDB][GPU] Decouple plugins and scaffold documentation #11

walter-erquinigo · 2025-07-01T21:12:22Z

Add LLDB_ENABLE_[GPU plugin]_PLUGIN variables to programmatically enable each GPU plugin
Make all the activation logic for all those plugins independent. This results in no else conditions, which makes rebases simpler.
Theoretically, this would allow multiple plugins to be running at the same time. This sounds nice even though I don't think we'll ever work on it.
Scaffold a tiny documentation file where we can put all the information related to the GPU plugins.

- Add LLDB_ENABLE_<GPU plugin>_PLUGIN variables to programmatically enable each GPU plugin - Make all the activation logic for all those plugins independent. This results in no `else` conditions, which makes rebases simpler. - Theoretically, this would allow multiple plugins to be running at the same time. This sounds nice even though I don't think we'll ever work on it. - Scaffold a tiny documentation file where we can put all the information related to the GPU plugins.

dmpots

LGTM

clayborg · 2025-07-01T21:36:43Z

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h

+#include "LLDBServerPlugin.h"

 #include "GDBRemoteCommunicationServerCommon.h"

 class StringExtractorGDBRemote;

 namespace lldb_private {
-
-namespace lldb_server {
-  class LLDBServerPlugin;
-}
-


Why is the #include needed here? If we don't use and instance of the LLDBServerPlugin in this file, then we shouldn't include the header file in here and we should stick with the forward declaration

We need it because otherwise we get a compilation error. At some point the destructor of LLDBServerPlugin needs to be resolved for having a proper definition of std::unique_ptr and it's not available.

I'll try to move it to a cpp file

Notice that this was hit when I tried to build lldb without any plugin. When you have at least one plugin, you end up including that header transitively.

@clayborg , I just pushed the fix

clayborg · 2025-07-02T16:33:55Z

lldb/tools/lldb-server/lldb-gdbserver.cpp

 #if defined(LLDB_ENABLE_AMDGPU_PLUGIN)
 #include "Plugins/AMDGPU/LLDBServerPluginAMDGPU.h"
-typedef lldb_private::lldb_server::LLDBServerPluginAMDGPU LLDBServerGPUPlugin;
-#elif defined(LLDB_ENABLE_MOCKGPU_PLUGIN)
+#endif  
+
+#if defined(LLDB_ENABLE_MOCKGPU_PLUGIN)
 #include "Plugins/MockGPU/LLDBServerPluginMockGPU.h"
-typedef lldb_private::lldb_server::LLDBServerPluginMockGPU LLDBServerGPUPlugin;
 #endif


What happens if both plug-ins are enabled? It seems this would redefine the LLDBServerGPUPlugin typedef and case a build error:

(lldb) expr Enter expressions, then terminate with an empty line to evaluate: 1: typedef int Foo; 2: typedef float Foo; error: <user expression 0>:2:15: typedef redefinition with different types ('float' vs 'int') 2 | typedef float Foo; | ^ note: <user expression 0>:1:13: previous definition is here 1 | typedef int Foo; | ^

Greg, there are no more typedefs!

walter-erquinigo · 2025-07-02T18:03:30Z

Add a CMake check that ensure only one plugin is enabled

A recent change adding a new sanitizer kind (via Sanitizers.def) was reverted in c74fa20 ("Revert "[Clang][CodeGen] Introduce the AllocToken SanitizerKind" (llvm#162413)"). The reason was this ASan report, when running the test cases in clang/test/Preprocessor/print-header-json.c: ``` ==clang==483265==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7d82b97e8b58 at pc 0x562cd432231f bp 0x7fff3fad0850 sp 0x7fff3fad0848 READ of size 16 at 0x7d82b97e8b58 thread T0 #0 0x562cd432231e in __copy_non_overlapping_range<const unsigned long *, const unsigned long *> zorg-test/libcxx_install_asan_ubsan/include/c++/v1/string:2144:38 clayborg#1 0x562cd432231e in void std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>::__init_with_size[abi:nn220000]<unsigned long const*, unsigned long const*>(unsigned long const*, unsigned long const*, unsigned long) zorg-test/libcxx_install_asan_ubsan/include/c++/v1/string:2685:18 clayborg#2 0x562cd41e2797 in __init<const unsigned long *, 0> zorg-test/libcxx_install_asan_ubsan/include/c++/v1/string:2673:3 clayborg#3 0x562cd41e2797 in basic_string<const unsigned long *, 0> zorg-test/libcxx_install_asan_ubsan/include/c++/v1/string:1174:5 clayborg#4 0x562cd41e2797 in clang::ASTReader::ReadString(llvm::SmallVectorImpl<unsigned long> const&, unsigned int&) clang/lib/Serialization/ASTReader.cpp:10171:15 clayborg#5 0x562cd41fd89a in clang::ASTReader::ParseLanguageOptions(llvm::SmallVector<unsigned long, 64u> const&, llvm::StringRef, bool, clang::ASTReaderListener&, bool) clang/lib/Serialization/ASTReader.cpp:6475:28 clayborg#6 0x562cd41eea53 in clang::ASTReader::ReadOptionsBlock(llvm::BitstreamCursor&, llvm::StringRef, unsigned int, bool, clang::ASTReaderListener&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>&) clang/lib/Serialization/ASTReader.cpp:3069:11 clayborg#7 0x562cd4204ab8 in clang::ASTReader::ReadControlBlock(clang::serialization::ModuleFile&, llvm::SmallVectorImpl<clang::ASTReader::ImportedModule>&, clang::serialization::ModuleFile const*, unsigned int) clang/lib/Serialization/ASTReader.cpp:3249:15 clayborg#8 0x562cd42097d2 in clang::ASTReader::ReadASTCore(llvm::StringRef, clang::serialization::ModuleKind, clang::SourceLocation, clang::serialization::ModuleFile*, llvm::SmallVectorImpl<clang::ASTReader::ImportedModule>&, long, long, clang::ASTFileSignature, unsigned int) clang/lib/Serialization/ASTReader.cpp:5182:15 clayborg#9 0x562cd421ec77 in clang::ASTReader::ReadAST(llvm::StringRef, clang::serialization::ModuleKind, clang::SourceLocation, unsigned int, clang::serialization::ModuleFile**) clang/lib/Serialization/ASTReader.cpp:4828:11 clayborg#10 0x562cd3d07b74 in clang::CompilerInstance::findOrCompileModuleAndReadAST(llvm::StringRef, clang::SourceLocation, clang::SourceLocation, bool) clang/lib/Frontend/CompilerInstance.cpp:1805:27 clayborg#11 0x562cd3d0b2ef in clang::CompilerInstance::loadModule(clang::SourceLocation, llvm::ArrayRef<clang::IdentifierLoc>, clang::Module::NameVisibilityKind, bool) clang/lib/Frontend/CompilerInstance.cpp:1956:31 clayborg#12 0x562cdb04eb1c in clang::Preprocessor::HandleHeaderIncludeOrImport(clang::SourceLocation, clang::Token&, clang::Token&, clang::SourceLocation, clang::detail::SearchDirIteratorImpl<true>, clang::FileEntry const*) clang/lib/Lex/PPDirectives.cpp:2423:49 clayborg#13 0x562cdb042222 in clang::Preprocessor::HandleIncludeDirective(clang::SourceLocation, clang::Token&, clang::detail::SearchDirIteratorImpl<true>, clang::FileEntry const*) clang/lib/Lex/PPDirectives.cpp:2101:17 clayborg#14 0x562cdb043366 in clang::Preprocessor::HandleDirective(clang::Token&) clang/lib/Lex/PPDirectives.cpp:1338:14 clayborg#15 0x562cdafa84bc in clang::Lexer::LexTokenInternal(clang::Token&, bool) clang/lib/Lex/Lexer.cpp:4512:7 clayborg#16 0x562cdaf9f20b in clang::Lexer::Lex(clang::Token&) clang/lib/Lex/Lexer.cpp:3729:24 clayborg#17 0x562cdb0d4ffa in clang::Preprocessor::Lex(clang::Token&) clang/lib/Lex/Preprocessor.cpp:896:11 clayborg#18 0x562cd77da950 in clang::ParseAST(clang::Sema&, bool, bool) clang/lib/Parse/ParseAST.cpp:163:7 [...] 0x7d82b97e8b58 is located 0 bytes after 3288-byte region [0x7d82b97e7e80,0x7d82b97e8b58) allocated by thread T0 here: #0 0x562cca76f604 in malloc zorg-test/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:67:3 clayborg#1 0x562cd1cce452 in safe_malloc llvm/include/llvm/Support/MemAlloc.h:26:18 clayborg#2 0x562cd1cce452 in llvm::SmallVectorBase<unsigned int>::grow_pod(void*, unsigned long, unsigned long) llvm/lib/Support/SmallVector.cpp:151:15 clayborg#3 0x562cdbe1768b in grow_pod llvm/include/llvm/ADT/SmallVector.h:139:11 clayborg#4 0x562cdbe1768b in grow llvm/include/llvm/ADT/SmallVector.h:525:41 clayborg#5 0x562cdbe1768b in reserve llvm/include/llvm/ADT/SmallVector.h:665:13 clayborg#6 0x562cdbe1768b in llvm::BitstreamCursor::readRecord(unsigned int, llvm::SmallVectorImpl<unsigned long>&, llvm::StringRef*) llvm/lib/Bitstream/Reader/BitstreamReader.cpp:230:10 clayborg#7 0x562cd41ee8ab in clang::ASTReader::ReadOptionsBlock(llvm::BitstreamCursor&, llvm::StringRef, unsigned int, bool, clang::ASTReaderListener&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>&) clang/lib/Serialization/ASTReader.cpp:3060:49 clayborg#8 0x562cd4204ab8 in clang::ASTReader::ReadControlBlock(clang::serialization::ModuleFile&, llvm::SmallVectorImpl<clang::ASTReader::ImportedModule>&, clang::serialization::ModuleFile const*, unsigned int) clang/lib/Serialization/ASTReader.cpp:3249:15 clayborg#9 0x562cd42097d2 in clang::ASTReader::ReadASTCore(llvm::StringRef, clang::serialization::ModuleKind, clang::SourceLocation, clang::serialization::ModuleFile*, llvm::SmallVectorImpl<clang::ASTReader::ImportedModule>&, long, long, clang::ASTFileSignature, unsigned int) clang/lib/Serialization/ASTReader.cpp:5182:15 clayborg#10 0x562cd421ec77 in clang::ASTReader::ReadAST(llvm::StringRef, clang::serialization::ModuleKind, clang::SourceLocation, unsigned int, clang::serialization::ModuleFile**) clang/lib/Serialization/ASTReader.cpp:4828:11 clayborg#11 0x562cd3d07b74 in clang::CompilerInstance::findOrCompileModuleAndReadAST(llvm::StringRef, clang::SourceLocation, clang::SourceLocation, bool) clang/lib/Frontend/CompilerInstance.cpp:1805:27 clayborg#12 0x562cd3d0b2ef in clang::CompilerInstance::loadModule(clang::SourceLocation, llvm::ArrayRef<clang::IdentifierLoc>, clang::Module::NameVisibilityKind, bool) clang/lib/Frontend/CompilerInstance.cpp:1956:31 clayborg#13 0x562cdb04eb1c in clang::Preprocessor::HandleHeaderIncludeOrImport(clang::SourceLocation, clang::Token&, clang::Token&, clang::SourceLocation, clang::detail::SearchDirIteratorImpl<true>, clang::FileEntry const*) clang/lib/Lex/PPDirectives.cpp:2423:49 clayborg#14 0x562cdb042222 in clang::Preprocessor::HandleIncludeDirective(clang::SourceLocation, clang::Token&, clang::detail::SearchDirIteratorImpl<true>, clang::FileEntry const*) clang/lib/Lex/PPDirectives.cpp:2101:17 clayborg#15 0x562cdb043366 in clang::Preprocessor::HandleDirective(clang::Token&) clang/lib/Lex/PPDirectives.cpp:1338:14 clayborg#16 0x562cdafa84bc in clang::Lexer::LexTokenInternal(clang::Token&, bool) clang/lib/Lex/Lexer.cpp:4512:7 clayborg#17 0x562cdaf9f20b in clang::Lexer::Lex(clang::Token&) clang/lib/Lex/Lexer.cpp:3729:24 clayborg#18 0x562cdb0d4ffa in clang::Preprocessor::Lex(clang::Token&) clang/lib/Lex/Preprocessor.cpp:896:11 clayborg#19 0x562cd77da950 in clang::ParseAST(clang::Sema&, bool, bool) clang/lib/Parse/ParseAST.cpp:163:7 [...] SUMMARY: AddressSanitizer: heap-buffer-overflow clang/lib/Serialization/ASTReader.cpp:10171:15 in clang::ASTReader::ReadString(llvm::SmallVectorImpl<unsigned long> const&, unsigned int&) ``` The reason is this particular RUN line: ``` // RUN: env CC_PRINT_HEADERS_FORMAT=json CC_PRINT_HEADERS_FILTERING=direct-per-file CC_PRINT_HEADERS_FILE=%t.txt %clang -fsyntax-only -I %S/Inputs/print-header-json -isystem %S/Inputs/print-header-json/system -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -o /dev/null ``` which was added in 8df194f ("[Clang] Support includes translated to module imports in -header-include-filtering=direct-per-file (llvm#156756)"). The problem is caused by an incremental build reusing stale cached module files (.pcm) that are no longer binary-compatible with the updated compiler. Adding a new sanitizer option altered the implicit binary layout of the serialized LangOptions data structure. The build + test system is oblivious to such changes. When the new compiler attempted to read the old module file (from the previous test invocation), it misinterpreted the data due to the layout mismatch, resulting in a heap-buffer-overflow. Unfortunately Clang's PCM format does not encode nor detect version mismatches here; a more graceful failure mode would be preferable. For now, fix the test to be more robust with incremental build + test.

walter-erquinigo requested review from agontarek, clayborg and dmpots July 1, 2025 21:12

dmpots approved these changes Jul 1, 2025

View reviewed changes

clayborg reviewed Jul 1, 2025

View reviewed changes

fix header

6abde2a

clayborg reviewed Jul 2, 2025

View reviewed changes

ensure only one plugin is enabled at a time

aa18e8f

clayborg approved these changes Jul 7, 2025

View reviewed changes

walter-erquinigo merged commit e509034 into llvm-server-plugins Jul 7, 2025
6 checks passed

walter-erquinigo deleted the clayborg/walter/cmake branch July 7, 2025 20:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[LLDB][GPU] Decouple plugins and scaffold documentation #11

[LLDB][GPU] Decouple plugins and scaffold documentation #11

Uh oh!

walter-erquinigo commented Jul 1, 2025 •

edited

Loading

Uh oh!

dmpots left a comment

Uh oh!

clayborg Jul 1, 2025

Uh oh!

walter-erquinigo Jul 1, 2025 •

edited

Loading

Uh oh!

walter-erquinigo Jul 1, 2025

Uh oh!

walter-erquinigo Jul 1, 2025

Uh oh!

walter-erquinigo Jul 1, 2025

Uh oh!

clayborg Jul 2, 2025

Uh oh!

walter-erquinigo Jul 2, 2025

Uh oh!

walter-erquinigo commented Jul 2, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[LLDB][GPU] Decouple plugins and scaffold documentation #11

[LLDB][GPU] Decouple plugins and scaffold documentation #11

Uh oh!

Conversation

walter-erquinigo commented Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dmpots left a comment

Choose a reason for hiding this comment

Uh oh!

clayborg Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

walter-erquinigo Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

walter-erquinigo Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

walter-erquinigo Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

walter-erquinigo Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

clayborg Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

walter-erquinigo Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

walter-erquinigo commented Jul 2, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

walter-erquinigo commented Jul 1, 2025 •

edited

Loading

walter-erquinigo Jul 1, 2025 •

edited

Loading