[Lldb-commits] [PATCH] D158205: [lldb] Fix performance regression after adding GNUstep ObjC runtime

2023-08-17 Thread David Chisnall via Phabricator via lldb-commits
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

2022-02-16 Thread David Chisnall via Phabricator via lldb-commits
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

2023-03-14 Thread David Chisnall via Phabricator via lldb-commits
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

2023-03-15 Thread David Chisnall via Phabricator via lldb-commits
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

2023-03-16 Thread David Chisnall via Phabricator via lldb-commits
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

2023-03-16 Thread David Chisnall via Phabricator via lldb-commits
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

2020-11-06 Thread David Chisnall via Phabricator via lldb-commits
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