[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)
labath wrote: I've been trying to stay out of this, as I don't think I can be objective here, but since I've been summoned, here's what I think. :) For me, the worst possible outcome of this is an incomplete transition -- having a third (maybe fourth, since I sort of also count dwim-print) expression-like syntax, which is "almost" complete, but for whatever reason (technical or political) can't replace the previous one. With that in mind, I'd try to stage this in a way that minimizes this risk. Building the whole thing in layers from the bottom up is not the best way to achieve that, I think. If I were doing this, I'd prioritise replacing the current "frame var" implementation as soon as possible. Ideally, the steps would be something like: 1. implement the minimal necessary functionality to match current "frame var" feature set. (I think that's `&`, `[]`, `*` and `->`) 2. flip the switch 3. remove the old implementation 4. add other features, one group of related operators at a time Besides managing that risk (if this fades out for whatever reason, there's less dead code around), I think this would also make it easier to review, as we could have patches like "add operator/ to the DIL", which would focus solely on that, and also come with associated tests. I think it would also make it easier to discuss things like whether we want to add modifying operations (operator++) or obviously language-specific features (dynamic_cast), both of which I think are very good questions. https://github.com/llvm/llvm-project/pull/95738 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Remove parsing recursion when searching for definition DIEs (PR #96484)
labath wrote: > > > This patch as-is is NFC? > > > > > > NFC_**I**_, I would say :) I don't think this should change the behavior in > > any way, but it's pretty hard to guarantee that. > > Sure enough - I take any claim as a statement of intent/belief, not of > something mathematically proved, etc. True, but in case of this code, even believing you know what it does may be a tough ask. :D > Perhaps a separate commit could add another RUN line to the existing test you > added to demonstrate the reason for the revert? Rather than worrying about > which comes first (the type unit patch or the delay patch) > > But in any case, I /think/ I understand why this patch doesn't need a test > (because this patch avoids the delay patch causing a crash (yeah, more > complex than that because the patch doesn't apply cleanly over this one) and > that crash already has a test committed) - thanks for the explanation. Correct. The reason for revert has already been established with the first test. I'll create a separate patch for the type unit bug, but it will be slightly more complicated than adding a RUN line, because, due to the bug, the lldb will print the wrong type. https://github.com/llvm/llvm-project/pull/96484 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 8395f9c - [lldb/DWARF] Remove parsing recursion when searching for definition DIEs (#96484)
Author: Pavel Labath Date: 2024-06-25T10:52:11+02:00 New Revision: 8395f9cecd34af8a79c96e661e46a80d0d471fb1 URL: https://github.com/llvm/llvm-project/commit/8395f9cecd34af8a79c96e661e46a80d0d471fb1 DIFF: https://github.com/llvm/llvm-project/commit/8395f9cecd34af8a79c96e661e46a80d0d471fb1.diff LOG: [lldb/DWARF] Remove parsing recursion when searching for definition DIEs (#96484) If ParseStructureLikeDIE (or ParseEnum) encountered a declaration DIE, it would call FindDefinitionTypeForDIE. This returned a fully formed type, which it achieved by recursing back into ParseStructureLikeDIE with the definition DIE. This obscured the control flow and caused us to repeat some work (e.g. the UniqueDWARFASTTypeMap lookup), but it mostly worked until we tried to delay the definition search in #90663. After this patch, the two ParseStructureLikeDIE calls were no longer recursive, but rather the second call happened as a part of the CompleteType() call. This opened the door to inconsistencies, as the second ParseStructureLikeDIE call was not aware it was called to process a definition die for an existing type. To make that possible, this patch removes the recusive type resolution from this function, and leaves just the "find definition die" functionality. After finding the definition DIE, we just go back to the original ParseStructureLikeDIE call, and have it finish the parsing process with the new DIE. While this patch is motivated by the work on delaying the definition searching, I believe it is also useful on its own. Added: Modified: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h Removed: diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 52f4d765cbbd4..f36e2af9589b8 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -39,10 +39,12 @@ #include "lldb/Utility/StreamString.h" #include "clang/AST/CXXInheritance.h" +#include "clang/AST/DeclBase.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/DeclTemplate.h" #include "clang/AST/Type.h" +#include "llvm/ADT/StringExtras.h" #include "llvm/Demangle/Demangle.h" #include @@ -835,54 +837,50 @@ DWARFASTParserClang::GetDIEClassTemplateParams(const DWARFDIE &die) { } TypeSP DWARFASTParserClang::ParseEnum(const SymbolContext &sc, - const DWARFDIE &die, + const DWARFDIE &decl_die, ParsedDWARFTypeAttributes &attrs) { Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups); - SymbolFileDWARF *dwarf = die.GetDWARF(); - const dw_tag_t tag = die.Tag(); - TypeSP type_sp; + SymbolFileDWARF *dwarf = decl_die.GetDWARF(); + const dw_tag_t tag = decl_die.Tag(); + DWARFDIE def_die; if (attrs.is_forward_declaration) { -type_sp = ParseTypeFromClangModule(sc, die, log); -if (type_sp) +if (TypeSP type_sp = ParseTypeFromClangModule(sc, decl_die, log)) return type_sp; -type_sp = dwarf->FindDefinitionTypeForDWARFDeclContext(die); +def_die = dwarf->FindDefinitionDIE(decl_die); -if (!type_sp) { +if (!def_die) { SymbolFileDWARFDebugMap *debug_map_symfile = dwarf->GetDebugMapSymfile(); if (debug_map_symfile) { // We weren't able to find a full declaration in this DWARF, // see if we have a declaration anywhere else... -type_sp = debug_map_symfile->FindDefinitionTypeForDWARFDeclContext(die); +def_die = debug_map_symfile->FindDefinitionDIE(decl_die); } } -if (type_sp) { - if (log) { -dwarf->GetObjectFile()->GetModule()->LogMessage( -log, -"SymbolFileDWARF({0:p}) - {1:x16}}: {2} ({3}) type \"{4}\" is a " -"forward declaration, complete type is {5:x8}", -static_cast(this), die.GetOffset(), -DW_TAG_value_to_name(tag), tag, attrs.name.GetCString(), -type_sp->GetID()); - } - - // We found a real definition for this type elsewhere so must link its - // DeclContext to this die. - if (clang::DeclContext *defn_decl_ctx = - GetCachedClangDeclContextForDIE(dwarf->GetDIE(type_sp->GetID( -LinkDeclContextToDIE(defn_decl_ctx, die); - return type_sp; +if (log) { + dwarf->GetObjectFile()->GetModule()->LogMessage
[Lldb-commits] [lldb] [lldb/DWARF] Remove parsing recursion when searching for definition DIEs (PR #96484)
https://github.com/labath closed https://github.com/llvm/llvm-project/pull/96484 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][LibCxx] Move incorrect nullptr check (PR #96635)
https://github.com/Michael137 ready_for_review https://github.com/llvm/llvm-project/pull/96635 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][LibCxx] Move incorrect nullptr check (PR #96635)
https://github.com/Michael137 created https://github.com/llvm/llvm-project/pull/96635 Found this while skimming this code. Don't have a reproducible test case for this but the nullptr check should clearly occur before we try to dereference `location_sp`. >From 87edb6b9ba8b48e1bcddd2d73314cdb8f4e0a73b Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Tue, 25 Jun 2024 14:25:07 +0100 Subject: [PATCH] [lldb][LibCxx] Move incorrect nullptr check Found this while skimming this code. Don't have a reproducible test case for this but the nullptr check should clearly occur before we try to dereference `location_sp`. --- lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp index b0e6fb7d6f5af..0f9f93b727ce8 100644 --- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp +++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp @@ -808,6 +808,9 @@ ExtractLibcxxStringInfo(ValueObject &valobj) { size = (layout == StringLayout::DSC) ? size_mode_value : ((size_mode_value >> 1) % 256); +if (!location_sp) + return {}; + // When the small-string optimization takes place, the data must fit in the // inline string buffer (23 bytes on x86_64/Darwin). If it doesn't, it's // likely that the string isn't initialized and we're reading garbage. @@ -815,7 +818,7 @@ ExtractLibcxxStringInfo(ValueObject &valobj) { const std::optional max_bytes = location_sp->GetCompilerType().GetByteSize( exe_ctx.GetBestExecutionContextScope()); -if (!max_bytes || size > *max_bytes || !location_sp) +if (!max_bytes || size > *max_bytes) return {}; return std::make_pair(size, location_sp); ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][LibCxx] Move incorrect nullptr check (PR #96635)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) Changes Found this while skimming this code. Don't have a reproducible test case for this but the nullptr check should clearly occur before we try to dereference `location_sp`. --- Full diff: https://github.com/llvm/llvm-project/pull/96635.diff 1 Files Affected: - (modified) lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp (+4-1) ``diff diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp index b0e6fb7d6f5af..0f9f93b727ce8 100644 --- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp +++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp @@ -808,6 +808,9 @@ ExtractLibcxxStringInfo(ValueObject &valobj) { size = (layout == StringLayout::DSC) ? size_mode_value : ((size_mode_value >> 1) % 256); +if (!location_sp) + return {}; + // When the small-string optimization takes place, the data must fit in the // inline string buffer (23 bytes on x86_64/Darwin). If it doesn't, it's // likely that the string isn't initialized and we're reading garbage. @@ -815,7 +818,7 @@ ExtractLibcxxStringInfo(ValueObject &valobj) { const std::optional max_bytes = location_sp->GetCompilerType().GetByteSize( exe_ctx.GetBestExecutionContextScope()); -if (!max_bytes || size > *max_bytes || !location_sp) +if (!max_bytes || size > *max_bytes) return {}; return std::make_pair(size, location_sp); `` https://github.com/llvm/llvm-project/pull/96635 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][LibCxx] Move incorrect nullptr check (PR #96635)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/96635 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Set target OS for API tests in case of remote testing (PR #96654)
https://github.com/dzhidzhoev created https://github.com/llvm/llvm-project/pull/96654 Makefile.rules uses HOST_OS and OS variables for determining host and target OSes for API tests compilation. When lldb's target is set to remote-linux, Makefile.rules script should be executed with the target OS variable set to Linux. This is useful for the case of Windows-to-Linux cross-testing. >From c1cb536d21c221d4e67ba66c68d7902aedd9 Mon Sep 17 00:00:00 2001 From: Vladislav Dzhidzhoev Date: Wed, 19 Jun 2024 23:50:18 + Subject: [PATCH] [lldb][test] Set target OS for API tests in case of remote testing Makefile.rules uses HOST_OS and OS variables for determining host and target OSes for API tests compilation. When lldb's target is set to remote-linux, Makefile.rules script should be executed with the target OS variable set to Linux. This is useful for the case of Windows-to-Linux cross-testing. --- lldb/packages/Python/lldbsuite/test/lldbplatformutil.py | 7 +++ 1 file changed, 7 insertions(+) diff --git a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py index 21f2095db90f8..c39d297a78f8f 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py +++ b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py @@ -56,6 +56,10 @@ def target_is_android(): return configuration.lldb_platform_name == "remote-android" +def target_is_remote_linux(): +return configuration.lldb_platform_name == "remote-linux" + + def android_device_api(): if not hasattr(android_device_api, "result"): assert configuration.lldb_platform_url is not None @@ -97,6 +101,9 @@ def finalize_build_dictionary(dictionary): dictionary = {} dictionary["OS"] = "Android" dictionary["PIE"] = 1 +elif target_is_remote_linux(): +dictionary = dictionary or {} +dictionary["OS"] = "Linux" return dictionary ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Set target OS for API tests in case of remote testing (PR #96654)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Vladislav Dzhidzhoev (dzhidzhoev) Changes Makefile.rules uses HOST_OS and OS variables for determining host and target OSes for API tests compilation. When lldb's target is set to remote-linux, Makefile.rules script should be executed with the target OS variable set to Linux. This is useful for the case of Windows-to-Linux cross-testing. --- Full diff: https://github.com/llvm/llvm-project/pull/96654.diff 1 Files Affected: - (modified) lldb/packages/Python/lldbsuite/test/lldbplatformutil.py (+7) ``diff diff --git a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py index 21f2095db90f8..c39d297a78f8f 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py +++ b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py @@ -56,6 +56,10 @@ def target_is_android(): return configuration.lldb_platform_name == "remote-android" +def target_is_remote_linux(): +return configuration.lldb_platform_name == "remote-linux" + + def android_device_api(): if not hasattr(android_device_api, "result"): assert configuration.lldb_platform_url is not None @@ -97,6 +101,9 @@ def finalize_build_dictionary(dictionary): dictionary = {} dictionary["OS"] = "Android" dictionary["PIE"] = 1 +elif target_is_remote_linux(): +dictionary = dictionary or {} +dictionary["OS"] = "Linux" return dictionary `` https://github.com/llvm/llvm-project/pull/96654 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Add --target-os argument to dotest.py (PR #93887)
dzhidzhoev wrote: Closed in favor of https://github.com/llvm/llvm-project/pull/96654 https://github.com/llvm/llvm-project/pull/93887 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Add --target-os argument to dotest.py (PR #93887)
https://github.com/dzhidzhoev closed https://github.com/llvm/llvm-project/pull/93887 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] e951bd0 - Reapply PR/87550 (again) (#95571)
Author: Vy Nguyen Date: 2024-06-25T12:01:17-04:00 New Revision: e951bd0f51f8b077296f09d9c60ddf150048042f URL: https://github.com/llvm/llvm-project/commit/e951bd0f51f8b077296f09d9c60ddf150048042f DIFF: https://github.com/llvm/llvm-project/commit/e951bd0f51f8b077296f09d9c60ddf150048042f.diff LOG: Reapply PR/87550 (again) (#95571) New fixes: - properly init the `std::optional` to an empty vector as opposed to `{}` (which was effectively `std::nullopt`). - Co-authored-by: Vy Nguyen Added: Modified: lldb/include/lldb/API/SBDebugger.h lldb/include/lldb/Symbol/TypeSystem.h lldb/source/API/SBDebugger.cpp lldb/source/Symbol/TypeSystem.cpp lldb/tools/lldb-dap/DAP.cpp lldb/tools/lldb-dap/DAP.h lldb/tools/lldb-dap/lldb-dap.cpp Removed: diff --git a/lldb/include/lldb/API/SBDebugger.h b/lldb/include/lldb/API/SBDebugger.h index af19b1faf3bf5..84ea9c0f772e1 100644 --- a/lldb/include/lldb/API/SBDebugger.h +++ b/lldb/include/lldb/API/SBDebugger.h @@ -57,6 +57,8 @@ class LLDB_API SBDebugger { static const char *GetBroadcasterClass(); + static bool SupportsLanguage(lldb::LanguageType language); + lldb::SBBroadcaster GetBroadcaster(); /// Get progress data from a SBEvent whose type is eBroadcastBitProgress. diff --git a/lldb/include/lldb/Symbol/TypeSystem.h b/lldb/include/lldb/Symbol/TypeSystem.h index b4025c173a186..7d48f9b316138 100644 --- a/lldb/include/lldb/Symbol/TypeSystem.h +++ b/lldb/include/lldb/Symbol/TypeSystem.h @@ -209,6 +209,7 @@ class TypeSystem : public PluginInterface, // TypeSystems can support more than one language virtual bool SupportsLanguage(lldb::LanguageType language) = 0; + static bool SupportsLanguageStatic(lldb::LanguageType language); // Type Completion virtual bool GetCompleteType(lldb::opaque_compiler_type_t type) = 0; diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp index 7ef0d6efd4aaa..29da7d33dd80b 100644 --- a/lldb/source/API/SBDebugger.cpp +++ b/lldb/source/API/SBDebugger.cpp @@ -1742,3 +1742,7 @@ bool SBDebugger::InterruptRequested() { return m_opaque_sp->InterruptRequested(); return false; } + +bool SBDebugger::SupportsLanguage(lldb::LanguageType language) { + return TypeSystem::SupportsLanguageStatic(language); +} diff --git a/lldb/source/Symbol/TypeSystem.cpp b/lldb/source/Symbol/TypeSystem.cpp index 4956f10a0b0a7..931ce1b0203a9 100644 --- a/lldb/source/Symbol/TypeSystem.cpp +++ b/lldb/source/Symbol/TypeSystem.cpp @@ -335,3 +335,14 @@ TypeSystemMap::GetTypeSystemForLanguage(lldb::LanguageType language, } return GetTypeSystemForLanguage(language); } + +bool TypeSystem::SupportsLanguageStatic(lldb::LanguageType language) { + if (language == eLanguageTypeUnknown || language >= eNumLanguageTypes) +return false; + + LanguageSet languages = + PluginManager::GetAllTypeSystemSupportedLanguagesForTypes(); + if (languages.Empty()) +return false; + return languages[language]; +} diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index d419f821999e6..0196aed819f2b 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -32,14 +32,7 @@ namespace lldb_dap { DAP g_dap; DAP::DAP() -: broadcaster("lldb-dap"), - exception_breakpoints( - {{"cpp_catch", "C++ Catch", lldb::eLanguageTypeC_plus_plus}, - {"cpp_throw", "C++ Throw", lldb::eLanguageTypeC_plus_plus}, - {"objc_catch", "Objective-C Catch", lldb::eLanguageTypeObjC}, - {"objc_throw", "Objective-C Throw", lldb::eLanguageTypeObjC}, - {"swift_catch", "Swift Catch", lldb::eLanguageTypeSwift}, - {"swift_throw", "Swift Throw", lldb::eLanguageTypeSwift}}), +: broadcaster("lldb-dap"), exception_breakpoints(), focus_tid(LLDB_INVALID_THREAD_ID), stop_at_entry(false), is_attach(false), enable_auto_variable_summaries(false), enable_synthetic_child_debugging(false), @@ -65,8 +58,51 @@ DAP::DAP() DAP::~DAP() = default; +void DAP::PopulateExceptionBreakpoints() { + llvm::call_once(init_exception_breakpoints_flag, [this]() { +exception_breakpoints = std::vector {}; + +if (lldb::SBDebugger::SupportsLanguage(lldb::eLanguageTypeC_plus_plus)) { + exception_breakpoints->emplace_back("cpp_catch", "C++ Catch", + lldb::eLanguageTypeC_plus_plus); + exception_breakpoints->emplace_back("cpp_throw", "C++ Throw", + lldb::eLanguageTypeC_plus_plus); +} +if (lldb::SBDebugger::SupportsLanguage(lldb::eLanguageTypeObjC)) { + exception_breakpoints->emplace_back("objc_catch", "Objective-C Catch", + lldb::eLanguageTypeObjC); + exception_breakpoints->emplace_back("objc_throw", "Objective-C Throw", +
[Lldb-commits] [lldb] Reapply PR/87550 (again) (PR #95571)
https://github.com/oontvoo closed https://github.com/llvm/llvm-project/pull/95571 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)
walter-erquinigo wrote: I second everything that Pavel says. I think this would be the best approach. The existing frame var implementation is not that "smart", so it should be practical to fully replace it with the new DIL in its first stage. An additional consideration is that lldb-dap, which is being more and more frequently used, relies on `frame var` for executing most expressions. `expr` is just a fallback. Therefore, replacing `frame var` with DIL right away has the benefit of getting tested right away by all the lldb-dap users. https://github.com/llvm/llvm-project/pull/95738 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
Jlalond wrote: @vvereschaka I'll work on this today. Is it time pressing enough you would want everything reverted? https://github.com/llvm/llvm-project/pull/95312 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
vvereschaka wrote: @Jlalond , perfect, thank you. It is ok for me if you'll fix the problem during today. https://github.com/llvm/llvm-project/pull/95312 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)
labath wrote: It looks like the test is flaky: https://lab.llvm.org/buildbot/#/builders/59/builds/535 It looks like the order of the types is nondeterministic. I think you should be able to use CHECK-DAG along with [this trick to match newlines](https://llvm.org/docs/CommandGuide/FileCheck.html#matching-newline-characters) to make the test accept both orderings. https://github.com/llvm/llvm-project/pull/87740 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][LibCxx] Move incorrect nullptr check (PR #96635)
https://github.com/bulbazord approved this pull request. Oh, yeah, that makes a lot more sense. https://github.com/llvm/llvm-project/pull/96635 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Set target OS for API tests in case of remote testing (PR #96654)
@@ -97,6 +101,9 @@ def finalize_build_dictionary(dictionary): dictionary = {} dictionary["OS"] = "Android" dictionary["PIE"] = 1 +elif target_is_remote_linux(): +dictionary = dictionary or {} bulbazord wrote: Suggestion: Pull the dictionary checking-logic out of these blocks and put it at the top of the function: ``` def finalize_build_dictionary(dictionary): if dictionary is None: dictionary = {} if target_is_android(): # ... https://github.com/llvm/llvm-project/pull/96654 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 21ab32e - [lldb][LibCxx] Move incorrect nullptr check (#96635)
Author: Michael Buch Date: 2024-06-25T19:05:16+01:00 New Revision: 21ab32e1c144b42458b7b3181e84bfb45aadcc54 URL: https://github.com/llvm/llvm-project/commit/21ab32e1c144b42458b7b3181e84bfb45aadcc54 DIFF: https://github.com/llvm/llvm-project/commit/21ab32e1c144b42458b7b3181e84bfb45aadcc54.diff LOG: [lldb][LibCxx] Move incorrect nullptr check (#96635) Found while skimming this code. Don't have a reproducible test case for this but the nullptr check should clearly occur before we try to dereference `location_sp`. Added: Modified: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp Removed: diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp index b0e6fb7d6f5af..0f9f93b727ce8 100644 --- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp +++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp @@ -808,6 +808,9 @@ ExtractLibcxxStringInfo(ValueObject &valobj) { size = (layout == StringLayout::DSC) ? size_mode_value : ((size_mode_value >> 1) % 256); +if (!location_sp) + return {}; + // When the small-string optimization takes place, the data must fit in the // inline string buffer (23 bytes on x86_64/Darwin). If it doesn't, it's // likely that the string isn't initialized and we're reading garbage. @@ -815,7 +818,7 @@ ExtractLibcxxStringInfo(ValueObject &valobj) { const std::optional max_bytes = location_sp->GetCompilerType().GetByteSize( exe_ctx.GetBestExecutionContextScope()); -if (!max_bytes || size > *max_bytes || !location_sp) +if (!max_bytes || size > *max_bytes) return {}; return std::make_pair(size, location_sp); ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][LibCxx] Move incorrect nullptr check (PR #96635)
https://github.com/Michael137 closed https://github.com/llvm/llvm-project/pull/96635 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang-tools-extra] [compiler-rt] [lldb] [llvm] [Memprof] Adds the option to collect AccessCountHistograms for memprof. (PR #94264)
@@ -216,6 +228,35 @@ u64 GetShadowCount(uptr p, u32 size) { return count; } +// Accumulates the access count from the shadow for the given pointer and size. +// See memprof_mapping.h for an overview on histogram counters. +u64 GetShadowCountHistogram(uptr p, u32 size) { + u8 *shadow = (u8 *)HISTOGRAM_MEM_TO_SHADOW(p); + u8 *shadow_end = (u8 *)HISTOGRAM_MEM_TO_SHADOW(p + size); + u64 count = 0; + for (; shadow <= shadow_end; shadow++) +count += *shadow; + return count; +} + +// If we use the normal approach from clearCountersWithoutHistogram, the +// histogram will clear too much data and may overwrite shadow counters that are +// in use. Likely because of rounding up the shadow_end pointer. +// See memprof_mapping.h for an overview on histogram counters. +void clearCountersHistogram(uptr addr, uptr size) { + u8 *shadow_8 = (u8 *)HISTOGRAM_MEM_TO_SHADOW(addr); + u8 *shadow_end_8 = (u8 *)HISTOGRAM_MEM_TO_SHADOW(addr + size); + for (; shadow_8 < shadow_end_8; shadow_8++) { +*shadow_8 = 0; + } +} + +void clearCountersWithoutHistogram(uptr addr, uptr size) { + uptr shadow_beg = MEM_TO_SHADOW(addr); + uptr shadow_end = MEM_TO_SHADOW(addr + size - SHADOW_GRANULARITY) + 1; + REAL(memset)((void *)shadow_beg, 0, shadow_end - shadow_beg); +} + // Clears the shadow counters (when memory is allocated). void ClearShadow(uptr addr, uptr size) { mattweingarten wrote: Yes I agree, this is nicer. Changed. https://github.com/llvm/llvm-project/pull/94264 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang-tools-extra] [compiler-rt] [lldb] [llvm] [Memprof] Adds the option to collect AccessCountHistograms for memprof. (PR #94264)
https://github.com/mattweingarten updated https://github.com/llvm/llvm-project/pull/94264 error: too big or took too long to generate ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang-tools-extra] [compiler-rt] [lldb] [llvm] [Memprof] Adds the option to collect AccessCountHistograms for memprof. (PR #94264)
https://github.com/teresajohnson approved this pull request. lgtm https://github.com/llvm/llvm-project/pull/94264 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
petrhosek wrote: We're seeing the same issue as well on our builders, will you be able to address this today? https://github.com/llvm/llvm-project/pull/95312 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix flake in TestZerothFrame.py (PR #96685)
https://github.com/kendalharland created https://github.com/llvm/llvm-project/pull/96685 This test is currently flaky on a local Windows amd64 build. If we print lldb's inputs and outputs while running, we can see that the breakpoints are always being set correctly, and always being hit: ```sh runCmd: breakpoint set -f "main.c" -l 2 output: Breakpoint 1: where = a.out`func_inner + 1 at main.c:2:9, address = 0x000140001001 runCmd: breakpoint set -f "main.c" -l 7 output: Breakpoint 2: where = a.out`main + 17 at main.c:7:5, address = 0x000140001021 runCmd: run output: Process 52328 launched: 'C:\workspace\llvm-project\llvm\build\lldb-test-build.noindex\functionalities\unwind\zeroth_frame\TestZerothFrame.test_dwarf\a.out' (x86_64) Process 52328 stopped * thread #1, stop reason = breakpoint 1.1 frame #0: 0x7ff68f6b1001 a.out`func_inner at main.c:2:9 1void func_inner() { -> 2int a = 1; // Set breakpoint 1 here ^ 3} 4 5int main() { 6func_inner(); 7return 0; // Set breakpoint 2 here ``` However, sometimes the backtrace printed in this test shows that the process is stopped inside NtWaitForWorkViaWorkerFactory from `ntdll.dll`: ```sh Backtrace at the first breakpoint: frame #0: 0x7ffecc7b3bf4 ntdll.dll`NtWaitForWorkViaWorkerFactory + 20 frame #1: 0x7ffecc74585e ntdll.dll`RtlClearThreadWorkOnBehalfTicket + 862 frame #2: 0x7ffecc3e257d kernel32.dll`BaseThreadInitThunk + 29 frame #3: 0x7ffecc76af28 ntdll.dll`RtlUserThreadStart + 40 ``` If we print the list of threads each time the test is run, we notice that threads are sometimes in a different order, within `process.threads`: ```sh Thread 0: thread #4: tid = 0x9c38, 0x7ffecc7b3bf4 ntdll.dll`NtWaitForWorkViaWorkerFactory + 20 Thread 1: thread #2: tid = 0xa950, 0x7ffecc7b3bf4 ntdll.dll`NtWaitForWorkViaWorkerFactory + 20 Thread 2: thread #1: tid = 0xab18, 0x7ff64bc81001 a.out`func_inner at main.c:2:9, stop reason = breakpoint 1.1 Thread 3: thread #3: tid = 0xc514, 0x7ffecc7b3bf4 ntdll.dll`NtWaitForWorkViaWorkerFactory + 20 Thread 0: thread #3: tid = 0x018c, 0x7ffecc7b3bf4 ntdll.dll`NtWaitForWorkViaWorkerFactory + 20 Thread 1: thread #1: tid = 0x85c8, 0x7ff7130c1001 a.out`func_inner at main.c:2:9, stop reason = breakpoint 1.1 Thread 2: thread #2: tid = 0xf344, 0x7ffecc7b3bf4 ntdll.dll`NtWaitForWorkViaWorkerFactory + 20 Thread 3: thread #4: tid = 0x6a50, 0x7ffecc7b3bf4 ntdll.dll`NtWaitForWorkViaWorkerFactory + 20 ``` We're interested in whichever thread is executing `a.out`. If we print more info, we can see that this thread always has an `IndexID` of 1 regardless of where it appars in `process.threads`: ```sh Thread 0: thread #3: tid = 0x2474, 0x7ffecc7b3bf4 ntdll.dll`NtWaitForWorkViaWorkerFactory + 20 -- GetName: -- GetQueueName: None -- GetQueueID: 0 -- GetIndexId: 3 Thread 1: thread #2: tid = 0x2864, 0x7ffecc7b3bf4 ntdll.dll`NtWaitForWorkViaWorkerFactory + 20 -- GetName: -- GetQueueName: None -- GetQueueID: 0 -- GetIndexId: 2 Thread 2: thread #4: tid = 0x9c90, 0x7ffecc7b3bf4 ntdll.dll`NtWaitForWorkViaWorkerFactory + 20 -- GetName: -- GetQueueName: None -- GetQueueID: 0 -- GetIndexId: 4 Thread 3: thread #1: tid = 0xebbc, 0x7ff643331001 a.out`func_inner at main.c:2:9, stop reason = breakpoint 1.1 -- GetName: -- GetQueueName: None -- GetQueueID: 0 -- GetIndexId: 1 ``` By switching from `process.GetThreadAtIndex` to `process.GetThreadByIndex` we consistently retrieve the correct thread. This raises a few more questions that might lead to a different followup solution: 1. Why does our target thread always have an `IndexID` of 1? Why not 0 or any other value? 2. Why is `process.threads` non-deterministically ordered? >From 5f23d2c48252c880a20e04f273d2d34b80758b6f Mon Sep 17 00:00:00 2001 From: kendal Date: Mon, 24 Jun 2024 13:42:20 -0700 Subject: [PATCH] Fix flake in TestZerothFrame.py If we print lldb's input and output while running this test, we can see that the breakpoints are always being set correctly, and always being hit: ```sh runCmd: breakpoint set -f "main.c" -l 2 output: Breakpoint 1: where = a.out`func_inner + 1 at main.c:2:9, address = 0x000140001001 runCmd: breakpoint set -f "main.c" -l 7 output: Breakpoint 2: where = a.out`main + 17 at main.c:7:5, address = 0x000140001021 runCmd: run output: Process 52328 launched: 'C:\workspace\llvm-project\llvm\build\lldb-test-build.noindex\functionalities\unwind\zeroth_frame\TestZerothFrame.test_dwarf\a.out' (x86_64) Process 52328 stopped * thread #1, stop reason = breakpoint 1.1 frame #0: 0x7ff68f6b1001 a.out`func_inner at main.c:2:9 1void func_inner() { -> 2int a = 1; // Set breakpoint 1 here ^ 3} 4 5int main() { 6func_inner(); 7return 0; // Set breakpoint 2 here ``` However, sometimes the backtrace printed in this test shows that the p
[Lldb-commits] [lldb] Fix flake in TestZerothFrame.py (PR #96685)
github-actions[bot] wrote: Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using `@` followed by their GitHub username. If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the [LLVM GitHub User Guide](https://llvm.org/docs/GitHub.html). You can also ask questions in a comment on this PR, on the [LLVM Discord](https://discord.com/invite/xS7Z362) or on the [forums](https://discourse.llvm.org/). https://github.com/llvm/llvm-project/pull/96685 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix flake in TestZerothFrame.py (PR #96685)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Kendal Harland (kendalharland) Changes This test is currently flaky on a local Windows amd64 build. If we print lldb's inputs and outputs while running, we can see that the breakpoints are always being set correctly, and always being hit: ```sh runCmd: breakpoint set -f "main.c" -l 2 output: Breakpoint 1: where = a.out`func_inner + 1 at main.c:2:9, address = 0x000140001001 runCmd: breakpoint set -f "main.c" -l 7 output: Breakpoint 2: where = a.out`main + 17 at main.c:7:5, address = 0x000140001021 runCmd: run output: Process 52328 launched: 'C:\workspace\llvm-project\llvm\build\lldb-test-build.noindex\functionalities\unwind\zeroth_frame\TestZerothFrame.test_dwarf\a.out' (x86_64) Process 52328 stopped * thread #1, stop reason = breakpoint 1.1 frame #0: 0x7ff68f6b1001 a.out`func_inner at main.c:2:9 1void func_inner() { -> 2int a = 1; // Set breakpoint 1 here ^ 3} 4 5int main() { 6func_inner(); 7return 0; // Set breakpoint 2 here ``` However, sometimes the backtrace printed in this test shows that the process is stopped inside NtWaitForWorkViaWorkerFactory from `ntdll.dll`: ```sh Backtrace at the first breakpoint: frame #0: 0x7ffecc7b3bf4 ntdll.dll`NtWaitForWorkViaWorkerFactory + 20 frame #1: 0x7ffecc74585e ntdll.dll`RtlClearThreadWorkOnBehalfTicket + 862 frame #2: 0x7ffecc3e257d kernel32.dll`BaseThreadInitThunk + 29 frame #3: 0x7ffecc76af28 ntdll.dll`RtlUserThreadStart + 40 ``` If we print the list of threads each time the test is run, we notice that threads are sometimes in a different order, within `process.threads`: ```sh Thread 0: thread #4: tid = 0x9c38, 0x7ffecc7b3bf4 ntdll.dll`NtWaitForWorkViaWorkerFactory + 20 Thread 1: thread #2: tid = 0xa950, 0x7ffecc7b3bf4 ntdll.dll`NtWaitForWorkViaWorkerFactory + 20 Thread 2: thread #1: tid = 0xab18, 0x7ff64bc81001 a.out`func_inner at main.c:2:9, stop reason = breakpoint 1.1 Thread 3: thread #3: tid = 0xc514, 0x7ffecc7b3bf4 ntdll.dll`NtWaitForWorkViaWorkerFactory + 20 Thread 0: thread #3: tid = 0x018c, 0x7ffecc7b3bf4 ntdll.dll`NtWaitForWorkViaWorkerFactory + 20 Thread 1: thread #1: tid = 0x85c8, 0x7ff7130c1001 a.out`func_inner at main.c:2:9, stop reason = breakpoint 1.1 Thread 2: thread #2: tid = 0xf344, 0x7ffecc7b3bf4 ntdll.dll`NtWaitForWorkViaWorkerFactory + 20 Thread 3: thread #4: tid = 0x6a50, 0x7ffecc7b3bf4 ntdll.dll`NtWaitForWorkViaWorkerFactory + 20 ``` We're interested in whichever thread is executing `a.out`. If we print more info, we can see that this thread always has an `IndexID` of 1 regardless of where it appars in `process.threads`: ```sh Thread 0: thread #3: tid = 0x2474, 0x7ffecc7b3bf4 ntdll.dll`NtWaitForWorkViaWorkerFactory + 20 -- GetName: -- GetQueueName: None -- GetQueueID: 0 -- GetIndexId: 3 Thread 1: thread #2: tid = 0x2864, 0x7ffecc7b3bf4 ntdll.dll`NtWaitForWorkViaWorkerFactory + 20 -- GetName: -- GetQueueName: None -- GetQueueID: 0 -- GetIndexId: 2 Thread 2: thread #4: tid = 0x9c90, 0x7ffecc7b3bf4 ntdll.dll`NtWaitForWorkViaWorkerFactory + 20 -- GetName: -- GetQueueName: None -- GetQueueID: 0 -- GetIndexId: 4 Thread 3: thread #1: tid = 0xebbc, 0x7ff643331001 a.out`func_inner at main.c:2:9, stop reason = breakpoint 1.1 -- GetName: -- GetQueueName: None -- GetQueueID: 0 -- GetIndexId: 1 ``` By switching from `process.GetThreadAtIndex` to `process.GetThreadByIndex` we consistently retrieve the correct thread. This raises a few more questions that might lead to a different followup solution: 1. Why does our target thread always have an `IndexID` of 1? Why not 0 or any other value? 2. Why is `process.threads` non-deterministically ordered? --- Full diff: https://github.com/llvm/llvm-project/pull/96685.diff 1 Files Affected: - (modified) lldb/test/API/functionalities/unwind/zeroth_frame/TestZerothFrame.py (+4-3) ``diff diff --git a/lldb/test/API/functionalities/unwind/zeroth_frame/TestZerothFrame.py b/lldb/test/API/functionalities/unwind/zeroth_frame/TestZerothFrame.py index f4e883d314644..7e4078bbe887f 100644 --- a/lldb/test/API/functionalities/unwind/zeroth_frame/TestZerothFrame.py +++ b/lldb/test/API/functionalities/unwind/zeroth_frame/TestZerothFrame.py @@ -53,14 +53,15 @@ def test(self): process = target.LaunchSimple(None, None, self.get_process_working_directory()) self.assertTrue(process, VALID_PROCESS) -thread = process.GetThreadAtIndex(0) +thread = process.GetThreadByIndexID(1) if self.TraceOn(): print("Backtrace at the first breakpoint:") for f in thread.frames: print(f) + # Check that we have stopped at correct breakpoint. self.assertEqual( -process.GetThreadAtIndex(0).frame[0].GetLineEntry().GetLine(), +thread.frame[0].GetLineEntry().GetLine(),
[Lldb-commits] [lldb] Fix flake in TestZerothFrame.py (PR #96685)
https://github.com/kendalharland edited https://github.com/llvm/llvm-project/pull/96685 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix test assertions in TestDAP_stepInTargets.py (PR #96687)
https://github.com/kendalharland created https://github.com/llvm/llvm-project/pull/96687 The strings this test is using seem to consistently fail to match against the expected values when built & run targeting Windows amd64. This PR updates them to the expected values. >From 2e3ac00255e5608ff39bf9cd69f1fb4590d2b90d Mon Sep 17 00:00:00 2001 From: kendal Date: Mon, 24 Jun 2024 14:01:31 -0700 Subject: [PATCH] Fix test assertions in TestDAP_stepInTargets.py --- .../tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py b/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py index 6296f6554d07e..cc53954885cdf 100644 --- a/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py +++ b/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py @@ -55,14 +55,14 @@ def test_basic(self): self.assertEqual(len(step_in_targets), 3, "expect 3 step in targets") # Verify the target names are correct. -self.assertEqual(step_in_targets[0]["label"], "bar()", "expect bar()") -self.assertEqual(step_in_targets[1]["label"], "bar2()", "expect bar2()") +self.assertEqual(step_in_targets[0]["label"], "int bar2(void)", "expect bar2()") +self.assertEqual(step_in_targets[1]["label"], "int bar(void)", "expect bar()") self.assertEqual( -step_in_targets[2]["label"], "foo(int, int)", "expect foo(int, int)" +step_in_targets[2]["label"], "int foo(int, int)", "expect foo(int, int)" ) # Choose to step into second target and verify that we are in bar2() self.stepIn(threadId=tid, targetId=step_in_targets[1]["id"], waitForStop=True) leaf_frame = self.dap_server.get_stackFrame() self.assertIsNotNone(leaf_frame, "expect a leaf frame") -self.assertEqual(leaf_frame["name"], "bar2()") +self.assertEqual(leaf_frame["name"], "int bar2(void)") ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix test assertions in TestDAP_stepInTargets.py (PR #96687)
github-actions[bot] wrote: Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using `@` followed by their GitHub username. If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the [LLVM GitHub User Guide](https://llvm.org/docs/GitHub.html). You can also ask questions in a comment on this PR, on the [LLVM Discord](https://discord.com/invite/xS7Z362) or on the [forums](https://discourse.llvm.org/). https://github.com/llvm/llvm-project/pull/96687 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix test assertions in TestDAP_stepInTargets.py (PR #96687)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Kendal Harland (kendalharland) Changes The strings this test is using seem to consistently fail to match against the expected values when built & run targeting Windows amd64. This PR updates them to the expected values. --- Full diff: https://github.com/llvm/llvm-project/pull/96687.diff 1 Files Affected: - (modified) lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py (+4-4) ``diff diff --git a/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py b/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py index 6296f6554d07e..cc53954885cdf 100644 --- a/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py +++ b/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py @@ -55,14 +55,14 @@ def test_basic(self): self.assertEqual(len(step_in_targets), 3, "expect 3 step in targets") # Verify the target names are correct. -self.assertEqual(step_in_targets[0]["label"], "bar()", "expect bar()") -self.assertEqual(step_in_targets[1]["label"], "bar2()", "expect bar2()") +self.assertEqual(step_in_targets[0]["label"], "int bar2(void)", "expect bar2()") +self.assertEqual(step_in_targets[1]["label"], "int bar(void)", "expect bar()") self.assertEqual( -step_in_targets[2]["label"], "foo(int, int)", "expect foo(int, int)" +step_in_targets[2]["label"], "int foo(int, int)", "expect foo(int, int)" ) # Choose to step into second target and verify that we are in bar2() self.stepIn(threadId=tid, targetId=step_in_targets[1]["id"], waitForStop=True) leaf_frame = self.dap_server.get_stackFrame() self.assertIsNotNone(leaf_frame, "expect a leaf frame") -self.assertEqual(leaf_frame["name"], "bar2()") +self.assertEqual(leaf_frame["name"], "int bar2(void)") `` https://github.com/llvm/llvm-project/pull/96687 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix test assertions in TestDAP_stepInTargets.py (PR #96687)
kendalharland wrote: This PR was really just the result of me looking at the test output on my own machine and copy-pasting the values that were generated by the test's data. I am not familiar with this test's implementation and would appreciate any guidance on whether there really is a bug here, and how I can test this on different platforms/architectures the same way that the llvm-project CI does it. https://github.com/llvm/llvm-project/pull/96687 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix flake in TestZerothFrame.py (PR #96685)
https://github.com/kendalharland updated https://github.com/llvm/llvm-project/pull/96685 >From 97242b723de7fd4dcb14bd8481acc2254ab852ea Mon Sep 17 00:00:00 2001 From: kendal Date: Mon, 24 Jun 2024 13:42:20 -0700 Subject: [PATCH] Fix flake in TestZerothFrame.py This test is relying on the order of `process.threads` which is nondeterministic. By switching from `process.GetThreadAtIndex` to `process.GetThreadByIndex` we consistently retrieve the correct thread. --- .../functionalities/unwind/zeroth_frame/TestZerothFrame.py | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lldb/test/API/functionalities/unwind/zeroth_frame/TestZerothFrame.py b/lldb/test/API/functionalities/unwind/zeroth_frame/TestZerothFrame.py index f4e883d314644..7e4078bbe887f 100644 --- a/lldb/test/API/functionalities/unwind/zeroth_frame/TestZerothFrame.py +++ b/lldb/test/API/functionalities/unwind/zeroth_frame/TestZerothFrame.py @@ -53,14 +53,15 @@ def test(self): process = target.LaunchSimple(None, None, self.get_process_working_directory()) self.assertTrue(process, VALID_PROCESS) -thread = process.GetThreadAtIndex(0) +thread = process.GetThreadByIndexID(1) if self.TraceOn(): print("Backtrace at the first breakpoint:") for f in thread.frames: print(f) + # Check that we have stopped at correct breakpoint. self.assertEqual( -process.GetThreadAtIndex(0).frame[0].GetLineEntry().GetLine(), +thread.frame[0].GetLineEntry().GetLine(), bp1_line, "LLDB reported incorrect line number.", ) @@ -70,7 +71,7 @@ def test(self): # 'continue' command. process.Continue() -thread = process.GetThreadAtIndex(0) +thread = process.GetThreadByIndexID(1) if self.TraceOn(): print("Backtrace at the second breakpoint:") for f in thread.frames: ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix flake in TestZerothFrame.py (PR #96685)
https://github.com/kendalharland edited https://github.com/llvm/llvm-project/pull/96685 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix flake in TestZerothFrame.py (PR #96685)
https://github.com/kendalharland edited https://github.com/llvm/llvm-project/pull/96685 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix flake in TestZerothFrame.py (PR #96685)
https://github.com/kendalharland edited https://github.com/llvm/llvm-project/pull/96685 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix flake in TestZerothFrame.py (PR #96685)
https://github.com/kendalharland edited https://github.com/llvm/llvm-project/pull/96685 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix test assertions in TestDAP_stepInTargets.py (PR #96687)
https://github.com/jeffreytan81 requested changes to this pull request. Have you tried to run this test on Linux? It is passing on my machine. I am pretty sure this might be Windows platform (like compiler and disassembler) specific behavior which you shouldn't change the default value for the test. You either have to disable this test for Windows (which I never tested) or write different expected value for different platform https://github.com/llvm/llvm-project/pull/96687 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix test assertions in TestDAP_stepInTargets.py (PR #96687)
bulbazord wrote: One option might be to change the test to match against some list of expected strings. I'm not super familiar with this test but as Jeffrey said this could be OS/Platform dependent. https://github.com/llvm/llvm-project/pull/96687 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix flake in TestZerothFrame.py (PR #96685)
https://github.com/bulbazord requested changes to this pull request. Instead of assuming the thread's index or index ID, it might be a good idea to have a little helper method that can look through the inferior's threads to find the thread with the property that we care about. In this case, we're looking for the thread that's stopped in a specific module (`a.out` in this case). Concurrency and thread order is not guaranteed to be stable, even if in practice they end up being that way on some platforms. The less we can rely on specific indices or IDs that "should" be correct, the better. https://github.com/llvm/llvm-project/pull/96685 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix flake in TestZerothFrame.py (PR #96685)
https://github.com/bulbazord edited https://github.com/llvm/llvm-project/pull/96685 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix flake in TestZerothFrame.py (PR #96685)
kendalharland wrote: > Instead of assuming the thread's index or index ID, it might be a good idea > to have a little helper method that can look through the inferior's threads > to find the thread with the property that we care about. In this case, we're > looking for the thread that's stopped in a specific module (`a.out` in this > case). > > Concurrent execution and thread order is not guaranteed to be stable between > runs, even if in practice they end up being that way on some platforms. The > less we can rely on specific indices or IDs that "should" be correct, the > better. Sounds reasonable. I'll work that out with a helper method and resend for review once I upload a new commit. @bulbazord any idea how I can access the module from the `SBThread` object? I'm having trouble finding the right API. https://github.com/llvm/llvm-project/pull/96685 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
Jlalond wrote: @petrhosek I am working on this. I think it's only the header thankfully https://github.com/llvm/llvm-project/pull/95312 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Remove parsing recursion when searching for definition DIEs (PR #96484)
@@ -897,32 +895,39 @@ TypeSP DWARFASTParserClang::ParseEnum(const SymbolContext &sc, } CompilerType clang_type = m_ast.CreateEnumerationType( - attrs.name.GetStringRef(), GetClangDeclContextContainingDIE(die, nullptr), - GetOwningClangModule(die), attrs.decl, enumerator_clang_type, + attrs.name.GetStringRef(), GetClangDeclContextContainingDIE(def_die, nullptr), + GetOwningClangModule(def_die), attrs.decl, enumerator_clang_type, attrs.is_scoped_enum); - - LinkDeclContextToDIE(TypeSystemClang::GetDeclContextForType(clang_type), die); - - type_sp = - dwarf->MakeType(die.GetID(), attrs.name, attrs.byte_size, nullptr, + TypeSP type_sp = + dwarf->MakeType(def_die.GetID(), attrs.name, attrs.byte_size, nullptr, ZequanWu wrote: If two different enum decl_die (reference the same definition def_die) were called with this function, doesn't it create two `CompilerType` and two `Type` from the same def_die? It's not a problem for `ParseStructureLikeDIE` because it will check if we have already created the type in `UniqueDWARFASTTypeMap`. https://github.com/llvm/llvm-project/pull/96484 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix flake in TestZerothFrame.py (PR #96685)
JDevlieghere wrote: > Sounds reasonable. I'll work that out with a helper method and resend for > review once I upload a new commit. @bulbazord any idea how I can access the > module from the `SBThread` object, to compare it against the name `a.out`? > I'm having trouble finding the right API. The module is a property of the target, so you could do: ``` process = thread.GetProcess() target = process.GetTarget() module = target.GetModuleAtIndex(0) ``` or you can create an ExecutionContext from the thread, and get the target from that: ``` exe_ctx = SBExecutionContext(thread) exe_ctx.target.GetModuleAtIndex(0) ``` https://github.com/llvm/llvm-project/pull/96685 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Removed header and validated on new windows machine (PR #96724)
https://github.com/Jlalond created https://github.com/llvm/llvm-project/pull/96724  In #95312 uint and `#include ` were introduced. These broke the windows build. I addressed uint in #96564, but include went unfixed. So I acquired myself a windows machine to validate. >From aa9c8fae63f9124c59332cbfd4e84e3e229b1956 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Tue, 25 Jun 2024 20:04:13 -0700 Subject: [PATCH] Removed header and validated on new windows machine --- lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp index 3668c37c5191d..7c875aa3223ed 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp @@ -18,7 +18,6 @@ #include "lldb/Utility/Log.h" #include "llvm/Support/FileSystem.h" -#include using namespace lldb; using namespace lldb_private; ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Removed header and validated on new windows machine (PR #96724)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Jacob Lalonde (Jlalond) Changes  In #95312 uint and `#include` were introduced. These broke the windows build. I addressed uint in #96564, but include went unfixed. So I acquired myself a windows machine to validate. --- Full diff: https://github.com/llvm/llvm-project/pull/96724.diff 1 Files Affected: - (modified) lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp (-1) ``diff diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp index 3668c37c5191d..7c875aa3223ed 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp @@ -18,7 +18,6 @@ #include "lldb/Utility/Log.h" #include "llvm/Support/FileSystem.h" -#include using namespace lldb; using namespace lldb_private; `` https://github.com/llvm/llvm-project/pull/96724 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Removed header and validated on new windows machine (PR #96724)
Jlalond wrote: Hey @petrhosek @vvereschaka apologies about the delay, I had to get a Windows machine up to validate this. https://github.com/llvm/llvm-project/pull/96724 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Removed header and validated on new windows machine (PR #96724)
vvereschaka wrote: Thank you @Jlalond , I started checking the local builds on Windows/Linux hosts with these changes. https://github.com/llvm/llvm-project/pull/96724 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Removed header and validated on new windows machine (PR #96724)
https://github.com/vvereschaka approved this pull request. LGTM. https://github.com/llvm/llvm-project/pull/96724 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] cb3469a - Removed header and validated on new windows machine (#96724)
Author: Jacob Lalonde Date: 2024-06-25T21:26:31-07:00 New Revision: cb3469a30f875b9cd54a263803fffc93554bec12 URL: https://github.com/llvm/llvm-project/commit/cb3469a30f875b9cd54a263803fffc93554bec12 DIFF: https://github.com/llvm/llvm-project/commit/cb3469a30f875b9cd54a263803fffc93554bec12.diff LOG: Removed header and validated on new windows machine (#96724)  In #95312 uint and `#include ` were introduced. These broke the windows build. I addressed uint in #96564, but include went unfixed. So I acquired myself a windows machine to validate. Added: Modified: lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp Removed: diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp index 3668c37c5191d..7c875aa3223ed 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp @@ -18,7 +18,6 @@ #include "lldb/Utility/Log.h" #include "llvm/Support/FileSystem.h" -#include using namespace lldb; using namespace lldb_private; ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Removed header and validated on new windows machine (PR #96724)
https://github.com/Jlalond closed https://github.com/llvm/llvm-project/pull/96724 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)
vvereschaka wrote: This build got the same failed test on the windows host - https://lab.llvm.org/buildbot/#/builders/141/builds/318 ``` Failed Tests (1): lldb-shell :: SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp ``` ``` # .---command stderr # | C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\test\Shell\SymbolFile\DWARF\x86\dwp-foreign-type-units.cpp:34:16: error: NODWP-NEXT: is not on the line after the previous match # | // NODWP-NEXT: unsigned int # |^ # | :20:10: note: 'next' match was here # | typedef unsigned int IntegerType; # | ^ # | :7:13: note: previous match ended here # | unsigned int # | ^ # | :8:1: note: non-matching line after previous match is here # | int # | ^ # | # | Input file: # | Check file: C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\test\Shell\SymbolFile\DWARF\x86\dwp-foreign-type-units.cpp # | # | -dump-input=help explains the following input dump. # | # | Input was: # | << # | . # | . # | . # | 15: typedef double FloatType; # | 16: CustomType::IntegerType x; # | 17: CustomType::FloatType y; # | 18: } # | 19: struct CustomType { # | 20: typedef unsigned int IntegerType; # | next:34 !~~~ error: match on wrong line # | 21: typedef float FloatType; # | 22: CustomType::IntegerType x; # | 23: CustomType::FloatType y; # | 24: } # | >> # `- # error: command failed with exit status: 1 ``` https://github.com/llvm/llvm-project/pull/87740 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits