[Lldb-commits] [PATCH] D158205: [lldb] Fix performance regression after adding GNUstep ObjC runtime
theraven added inline comments. Comment at: lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp:49 + if (TT.isOSBinFormatELF()) +return filename.starts_with("libobjc.so"); + if (TT.isOSWindows()) This is a bit unfortunate. I know some downstream users that link the Objective-C runtime components into another .so, so we can't really rely on the name. It would be nice if there were some mechanism for the user to specify the name of the runtime if they're using something non-standard. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158205/new/ https://reviews.llvm.org/D158205 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D119044: Add a case for Rust in LLDB's PDB reader
This revision was automatically updated to reflect the committed changes. Closed by commit rG1ccfef14e57e: Add a case for Rust in LLDB's PDB reader (authored by arlosi, committed by theraven). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119044/new/ https://reviews.llvm.org/D119044 Files: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp === --- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp +++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp @@ -74,6 +74,8 @@ return lldb::LanguageType::eLanguageTypeC; case PDB_Lang::Swift: return lldb::LanguageType::eLanguageTypeSwift; + case PDB_Lang::Rust: +return lldb::LanguageType::eLanguageTypeRust; default: return lldb::LanguageType::eLanguageTypeUnknown; } Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp === --- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp +++ lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp @@ -80,6 +80,8 @@ return lldb::LanguageType::eLanguageTypeC; case PDB_Lang::Swift: return lldb::LanguageType::eLanguageTypeSwift; + case PDB_Lang::Rust: +return lldb::LanguageType::eLanguageTypeRust; default: return lldb::LanguageType::eLanguageTypeUnknown; } Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp === --- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp +++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp @@ -74,6 +74,8 @@ return lldb::LanguageType::eLanguageTypeC; case PDB_Lang::Swift: return lldb::LanguageType::eLanguageTypeSwift; + case PDB_Lang::Rust: +return lldb::LanguageType::eLanguageTypeRust; default: return lldb::LanguageType::eLanguageTypeUnknown; } Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp === --- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp +++ lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp @@ -80,6 +80,8 @@ return lldb::LanguageType::eLanguageTypeC; case PDB_Lang::Swift: return lldb::LanguageType::eLanguageTypeSwift; + case PDB_Lang::Rust: +return lldb::LanguageType::eLanguageTypeRust; default: return lldb::LanguageType::eLanguageTypeUnknown; } ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D146058: [lldb][gnustep] Add basic test and infrastructure for GNUstep ObjC runtime
theraven added inline comments. Comment at: lldb/test/CMakeLists.txt:39 +if (LLDB_TEST_OBJC_GNUSTEP_DIR) + message(STATUS "Found GNUstep ObjC runtime: ${LLDB_TEST_OBJC_GNUSTEP_DIR}/${gnustep_info}") +endif() DavidSpickett wrote: > sgraenitz wrote: > > Might be worth a `FindGNUstep.cmake` at some point? (Or is there one > > somewhere?) > > > > Windows default install dir: > > ``` > > -- Found GNUstep ObjC runtime: C:/Program Files (x86)/libobjc/lib/objc.dll > > ``` > > > > Linux default install dir: > > ``` > > -- Found GNUstep ObjC runtime: /usr/local/lib/libobjc.so > > ``` > I think we generally use UPPERCASE names for cmake variables. This is somewhat complicated. If `gnustep-config` exists, the runtime supports installing itself into a location defined by the gnustep-make install, which supports NeXT, Apple, and FHS-style layouts. It's probably not worth trying to autodetect the general case. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146058/new/ https://reviews.llvm.org/D146058 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D146154: [lldb][gnustep] Add minimal GNUstepObjCRuntime plugin for LanguageTypeObjC on non-Apple platforms
theraven added inline comments. Comment at: lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp:42 +LanguageType language) { + if (language != eLanguageTypeObjC) +return nullptr; I'm not familiar with lldb internals, will this exclude ObjC++? Comment at: lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp:47 + const llvm::Triple &TT = process->GetTarget().GetArchitecture().GetTriple(); + if (TT.getVendor() == llvm::Triple::VendorType::Apple) +return nullptr; I never finished the v2 ABI support for Mach-O, but v1 works on Apple platforms and v2 will eventually. We definitely shouldn't be the default here, but it would be nice not to prevent it working on macOS. Comment at: lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp:131 +ExecutionContext &exe_ctx) { + // TODO: This function is supposed to check whether an ObjC selector is + // present for an object. Might be implemented similar as in the Apple V2 I'm not sure how this is meant to work. I'd expect this to call one of the runtime's introspection functions. I believe the Apple version has variants of these that run without acquiring locks, which can be used in the debugger. We can add these quite easily if necessary. Comment at: lldb/test/Shell/Expr/objc-gnustep-print.m:68 +// +// MEMBERS_OUTSIDE: (lldb) p t->_int +// MEMBERS_OUTSIDE: (int) $0 = 0 It's not clear from this test if it's correctly computing the offsets. I suspect that it isn't, since I don't see any reference to the ivar offset variables. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146154/new/ https://reviews.llvm.org/D146154 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D146154: [lldb][gnustep] Add minimal GNUstepObjCRuntime plugin for LanguageTypeObjC on non-Apple platforms
theraven added inline comments. Comment at: lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp:131 +ExecutionContext &exe_ctx) { + // TODO: This function is supposed to check whether an ObjC selector is + // present for an object. Might be implemented similar as in the Apple V2 sgraenitz wrote: > theraven wrote: > > I'm not sure how this is meant to work. I'd expect this to call one of the > > runtime's introspection functions. I believe the Apple version has > > variants of these that run without acquiring locks, which can be used in > > the debugger. We can add these quite easily if necessary. > The Apple V2 runtime has two ways to do it: > https://github.com/llvm/llvm-project/blob/release/16.x/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp#L1179 > I experimented with the one that doesn't require `void > *gdb_class_getClass(void*)` from the runtime library, but the JIT kept on > crashing. So, for this first version I opted for a minimal implementation > that doesn't do any selector checks. Let's look at it in another review. Can you open bugs against the runtime for the gdb_* symbols that we need? I will add them. The other method will crash on small objects because they don't have an isa pointer, their isa is in the low 1-3 bits of the object pointer, indexing into a table in the runtime. Comment at: lldb/test/Shell/Expr/objc-gnustep-print.m:68 +// +// MEMBERS_OUTSIDE: (lldb) p t->_int +// MEMBERS_OUTSIDE: (int) $0 = 0 sgraenitz wrote: > theraven wrote: > > It's not clear from this test if it's correctly computing the offsets. I > > suspect that it isn't, since I don't see any reference to the ivar offset > > variables. > What exactly would you expect to see in the output? If the offsets are all wrong, this will still work if it happens to land within zeroed memory, which is quite likely. It would be good to have a method that set all of the ivars to something and then check for those values explicitly. Or check for `(char*)t->{ivar} - (char*)t` and make sure that this is the right value (this will change between 32/64/128-bit platforms). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146154/new/ https://reviews.llvm.org/D146154 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D146154: [lldb][gnustep] Add minimal GNUstepObjCRuntime plugin for LanguageTypeObjC on non-Apple platforms
theraven added a comment. The test looks fine to me, I'm quite surprised that it passes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146154/new/ https://reviews.llvm.org/D146154 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D90757: [lldb] Enable FreeBSDRemote plugin by default and update test status
theraven added a comment. Does the new plugin work with processes that are created with `pdfork`? The last time I tried this, it caused the old plugin to lock up the debugger entirely. Please can you ensure that there are tests that cover `pdfork` and `cap_enter` in the child? These are currently quite badly broken. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90757/new/ https://reviews.llvm.org/D90757 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits