date:20230327

[Lldb-commits] [PATCH] D146668: [lldb-server] Use Platform plugin corresponding to the host

2023-03-27 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

I'm sorry, but I don't think this is a good change (which I guess means the 
previous one is not good either, but this one drives the point). I really don't 
want lldb-server to depend on a platform plugins . I have two reasons for that:

- lldb-server is always running on the "host" platform, so the abstraction is 
largely useless
- the platform plugins end up (transitively) depending on the whole world -- 
which includes tons of functionality that lldb-server does not need. With these 
changes the size of the lldb-server binary has increased by over 50% in my 
build (release+asserts on linux) -- from 40MB (which is already quite big for 
what lldb-server does) to 62MB (which is just ludicrous).

Of these, the second reason is the main one. If the platform plugin had a 
simple self-contained API, then using it in lldb-server would not be a problem. 
It might even be nice. However, we're very far from that state, and I don't 
think it makes sense to try to achieve it. We use it as a repository for 
os-specific logic in lldb, but here "lldb" really means the lldb client. 
lldb-server doesn't need 90% of the Platform functionality, and most of the 
other 10% (e.g. the file API) can be easily replaced by using the host 
equivalents directly. Before this patch set, unix signals belonged in those 
other 10 percent.

I think we need to revert these patches and figure out a different way to solve 
the issue at hand. The 50% increase is just too high. To be clear, I totally 
understand and support what you're trying to achieve. Having no outgoing plugin 
dependencies from the lldb core is a worthwhile endeavour, and in fact could 
help shrink lldb-server size as well (because it wants to depend on some -- and 
only some -- parts of lldb core). However, that won't be the case if we solve 
that by reattaching all of the dependencies to lldb-server.

From the lldb client perspective, there's nothing wrong with having a 
Platform::GetUnixSignals api. It fits in very well into the existing 
functionality. However, since this is also used by lldb-server, we should also 
have a non-platform way to access that data. One way to solve that would be to 
have some sort of a "platform lite" plugin, which would contain the signal API, 
and any other functionality that is useful for both lldb and lldb-server -- and 
the real platform plugin could subsume that.

However, I find that a bit overkill. The unix signals classes are very simple 
(and very repetitive), so it seems to me like the plugin machinery would just 
add a bunch of boilerplate (and institutionalize the repetition). I could be 
convinced otherwise (particularly, if we find some other platform-like 
functionality that could be placed into this plugin), but right now, I think it 
would be better to just de-pluginize the entire UnixSignals machinery. For 
example, we could just move it down to Utility so it can be accessed from 
anywhere. Later, with some refactoring, I think we could even merge the various 
***Signals files into one, since all they should really differ is in the values 
of signal constants. Like, we probably would not want to have a different 
signal disposition settings for SIGINT on linux than say on FreeBSD, so the 
fact that these are repeated in every implementation is just a potential source 
of bugs.

And we can still have Platform::GetUnixSignals to wrap this, and call it from 
whereever it makes sense.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146668/new/

https://reviews.llvm.org/D146668

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D144240: Clear read_fd_set if EINTR received

2023-03-27 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

Yes, that is the correct process.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144240/new/

https://reviews.llvm.org/D144240

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D146590: [lldb] Update some uses of Python2 API in typemaps.

2023-03-27 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

In D146590#4220388 , @jgorbe wrote:

> I believe (but I don't have any experience with SWIG typemaps so this is an 
> educated guess) that the call to `free` in the error path comes from the 
> `%typemap(freearg)` immediately after that one. So if freearg already takes 
> care of it, the error handling logic in `%typemap(in)` should just call 
> `SWIG_fail`. Does that sound correct?

Makes sense to me.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146590/new/

https://reviews.llvm.org/D146590

___
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-27 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

In D146058#4193594 , @sgraenitz wrote:

>> Pavel runs the x86 debian bot and I think the x86 Windows bot may have been 
>> retired, or at least is offline at the moment 
>> (https://lab.llvm.org/buildbot/#/builders/83)
>
> Yes, that would be cool. @labath What do you think?

What kind of setup is necessary to make that work? If it's not too complicated, 
I think we could make something work.

Speaking generally, my only concern with that is the that the bot doesn't 
remain broken (or even worse -- flaky!) for long stretches of time. I don't 
have time to debug objc failures, and reverting other people's changes because 
they break something that might be an "experimental" feature for quite some 
time could be problematic.

In D146058#4194112 , @sgraenitz wrote:

> My impression was that API tests are LLDB legacy and new tests would usually 
> be Shell. It might still be worth getting (some of) the API tests to work 
> with GNUstep as well, but I might need some advice on how to do that.

The situation is definitely more complicated than that. If you're serious about 
objc support, I'd definitely recommend getting the existing tests working, as 
that's the best way to validate correctness and consistency (with the macos 
runtime). If you're adding some new tests, then they can be shell tests, 
particularly if they're gnustep-specific.

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] D145803: [clang][DebugInfo] Emit DW_AT_type of preferred name if available

2023-03-27 Thread Michael Buch via Phabricator via lldb-commits

Michael137 updated this revision to Diff 508562.
Michael137 added a comment.

- Refine tests:
  - Check for type of template parameter
  - Add test for chained typedefs.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145803/new/

https://reviews.llvm.org/D145803

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/test/CodeGen/preferred_name-chain.cpp
  clang/test/CodeGen/preferred_name.cpp
  clang/test/Modules/Inputs/gmodules-preferred-name-alias.h
  clang/test/Modules/Inputs/gmodules-preferred-name-alias.modulemap
  clang/test/Modules/Inputs/gmodules-preferred-name-typedef.h
  clang/test/Modules/Inputs/gmodules-preferred-name-typedef.modulemap
  clang/test/Modules/gmodules-preferred-name-alias.cpp
  clang/test/Modules/gmodules-preferred-name-typedef.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/TestDataFormatterLibcxxUniquePtr.py

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/TestDataFormatterLibcxxUniquePtr.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/TestDataFormatterLibcxxUniquePtr.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/TestDataFormatterLibcxxUniquePtr.py
@@ -22,7 +22,7 @@
 
 def make_expected_basic_string_ptr(self) -> str:
 if self.expectedCompiler(["clang"]) and self.expectedCompilerVersion(['>', '16.0']):
-return f'std::unique_ptr >'
+return f'std::unique_ptr'
 else:
 return 'std::unique_ptr, std::allocator >, ' \
'std::default_delete, std::allocator > > >'
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py
@@ -59,13 +59,13 @@
 self.assertNotEqual(valobj.child[0].unsigned, 0)
 
 if self.expectedCompiler(["clang"]) and self.expectedCompilerVersion(['>', '16.0']):
-string_type = "std::basic_string"
+string_type = "std::string"
 else:
-string_type = "std::basic_string, std::allocator >"
+string_type = "std::basic_string, std::allocator > "
 
 valobj = self.expect_var_path(
 "sp_str",
-type="std::shared_ptr<" + string_type + " >",
+type="std::shared_ptr<" + string_type + ">",
 children=[ValueCheck(name="__ptr_", summary='"hello"')],
 )
 self.assertRegex(valobj.summary, r'^"hello"( strong=1)? weak=1$')
Index: clang/test/Modules/gmodules-preferred-name-typedef.cpp
===
--- /dev/null
+++ clang/test/Modules/gmodules-preferred-name-typedef.cpp
@@ -0,0 +1,12 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -std=c++11 -dwarf-ext-refs -fmodule-format=obj \
+// RUN: -fmodule-map-file=%S/Inputs/gmodules-preferred-name-typedef.modulemap \
+// RUN: -fmodules-cache-path=%t -debug-info-kind=standalone -debugger-tuning=lldb \
+// RUN: -fmodules -mllvm -debug-only=pchcontainer -x c++ \
+// RUN: -I %S/Inputs %s &> %t.ll
+// RUN: cat %t.ll | FileCheck %s
+
+#include "gmodules-preferred-name-typedef.h"
+
+// CHECK: ![[#]] = !DIDerivedType(tag: DW_TAG_typedef, name: "Bar", scope: ![[#]], file: ![[#]], line: [[#]], baseType: ![[PREF_BASE:[0-9]+]])
+// CHECK: ![[PREF_BASE]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Foo"
Index: clang/test/Modules/gmodules-preferred-name-alias.cpp
===
--- /dev/null
+++ clang/test/Modules/gmodules-preferred-name-alias.cpp
@@ -0,0 +1,12 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -std=c++11 -dwarf-ext-refs -fmodule-format=obj \
+// RUN: -fmodule-map-file=%S/Inputs/gmodules-preferred-name-alias.modulemap \
+// RUN: -fmodules-cache-path=%t -debug-info-kind=standalone -debugger-tuning=lldb \
+// RUN: -fmodules -mllvm -debug-only=pchcontainer -x c++ \
+// RUN: -I %S/Inputs %s &> %t.ll
+// RUN: cat %t.ll | FileCheck %s
+
+#include "gmodules-preferred-name-alias.h"
+
+// CHECK: ![[#]] = !DIDerivedType(tag: DW_TAG_typedef, name: "Bar", scope: ![[#]], file: ![[#]], line: [[#]], baseType: ![[PREF_BASE:[0-9]+]])
+// CHECK: ![[PREF_BASE]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Foo"
Index: clang/test/Modules/Inputs/gmodules-preferred-name-typedef.modulemap
===

[Lldb-commits] [PATCH] D145803: [clang][DebugInfo] Emit DW_AT_type of preferred name if available

2023-03-27 Thread Michael Buch via Phabricator via lldb-commits

Michael137 updated this revision to Diff 508563.
Michael137 added a comment.

- Fix function docstring


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145803/new/

https://reviews.llvm.org/D145803

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/test/CodeGen/preferred_name-chain.cpp
  clang/test/CodeGen/preferred_name.cpp
  clang/test/Modules/Inputs/gmodules-preferred-name-alias.h
  clang/test/Modules/Inputs/gmodules-preferred-name-alias.modulemap
  clang/test/Modules/Inputs/gmodules-preferred-name-typedef.h
  clang/test/Modules/Inputs/gmodules-preferred-name-typedef.modulemap
  clang/test/Modules/gmodules-preferred-name-alias.cpp
  clang/test/Modules/gmodules-preferred-name-typedef.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/TestDataFormatterLibcxxUniquePtr.py

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/TestDataFormatterLibcxxUniquePtr.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/TestDataFormatterLibcxxUniquePtr.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/TestDataFormatterLibcxxUniquePtr.py
@@ -22,7 +22,7 @@
 
 def make_expected_basic_string_ptr(self) -> str:
 if self.expectedCompiler(["clang"]) and self.expectedCompilerVersion(['>', '16.0']):
