[Lldb-commits] [lldb] [lldb] Unify CalculateMD5 return types (PR #91029)
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)
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)
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)
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)
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)
@@ -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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
@@ -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)
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)
@@ -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)
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)
@@ -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)
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)
@@ -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)
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)
@@ -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)
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)
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)
@@ -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)
@@ -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)
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)
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)
@@ -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)
@@ -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)
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)
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)
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)
@@ -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)
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)
@@ -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)
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)
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)
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)
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)
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)
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)
@@ -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)
@@ -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)
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)
@@ -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)
@@ -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)
@@ -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)
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)
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)
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)
@@ -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)
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)
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)
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)
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)
@@ -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)
@@ -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)
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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
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)
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)
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)
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)
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)
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)
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)
@@ -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"
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)
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)
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)
@@ -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)
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)
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)
@@ -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)
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)
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