[Lldb-commits] [lldb] [lldb] Unify CalculateMD5 return types (PR #91029)

2024-05-03 Thread Anthony Ha via lldb-commits

https://github.com/Awfa updated https://github.com/llvm/llvm-project/pull/91029

>From 3112e3da309a24001aa610b15f2a23892b660fc8 Mon Sep 17 00:00:00 2001
From: Anthony Ha 
Date: Thu, 2 May 2024 23:55:47 +
Subject: [PATCH 1/3] Unify CalculateMD5 return types

---
 lldb/include/lldb/Target/Platform.h   |  4 +--
 .../include/lldb/Target/RemoteAwarePlatform.h |  4 +--
 .../Platform/MacOSX/PlatformDarwinDevice.cpp  | 16 +
 .../gdb-server/PlatformRemoteGDBServer.cpp|  8 ++---
 .../gdb-server/PlatformRemoteGDBServer.h  |  4 +--
 .../GDBRemoteCommunicationClient.cpp  | 30 ++--
 .../gdb-remote/GDBRemoteCommunicationClient.h |  2 +-
 lldb/source/Target/Platform.cpp   | 36 +--
 lldb/source/Target/RemoteAwarePlatform.cpp|  8 ++---
 9 files changed, 60 insertions(+), 52 deletions(-)

diff --git a/lldb/include/lldb/Target/Platform.h 
b/lldb/include/lldb/Target/Platform.h
index ad9c9dcbe684ba..e05c79cb501bf0 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -649,8 +649,8 @@ class Platform : public PluginInterface {
 
   virtual std::string GetPlatformSpecificConnectionInformation() { return ""; }
 
-  virtual bool CalculateMD5(const FileSpec _spec, uint64_t ,
-uint64_t );
+  virtual llvm::ErrorOr
+  CalculateMD5(const FileSpec _spec);
 
   virtual uint32_t GetResumeCountForLaunchInfo(ProcessLaunchInfo _info) 
{
 return 1;
diff --git a/lldb/include/lldb/Target/RemoteAwarePlatform.h 
b/lldb/include/lldb/Target/RemoteAwarePlatform.h
index d183815e1c8b07..0b9d79f9ff0380 100644
--- a/lldb/include/lldb/Target/RemoteAwarePlatform.h
+++ b/lldb/include/lldb/Target/RemoteAwarePlatform.h
@@ -58,8 +58,8 @@ class RemoteAwarePlatform : public Platform {
   Status SetFilePermissions(const FileSpec _spec,
 uint32_t file_permissions) override;
 
-  bool CalculateMD5(const FileSpec _spec, uint64_t ,
-uint64_t ) override;
+  llvm::ErrorOr
+  CalculateMD5(const FileSpec _spec) override;
 
   Status GetFileWithUUID(const FileSpec _file, const UUID *uuid,
  FileSpec _file) override;
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp 
b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
index 52777909a1f825..82156aca8cf159 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
@@ -405,17 +405,21 @@ lldb_private::Status 
PlatformDarwinDevice::GetSharedModuleWithLocalCache(
   // when going over the *slow* GDB remote transfer mechanism we first
   // check the hashes of the files - and only do the actual transfer if
   // they differ
-  uint64_t high_local, high_remote, low_local, low_remote;
   auto MD5 = llvm::sys::fs::md5_contents(module_cache_spec.GetPath());
   if (!MD5)
 return Status(MD5.getError());
-  std::tie(high_local, low_local) = MD5->words();
 
-  m_remote_platform_sp->CalculateMD5(module_spec.GetFileSpec(),
- low_remote, high_remote);
-  if (low_local != low_remote || high_local != high_remote) {
+  Log *log = GetLog(LLDBLog::Platform);
+  bool requires_transfer = true;
+  llvm::ErrorOr remote_md5 =
+  m_remote_platform_sp->CalculateMD5(module_spec.GetFileSpec());
+  if (std::error_code ec = remote_md5.getError())
+LLDB_LOG(log, "couldn't get md5 sum from remote: {0}",
+ ec.message());
+  else
+requires_transfer = *MD5 != *remote_md5;
+  if (requires_transfer) {
 // bring in the remote file
-Log *log = GetLog(LLDBLog::Platform);
 LLDB_LOGF(log,
   "[%s] module %s/%s needs to be replaced from remote 
copy",
   (IsHost() ? "host" : "remote"),
diff --git 
a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp 
b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
index 0dce5add2e3759..4684947ede209f 100644
--- a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
+++ b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
@@ -684,12 +684,12 @@ Status PlatformRemoteGDBServer::RunShellCommand(
   signo_ptr, command_output, timeout);
 }
 
-bool PlatformRemoteGDBServer::CalculateMD5(const FileSpec _spec,
-   uint64_t , uint64_t ) {
+llvm::ErrorOr
+PlatformRemoteGDBServer::CalculateMD5(const FileSpec _spec) {
   if (!IsConnected())
-return false;
+return std::make_error_code(std::errc::not_connected);
 
-  return m_gdb_client_up->CalculateMD5(file_spec, low, high);
+  return m_gdb_client_up->CalculateMD5(file_spec);
 }
 
 void 

[Lldb-commits] [lldb] [lldb] Unify CalculateMD5 return types (PR #91029)

2024-05-03 Thread Anthony Ha via lldb-commits

Awfa wrote:

@JDevlieghere , do you know if there's a way to run buildbot on a merge of this 
PR and main branch - just to validate the build/tests work before this merge?

The [llvm contributing 
guide](https://llvm.org/docs/MyFirstTypoFix.html#myfirsttypofix-issues-after-landing-your-pr)
 says this is normal workflow - but I worry since I am using a lot of the 
maintainers' time to merge / revert the changes for me since I don't have write 
access.

https://github.com/llvm/llvm-project/pull/91029
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Unify CalculateMD5 return types (PR #91029)

2024-05-03 Thread Anthony Ha via lldb-commits

https://github.com/Awfa created https://github.com/llvm/llvm-project/pull/91029

This is a retake of https://github.com/llvm/llvm-project/pull/90921 which got 
reverted because I forgot to modify the CalculateMD5 unit test I had added in 
https://github.com/llvm/llvm-project/pull/88812

The prior failing build is here: 
https://lab.llvm.org/buildbot/#/builders/68/builds/73622
To make sure this error doesn't happen, I ran `ninja ProcessGdbRemoteTests` and 
then executed the resulting test binary and observed the `CalculateMD5` test 
passed.

# Overview
In my previous PR: https://github.com/llvm/llvm-project/pull/88812, 
@JDevlieghere suggested to match return types of the various calculate md5 
functions.

This PR achieves that by changing the various calculate md5 functions to return 
`llvm::ErrorOr`.
 
The suggestion was to go for `std::optional<>` but I opted for 
`llvm::ErrorOr<>` because local calculate md5 was already possibly returning 
`ErrorOr`.

To make sure I didn't break the md5 calculation functionality, I ran some tests 
for the gdb remote client, and things seem to work.

# Testing
1. Remote file doesn't exist
![image](https://github.com/llvm/llvm-project/assets/1326275/b26859e2-18c3-4685-be8f-c6b6a5a4bc77)

1. Remote file differs
![image](https://github.com/llvm/llvm-project/assets/1326275/cbdb3c58-555a-401b-9444-c5ff4c04c491)

1. Remote file matches
![image](https://github.com/llvm/llvm-project/assets/1326275/07561572-22d1-4e0a-988f-bc91b5c2ffce)

## Test gaps
Unfortunately, I had to modify 
`lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp` and I can't test 
the changes there. Hopefully, the existing test suite / code review from 
whomever is reading this will catch any issues.

>From 3112e3da309a24001aa610b15f2a23892b660fc8 Mon Sep 17 00:00:00 2001
From: Anthony Ha 
Date: Thu, 2 May 2024 23:55:47 +
Subject: [PATCH 1/2] Unify CalculateMD5 return types

---
 lldb/include/lldb/Target/Platform.h   |  4 +--
 .../include/lldb/Target/RemoteAwarePlatform.h |  4 +--
 .../Platform/MacOSX/PlatformDarwinDevice.cpp  | 16 +
 .../gdb-server/PlatformRemoteGDBServer.cpp|  8 ++---
 .../gdb-server/PlatformRemoteGDBServer.h  |  4 +--
 .../GDBRemoteCommunicationClient.cpp  | 30 ++--
 .../gdb-remote/GDBRemoteCommunicationClient.h |  2 +-
 lldb/source/Target/Platform.cpp   | 36 +--
 lldb/source/Target/RemoteAwarePlatform.cpp|  8 ++---
 9 files changed, 60 insertions(+), 52 deletions(-)

diff --git a/lldb/include/lldb/Target/Platform.h 
b/lldb/include/lldb/Target/Platform.h
index ad9c9dcbe684ba..e05c79cb501bf0 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -649,8 +649,8 @@ class Platform : public PluginInterface {
 
   virtual std::string GetPlatformSpecificConnectionInformation() { return ""; }
 
-  virtual bool CalculateMD5(const FileSpec _spec, uint64_t ,
-uint64_t );
+  virtual llvm::ErrorOr
+  CalculateMD5(const FileSpec _spec);
 
   virtual uint32_t GetResumeCountForLaunchInfo(ProcessLaunchInfo _info) 
{
 return 1;
diff --git a/lldb/include/lldb/Target/RemoteAwarePlatform.h 
b/lldb/include/lldb/Target/RemoteAwarePlatform.h
index d183815e1c8b07..0b9d79f9ff0380 100644
--- a/lldb/include/lldb/Target/RemoteAwarePlatform.h
+++ b/lldb/include/lldb/Target/RemoteAwarePlatform.h
@@ -58,8 +58,8 @@ class RemoteAwarePlatform : public Platform {
   Status SetFilePermissions(const FileSpec _spec,
 uint32_t file_permissions) override;
 
-  bool CalculateMD5(const FileSpec _spec, uint64_t ,
-uint64_t ) override;
+  llvm::ErrorOr
+  CalculateMD5(const FileSpec _spec) override;
 
   Status GetFileWithUUID(const FileSpec _file, const UUID *uuid,
  FileSpec _file) override;
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp 
b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
index 52777909a1f825..82156aca8cf159 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
@@ -405,17 +405,21 @@ lldb_private::Status 
PlatformDarwinDevice::GetSharedModuleWithLocalCache(
   // when going over the *slow* GDB remote transfer mechanism we first
   // check the hashes of the files - and only do the actual transfer if
   // they differ
-  uint64_t high_local, high_remote, low_local, low_remote;
   auto MD5 = llvm::sys::fs::md5_contents(module_cache_spec.GetPath());
   if (!MD5)
 return Status(MD5.getError());
-  std::tie(high_local, low_local) = MD5->words();
 
-  m_remote_platform_sp->CalculateMD5(module_spec.GetFileSpec(),
- low_remote, high_remote);
-  if (low_local != low_remote || high_local != high_remote) {
+  Log *log = 

[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

2024-05-03 Thread Greg Clayton via lldb-commits

clayborg wrote:

> The tests your described testing this change doesn't break things by delaying 
> definition DIE searching, which I think is already covered by existing tests 
> (created for other purposes, but also covers this case). I was thinking about 
> testing the definition DIE searching is actually delayed. Maybe there isn't a 
> way to do it.

You could enable logging and check for specific logging after steps. In the 
test I described above if you just print the "Foo *foo" variable, it won't need 
to complete the definition, you could check for logging, and then if you print 
"*foo", then it should complete the definition and you should see logging. Not 
sure if that is what you meant

https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] Fix dap variable value format issue (PR #90799)

2024-05-03 Thread Walter Erquinigo via lldb-commits

walter-erquinigo wrote:

I thought you were meaning UI changes via the fblldb extension, but if you are 
modying your fork of VSCode, there's nothing to be done then.

https://github.com/llvm/llvm-project/pull/90799
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] fix step in AArch64 trampoline (PR #90783)

2024-05-03 Thread Vincent Belliard via lldb-commits


@@ -506,9 +506,29 @@ 
DynamicLoaderPOSIXDYLD::GetStepThroughTrampolinePlan(Thread ,
   Target  = thread.GetProcess()->GetTarget();
   const ModuleList  = target.GetImages();
 
-  images.FindSymbolsWithNameAndType(sym_name, eSymbolTypeCode, target_symbols);
-  if (!target_symbols.GetSize())
-return thread_plan_sp;
+  llvm::StringRef target_name = sym_name.GetStringRef();
+  // On AArch64, the trampoline name has a prefix (__AArch64ADRPThunk_ or
+  // __AArch64AbsLongThunk_) added to the function name. If we detect a
+  // trampoline with the prefix, we need to remove the prefix to find the
+  // function symbol.
+  if (target_name.consume_front("__AArch64ADRPThunk_")) {

v-bulle wrote:

done

https://github.com/llvm/llvm-project/pull/90783
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] fix step in AArch64 trampoline (PR #90783)

2024-05-03 Thread Vincent Belliard via lldb-commits

https://github.com/v-bulle updated 
https://github.com/llvm/llvm-project/pull/90783

>From 12464941c1b11ffad0ff2566642df3d30976a3f9 Mon Sep 17 00:00:00 2001
From: Vincent Belliard 
Date: Thu, 18 Apr 2024 10:39:59 -0700
Subject: [PATCH 1/3] [lldb] fix step in AArch64 trampoline

---
 .../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | 26 ---
 .../Plugins/ObjectFile/ELF/ObjectFileELF.cpp  | 19 +-
 .../StepIn/Inputs/aarch64_thunk.cc| 15 +++
 .../StepIn/step_through-aarch64-thunk.test| 17 
 4 files changed, 73 insertions(+), 4 deletions(-)
 create mode 100644 lldb/test/Shell/ExecControl/StepIn/Inputs/aarch64_thunk.cc
 create mode 100644 
lldb/test/Shell/ExecControl/StepIn/step_through-aarch64-thunk.test

diff --git 
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp 
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 9fa245fc41d40c..232030268e42c8 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -506,9 +506,29 @@ 
DynamicLoaderPOSIXDYLD::GetStepThroughTrampolinePlan(Thread ,
   Target  = thread.GetProcess()->GetTarget();
   const ModuleList  = target.GetImages();
 
-  images.FindSymbolsWithNameAndType(sym_name, eSymbolTypeCode, target_symbols);
-  if (!target_symbols.GetSize())
-return thread_plan_sp;
+  llvm::StringRef target_name = sym_name.GetStringRef();
+  // On AArch64, the trampoline name has a prefix (__AArch64ADRPThunk_ or
+  // __AArch64AbsLongThunk_) added to the function name. If we detect a
+  // trampoline with the prefix, we need to remove the prefix to find the
+  // function symbol.
+  if (target_name.consume_front("__AArch64ADRPThunk_")) {
+// An empty target name can happen when for trampolines generated for
+// section-referencing relocations.
+if (!target_name.empty()) {
+  images.FindSymbolsWithNameAndType(ConstString(target_name),
+eSymbolTypeCode, target_symbols);
+}
+  } else if (target_name.consume_front("__AArch64AbsLongThunk_")) {
+// An empty target name can happen when for trampolines generated for
+// section-referencing relocations.
+if (!target_name.empty()) {
+  images.FindSymbolsWithNameAndType(ConstString(target_name),
+eSymbolTypeCode, target_symbols);
+}
+  } else {
+images.FindSymbolsWithNameAndType(sym_name, eSymbolTypeCode,
+  target_symbols);
+  }
 
   typedef std::vector AddressVector;
   AddressVector addrs;
diff --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp 
b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
index 16f6d2e884b577..1646ee9aa34a61 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2356,13 +2356,30 @@ unsigned ObjectFileELF::ParseSymbols(Symtab *symtab, 
user_id_t start_id,
 bool symbol_size_valid =
 symbol.st_size != 0 || symbol.getType() != STT_FUNC;
 
+bool is_trampoline = false;
+if (arch.IsValid() && (arch.GetMachine() == llvm::Triple::aarch64)) {
+  // On AArch64, trampolines are registered as code.
+  // If we detect a trampoline (which starts with __AArch64ADRPThunk_ or
+  // __AArch64AbsLongThunk_) we register the symbol as a trampoline. This
+  // way we will be able to detect the trampoline when we step in a 
function
+  // and step through the trampoline.
+  if (symbol_type == eSymbolTypeCode) {
+llvm::StringRef trampoline_name = mangled.GetName().GetStringRef();
+if (trampoline_name.starts_with("__AArch64ADRPThunk_") ||
+trampoline_name.starts_with("__AArch64AbsLongThunk_")) {
+  symbol_type = eSymbolTypeTrampoline;
+  is_trampoline = true;
+}
+  }
+}
+
 Symbol dc_symbol(
 i + start_id, // ID is the original symbol table index.
 mangled,
 symbol_type,// Type of this symbol
 is_global,  // Is this globally visible?
 false,  // Is this symbol debug info?
-false,  // Is this symbol a trampoline?
+is_trampoline,  // Is this symbol a trampoline?
 false,  // Is this symbol artificial?
 AddressRange(symbol_section_sp, // Section in which this symbol is
 // defined or null.
diff --git a/lldb/test/Shell/ExecControl/StepIn/Inputs/aarch64_thunk.cc 
b/lldb/test/Shell/ExecControl/StepIn/Inputs/aarch64_thunk.cc
new file mode 100644
index 00..02f3bef32a59a3
--- /dev/null
+++ b/lldb/test/Shell/ExecControl/StepIn/Inputs/aarch64_thunk.cc
@@ -0,0 +1,15 @@
+extern "C" int __attribute__((naked)) __AArch64ADRPThunk_step_here() 

[Lldb-commits] [lldb] [lldb] fix step in AArch64 trampoline (PR #90783)

2024-05-03 Thread Vincent Belliard via lldb-commits

https://github.com/v-bulle updated 
https://github.com/llvm/llvm-project/pull/90783

>From 12464941c1b11ffad0ff2566642df3d30976a3f9 Mon Sep 17 00:00:00 2001
From: Vincent Belliard 
Date: Thu, 18 Apr 2024 10:39:59 -0700
Subject: [PATCH 1/2] [lldb] fix step in AArch64 trampoline

---
 .../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | 26 ---
 .../Plugins/ObjectFile/ELF/ObjectFileELF.cpp  | 19 +-
 .../StepIn/Inputs/aarch64_thunk.cc| 15 +++
 .../StepIn/step_through-aarch64-thunk.test| 17 
 4 files changed, 73 insertions(+), 4 deletions(-)
 create mode 100644 lldb/test/Shell/ExecControl/StepIn/Inputs/aarch64_thunk.cc
 create mode 100644 
lldb/test/Shell/ExecControl/StepIn/step_through-aarch64-thunk.test

diff --git 
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp 
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 9fa245fc41d40c..232030268e42c8 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -506,9 +506,29 @@ 
DynamicLoaderPOSIXDYLD::GetStepThroughTrampolinePlan(Thread ,
   Target  = thread.GetProcess()->GetTarget();
   const ModuleList  = target.GetImages();
 
-  images.FindSymbolsWithNameAndType(sym_name, eSymbolTypeCode, target_symbols);
-  if (!target_symbols.GetSize())
-return thread_plan_sp;
+  llvm::StringRef target_name = sym_name.GetStringRef();
+  // On AArch64, the trampoline name has a prefix (__AArch64ADRPThunk_ or
+  // __AArch64AbsLongThunk_) added to the function name. If we detect a
+  // trampoline with the prefix, we need to remove the prefix to find the
+  // function symbol.
+  if (target_name.consume_front("__AArch64ADRPThunk_")) {
+// An empty target name can happen when for trampolines generated for
+// section-referencing relocations.
+if (!target_name.empty()) {
+  images.FindSymbolsWithNameAndType(ConstString(target_name),
+eSymbolTypeCode, target_symbols);
+}
+  } else if (target_name.consume_front("__AArch64AbsLongThunk_")) {
+// An empty target name can happen when for trampolines generated for
+// section-referencing relocations.
+if (!target_name.empty()) {
+  images.FindSymbolsWithNameAndType(ConstString(target_name),
+eSymbolTypeCode, target_symbols);
+}
+  } else {
+images.FindSymbolsWithNameAndType(sym_name, eSymbolTypeCode,
+  target_symbols);
+  }
 
   typedef std::vector AddressVector;
   AddressVector addrs;
diff --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp 
b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
index 16f6d2e884b577..1646ee9aa34a61 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2356,13 +2356,30 @@ unsigned ObjectFileELF::ParseSymbols(Symtab *symtab, 
user_id_t start_id,
 bool symbol_size_valid =
 symbol.st_size != 0 || symbol.getType() != STT_FUNC;
 
+bool is_trampoline = false;
+if (arch.IsValid() && (arch.GetMachine() == llvm::Triple::aarch64)) {
+  // On AArch64, trampolines are registered as code.
+  // If we detect a trampoline (which starts with __AArch64ADRPThunk_ or
+  // __AArch64AbsLongThunk_) we register the symbol as a trampoline. This
+  // way we will be able to detect the trampoline when we step in a 
function
+  // and step through the trampoline.
+  if (symbol_type == eSymbolTypeCode) {
+llvm::StringRef trampoline_name = mangled.GetName().GetStringRef();
+if (trampoline_name.starts_with("__AArch64ADRPThunk_") ||
+trampoline_name.starts_with("__AArch64AbsLongThunk_")) {
+  symbol_type = eSymbolTypeTrampoline;
+  is_trampoline = true;
+}
+  }
+}
+
 Symbol dc_symbol(
 i + start_id, // ID is the original symbol table index.
 mangled,
 symbol_type,// Type of this symbol
 is_global,  // Is this globally visible?
 false,  // Is this symbol debug info?
-false,  // Is this symbol a trampoline?
+is_trampoline,  // Is this symbol a trampoline?
 false,  // Is this symbol artificial?
 AddressRange(symbol_section_sp, // Section in which this symbol is
 // defined or null.
diff --git a/lldb/test/Shell/ExecControl/StepIn/Inputs/aarch64_thunk.cc 
b/lldb/test/Shell/ExecControl/StepIn/Inputs/aarch64_thunk.cc
new file mode 100644
index 00..02f3bef32a59a3
--- /dev/null
+++ b/lldb/test/Shell/ExecControl/StepIn/Inputs/aarch64_thunk.cc
@@ -0,0 +1,15 @@
+extern "C" int __attribute__((naked)) __AArch64ADRPThunk_step_here() 

[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

2024-05-03 Thread Zequan Wu via lldb-commits

https://github.com/ZequanWu updated 
https://github.com/llvm/llvm-project/pull/90663

>From 4e83099b593e66f12dc21be5fbac5279e03e Mon Sep 17 00:00:00 2001
From: Zequan Wu 
Date: Tue, 30 Apr 2024 16:23:11 -0400
Subject: [PATCH 1/5] [lldb][DWARF] Delay struct/class/union definition DIE
 searching when parsing declaration DIEs.

---
 .../SymbolFile/DWARF/DWARFASTParserClang.cpp  | 270 +++---
 .../SymbolFile/DWARF/SymbolFileDWARF.cpp  | 105 ++-
 .../SymbolFile/DWARF/SymbolFileDWARF.h|  14 +
 .../SymbolFile/DWARF/SymbolFileDWARFDwo.cpp   |   5 +
 .../SymbolFile/DWARF/SymbolFileDWARFDwo.h |   2 +
 .../SymbolFile/DWARF/UniqueDWARFASTType.cpp   |  95 +++---
 .../SymbolFile/DWARF/UniqueDWARFASTType.h |  21 +-
 7 files changed, 296 insertions(+), 216 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index bea11e0e3840af..7ad661c9a9d49b 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -252,7 +252,8 @@ static void ForcefullyCompleteType(CompilerType type) {
 static void PrepareContextToReceiveMembers(TypeSystemClang ,
ClangASTImporter _importer,
clang::DeclContext *decl_ctx,
-   DWARFDIE die,
+   const DWARFDIE _ctx_die,
+   const DWARFDIE ,
const char *type_name_cstr) {
   auto *tag_decl_ctx = clang::dyn_cast(decl_ctx);
   if (!tag_decl_ctx)
@@ -279,6 +280,13 @@ static void PrepareContextToReceiveMembers(TypeSystemClang 
,
 type_name_cstr ? type_name_cstr : "", die.GetOffset());
   }
 
+  // By searching for the definition DIE of the decl_ctx type, we will either:
+  // 1. Found the the definition DIE and start its definition with
+  // TypeSystemClang::StartTagDeclarationDefinition.
+  // 2. Unable to find it, then need to forcefully complete it.
+  die.GetDWARF()->FindDefinitionDIE(decl_ctx_die);
+  if (tag_decl_ctx->isCompleteDefinition() || tag_decl_ctx->isBeingDefined())
+return;
   // We don't have a type definition and/or the import failed. We must
   // forcefully complete the type to avoid crashes.
   ForcefullyCompleteType(type);
@@ -620,10 +628,11 @@ DWARFASTParserClang::ParseTypeModifier(const 
SymbolContext ,
   if (tag == DW_TAG_typedef) {
 // DeclContext will be populated when the clang type is materialized in
 // Type::ResolveCompilerType.
-PrepareContextToReceiveMembers(
-m_ast, GetClangASTImporter(),
-GetClangDeclContextContainingDIE(die, nullptr), die,
-attrs.name.GetCString());
+DWARFDIE decl_ctx_die;
+clang::DeclContext *decl_ctx =
+GetClangDeclContextContainingDIE(die, _ctx_die);
+PrepareContextToReceiveMembers(m_ast, GetClangASTImporter(), decl_ctx,
+   decl_ctx_die, die, attrs.name.GetCString());
 
 if (attrs.type.IsValid()) {
   // Try to parse a typedef from the (DWARF embedded in the) Clang
@@ -1100,32 +1109,6 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE ,
 // struct and see if this is actually a C++ method
 Type *class_type = dwarf->ResolveType(decl_ctx_die);
 if (class_type) {
-  if (class_type->GetID() != decl_ctx_die.GetID() ||
-  IsClangModuleFwdDecl(decl_ctx_die)) {
-
-// We uniqued the parent class of this function to another
-// class so we now need to associate all dies under
-// "decl_ctx_die" to DIEs in the DIE for "class_type"...
-DWARFDIE class_type_die = dwarf->GetDIE(class_type->GetID());
-
-if (class_type_die) {
-  std::vector failures;
-
-  CopyUniqueClassMethodTypes(decl_ctx_die, class_type_die,
- class_type, failures);
-
-  // FIXME do something with these failures that's
-  // smarter than just dropping them on the ground.
-  // Unfortunately classes don't like having stuff added
-  // to them after their definitions are complete...
-
-  Type *type_ptr = dwarf->GetDIEToType()[die.GetDIE()];
-  if (type_ptr && type_ptr != DIE_IS_BEING_PARSED) {
-return type_ptr->shared_from_this();
-  }
-}
-  }
-
   if (attrs.specification.IsValid()) {
 // We have a specification which we are going to base our
 // function prototype off of, so we need this type to be
@@ -1260,6 +1243,39 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE ,
   }
 }
   }
+  // By here, we should have already completed the c++ class_type
+  // 

[Lldb-commits] [lldb] [lldb][NFCI] Remove unused DWARF value-to-name functions (PR #91010)

2024-05-03 Thread Alex Langford via lldb-commits

https://github.com/bulbazord created 
https://github.com/llvm/llvm-project/pull/91010

I was cleaning up this portion of the code and realized these are completely 
unused.

>From be459e6a4e611650094ed9d362bf84acb9aabe16 Mon Sep 17 00:00:00 2001
From: Alex Langford 
Date: Fri, 3 May 2024 13:35:19 -0700
Subject: [PATCH] [lldb][NFCI] Remove unused DWARF value-to-name functions

I was cleaning up this portion of the code and realized these are
completely unused.
---
 .../Plugins/SymbolFile/DWARF/DWARFDefines.cpp | 50 ---
 .../Plugins/SymbolFile/DWARF/DWARFDefines.h   | 12 -
 2 files changed, 62 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDefines.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDefines.cpp
index 1f0e10ef27018b..2fb0c224bf8e84 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDefines.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDefines.cpp
@@ -23,26 +23,6 @@ llvm::StringRef DW_TAG_value_to_name(dw_tag_t tag) {
   return s_unknown_tag_name;
 }
 
-const char *DW_AT_value_to_name(uint32_t val) {
-  static char invalid[100];
-  llvm::StringRef llvmstr = llvm::dwarf::AttributeString(val);
-  if (llvmstr.empty()) {
-snprintf(invalid, sizeof(invalid), "Unknown DW_AT constant: 0x%x", val);
-return invalid;
-  }
-  return llvmstr.data();
-}
-
-const char *DW_FORM_value_to_name(uint32_t val) {
-  static char invalid[100];
-  llvm::StringRef llvmstr = llvm::dwarf::FormEncodingString(val);
-  if (llvmstr.empty()) {
-snprintf(invalid, sizeof(invalid), "Unknown DW_FORM constant: 0x%x", val);
-return invalid;
-  }
-  return llvmstr.data();
-}
-
 const char *DW_OP_value_to_name(uint32_t val) {
   static char invalid[100];
   llvm::StringRef llvmstr = llvm::dwarf::OperationEncodingString(val);
@@ -53,35 +33,5 @@ const char *DW_OP_value_to_name(uint32_t val) {
   return llvmstr.data();
 }
 
-const char *DW_ATE_value_to_name(uint32_t val) {
-  static char invalid[100];
-  llvm::StringRef llvmstr = llvm::dwarf::AttributeEncodingString(val);
-  if (llvmstr.empty()) {
-snprintf(invalid, sizeof(invalid), "Unknown DW_ATE constant: 0x%x", val);
-return invalid;
-  }
-  return llvmstr.data();
-}
-
-const char *DW_LANG_value_to_name(uint32_t val) {
-  static char invalid[100];
-  llvm::StringRef llvmstr = llvm::dwarf::LanguageString(val);
-  if (llvmstr.empty()) {
-snprintf(invalid, sizeof(invalid), "Unknown DW_LANG constant: 0x%x", val);
-return invalid;
-  }
-  return llvmstr.data();
-}
-
-const char *DW_LNS_value_to_name(uint32_t val) {
-  static char invalid[100];
-  llvm::StringRef llvmstr = llvm::dwarf::LNStandardString(val);
-  if (llvmstr.empty()) {
-snprintf(invalid, sizeof(invalid), "Unknown DW_LNS constant: 0x%x", val);
-return invalid;
-  }
-  return llvmstr.data();
-}
-
 } // namespace dwarf
 } // namespace lldb_private::plugin
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDefines.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDefines.h
index e87ac458fe962c..be81cb0f5df1ea 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDefines.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDefines.h
@@ -15,22 +15,10 @@
 namespace lldb_private::plugin {
 namespace dwarf {
 
-typedef uint32_t DRC_class; // Holds DRC_* class bitfields
-
 llvm::StringRef DW_TAG_value_to_name(dw_tag_t tag);
 
-const char *DW_AT_value_to_name(uint32_t val);
-
-const char *DW_FORM_value_to_name(uint32_t val);
-
 const char *DW_OP_value_to_name(uint32_t val);
 
-const char *DW_ATE_value_to_name(uint32_t val);
-
-const char *DW_LANG_value_to_name(uint32_t val);
-
-const char *DW_LNS_value_to_name(uint32_t val);
-
 } // namespace dwarf
 } // namespace lldb_private::plugin
 

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


[Lldb-commits] [lldb] Fix dap variable value format issue (PR #90799)

2024-05-03 Thread via lldb-commits

https://github.com/jeffreytan81 closed 
https://github.com/llvm/llvm-project/pull/90799
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] b8d38bb - Fix dap variable value format issue (#90799)

2024-05-03 Thread via lldb-commits

Author: jeffreytan81
Date: 2024-05-03T13:36:23-07:00
New Revision: b8d38bb56d59bee39872fee348a07f79c12f51ae

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

LOG: Fix dap variable value format issue (#90799)

While adding a UI feature in VSCode to toggle hex/dec in variables view
window. I noticed that it does not work after second toggle. Then I
noticed that there is a bug that we only explicitly set hex format not
reset back to default during further toggle. The new test demonstrates
the bug.

This PR resets the format back to default if not using hex. One
complexity is that, we explicitly set registers value format to
AddressInfo, which shouldn't be overridden by default or hex settings.

-

Co-authored-by: jeffreytan81 

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
lldb/tools/lldb-dap/JSONUtils.cpp

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index 5838281bcb1a10f..e2126d67a5fe778 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -448,7 +448,7 @@ def get_completions(self, text, frameId=None):
 response = self.request_completions(text, frameId)
 return response["body"]["targets"]
 
-def get_scope_variables(self, scope_name, frameIndex=0, threadId=None):
+def get_scope_variables(self, scope_name, frameIndex=0, threadId=None, 
is_hex=None):
 stackFrame = self.get_stackFrame(frameIndex=frameIndex, 
threadId=threadId)
 if stackFrame is None:
 return []
@@ -462,7 +462,7 @@ def get_scope_variables(self, scope_name, frameIndex=0, 
threadId=None):
 for scope in frame_scopes:
 if scope["name"] == scope_name:
 varRef = scope["variablesReference"]
-variables_response = self.request_variables(varRef)
+variables_response = self.request_variables(varRef, 
is_hex=is_hex)
 if variables_response:
 if "body" in variables_response:
 body = variables_response["body"]
@@ -476,9 +476,9 @@ def get_global_variables(self, frameIndex=0, threadId=None):
 "Globals", frameIndex=frameIndex, threadId=threadId
 )
 
-def get_local_variables(self, frameIndex=0, threadId=None):
+def get_local_variables(self, frameIndex=0, threadId=None, is_hex=None):
 return self.get_scope_variables(
-"Locals", frameIndex=frameIndex, threadId=threadId
+"Locals", frameIndex=frameIndex, threadId=threadId, is_hex=is_hex
 )
 
 def get_registers(self, frameIndex=0, threadId=None):
@@ -486,28 +486,32 @@ def get_registers(self, frameIndex=0, threadId=None):
 "Registers", frameIndex=frameIndex, threadId=threadId
 )
 
-def get_local_variable(self, name, frameIndex=0, threadId=None):
-locals = self.get_local_variables(frameIndex=frameIndex, 
threadId=threadId)
+def get_local_variable(self, name, frameIndex=0, threadId=None, 
is_hex=None):
+locals = self.get_local_variables(
+frameIndex=frameIndex, threadId=threadId, is_hex=is_hex
+)
 for local in locals:
 if "name" in local and local["name"] == name:
 return local
 return None
 
-def get_local_variable_value(self, name, frameIndex=0, threadId=None):
+def get_local_variable_value(self, name, frameIndex=0, threadId=None, 
is_hex=None):
 variable = self.get_local_variable(
-name, frameIndex=frameIndex, threadId=threadId
+name, frameIndex=frameIndex, threadId=threadId, is_hex=is_hex
 )
 if variable and "value" in variable:
 return variable["value"]
 return None
 
-def get_local_variable_child(self, name, child_name, frameIndex=0, 
threadId=None):
+def get_local_variable_child(
+self, name, child_name, frameIndex=0, threadId=None, is_hex=None
+):
 local = self.get_local_variable(name, frameIndex, threadId)
 if local["variablesReference"] == 0:
 return None
-children = self.request_variables(local["variablesReference"])["body"][
-"variables"
-]
+children = self.request_variables(local["variablesReference"], 
is_hex=is_hex)[
+"body"
+]["variables"]
 for child in children:
 if child["name"] == child_name:
 return child
@@ -1035,12 +1039,16 @@ def request_threads(self):
 self.threads = 

[Lldb-commits] [lldb] Fix dap variable value format issue (PR #90799)

2024-05-03 Thread via lldb-commits

jeffreytan81 wrote:

> lgtm. It would be nice if new UI features could be added in the typescript 
> code of lldb-dap, so that all users benefit from them.

@walter-erquinigo , do you mean upstream to VSCode? Yeah, we would like to 
upstream the changes if Microsoft is willing to take them. 

https://github.com/llvm/llvm-project/pull/90799
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] Revert "[lldb] Unify CalculateMD5 return types" (PR #90998)

2024-05-03 Thread Jonas Devlieghere via lldb-commits

https://github.com/JDevlieghere closed 
https://github.com/llvm/llvm-project/pull/90998
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] ca8b064 - Revert "[lldb] Unify CalculateMD5 return types" (#90998)

2024-05-03 Thread via lldb-commits

Author: Jonas Devlieghere
Date: 2024-05-03T12:14:45-07:00
New Revision: ca8b064973b5bf31168a60b41ee9c071cf321777

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

LOG: Revert "[lldb] Unify CalculateMD5 return types" (#90998)

Reverts llvm/llvm-project#90921

Added: 


Modified: 
lldb/include/lldb/Target/Platform.h
lldb/include/lldb/Target/RemoteAwarePlatform.h
lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/source/Target/Platform.cpp
lldb/source/Target/RemoteAwarePlatform.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Platform.h 
b/lldb/include/lldb/Target/Platform.h
index e05c79cb501bf0..ad9c9dcbe684ba 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -649,8 +649,8 @@ class Platform : public PluginInterface {
 
   virtual std::string GetPlatformSpecificConnectionInformation() { return ""; }
 
-  virtual llvm::ErrorOr
-  CalculateMD5(const FileSpec _spec);
+  virtual bool CalculateMD5(const FileSpec _spec, uint64_t ,
+uint64_t );
 
   virtual uint32_t GetResumeCountForLaunchInfo(ProcessLaunchInfo _info) 
{
 return 1;

diff  --git a/lldb/include/lldb/Target/RemoteAwarePlatform.h 
b/lldb/include/lldb/Target/RemoteAwarePlatform.h
index 0b9d79f9ff0380..d183815e1c8b07 100644
--- a/lldb/include/lldb/Target/RemoteAwarePlatform.h
+++ b/lldb/include/lldb/Target/RemoteAwarePlatform.h
@@ -58,8 +58,8 @@ class RemoteAwarePlatform : public Platform {
   Status SetFilePermissions(const FileSpec _spec,
 uint32_t file_permissions) override;
 
-  llvm::ErrorOr
-  CalculateMD5(const FileSpec _spec) override;
+  bool CalculateMD5(const FileSpec _spec, uint64_t ,
+uint64_t ) override;
 
   Status GetFileWithUUID(const FileSpec _file, const UUID *uuid,
  FileSpec _file) override;

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp 
b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
index 82156aca8cf159..52777909a1f825 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
@@ -405,21 +405,17 @@ lldb_private::Status 
PlatformDarwinDevice::GetSharedModuleWithLocalCache(
   // when going over the *slow* GDB remote transfer mechanism we first
   // check the hashes of the files - and only do the actual transfer if
   // they 
diff er
+  uint64_t high_local, high_remote, low_local, low_remote;
   auto MD5 = llvm::sys::fs::md5_contents(module_cache_spec.GetPath());
   if (!MD5)
 return Status(MD5.getError());
+  std::tie(high_local, low_local) = MD5->words();
 
-  Log *log = GetLog(LLDBLog::Platform);
-  bool requires_transfer = true;
-  llvm::ErrorOr remote_md5 =
-  m_remote_platform_sp->CalculateMD5(module_spec.GetFileSpec());
-  if (std::error_code ec = remote_md5.getError())
-LLDB_LOG(log, "couldn't get md5 sum from remote: {0}",
- ec.message());
-  else
-requires_transfer = *MD5 != *remote_md5;
-  if (requires_transfer) {
+  m_remote_platform_sp->CalculateMD5(module_spec.GetFileSpec(),
+ low_remote, high_remote);
+  if (low_local != low_remote || high_local != high_remote) {
 // bring in the remote file
+Log *log = GetLog(LLDBLog::Platform);
 LLDB_LOGF(log,
   "[%s] module %s/%s needs to be replaced from remote 
copy",
   (IsHost() ? "host" : "remote"),

diff  --git 
a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp 
b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
index 4684947ede209f..0dce5add2e3759 100644
--- a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
+++ b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
@@ -684,12 +684,12 @@ Status PlatformRemoteGDBServer::RunShellCommand(
   signo_ptr, command_output, timeout);
 }
 
-llvm::ErrorOr
-PlatformRemoteGDBServer::CalculateMD5(const FileSpec _spec) {
+bool PlatformRemoteGDBServer::CalculateMD5(const FileSpec _spec,
+   uint64_t , uint64_t ) {
   if (!IsConnected())
-return 

[Lldb-commits] [lldb] Revert "[lldb] Unify CalculateMD5 return types" (PR #90998)

2024-05-03 Thread Jonas Devlieghere via lldb-commits

https://github.com/JDevlieghere created 
https://github.com/llvm/llvm-project/pull/90998

Reverts llvm/llvm-project#90921

>From 2beb507e10a6dbe6387bc67143601a66b0168293 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Fri, 3 May 2024 12:14:36 -0700
Subject: [PATCH] Revert "[lldb] Unify CalculateMD5 return types (#90921)"

This reverts commit 2f58b9aae2d6f1aeaecd98766ef31cebc0dcbb5b.
---
 lldb/include/lldb/Target/Platform.h   |  4 +--
 .../include/lldb/Target/RemoteAwarePlatform.h |  4 +--
 .../Platform/MacOSX/PlatformDarwinDevice.cpp  | 16 -
 .../gdb-server/PlatformRemoteGDBServer.cpp|  8 ++---
 .../gdb-server/PlatformRemoteGDBServer.h  |  4 +--
 .../GDBRemoteCommunicationClient.cpp  | 30 ++--
 .../gdb-remote/GDBRemoteCommunicationClient.h |  2 +-
 lldb/source/Target/Platform.cpp   | 36 ++-
 lldb/source/Target/RemoteAwarePlatform.cpp|  8 ++---
 9 files changed, 52 insertions(+), 60 deletions(-)

diff --git a/lldb/include/lldb/Target/Platform.h 
b/lldb/include/lldb/Target/Platform.h
index e05c79cb501bf0..ad9c9dcbe684ba 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -649,8 +649,8 @@ class Platform : public PluginInterface {
 
   virtual std::string GetPlatformSpecificConnectionInformation() { return ""; }
 
-  virtual llvm::ErrorOr
-  CalculateMD5(const FileSpec _spec);
+  virtual bool CalculateMD5(const FileSpec _spec, uint64_t ,
+uint64_t );
 
   virtual uint32_t GetResumeCountForLaunchInfo(ProcessLaunchInfo _info) 
{
 return 1;
diff --git a/lldb/include/lldb/Target/RemoteAwarePlatform.h 
b/lldb/include/lldb/Target/RemoteAwarePlatform.h
index 0b9d79f9ff0380..d183815e1c8b07 100644
--- a/lldb/include/lldb/Target/RemoteAwarePlatform.h
+++ b/lldb/include/lldb/Target/RemoteAwarePlatform.h
@@ -58,8 +58,8 @@ class RemoteAwarePlatform : public Platform {
   Status SetFilePermissions(const FileSpec _spec,
 uint32_t file_permissions) override;
 
-  llvm::ErrorOr
-  CalculateMD5(const FileSpec _spec) override;
+  bool CalculateMD5(const FileSpec _spec, uint64_t ,
+uint64_t ) override;
 
   Status GetFileWithUUID(const FileSpec _file, const UUID *uuid,
  FileSpec _file) override;
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp 
b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
index 82156aca8cf159..52777909a1f825 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
@@ -405,21 +405,17 @@ lldb_private::Status 
PlatformDarwinDevice::GetSharedModuleWithLocalCache(
   // when going over the *slow* GDB remote transfer mechanism we first
   // check the hashes of the files - and only do the actual transfer if
   // they differ
+  uint64_t high_local, high_remote, low_local, low_remote;
   auto MD5 = llvm::sys::fs::md5_contents(module_cache_spec.GetPath());
   if (!MD5)
 return Status(MD5.getError());
+  std::tie(high_local, low_local) = MD5->words();
 
-  Log *log = GetLog(LLDBLog::Platform);
-  bool requires_transfer = true;
-  llvm::ErrorOr remote_md5 =
-  m_remote_platform_sp->CalculateMD5(module_spec.GetFileSpec());
-  if (std::error_code ec = remote_md5.getError())
-LLDB_LOG(log, "couldn't get md5 sum from remote: {0}",
- ec.message());
-  else
-requires_transfer = *MD5 != *remote_md5;
-  if (requires_transfer) {
+  m_remote_platform_sp->CalculateMD5(module_spec.GetFileSpec(),
+ low_remote, high_remote);
+  if (low_local != low_remote || high_local != high_remote) {
 // bring in the remote file
+Log *log = GetLog(LLDBLog::Platform);
 LLDB_LOGF(log,
   "[%s] module %s/%s needs to be replaced from remote 
copy",
   (IsHost() ? "host" : "remote"),
diff --git 
a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp 
b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
index 4684947ede209f..0dce5add2e3759 100644
--- a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
+++ b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
@@ -684,12 +684,12 @@ Status PlatformRemoteGDBServer::RunShellCommand(
   signo_ptr, command_output, timeout);
 }
 
-llvm::ErrorOr
-PlatformRemoteGDBServer::CalculateMD5(const FileSpec _spec) {
+bool PlatformRemoteGDBServer::CalculateMD5(const FileSpec _spec,
+   uint64_t , uint64_t ) {
   if (!IsConnected())
-return std::make_error_code(std::errc::not_connected);
+return false;
 
-  return 

[Lldb-commits] [lldb] [lldb] Have lldb-server assign ports to children in platform mode (PR #88845)

2024-05-03 Thread Anthony Ha via lldb-commits

Awfa wrote:

Is there a way to invoke BuildBot on this branch before it's merged in by the 
way?

I got bit on another PR where it caught some failures I wished were caught 
before merge.

https://github.com/llvm/llvm-project/pull/88845
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Unify CalculateMD5 return types (PR #90921)

2024-05-03 Thread Anthony Ha via lldb-commits

Awfa wrote:

@JDevlieghere sorry for the churn - can I get a revert? BuildBot is showing me 
that I didn't check for compile errors in the gdb platform tests.

After revert, I'll go and push a new PR to fix.

https://github.com/llvm/llvm-project/pull/90921
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Have lldb-server assign ports to children in platform mode (PR #88845)

2024-05-03 Thread Anthony Ha via lldb-commits

Awfa wrote:

> > And make the note at the end of lldb/docs/use/qemu-testing.rst outdated.
> 
> Removing this is the last thing to do here.

Fixed! If this looks good to you, would you also be able to merge it? I don't 
have write permissions.

https://github.com/llvm/llvm-project/pull/88845
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Have lldb-server assign ports to children in platform mode (PR #88845)

2024-05-03 Thread Anthony Ha via lldb-commits

https://github.com/Awfa updated https://github.com/llvm/llvm-project/pull/88845

>From 3d75f42b5f61ea126001919491aa09ebd15ba9f8 Mon Sep 17 00:00:00 2001
From: Anthony Ha 
Date: Mon, 15 Apr 2024 19:36:34 +
Subject: [PATCH 1/4] [lldb] Have lldb-server assign ports to children in
 platform mode

---
 lldb/tools/lldb-server/lldb-platform.cpp | 41 
 1 file changed, 28 insertions(+), 13 deletions(-)

diff --git a/lldb/tools/lldb-server/lldb-platform.cpp 
b/lldb/tools/lldb-server/lldb-platform.cpp
index 3e126584eb25b4..384709ba79b656 100644
--- a/lldb/tools/lldb-server/lldb-platform.cpp
+++ b/lldb/tools/lldb-server/lldb-platform.cpp
@@ -282,17 +282,10 @@ int main_platform(int argc, char *argv[]) {
 }
   }
 
-  do {
-GDBRemoteCommunicationServerPlatform platform(
-acceptor_up->GetSocketProtocol(), acceptor_up->GetSocketScheme());
-
-if (port_offset > 0)
-  platform.SetPortOffset(port_offset);
-
-if (!gdbserver_portmap.empty()) {
-  platform.SetPortMap(std::move(gdbserver_portmap));
-}
+  GDBRemoteCommunicationServerPlatform platform(
+  acceptor_up->GetSocketProtocol(), acceptor_up->GetSocketScheme());
 
+  do {
 const bool children_inherit_accept_socket = true;
 Connection *conn = nullptr;
 error = acceptor_up->Accept(children_inherit_accept_socket, conn);
@@ -301,13 +294,31 @@ int main_platform(int argc, char *argv[]) {
   exit(socket_error);
 }
 printf("Connection established.\n");
+
+std::optional port = 0;
 if (g_server) {
   // Collect child zombie processes.
 #if !defined(_WIN32)
-  while (waitpid(-1, nullptr, WNOHANG) > 0)
-;
+  auto waitResult = waitpid(-1, nullptr, WNOHANG);
+  while (waitResult > 0) {
+// waitResult is the child pid
+gdbserver_portmap.FreePortForProcess(waitResult);
+waitResult = waitpid(-1, nullptr, WNOHANG);
+  }
 #endif
-  if (fork()) {
+  llvm::Expected available_port =
+  gdbserver_portmap.GetNextAvailablePort();
+  if (available_port)
+port = *available_port;
+
+  else {
+fprintf(stderr, "no available port for connection - dropping...\n");
+delete conn;
+continue;
+  }
+  auto childPid = fork();
+  if (childPid) {
+gdbserver_portmap.AssociatePortWithProcess(*available_port, childPid);
 // Parent doesn't need a connection to the lldb client
 delete conn;
 
@@ -324,6 +335,10 @@ int main_platform(int argc, char *argv[]) {
   // connections while a connection is active.
   acceptor_up.reset();
 }
+
+GDBRemoteCommunicationServerPlatform::PortMap portmap_for_child;
+portmap_for_child.AllowPort(*port);
+platform.SetPortMap(std::move(portmap_for_child));
 platform.SetConnection(std::unique_ptr(conn));
 
 if (platform.IsConnected()) {

>From 27782fb03b3705e9ff4a5b3cc0d17c8448919be9 Mon Sep 17 00:00:00 2001
From: Anthony Ha 
Date: Wed, 17 Apr 2024 00:46:58 +
Subject: [PATCH 2/4] amend! [lldb] Have lldb-server assign ports to children
 in platform mode

[lldb] Have lldb-server assign ports to children in platform mode
---
 lldb/tools/lldb-server/lldb-platform.cpp | 24 ++--
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/lldb/tools/lldb-server/lldb-platform.cpp 
b/lldb/tools/lldb-server/lldb-platform.cpp
index 384709ba79b656..ff91a3318cf0d5 100644
--- a/lldb/tools/lldb-server/lldb-platform.cpp
+++ b/lldb/tools/lldb-server/lldb-platform.cpp
@@ -295,27 +295,31 @@ int main_platform(int argc, char *argv[]) {
 }
 printf("Connection established.\n");
 
-std::optional port = 0;
 if (g_server) {
   // Collect child zombie processes.
 #if !defined(_WIN32)
-  auto waitResult = waitpid(-1, nullptr, WNOHANG);
-  while (waitResult > 0) {
+  ::pid_t waitResult;
+  while ((waitResult = waitpid(-1, nullptr, WNOHANG)) > 0) {
 // waitResult is the child pid
 gdbserver_portmap.FreePortForProcess(waitResult);
-waitResult = waitpid(-1, nullptr, WNOHANG);
   }
 #endif
+  // TODO: Clean up portmap for Windows when children die
+
+  // After collecting zombie ports, get the next available
+  GDBRemoteCommunicationServerPlatform::PortMap portmap_for_child;
   llvm::Expected available_port =
   gdbserver_portmap.GetNextAvailablePort();
   if (available_port)
-port = *available_port;
-
+portmap_for_child.AllowPort(*available_port);
   else {
-fprintf(stderr, "no available port for connection - dropping...\n");
+fprintf(stderr,
+"no available gdbserver port for connection - dropping...\n");
 delete conn;
 continue;
   }
+  platform.SetPortMap(std::move(portmap_for_child));
+
   auto childPid = fork();
   if (childPid) {
 gdbserver_portmap.AssociatePortWithProcess(*available_port, childPid);
@@ -334,11 +338,11 @@ int 

[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

2024-05-03 Thread Zequan Wu via lldb-commits

ZequanWu wrote:

> > > > > Is any of it testable?
> > > > 
> > > > 
> > > > Good question. Though this is mostly meant to be "NFC" (with very large 
> > > > quotes), I can imagine us doing something like forcing the parsing of a 
> > > > specific type (`type lookup ` ?), and then checking that the 
> > > > module ast (`image dump ast`) does _not_ contain specific types -- as 
> > > > that's basically what we're trying to achieve.
> > > 
> > > 
> > > Yea that could work. But if it's going to be very painful or fragile to 
> > > test then don't let that hold back the PR
> > 
> > 
> > In terms of testing, since this only delays definition DIE searching not 
> > type completion, we need to construct a test so that lldb finds the 
> > declaration DIE first without trigger a type completion on it and somehow 
> > test the incomplete type. The first part is tricky. I'm not sure how to 
> > achieve it.
> 
> You should be able to make a test case with two files. One file contains the 
> class definition and the other uses only a forward declaration. You could 
> test by running the binary stopping only in the one with the forward 
> declaration. So file 1 would be something like `foo.cpp` containing
> 
> ```
> struct Foo { 
>   int value;
>   Foo(int v): value(v) {}
> };
> 
> Foo *CreateFoo() {
>   return new Foo(1);
> }
> ```
> 
> Then having `main.cpp` contain:
> 
> ```
> struct Foo; // Forward declare Foo
> // Prototypes that don't need Foo definition
> Foo *CreateFoo();
> 
> int main() {
>   Foo *foo = CreateFoo();
>   return 0; // Breakpoint 1 here
> }
> ```
> 
> Then run to "Breakpoint 1 here" and then run `frame variable foo`. Foo won't 
> need to be completed for this. But if you then run `frame variable *foo`, it 
> should cause the type to be completed and show the instance variable 
> correctly.

The tests your described testing this change doesn't break things by delaying 
definition DIE searching, which I think is already covered by existing tests 
(created for other purposes, but also covers this case). I was thinking about 
testing the definition DIE searching is actually delayed. Maybe there isn't a 
way to do it.

https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Unify CalculateMD5 return types (PR #90921)

2024-05-03 Thread Jonas Devlieghere via lldb-commits

https://github.com/JDevlieghere closed 
https://github.com/llvm/llvm-project/pull/90921
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 2f58b9a - [lldb] Unify CalculateMD5 return types (#90921)

2024-05-03 Thread via lldb-commits

Author: Anthony Ha
Date: 2024-05-03T11:51:25-07:00
New Revision: 2f58b9aae2d6f1aeaecd98766ef31cebc0dcbb5b

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

LOG: [lldb] Unify CalculateMD5 return types (#90921)

# Overview
In my previous PR: https://github.com/llvm/llvm-project/pull/88812,
@JDevlieghere suggested to match return types of the various calculate
md5 functions.

This PR achieves that by changing the various calculate md5 functions to
return `llvm::ErrorOr`.
 
The suggestion was to go for `std::optional<>` but I opted for
`llvm::ErrorOr<>` because local calculate md5 was already possibly
returning `ErrorOr`.

To make sure I didn't break the md5 calculation functionality, I ran
some tests for the gdb remote client, and things seem to work.

# Testing
1. Remote file doesn't exist

![image](https://github.com/llvm/llvm-project/assets/1326275/b26859e2-18c3-4685-be8f-c6b6a5a4bc77)

1. Remote file differs

![image](https://github.com/llvm/llvm-project/assets/1326275/cbdb3c58-555a-401b-9444-c5ff4c04c491)

1. Remote file matches

![image](https://github.com/llvm/llvm-project/assets/1326275/07561572-22d1-4e0a-988f-bc91b5c2ffce)

## Test gaps
Unfortunately, I had to modify
`lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp` and I
can't test the changes there. Hopefully, the existing test suite / code
review from whomever is reading this will catch any issues.

Co-authored-by: Anthony Ha 

Added: 


Modified: 
lldb/include/lldb/Target/Platform.h
lldb/include/lldb/Target/RemoteAwarePlatform.h
lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/source/Target/Platform.cpp
lldb/source/Target/RemoteAwarePlatform.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Platform.h 
b/lldb/include/lldb/Target/Platform.h
index ad9c9dcbe684ba..e05c79cb501bf0 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -649,8 +649,8 @@ class Platform : public PluginInterface {
 
   virtual std::string GetPlatformSpecificConnectionInformation() { return ""; }
 
-  virtual bool CalculateMD5(const FileSpec _spec, uint64_t ,
-uint64_t );
+  virtual llvm::ErrorOr
+  CalculateMD5(const FileSpec _spec);
 
   virtual uint32_t GetResumeCountForLaunchInfo(ProcessLaunchInfo _info) 
{
 return 1;

diff  --git a/lldb/include/lldb/Target/RemoteAwarePlatform.h 
b/lldb/include/lldb/Target/RemoteAwarePlatform.h
index d183815e1c8b07..0b9d79f9ff0380 100644
--- a/lldb/include/lldb/Target/RemoteAwarePlatform.h
+++ b/lldb/include/lldb/Target/RemoteAwarePlatform.h
@@ -58,8 +58,8 @@ class RemoteAwarePlatform : public Platform {
   Status SetFilePermissions(const FileSpec _spec,
 uint32_t file_permissions) override;
 
-  bool CalculateMD5(const FileSpec _spec, uint64_t ,
-uint64_t ) override;
+  llvm::ErrorOr
+  CalculateMD5(const FileSpec _spec) override;
 
   Status GetFileWithUUID(const FileSpec _file, const UUID *uuid,
  FileSpec _file) override;

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp 
b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
index 52777909a1f825..82156aca8cf159 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
@@ -405,17 +405,21 @@ lldb_private::Status 
PlatformDarwinDevice::GetSharedModuleWithLocalCache(
   // when going over the *slow* GDB remote transfer mechanism we first
   // check the hashes of the files - and only do the actual transfer if
   // they 
diff er
-  uint64_t high_local, high_remote, low_local, low_remote;
   auto MD5 = llvm::sys::fs::md5_contents(module_cache_spec.GetPath());
   if (!MD5)
 return Status(MD5.getError());
-  std::tie(high_local, low_local) = MD5->words();
 
-  m_remote_platform_sp->CalculateMD5(module_spec.GetFileSpec(),
- low_remote, high_remote);
-  if (low_local != low_remote || high_local != high_remote) {
+  Log *log = GetLog(LLDBLog::Platform);
+  bool requires_transfer = true;
+  llvm::ErrorOr remote_md5 =
+  m_remote_platform_sp->CalculateMD5(module_spec.GetFileSpec());
+  if (std::error_code ec = remote_md5.getError())
+LLDB_LOG(log, "couldn't get md5 sum from remote: {0}",
+ 

[Lldb-commits] [lldb] [lldb] Unify CalculateMD5 return types (PR #90921)

2024-05-03 Thread Anthony Ha via lldb-commits

Awfa wrote:

> Thanks! LGTM.

Cool - can you merge this for me? I don't have write access.

https://github.com/llvm/llvm-project/pull/90921
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

2024-05-03 Thread Greg Clayton via lldb-commits

clayborg wrote:

> > > > Is any of it testable?
> > > 
> > > 
> > > Good question. Though this is mostly meant to be "NFC" (with very large 
> > > quotes), I can imagine us doing something like forcing the parsing of a 
> > > specific type (`type lookup ` ?), and then checking that the 
> > > module ast (`image dump ast`) does _not_ contain specific types -- as 
> > > that's basically what we're trying to achieve.
> > 
> > 
> > Yea that could work. But if it's going to be very painful or fragile to 
> > test then don't let that hold back the PR
> 
> In terms of testing, since this only delays definition DIE searching not type 
> completion, we need to construct a test so that lldb finds the declaration 
> DIE first without trigger a type completion on it and somehow test the 
> incomplete type. The first part is tricky. I'm not sure how to achieve it.

You should be able to make a test case with two files. One file contains the 
class definition and the other uses only a forward declaration. You could test 
by running the binary stopping only in the one with the forward declaration. So 
file 1 would be something like `foo.cpp` containing
```
struct Foo { 
  int value;
  Foo(int v): value(v) {}
};

Foo *CreateFoo() {
  return new Foo(1);
}

```
Then having `main.cpp` contain:
```
struct Foo; // Forward declare Foo
// Prototypes that don't need Foo definition
void Increment(Foo *); 
Foo *CreateFoo();

int main() {
  Foo *foo = CreateFoo();
  return 0; // Breakpoint 1 here
}
```
Then run to "Breakpoint 1 here" and then run `frame variable foo`. Foo won't 
need to be completed for this. But if you then run `frame variable *foo`, it 
should cause the type to be completed and show the instance variable correctly.


https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Add TeeLogHandler to log to 2 handlers (PR #90984)

2024-05-03 Thread Jonas Devlieghere via lldb-commits

https://github.com/JDevlieghere closed 
https://github.com/llvm/llvm-project/pull/90984
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] a8fbe50 - [lldb] Add TeeLogHandler to log to 2 handlers (#90984)

2024-05-03 Thread via lldb-commits

Author: Jonas Devlieghere
Date: 2024-05-03T11:08:50-07:00
New Revision: a8fbe500fe2ecdbd3c09ed06788092937819411f

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

LOG: [lldb] Add TeeLogHandler to log to 2 handlers (#90984)

Add a T-style log handler that multiplexes messages to two log handlers.
The goal is to use this in combination with the SystemLogHandler to log
messages both to the user requested file as well as the system log. The
latter is part of a sysdiagnose on Darwin which is commonly attached to
bug reports.

Added: 


Modified: 
lldb/include/lldb/Utility/Log.h
lldb/source/Utility/Log.cpp
lldb/unittests/Utility/LogTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/Log.h b/lldb/include/lldb/Utility/Log.h
index 01876ad732d4b5..27707c17f9b824 100644
--- a/lldb/include/lldb/Utility/Log.h
+++ b/lldb/include/lldb/Utility/Log.h
@@ -112,6 +112,23 @@ class RotatingLogHandler : public LogHandler {
   static char ID;
 };
 
+/// A T-style log handler that multiplexes messages to two log handlers.
+class TeeLogHandler : public LogHandler {
+public:
+  TeeLogHandler(std::shared_ptr first_log_handler,
+std::shared_ptr second_log_handler);
+
+  void Emit(llvm::StringRef message) override;
+
+  bool isA(const void *ClassID) const override { return ClassID ==  }
+  static bool classof(const LogHandler *obj) { return obj->isA(); }
+
+private:
+  std::shared_ptr m_first_log_handler;
+  std::shared_ptr m_second_log_handler;
+  static char ID;
+};
+
 class Log final {
 public:
   /// The underlying type of all log channel enums. Declare them as:

diff  --git a/lldb/source/Utility/Log.cpp b/lldb/source/Utility/Log.cpp
index 3a45a0285d3e25..6713a5bd758204 100644
--- a/lldb/source/Utility/Log.cpp
+++ b/lldb/source/Utility/Log.cpp
@@ -39,6 +39,7 @@ char LogHandler::ID;
 char StreamLogHandler::ID;
 char CallbackLogHandler::ID;
 char RotatingLogHandler::ID;
+char TeeLogHandler::ID;
 
 llvm::ManagedStatic Log::g_channel_map;
 
@@ -438,3 +439,16 @@ void RotatingLogHandler::Dump(llvm::raw_ostream ) 
const {
   }
   stream.flush();
 }
+
+TeeLogHandler::TeeLogHandler(std::shared_ptr first_log_handler,
+ std::shared_ptr second_log_handler)
+: m_first_log_handler(first_log_handler),
+  m_second_log_handler(second_log_handler) {
+  assert(m_first_log_handler && "first log handler must be valid");
+  assert(m_second_log_handler && "second log handler must be valid");
+}
+
+void TeeLogHandler::Emit(llvm::StringRef message) {
+  m_first_log_handler->Emit(message);
+  m_second_log_handler->Emit(message);
+}

diff  --git a/lldb/unittests/Utility/LogTest.cpp 
b/lldb/unittests/Utility/LogTest.cpp
index 1dac19486a8f7f..b9b0af4133da92 100644
--- a/lldb/unittests/Utility/LogTest.cpp
+++ b/lldb/unittests/Utility/LogTest.cpp
@@ -200,6 +200,18 @@ TEST(LogHandlerTest, RotatingLogHandler) {
   EXPECT_EQ(GetDumpAsString(handler), "bazquxquux");
 }
 
+TEST(LogHandlerTest, TeeLogHandler) {
+  auto handler1 = std::make_shared(2);
+  auto handler2 = std::make_shared(2);
+  TeeLogHandler handler(handler1, handler2);
+
+  handler.Emit("foo");
+  handler.Emit("bar");
+
+  EXPECT_EQ(GetDumpAsString(*handler1), "foobar");
+  EXPECT_EQ(GetDumpAsString(*handler2), "foobar");
+}
+
 TEST_F(LogChannelTest, Enable) {
   EXPECT_EQ(nullptr, GetLog(TestChannel::FOO));
   std::string message;



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


[Lldb-commits] [lldb] [lldb] Add TeeLogHandler to log to 2 handlers (PR #90984)

2024-05-03 Thread Jonas Devlieghere via lldb-commits

https://github.com/JDevlieghere updated 
https://github.com/llvm/llvm-project/pull/90984

>From d1adf630a9981f275f24e4d0c2c613a90ff38290 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Fri, 3 May 2024 10:11:40 -0700
Subject: [PATCH 1/2] [lldb] Add TeeLogHandler to log to 2 handlers

Add a T-style log handler that multiplexes messages to two log handlers.
The goal is to use this in combination with the SystemLogHandler to log
messages both to the user requested file as well as the system log. The
latter is part of a sysdiagnose on Darwin which is commonly attached to
bug reports.
---
 lldb/include/lldb/Utility/Log.h| 16 
 lldb/source/Utility/Log.cpp| 13 +
 lldb/unittests/Utility/LogTest.cpp | 12 
 3 files changed, 41 insertions(+)

diff --git a/lldb/include/lldb/Utility/Log.h b/lldb/include/lldb/Utility/Log.h
index 01876ad732d4b5..bea117c440a46d 100644
--- a/lldb/include/lldb/Utility/Log.h
+++ b/lldb/include/lldb/Utility/Log.h
@@ -112,6 +112,22 @@ class RotatingLogHandler : public LogHandler {
   static char ID;
 };
 
+class TeeLogHandler : public LogHandler {
+public:
+  TeeLogHandler(std::shared_ptr first_log_handler,
+std::shared_ptr second_log_handler);
+
+  void Emit(llvm::StringRef message) override;
+
+  bool isA(const void *ClassID) const override { return ClassID ==  }
+  static bool classof(const LogHandler *obj) { return obj->isA(); }
+
+private:
+  std::shared_ptr m_first_log_handler;
+  std::shared_ptr m_second_log_handler;
+  static char ID;
+};
+
 class Log final {
 public:
   /// The underlying type of all log channel enums. Declare them as:
diff --git a/lldb/source/Utility/Log.cpp b/lldb/source/Utility/Log.cpp
index 3a45a0285d3e25..1e237e7ff5e219 100644
--- a/lldb/source/Utility/Log.cpp
+++ b/lldb/source/Utility/Log.cpp
@@ -39,6 +39,7 @@ char LogHandler::ID;
 char StreamLogHandler::ID;
 char CallbackLogHandler::ID;
 char RotatingLogHandler::ID;
+char TeeLogHandler::ID;
 
 llvm::ManagedStatic Log::g_channel_map;
 
@@ -438,3 +439,15 @@ void RotatingLogHandler::Dump(llvm::raw_ostream ) 
const {
   }
   stream.flush();
 }
+
+TeeLogHandler::TeeLogHandler(std::shared_ptr first_log_handler,
+ std::shared_ptr second_log_handler)
+: m_first_log_handler(first_log_handler),
+  m_second_log_handler(second_log_handler) {}
+
+void TeeLogHandler::Emit(llvm::StringRef message) {
+  if (m_first_log_handler)
+m_first_log_handler->Emit(message);
+  if (m_second_log_handler)
+m_second_log_handler->Emit(message);
+}
diff --git a/lldb/unittests/Utility/LogTest.cpp 
b/lldb/unittests/Utility/LogTest.cpp
index 1dac19486a8f7f..b9b0af4133da92 100644
--- a/lldb/unittests/Utility/LogTest.cpp
+++ b/lldb/unittests/Utility/LogTest.cpp
@@ -200,6 +200,18 @@ TEST(LogHandlerTest, RotatingLogHandler) {
   EXPECT_EQ(GetDumpAsString(handler), "bazquxquux");
 }
 
+TEST(LogHandlerTest, TeeLogHandler) {
+  auto handler1 = std::make_shared(2);
+  auto handler2 = std::make_shared(2);
+  TeeLogHandler handler(handler1, handler2);
+
+  handler.Emit("foo");
+  handler.Emit("bar");
+
+  EXPECT_EQ(GetDumpAsString(*handler1), "foobar");
+  EXPECT_EQ(GetDumpAsString(*handler2), "foobar");
+}
+
 TEST_F(LogChannelTest, Enable) {
   EXPECT_EQ(nullptr, GetLog(TestChannel::FOO));
   std::string message;

>From 523ee979643494060fde757a64e26190e9241801 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Fri, 3 May 2024 11:05:03 -0700
Subject: [PATCH 2/2] Address Adrian's feedback

---
 lldb/include/lldb/Utility/Log.h |  1 +
 lldb/source/Utility/Log.cpp | 11 ++-
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/lldb/include/lldb/Utility/Log.h b/lldb/include/lldb/Utility/Log.h
index bea117c440a46d..27707c17f9b824 100644
--- a/lldb/include/lldb/Utility/Log.h
+++ b/lldb/include/lldb/Utility/Log.h
@@ -112,6 +112,7 @@ class RotatingLogHandler : public LogHandler {
   static char ID;
 };
 
+/// A T-style log handler that multiplexes messages to two log handlers.
 class TeeLogHandler : public LogHandler {
 public:
   TeeLogHandler(std::shared_ptr first_log_handler,
diff --git a/lldb/source/Utility/Log.cpp b/lldb/source/Utility/Log.cpp
index 1e237e7ff5e219..6713a5bd758204 100644
--- a/lldb/source/Utility/Log.cpp
+++ b/lldb/source/Utility/Log.cpp
@@ -443,11 +443,12 @@ void RotatingLogHandler::Dump(llvm::raw_ostream ) 
const {
 TeeLogHandler::TeeLogHandler(std::shared_ptr first_log_handler,
  std::shared_ptr second_log_handler)
 : m_first_log_handler(first_log_handler),
-  m_second_log_handler(second_log_handler) {}
+  m_second_log_handler(second_log_handler) {
+  assert(m_first_log_handler && "first log handler must be valid");
+  assert(m_second_log_handler && "second log handler must be valid");
+}
 
 void TeeLogHandler::Emit(llvm::StringRef message) {
-  if (m_first_log_handler)
-m_first_log_handler->Emit(message);
-  if (m_second_log_handler)
-

[Lldb-commits] [lldb] [lldb][NFCI] Unify DW_TAG -> string conversions (PR #90657)

2024-05-03 Thread Alex Langford via lldb-commits

https://github.com/bulbazord closed 
https://github.com/llvm/llvm-project/pull/90657
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] e2b3e4e - [lldb][NFCI] Unify DW_TAG -> string conversions (#90657)

2024-05-03 Thread via lldb-commits

Author: Alex Langford
Date: 2024-05-03T11:05:11-07:00
New Revision: e2b3e4ea9f2d0cb34d197439cfbc5090cdacb124

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

LOG: [lldb][NFCI] Unify DW_TAG -> string conversions (#90657)

The high level goal is to have 1 way of converting a DW_TAG value into a
human-readable string.

There are 3 ways this change accomplishes that:
1.) Changing DW_TAG_value_to_name to not create custom error strings.
  The way it was doing this is error-prone: Specifically, it was using a
  function-local static char buffer and handing out a pointer to it.
  Initialization of this is thread-safe, but mutating it is definitely
  not. Multiple threads that want to call this function could step on
  each others toes. The implementation in this patch sidesteps the issue
  by just returning a StringRef with no mention of the tag value in it.
2.) Changing all uses of DW_TAG_value_to_name to log the value of the
  tag since the function doesn't create a string with the value in it
  anymore.
3.) Removing `DWARFBaseDIE::GetTagAsCString()`. Callers should call
  DW_TAG_value_to_name on the tag directly.

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h
lldb/source/Plugins/SymbolFile/DWARF/DWARFDefines.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFDefines.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index bea11e0e3840af..f8101aba5c6277 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -452,9 +452,9 @@ TypeSP DWARFASTParserClang::ParseTypeFromDWARF(const 
SymbolContext ,
 log,
 "DWARFASTParserClang::ParseTypeFromDWARF "
 "(die = {0:x16}, decl_ctx = {1:p} (die "
-"{2:x16})) {3} name = '{4}')",
+"{2:x16})) {3} ({4}) name = '{5}')",
 die.GetOffset(), static_cast(context), context_die.GetOffset(),
-die.GetTagAsCString(), die.GetName());
+DW_TAG_value_to_name(die.Tag()), die.Tag(), die.GetName());
   }
 
   Type *type_ptr = dwarf->GetDIEToType().lookup(die.GetDIE());
@@ -765,9 +765,10 @@ DWARFASTParserClang::ParseTypeModifier(const SymbolContext 
,
   if (log)
 dwarf->GetObjectFile()->GetModule()->LogMessage(
 log,
-"SymbolFileDWARF::ParseType (die = {0:x16}) {1} '{2}' "
+"SymbolFileDWARF::ParseType (die = {0:x16}) {1} ({2}) '{3}' "
 "is Objective-C 'id' built-in type.",
-die.GetOffset(), die.GetTagAsCString(), die.GetName());
+die.GetOffset(), DW_TAG_value_to_name(die.Tag()), die.Tag(),
+die.GetName());
   clang_type = m_ast.GetBasicType(eBasicTypeObjCID);
   encoding_data_type = Type::eEncodingIsUID;
   attrs.type.Clear();
@@ -776,9 +777,10 @@ DWARFASTParserClang::ParseTypeModifier(const SymbolContext 
,
   if (log)
 dwarf->GetObjectFile()->GetModule()->LogMessage(
 log,
-"SymbolFileDWARF::ParseType (die = {0:x16}) {1} '{2}' "
+"SymbolFileDWARF::ParseType (die = {0:x16}) {1} ({2}) '{3}' "
 "is Objective-C 'Class' built-in type.",
-die.GetOffset(), die.GetTagAsCString(), die.GetName());
+die.GetOffset(), DW_TAG_value_to_name(die.Tag()), die.Tag(),
+die.GetName());
   clang_type = m_ast.GetBasicType(eBasicTypeObjCClass);
   encoding_data_type = Type::eEncodingIsUID;
   attrs.type.Clear();
@@ -787,9 +789,10 @@ DWARFASTParserClang::ParseTypeModifier(const SymbolContext 
,
   if (log)
 dwarf->GetObjectFile()->GetModule()->LogMessage(
 log,
-"SymbolFileDWARF::ParseType (die = {0:x16}) {1} '{2}' "
+"SymbolFileDWARF::ParseType (die = {0:x16}) {1} ({2}) '{3}' "
 "is Objective-C 'selector' built-in type.",
-die.GetOffset(), die.GetTagAsCString(), die.GetName());
+die.GetOffset(), DW_TAG_value_to_name(die.Tag()), die.Tag(),
+die.GetName());
   clang_type = m_ast.GetBasicType(eBasicTypeObjCSel);
   encoding_data_type = Type::eEncodingIsUID;
   attrs.type.Clear();
@@ -808,10 +811,10 @@ DWARFASTParserClang::ParseTypeModifier(const 
SymbolContext ,
 if (log)
   dwarf->GetObjectFile()->GetModule()->LogMessage(
   

[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

2024-05-03 Thread Greg Clayton via lldb-commits


@@ -154,6 +154,27 @@ static bool TagIsRecordType(dw_tag_t tag) {
   }
 }
 
+static bool
+IsForwardDeclaration(const lldb_private::plugin::dwarf::DWARFDIE ,
+ const ParsedDWARFTypeAttributes ,
+ LanguageType cu_language) {
+  if (attrs.byte_size && *attrs.byte_size == 0 && attrs.name &&

clayborg wrote:

Do we want to check `attrs.is_forward_declaration` before doing any of this and 
quick return? Then only do this more expensive `if` check if 
`attrs.is_forward_declaration` is false?

https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

2024-05-03 Thread Greg Clayton via lldb-commits

https://github.com/clayborg commented:

Looks pretty good to me as long as the test suite is happy.

https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

2024-05-03 Thread Greg Clayton via lldb-commits


@@ -1631,27 +1631,34 @@ bool SymbolFileDWARF::CompleteType(CompilerType 
_type) {
 return true;
   }
 
-  DWARFDIE dwarf_die = GetDIE(die_it->getSecond());
-  if (dwarf_die) {
-// Once we start resolving this type, remove it from the forward
-// declaration map in case anyone child members or other types require this
-// type to get resolved. The type will get resolved when all of the calls
-// to SymbolFileDWARF::ResolveClangOpaqueTypeDefinition are done.
-GetForwardDeclCompilerTypeToDIE().erase(die_it);
-
-Type *type = GetDIEToType().lookup(dwarf_die.GetDIE());
+  // Once we start resolving this type, remove it from the forward
+  // declaration map in case anyone child members or other types require this

clayborg wrote:

s/anyone/anyone's/ to fix bad previous comment

https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

2024-05-03 Thread Greg Clayton via lldb-commits

https://github.com/clayborg edited 
https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Add TeeLogHandler to log to 2 handlers (PR #90984)

2024-05-03 Thread Jonas Devlieghere via lldb-commits


@@ -438,3 +439,15 @@ void RotatingLogHandler::Dump(llvm::raw_ostream ) 
const {
   }
   stream.flush();
 }
+
+TeeLogHandler::TeeLogHandler(std::shared_ptr first_log_handler,
+ std::shared_ptr second_log_handler)
+: m_first_log_handler(first_log_handler),
+  m_second_log_handler(second_log_handler) {}
+
+void TeeLogHandler::Emit(llvm::StringRef message) {
+  if (m_first_log_handler)

JDevlieghere wrote:

No, it's just being resilient agains the shared_ptrs being null. The 
alternative would be an assert in the ctor. Let me know if you prefer that? It 
would be slightly more efficient. 

https://github.com/llvm/llvm-project/pull/90984
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Add TeeLogHandler to log to 2 handlers (PR #90984)

2024-05-03 Thread Adrian Prantl via lldb-commits

https://github.com/adrian-prantl approved this pull request.


https://github.com/llvm/llvm-project/pull/90984
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Add TeeLogHandler to log to 2 handlers (PR #90984)

2024-05-03 Thread Adrian Prantl via lldb-commits


@@ -438,3 +439,15 @@ void RotatingLogHandler::Dump(llvm::raw_ostream ) 
const {
   }
   stream.flush();
 }
+
+TeeLogHandler::TeeLogHandler(std::shared_ptr first_log_handler,
+ std::shared_ptr second_log_handler)
+: m_first_log_handler(first_log_handler),
+  m_second_log_handler(second_log_handler) {}
+
+void TeeLogHandler::Emit(llvm::StringRef message) {
+  if (m_first_log_handler)

adrian-prantl wrote:

Is the idea that one will ne optional in the typical use-case?

https://github.com/llvm/llvm-project/pull/90984
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Add TeeLogHandler to log to 2 handlers (PR #90984)

2024-05-03 Thread Adrian Prantl via lldb-commits

https://github.com/adrian-prantl edited 
https://github.com/llvm/llvm-project/pull/90984
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] Add new Python API `SBCommandInterpreter::GetTranscript()` (PR #90703)

2024-05-03 Thread Alex Langford via lldb-commits


@@ -85,3 +86,84 @@ def test_command_output(self):
 self.assertEqual(res.GetOutput(), "")
 self.assertIsNotNone(res.GetError())
 self.assertEqual(res.GetError(), "")
+
+def test_structured_transcript(self):
+"""Test structured transcript generation and retrieval."""
+# Get command interpreter and create a target
+self.build()
+exe = self.getBuildArtifact("a.out")
+
+target = self.dbg.CreateTarget(exe)
+self.assertTrue(target, VALID_TARGET)
+
+ci = self.dbg.GetCommandInterpreter()
+self.assertTrue(ci, VALID_COMMAND_INTERPRETER)
+
+# Send a few commands through the command interpreter
+res = lldb.SBCommandReturnObject()
+ci.HandleCommand("version", res)
+ci.HandleCommand("an-unknown-command", res)
+ci.HandleCommand("breakpoint set -f main.c -l %d" % self.line, res)
+ci.HandleCommand("r", res)
+ci.HandleCommand("p a", res)
+total_number_of_commands = 5
+
+# Retrieve the transcript and convert it into a Python object
+transcript = ci.GetTranscript()
+self.assertTrue(transcript.IsValid())
+
+stream = lldb.SBStream()
+self.assertTrue(stream)
+
+error = transcript.GetAsJSON(stream)
+self.assertSuccess(error)
+
+transcript = json.loads(stream.GetData())
+
+# The transcript will contain a bunch of commands that are run
+# automatically. We only want to validate for the ones that are
+# listed above, hence trimming to the last parts.
+transcript = transcript[-total_number_of_commands:]

bulbazord wrote:

Instead of trimming, why not verify that the transcript contains exactly the 
number we care about? Or is there a reason there would be more? Perhaps from 
some setup code?

https://github.com/llvm/llvm-project/pull/90703
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] Add new Python API `SBCommandInterpreter::GetTranscript()` (PR #90703)

2024-05-03 Thread Alex Langford via lldb-commits

https://github.com/bulbazord edited 
https://github.com/llvm/llvm-project/pull/90703
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] Add new Python API `SBCommandInterpreter::GetTranscript()` (PR #90703)

2024-05-03 Thread Alex Langford via lldb-commits

https://github.com/bulbazord commented:

Looking better. Thanks for sticking with us through the review! :)

https://github.com/llvm/llvm-project/pull/90703
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] Add new Python API `SBCommandInterpreter::GetTranscript()` (PR #90703)

2024-05-03 Thread Alex Langford via lldb-commits


@@ -289,3 +290,19 @@ void 
StructuredData::Null::GetDescription(lldb_private::Stream ) const {
 void StructuredData::Generic::GetDescription(lldb_private::Stream ) const {
   s.Printf("%p", m_object);
 }
+
+StructuredData::ArraySP StructuredData::Array::SplitString(llvm::StringRef s,
+   char separator,
+   int maxSplit,
+   bool keepEmpty) {
+  // Split the string into a small vector.
+  llvm::SmallVector small_vec;
+  s.split(small_vec, separator, maxSplit, keepEmpty);
+
+  // Copy the substrings from the small vector into the output array.
+  auto array_sp = std::make_shared();
+  for (auto substring : small_vec) {
+array_sp->AddStringItem(std::move(substring));

bulbazord wrote:

nit: no need for the move, StringRefs are pretty cheap to copy

https://github.com/llvm/llvm-project/pull/90703
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Add TeeLogHandler to log to 2 handlers (PR #90984)

2024-05-03 Thread Adrian Prantl via lldb-commits


@@ -112,6 +112,22 @@ class RotatingLogHandler : public LogHandler {
   static char ID;
 };
 
+class TeeLogHandler : public LogHandler {

adrian-prantl wrote:

Doxygen comment?

https://github.com/llvm/llvm-project/pull/90984
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [llvm] [AArch64] move extension information into tablgen (PR #90987)

2024-05-03 Thread Tomas Matheson via lldb-commits

https://github.com/tmatheson-arm updated 
https://github.com/llvm/llvm-project/pull/90987

>From 4b8b776348438847c2eb238dac973e93fe93294e Mon Sep 17 00:00:00 2001
From: Tomas Matheson 
Date: Mon, 29 Apr 2024 19:57:17 +0100
Subject: [PATCH 1/3] [AArch64] move extension information into tablgen

Generate target features and FMVExtensions from tablegen.

Use MArchName/ArchKindEnumSpelling to avoid renamings.
Cases where there is simply a case difference are handled by
consistently uppercasing the AEK_ name in the emitted code.

Remove some Extensions which were not needed.
These had AEK entries but were never actually used for anything.
They are not present in Extensions[] data.
---
 .../command-disassemble-aarch64-extensions.s  |   2 +-
 .../llvm/TargetParser/AArch64TargetParser.h   | 139 +--
 llvm/lib/Target/AArch64/AArch64Features.td| 216 ++
 .../TargetParser/TargetParserTest.cpp |   8 +-
 llvm/utils/TableGen/ARMTargetDefEmitter.cpp   |  60 -
 5 files changed, 236 insertions(+), 189 deletions(-)

diff --git a/lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s 
b/lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s
index e154f544e7cc6e..685d0a84ec2896 100644
--- a/lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s
+++ b/lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s
@@ -59,7 +59,7 @@ fn:
   bdep z0.b, z1.b, z31.b// AEK_SVE2BITPERM
   rax1 z0.d, z0.d, z0.d // AEK_SVE2SHA3
   sm4e z0.s, z0.s, z0.s // AEK_SVE2SM4
-  addqv   v0.8h, p0, z0.h   // AEK_SVE2p1 / AEK_SME2p1
+  addqv   v0.8h, p0, z0.h   // AEK_SVE2P1 / AEK_SME2P1
   rcwswp x0, x1, [x2]   // AEK_THE
   tcommit   // AEK_TME
 lbl:
diff --git a/llvm/include/llvm/TargetParser/AArch64TargetParser.h 
b/llvm/include/llvm/TargetParser/AArch64TargetParser.h
index 04fbaf07adfbcb..1124420daf8d80 100644
--- a/llvm/include/llvm/TargetParser/AArch64TargetParser.h
+++ b/llvm/include/llvm/TargetParser/AArch64TargetParser.h
@@ -104,29 +104,9 @@ static_assert(FEAT_MAX < 62,
   "Number of features in CPUFeatures are limited to 62 entries");
 
 // Each ArchExtKind correponds directly to a possible -target-feature.
-enum ArchExtKind : unsigned {
-  AEK_NONE = 1,
-#define ARM_EXTENSION(NAME, ENUM) ENUM,
+#define EMIT_ARCHEXTKIND_ENUM
 #include "llvm/TargetParser/AArch64TargetParserDef.inc"
-  AEK_NUM_EXTENSIONS,
-
-  // FIXME temporary fixes for inconsistent naming.
-  AEK_F32MM = AEK_MATMULFP32,
-  AEK_F64MM = AEK_MATMULFP64,
-  AEK_FCMA = AEK_COMPLXNUM,
-  AEK_FP = AEK_FPARMV8,
-  AEK_FP16 = AEK_FULLFP16,
-  AEK_I8MM = AEK_MATMULINT8,
-  AEK_JSCVT = AEK_JS,
-  AEK_PROFILE = AEK_SPE,
-  AEK_RASv2 = AEK_RASV2,
-  AEK_RAND = AEK_RANDGEN,
-  AEK_SIMD = AEK_NEON,
-  AEK_SME2p1 = AEK_SME2P1,
-  AEK_SVE2p1 = AEK_SVE2P1,
-  AEK_SME_LUTv2 = AEK_SME_LUTV2,
 
-};
 using ExtensionBitset = Bitset;
 
 // Represents an extension that can be enabled with -march=+.
@@ -148,111 +128,8 @@ struct ExtensionInfo {
   1000; // Maximum priority for FMV feature
 };
 
-// NOTE: If adding a new extension here, consider adding it to ExtensionMap
-// in AArch64AsmParser too, if supported as an extension name by binutils.
-// clang-format off
-inline constexpr ExtensionInfo Extensions[] = {
-{"aes", AArch64::AEK_AES, "+aes", "-aes", FEAT_AES, "+fp-armv8,+neon", 
150},
-{"b16b16", AArch64::AEK_B16B16, "+b16b16", "-b16b16", FEAT_INIT, "", 0},
-{"bf16", AArch64::AEK_BF16, "+bf16", "-bf16", FEAT_BF16, "+bf16", 280},
-{"brbe", AArch64::AEK_BRBE, "+brbe", "-brbe", FEAT_INIT, "", 0},
-{"bti", AArch64::AEK_NONE, {}, {}, FEAT_BTI, "+bti", 510},
-{"crc", AArch64::AEK_CRC, "+crc", "-crc", FEAT_CRC, "+crc", 110},
-{"crypto", AArch64::AEK_CRYPTO, "+crypto", "-crypto", FEAT_INIT, 
"+aes,+sha2", 0},
-{"cssc", AArch64::AEK_CSSC, "+cssc", "-cssc", FEAT_INIT, "", 0},
-{"d128", AArch64::AEK_D128, "+d128", "-d128", FEAT_INIT, "", 0},
-{"dgh", AArch64::AEK_NONE, {}, {}, FEAT_DGH, "", 260},
-{"dit", AArch64::AEK_NONE, {}, {}, FEAT_DIT, "+dit", 180},
-{"dotprod", AArch64::AEK_DOTPROD, "+dotprod", "-dotprod", FEAT_DOTPROD, 
"+dotprod,+fp-armv8,+neon", 104},
-{"dpb", AArch64::AEK_NONE, {}, {}, FEAT_DPB, "+ccpp", 190},
-{"dpb2", AArch64::AEK_NONE, {}, {}, FEAT_DPB2, "+ccpp,+ccdp", 200},
-{"ebf16", AArch64::AEK_NONE, {}, {}, FEAT_EBF16, "+bf16", 290},
-{"f32mm", AArch64::AEK_F32MM, "+f32mm", "-f32mm", FEAT_SVE_F32MM, 
"+sve,+f32mm,+fullfp16,+fp-armv8,+neon", 350},
-{"f64mm", AArch64::AEK_F64MM, "+f64mm", "-f64mm", FEAT_SVE_F64MM, 
"+sve,+f64mm,+fullfp16,+fp-armv8,+neon", 360},
-{"fcma", AArch64::AEK_FCMA, "+complxnum", "-complxnum", FEAT_FCMA, 
"+fp-armv8,+neon,+complxnum", 220},
-{"flagm", AArch64::AEK_FLAGM, "+flagm", "-flagm", FEAT_FLAGM, "+flagm", 
20},
-{"flagm2", AArch64::AEK_NONE, {}, {}, FEAT_FLAGM2, 

[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

2024-05-03 Thread Zequan Wu via lldb-commits

ZequanWu wrote:

> > > Is any of it testable?
> > 
> > 
> > Good question. Though this is mostly meant to be "NFC" (with very large 
> > quotes), I can imagine us doing something like forcing the parsing of a 
> > specific type (`type lookup ` ?), and then checking that the 
> > module ast (`image dump ast`) does _not_ contain specific types -- as 
> > that's basically what we're trying to achieve.
> 
> Yea that could work. But if it's going to be very painful or fragile to test 
> then don't let that hold back the PR

In terms of testing, since this only delays definition DIE searching not type 
completion, we need to construct a test so that lldb finds the declaration DIE 
first without trigger a type completion on it and somehow test the incomplete 
type. The first part is tricky. I'm not sure how to achieve it.

https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] LLDB Debuginfod tests and a fix or two (PR #90622)

2024-05-03 Thread Kevin Frei via lldb-commits


@@ -87,8 +105,15 @@ SymbolVendorELF::CreateInstance(const lldb::ModuleSP 
_sp,
   FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths();
   FileSpec dsym_fspec =
   PluginManager::LocateExecutableSymbolFile(module_spec, search_paths);
-  if (!dsym_fspec)
-return nullptr;
+  if (!dsym_fspec || IsDwpSymbolFile(module_sp, dsym_fspec)) {
+// If we have a stripped binary or if we got a DWP file, we should prefer
+// symbols in the executable acquired through a plugin.
+ModuleSpec unstripped_spec =
+PluginManager::LocateExecutableObjectFile(module_spec);
+if (!unstripped_spec)
+  return nullptr;
+dsym_fspec = unstripped_spec.GetFileSpec();
+  }

kevinfrei wrote:

@DavidSpickett if there's anything I can do to help, please ping me. Feel free 
to use my github handle at hotmail to ping me, as I'm going to be on a "doing 
nothing in nicer weather" vacation next week.

https://github.com/llvm/llvm-project/pull/90622
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [lldb] [llvm] [mlir] [openmp] [pstl] Finally formalise our defacto line-ending policy (PR #86318)

2024-05-03 Thread Fangrui Song via lldb-commits


@@ -1,3 +1,10 @@
+# Checkout as native, commit as LF except in specific circumstances
+* text=auto
+*.bat text eol=crlf
+*.rc text eol=crlf
+*.sln text eol=crlf

MaskRay wrote:

Do we need `.sln`? There is only one file in `clang/tools/clang-format-vs`. 
There are a few other text files in this directory, so perhaps a gitattributes 
in that directory is better?

https://github.com/llvm/llvm-project/pull/86318
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [llvm] [AArch64] move extension information into tablgen (PR #90987)

2024-05-03 Thread Tomas Matheson via lldb-commits

https://github.com/tmatheson-arm created 
https://github.com/llvm/llvm-project/pull/90987

Generate target features and FMVExtensions from tablegen.

Use MArchName/ArchKindEnumSpelling to avoid renamings.
Cases where there is simply a case difference are handled by
consistently uppercasing the AEK_ name in the emitted code.

Remove some Extensions which were not needed.
These had AEK entries but were never actually used for anything.
They are not present in Extensions[] data.

>From 4b8b776348438847c2eb238dac973e93fe93294e Mon Sep 17 00:00:00 2001
From: Tomas Matheson 
Date: Mon, 29 Apr 2024 19:57:17 +0100
Subject: [PATCH 1/2] [AArch64] move extension information into tablgen

Generate target features and FMVExtensions from tablegen.

Use MArchName/ArchKindEnumSpelling to avoid renamings.
Cases where there is simply a case difference are handled by
consistently uppercasing the AEK_ name in the emitted code.

Remove some Extensions which were not needed.
These had AEK entries but were never actually used for anything.
They are not present in Extensions[] data.
---
 .../command-disassemble-aarch64-extensions.s  |   2 +-
 .../llvm/TargetParser/AArch64TargetParser.h   | 139 +--
 llvm/lib/Target/AArch64/AArch64Features.td| 216 ++
 .../TargetParser/TargetParserTest.cpp |   8 +-
 llvm/utils/TableGen/ARMTargetDefEmitter.cpp   |  60 -
 5 files changed, 236 insertions(+), 189 deletions(-)

diff --git a/lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s 
b/lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s
index e154f544e7cc6e..685d0a84ec2896 100644
--- a/lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s
+++ b/lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s
@@ -59,7 +59,7 @@ fn:
   bdep z0.b, z1.b, z31.b// AEK_SVE2BITPERM
   rax1 z0.d, z0.d, z0.d // AEK_SVE2SHA3
   sm4e z0.s, z0.s, z0.s // AEK_SVE2SM4
-  addqv   v0.8h, p0, z0.h   // AEK_SVE2p1 / AEK_SME2p1
+  addqv   v0.8h, p0, z0.h   // AEK_SVE2P1 / AEK_SME2P1
   rcwswp x0, x1, [x2]   // AEK_THE
   tcommit   // AEK_TME
 lbl:
diff --git a/llvm/include/llvm/TargetParser/AArch64TargetParser.h 
b/llvm/include/llvm/TargetParser/AArch64TargetParser.h
index 04fbaf07adfbcb..1124420daf8d80 100644
--- a/llvm/include/llvm/TargetParser/AArch64TargetParser.h
+++ b/llvm/include/llvm/TargetParser/AArch64TargetParser.h
@@ -104,29 +104,9 @@ static_assert(FEAT_MAX < 62,
   "Number of features in CPUFeatures are limited to 62 entries");
 
 // Each ArchExtKind correponds directly to a possible -target-feature.
-enum ArchExtKind : unsigned {
-  AEK_NONE = 1,
-#define ARM_EXTENSION(NAME, ENUM) ENUM,
+#define EMIT_ARCHEXTKIND_ENUM
 #include "llvm/TargetParser/AArch64TargetParserDef.inc"
-  AEK_NUM_EXTENSIONS,
-
-  // FIXME temporary fixes for inconsistent naming.
-  AEK_F32MM = AEK_MATMULFP32,
-  AEK_F64MM = AEK_MATMULFP64,
-  AEK_FCMA = AEK_COMPLXNUM,
-  AEK_FP = AEK_FPARMV8,
-  AEK_FP16 = AEK_FULLFP16,
-  AEK_I8MM = AEK_MATMULINT8,
-  AEK_JSCVT = AEK_JS,
-  AEK_PROFILE = AEK_SPE,
-  AEK_RASv2 = AEK_RASV2,
-  AEK_RAND = AEK_RANDGEN,
-  AEK_SIMD = AEK_NEON,
-  AEK_SME2p1 = AEK_SME2P1,
-  AEK_SVE2p1 = AEK_SVE2P1,
-  AEK_SME_LUTv2 = AEK_SME_LUTV2,
 
-};
 using ExtensionBitset = Bitset;
 
 // Represents an extension that can be enabled with -march=+.
@@ -148,111 +128,8 @@ struct ExtensionInfo {
   1000; // Maximum priority for FMV feature
 };
 
-// NOTE: If adding a new extension here, consider adding it to ExtensionMap
-// in AArch64AsmParser too, if supported as an extension name by binutils.
-// clang-format off
-inline constexpr ExtensionInfo Extensions[] = {
-{"aes", AArch64::AEK_AES, "+aes", "-aes", FEAT_AES, "+fp-armv8,+neon", 
150},
-{"b16b16", AArch64::AEK_B16B16, "+b16b16", "-b16b16", FEAT_INIT, "", 0},
-{"bf16", AArch64::AEK_BF16, "+bf16", "-bf16", FEAT_BF16, "+bf16", 280},
-{"brbe", AArch64::AEK_BRBE, "+brbe", "-brbe", FEAT_INIT, "", 0},
-{"bti", AArch64::AEK_NONE, {}, {}, FEAT_BTI, "+bti", 510},
-{"crc", AArch64::AEK_CRC, "+crc", "-crc", FEAT_CRC, "+crc", 110},
-{"crypto", AArch64::AEK_CRYPTO, "+crypto", "-crypto", FEAT_INIT, 
"+aes,+sha2", 0},
-{"cssc", AArch64::AEK_CSSC, "+cssc", "-cssc", FEAT_INIT, "", 0},
-{"d128", AArch64::AEK_D128, "+d128", "-d128", FEAT_INIT, "", 0},
-{"dgh", AArch64::AEK_NONE, {}, {}, FEAT_DGH, "", 260},
-{"dit", AArch64::AEK_NONE, {}, {}, FEAT_DIT, "+dit", 180},
-{"dotprod", AArch64::AEK_DOTPROD, "+dotprod", "-dotprod", FEAT_DOTPROD, 
"+dotprod,+fp-armv8,+neon", 104},
-{"dpb", AArch64::AEK_NONE, {}, {}, FEAT_DPB, "+ccpp", 190},
-{"dpb2", AArch64::AEK_NONE, {}, {}, FEAT_DPB2, "+ccpp,+ccdp", 200},
-{"ebf16", AArch64::AEK_NONE, {}, {}, FEAT_EBF16, "+bf16", 290},
-{"f32mm", AArch64::AEK_F32MM, "+f32mm", "-f32mm", FEAT_SVE_F32MM, 

[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [lldb] [llvm] [mlir] [openmp] [pstl] Finally formalise our defacto line-ending policy (PR #86318)

2024-05-03 Thread Fangrui Song via lldb-commits

https://github.com/MaskRay approved this pull request.


https://github.com/llvm/llvm-project/pull/86318
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Add TeeLogHandler to log to 2 handlers (PR #90984)

2024-05-03 Thread Jonas Devlieghere via lldb-commits

https://github.com/JDevlieghere created 
https://github.com/llvm/llvm-project/pull/90984

Add a T-style log handler that multiplexes messages to two log handlers. The 
goal is to use this in combination with the SystemLogHandler to log messages 
both to the user requested file as well as the system log. The latter is part 
of a sysdiagnose on Darwin which is commonly attached to bug reports.

>From d1adf630a9981f275f24e4d0c2c613a90ff38290 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Fri, 3 May 2024 10:11:40 -0700
Subject: [PATCH] [lldb] Add TeeLogHandler to log to 2 handlers

Add a T-style log handler that multiplexes messages to two log handlers.
The goal is to use this in combination with the SystemLogHandler to log
messages both to the user requested file as well as the system log. The
latter is part of a sysdiagnose on Darwin which is commonly attached to
bug reports.
---
 lldb/include/lldb/Utility/Log.h| 16 
 lldb/source/Utility/Log.cpp| 13 +
 lldb/unittests/Utility/LogTest.cpp | 12 
 3 files changed, 41 insertions(+)

diff --git a/lldb/include/lldb/Utility/Log.h b/lldb/include/lldb/Utility/Log.h
index 01876ad732d4b5..bea117c440a46d 100644
--- a/lldb/include/lldb/Utility/Log.h
+++ b/lldb/include/lldb/Utility/Log.h
@@ -112,6 +112,22 @@ class RotatingLogHandler : public LogHandler {
   static char ID;
 };
 
+class TeeLogHandler : public LogHandler {
+public:
+  TeeLogHandler(std::shared_ptr first_log_handler,
+std::shared_ptr second_log_handler);
+
+  void Emit(llvm::StringRef message) override;
+
+  bool isA(const void *ClassID) const override { return ClassID ==  }
+  static bool classof(const LogHandler *obj) { return obj->isA(); }
+
+private:
+  std::shared_ptr m_first_log_handler;
+  std::shared_ptr m_second_log_handler;
+  static char ID;
+};
+
 class Log final {
 public:
   /// The underlying type of all log channel enums. Declare them as:
diff --git a/lldb/source/Utility/Log.cpp b/lldb/source/Utility/Log.cpp
index 3a45a0285d3e25..1e237e7ff5e219 100644
--- a/lldb/source/Utility/Log.cpp
+++ b/lldb/source/Utility/Log.cpp
@@ -39,6 +39,7 @@ char LogHandler::ID;
 char StreamLogHandler::ID;
 char CallbackLogHandler::ID;
 char RotatingLogHandler::ID;
+char TeeLogHandler::ID;
 
 llvm::ManagedStatic Log::g_channel_map;
 
@@ -438,3 +439,15 @@ void RotatingLogHandler::Dump(llvm::raw_ostream ) 
const {
   }
   stream.flush();
 }
+
+TeeLogHandler::TeeLogHandler(std::shared_ptr first_log_handler,
+ std::shared_ptr second_log_handler)
+: m_first_log_handler(first_log_handler),
+  m_second_log_handler(second_log_handler) {}
+
+void TeeLogHandler::Emit(llvm::StringRef message) {
+  if (m_first_log_handler)
+m_first_log_handler->Emit(message);
+  if (m_second_log_handler)
+m_second_log_handler->Emit(message);
+}
diff --git a/lldb/unittests/Utility/LogTest.cpp 
b/lldb/unittests/Utility/LogTest.cpp
index 1dac19486a8f7f..b9b0af4133da92 100644
--- a/lldb/unittests/Utility/LogTest.cpp
+++ b/lldb/unittests/Utility/LogTest.cpp
@@ -200,6 +200,18 @@ TEST(LogHandlerTest, RotatingLogHandler) {
   EXPECT_EQ(GetDumpAsString(handler), "bazquxquux");
 }
 
+TEST(LogHandlerTest, TeeLogHandler) {
+  auto handler1 = std::make_shared(2);
+  auto handler2 = std::make_shared(2);
+  TeeLogHandler handler(handler1, handler2);
+
+  handler.Emit("foo");
+  handler.Emit("bar");
+
+  EXPECT_EQ(GetDumpAsString(*handler1), "foobar");
+  EXPECT_EQ(GetDumpAsString(*handler2), "foobar");
+}
+
 TEST_F(LogChannelTest, Enable) {
   EXPECT_EQ(nullptr, GetLog(TestChannel::FOO));
   std::string message;

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


[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [lldb] [llvm] [mlir] [openmp] [pstl] Finally formalise our defacto line-ending policy (PR #86318)

2024-05-03 Thread via lldb-commits


@@ -0,0 +1 @@
+dos-style-eol.txt

ldrumm wrote:

Thanks `text eol=crlf`. Updated in 64350b342a09ba69803a541a89b5681a12925ff0

https://github.com/llvm/llvm-project/pull/86318
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][NFCI] Unify DW_TAG -> string conversions (PR #90657)

2024-05-03 Thread Jonas Devlieghere via lldb-commits

https://github.com/JDevlieghere approved this pull request.


https://github.com/llvm/llvm-project/pull/90657
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [lld] [lldb] [llvm] [mlir] [openmp] [pstl] Finally formalise our defacto line-ending policy (PR #86318)

2024-05-03 Thread Fangrui Song via lldb-commits


@@ -0,0 +1 @@
+dos-style-eol.txt

MaskRay wrote:

missing text=auto eol=crlf ?

https://github.com/llvm/llvm-project/pull/86318
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Always emit diagnostic events to the system log (PR #90913)

2024-05-03 Thread Jonas Devlieghere via lldb-commits

https://github.com/JDevlieghere closed 
https://github.com/llvm/llvm-project/pull/90913
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] f9d91fb - [lldb] Always emit diagnostic events to the system log (#90913)

2024-05-03 Thread via lldb-commits

Author: Jonas Devlieghere
Date: 2024-05-03T09:45:16-07:00
New Revision: f9d91fbe86519f3083a9ae37b1e2038f6b8992a6

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

LOG: [lldb] Always emit diagnostic events to the system log (#90913)

Always emit diagnostic events to the system log so that they end up in
the sysdiagnose on Darwin.

Added: 


Modified: 
lldb/source/Core/Debugger.cpp

Removed: 




diff  --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 065f70c3880aaa..976420a4344394 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -1512,6 +1512,9 @@ void Debugger::ReportDiagnosticImpl(Severity severity, 
std::string message,
 std::optional debugger_id,
 std::once_flag *once) {
   auto ReportDiagnosticLambda = [&]() {
+// Always log diagnostics to the system log.
+Host::SystemLog(severity, message);
+
 // The diagnostic subsystem is optional but we still want to broadcast
 // events when it's disabled.
 if (Diagnostics::Enabled())



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


[Lldb-commits] [lldb] [lldb] Always emit diagnostic events to the system log (PR #90913)

2024-05-03 Thread Jonas Devlieghere via lldb-commits

https://github.com/JDevlieghere updated 
https://github.com/llvm/llvm-project/pull/90913

>From 7ed4008b32d6ec7809a9cea0eb4462d1b90f4e52 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Thu, 2 May 2024 15:47:46 -0700
Subject: [PATCH] [lldb] Always emit diagnostic events to the system log

Always emit diagnostic events to the system log so that they end up in
the sysdiagnose on Darwin.
---
 lldb/source/Core/Debugger.cpp | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 065f70c3880aaa..976420a4344394 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -1512,6 +1512,9 @@ void Debugger::ReportDiagnosticImpl(Severity severity, 
std::string message,
 std::optional debugger_id,
 std::once_flag *once) {
   auto ReportDiagnosticLambda = [&]() {
+// Always log diagnostics to the system log.
+Host::SystemLog(severity, message);
+
 // The diagnostic subsystem is optional but we still want to broadcast
 // events when it's disabled.
 if (Diagnostics::Enabled())

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


[Lldb-commits] [lldb] 528f5ba - [lldb] Create a single Severity enum in lldb-enumerations (#90917)

2024-05-03 Thread via lldb-commits

Author: Jonas Devlieghere
Date: 2024-05-03T09:25:38-07:00
New Revision: 528f5ba7af2729bb7e23f0846b75e4f25af2bf8d

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

LOG: [lldb] Create a single Severity enum in lldb-enumerations (#90917)

We have 3 different enums all expressing severity (info, warning,
error). Remove all uses with a new Severity enum in lldb-enumerations.h.

Added: 


Modified: 
lldb/include/lldb/Core/Debugger.h
lldb/include/lldb/Core/DebuggerEvents.h
lldb/include/lldb/Expression/DiagnosticManager.h
lldb/include/lldb/Host/Host.h
lldb/include/lldb/lldb-enumerations.h
lldb/source/Core/Debugger.cpp
lldb/source/Core/DebuggerEvents.cpp
lldb/source/Expression/DiagnosticManager.cpp
lldb/source/Expression/FunctionCaller.cpp
lldb/source/Expression/LLVMUserExpression.cpp
lldb/source/Host/common/Host.cpp
lldb/source/Host/macosx/objcxx/Host.mm
lldb/source/Plugins/ExpressionParser/Clang/ClangDiagnostic.h
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangFunctionCaller.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
lldb/source/Target/Process.cpp
lldb/unittests/Expression/DiagnosticManagerTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/Debugger.h 
b/lldb/include/lldb/Core/Debugger.h
index 49ff0737acef82..c0f7c732ad2d46 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -630,8 +630,7 @@ class Debugger : public 
std::enable_shared_from_this,
  std::optional debugger_id,
  uint32_t progress_category_bit = eBroadcastBitProgress);
 
-  static void ReportDiagnosticImpl(DiagnosticEventData::Type type,
-   std::string message,
+  static void ReportDiagnosticImpl(lldb::Severity severity, std::string 
message,
std::optional debugger_id,
std::once_flag *once);
 

diff  --git a/lldb/include/lldb/Core/DebuggerEvents.h 
b/lldb/include/lldb/Core/DebuggerEvents.h
index 74bb05e6e6bf88..49a4ecf8e537e3 100644
--- a/lldb/include/lldb/Core/DebuggerEvents.h
+++ b/lldb/include/lldb/Core/DebuggerEvents.h
@@ -76,19 +76,15 @@ class ProgressEventData : public EventData {
 
 class DiagnosticEventData : public EventData {
 public:
-  enum class Type {
-Info,
-Warning,
-Error,
-  };
-  DiagnosticEventData(Type type, std::string message, bool debugger_specific)
-  : m_message(std::move(message)), m_type(type),
+  DiagnosticEventData(lldb::Severity severity, std::string message,
+  bool debugger_specific)
+  : m_message(std::move(message)), m_severity(severity),
 m_debugger_specific(debugger_specific) {}
   ~DiagnosticEventData() override = default;
 
   const std::string () const { return m_message; }
   bool IsDebuggerSpecific() const { return m_debugger_specific; }
-  Type GetType() const { return m_type; }
+  lldb::Severity GetSeverity() const { return m_severity; }
 
   llvm::StringRef GetPrefix() const;
 
@@ -105,7 +101,7 @@ class DiagnosticEventData : public EventData {
 
 protected:
   std::string m_message;
-  Type m_type;
+  lldb::Severity m_severity;
   const bool m_debugger_specific;
 
   DiagnosticEventData(const DiagnosticEventData &) = delete;

diff  --git a/lldb/include/lldb/Expression/DiagnosticManager.h 
b/lldb/include/lldb/Expression/DiagnosticManager.h
index 06bf1d115f1541..d49b7c99b114fb 100644
--- a/lldb/include/lldb/Expression/DiagnosticManager.h
+++ b/lldb/include/lldb/Expression/DiagnosticManager.h
@@ -28,12 +28,6 @@ enum DiagnosticOrigin {
   eDiagnosticOriginLLVM
 };
 
-enum DiagnosticSeverity {
-  eDiagnosticSeverityError,
-  eDiagnosticSeverityWarning,
-  eDiagnosticSeverityRemark
-};
-
 const uint32_t LLDB_INVALID_COMPILER_ID = UINT32_MAX;
 
 class Diagnostic {
@@ -55,7 +49,7 @@ class Diagnostic {
 }
   }
 
-  Diagnostic(llvm::StringRef message, DiagnosticSeverity severity,
+  Diagnostic(llvm::StringRef message, lldb::Severity severity,
  DiagnosticOrigin origin, uint32_t compiler_id)
   : m_message(message), m_severity(severity), m_origin(origin),
 m_compiler_id(compiler_id) {}
@@ -68,7 +62,7 @@ class Diagnostic {
 
   virtual bool HasFixIts() const { return false; }
 
-  DiagnosticSeverity GetSeverity() const { return m_severity; }
+  lldb::Severity GetSeverity() const { return m_severity; }
 
   uint32_t GetCompilerID() const { return m_compiler_id; }
 
@@ -83,7 +77,7 @@ class Diagnostic {
 
 protected:
   std::string 

[Lldb-commits] [lldb] [lldb] Create a single Severity enum in lldb-enumerations (PR #90917)

2024-05-03 Thread Jonas Devlieghere via lldb-commits

https://github.com/JDevlieghere closed 
https://github.com/llvm/llvm-project/pull/90917
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Create a single Severity enum in lldb-enumerations (PR #90917)

2024-05-03 Thread Jonas Devlieghere via lldb-commits

https://github.com/JDevlieghere updated 
https://github.com/llvm/llvm-project/pull/90917

>From 23b16ba8418f03dd11190798ccddf218cbfaf3f1 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Thu, 2 May 2024 16:44:18 -0700
Subject: [PATCH 1/2] [lldb] Create a single Severity enum in lldb-enumerations

We have 3 different enums all expressing severity (info, warning,
error). Remove all uses with a new Severity enum in lldb-enumerations.h.
---
 lldb/include/lldb/Core/Debugger.h |  3 +-
 lldb/include/lldb/Core/DebuggerEvents.h   | 14 ++---
 .../lldb/Expression/DiagnosticManager.h   | 18 ++
 lldb/include/lldb/Host/Host.h |  9 +--
 lldb/include/lldb/lldb-enumerations.h |  6 ++
 lldb/source/Core/Debugger.cpp | 37 ++--
 lldb/source/Core/DebuggerEvents.cpp   | 10 ++--
 lldb/source/Expression/DiagnosticManager.cpp  | 16 ++---
 lldb/source/Expression/FunctionCaller.cpp | 25 
 lldb/source/Expression/LLVMUserExpression.cpp | 39 ++---
 lldb/source/Host/common/Host.cpp  | 22 +++
 lldb/source/Host/macosx/objcxx/Host.mm| 10 ++--
 .../ExpressionParser/Clang/ClangDiagnostic.h  |  2 +-
 .../Clang/ClangExpressionDeclMap.cpp  |  3 +-
 .../Clang/ClangExpressionParser.cpp   | 14 ++---
 .../Clang/ClangFunctionCaller.cpp |  4 +-
 .../Clang/ClangUserExpression.cpp | 34 +--
 .../Clang/ClangUtilityFunction.cpp| 15 +++--
 lldb/source/Target/Process.cpp| 27 +
 .../Expression/DiagnosticManagerTest.cpp  | 58 +++
 20 files changed, 163 insertions(+), 203 deletions(-)

diff --git a/lldb/include/lldb/Core/Debugger.h 
b/lldb/include/lldb/Core/Debugger.h
index 49ff0737acef82..c0f7c732ad2d46 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -630,8 +630,7 @@ class Debugger : public 
std::enable_shared_from_this,
  std::optional debugger_id,
  uint32_t progress_category_bit = eBroadcastBitProgress);
 
-  static void ReportDiagnosticImpl(DiagnosticEventData::Type type,
-   std::string message,
+  static void ReportDiagnosticImpl(lldb::Severity severity, std::string 
message,
std::optional debugger_id,
std::once_flag *once);
 
diff --git a/lldb/include/lldb/Core/DebuggerEvents.h 
b/lldb/include/lldb/Core/DebuggerEvents.h
index 74bb05e6e6bf88..49a4ecf8e537e3 100644
--- a/lldb/include/lldb/Core/DebuggerEvents.h
+++ b/lldb/include/lldb/Core/DebuggerEvents.h
@@ -76,19 +76,15 @@ class ProgressEventData : public EventData {
 
 class DiagnosticEventData : public EventData {
 public:
-  enum class Type {
-Info,
-Warning,
-Error,
-  };
-  DiagnosticEventData(Type type, std::string message, bool debugger_specific)
-  : m_message(std::move(message)), m_type(type),
+  DiagnosticEventData(lldb::Severity severity, std::string message,
+  bool debugger_specific)
+  : m_message(std::move(message)), m_severity(severity),
 m_debugger_specific(debugger_specific) {}
   ~DiagnosticEventData() override = default;
 
   const std::string () const { return m_message; }
   bool IsDebuggerSpecific() const { return m_debugger_specific; }
-  Type GetType() const { return m_type; }
+  lldb::Severity GetSeverity() const { return m_severity; }
 
   llvm::StringRef GetPrefix() const;
 
@@ -105,7 +101,7 @@ class DiagnosticEventData : public EventData {
 
 protected:
   std::string m_message;
-  Type m_type;
+  lldb::Severity m_severity;
   const bool m_debugger_specific;
 
   DiagnosticEventData(const DiagnosticEventData &) = delete;
diff --git a/lldb/include/lldb/Expression/DiagnosticManager.h 
b/lldb/include/lldb/Expression/DiagnosticManager.h
index 06bf1d115f1541..d49b7c99b114fb 100644
--- a/lldb/include/lldb/Expression/DiagnosticManager.h
+++ b/lldb/include/lldb/Expression/DiagnosticManager.h
@@ -28,12 +28,6 @@ enum DiagnosticOrigin {
   eDiagnosticOriginLLVM
 };
 
-enum DiagnosticSeverity {
-  eDiagnosticSeverityError,
-  eDiagnosticSeverityWarning,
-  eDiagnosticSeverityRemark
-};
-
 const uint32_t LLDB_INVALID_COMPILER_ID = UINT32_MAX;
 
 class Diagnostic {
@@ -55,7 +49,7 @@ class Diagnostic {
 }
   }
 
-  Diagnostic(llvm::StringRef message, DiagnosticSeverity severity,
+  Diagnostic(llvm::StringRef message, lldb::Severity severity,
  DiagnosticOrigin origin, uint32_t compiler_id)
   : m_message(message), m_severity(severity), m_origin(origin),
 m_compiler_id(compiler_id) {}
@@ -68,7 +62,7 @@ class Diagnostic {
 
   virtual bool HasFixIts() const { return false; }
 
-  DiagnosticSeverity GetSeverity() const { return m_severity; }
+  lldb::Severity GetSeverity() const { return m_severity; }
 
   uint32_t GetCompilerID() const { return m_compiler_id; }
 
@@ -83,7 +77,7 @@ class Diagnostic {
 
 

[Lldb-commits] [lldb] [lldb] Create a single Severity enum in lldb-enumerations (PR #90917)

2024-05-03 Thread Jonas Devlieghere via lldb-commits


@@ -1347,6 +1347,12 @@ enum DebuggerBroadcastBit {
   eBroadcastBitProgressCategory = (1 << 3),
 };
 
+enum Severity {
+  eSeverityError,
+  eSeverityWarning,
+  eSeverityInfo,

JDevlieghere wrote:

I considered the latter, but we use `eSeverityRemark` so infrequently and not 
even always for `DiagnosticsEngine::Level::Remark` that I decided against it. 
The comment however seems like a good compromise.  

https://github.com/llvm/llvm-project/pull/90917
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Create a single Severity enum in lldb-enumerations (PR #90917)

2024-05-03 Thread Jonas Devlieghere via lldb-commits


@@ -1347,6 +1347,12 @@ enum DebuggerBroadcastBit {
   eBroadcastBitProgressCategory = (1 << 3),
 };
 
+enum Severity {

JDevlieghere wrote:

It would, but `lldb-enumerations.h` exclusively uses `enum` and I think 
consistency outweighs the small benefit of using `enum class`. FWIW this came 
up previously in 
https://github.com/llvm/llvm-project/pull/80167#issuecomment-2043721298 too. 

https://github.com/llvm/llvm-project/pull/90917
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Create a single Severity enum in lldb-enumerations (PR #90917)

2024-05-03 Thread Adrian Prantl via lldb-commits

https://github.com/adrian-prantl approved this pull request.


https://github.com/llvm/llvm-project/pull/90917
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Create a single Severity enum in lldb-enumerations (PR #90917)

2024-05-03 Thread Adrian Prantl via lldb-commits


@@ -1347,6 +1347,12 @@ enum DebuggerBroadcastBit {
   eBroadcastBitProgressCategory = (1 << 3),
 };
 
+enum Severity {

adrian-prantl wrote:

Would `enum class` prevent people from accidentally casting this to an 
unrelated enum with similar purpose but potentially different values?

https://github.com/llvm/llvm-project/pull/90917
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Create a single Severity enum in lldb-enumerations (PR #90917)

2024-05-03 Thread Adrian Prantl via lldb-commits


@@ -1347,6 +1347,12 @@ enum DebuggerBroadcastBit {
   eBroadcastBitProgressCategory = (1 << 3),
 };
 
+enum Severity {
+  eSeverityError,
+  eSeverityWarning,
+  eSeverityInfo,

adrian-prantl wrote:

Or even `eSeverityRemark = eSeverityInfo`

https://github.com/llvm/llvm-project/pull/90917
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Create a single Severity enum in lldb-enumerations (PR #90917)

2024-05-03 Thread Adrian Prantl via lldb-commits


@@ -1347,6 +1347,12 @@ enum DebuggerBroadcastBit {
   eBroadcastBitProgressCategory = (1 << 3),
 };
 
+enum Severity {
+  eSeverityError,
+  eSeverityWarning,
+  eSeverityInfo,

adrian-prantl wrote:

Can you comment that "Clang also calls these Remarks, it's conceptually the 
same level"

https://github.com/llvm/llvm-project/pull/90917
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

2024-05-03 Thread Zequan Wu via lldb-commits

https://github.com/ZequanWu edited 
https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

2024-05-03 Thread Zequan Wu via lldb-commits

https://github.com/ZequanWu edited 
https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Unify CalculateMD5 return types (PR #90921)

2024-05-03 Thread Jonas Devlieghere via lldb-commits

https://github.com/JDevlieghere approved this pull request.

Thanks! LGTM.

https://github.com/llvm/llvm-project/pull/90921
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] LLDB Debuginfod tests and a fix or two (PR #90622)

2024-05-03 Thread David Spickett via lldb-commits


@@ -87,8 +105,15 @@ SymbolVendorELF::CreateInstance(const lldb::ModuleSP 
_sp,
   FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths();
   FileSpec dsym_fspec =
   PluginManager::LocateExecutableSymbolFile(module_spec, search_paths);
-  if (!dsym_fspec)
-return nullptr;
+  if (!dsym_fspec || IsDwpSymbolFile(module_sp, dsym_fspec)) {
+// If we have a stripped binary or if we got a DWP file, we should prefer
+// symbols in the executable acquired through a plugin.
+ModuleSpec unstripped_spec =
+PluginManager::LocateExecutableObjectFile(module_spec);
+if (!unstripped_spec)
+  return nullptr;
+dsym_fspec = unstripped_spec.GetFileSpec();
+  }

DavidSpickett wrote:

We try to allocate memory to JIT the expression and calling `mmap` fails with a 
signal. In https://github.com/llvm/llvm-project/issues/68987 this was because 
we had lost the section info that told us whether to call it as Thumb or Arm, 
if we get that wrong it causes a SIGILL, so it could be the same thing again.

I will look into it more next week and assuming I find a fix, reland the 
changes for you.

https://github.com/llvm/llvm-project/pull/90622
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [lld] [lldb] [llvm] [mlir] [openmp] [pstl] Finally formalise our defacto line-ending policy (PR #86318)

2024-05-03 Thread via lldb-commits

ldrumm wrote:

> There are a bunch of tests that fail if you don't.

Yes. Even better than that: this patch just uncovered a [legitimate bug in the 
C++ AST parser tests for clang

> Our instructions tell people they need to disable autocrlf

I didn't see that. Shall I remove that instruction now, or after we've merged?

> I don't know that this PR is perfect, but IMO it is a huge step in the right 
> direction to allow Windows contributors to build, test and contribute to LLVM 
> using the default configurations of the OS and tools.

Absolutely. This patch is not a panacea but it enables us to correctly 
formalize the case where we actually *do* care about line-endings. Before this 
patch, it was basically incredibly brittle, but now we actually have control 
for those tests that matter. For things that don't really matter a whole class 
of irritating paper-cuts go away forever.

I understand folks' hesitance around downstream rebases, but since this will go 
in as two commits, resolving a merge conflict becomes an easy step (I've [given 
instructions in the commit that will cause 
conflicts](https://github.com/llvm/llvm-project/pull/86318/commits/0b70d26534ac788741dc412777fa3396f8c1407c)).
 I've bumped downstream llvm projects through many major releases in my life 
(llvm v3 through 13), and change-of-method-name has been a much greater source 
of merge conflicts than whitespace. LLVM's codebase is healthy and clean 
largely because we give people the ability to making sweeping improvements. In 
any case, this will be the last time that a downstream will need to resolve 
major CRLF changes during a bump, so I consider that a long-term win too.

https://github.com/llvm/llvm-project/pull/86318
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] Add new Python API `SBCommandInterpreter::GetTranscript()` (PR #90703)

2024-05-03 Thread via lldb-commits

https://github.com/royitaqi edited 
https://github.com/llvm/llvm-project/pull/90703
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb-dap] Fix test_exit_status_message_sigterm test. (PR #90223)

2024-05-03 Thread via lldb-commits

https://github.com/jeffreytan81 closed 
https://github.com/llvm/llvm-project/pull/90223
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] bab1098 - [lldb-dap] Fix test_exit_status_message_sigterm test. (#90223)

2024-05-03 Thread via lldb-commits

Author: Miro Bucko
Date: 2024-05-03T08:10:02-07:00
New Revision: bab10981f3c7a15231b8e2008f99a38a5582e3b0

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

LOG: [lldb-dap] Fix test_exit_status_message_sigterm test. (#90223)

Summary:
'test_exit_status_message_sigterm' is failing due to 'psutil' dependency
introduced in PR #89405. This fix removes 'deque' dependency and checks
if 'psutil' can be imported before running the test. If 'psutil' cannot
be imported, it emits a warning and skips the test.

Test Plan:
./bin/llvm-lit -sv
/path-to-llvm-project/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py
--filter=tools/lldb-dap/console/TestDAP_console.py

Reviewers:
@jeffreytan81,@clayborg,@kusmour, @JDevlieghere,@walter-erquinigo

Subscribers:

Tasks:
lldb-dap

Tags:

Added: 


Modified: 
lldb/test/API/tools/lldb-dap/console/TestDAP_console.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py 
b/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py
index 8f456aaf890c7f..8769f39633e62f 100644
--- a/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py
+++ b/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py
@@ -4,17 +4,15 @@
 
 import dap_server
 import lldbdap_testcase
-import psutil
-from collections import deque
 from lldbsuite.test import lldbutil
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 
 
-def get_subprocess(process_name):
-queue = deque([psutil.Process(os.getpid())])
+def get_subprocess(root_process, process_name):
+queue = [root_process]
 while queue:
-process = queue.popleft()
+process = queue.pop()
 if process.name() == process_name:
 return process
 queue.extend(process.children())
@@ -131,7 +129,17 @@ def test_exit_status_message_sigterm(self):
 process_name = (
 "debugserver" if platform.system() in ["Darwin"] else "lldb-server"
 )
-process = get_subprocess(process_name)
+
+try:
+import psutil
+except ImportError:
+print(
+"psutil not installed, please install using 'pip install 
psutil'. "
+"Skipping test_exit_status_message_sigterm test.",
+file=sys.stderr,
+)
+return
+process = get_subprocess(psutil.Process(os.getpid()), process_name)
 process.terminate()
 process.wait()
 



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


[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

2024-05-03 Thread Zequan Wu via lldb-commits


@@ -1664,13 +1793,40 @@ DWARFASTParserClang::ParseStructureLikeDIE(const 
SymbolContext ,
 }
 
 if (dwarf->GetUniqueDWARFASTTypeMap().Find(
-unique_typename, die, unique_decl, attrs.byte_size.value_or(-1),
-*unique_ast_entry_up)) {
+unique_typename, die, unique_decl, byte_size,
+attrs.is_forward_declaration, *unique_ast_entry_up)) {
   type_sp = unique_ast_entry_up->m_type_sp;
   if (type_sp) {
 dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
 LinkDeclContextToDIE(
 GetCachedClangDeclContextForDIE(unique_ast_entry_up->m_die), die);
+if (!attrs.is_forward_declaration) {
+  // If the parameter DIE is definition and the entry in the map is

ZequanWu wrote:

Done.

https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

2024-05-03 Thread Zequan Wu via lldb-commits


@@ -1921,38 +1970,33 @@ DWARFASTParserClang::ParseStructureLikeDIE(const 
SymbolContext ,
   GetClangASTImporter().SetRecordLayout(record_decl, layout);
 }
   }
-} else if (clang_type_was_created) {
-  // Start the definition if the class is not objective C since the
-  // underlying decls respond to isCompleteDefinition(). Objective
-  // C decls don't respond to isCompleteDefinition() so we can't
-  // start the declaration definition right away. For C++
-  // class/union/structs we want to start the definition in case the
-  // class is needed as the declaration context for a contained class
-  // or type without the need to complete that type..
-
-  if (attrs.class_language != eLanguageTypeObjC &&
-  attrs.class_language != eLanguageTypeObjC_plus_plus)
-TypeSystemClang::StartTagDeclarationDefinition(clang_type);
-
-  // Leave this as a forward declaration until we need to know the
-  // details of the type. lldb_private::Type will automatically call
-  // the SymbolFile virtual function
-  // "SymbolFileDWARF::CompleteType(Type *)" When the definition
-  // needs to be defined.
-  assert(!dwarf->GetForwardDeclCompilerTypeToDIE().count(
- ClangUtil::RemoveFastQualifiers(clang_type)
- .GetOpaqueQualType()) &&
- "Type already in the forward declaration map!");
-  // Can't assume m_ast.GetSymbolFile() is actually a
-  // SymbolFileDWARF, it can be a SymbolFileDWARFDebugMap for Apple
-  // binaries.
-  dwarf->GetForwardDeclCompilerTypeToDIE().try_emplace(
-  ClangUtil::RemoveFastQualifiers(clang_type).GetOpaqueQualType(),
-  *die.GetDIERef());
-  m_ast.SetHasExternalStorage(clang_type.GetOpaqueQualType(), true);
 }
+// Start the definition if the class is not objective C since the
+// underlying decls respond to isCompleteDefinition(). Objective
+// C decls don't respond to isCompleteDefinition() so we can't
+// start the declaration definition right away. For C++
+// class/union/structs we want to start the definition in case the
+// class is needed as the declaration context for a contained class
+// or type without the need to complete that type..
+
+if (attrs.class_language != eLanguageTypeObjC &&
+attrs.class_language != eLanguageTypeObjC_plus_plus)
+  TypeSystemClang::StartTagDeclarationDefinition(clang_type);
   }
 
+  // Leave this as a forward declaration until we need to know the

ZequanWu wrote:

I didn't modify the original comments. It doesn't make sense to me as well. I 
append "If this is a declaration DIE" in front of this comment. 

https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

2024-05-03 Thread Zequan Wu via lldb-commits

https://github.com/ZequanWu commented:

> Though this is mostly meant to be "NFC" (with very large quotes)

Yeah, this is mostly "NFC". A noticeable difference is we now set the type 
created from declaration with `TypeSystemClang::SetHasExternalStorage` without 
knowing if there's a definition or not. Based on my knowledge, it simply tells 
clang that the type can be completed by external AST source and ultimately 
calls `SymbolFileDWARF::CompleteType` to complete it. If there isn't one, we 
just fail complete it. Maybe we should also force completion of the type in 
this case?

https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

2024-05-03 Thread Zequan Wu via lldb-commits


@@ -108,6 +108,9 @@ class DWARFASTParserClang : public 
lldb_private::plugin::dwarf::DWARFASTParser {
   lldb_private::ConstString GetDIEClassTemplateParams(
   const lldb_private::plugin::dwarf::DWARFDIE ) override;

ZequanWu wrote:

Done.

https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

2024-05-03 Thread Zequan Wu via lldb-commits


@@ -1921,38 +1970,33 @@ DWARFASTParserClang::ParseStructureLikeDIE(const 
SymbolContext ,
   GetClangASTImporter().SetRecordLayout(record_decl, layout);
 }
   }
-} else if (clang_type_was_created) {
-  // Start the definition if the class is not objective C since the
-  // underlying decls respond to isCompleteDefinition(). Objective
-  // C decls don't respond to isCompleteDefinition() so we can't
-  // start the declaration definition right away. For C++
-  // class/union/structs we want to start the definition in case the
-  // class is needed as the declaration context for a contained class
-  // or type without the need to complete that type..
-
-  if (attrs.class_language != eLanguageTypeObjC &&
-  attrs.class_language != eLanguageTypeObjC_plus_plus)
-TypeSystemClang::StartTagDeclarationDefinition(clang_type);
-
-  // Leave this as a forward declaration until we need to know the
-  // details of the type. lldb_private::Type will automatically call
-  // the SymbolFile virtual function
-  // "SymbolFileDWARF::CompleteType(Type *)" When the definition
-  // needs to be defined.
-  assert(!dwarf->GetForwardDeclCompilerTypeToDIE().count(
- ClangUtil::RemoveFastQualifiers(clang_type)
- .GetOpaqueQualType()) &&
- "Type already in the forward declaration map!");
-  // Can't assume m_ast.GetSymbolFile() is actually a
-  // SymbolFileDWARF, it can be a SymbolFileDWARFDebugMap for Apple
-  // binaries.
-  dwarf->GetForwardDeclCompilerTypeToDIE().try_emplace(
-  ClangUtil::RemoveFastQualifiers(clang_type).GetOpaqueQualType(),
-  *die.GetDIERef());
-  m_ast.SetHasExternalStorage(clang_type.GetOpaqueQualType(), true);
 }
+// Start the definition if the class is not objective C since the
+// underlying decls respond to isCompleteDefinition(). Objective
+// C decls don't respond to isCompleteDefinition() so we can't
+// start the declaration definition right away. For C++
+// class/union/structs we want to start the definition in case the
+// class is needed as the declaration context for a contained class
+// or type without the need to complete that type..
+
+if (attrs.class_language != eLanguageTypeObjC &&
+attrs.class_language != eLanguageTypeObjC_plus_plus)
+  TypeSystemClang::StartTagDeclarationDefinition(clang_type);

ZequanWu wrote:

Oh, I didn't notice it. Moved this into an else branch.

https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

2024-05-03 Thread Zequan Wu via lldb-commits


@@ -60,6 +60,12 @@ class DWARFASTParser {
 
   virtual ConstString GetDIEClassTemplateParams(const DWARFDIE ) = 0;
 
+  // Return true if we found the definition DIE for it. is_forward_declaration
+  // is set to true if the parameter die is a declaration.
+  virtual bool
+  FindDefinitionDIE(const lldb_private::plugin::dwarf::DWARFDIE ,

ZequanWu wrote:

Done.

https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

2024-05-03 Thread Zequan Wu via lldb-commits


@@ -1632,6 +1669,96 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const 
DWARFDIE ) {
   return qualified_name;
 }
 

ZequanWu wrote:

Yes, it does more than finding the DIE. Updated to return `Type*` and renamed 
to `FindDefinitionTypeForDIE`.

https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

2024-05-03 Thread Zequan Wu via lldb-commits


@@ -16,60 +16,65 @@ using namespace lldb_private::plugin::dwarf;
 bool UniqueDWARFASTTypeList::Find(const DWARFDIE ,
   const lldb_private::Declaration ,
   const int32_t byte_size,
+  bool is_forward_declaration,
   UniqueDWARFASTType ) const {
   for (const UniqueDWARFASTType  : m_collection) {
 // Make sure the tags match
 if (udt.m_die.Tag() == die.Tag()) {
-  // Validate byte sizes of both types only if both are valid.
-  if (udt.m_byte_size < 0 || byte_size < 0 ||
-  udt.m_byte_size == byte_size) {
-// Make sure the file and line match
-if (udt.m_declaration == decl) {
-  // The type has the same name, and was defined on the same file and
-  // line. Now verify all of the parent DIEs match.
-  DWARFDIE parent_arg_die = die.GetParent();
-  DWARFDIE parent_pos_die = udt.m_die.GetParent();
-  bool match = true;
-  bool done = false;
-  while (!done && match && parent_arg_die && parent_pos_die) {
-const dw_tag_t parent_arg_tag = parent_arg_die.Tag();
-const dw_tag_t parent_pos_tag = parent_pos_die.Tag();
-if (parent_arg_tag == parent_pos_tag) {
-  switch (parent_arg_tag) {
-  case DW_TAG_class_type:
-  case DW_TAG_structure_type:
-  case DW_TAG_union_type:
-  case DW_TAG_namespace: {
-const char *parent_arg_die_name = parent_arg_die.GetName();
-if (parent_arg_die_name ==
-nullptr) // Anonymous (i.e. no-name) struct
-{
+  // If they are not both definition DIEs or both declaration DIEs, then
+  // don't check for byte size and declaration location, because 
declaration
+  // DIEs usually don't have those info.
+  bool matching_size_declaration =
+  udt.m_is_forward_declaration != is_forward_declaration
+  ? true
+  : (udt.m_byte_size < 0 || byte_size < 0 ||
+ udt.m_byte_size == byte_size) &&
+udt.m_declaration == decl;
+  if (matching_size_declaration) {

ZequanWu wrote:

Done.

https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

2024-05-03 Thread Zequan Wu via lldb-commits


@@ -1632,6 +1669,96 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const 
DWARFDIE ) {
   return qualified_name;
 }
 
+bool DWARFASTParserClang::FindDefinitionDIE(const DWARFDIE ,
+bool _forward_declaration) {

ZequanWu wrote:

I was about to use it in SymbolFileDWARF::CompleteType: 
https://github.com/llvm/llvm-project/pull/90663/files#diff-edef3a65d5d569bbb75a4158d35b827aa5d42ee03ccd3b0c1d4f354afa12210cR1645.
 But with some refactor change, it's no longer useful anymore. 

Removed.

https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

2024-05-03 Thread Zequan Wu via lldb-commits


@@ -249,11 +270,10 @@ static void ForcefullyCompleteType(CompilerType type) {
 /// This function serves a similar purpose as RequireCompleteType above, but it
 /// avoids completing the type if it is not immediately necessary. It only
 /// ensures we _can_ complete the type later.
-static void PrepareContextToReceiveMembers(TypeSystemClang ,
-   ClangASTImporter _importer,
-   clang::DeclContext *decl_ctx,
-   DWARFDIE die,
-   const char *type_name_cstr) {
+void DWARFASTParserClang::PrepareContextToReceiveMembers(
+TypeSystemClang , clang::DeclContext *decl_ctx,

ZequanWu wrote:

Done.

https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

2024-05-03 Thread Zequan Wu via lldb-commits

https://github.com/ZequanWu edited 
https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

2024-05-03 Thread Zequan Wu via lldb-commits

https://github.com/ZequanWu updated 
https://github.com/llvm/llvm-project/pull/90663

>From 4e83099b593e66f12dc21be5fbac5279e03e Mon Sep 17 00:00:00 2001
From: Zequan Wu 
Date: Tue, 30 Apr 2024 16:23:11 -0400
Subject: [PATCH 1/4] [lldb][DWARF] Delay struct/class/union definition DIE
 searching when parsing declaration DIEs.

---
 .../SymbolFile/DWARF/DWARFASTParserClang.cpp  | 270 +++---
 .../SymbolFile/DWARF/SymbolFileDWARF.cpp  | 105 ++-
 .../SymbolFile/DWARF/SymbolFileDWARF.h|  14 +
 .../SymbolFile/DWARF/SymbolFileDWARFDwo.cpp   |   5 +
 .../SymbolFile/DWARF/SymbolFileDWARFDwo.h |   2 +
 .../SymbolFile/DWARF/UniqueDWARFASTType.cpp   |  95 +++---
 .../SymbolFile/DWARF/UniqueDWARFASTType.h |  21 +-
 7 files changed, 296 insertions(+), 216 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index bea11e0e3840af..7ad661c9a9d49b 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -252,7 +252,8 @@ static void ForcefullyCompleteType(CompilerType type) {
 static void PrepareContextToReceiveMembers(TypeSystemClang ,
ClangASTImporter _importer,
clang::DeclContext *decl_ctx,
-   DWARFDIE die,
+   const DWARFDIE _ctx_die,
+   const DWARFDIE ,
const char *type_name_cstr) {
   auto *tag_decl_ctx = clang::dyn_cast(decl_ctx);
   if (!tag_decl_ctx)
@@ -279,6 +280,13 @@ static void PrepareContextToReceiveMembers(TypeSystemClang 
,
 type_name_cstr ? type_name_cstr : "", die.GetOffset());
   }
 
+  // By searching for the definition DIE of the decl_ctx type, we will either:
+  // 1. Found the the definition DIE and start its definition with
+  // TypeSystemClang::StartTagDeclarationDefinition.
+  // 2. Unable to find it, then need to forcefully complete it.
+  die.GetDWARF()->FindDefinitionDIE(decl_ctx_die);
+  if (tag_decl_ctx->isCompleteDefinition() || tag_decl_ctx->isBeingDefined())
+return;
   // We don't have a type definition and/or the import failed. We must
   // forcefully complete the type to avoid crashes.
   ForcefullyCompleteType(type);
@@ -620,10 +628,11 @@ DWARFASTParserClang::ParseTypeModifier(const 
SymbolContext ,
   if (tag == DW_TAG_typedef) {
 // DeclContext will be populated when the clang type is materialized in
 // Type::ResolveCompilerType.
-PrepareContextToReceiveMembers(
-m_ast, GetClangASTImporter(),
-GetClangDeclContextContainingDIE(die, nullptr), die,
-attrs.name.GetCString());
+DWARFDIE decl_ctx_die;
+clang::DeclContext *decl_ctx =
+GetClangDeclContextContainingDIE(die, _ctx_die);
+PrepareContextToReceiveMembers(m_ast, GetClangASTImporter(), decl_ctx,
+   decl_ctx_die, die, attrs.name.GetCString());
 
 if (attrs.type.IsValid()) {
   // Try to parse a typedef from the (DWARF embedded in the) Clang
@@ -1100,32 +1109,6 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE ,
 // struct and see if this is actually a C++ method
 Type *class_type = dwarf->ResolveType(decl_ctx_die);
 if (class_type) {
-  if (class_type->GetID() != decl_ctx_die.GetID() ||
-  IsClangModuleFwdDecl(decl_ctx_die)) {
-
-// We uniqued the parent class of this function to another
-// class so we now need to associate all dies under
-// "decl_ctx_die" to DIEs in the DIE for "class_type"...
-DWARFDIE class_type_die = dwarf->GetDIE(class_type->GetID());
-
-if (class_type_die) {
-  std::vector failures;
-
-  CopyUniqueClassMethodTypes(decl_ctx_die, class_type_die,
- class_type, failures);
-
-  // FIXME do something with these failures that's
-  // smarter than just dropping them on the ground.
-  // Unfortunately classes don't like having stuff added
-  // to them after their definitions are complete...
-
-  Type *type_ptr = dwarf->GetDIEToType()[die.GetDIE()];
-  if (type_ptr && type_ptr != DIE_IS_BEING_PARSED) {
-return type_ptr->shared_from_this();
-  }
-}
-  }
-
   if (attrs.specification.IsValid()) {
 // We have a specification which we are going to base our
 // function prototype off of, so we need this type to be
@@ -1260,6 +1243,39 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE ,
   }
 }
   }
+  // By here, we should have already completed the c++ class_type
+  // 

[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [lld] [lldb] [llvm] [mlir] [openmp] [pstl] Finally formalise our defacto line-ending policy (PR #86318)

2024-05-03 Thread Chris B via lldb-commits

llvm-beanz wrote:


> I don't know if the pre-commit testing guarantees that. Configuration 
> settings will permit the files to be checked out in either Unix (`\n`) or 
> Windows (`\r\n`) line-endings.

Today on Windows you basically have to check out LLVM as unix line endings. 
There are a bunch of tests that fail if you don't. Hopefully this is a step 
toward fixing that (which is why I'm in favor of this).

> If the builders are all configured to run in Unix line endings we lose that 
> coverage. 

I suspect the Windows bots are basically all configured that way today. Our 
instructions tell people they need to disable `autocrlf` (see: 
https://llvm.org/docs/GettingStarted.html#getting-the-source-code-and-building-llvm),
 and not disabling it causes problems for users still (see this message from 
today: 
https://discord.com/channels/636084430946959380/745925509971705877/1235913415902629958).

>While I understand that this change only changes how the LLVM sources are 
>stored, the issue is that the LLVM sources include the _tests_ directory. 
>These need to be tested in various manners to ensure that we test how we 
>handle the different inputs (in clang). One option might be to exclude the 
>tests directory from the line ending conversion if we cannot verify that we 
>have tests in both formats.

I see two issues with this:

1) because the repo doesn't work with autocrlf today, we actually can't rely on 
testing on different platforms to provide this coverage.
2) We shouldn't be relying on git settings to get this coverage anyways. We 
should have tests that require specific line endings, and we should use 
gitattributes to ensure that we run those tests regardless of the user's git 
settings.

> While, yes, it is possible to get the wrong behaviour currently, if the user 
> configures git explicitly for a line-ending, I would expect them to be aware 
> of that. The use of `.gitattributes` means that their defaults are not 
> necessarily honoured and thus this can catch them by surprise.

I think today not only is it possible to get the wrong behavior, if you're 
developing on Windows Git's default behavior causes the wrong behavior for 
LLVM. Which often means that contributors (especially new contributors) have no 
idea that they're doing something wrong.

I don't know that this PR is perfect, but IMO it is a huge step in the right 
direction to allow Windows contributors to build, test and contribute to LLVM 
using the default configurations of the OS and tools.

https://github.com/llvm/llvm-project/pull/86318
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Add SBType::GetByteAlign (PR #90960)

2024-05-03 Thread Michael Buch via lldb-commits

https://github.com/Michael137 approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/90960
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

2024-05-03 Thread Michael Buch via lldb-commits

Michael137 wrote:

> > Is any of it testable?
> 
> Good question. Though this is mostly meant to be "NFC" (with very large 
> quotes), I can imagine us doing something like forcing the parsing of a 
> specific type (`type lookup ` ?), and then checking that the 
> module ast (`image dump ast`) does _not_ contain specific types -- as that's 
> basically what we're trying to achieve.

Yea that could work. But if it's going to be very painful or fragile to test 
then don't let that hold back the PR

https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

2024-05-03 Thread Michael Buch via lldb-commits

https://github.com/Michael137 edited 
https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] LLDB Debuginfod tests and a fix or two (PR #90622)

2024-05-03 Thread David Spickett via lldb-commits

https://github.com/DavidSpickett edited 
https://github.com/llvm/llvm-project/pull/90622
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] LLDB Debuginfod tests and a fix or two (PR #90622)

2024-05-03 Thread David Spickett via lldb-commits


@@ -87,8 +105,15 @@ SymbolVendorELF::CreateInstance(const lldb::ModuleSP 
_sp,
   FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths();
   FileSpec dsym_fspec =
   PluginManager::LocateExecutableSymbolFile(module_spec, search_paths);
-  if (!dsym_fspec)
-return nullptr;
+  if (!dsym_fspec || IsDwpSymbolFile(module_sp, dsym_fspec)) {
+// If we have a stripped binary or if we got a DWP file, we should prefer
+// symbols in the executable acquired through a plugin.
+ModuleSpec unstripped_spec =
+PluginManager::LocateExecutableObjectFile(module_spec);
+if (!unstripped_spec)
+  return nullptr;
+dsym_fspec = unstripped_spec.GetFileSpec();
+  }

DavidSpickett wrote:

Something about this if block has broken `Expr/TestStringLiteralExpr.test` on 
Arm 32 bit, a platform that has been sensitive to changes in this area in the 
past. The test is built without debug info and something happens when the test 
file comes through here. For ld.so and the libc it's all the same as before.

```
(lldb) expr "hello there"
lldb ProcessGDBRemote::DoAllocateMemory no direct stub support for 
memory allocation, and InferiorCallMmap also failed - is stub missing register 
context save/restore capability?
```

Last time this happened it was because we lost the symbols that let us call 
`mmap`, but those are in ld.so usually so I'm not sure what's the problem this 
time.

This means we fall back the interpreter, which we didn't need to do before.
```
error: Can't evaluate the expression without a running target due to: 
Interpreter doesn't handle one of the expression's operands
```
I've reverted this 
(https://github.com/llvm/llvm-project/commit/327bfc971e4dce3f6798843c92406cda95c07ba1)
 and the follow up while I investigate locally.

https://github.com/llvm/llvm-project/pull/90622
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 327bfc9 - Revert "[lldb] Fix TestSharedLibStrippedSymbols for #90622"

2024-05-03 Thread David Spickett via lldb-commits

Author: David Spickett
Date: 2024-05-03T13:04:05Z
New Revision: 327bfc971e4dce3f6798843c92406cda95c07ba1

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

LOG: Revert "[lldb] Fix TestSharedLibStrippedSymbols for #90622"

And "LLDB Debuginfod tests and a fix or two (#90622)".

f8fedfb6802173372ec923f99f31d4af810fbcb0 /
2d4acb086541577ac6ab3a140b9ceb9659ce7094

As it has caused a test failure on 32 bit Arm:
https://lab.llvm.org/buildbot/#/builders/17/builds/52580

Expr/TestStringLiteralExpr.test. The follow up did fix
lang/c/shared_lib_stripped_symbols/TestSharedLibStrippedSymbols.py
but not the other failure.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/make/Makefile.rules
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolLocator/CMakeLists.txt
lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
lldb/test/API/lit.site.cfg.py.in
lldb/test/CMakeLists.txt

Removed: 
lldb/test/API/debuginfod/Normal/Makefile
lldb/test/API/debuginfod/Normal/TestDebuginfod.py
lldb/test/API/debuginfod/Normal/main.c
lldb/test/API/debuginfod/SplitDWARF/Makefile
lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py
lldb/test/API/debuginfod/SplitDWARF/main.c



diff  --git a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules 
b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
index b2f2516d8423f8..bd8eea3d6f5a04 100644
--- a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -51,7 +51,7 @@ LLDB_BASE_DIR := $(THIS_FILE_DIR)/../../../../../
 #
 # GNUWin32 uname gives "windows32" or "server version windows32" while
 # some versions of MSYS uname return "MSYS_NT*", but most environments
-# standardize on "Windows_NT", so we'll make it consistent here.
+# standardize on "Windows_NT", so we'll make it consistent here. 
 # When running tests from Visual Studio, the environment variable isn't
 # inherited all the way down to the process spawned for make.
 #--
@@ -210,12 +210,6 @@ else
ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES"
DSYM = $(EXE).debug
endif
-
-   ifeq "$(MAKE_DWP)" "YES"
-   MAKE_DWO := YES
-   DWP_NAME = $(EXE).dwp
-   DYLIB_DWP_NAME = $(DYLIB_NAME).dwp
-   endif
 endif
 
 LIMIT_DEBUG_INFO_FLAGS =
@@ -364,7 +358,6 @@ ifneq "$(OS)" "Darwin"
 
OBJCOPY ?= $(call replace_cc_with,objcopy)
ARCHIVER ?= $(call replace_cc_with,ar)
-   DWP ?= $(call replace_cc_with,dwp)
override AR = $(ARCHIVER)
 endif
 
@@ -535,10 +528,6 @@ ifneq "$(CXX)" ""
endif
 endif
 
-ifeq "$(GEN_GNU_BUILD_ID)" "YES"
-   LDFLAGS += -Wl,--build-id
-endif
-
 #--
 # DYLIB_ONLY variable can be used to skip the building of a.out.
 # See the sections below regarding dSYM file as well as the building of
@@ -577,17 +566,10 @@ else
 endif
 else
 ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES"
-ifeq "$(SAVE_FULL_DEBUG_BINARY)" "YES"
-   cp "$(EXE)" "$(EXE).unstripped"
-endif
$(OBJCOPY) --only-keep-debug "$(EXE)" "$(DSYM)"
$(OBJCOPY) --strip-debug --add-gnu-debuglink="$(DSYM)" "$(EXE)" "$(EXE)"
 endif
-ifeq "$(MAKE_DWP)" "YES"
-   $(DWP) -o "$(DWP_NAME)" $(DWOS)
 endif
-endif
-
 
 #--
 # Make the dylib
@@ -629,15 +611,9 @@ endif
 else
$(LD) $(DYLIB_OBJECTS) $(LDFLAGS) -shared -o "$(DYLIB_FILENAME)"
 ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES"
-ifeq "$(SAVE_FULL_DEBUG_BINARY)" "YES"
-   cp "$(DYLIB_FILENAME)" "$(DYLIB_FILENAME).unstripped"
-endif
$(OBJCOPY) --only-keep-debug "$(DYLIB_FILENAME)" 
"$(DYLIB_FILENAME).debug"
$(OBJCOPY) --strip-debug --add-gnu-debuglink="$(DYLIB_FILENAME).debug" 
"$(DYLIB_FILENAME)" "$(DYLIB_FILENAME)"
 endif
-ifeq "$(MAKE_DWP)" "YES"
-   $(DWP) -o $(DYLIB_DWP_FILE) $(DYLIB_DWOS)
-endif
 endif
 
 #--

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index dafdf241f9db0c..49f13d2c89e380 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -4378,38 +4378,26 @@ const std::shared_ptr 
::GetDwpSymbolFile() {
 FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths();
 ModuleSpec module_spec;
 module_spec.GetFileSpec() = m_objfile_sp->GetFileSpec();
-FileSpec dwp_filespec;
 for (const auto  : symfiles.files()) {
   module_spec.GetSymbolFileSpec() =
 

[Lldb-commits] [lldb] [lldb-dap] Fix test_exit_status_message_sigterm test. (PR #90223)

2024-05-03 Thread Miro Bucko via lldb-commits

https://github.com/mbucko updated 
https://github.com/llvm/llvm-project/pull/90223

>From c699eff1500a431225d4b292a51a467bd2c74e5d Mon Sep 17 00:00:00 2001
From: Miro Bucko 
Date: Fri, 26 Apr 2024 08:17:26 -0700
Subject: [PATCH] [lldb-dap] Fix test_exit_status_message_sigterm test.

Summary:
'test_exit_status_message_sigterm' is failing due to 'psutil' dependency 
introduced in PR #89405. This fix removes 'deque' dependency and checks if 
'psutil' can be imported before running the test. If 'psutil' cannot be 
imported, it emits a warning and skips the test.

Test Plan:
./bin/llvm-lit -sv 
/path-to-llvm-project/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py 
--filter=tools/lldb-dap/console/TestDAP_console.py

Reviewers:
@jeffreytan81,@clayborg,@kusmour, @JDevlieghere,@walter-erquinigo

Subscribers:

Tasks:
lldb-dap

Tags:
---
 .../tools/lldb-dap/console/TestDAP_console.py | 20 +--
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py 
b/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py
index 8f456aaf890c7f..8769f39633e62f 100644
--- a/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py
+++ b/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py
@@ -4,17 +4,15 @@
 
 import dap_server
 import lldbdap_testcase
-import psutil
-from collections import deque
 from lldbsuite.test import lldbutil
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 
 
-def get_subprocess(process_name):
-queue = deque([psutil.Process(os.getpid())])
+def get_subprocess(root_process, process_name):
+queue = [root_process]
 while queue:
-process = queue.popleft()
+process = queue.pop()
 if process.name() == process_name:
 return process
 queue.extend(process.children())
@@ -131,7 +129,17 @@ def test_exit_status_message_sigterm(self):
 process_name = (
 "debugserver" if platform.system() in ["Darwin"] else "lldb-server"
 )
-process = get_subprocess(process_name)
+
+try:
+import psutil
+except ImportError:
+print(
+"psutil not installed, please install using 'pip install 
psutil'. "
+"Skipping test_exit_status_message_sigterm test.",
+file=sys.stderr,
+)
+return
+process = get_subprocess(psutil.Process(os.getpid()), process_name)
 process.terminate()
 process.wait()
 

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


[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

2024-05-03 Thread Pavel Labath via lldb-commits

labath wrote:

> Is any of it testable?

Good question. Though this is mostly meant to be "NFC" (with very large 
quotes), I can imagine us doing something like forcing the parsing of a 
specific type (`type lookup ` ?), and then checking that the module 
ast (`image dump ast`) does *not* contain specific types -- as that's basically 
what we're trying to achieve.

https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

2024-05-03 Thread Michael Buch via lldb-commits


@@ -16,60 +16,65 @@ using namespace lldb_private::plugin::dwarf;
 bool UniqueDWARFASTTypeList::Find(const DWARFDIE ,
   const lldb_private::Declaration ,
   const int32_t byte_size,
+  bool is_forward_declaration,
   UniqueDWARFASTType ) const {
   for (const UniqueDWARFASTType  : m_collection) {
 // Make sure the tags match
 if (udt.m_die.Tag() == die.Tag()) {
-  // Validate byte sizes of both types only if both are valid.
-  if (udt.m_byte_size < 0 || byte_size < 0 ||
-  udt.m_byte_size == byte_size) {
-// Make sure the file and line match
-if (udt.m_declaration == decl) {
-  // The type has the same name, and was defined on the same file and
-  // line. Now verify all of the parent DIEs match.
-  DWARFDIE parent_arg_die = die.GetParent();
-  DWARFDIE parent_pos_die = udt.m_die.GetParent();
-  bool match = true;
-  bool done = false;
-  while (!done && match && parent_arg_die && parent_pos_die) {
-const dw_tag_t parent_arg_tag = parent_arg_die.Tag();
-const dw_tag_t parent_pos_tag = parent_pos_die.Tag();
-if (parent_arg_tag == parent_pos_tag) {
-  switch (parent_arg_tag) {
-  case DW_TAG_class_type:
-  case DW_TAG_structure_type:
-  case DW_TAG_union_type:
-  case DW_TAG_namespace: {
-const char *parent_arg_die_name = parent_arg_die.GetName();
-if (parent_arg_die_name ==
-nullptr) // Anonymous (i.e. no-name) struct
-{
+  // If they are not both definition DIEs or both declaration DIEs, then
+  // don't check for byte size and declaration location, because 
declaration
+  // DIEs usually don't have those info.
+  bool matching_size_declaration =
+  udt.m_is_forward_declaration != is_forward_declaration
+  ? true
+  : (udt.m_byte_size < 0 || byte_size < 0 ||
+ udt.m_byte_size == byte_size) &&
+udt.m_declaration == decl;
+  if (matching_size_declaration) {

Michael137 wrote:

For readability, can we do:
```
if (!match_size_declaration)
  continue;
```
?

https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

2024-05-03 Thread Michael Buch via lldb-commits

https://github.com/Michael137 commented:

The idea makes sense and I like that we could factor things out of 
`ParseStructureLikeDIE`, so generally LGTM (module Pavel's comments). Is any of 
it testable?

https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

2024-05-03 Thread Michael Buch via lldb-commits

https://github.com/Michael137 edited 
https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

2024-05-03 Thread Michael Buch via lldb-commits


@@ -1664,13 +1793,40 @@ DWARFASTParserClang::ParseStructureLikeDIE(const 
SymbolContext ,
 }
 
 if (dwarf->GetUniqueDWARFASTTypeMap().Find(
-unique_typename, die, unique_decl, attrs.byte_size.value_or(-1),
-*unique_ast_entry_up)) {
+unique_typename, die, unique_decl, byte_size,
+attrs.is_forward_declaration, *unique_ast_entry_up)) {
   type_sp = unique_ast_entry_up->m_type_sp;
   if (type_sp) {
 dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
 LinkDeclContextToDIE(
 GetCachedClangDeclContextForDIE(unique_ast_entry_up->m_die), die);
+if (!attrs.is_forward_declaration) {
+  // If the parameter DIE is definition and the entry in the map is

Michael137 wrote:

I find `parameter DIE` a bit confusing. Made me think of templates or 
functions. Could we just reference the parameter, e.g., `If the 'die' being 
parsed is a definition ...` or something like that

https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Add SBType::GetByteAlign (PR #90960)

2024-05-03 Thread Pavel Labath via lldb-commits

https://github.com/labath updated 
https://github.com/llvm/llvm-project/pull/90960

>From f6b47d2763b8217c329c6c5762ed75420ab3579b Mon Sep 17 00:00:00 2001
From: Pavel Labath 
Date: Fri, 3 May 2024 11:18:15 +
Subject: [PATCH] [lldb] Add SBType::GetByteAlign

lldb already mostly(*) tracks this information. This just makes it
available to the SB users.

(*) It does not do that for typedefs right now see llvm.org/pr90958
---
 lldb/include/lldb/API/SBType.h|  2 ++
 lldb/source/API/SBType.cpp| 13 
 lldb/test/API/python_api/type/TestTypeList.py | 21 +++
 lldb/test/API/python_api/type/main.cpp|  3 +++
 4 files changed, 39 insertions(+)

diff --git a/lldb/include/lldb/API/SBType.h b/lldb/include/lldb/API/SBType.h
index 5b9ff2170b2b24..63ba91082d5769 100644
--- a/lldb/include/lldb/API/SBType.h
+++ b/lldb/include/lldb/API/SBType.h
@@ -150,6 +150,8 @@ class SBType {
 
   uint64_t GetByteSize();
 
+  uint64_t GetByteAlign();
+
   bool IsPointerType();
 
   bool IsReferenceType();
diff --git a/lldb/source/API/SBType.cpp b/lldb/source/API/SBType.cpp
index 6cecb5c9ea810b..8a063e5ad61d9d 100644
--- a/lldb/source/API/SBType.cpp
+++ b/lldb/source/API/SBType.cpp
@@ -25,6 +25,7 @@
 #include "lldb/Utility/Stream.h"
 
 #include "llvm/ADT/APSInt.h"
+#include "llvm/Support/MathExtras.h"
 
 #include 
 #include 
@@ -132,6 +133,18 @@ uint64_t SBType::GetByteSize() {
   return 0;
 }
 
+uint64_t SBType::GetByteAlign() {
+  LLDB_INSTRUMENT_VA(this);
+
+  if (!IsValid())
+return 0;
+
+  std::optional bit_align =
+  m_opaque_sp->GetCompilerType(/*prefer_dynamic=*/false)
+  .GetTypeBitAlign(nullptr);
+  return llvm::divideCeil(bit_align.value_or(0), 8);
+}
+
 bool SBType::IsPointerType() {
   LLDB_INSTRUMENT_VA(this);
 
diff --git a/lldb/test/API/python_api/type/TestTypeList.py 
b/lldb/test/API/python_api/type/TestTypeList.py
index 17e27b624511cf..0498396903dcbd 100644
--- a/lldb/test/API/python_api/type/TestTypeList.py
+++ b/lldb/test/API/python_api/type/TestTypeList.py
@@ -272,3 +272,24 @@ def test(self):
 self.assertTrue(int_enum_uchar)
 self.DebugSBType(int_enum_uchar)
 self.assertEqual(int_enum_uchar.GetName(), "unsigned char")
+
+def test_GetByteAlign(self):
+"""Exercise SBType::GetByteAlign"""
+self.build()
+spec = lldb.SBModuleSpec()
+spec.SetFileSpec(lldb.SBFileSpec(self.getBuildArtifact()))
+module = lldb.SBModule(spec)
+self.assertTrue(module)
+
+# Invalid types should not crash.
+self.assertEqual(lldb.SBType().GetByteAlign(), 0)
+
+# Try a type with natural alignment.
+void_ptr = module.GetBasicType(lldb.eBasicTypeVoid).GetPointerType()
+self.assertTrue(void_ptr)
+# Not exactly guaranteed by the spec, but should be true everywhere we
+# care about.
+self.assertEqual(void_ptr.GetByteSize(), void_ptr.GetByteAlign())
+
+# And an over-aligned type.
+
self.assertEqual(module.FindFirstType("OverAlignedStruct").GetByteAlign(), 128)
diff --git a/lldb/test/API/python_api/type/main.cpp 
b/lldb/test/API/python_api/type/main.cpp
index 7384a3d8da16fb..986ed3009a15f6 100644
--- a/lldb/test/API/python_api/type/main.cpp
+++ b/lldb/test/API/python_api/type/main.cpp
@@ -50,6 +50,9 @@ enum EnumType {};
 enum class ScopedEnumType {};
 enum class EnumUChar : unsigned char {};
 
+struct alignas(128) OverAlignedStruct {};
+OverAlignedStruct over_aligned_struct;
+
 int main (int argc, char const *argv[])
 {
 Task *task_head = new Task(-1, NULL);

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


[Lldb-commits] [lldb] [lldb] Add SBType::GetByteAlign (PR #90960)

2024-05-03 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Pavel Labath (labath)


Changes

lldb already mostly(*) tracks this information. This just makes it available to 
the SB users.

(*) It does not do that for typedefs right now see llvm.org/pr90958

---
Full diff: https://github.com/llvm/llvm-project/pull/90960.diff


4 Files Affected:

- (modified) lldb/include/lldb/API/SBType.h (+2) 
- (modified) lldb/source/API/SBType.cpp (+12) 
- (modified) lldb/test/API/python_api/type/TestTypeList.py (+21) 
- (modified) lldb/test/API/python_api/type/main.cpp (+3) 


``diff
diff --git a/lldb/include/lldb/API/SBType.h b/lldb/include/lldb/API/SBType.h
index 5b9ff2170b2b24..63ba91082d5769 100644
--- a/lldb/include/lldb/API/SBType.h
+++ b/lldb/include/lldb/API/SBType.h
@@ -150,6 +150,8 @@ class SBType {
 
   uint64_t GetByteSize();
 
+  uint64_t GetByteAlign();
+
   bool IsPointerType();
 
   bool IsReferenceType();
diff --git a/lldb/source/API/SBType.cpp b/lldb/source/API/SBType.cpp
index 6cecb5c9ea810b..f9a2a0548ef83a 100644
--- a/lldb/source/API/SBType.cpp
+++ b/lldb/source/API/SBType.cpp
@@ -25,6 +25,7 @@
 #include "lldb/Utility/Stream.h"
 
 #include "llvm/ADT/APSInt.h"
+#include "llvm/Support/MathExtras.h"
 
 #include 
 #include 
@@ -132,6 +133,17 @@ uint64_t SBType::GetByteSize() {
   return 0;
 }
 
+uint64_t SBType::GetByteAlign() {
+  LLDB_INSTRUMENT_VA(this);
+
+  if (!IsValid()) return 0;
+
+  std::optional bit_align =
+  m_opaque_sp->GetCompilerType(/*prefer_dynamic=*/false)
+  .GetTypeBitAlign(nullptr);
+  return llvm::divideCeil(bit_align.value_or(0), 8);
+}
+
 bool SBType::IsPointerType() {
   LLDB_INSTRUMENT_VA(this);
 
diff --git a/lldb/test/API/python_api/type/TestTypeList.py 
b/lldb/test/API/python_api/type/TestTypeList.py
index 17e27b624511cf..0498396903dcbd 100644
--- a/lldb/test/API/python_api/type/TestTypeList.py
+++ b/lldb/test/API/python_api/type/TestTypeList.py
@@ -272,3 +272,24 @@ def test(self):
 self.assertTrue(int_enum_uchar)
 self.DebugSBType(int_enum_uchar)
 self.assertEqual(int_enum_uchar.GetName(), "unsigned char")
+
+def test_GetByteAlign(self):
+"""Exercise SBType::GetByteAlign"""
+self.build()
+spec = lldb.SBModuleSpec()
+spec.SetFileSpec(lldb.SBFileSpec(self.getBuildArtifact()))
+module = lldb.SBModule(spec)
+self.assertTrue(module)
+
+# Invalid types should not crash.
+self.assertEqual(lldb.SBType().GetByteAlign(), 0)
+
+# Try a type with natural alignment.
+void_ptr = module.GetBasicType(lldb.eBasicTypeVoid).GetPointerType()
+self.assertTrue(void_ptr)
+# Not exactly guaranteed by the spec, but should be true everywhere we
+# care about.
+self.assertEqual(void_ptr.GetByteSize(), void_ptr.GetByteAlign())
+
+# And an over-aligned type.
+
self.assertEqual(module.FindFirstType("OverAlignedStruct").GetByteAlign(), 128)
diff --git a/lldb/test/API/python_api/type/main.cpp 
b/lldb/test/API/python_api/type/main.cpp
index 7384a3d8da16fb..986ed3009a15f6 100644
--- a/lldb/test/API/python_api/type/main.cpp
+++ b/lldb/test/API/python_api/type/main.cpp
@@ -50,6 +50,9 @@ enum EnumType {};
 enum class ScopedEnumType {};
 enum class EnumUChar : unsigned char {};
 
+struct alignas(128) OverAlignedStruct {};
+OverAlignedStruct over_aligned_struct;
+
 int main (int argc, char const *argv[])
 {
 Task *task_head = new Task(-1, NULL);

``




https://github.com/llvm/llvm-project/pull/90960
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


  1   2   >