-return f'std::unique_ptr >'
+return f'std::unique_ptr'
 else:
 return 'std::unique_ptr, std::allocator >, ' \
'std::default_delete, std::allocator > > >'
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py
@@ -59,13 +59,13 @@
 self.assertNotEqual(valobj.child[0].unsigned, 0)
 
 if self.expectedCompiler(["clang"]) and self.expectedCompilerVersion(['>', '16.0']):
-string_type = "std::basic_string"
+string_type = "std::string"
 else:
-string_type = "std::basic_string, std::allocator >"
+string_type = "std::basic_string, std::allocator > "
 
 valobj = self.expect_var_path(
 "sp_str",
-type="std::shared_ptr<" + string_type + " >",
+type="std::shared_ptr<" + string_type + ">",
 children=[ValueCheck(name="__ptr_", summary='"hello"')],
 )
 self.assertRegex(valobj.summary, r'^"hello"( strong=1)? weak=1$')
Index: clang/test/Modules/gmodules-preferred-name-typedef.cpp
===
--- /dev/null
+++ clang/test/Modules/gmodules-preferred-name-typedef.cpp
@@ -0,0 +1,12 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -std=c++11 -dwarf-ext-refs -fmodule-format=obj \
+// RUN: -fmodule-map-file=%S/Inputs/gmodules-preferred-name-typedef.modulemap \
+// RUN: -fmodules-cache-path=%t -debug-info-kind=standalone -debugger-tuning=lldb \
+// RUN: -fmodules -mllvm -debug-only=pchcontainer -x c++ \
+// RUN: -I %S/Inputs %s &> %t.ll
+// RUN: cat %t.ll | FileCheck %s
+
+#include "gmodules-preferred-name-typedef.h"
+
+// CHECK: ![[#]] = !DIDerivedType(tag: DW_TAG_typedef, name: "Bar", scope: ![[#]], file: ![[#]], line: [[#]], baseType: ![[PREF_BASE:[0-9]+]])
+// CHECK: ![[PREF_BASE]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Foo"
Index: clang/test/Modules/gmodules-preferred-name-alias.cpp
===
--- /dev/null
+++ clang/test/Modules/gmodules-preferred-name-alias.cpp
@@ -0,0 +1,12 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -std=c++11 -dwarf-ext-refs -fmodule-format=obj \
+// RUN: -fmodule-map-file=%S/Inputs/gmodules-preferred-name-alias.modulemap \
+// RUN: -fmodules-cache-path=%t -debug-info-kind=standalone -debugger-tuning=lldb \
+// RUN: -fmodules -mllvm -debug-only=pchcontainer -x c++ \
+// RUN: -I %S/Inputs %s &> %t.ll
+// RUN: cat %t.ll | FileCheck %s
+
+#include "gmodules-preferred-name-alias.h"
+
+// CHECK: ![[#]] = !DIDerivedType(tag: DW_TAG_typedef, name: "Bar", scope: ![[#]], file: ![[#]], line: [[#]], baseType: ![[PREF_BASE:[0-9]+]])
+// CHECK: ![[PREF_BASE]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Foo"
Index: clang/test/Modules/Inputs/gmodules-preferred-name-typedef.modulemap
===
--- /dev/null
+++ clan

[Lldb-commits] [PATCH] D146058: [lldb][gnustep] Add basic test and infrastructure for GNUstep ObjC runtime

2023-03-27 Thread Stefan Gränitz via Phabricator via lldb-commits

sgraenitz added a comment.

In D146058#4223575 , @labath wrote:

> What kind of setup is necessary to make that work? If it's not too 
> complicated, I think we could make something work.

Thanks, that'd be awesome!

The last official release is from 2020, so for the time being it seems best to 
build and install from TOT once -- @theraven please correct me if I am wrong:

  > git clone https://github.com/gnustep/libobjc2
  > cd libobjc2
  > git submodule init && git submodule update
  > CC=clang-15 CXX=clang++-15 cmake -Bbuild -GNinja -DTESTS=On .
  > cd build
  > ninja
  ...
  > ctest
  ...
  100% tests passed, 0 tests failed out of 198
  > ninja install
  ...
  -- Installing: /usr/local/lib/libobjc.so.4.6
  -- Installing: /usr/local/lib/libobjc.so
  ...

Then apply this patch and re-run CMake. The new test should popup and pass:

  > arc patch D146058
  > cd build && cmake .
  ...
  -- Found GNUstep ObjC runtime: /usr/local/lib/libobjc.so
  ...
  > bin/llvm-lit --filter=objc-gnustep -vv tools/lldb/test
  -- Testing: 1 of 1979 tests, 1 workers --
  PASS: lldb-shell :: Expr/objc-gnustep-print.m (1 of 1)
  
  Testing Time: 2.06s
Excluded: 1583
Passed  :1



> Speaking generally, my only concern with that is the that the bot doesn't 
> remain broken (or even worse -- flaky!) for long stretches of time. I don't 
> have time to debug objc failures, and reverting other people's changes 
> because they break something that might be an "experimental" feature for 
> quite some time could be problematic.

Absolutely. What do you think is the best way to prevent that?

Right now, the `gnustep` test would run whenever `libobjc.so` was found. A lit 
config reset like this would make it `UNSUPPORTED` (but it gets wiped with the 
next CMake run):

  > grep objc_gnustep_dir build/tools/lldb/test/Shell/lit.site.cfg.py 
  config.objc_gnustep_dir = ""

How can I change this to support the well-behaved bot? Add 
`-DLLDB_TEST_OBJC_GNUSTEP=Off`? Or should it be off by default and enabled on 
demand?


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] D146965: [lldb] Add support for MSP430 in LLDB.

2023-03-27 Thread Ilia Kuklin via Phabricator via lldb-commits

kuilpd created this revision.
kuilpd added a reviewer: asl.
kuilpd added a project: LLDB.
Herald added subscribers: Michael137, JDevlieghere.
Herald added a project: All.
kuilpd requested review of this revision.
Herald added a subscriber: lldb-commits.

Add MSP430 to the list of available targets, implement MSP430 ABI, add support 
for debugging targets with 16-bit address size.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146965

Files:
  lldb/include/lldb/Utility/ArchSpec.h
  lldb/include/lldb/Utility/DataExtractor.h
  lldb/source/Expression/IRMemoryMap.cpp
  lldb/source/Expression/LLVMUserExpression.cpp
  lldb/source/Host/common/NativeProcessProtocol.cpp
  lldb/source/Plugins/ABI/CMakeLists.txt
  lldb/source/Plugins/ABI/MSP430/ABISysV_msp430.cpp
  lldb/source/Plugins/ABI/MSP430/ABISysV_msp430.h
  lldb/source/Plugins/ABI/MSP430/CMakeLists.txt
  lldb/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
  lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterFallback.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Target/Platform.cpp
  lldb/source/Utility/ArchSpec.cpp

Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -154,6 +154,10 @@
 {eByteOrderLittle, 8, 2, 4, llvm::Triple::mips64el,
  ArchSpec::eCore_mips64r6el, "mips64r6el"},
 
+// MSP430
+{eByteOrderLittle, 2, 2, 4, llvm::Triple::msp430, ArchSpec::eCore_msp430,
+ "msp430"},
+
 {eByteOrderBig, 4, 4, 4, llvm::Triple::ppc, ArchSpec::eCore_ppc_generic,
  "powerpc"},
 {eByteOrderBig, 4, 4, 4, llvm::Triple::ppc, ArchSpec::eCore_ppc_ppc601,
@@ -402,6 +406,8 @@
  ArchSpec::eMIPSSubType_mips64r2el, 0xu, 0xu}, // mips64r2el
 {ArchSpec::eCore_mips64r6el, llvm::ELF::EM_MIPS,
  ArchSpec::eMIPSSubType_mips64r6el, 0xu, 0xu}, // mips64r6el
+{ArchSpec::eCore_msp430, llvm::ELF::EM_MSP430, LLDB_INVALID_CPUTYPE,
+ 0xu, 0xu}, // MSP430
 {ArchSpec::eCore_hexagon_generic, llvm::ELF::EM_HEXAGON,
  LLDB_INVALID_CPUTYPE, 0xu, 0xu}, // HEXAGON
 {ArchSpec::eCore_arc, llvm::ELF::EM_ARC_COMPACT2, LLDB_INVALID_CPUTYPE,
@@ -899,6 +905,9 @@
   case llvm::ELF::ELFOSABI_SOLARIS:
 m_triple.setOS(llvm::Triple::OSType::Solaris);
 break;
+  case llvm::ELF::ELFOSABI_STANDALONE:
+m_triple.setOS(llvm::Triple::OSType::UnknownOS);
+break;
   }
 } else if (arch_type == eArchTypeCOFF && os == llvm::Triple::Win32) {
   m_triple.setVendor(llvm::Triple::PC);
Index: lldb/source/Target/Platform.cpp
===
--- lldb/source/Target/Platform.cpp
+++ lldb/source/Target/Platform.cpp
@@ -1895,6 +1895,12 @@
 trap_opcode_size = sizeof(g_hex_opcode);
   } break;
 
+  case llvm::Triple::msp430: {
+static const uint8_t g_msp430_opcode[] = {0x43, 0x43};
+trap_opcode = g_msp430_opcode;
+trap_opcode_size = sizeof(g_msp430_opcode);
+  } break;
+
   case llvm::Triple::systemz: {
 static const uint8_t g_hex_opcode[] = {0x00, 0x01};
 trap_opcode = g_hex_opcode;
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -940,7 +940,8 @@
 
   bool length_OK = data.ValidOffset(header.GetNextUnitOffset() - 1);
   bool version_OK = SymbolFileDWARF::SupportedVersion(header.m_version);
-  bool addr_size_OK = (header.m_addr_size == 4) || (header.m_addr_size == 8);
+  bool addr_size_OK = (header.m_addr_size == 2) || (header.m_addr_size == 4) ||
+  (header.m_addr_size == 8);
   bool type_offset_OK =
   !header.IsTypeUnit() || (header.m_type_offset <= header.GetLength());
 
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterFallback.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterFallback.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterFallback.cpp
@@ -19,6 +19,7 @@
   }
 #define R64(name) REG(name, 8)
 #define R32(name) REG(name, 4)
+#define R16(name) REG(name, 2)
 
 static std::vector GetRegisters_aarch64() {
   ConstString empty_alt_name;
@@ -35,6 +36,18 @@
   return registers;
 }
 
+static std::vector GetRegisters_msp430() {
+  ConstString empty_alt_name;
+  ConstString reg_set{"general purpose registers"};
+
+  std::vector registers{
+  R16(r0),  R16(r1),  R16(r2),  R16(r3), R16(r4),  R16(r5),
+  R16(r6),  R16(r7),  R16(r8),  R16(r9), R16(r10), R16(r11),
+  R16(r12), R16(r13), R16(r14), R16(r15)};
+
+  return registers;
+}
+
 static std::vector GetRegisters_x86() {
   Cons

[Lldb-commits] [PATCH] D146977: [lldb-server/linux] Use waitpid(-1) to collect inferior events

2023-03-27 Thread Pavel Labath via Phabricator via lldb-commits

labath created this revision.
labath added reviewers: DavidSpickett, yinghuitan.
Herald added a subscriber: emaste.
Herald added a project: All.
labath requested review of this revision.
Herald added a project: LLDB.

This is a follow-up to D116372 , which had a 
rather unfortunate side
effect of making the processing of a single SIGCHLD quadratic in the
number of threads -- which does not matter for simple applications, but
can get really bad for applications with thousands of threads.

This patch fixes the problem by implementing the other possibility
mentioned in the first patch -- doing waitpid(-1) centrally and then
routing the events to the correct process instance. The "uncollected"
threads are held in the process factory class -- which I've renamed to
Manager for this purpose, as it now does more than creating processes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146977

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/source/Host/common/NativeProcessProtocol.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
  lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
  lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/tools/lldb-server/lldb-gdbserver.cpp

Index: lldb/tools/lldb-server/lldb-gdbserver.cpp
===
--- lldb/tools/lldb-server/lldb-gdbserver.cpp
+++ lldb/tools/lldb-server/lldb-gdbserver.cpp
@@ -62,16 +62,16 @@
 
 namespace {
 #if defined(__linux__)
-typedef process_linux::NativeProcessLinux::Factory NativeProcessFactory;
+typedef process_linux::NativeProcessLinux::Manager NativeProcessManager;
 #elif defined(__FreeBSD__)
-typedef process_freebsd::NativeProcessFreeBSD::Factory NativeProcessFactory;
+typedef process_freebsd::NativeProcessFreeBSD::Manager NativeProcessManager;
 #elif defined(__NetBSD__)
-typedef process_netbsd::NativeProcessNetBSD::Factory NativeProcessFactory;
+typedef process_netbsd::NativeProcessNetBSD::Manager NativeProcessManager;
 #elif defined(_WIN32)
-typedef NativeProcessWindows::Factory NativeProcessFactory;
+typedef NativeProcessWindows::Manager NativeProcessManager;
 #else
 // Dummy implementation to make sure the code compiles
-class NativeProcessFactory : public NativeProcessProtocol::Factory {
+class NativeProcessManager : public NativeProcessProtocol::Manager {
 public:
   llvm::Expected>
   Launch(ProcessLaunchInfo &launch_info,
@@ -431,8 +431,8 @@
 return 1;
   }
 
-  NativeProcessFactory factory;
-  GDBRemoteCommunicationServerLLGS gdb_server(mainloop, factory);
+  NativeProcessManager manager(mainloop);
+  GDBRemoteCommunicationServerLLGS gdb_server(mainloop, manager);
 
   llvm::StringRef host_and_port;
   if (!Inputs.empty()) {
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -35,7 +35,7 @@
   // Constructors and Destructors
   GDBRemoteCommunicationServerLLGS(
   MainLoop &mainloop,
-  const NativeProcessProtocol::Factory &process_factory);
+  NativeProcessProtocol::Manager &process_manager);
 
   void SetLaunchInfo(const ProcessLaunchInfo &info);
 
@@ -99,7 +99,7 @@
 protected:
   MainLoop &m_mainloop;
   MainLoop::ReadHandleUP m_network_handle_up;
-  const NativeProcessProtocol::Factory &m_process_factory;
+  NativeProcessProtocol::Manager &m_process_manager;
   lldb::tid_t m_current_tid = LLDB_INVALID_THREAD_ID;
   lldb::tid_t m_continue_tid = LLDB_INVALID_THREAD_ID;
   NativeProcessProtocol *m_current_process;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -69,9 +69,9 @@
 
 // GDBRemoteCommunicationServerLLGS constructor
 GDBRemoteCommunicationServerLLGS::GDBRemoteCommunicationServerLLGS(
-MainLoop &mainloop, const NativeProcessProtocol::Factory &process_factory)
+MainLoop &mainloop, NativeProcessProtocol::Manager &process_manager)
 : GDBRemoteCommunicationServerCommon(), m_mainloop(mainloop),
-  m_process_factory(process_factory), m_current_process(nullptr),
+  m_proc

[Lldb-commits] [PATCH] D146668: [lldb-server] Use Platform plugin corresponding to the host

2023-03-27 Thread Alex Langford via Phabricator via lldb-commits

bulbazord added a comment.

In D146668#4223456 , @labath wrote:

> I'm sorry, but I don't think this is a good change (which I guess means the 
> previous one is not good either, but this one drives the point). I really 
> don't want lldb-server to depend on a platform plugins . I have two reasons 
> for that:
>
> - lldb-server is always running on the "host" platform, so the abstraction is 
> largely useless
> - the platform plugins end up (transitively) depending on the whole world -- 
> which includes tons of functionality that lldb-server does not need. With 
> these changes the size of the lldb-server binary has increased by over 50% in 
> my build (release+asserts on linux) -- from 40MB (which is already quite big 
> for what lldb-server does) to 62MB (which is just ludicrous).
>
> Of these, the second reason is the main one. If the platform plugin had a 
> simple self-contained API, then using it in lldb-server would not be a 
> problem. It might even be nice. However, we're very far from that state, and 
> I don't think it makes sense to try to achieve it. We use it as a repository 
> for os-specific logic in lldb, but here "lldb" really means the lldb client. 
> lldb-server doesn't need 90% of the Platform functionality, and most of the 
> other 10% (e.g. the file API) can be easily replaced by using the host 
> equivalents directly. Before this patch set, unix signals belonged in those 
> other 10 percent.
>
> I think we need to revert these patches and figure out a different way to 
> solve the issue at hand. The 50% increase is just too high. To be clear, I 
> totally understand and support what you're trying to achieve. Having no 
> outgoing plugin dependencies from the lldb core is a worthwhile endeavour, 
> and in fact could help shrink lldb-server size as well (because it wants to 
> depend on some -- and only some -- parts of lldb core). However, that won't 
> be the case if we solve that by reattaching all of the dependencies to 
> lldb-server.
>
> From the lldb client perspective, there's nothing wrong with having a 
> Platform::GetUnixSignals api. It fits in very well into the existing 
> functionality. However, since this is also used by lldb-server, we should 
> also have a non-platform way to access that data. One way to solve that would 
> be to have some sort of a "platform lite" plugin, which would contain the 
> signal API, and any other functionality that is useful for both lldb and 
> lldb-server -- and the real platform plugin could subsume that.
>
> However, I find that a bit overkill. The unix signals classes are very simple 
> (and very repetitive), so it seems to me like the plugin machinery would just 
> add a bunch of boilerplate (and institutionalize the repetition). I could be 
> convinced otherwise (particularly, if we find some other platform-like 
> functionality that could be placed into this plugin), but right now, I think 
> it would be better to just de-pluginize the entire UnixSignals machinery. For 
> example, we could just move it down to Utility so it can be accessed from 
> anywhere. Later, with some refactoring, I think we could even merge the 
> various ***Signals files into one, since all they should really differ is in 
> the values of signal constants. Like, we probably would not want to have a 
> different signal disposition settings for SIGINT on linux than say on 
> FreeBSD, so the fact that these are repeated in every implementation is just 
> a potential source of bugs.
>
> And we can still have Platform::GetUnixSignals to wrap this, and call it from 
> whereever it makes sense.

I'm alright with reverting these patches if it balloons lldb-server's size by 
50%. I really appreciate you trying to work with me to find a path forward 
here. I really do not want to revert without some idea of how we could solve 
this dependency/layering issue.

I like the idea of a "platform lite" plugin, but I also agree that it is 
probably overkill right now. Let's keep that idea in our back pockets. Sometime 
today I will revert this patch (I'll re-commit the test in a follow-up as it is 
still useful) and its predecessor (D146263 ). 
I'll also do some investigation work and write something up on the LLVM 
discourse where we can have a discussion about the best way forward.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146668/new/

https://reviews.llvm.org/D146668

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D146965: [lldb] Add support for MSP430 in LLDB.

2023-03-27 Thread Anton Korobeynikov via Phabricator via lldb-commits

asl added a comment.

@JDevlieghere @bulbazord I don't think I know lldb codebase well enough to 
cover the complete review here. But from MSP430-standpoint things seem to be 
correct. Will you be able to review or suggest some other reviewers?

Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146965/new/

https://reviews.llvm.org/D146965

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D146783: [lldb] Add ability to hide the root name of a value

2023-03-27 Thread Dave Lee via Phabricator via lldb-commits

kastiglione added a comment.

@jgorbe thanks for reporting. I have a fix for the missing space, I will look 
into the cause of the double space and fix that as well. I'll do this today.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146783/new/

https://reviews.llvm.org/D146783

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D145803: [clang][DebugInfo] Emit DW_AT_type of preferred name if available

2023-03-27 Thread David Blaikie via Phabricator via lldb-commits

dblaikie added inline comments.



Comment at: clang/test/CodeGen/preferred_name.cpp:49
+
+Foo> varFooInt;
+

Michael137 wrote:
> probinson wrote:
> > This doesn't become `Foo` ?
> The name stays as `Foo>` but the type of the template parameter 
> becomes `BarInt`. So the dwarf would look like:
> ```
> DW_TAG_structure_type
>   DW_AT_name  ("Foo >")
> 
>   DW_TAG_template_type_parameter
> DW_AT_type(0x0095 "BarInt")
> DW_AT_name("T")
> 
> ```
> Will add the parameter metadata to the test
Hmm, that seems buggy - why doesn't `Foo >` become `Foo`? (is 
the preferred name ever used in the DW_AT_name? I'd have thought it should be 
unless it's explicitly avoided to ensure type compatibility/collapsing with 
debug info built without this feature enabled?)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145803/new/

https://reviews.llvm.org/D145803

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D145803: [clang][DebugInfo] Emit DW_AT_type of preferred name if available

2023-03-27 Thread Michael Buch via Phabricator via lldb-commits

Michael137 marked an inline comment as not done.
Michael137 added inline comments.



Comment at: clang/test/CodeGen/preferred_name.cpp:49
+
+Foo> varFooInt;
+

dblaikie wrote:
> Michael137 wrote:
> > probinson wrote:
> > > This doesn't become `Foo` ?
> > The name stays as `Foo>` but the type of the template parameter 
> > becomes `BarInt`. So the dwarf would look like:
> > ```
> > DW_TAG_structure_type
> >   DW_AT_name  ("Foo >")
> > 
> >   DW_TAG_template_type_parameter
> > DW_AT_type(0x0095 "BarInt")
> > DW_AT_name("T")
> > 
> > ```
> > Will add the parameter metadata to the test
> Hmm, that seems buggy - why doesn't `Foo >` become `Foo`? 
> (is the preferred name ever used in the DW_AT_name? I'd have thought it 
> should be unless it's explicitly avoided to ensure type 
> compatibility/collapsing with debug info built without this feature enabled?)
I suspect it's because the name is constructed using the clang TypePrinter. And 
we turn off `PrintingPolicy::UsePreferredNames` by default to avoid divergence 
from GCC.

Will double check though


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145803/new/

https://reviews.llvm.org/D145803

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D144665: Use Resume not PrivateResume when asynchronously continuing after the start at stop

2023-03-27 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib added inline comments.



Comment at: lldb/test/API/python_api/run_locker/TestRunLocker.py:25
+self.build()
+self.runlocker_test(False)
+

@jingham, you're testing the same thing twice. You probably wanted to set the 
`stop_at_entry` argument to `True` here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144665/new/

https://reviews.llvm.org/D144665

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D146965: [lldb] Add support for MSP430 in LLDB.

2023-03-27 Thread Alex Langford via Phabricator via lldb-commits

bulbazord added a comment.

I'm not too familiar with MSP430 but the general idea looks fine. Others may 
have comments about areas I'm less familiar with.

One concern I have is that there are no tests. I can understand that it may be 
difficult to get automated tests running testing the debugging of an 
architecture like MSP430 but there are things we can test to sanity test MSP430 
support... At the very least, adding a unit test for the new ArchSpec support 
would be good -- `llvm-project/lldb/unittests/Utility/ArchSpecTest.cpp`.




Comment at: lldb/include/lldb/Utility/DataExtractor.h:846-848
 #ifdef LLDB_CONFIGURATION_DEBUG
-assert(addr_size == 4 || addr_size == 8);
+assert(addr_size == 2 || addr_size == 4 || addr_size == 8);
 #endif

I know this is not your code but maybe we should deconditionalize this 
assertion?



Comment at: lldb/source/Expression/IRMemoryMap.cpp:100-101
+uint64_t end_of_memory;
+uint32_t address_byte_size = process_sp->GetAddressByteSize();
+switch (address_byte_size) {
+case 2:

switch on the expression directly instead of creating a local?



Comment at: lldb/source/Expression/IRMemoryMap.cpp:102-110
+case 2:
+  end_of_memory = 0xull;
+  break;
+case 4:
+  end_of_memory = 0xull;
+  break;
+default:

Instead of making 8 the default, maybe we can assert on `default` or something? 
This can prevent future bugs where perhaps address_byte_size is not 8 for some 
reason and we do the wrong thing.



Comment at: lldb/source/Expression/IRMemoryMap.cpp:113-114
 
 lldbassert(process_sp->GetAddressByteSize() == 4 ||
end_of_memory != 0xull);
 

I think with the change above we should probably make this assertion more 
useful.



Comment at: lldb/source/Expression/IRMemoryMap.cpp:160-161
 break;
   default:
+ret = 0xdead0fffull;
 break;

Same here, let's not assume 8 if we hit default.



Comment at: lldb/source/Expression/IRMemoryMap.cpp:169-170
 size_t alloc_size = back->second.m_size;
-ret = llvm::alignTo(addr + alloc_size, 4096);
+auto align = GetAddressByteSize() == 2 ? 512 : 4096;
+ret = llvm::alignTo(addr + alloc_size, align);
   }

Is this true for all machines where the address byte size is 2? Seems like 512 
and 4096 should be based on something other than the address byte size. 
Platform/ABI perhaps?



Comment at: lldb/source/Expression/LLVMUserExpression.cpp:336-338
+  const size_t stack_frame_size =
+  target->GetArchitecture().GetAddressByteSize() == 2 ? 512
+  : 512 * 1024;

Same here, this seems dependent on Platform/ABI.



Comment at: lldb/source/Plugins/ABI/MSP430/ABISysV_msp430.h:13-16
+// C Includes
+// C++ Includes
+// Other libraries and framework includes
+// Project includes

You can remove these comments, I don't think we track that kind of thing 
anymore... I think?



Comment at: lldb/source/Plugins/ABI/MSP430/ABISysV_msp430.h:51-54
+if (cfa & 0x01)
+  return false; // Not 2 byte aligned
+if (cfa == 0)
+  return false; // Zero is not a valid stack address

nit: you can merge these


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146965/new/

https://reviews.llvm.org/D146965

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D146590: [lldb] Update some uses of Python2 API in typemaps.

2023-03-27 Thread Alex Langford via Phabricator via lldb-commits

bulbazord added a comment.

In D146590#4220388 , @jgorbe wrote:

> I found the actual reason for the crash I was talking about. This patch only 
> addressed the incorrect `OverflowError`, but the crash comes from a double 
> free (as @rupprecht mentioned) in the error handling logic. The error path 
> here 
> 
>  does both `free($1);` and `SWIG_fail;`. The same goes for another error 
> check a few lines below that. In the generated code, the `SWIG_fail` macro is 
> expanded to `goto fail` and the `fail` label also frees the same memory 
> buffer.
>
> I believe (but I don't have any experience with SWIG typemaps so this is an 
> educated guess) that the call to `free` in the error path comes from the 
> `%typemap(freearg)` immediately after that one. So if freearg already takes 
> care of it, the error handling logic in `%typemap(in)` should just call 
> `SWIG_fail`. Does that sound correct?

Yea, after reading the SWIG documentation, this diagnosis looks correct. If 
you'd like to fix this feel free to upload a patch and list myself and @mib as 
reviewers. Otherwise let me know and I can take care of it. Thanks for doing 
that investigation!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146590/new/

https://reviews.llvm.org/D146590

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 64b3bbc - [lldb] Remove NO_PLUGIN_DEPENDENCIES from lldbTarget

2023-03-27 Thread Alex Langford via lldb-commits


Author: Alex Langford
Date: 2023-03-27T13:35:48-07:00
New Revision: 64b3bbc5220615b13d4a003877c773b8745a4d00

URL: 
https://github.com/llvm/llvm-project/commit/64b3bbc5220615b13d4a003877c773b8745a4d00
DIFF: 
https://github.com/llvm/llvm-project/commit/64b3bbc5220615b13d4a003877c773b8745a4d00.diff

LOG: [lldb] Remove NO_PLUGIN_DEPENDENCIES from lldbTarget

As discussed in https://reviews.llvm.org/D146668 I will be reverting
0c5cee779929f840f4f286c5894a01f583ee7b4a and 
ee232506b870ce5282cc4da5ca493d41d361feb3.
I'm removing NO_PLUGIN_DEPENDENCIES from lldbTarget first so that these
reverts will not leave LLDB in a bad state afterwards.

Added: 


Modified: 
lldb/source/Target/CMakeLists.txt

Removed: 




diff  --git a/lldb/source/Target/CMakeLists.txt 
b/lldb/source/Target/CMakeLists.txt
index 3823daf370b4f..37040294c618c 100644
--- a/lldb/source/Target/CMakeLists.txt
+++ b/lldb/source/Target/CMakeLists.txt
@@ -6,7 +6,8 @@ lldb_tablegen(TargetPropertiesEnum.inc 
-gen-lldb-property-enum-defs
   SOURCE TargetProperties.td
   TARGET LLDBTargetPropertiesEnumGen)
 
-add_lldb_library(lldbTarget NO_PLUGIN_DEPENDENCIES
+# TODO: Add property `NO_PLUGIN_DEPENDENCIES` to lldbTarget
+add_lldb_library(lldbTarget
   ABI.cpp
   AssertFrameRecognizer.cpp
   DynamicRegisterInfo.cpp



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] ea157f9 - Revert "[lldb-server] Use Platform plugin corresponding to the host"

2023-03-27 Thread Alex Langford via lldb-commits


Author: Alex Langford
Date: 2023-03-27T13:42:14-07:00
New Revision: ea157f9b3912209cdda4d3acc07245624ed68b6f

URL: 
https://github.com/llvm/llvm-project/commit/ea157f9b3912209cdda4d3acc07245624ed68b6f
DIFF: 
https://github.com/llvm/llvm-project/commit/ea157f9b3912209cdda4d3acc07245624ed68b6f.diff

LOG: Revert "[lldb-server] Use Platform plugin corresponding to the host"

This reverts commit 0c5cee779929f840f4f286c5894a01f583ee7b4a.
As discussed in https://reviews.llvm.org/D146668 we'll find another way
forward.

Added: 


Modified: 
lldb/test/API/functionalities/inferior-crashing/TestInferiorCrashing.py
lldb/tools/lldb-server/CMakeLists.txt
lldb/tools/lldb-server/SystemInitializerLLGS.cpp

Removed: 




diff  --git 
a/lldb/test/API/functionalities/inferior-crashing/TestInferiorCrashing.py 
b/lldb/test/API/functionalities/inferior-crashing/TestInferiorCrashing.py
index 172c00eb59dc2..b63a09d047024 100644
--- a/lldb/test/API/functionalities/inferior-crashing/TestInferiorCrashing.py
+++ b/lldb/test/API/functionalities/inferior-crashing/TestInferiorCrashing.py
@@ -63,9 +63,7 @@ def inferior_crashing(self):
 # The exact stop reason depends on the platform
 if self.platformIsDarwin():
 stop_reason = 'stop reason = EXC_BAD_ACCESS'
-elif self.getPlatform() == "linux":
-stop_reason = 'stop reason = signal SIGSEGV: address not mapped to 
object'
-elif self.getPlatform() == "freebsd":
+elif self.getPlatform() == "linux" or self.getPlatform() == "freebsd":
 stop_reason = 'stop reason = signal SIGSEGV'
 else:
 stop_reason = 'stop reason = invalid address'

diff  --git a/lldb/tools/lldb-server/CMakeLists.txt 
b/lldb/tools/lldb-server/CMakeLists.txt
index 56da4c8b56807..67103e87a1d4a 100644
--- a/lldb/tools/lldb-server/CMakeLists.txt
+++ b/lldb/tools/lldb-server/CMakeLists.txt
@@ -7,29 +7,20 @@ set(LLDB_PLUGINS)
 
 if(CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
   list(APPEND LLDB_PLUGINS lldbPluginProcessLinux)
-  if (CMAKE_SYSTEM_NAME MATCHES "Linux")
-list(APPEND LLDB_PLUGINS lldbPluginPlatformLinux)
-  else()
-list(APPEND LLDB_PLUGINS lldbPluginPlatformAndroid)
-  endif()
 endif()
 
 if(CMAKE_SYSTEM_NAME MATCHES "FreeBSD")
   list(APPEND LLDB_PLUGINS lldbPluginProcessFreeBSD)
-  list(APPEND LLDB_PLUGINS lldbPluginPlatformFreeBSD)
 endif()
 
 if(CMAKE_SYSTEM_NAME MATCHES "NetBSD")
   list(APPEND LLDB_PLUGINS lldbPluginProcessNetBSD)
-  list(APPEND LLDB_PLUGINS lldbPluginPlatformNetBSD)
 endif()
 
 if(CMAKE_SYSTEM_NAME MATCHES "Darwin")
   list(APPEND LLDB_PLUGINS lldbPluginObjectFileMachO)
-  list(APPEND LLDB_PLUGINS lldbPluginPlatformMacOSX)
 elseif(CMAKE_SYSTEM_NAME MATCHES "Windows")
   list(APPEND LLDB_PLUGINS lldbPluginObjectFilePECOFF)
-  list(APPEND LLDB_PLUGINS lldbPluginPlatformWindows)
 else()
   list(APPEND LLDB_PLUGINS lldbPluginObjectFileELF)
 endif()

diff  --git a/lldb/tools/lldb-server/SystemInitializerLLGS.cpp 
b/lldb/tools/lldb-server/SystemInitializerLLGS.cpp
index 1909ea4dc7984..4233252a84dfc 100644
--- a/lldb/tools/lldb-server/SystemInitializerLLGS.cpp
+++ b/lldb/tools/lldb-server/SystemInitializerLLGS.cpp
@@ -11,29 +11,12 @@
 #if defined(__APPLE__)
 #include "Plugins/ObjectFile/Mach-O/ObjectFileMachO.h"
 using HostObjectFile = ObjectFileMachO;
-#include "Plugins/Platform/MacOSX/PlatformMacOSX.h"
-using HostPlatform = lldb_private::PlatformMacOSX;
 #elif defined(_WIN32)
 #include "Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h"
 using HostObjectFile = ObjectFilePECOFF;
-#include "Plugins/Platform/Windows/PlatformWindows.h"
-using HostPlatform = lldb_private::PlatformWindows;
 #else
 #include "Plugins/ObjectFile/ELF/ObjectFileELF.h"
 using HostObjectFile = ObjectFileELF;
-#if defined(__ANDROID__)
-#include "Plugins/Platform/Android/PlatformAndroid.h"
-using HostPlatform = lldb_private::platform_android::PlatformAndroid;
-#elif defined(__FreeBSD__)
-#include "Plugins/Platform/FreeBSD/PlatformFreeBSD.h"
-using HostPlatform = lldb_private::platform_freebsd::PlatformFreeBSD;
-#elif defined(__linux__)
-#include "Plugins/Platform/Linux/PlatformLinux.h"
-using HostPlatform = lldb_private::platform_linux::PlatformLinux;
-#elif defined(__NetBSD__)
-#include "Plugins/Platform/NetBSD/PlatformNetBSD.h"
-using HostPlatform = lldb_private::platform_netbsd::PlatformNetBSD;
-#endif
 #endif
 
 #if defined(__arm64__) || defined(__aarch64__) || defined(_M_ARM64)
@@ -75,7 +58,6 @@ llvm::Error SystemInitializerLLGS::Initialize() {
 return e;
 
   HostObjectFile::Initialize();
-  HostPlatform::Initialize();
 
 #if defined(LLDB_TARGET_ARM) || defined(LLDB_TARGET_ARM64)
   EmulateInstructionARM::Initialize();
@@ -98,7 +80,6 @@ llvm::Error SystemInitializerLLGS::Initialize() {
 
 void SystemInitializerLLGS::Terminate() {
   HostObjectFile::Terminate();
-  HostPlatform::Terminate();
 
 #if defined(LLDB_TARGET_ARM) || defi

[Lldb-commits] [lldb] 41b6d58 - Revert "[lldb] Move UnixSignals creation into Platform plugins"

2023-03-27 Thread Alex Langford via lldb-commits


Author: Alex Langford
Date: 2023-03-27T13:42:14-07:00
New Revision: 41b6d5863fef94bb3a9d184802c5bb6e2759da15

URL: 
https://github.com/llvm/llvm-project/commit/41b6d5863fef94bb3a9d184802c5bb6e2759da15
DIFF: 
https://github.com/llvm/llvm-project/commit/41b6d5863fef94bb3a9d184802c5bb6e2759da15.diff

LOG: Revert "[lldb] Move UnixSignals creation into Platform plugins"

This reverts commit ee232506b870ce5282cc4da5ca493d41d361feb3.
As discussed in https://reviews.llvm.org/D146668 we'll find another way
forward.

Added: 
lldb/source/Plugins/Process/Utility/FreeBSDSignals.cpp
lldb/source/Plugins/Process/Utility/FreeBSDSignals.h
lldb/source/Plugins/Process/Utility/GDBRemoteSignals.cpp
lldb/source/Plugins/Process/Utility/GDBRemoteSignals.h
lldb/source/Plugins/Process/Utility/LinuxSignals.cpp
lldb/source/Plugins/Process/Utility/LinuxSignals.h
lldb/source/Plugins/Process/Utility/NetBSDSignals.cpp
lldb/source/Plugins/Process/Utility/NetBSDSignals.h

Modified: 
lldb/include/lldb/Target/Platform.h
lldb/include/lldb/Target/UnixSignals.h
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/CMakeLists.txt
lldb/source/Plugins/Platform/FreeBSD/CMakeLists.txt
lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp
lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.h
lldb/source/Plugins/Platform/Linux/CMakeLists.txt
lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
lldb/source/Plugins/Platform/Linux/PlatformLinux.h
lldb/source/Plugins/Platform/NetBSD/CMakeLists.txt
lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.h
lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.h
lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.h
lldb/source/Plugins/Platform/Windows/PlatformWindows.h
lldb/source/Plugins/Platform/gdb-server/CMakeLists.txt
lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
lldb/source/Plugins/Process/Utility/CMakeLists.txt
lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
lldb/source/Target/CMakeLists.txt
lldb/source/Target/Platform.cpp
lldb/source/Target/UnixSignals.cpp
lldb/unittests/Process/gdb-remote/CMakeLists.txt
lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
lldb/unittests/Target/RemoteAwarePlatformTest.cpp

Removed: 
lldb/source/Plugins/Platform/FreeBSD/FreeBSDSignals.cpp
lldb/source/Plugins/Platform/FreeBSD/FreeBSDSignals.h
lldb/source/Plugins/Platform/Linux/LinuxSignals.cpp
lldb/source/Plugins/Platform/Linux/LinuxSignals.h
lldb/source/Plugins/Platform/NetBSD/NetBSDSignals.cpp
lldb/source/Plugins/Platform/NetBSD/NetBSDSignals.h
lldb/source/Plugins/Platform/gdb-server/GDBRemoteSignals.cpp
lldb/source/Plugins/Platform/gdb-server/GDBRemoteSignals.h



diff  --git a/lldb/include/lldb/Target/Platform.h 
b/lldb/include/lldb/Target/Platform.h
index e184249a62980..08e47cc132473 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -619,12 +619,10 @@ class Platform : public PluginInterface {
 return 1;
   }
 
-  virtual lldb::UnixSignalsSP GetRemoteUnixSignals();
+  virtual const lldb::UnixSignalsSP &GetRemoteUnixSignals();
 
   lldb::UnixSignalsSP GetUnixSignals();
 
-  virtual lldb::UnixSignalsSP CreateUnixSignals() = 0;
-
   /// Locate a queue name given a thread's qaddr
   ///
   /// On a system using libdispatch ("Grand Central Dispatch") style queues, a

diff  --git a/lldb/include/lldb/Target/UnixSignals.h 
b/lldb/include/lldb/Target/UnixSignals.h
index 65eac7ebdd823..859cf0c814f69 100644
--- a/lldb/include/lldb/Target/UnixSignals.h
+++ b/lldb/include/lldb/Target/UnixSignals.h
@@ -22,6 +22,7 @@ namespace lldb_private {
 
 class UnixSignals {
 public:
+  static lldb::UnixSignalsSP Create(const ArchSpec &arch);
   static lldb::UnixSignalsSP CreateForHost();
 
   // Constructors and Destructors

diff  --git 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/CMakeLists.txt 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/CMakeLists.txt
index 2844ba6b2bda2..3789f56325980 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/CMakeLists.txt
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/CMakeLists.txt
@@ -19,7 +19,6 @@ add_lldb_library(lldbPluginAppleObjCRuntime PLUGIN
 lldbUtility
 lldbPluginExpressionParserClang
 lldbPluginCPPRuntime
-lldbPluginProcessUtility
 lldbPluginTypeSystemClang
   CLANG_LIBS
 clangAST

diff  --git a/lldb/source/Plugins/Platform/FreeBSD/CMakeLists.txt 
b/lldb/source/Plugins/Platform/

[Lldb-commits] [PATCH] D147001: [lldb] TestInferiorCrashing.py should check for crash reason

2023-03-27 Thread Alex Langford via Phabricator via lldb-commits

bulbazord created this revision.
bulbazord added reviewers: labath, rupprecht.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

In a now-reverted series of patches, I inadvertently broke the ability
for lldb-server to explain a crash reason. To ensure that this feature
continues to work after future refactors, let's test the feature.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147001

Files:
  lldb/test/API/functionalities/inferior-crashing/TestInferiorCrashing.py


Index: lldb/test/API/functionalities/inferior-crashing/TestInferiorCrashing.py
===
--- lldb/test/API/functionalities/inferior-crashing/TestInferiorCrashing.py
+++ lldb/test/API/functionalities/inferior-crashing/TestInferiorCrashing.py
@@ -64,7 +64,7 @@
 if self.platformIsDarwin():
 stop_reason = 'stop reason = EXC_BAD_ACCESS'
 elif self.getPlatform() == "linux" or self.getPlatform() == "freebsd":
-stop_reason = 'stop reason = signal SIGSEGV'
+stop_reason = 'stop reason = signal SIGSEGV: address not mapped to 
object'
 else:
 stop_reason = 'stop reason = invalid address'
 self.expect("thread list", STOPPED_DUE_TO_EXC_BAD_ACCESS,


Index: lldb/test/API/functionalities/inferior-crashing/TestInferiorCrashing.py
===
--- lldb/test/API/functionalities/inferior-crashing/TestInferiorCrashing.py
+++ lldb/test/API/functionalities/inferior-crashing/TestInferiorCrashing.py
@@ -64,7 +64,7 @@
 if self.platformIsDarwin():
 stop_reason = 'stop reason = EXC_BAD_ACCESS'
 elif self.getPlatform() == "linux" or self.getPlatform() == "freebsd":
-stop_reason = 'stop reason = signal SIGSEGV'
+stop_reason = 'stop reason = signal SIGSEGV: address not mapped to object'
 else:
 stop_reason = 'stop reason = invalid address'
 self.expect("thread list", STOPPED_DUE_TO_EXC_BAD_ACCESS,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D147006: [lldb] Fix value printing for a specific case

2023-03-27 Thread Dave Lee via Phabricator via lldb-commits

kastiglione created this revision.
kastiglione added reviewers: aprantl, jingham, jgorbe.
Herald added a project: All.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Fixes printing of spaces in cases where the following are true:

1. Persistent results are disabled
2. The type has a summary string

As reported by @jgorbe in D146783 , two 
spaces were being printed before the summary
string, and no spaces were printed after.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147006

Files:
  lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
  lldb/source/DataFormatters/ValueObjectPrinter.cpp
  lldb/test/API/commands/dwim-print/TestDWIMPrint.py


Index: lldb/test/API/commands/dwim-print/TestDWIMPrint.py
===
--- lldb/test/API/commands/dwim-print/TestDWIMPrint.py
+++ lldb/test/API/commands/dwim-print/TestDWIMPrint.py
@@ -122,3 +122,12 @@
 self.runCmd("settings set auto-one-line-summaries false")
 self._expect_cmd(f"dwim-print s", "frame variable")
 self._expect_cmd(f"dwim-print (struct Structure)s", "expression")
+
+def test_summary_strings(self):
+"""Test dwim-print with nested values (structs, etc)."""
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here", 
lldb.SBFileSpec("main.c"))
+self.runCmd("settings set auto-one-line-summaries false")
+self.runCmd("type summary add -e -s 'stub summary' Structure")
+self._expect_cmd(f"dwim-print s", "frame variable")
+self._expect_cmd(f"dwim-print (struct Structure)s", "expression")
Index: lldb/source/DataFormatters/ValueObjectPrinter.cpp
===
--- lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -445,7 +445,9 @@
   }
 
   if (m_summary.size()) {
-m_stream->Printf(" %s", m_summary.c_str());
+if (ShouldShowName() || value_printed)
+  m_stream->PutChar(' ');
+m_stream->PutCString(m_summary);
 summary_printed = true;
   }
 }
@@ -560,7 +562,8 @@
   return m_valobj;
 }
 
-void ValueObjectPrinter::PrintChildrenPreamble() {
+void ValueObjectPrinter::PrintChildrenPreamble(bool value_printed,
+   bool summary_printed) {
   if (m_options.m_flat_output) {
 if (ShouldPrintValueObject())
   m_stream->EOL();
@@ -568,7 +571,7 @@
 if (ShouldPrintValueObject()) {
   if (IsRef()) {
 m_stream->PutCString(": ");
-  } else if (ShouldShowName()) {
+  } else if (value_printed || summary_printed || ShouldShowName()) {
 m_stream->PutChar(' ');
   }
   m_stream->PutCString("{\n");
@@ -694,7 +697,7 @@
 for (size_t idx = 0; idx < num_children; ++idx) {
   if (ValueObjectSP child_sp = GenerateChild(synth_m_valobj, idx)) {
 if (!any_children_printed) {
-  PrintChildrenPreamble();
+  PrintChildrenPreamble(value_printed, summary_printed);
   any_children_printed = true;
 }
 PrintChild(child_sp, curr_ptr_depth);
Index: lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
===
--- lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
+++ lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
@@ -98,7 +98,7 @@
 
   ValueObject *GetValueObjectForChildrenGeneration();
 
-  void PrintChildrenPreamble();
+  void PrintChildrenPreamble(bool value_printed, bool summary_printed);
 
   void PrintChildrenPostamble(bool print_dotdotdot);
 


Index: lldb/test/API/commands/dwim-print/TestDWIMPrint.py
===
--- lldb/test/API/commands/dwim-print/TestDWIMPrint.py
+++ lldb/test/API/commands/dwim-print/TestDWIMPrint.py
@@ -122,3 +122,12 @@
 self.runCmd("settings set auto-one-line-summaries false")
 self._expect_cmd(f"dwim-print s", "frame variable")
 self._expect_cmd(f"dwim-print (struct Structure)s", "expression")
+
+def test_summary_strings(self):
+"""Test dwim-print with nested values (structs, etc)."""
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here", lldb.SBFileSpec("main.c"))
+self.runCmd("settings set auto-one-line-summaries false")
+self.runCmd("type summary add -e -s 'stub summary' Structure")
+self._expect_cmd(f"dwim-print s", "frame variable")
+self._expect_cmd(f"dwim-print (struct Structure)s", "expression")
Index: lldb/source/DataFormatters/ValueObjectPrinter.cpp
===
--- lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -445,7 +445,9 @@
   }
 
   if (m

[Lldb-commits] [PATCH] D147007: [lldb] Fix double free in python bindings error handling.

2023-03-27 Thread Jorge Gorbe Moya via Phabricator via lldb-commits

jgorbe created this revision.
jgorbe added reviewers: bulbazord, mib, labath.
jgorbe added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
jgorbe requested review of this revision.

If we have a `%typemap(freearg)` that frees the argument, we shouldn't
free it manually on an error path before calling `SWIG_fail`.
`SWIG_fail` will already free the memory in this case, and doing it
manually results in a double free.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147007

Files:
  lldb/bindings/python/python-typemaps.swig


Index: lldb/bindings/python/python-typemaps.swig
===
--- lldb/bindings/python/python-typemaps.swig
+++ lldb/bindings/python/python-typemaps.swig
@@ -17,7 +17,6 @@
   PythonString py_str = list.GetItemAtIndex(i).AsType();
   if (!py_str.IsAllocated()) {
 PyErr_SetString(PyExc_TypeError, "list must contain strings");
-free($1);
 SWIG_fail;
   }
 
@@ -294,12 +293,10 @@
   PyObject *o = PyList_GetItem($input, i);
   if (!SetNumberFromPyObject($1[i], o)) {
 PyErr_SetString(PyExc_TypeError, "list must contain numbers");
-free($1);
 SWIG_fail;
   }
 
   if (PyErr_Occurred()) {
-free($1);
 SWIG_fail;
   }
 }


Index: lldb/bindings/python/python-typemaps.swig
===
--- lldb/bindings/python/python-typemaps.swig
+++ lldb/bindings/python/python-typemaps.swig
@@ -17,7 +17,6 @@
   PythonString py_str = list.GetItemAtIndex(i).AsType();
   if (!py_str.IsAllocated()) {
 PyErr_SetString(PyExc_TypeError, "list must contain strings");
-free($1);
 SWIG_fail;
   }
 
@@ -294,12 +293,10 @@
   PyObject *o = PyList_GetItem($input, i);
   if (!SetNumberFromPyObject($1[i], o)) {
 PyErr_SetString(PyExc_TypeError, "list must contain numbers");
-free($1);
 SWIG_fail;
   }
 
   if (PyErr_Occurred()) {
-free($1);
 SWIG_fail;
   }
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D146590: [lldb] Update some uses of Python2 API in typemaps.

2023-03-27 Thread Jorge Gorbe Moya via Phabricator via lldb-commits

jgorbe added a comment.

In D146590#4225500 , @bulbazord wrote:

> Yea, after reading the SWIG documentation, this diagnosis looks correct. If 
> you'd like to fix this feel free to upload a patch and list myself and @mib 
> as reviewers. Otherwise let me know and I can take care of it. Thanks for 
> doing that investigation!

Thanks! Just sent you all https://reviews.llvm.org/D147007


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146590/new/

https://reviews.llvm.org/D146590

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D147007: [lldb] Fix double free in python bindings error handling.

2023-03-27 Thread Alex Langford via Phabricator via lldb-commits

bulbazord accepted this revision.
bulbazord added a comment.
This revision is now accepted and ready to land.

Thanks for taking care of that!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147007/new/

https://reviews.llvm.org/D147007

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D146977: [lldb-server/linux] Use waitpid(-1) to collect inferior events

2023-03-27 Thread jeffrey tan via Phabricator via lldb-commits

yinghuitan added inline comments.



Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:376-386
+bool handled = llvm::any_of(m_processes, [&](NativeProcessLinux *process) {
+  return process->TryHandleWaitStatus(pid, status);
+});
+if (!handled) {
+  if (status.type == WaitStatus::Stop && status.status == SIGSTOP) {
+// Store the thread for later collection.
+m_unowned_threads.insert(pid);

Maybe I misunderstood the purpose -- the code stores `pid` into 
`m_unowned_threads` for later collection if **none** of the process handle it. 
Instead, I thought we want to store into `m_unowned_threads` if **at least one 
process** did not handle it? 

Can you add some comment to clarify the intention? Thanks.



Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:538
+  (status.type == WaitStatus::Exit || status.type == WaitStatus::Signal)) {
+SetExitStatus(status, true);
+return true;

Can you add back the original comment which I find useful? Thanks
```
// The process exited.  We're done monitoring.  Report to delegate.
```



Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.h:70-72
+llvm::SmallPtrSet m_processes;
+
+llvm::DenseSet<::pid_t> m_unowned_threads;

I assume no lock is needed? 



Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.h:72
+
+llvm::DenseSet<::pid_t> m_unowned_threads;
+

Add a comment for this field.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146977/new/

https://reviews.llvm.org/D146977

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D147007: [lldb] Fix double free in python bindings error handling.

2023-03-27 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere accepted this revision.
JDevlieghere added a comment.

LGTM. I think it would be worth adding that information to the top of the file 
to prevent similar mistakes in the future.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147007/new/

https://reviews.llvm.org/D147007

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D147009: [lldb] Remove lldbExpression's dependency on ObjectFileJIT

2023-03-27 Thread Alex Langford via Phabricator via lldb-commits

bulbazord created this revision.
bulbazord added reviewers: JDevlieghere, labath, clayborg, mib, jingham.
Herald added subscribers: pmatos, asb, sbc100, emaste.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added subscribers: lldb-commits, MaskRay, aheejin.
Herald added a project: LLDB.

The high level goal is the title. The way this is accomplished is by adding a
new callback ObjectFileCreateInstanceWithDelegate where the intention is
to be able to create ObjectFile instances that are backed by a delegate
(like the way ObjectFileJIT works now with IRExecutionUnit as its
delegate).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147009

Files:
  lldb/include/lldb/Core/Module.h
  lldb/include/lldb/Core/PluginManager.h
  lldb/include/lldb/Expression/IRExecutionUnit.h
  lldb/include/lldb/Symbol/ObjectFile.h
  lldb/include/lldb/lldb-forward.h
  lldb/include/lldb/lldb-private-interfaces.h
  lldb/source/Core/Module.cpp
  lldb/source/Core/PluginManager.cpp
  lldb/source/Expression/CMakeLists.txt
  lldb/source/Expression/FunctionCaller.cpp
  lldb/source/Expression/IRExecutionUnit.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
  lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
  lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
  lldb/source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp
  lldb/source/Plugins/ObjectFile/JIT/ObjectFileJIT.h
  lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp
  lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.h
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
  lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
  lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h
  lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
  lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.h
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
  lldb/source/Symbol/ObjectFile.cpp

Index: lldb/source/Symbol/ObjectFile.cpp
===
--- lldb/source/Symbol/ObjectFile.cpp
+++ lldb/source/Symbol/ObjectFile.cpp
@@ -184,6 +184,28 @@
   return object_file_sp;
 }
 
+ObjectFileSP
+ObjectFile::FindPlugin(const lldb::ModuleSP &module_sp,
+   const lldb::ObjectFileDelegateSP &delegate_sp) {
+  if (!module_sp)
+return ObjectFileSP();
+
+  ObjectFileSP object_file_sp;
+  ObjectFileCreateInstanceWithDelegate create_callback;
+  for (uint32_t idx = 0;
+   (create_callback =
+PluginManager::GetObjectFileCreateWithDelegateCallbackAtIndex(
+idx)) != nullptr;
+   idx++) {
+object_file_sp.reset(create_callback(module_sp, delegate_sp));
+if (object_file_sp)
+  return object_file_sp;
+  }
+
+  object_file_sp.reset();
+  return object_file_sp;
+}
+
 size_t ObjectFile::GetModuleSpecifications(const FileSpec &file,
lldb::offset_t file_offset,
lldb::offset_t file_size,
Index: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
===
--- lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
+++ lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
@@ -40,6 +40,10 @@
   const lldb::ProcessSP &process_sp,
   lldb::addr_t header_addr);
 
+  static ObjectFile *
+  CreateInstanceWithDelegate(const lldb::ModuleSP &module_sp,
+ const lldb::ObjectFileDelegateSP &delegate_sp);
+
   static size_t GetModuleSpecifications(const FileSpec &file,
 lldb::DataBufferSP &data_sp,
 lldb::offset_t data_offset,
Index: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
===
--- lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
+++ lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
@@ -79,9 +79,11 @@
 char ObjectFileWasm::ID;
 
 void ObjectFileWasm::Initialize() {
-  PluginManager::RegisterPlugin(GetPluginNameStatic(),
-GetPluginDescriptionStatic(), CreateInstance,
-CreateMemoryInstance, GetModuleSpecifications);
+  PluginManager::RegisterPlugin(
+  GetPluginNameStatic(), GetPluginDescriptionStatic(), CreateInstance,
+  CreateMemoryInstance,
+  /* ObjectFileCreateInstanceWithDelegate = */ nullptr,
+  GetM

[Lldb-commits] [PATCH] D147009: [lldb] Remove lldbExpression's dependency on ObjectFileJIT

2023-03-27 Thread Alex Langford via Phabricator via lldb-commits

bulbazord updated this revision to Diff 508817.
bulbazord added a comment.
Herald added a subscriber: Michael137.

Minor fix to ObjectFileWasm


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147009/new/

https://reviews.llvm.org/D147009

Files:
  lldb/include/lldb/Core/Module.h
  lldb/include/lldb/Core/PluginManager.h
  lldb/include/lldb/Expression/IRExecutionUnit.h
  lldb/include/lldb/Symbol/ObjectFile.h
  lldb/include/lldb/lldb-forward.h
  lldb/include/lldb/lldb-private-interfaces.h
  lldb/source/Core/Module.cpp
  lldb/source/Core/PluginManager.cpp
  lldb/source/Expression/CMakeLists.txt
  lldb/source/Expression/FunctionCaller.cpp
  lldb/source/Expression/IRExecutionUnit.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
  lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
  lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
  lldb/source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp
  lldb/source/Plugins/ObjectFile/JIT/ObjectFileJIT.h
  lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp
  lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.h
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
  lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
  lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h
  lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
  lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.h
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
  lldb/source/Symbol/ObjectFile.cpp

Index: lldb/source/Symbol/ObjectFile.cpp
===
--- lldb/source/Symbol/ObjectFile.cpp
+++ lldb/source/Symbol/ObjectFile.cpp
@@ -184,6 +184,28 @@
   return object_file_sp;
 }
 
+ObjectFileSP
+ObjectFile::FindPlugin(const lldb::ModuleSP &module_sp,
+   const lldb::ObjectFileDelegateSP &delegate_sp) {
+  if (!module_sp)
+return ObjectFileSP();
+
+  ObjectFileSP object_file_sp;
+  ObjectFileCreateInstanceWithDelegate create_callback;
+  for (uint32_t idx = 0;
+   (create_callback =
+PluginManager::GetObjectFileCreateWithDelegateCallbackAtIndex(
+idx)) != nullptr;
+   idx++) {
+object_file_sp.reset(create_callback(module_sp, delegate_sp));
+if (object_file_sp)
+  return object_file_sp;
+  }
+
+  object_file_sp.reset();
+  return object_file_sp;
+}
+
 size_t ObjectFile::GetModuleSpecifications(const FileSpec &file,
lldb::offset_t file_offset,
lldb::offset_t file_size,
Index: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
===
--- lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
+++ lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
@@ -40,6 +40,10 @@
   const lldb::ProcessSP &process_sp,
   lldb::addr_t header_addr);
 
+  static ObjectFile *
+  CreateInstanceWithDelegate(const lldb::ModuleSP &module_sp,
+ const lldb::ObjectFileDelegateSP &delegate_sp);
+
   static size_t GetModuleSpecifications(const FileSpec &file,
 lldb::DataBufferSP &data_sp,
 lldb::offset_t data_offset,
Index: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
===
--- lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
+++ lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
@@ -79,9 +79,10 @@
 char ObjectFileWasm::ID;
 
 void ObjectFileWasm::Initialize() {
-  PluginManager::RegisterPlugin(GetPluginNameStatic(),
-GetPluginDescriptionStatic(), CreateInstance,
-CreateMemoryInstance, GetModuleSpecifications);
+  PluginManager::RegisterPlugin(
+  GetPluginNameStatic(), GetPluginDescriptionStatic(), CreateInstance,
+  CreateMemoryInstance, CreateInstanceWithDelegate,
+  GetModuleSpecifications);
 }
 
 void ObjectFileWasm::Terminate() {
@@ -156,6 +157,11 @@
   return nullptr;
 }
 
+ObjectFile *ObjectFileWasm::CreateInstanceWithDelegate(
+const ModuleSP &module_sp, const ObjectFileDelegateSP &delegate_sp) {
+  return nullptr;
+}
+
 bool ObjectFileWasm::DecodeNextSection(lldb::offset_t *offset_ptr) {
   // Buffer sufficient to read a section header and find the pointer to the next
   // section.
Index: lldb/sour

[Lldb-commits] [lldb] 4a38d33 - [lldb] Fix double free in python bindings error handling.

2023-03-27 Thread Jorge Gorbe Moya via lldb-commits


Author: Jorge Gorbe Moya
Date: 2023-03-27T16:07:36-07:00
New Revision: 4a38d33268959309dd97d9ef423327607bda4104

URL: 
https://github.com/llvm/llvm-project/commit/4a38d33268959309dd97d9ef423327607bda4104
DIFF: 
https://github.com/llvm/llvm-project/commit/4a38d33268959309dd97d9ef423327607bda4104.diff

LOG: [lldb] Fix double free in python bindings error handling.

If we have a `%typemap(freearg)` that frees the argument, we shouldn't
free it manually on an error path before calling `SWIG_fail`.
`SWIG_fail` will already free the memory in this case, and doing it
manually results in a double free.

Differential Revision: https://reviews.llvm.org/D147007

Added: 


Modified: 
lldb/bindings/python/python-typemaps.swig

Removed: 




diff  --git a/lldb/bindings/python/python-typemaps.swig 
b/lldb/bindings/python/python-typemaps.swig
index 3e9675c8c00f1..d2520c415e741 100644
--- a/lldb/bindings/python/python-typemaps.swig
+++ b/lldb/bindings/python/python-typemaps.swig
@@ -1,4 +1,10 @@
-/* Typemap definitions, to allow SWIG to properly handle 'char**' data types. 
*/
+/* Typemap definitions, to allow SWIG to properly handle 'char**' data types.
+
+NOTE: If there's logic to free memory in a %typemap(freearg), it will also be
+run if you call SWIG_fail on an error path. Don't manually free() an argument
+AND call SWIG_fail at the same time, because it will result in a double free.
+
+*/
 
 %inline %{
 
@@ -17,7 +23,6 @@
   PythonString py_str = list.GetItemAtIndex(i).AsType();
   if (!py_str.IsAllocated()) {
 PyErr_SetString(PyExc_TypeError, "list must contain strings");
-free($1);
 SWIG_fail;
   }
 
@@ -294,12 +299,10 @@ template <> bool SetNumberFromPyObject(double 
&number, PyObject *obj) {
   PyObject *o = PyList_GetItem($input, i);
   if (!SetNumberFromPyObject($1[i], o)) {
 PyErr_SetString(PyExc_TypeError, "list must contain numbers");
-free($1);
 SWIG_fail;
   }
 
   if (PyErr_Occurred()) {
-free($1);
 SWIG_fail;
   }
 }



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D147007: [lldb] Fix double free in python bindings error handling.

2023-03-27 Thread Jorge Gorbe Moya via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4a38d3326895: [lldb] Fix double free in python bindings 
error handling. (authored by jgorbe).

Changed prior to commit:
  https://reviews.llvm.org/D147007?vs=508811&id=508827#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147007/new/

https://reviews.llvm.org/D147007

Files:
  lldb/bindings/python/python-typemaps.swig


Index: lldb/bindings/python/python-typemaps.swig
===
--- lldb/bindings/python/python-typemaps.swig
+++ lldb/bindings/python/python-typemaps.swig
@@ -1,4 +1,10 @@
-/* Typemap definitions, to allow SWIG to properly handle 'char**' data types. 
*/
+/* Typemap definitions, to allow SWIG to properly handle 'char**' data types.
+
+NOTE: If there's logic to free memory in a %typemap(freearg), it will also be
+run if you call SWIG_fail on an error path. Don't manually free() an argument
+AND call SWIG_fail at the same time, because it will result in a double free.
+
+*/
 
 %inline %{
 
@@ -17,7 +23,6 @@
   PythonString py_str = list.GetItemAtIndex(i).AsType();
   if (!py_str.IsAllocated()) {
 PyErr_SetString(PyExc_TypeError, "list must contain strings");
-free($1);
 SWIG_fail;
   }
 
@@ -294,12 +299,10 @@
   PyObject *o = PyList_GetItem($input, i);
   if (!SetNumberFromPyObject($1[i], o)) {
 PyErr_SetString(PyExc_TypeError, "list must contain numbers");
-free($1);
 SWIG_fail;
   }
 
   if (PyErr_Occurred()) {
-free($1);
 SWIG_fail;
   }
 }


Index: lldb/bindings/python/python-typemaps.swig
===
--- lldb/bindings/python/python-typemaps.swig
+++ lldb/bindings/python/python-typemaps.swig
@@ -1,4 +1,10 @@
-/* Typemap definitions, to allow SWIG to properly handle 'char**' data types. */
+/* Typemap definitions, to allow SWIG to properly handle 'char**' data types.
+
+NOTE: If there's logic to free memory in a %typemap(freearg), it will also be
+run if you call SWIG_fail on an error path. Don't manually free() an argument
+AND call SWIG_fail at the same time, because it will result in a double free.
+
+*/
 
 %inline %{
 
@@ -17,7 +23,6 @@
   PythonString py_str = list.GetItemAtIndex(i).AsType();
   if (!py_str.IsAllocated()) {
 PyErr_SetString(PyExc_TypeError, "list must contain strings");
-free($1);
 SWIG_fail;
   }
 
@@ -294,12 +299,10 @@
   PyObject *o = PyList_GetItem($input, i);
   if (!SetNumberFromPyObject($1[i], o)) {
 PyErr_SetString(PyExc_TypeError, "list must contain numbers");
-free($1);
 SWIG_fail;
   }
 
   if (PyErr_Occurred()) {
-free($1);
 SWIG_fail;
   }
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D147007: [lldb] Fix double free in python bindings error handling.

2023-03-27 Thread Jorge Gorbe Moya via Phabricator via lldb-commits

jgorbe added a comment.

In D147007#4225762 , @JDevlieghere 
wrote:

> LGTM. I think it would be worth adding that information to the top of the 
> file to prevent similar mistakes in the future.

Good idea, thanks. I added a note at the top before committing.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147007/new/

https://reviews.llvm.org/D147007

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D147012: [lldb] Support Universal Mach-O binaries with a fat64 header

2023-03-27 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere created this revision.
JDevlieghere added reviewers: jasonmolenda, pete, aprantl.
Herald added a project: All.
JDevlieghere requested review of this revision.

Support universal Mach-O binaries with a fat64 header. After D146879 
, dsymutil can now generate such binaries 
when the offsets would otherwise overflow the 32-bit offsets in the regular fat 
header.

rdar://107289570


https://reviews.llvm.org/D147012

Files:
  lldb/packages/Python/lldbsuite/test/make/Makefile.rules
  
lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp
  
lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.h
  lldb/test/API/macosx/universal64/Makefile
  lldb/test/API/macosx/universal64/TestUniversal64.py
  lldb/test/API/macosx/universal64/main.c

Index: lldb/test/API/macosx/universal64/main.c
===
--- /dev/null
+++ lldb/test/API/macosx/universal64/main.c
@@ -0,0 +1,5 @@
+#include 
+
+int foo() { return 0; }
+
+int main(int argc, char **argv) { return foo(); }
Index: lldb/test/API/macosx/universal64/TestUniversal64.py
===
--- /dev/null
+++ lldb/test/API/macosx/universal64/TestUniversal64.py
@@ -0,0 +1,39 @@
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class Universal64TestCase(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+def do_test(self):
+# Get the executable.
+exe = self.getBuildArtifact("fat.out")
+
+# Create a target.
+self.target = self.dbg.CreateTarget(exe)
+
+# Create a breakpoint on main.
+main_bp = self.target.BreakpointCreateByName("main")
+self.assertTrue(main_bp, VALID_BREAKPOINT)
+
+# Make sure the binary and the dSYM are in the image list.
+self.expect("image list ", patterns=['fat.out', 'fat.out.dSYM'])
+
+# The dynamic loader doesn't support fat64 executables so we can't
+# actually launch them here.
+
+@skipUnlessDarwin
+@skipIfDarwinEmbedded
+def test_universal64_executable(self):
+"""Test fat64 universal executable"""
+self.build(debug_info="dsym")
+self.do_test()
+
+@skipUnlessDarwin
+@skipIfDarwinEmbedded
+@skipIf(compiler="clang", compiler_version=['<', '7.0'])
+def test_universal64_dsym(self):
+"""Test fat64 universal dSYM"""
+self.build(debug_info="dsym", dictionary={'FAT64_DSYM': '1'})
+self.do_test()
Index: lldb/test/API/macosx/universal64/Makefile
===
--- /dev/null
+++ lldb/test/API/macosx/universal64/Makefile
@@ -0,0 +1,24 @@
+EXE := fat.out
+
+ifdef FAT64_DSYM
+	DSFLAGS_EXTRAS=-fat64
+endif
+
+include Makefile.rules
+
+all: fat.out
+
+fat.out: fat.arm64.out fat.x86_64.out
+	lipo -fat64 -create -o $@ $^
+
+fat.x86_64.out: fat.x86_64.o
+	$(CC) -isysroot $(SDKROOT) -target x86_64-apple-macosx10.9 -o $@ $<
+
+fat.arm64.out: fat.arm64.o
+	$(CC) -isysroot $(SDKROOT) -target arm64-apple-macosx10.9 -o $@ $<
+
+fat.x86_64.o: main.c
+	$(CC) -isysroot $(SDKROOT) -g -O0 -target x86_64-apple-macosx11 -c -o $@ $<
+
+fat.arm64.o: main.c
+	$(CC) -isysroot $(SDKROOT) -g -O0 -target arm64-apple-macosx11 -c -o $@ $<
Index: lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.h
===
--- lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.h
+++ lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.h
@@ -63,11 +63,46 @@
 
 protected:
   llvm::MachO::fat_header m_header;
-  std::vector m_fat_archs;
+
+  struct FatArch {
+FatArch(llvm::MachO::fat_arch arch) : m_arch(arch), m_is_fat64(false) {}
+FatArch(llvm::MachO::fat_arch_64 arch) : m_arch(arch), m_is_fat64(true) {}
+
+uint32_t GetCPUType() const {
+  return m_is_fat64 ? m_arch.fat_arch_64.cputype : m_arch.fat_arch.cputype;
+}
+
+uint32_t GetCPUSubType() const {
+  return m_is_fat64 ? m_arch.fat_arch_64.cpusubtype
+: m_arch.fat_arch.cpusubtype;
+}
+
+uint64_t GetOffset() const {
+  return m_is_fat64 ? m_arch.fat_arch_64.offset : m_arch.fat_arch.offset;
+}
+
+uint64_t GetSize() const {
+  return m_is_fat64 ? m_arch.fat_arch_64.size : m_arch.fat_arch.size;
+}
+
+uint32_t GetAlign() const {
+  return m_is_fat64 ? m_arch.fat_arch_64.align : m_arch.fat_arch.align;
+}
+
+  private:
+const union Arch {
+  Arch(llvm::MachO::fat_arch arch) : fat_arch(arch) {}
+  Arch(llvm::MachO::fat_arch_64 arch) : fat_arch_64(arch) {}
+  llvm::MachO::fat_arch fat_arch;
+  llvm::MachO::fat_arch_64 fat_arch_64;
+} m_arch;
+const bool m_is_fat64;
+  };
+  std::vector m_fat_archs;
 
   sta

[Lldb-commits] [PATCH] D147007: [lldb] Fix double free in python bindings error handling.

2023-03-27 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib added a comment.

LGTM! Thanks @jgorbe !


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147007/new/

https://reviews.llvm.org/D147007

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D145803: [clang][DebugInfo] Emit DW_AT_type of preferred name if available

2023-03-27 Thread David Blaikie via Phabricator via lldb-commits

dblaikie added inline comments.



Comment at: clang/test/CodeGen/preferred_name.cpp:49
+
+Foo> varFooInt;
+

Michael137 wrote:
> dblaikie wrote:
> > Michael137 wrote:
> > > probinson wrote:
> > > > This doesn't become `Foo` ?
> > > The name stays as `Foo>` but the type of the template parameter 
> > > becomes `BarInt`. So the dwarf would look like:
> > > ```
> > > DW_TAG_structure_type
> > >   DW_AT_name  ("Foo >")
> > > 
> > >   DW_TAG_template_type_parameter
> > > DW_AT_type(0x0095 "BarInt")
> > > DW_AT_name("T")
> > > 
> > > ```
> > > Will add the parameter metadata to the test
> > Hmm, that seems buggy - why doesn't `Foo >` become `Foo`? 
> > (is the preferred name ever used in the DW_AT_name? I'd have thought it 
> > should be unless it's explicitly avoided to ensure type 
> > compatibility/collapsing with debug info built without this feature 
> > enabled?)
> I suspect it's because the name is constructed using the clang TypePrinter. 
> And we turn off `PrintingPolicy::UsePreferredNames` by default to avoid 
> divergence from GCC.
> 
> Will double check though
Yeah, that sounds right/OK to me. Sorry for the diversion.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145803/new/

https://reviews.llvm.org/D145803

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D147006: [lldb] Fix value printing for a specific case

2023-03-27 Thread Jorge Gorbe Moya via Phabricator via lldb-commits

jgorbe accepted this revision.
jgorbe added a comment.
This revision is now accepted and ready to land.

Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147006/new/

https://reviews.llvm.org/D147006

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D147009: [lldb] Remove lldbExpression's dependency on ObjectFileJIT

2023-03-27 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib added inline comments.



Comment at: lldb/include/lldb/Symbol/ObjectFile.h:28
 
-class ObjectFileJITDelegate {
+class ObjectFileDelegate {
 public:

nit: What about nesting this in the `ObjectFile` class and rename it to 
`Delegate`. I would be better in term of namespacing IMO



Comment at: lldb/source/Expression/CMakeLists.txt:1
-# TODO: Add property `NO_PLUGIN_DEPENDENCIES` to lldbExpression
-add_lldb_library(lldbExpression
+add_lldb_library(lldbExpression NO_PLUGIN_DEPENDENCIES
   DiagnosticManager.cpp

Nice!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147009/new/

https://reviews.llvm.org/D147009

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

37 matches

Mail list logo