[Lldb-commits] [lldb] 5d39e0c - Revert "[LLDB] Add a target.launch-working-dir setting" (#114973)
Author: Walter Erquinigo Date: 2024-11-05T07:12:20-05:00 New Revision: 5d39e0c7e1b50fc9a0f77daeef5eb63bcbba5b35 URL: https://github.com/llvm/llvm-project/commit/5d39e0c7e1b50fc9a0f77daeef5eb63bcbba5b35 DIFF: https://github.com/llvm/llvm-project/commit/5d39e0c7e1b50fc9a0f77daeef5eb63bcbba5b35.diff LOG: Revert "[LLDB] Add a target.launch-working-dir setting" (#114973) Reverts llvm/llvm-project#113521 due to build bot failures mentioned in the original PR. Added: Modified: lldb/include/lldb/Target/Target.h lldb/source/Commands/CommandObjectProcess.cpp lldb/source/Commands/Options.td lldb/source/Target/Target.cpp lldb/source/Target/TargetProperties.td lldb/test/API/commands/process/launch/TestProcessLaunch.py llvm/docs/ReleaseNotes.md Removed: diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index cab21c29a7486f..e4848f19e64d62 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -37,7 +37,6 @@ #include "lldb/Utility/RealpathPrefixes.h" #include "lldb/Utility/Timeout.h" #include "lldb/lldb-public.h" -#include "llvm/ADT/StringRef.h" namespace lldb_private { @@ -115,8 +114,6 @@ class TargetProperties : public Properties { void SetDisableSTDIO(bool b); - llvm::StringRef GetLaunchWorkingDirectory() const; - const char *GetDisassemblyFlavor() const; InlineStrategy GetInlineStrategy() const; diff --git a/lldb/source/Commands/CommandObjectProcess.cpp b/lldb/source/Commands/CommandObjectProcess.cpp index 7444e46aa729e7..e7c7d07ad47722 100644 --- a/lldb/source/Commands/CommandObjectProcess.cpp +++ b/lldb/source/Commands/CommandObjectProcess.cpp @@ -201,13 +201,6 @@ class CommandObjectProcessLaunch : public CommandObjectProcessLaunchOrAttach { if (target->GetDisableSTDIO()) m_options.launch_info.GetFlags().Set(eLaunchFlagDisableSTDIO); -if (!m_options.launch_info.GetWorkingDirectory()) { - if (llvm::StringRef wd = target->GetLaunchWorkingDirectory(); - !wd.empty()) { -m_options.launch_info.SetWorkingDirectory(FileSpec(wd)); - } -} - // Merge the launch info environment with the target environment. Environment target_env = target->GetEnvironment(); m_options.launch_info.GetEnvironment().insert(target_env.begin(), diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td index 9d8d45d083eca4..4276d9e7f9c8b0 100644 --- a/lldb/source/Commands/Options.td +++ b/lldb/source/Commands/Options.td @@ -691,10 +691,7 @@ let Command = "process launch" in { def process_launch_plugin : Option<"plugin", "P">, Arg<"Plugin">, Desc<"Name of the process plugin you want to use.">; def process_launch_working_dir : Option<"working-dir", "w">, Arg<"DirectoryName">, -Desc<"Set the current working directory to when running the inferior. This option " - "applies only to the current `process launch` invocation. If " - "`target.launch-working-dir` is set and this option is given, the value of this " - "option will be used instead of the setting.">; +Desc<"Set the current working directory to when running the inferior.">; def process_launch_arch : Option<"arch", "a">, Arg<"Architecture">, Desc<"Set the architecture for the process to launch when ambiguous.">; def process_launch_environment : Option<"environment", "E">, diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index 242d2eaec2a15a..8cd3fa8af6bae1 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -4471,11 +4471,6 @@ void TargetProperties::SetDisableSTDIO(bool b) { const uint32_t idx = ePropertyDisableSTDIO; SetPropertyAtIndex(idx, b); } -llvm::StringRef TargetProperties::GetLaunchWorkingDirectory() const { - const uint32_t idx = ePropertyLaunchWorkingDir; - return GetPropertyAtIndexAs( - idx, g_target_properties[idx].default_cstr_value); -} const char *TargetProperties::GetDisassemblyFlavor() const { const uint32_t idx = ePropertyDisassemblyFlavor; diff --git a/lldb/source/Target/TargetProperties.td b/lldb/source/Target/TargetProperties.td index 00ad8dd2a9f7f9..fb61478fb752dc 100644 --- a/lldb/source/Target/TargetProperties.td +++ b/lldb/source/Target/TargetProperties.td @@ -201,13 +201,6 @@ let Definition = "target" in { def DebugUtilityExpression: Property<"debug-utility-expression", "Boolean">, DefaultFalse, Desc<"Enable debugging of LLDB-internal utility expressions.">; - def LaunchWorkingDir: Property<"launch-working-dir", "String">, -DefaultStringValue<"">, -Desc<"A default value for the working directory to use when launching processes. " - "It is ignored when empty. This setting is only used when the target is " - "launched. If you change this setting, the new value will only apply to " -
[Lldb-commits] [lldb] [llvm] Revert "[LLDB] Add a target.launch-working-dir setting" (PR #114973)
https://github.com/walter-erquinigo edited https://github.com/llvm/llvm-project/pull/114973 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Revert "[LLDB] Add a target.launch-working-dir setting" (PR #114973)
https://github.com/walter-erquinigo closed https://github.com/llvm/llvm-project/pull/114973 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Revert "Fix pointer to reference type (#113596)" (PR #114831)
https://github.com/labath updated https://github.com/llvm/llvm-project/pull/114831 >From a98e1f6034479800f0c4ea053f9bee854151b04d Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Mon, 4 Nov 2024 17:56:06 +0100 Subject: [PATCH] Revert "Fix pointer to reference type (#113596)" This reverts commit 25909b811a7ddc983d042b15cb54ec271a673d63 due to unresolved questions about the behavior of "frame var" and ValueObject in the presence of references (see the original patch for discussion). --- lldb/source/ValueObject/ValueObject.cpp | 9 .../TestCPPDereferencingReferences.py | 21 --- .../cpp/dereferencing_references/main.cpp | 2 -- 3 files changed, 32 deletions(-) diff --git a/lldb/source/ValueObject/ValueObject.cpp b/lldb/source/ValueObject/ValueObject.cpp index aca43175d12fac..4006f6e6fd0a5e 100644 --- a/lldb/source/ValueObject/ValueObject.cpp +++ b/lldb/source/ValueObject/ValueObject.cpp @@ -2907,15 +2907,6 @@ ValueObjectSP ValueObject::AddressOf(Status &error) { AddressType address_type = eAddressTypeInvalid; const bool scalar_is_load_address = false; - - // For reference type we need to get the address of the object that - // it refers to. - if (GetCompilerType().IsReferenceType()) { -ValueObjectSP deref_obj = Dereference(error); -if (error.Fail() || !deref_obj) - return ValueObjectSP(); -return deref_obj->AddressOf(error); - } addr_t addr = GetAddressOf(scalar_is_load_address, &address_type); error.Clear(); if (addr != LLDB_INVALID_ADDRESS && address_type != eAddressTypeHost) { diff --git a/lldb/test/API/lang/cpp/dereferencing_references/TestCPPDereferencingReferences.py b/lldb/test/API/lang/cpp/dereferencing_references/TestCPPDereferencingReferences.py index 1374d4e1ec67ab..938fb1a6edf32c 100644 --- a/lldb/test/API/lang/cpp/dereferencing_references/TestCPPDereferencingReferences.py +++ b/lldb/test/API/lang/cpp/dereferencing_references/TestCPPDereferencingReferences.py @@ -25,24 +25,3 @@ def test(self): # Typedef to a reference should dereference to the underlying type. td_val = self.expect_var_path("td_to_ref_type", type="td_int_ref") self.assertEqual(td_val.Dereference().GetType().GetName(), "int") - -def test_take_address_of_reference(self): -"""Tests taking address of lvalue/rvalue references in lldb works correctly.""" -self.build() -lldbutil.run_to_source_breakpoint( -self, "// break here", lldb.SBFileSpec("main.cpp") -) - -plref_val_from_code = self.expect_var_path("pl_ref", type="TTT *") -plref_val_from_expr_path = self.expect_var_path("&l_ref", type="TTT *") -self.assertEqual( -plref_val_from_code.GetValueAsAddress(), -plref_val_from_expr_path.GetValueAsAddress(), -) - -prref_val_from_code = self.expect_var_path("pr_ref", type="TTT *") -prref_val_from_expr_path = self.expect_var_path("&r_ref", type="TTT *") -self.assertEqual( -prref_val_from_code.GetValueAsAddress(), -prref_val_from_expr_path.GetValueAsAddress(), -) diff --git a/lldb/test/API/lang/cpp/dereferencing_references/main.cpp b/lldb/test/API/lang/cpp/dereferencing_references/main.cpp index 4ddffd167ddeed..b64978a9029f81 100644 --- a/lldb/test/API/lang/cpp/dereferencing_references/main.cpp +++ b/lldb/test/API/lang/cpp/dereferencing_references/main.cpp @@ -9,7 +9,5 @@ int main() { // typedef of a reference td_int_ref td_to_ref_type = i; - TTT *pl_ref = &l_ref; - TTT *pr_ref = &r_ref; return l_ref; // break here } ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB] Add a target.launch-working-dir setting (PR #113521)
https://github.com/walter-erquinigo closed https://github.com/llvm/llvm-project/pull/113521 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [llvm] [WIP][lldb][Expression] More reliable function call resolution (PR #114529)
https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/114529 >From 9337e170d920eaabe2b59a25622f0c554ca5afcf Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Sun, 20 Oct 2024 11:35:15 +0100 Subject: [PATCH 1/2] [WIP][lldb][Expression] More reliable function call resolution Implements all the parts of following RFC: https://discourse.llvm.org/t/rfc-lldb-handling-abi-tagged-constructors-destructors-in-expression-evaluator/82816 Main changes: 1. Instead of relying on linkage names to resolve function symbols, encode the exact function DIE and module in the `AsmLabelAttr` 2. Teach the LLDB symbol resolve about (1) 3. Introduce new Clang attribute to allow specifying multiple `asm` labels for ctors/dtors (one for each variant) 4. Attach the new attribute in (3), where the mangled names use the format from (1). To determine which variant a DIE corresponds to we add a new API to the `ItaniumPartialDemangler` (though could be made into a DWARF attribute for quicker determination). --- clang/include/clang/Basic/Attr.td | 8 ++ clang/include/clang/Basic/AttrDocs.td | 5 ++ clang/lib/AST/Mangle.cpp | 69 +++- clang/lib/Sema/SemaDeclAttr.cpp | 22 + lldb/source/Expression/IRExecutionUnit.cpp| 36 + .../SymbolFile/DWARF/DWARFASTParserClang.cpp | 80 +++ .../TypeSystem/Clang/TypeSystemClang.cpp | 15 +++- .../TypeSystem/Clang/TypeSystemClang.h| 3 +- lldb/test/Shell/Expr/TestAbiTagCtorsDtors.cpp | 28 +++ llvm/include/llvm/Demangle/Demangle.h | 3 + llvm/include/llvm/Demangle/ItaniumDemangle.h | 2 + llvm/lib/Demangle/ItaniumDemangle.cpp | 18 +++-- 12 files changed, 264 insertions(+), 25 deletions(-) create mode 100644 lldb/test/Shell/Expr/TestAbiTagCtorsDtors.cpp diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 70fad60d4edbb5..407eece2a728a2 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -784,6 +784,14 @@ def AbiTag : Attr { let Documentation = [AbiTagsDocs]; } +def StructorMangledNames : Attr { + let Spellings = [Clang<"structor_names">]; + let Args = [VariadicStringArgument<"MangledNames">]; + let Subjects = SubjectList<[Function], ErrorDiag>; + let Documentation = [StructorMangledNamesDocs]; +} + + def AddressSpace : TypeAttr { let Spellings = [Clang<"address_space">]; let Args = [IntArgument<"AddressSpace">]; diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 546e5100b79dd9..2b886aecd193de 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -3568,6 +3568,11 @@ manipulating bits of the enumerator when issuing warnings. }]; } +def StructorMangledNamesDocs : Documentation { + let Category = DocCatDecl; + let Content = [{ TODO }]; +} + def AsmLabelDocs : Documentation { let Category = DocCatDecl; let Content = [{ diff --git a/clang/lib/AST/Mangle.cpp b/clang/lib/AST/Mangle.cpp index 4875e8537b3c11..9b304d28113625 100644 --- a/clang/lib/AST/Mangle.cpp +++ b/clang/lib/AST/Mangle.cpp @@ -9,19 +9,20 @@ // Implements generic name mangling support for blocks and Objective-C. // //===--===// -#include "clang/AST/Attr.h" +#include "clang/AST/Mangle.h" #include "clang/AST/ASTContext.h" +#include "clang/AST/Attr.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/DeclTemplate.h" #include "clang/AST/ExprCXX.h" -#include "clang/AST/Mangle.h" #include "clang/AST/VTableBuilder.h" #include "clang/Basic/ABI.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TargetInfo.h" #include "llvm/ADT/StringExtras.h" +#include "llvm/ADT/StringSwitch.h" #include "llvm/IR/DataLayout.h" #include "llvm/IR/Mangler.h" #include "llvm/Support/ErrorHandling.h" @@ -126,7 +127,7 @@ bool MangleContext::shouldMangleDeclName(const NamedDecl *D) { // Any decl can be declared with __asm("foo") on it, and this takes precedence // over all other naming in the .o file. - if (D->hasAttr()) + if (D->hasAttr() || D->hasAttr()) return true; // Declarations that don't have identifier names always need to be mangled. @@ -140,6 +141,68 @@ void MangleContext::mangleName(GlobalDecl GD, raw_ostream &Out) { const ASTContext &ASTContext = getASTContext(); const NamedDecl *D = cast(GD.getDecl()); + if (const StructorMangledNamesAttr *SMA = + D->getAttr()) { +CXXConstructorDecl const *Ctor = dyn_cast(D); +CXXDestructorDecl const *Dtor = dyn_cast(D); +assert(Ctor || Dtor); +enum CtorDtor { + None = -1, + Deleting = 0, + Base, + Complete, + Allocating +} CtorDtorVariant = None; + +// Map ctor/dtor variant to a the variant that LLDB encoded in the
[Lldb-commits] [lldb] [llvm] Revert "[LLDB] Add a target.launch-working-dir setting" (PR #114973)
https://github.com/walter-erquinigo created https://github.com/llvm/llvm-project/pull/114973 Reverts llvm/llvm-project#113521 >From 84633d7ccc926abff46c6480dbe7ccc7e48247ce Mon Sep 17 00:00:00 2001 From: Walter Erquinigo Date: Tue, 5 Nov 2024 07:11:32 -0500 Subject: [PATCH] Revert "[LLDB] Add a target.launch-working-dir setting (#113521)" This reverts commit 6620cd25234a42ca4b51490627afcb93fa443dc3. --- lldb/include/lldb/Target/Target.h | 3 - lldb/source/Commands/CommandObjectProcess.cpp | 7 --- lldb/source/Commands/Options.td | 5 +- lldb/source/Target/Target.cpp | 5 -- lldb/source/Target/TargetProperties.td| 7 --- .../process/launch/TestProcessLaunch.py | 57 --- llvm/docs/ReleaseNotes.md | 2 - 7 files changed, 1 insertion(+), 85 deletions(-) diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index cab21c29a7486f..e4848f19e64d62 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -37,7 +37,6 @@ #include "lldb/Utility/RealpathPrefixes.h" #include "lldb/Utility/Timeout.h" #include "lldb/lldb-public.h" -#include "llvm/ADT/StringRef.h" namespace lldb_private { @@ -115,8 +114,6 @@ class TargetProperties : public Properties { void SetDisableSTDIO(bool b); - llvm::StringRef GetLaunchWorkingDirectory() const; - const char *GetDisassemblyFlavor() const; InlineStrategy GetInlineStrategy() const; diff --git a/lldb/source/Commands/CommandObjectProcess.cpp b/lldb/source/Commands/CommandObjectProcess.cpp index 7444e46aa729e7..e7c7d07ad47722 100644 --- a/lldb/source/Commands/CommandObjectProcess.cpp +++ b/lldb/source/Commands/CommandObjectProcess.cpp @@ -201,13 +201,6 @@ class CommandObjectProcessLaunch : public CommandObjectProcessLaunchOrAttach { if (target->GetDisableSTDIO()) m_options.launch_info.GetFlags().Set(eLaunchFlagDisableSTDIO); -if (!m_options.launch_info.GetWorkingDirectory()) { - if (llvm::StringRef wd = target->GetLaunchWorkingDirectory(); - !wd.empty()) { -m_options.launch_info.SetWorkingDirectory(FileSpec(wd)); - } -} - // Merge the launch info environment with the target environment. Environment target_env = target->GetEnvironment(); m_options.launch_info.GetEnvironment().insert(target_env.begin(), diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td index 9d8d45d083eca4..4276d9e7f9c8b0 100644 --- a/lldb/source/Commands/Options.td +++ b/lldb/source/Commands/Options.td @@ -691,10 +691,7 @@ let Command = "process launch" in { def process_launch_plugin : Option<"plugin", "P">, Arg<"Plugin">, Desc<"Name of the process plugin you want to use.">; def process_launch_working_dir : Option<"working-dir", "w">, Arg<"DirectoryName">, -Desc<"Set the current working directory to when running the inferior. This option " - "applies only to the current `process launch` invocation. If " - "`target.launch-working-dir` is set and this option is given, the value of this " - "option will be used instead of the setting.">; +Desc<"Set the current working directory to when running the inferior.">; def process_launch_arch : Option<"arch", "a">, Arg<"Architecture">, Desc<"Set the architecture for the process to launch when ambiguous.">; def process_launch_environment : Option<"environment", "E">, diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index 242d2eaec2a15a..8cd3fa8af6bae1 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -4471,11 +4471,6 @@ void TargetProperties::SetDisableSTDIO(bool b) { const uint32_t idx = ePropertyDisableSTDIO; SetPropertyAtIndex(idx, b); } -llvm::StringRef TargetProperties::GetLaunchWorkingDirectory() const { - const uint32_t idx = ePropertyLaunchWorkingDir; - return GetPropertyAtIndexAs( - idx, g_target_properties[idx].default_cstr_value); -} const char *TargetProperties::GetDisassemblyFlavor() const { const uint32_t idx = ePropertyDisassemblyFlavor; diff --git a/lldb/source/Target/TargetProperties.td b/lldb/source/Target/TargetProperties.td index 00ad8dd2a9f7f9..fb61478fb752dc 100644 --- a/lldb/source/Target/TargetProperties.td +++ b/lldb/source/Target/TargetProperties.td @@ -201,13 +201,6 @@ let Definition = "target" in { def DebugUtilityExpression: Property<"debug-utility-expression", "Boolean">, DefaultFalse, Desc<"Enable debugging of LLDB-internal utility expressions.">; - def LaunchWorkingDir: Property<"launch-working-dir", "String">, -DefaultStringValue<"">, -Desc<"A default value for the working directory to use when launching processes. " - "It is ignored when empty. This setting is only used when the target is " - "launched. If you change this setting, the new value will only apply to " - "subsequent launche
[Lldb-commits] [lldb] [llvm] [LLDB] Add a target.launch-working-dir setting (PR #113521)
llvm-ci wrote: LLVM Buildbot has detected a new failure on builder `lldb-remote-linux-ubuntu` running on `as-builder-9` while building `lldb,llvm` at step 16 "test-check-lldb-api". Full details are available at: https://lab.llvm.org/buildbot/#/builders/195/builds/622 Here is the relevant piece of the build log for the reference ``` Step 16 (test-check-lldb-api) failure: Test just built components: check-lldb-api completed (failure) TEST 'lldb-api :: commands/process/launch/TestProcessLaunch.py' FAILED Script: -- /usr/bin/python3.12 /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS --env LLVM_LIBS_DIR=/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./lib --env LLVM_INCLUDE_DIR=/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/include --env LLVM_TOOLS_DIR=/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./bin --libcxx-include-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/include/c++/v1 --libcxx-include-target-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/include/aarch64-unknown-linux-gnu/c++/v1 --libcxx-library-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./lib/aarch64-unknown-linux-gnu --arch aarch64 --build-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/lldb-test-build.noindex --lldb-module-cache-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/lldb-test-build.noindex/module-cache-lldb/lldb-api --clang-module-cache-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/lldb-test-build.noindex/module-cache-clang/lldb-api --executable /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./bin/lldb --compiler /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/bin/clang --dsymutil /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./bin/dsymutil --make /usr/bin/make --llvm-tools-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./bin --lldb-obj-root /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/tools/lldb --lldb-libs-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./lib --platform-url connect://jetson-agx-2198.lab.llvm.org:1234 --platform-working-dir /home/ubuntu/lldb-tests --sysroot /mnt/fs/jetson-agx-ubuntu --env ARCH_CFLAGS=-mcpu=cortex-a78 --platform-name remote-linux /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/lldb/test/API/commands/process/launch -p TestProcessLaunch.py -- Exit Code: 1 Command Output (stdout): -- lldb version 20.0.0git (https://github.com/llvm/llvm-project.git revision 6620cd25234a42ca4b51490627afcb93fa443dc3) clang revision 6620cd25234a42ca4b51490627afcb93fa443dc3 llvm revision 6620cd25234a42ca4b51490627afcb93fa443dc3 Setting up remote platform 'remote-linux' Connecting to remote platform 'remote-linux' at 'connect://jetson-agx-2198.lab.llvm.org:1234'... Connected. Setting remote platform working directory to '/home/ubuntu/lldb-tests'... Skipping the following test categories: ['dsym', 'gmodules', 'debugserver', 'objc', 'lldb-dap'] -- Command Output (stderr): -- WARNING:root:Custom libc++ is not supported for remote runs: ignoring --libcxx arguments PASS: LLDB (/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/bin/clang-aarch64) :: test_environment_with_special_char (TestProcessLaunch.ProcessLaunchTestCase.test_environment_with_special_char) UNSUPPORTED: LLDB (/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/bin/clang-aarch64) :: test_io (TestProcessLaunch.ProcessLaunchTestCase.test_io) (skip on remote platform) UNSUPPORTED: LLDB (/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/bin/clang-aarch64) :: test_set_working_dir_existing (TestProcessLaunch.ProcessLaunchTestCase.test_set_working_dir_existing) (skip on remote platform) UNSUPPORTED: LLDB (/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/bin/clang-aarch64) :: test_set_working_dir_nonexisting (TestProcessLaunch.ProcessLaunchTestCase.test_set_working_dir_nonexisting) (skip on remote platform) FAIL: LLDB (/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/bin/clang-aarch64) :: test_target_launch_working_dir_prop (TestProcessLaunch.ProcessLaunchTestCase.test_target_launch_working_dir_prop) == FAIL: test_target_launch_working_dir_prop (TestProcessLaunch.ProcessLaunchTestCase.test_target_launch_working_dir_prop) Test that the setting `target.launch-working-dir` is correctly used when launching a process. -- Traceback (most recent call last): File "/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/
[Lldb-commits] [lldb] [llvm] Revert "[LLDB] Add a target.launch-working-dir setting" (PR #114973)
llvm-ci wrote: LLVM Buildbot has detected a new failure on builder `openmp-s390x-linux` running on `systemz-1` while building `lldb,llvm` at step 6 "test-openmp". Full details are available at: https://lab.llvm.org/buildbot/#/builders/88/builds/4281 Here is the relevant piece of the build log for the reference ``` Step 6 (test-openmp) failure: test (failure) TEST 'libomp :: tasking/issue-94260-2.c' FAILED Exit Code: -11 Command Output (stdout): -- # RUN: at line 1 /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/./bin/clang -fopenmp -I /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -I /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.src/openmp/runtime/test -L /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -fno-omit-frame-pointer -mbackchain -I /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.src/openmp/runtime/test/ompt /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.src/openmp/runtime/test/tasking/issue-94260-2.c -o /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/test/tasking/Output/issue-94260-2.c.tmp -lm -latomic && /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/test/tasking/Output/issue-94260-2.c.tmp # executed command: /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/./bin/clang -fopenmp -I /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -I /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.src/openmp/runtime/test -L /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -fno-omit-frame-pointer -mbackchain -I /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.src/openmp/runtime/test/ompt /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.src/openmp/runtime/test/tasking/issue-94260-2.c -o /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/test/tasking/Output/issue-94260-2.c.tmp -lm -latomic # executed command: /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/test/tasking/Output/issue-94260-2.c.tmp # note: command had no output on stdout or stderr # error: command failed with exit status: -11 -- ``` https://github.com/llvm/llvm-project/pull/114973 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Revert "[LLDB] Add a target.launch-working-dir setting" (PR #114973)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Walter Erquinigo (walter-erquinigo) Changes Reverts llvm/llvm-project#113521 due to build bot failures mentioned in the original PR. --- Full diff: https://github.com/llvm/llvm-project/pull/114973.diff 7 Files Affected: - (modified) lldb/include/lldb/Target/Target.h (-3) - (modified) lldb/source/Commands/CommandObjectProcess.cpp (-7) - (modified) lldb/source/Commands/Options.td (+1-4) - (modified) lldb/source/Target/Target.cpp (-5) - (modified) lldb/source/Target/TargetProperties.td (-7) - (modified) lldb/test/API/commands/process/launch/TestProcessLaunch.py (-57) - (modified) llvm/docs/ReleaseNotes.md (-2) ``diff diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index cab21c29a7486f..e4848f19e64d62 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -37,7 +37,6 @@ #include "lldb/Utility/RealpathPrefixes.h" #include "lldb/Utility/Timeout.h" #include "lldb/lldb-public.h" -#include "llvm/ADT/StringRef.h" namespace lldb_private { @@ -115,8 +114,6 @@ class TargetProperties : public Properties { void SetDisableSTDIO(bool b); - llvm::StringRef GetLaunchWorkingDirectory() const; - const char *GetDisassemblyFlavor() const; InlineStrategy GetInlineStrategy() const; diff --git a/lldb/source/Commands/CommandObjectProcess.cpp b/lldb/source/Commands/CommandObjectProcess.cpp index 7444e46aa729e7..e7c7d07ad47722 100644 --- a/lldb/source/Commands/CommandObjectProcess.cpp +++ b/lldb/source/Commands/CommandObjectProcess.cpp @@ -201,13 +201,6 @@ class CommandObjectProcessLaunch : public CommandObjectProcessLaunchOrAttach { if (target->GetDisableSTDIO()) m_options.launch_info.GetFlags().Set(eLaunchFlagDisableSTDIO); -if (!m_options.launch_info.GetWorkingDirectory()) { - if (llvm::StringRef wd = target->GetLaunchWorkingDirectory(); - !wd.empty()) { -m_options.launch_info.SetWorkingDirectory(FileSpec(wd)); - } -} - // Merge the launch info environment with the target environment. Environment target_env = target->GetEnvironment(); m_options.launch_info.GetEnvironment().insert(target_env.begin(), diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td index 9d8d45d083eca4..4276d9e7f9c8b0 100644 --- a/lldb/source/Commands/Options.td +++ b/lldb/source/Commands/Options.td @@ -691,10 +691,7 @@ let Command = "process launch" in { def process_launch_plugin : Option<"plugin", "P">, Arg<"Plugin">, Desc<"Name of the process plugin you want to use.">; def process_launch_working_dir : Option<"working-dir", "w">, Arg<"DirectoryName">, -Desc<"Set the current working directory to when running the inferior. This option " - "applies only to the current `process launch` invocation. If " - "`target.launch-working-dir` is set and this option is given, the value of this " - "option will be used instead of the setting.">; +Desc<"Set the current working directory to when running the inferior.">; def process_launch_arch : Option<"arch", "a">, Arg<"Architecture">, Desc<"Set the architecture for the process to launch when ambiguous.">; def process_launch_environment : Option<"environment", "E">, diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index 242d2eaec2a15a..8cd3fa8af6bae1 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -4471,11 +4471,6 @@ void TargetProperties::SetDisableSTDIO(bool b) { const uint32_t idx = ePropertyDisableSTDIO; SetPropertyAtIndex(idx, b); } -llvm::StringRef TargetProperties::GetLaunchWorkingDirectory() const { - const uint32_t idx = ePropertyLaunchWorkingDir; - return GetPropertyAtIndexAs( - idx, g_target_properties[idx].default_cstr_value); -} const char *TargetProperties::GetDisassemblyFlavor() const { const uint32_t idx = ePropertyDisassemblyFlavor; diff --git a/lldb/source/Target/TargetProperties.td b/lldb/source/Target/TargetProperties.td index 00ad8dd2a9f7f9..fb61478fb752dc 100644 --- a/lldb/source/Target/TargetProperties.td +++ b/lldb/source/Target/TargetProperties.td @@ -201,13 +201,6 @@ let Definition = "target" in { def DebugUtilityExpression: Property<"debug-utility-expression", "Boolean">, DefaultFalse, Desc<"Enable debugging of LLDB-internal utility expressions.">; - def LaunchWorkingDir: Property<"launch-working-dir", "String">, -DefaultStringValue<"">, -Desc<"A default value for the working directory to use when launching processes. " - "It is ignored when empty. This setting is only used when the target is " - "launched. If you change this setting, the new value will only apply to " - "subsequent launches. Commands that take an explicit working directory " - "will override this setting.">; } let Definition = "process_experimental" in { diff --git a/lldb
[Lldb-commits] [clang] [lldb] [llvm] [WIP][lldb][Expression] More reliable function call resolution (PR #114529)
https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/114529 >From 9337e170d920eaabe2b59a25622f0c554ca5afcf Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Sun, 20 Oct 2024 11:35:15 +0100 Subject: [PATCH 1/2] [WIP][lldb][Expression] More reliable function call resolution Implements all the parts of following RFC: https://discourse.llvm.org/t/rfc-lldb-handling-abi-tagged-constructors-destructors-in-expression-evaluator/82816 Main changes: 1. Instead of relying on linkage names to resolve function symbols, encode the exact function DIE and module in the `AsmLabelAttr` 2. Teach the LLDB symbol resolve about (1) 3. Introduce new Clang attribute to allow specifying multiple `asm` labels for ctors/dtors (one for each variant) 4. Attach the new attribute in (3), where the mangled names use the format from (1). To determine which variant a DIE corresponds to we add a new API to the `ItaniumPartialDemangler` (though could be made into a DWARF attribute for quicker determination). --- clang/include/clang/Basic/Attr.td | 8 ++ clang/include/clang/Basic/AttrDocs.td | 5 ++ clang/lib/AST/Mangle.cpp | 69 +++- clang/lib/Sema/SemaDeclAttr.cpp | 22 + lldb/source/Expression/IRExecutionUnit.cpp| 36 + .../SymbolFile/DWARF/DWARFASTParserClang.cpp | 80 +++ .../TypeSystem/Clang/TypeSystemClang.cpp | 15 +++- .../TypeSystem/Clang/TypeSystemClang.h| 3 +- lldb/test/Shell/Expr/TestAbiTagCtorsDtors.cpp | 28 +++ llvm/include/llvm/Demangle/Demangle.h | 3 + llvm/include/llvm/Demangle/ItaniumDemangle.h | 2 + llvm/lib/Demangle/ItaniumDemangle.cpp | 18 +++-- 12 files changed, 264 insertions(+), 25 deletions(-) create mode 100644 lldb/test/Shell/Expr/TestAbiTagCtorsDtors.cpp diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 70fad60d4edbb5..407eece2a728a2 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -784,6 +784,14 @@ def AbiTag : Attr { let Documentation = [AbiTagsDocs]; } +def StructorMangledNames : Attr { + let Spellings = [Clang<"structor_names">]; + let Args = [VariadicStringArgument<"MangledNames">]; + let Subjects = SubjectList<[Function], ErrorDiag>; + let Documentation = [StructorMangledNamesDocs]; +} + + def AddressSpace : TypeAttr { let Spellings = [Clang<"address_space">]; let Args = [IntArgument<"AddressSpace">]; diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 546e5100b79dd9..2b886aecd193de 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -3568,6 +3568,11 @@ manipulating bits of the enumerator when issuing warnings. }]; } +def StructorMangledNamesDocs : Documentation { + let Category = DocCatDecl; + let Content = [{ TODO }]; +} + def AsmLabelDocs : Documentation { let Category = DocCatDecl; let Content = [{ diff --git a/clang/lib/AST/Mangle.cpp b/clang/lib/AST/Mangle.cpp index 4875e8537b3c11..9b304d28113625 100644 --- a/clang/lib/AST/Mangle.cpp +++ b/clang/lib/AST/Mangle.cpp @@ -9,19 +9,20 @@ // Implements generic name mangling support for blocks and Objective-C. // //===--===// -#include "clang/AST/Attr.h" +#include "clang/AST/Mangle.h" #include "clang/AST/ASTContext.h" +#include "clang/AST/Attr.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/DeclTemplate.h" #include "clang/AST/ExprCXX.h" -#include "clang/AST/Mangle.h" #include "clang/AST/VTableBuilder.h" #include "clang/Basic/ABI.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TargetInfo.h" #include "llvm/ADT/StringExtras.h" +#include "llvm/ADT/StringSwitch.h" #include "llvm/IR/DataLayout.h" #include "llvm/IR/Mangler.h" #include "llvm/Support/ErrorHandling.h" @@ -126,7 +127,7 @@ bool MangleContext::shouldMangleDeclName(const NamedDecl *D) { // Any decl can be declared with __asm("foo") on it, and this takes precedence // over all other naming in the .o file. - if (D->hasAttr()) + if (D->hasAttr() || D->hasAttr()) return true; // Declarations that don't have identifier names always need to be mangled. @@ -140,6 +141,68 @@ void MangleContext::mangleName(GlobalDecl GD, raw_ostream &Out) { const ASTContext &ASTContext = getASTContext(); const NamedDecl *D = cast(GD.getDecl()); + if (const StructorMangledNamesAttr *SMA = + D->getAttr()) { +CXXConstructorDecl const *Ctor = dyn_cast(D); +CXXDestructorDecl const *Dtor = dyn_cast(D); +assert(Ctor || Dtor); +enum CtorDtor { + None = -1, + Deleting = 0, + Base, + Complete, + Allocating +} CtorDtorVariant = None; + +// Map ctor/dtor variant to a the variant that LLDB encoded in the
[Lldb-commits] [lldb] [lldb][breakpoint] Grey out disabled breakpoints (PR #91404)
@@ -47,6 +48,10 @@ class LLDB_API SBStream { void Print(const char *str); + bool HasColor(); + + void FormatAnsiTerminalCodes(llvm::StringRef format); labath wrote: You can't have a StringRef in the SB API. This needs to be a const char *. All the internal APIs can and should use StringRefs though. https://github.com/llvm/llvm-project/pull/91404 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap to support running in a server mode, allowing multiple connections. (PR #114881)
@@ -9,21 +9,28 @@ #ifndef LLDB_TOOLS_LLDB_DAP_BREAKPOINTBASE_H #define LLDB_TOOLS_LLDB_DAP_BREAKPOINTBASE_H -#include "llvm/Support/JSON.h" #include +#include "llvm/Support/JSON.h" + +#include "DAPForward.h" + namespace lldb_dap { struct BreakpointBase { + // Associated DAP session. + DAP *dap; labath wrote: Although I don't consider myself an lldb-dap maintainer, I want to speak out, as strongly as I can, against the usage of shared/weak_ptr here. I have a couple of reasons for that: 1. it's an LLDB tradition that can't be found almost anywhere else in llvm 2. it encourages "vague ownership" semantics, where noone can be really sure whether some object still exists or not (so it handles the "not" case "just in case"). This in turn leads to a lot of dead/untested code. If what you say is true and the DAP object is the owner of this object, then I would say this is a reason to *not* use weak (or shared) pointers, as that the owned object should be able to assume that its owner is always around (and not worry about what to do with a possibly empty result of `weak_ptr::lock`). LLDB sort of has an excuse for using shared_ptrs (a python scripting API, which means code outside our influence can hold on to lldb objects), but I don't think anything like that applies to lldb-dap. 3. it's impossible to distinguish a "potentially empty" shared/weak pointer from one that is always set. This is kind of similar to the previous one, but without the temporal aspect. Even if you ignore the possibility of the object going away mid-flight, you still can't enforce that it is always there to start with. With raw pointers/reference, you can. 4. Makes it harder to maintain const correctness. It still possible (`shared_ptr` is a thing), but it seems people don't usually bother to do that. Const correctness is of particular importance for multithreaded code. 5. All of this copying, locking and atomic operations inherent in shared pointers make things slower. https://github.com/llvm/llvm-project/pull/114881 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [lldb] Analyze enum promotion type during parsing (PR #115005)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Ilia Kuklin (kuilpd) Changes The information about an enum's best promotion type is discarded after compilation and is not present in debug info. This patch repeats the same analysis of each enum value as in the front-end to determine the best promotion type during DWARF info parsing. Fixes #86989 --- Full diff: https://github.com/llvm/llvm-project/pull/115005.diff 6 Files Affected: - (modified) clang/include/clang/AST/Decl.h (+1-1) - (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp (+95-1) - (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp (+5-24) - (added) lldb/test/API/lang/cpp/enum_promotion/Makefile (+3) - (added) lldb/test/API/lang/cpp/enum_promotion/TestCPPEnumPromotion.py (+37) - (added) lldb/test/API/lang/cpp/enum_promotion/main.cpp (+22) ``diff diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h index 8c39ef3d5a9fa6..a78e6c92abb22b 100644 --- a/clang/include/clang/AST/Decl.h +++ b/clang/include/clang/AST/Decl.h @@ -3891,6 +3891,7 @@ class EnumDecl : public TagDecl { void setInstantiationOfMemberEnum(ASTContext &C, EnumDecl *ED, TemplateSpecializationKind TSK); +public: /// Sets the width in bits required to store all the /// non-negative enumerators of this enum. void setNumPositiveBits(unsigned Num) { @@ -3902,7 +3903,6 @@ class EnumDecl : public TagDecl { /// negative enumerators of this enum. (see getNumNegativeBits) void setNumNegativeBits(unsigned Num) { EnumDeclBits.NumNegativeBits = Num; } -public: /// True if this tag declaration is a scoped enumeration. Only /// possible in C++11 mode. void setScoped(bool Scoped = true) { EnumDeclBits.IsScoped = Scoped; } diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index a30d898a93cc4d..520fd40eda4825 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -2248,6 +2248,8 @@ size_t DWARFASTParserClang::ParseChildEnumerators( return 0; size_t enumerators_added = 0; + unsigned NumNegativeBits = 0; + unsigned NumPositiveBits = 0; for (DWARFDIE die : parent_die.children()) { const dw_tag_t tag = die.Tag(); @@ -2299,11 +2301,103 @@ size_t DWARFASTParserClang::ParseChildEnumerators( } if (name && name[0] && got_value) { - m_ast.AddEnumerationValueToEnumerationType( + auto ECD = m_ast.AddEnumerationValueToEnumerationType( clang_type, decl, name, enum_value, enumerator_byte_size * 8); ++enumerators_added; + + llvm::APSInt InitVal = ECD->getInitVal(); + // Keep track of the size of positive and negative values. + if (InitVal.isUnsigned() || InitVal.isNonNegative()) { +// If the enumerator is zero that should still be counted as a positive +// bit since we need a bit to store the value zero. +unsigned ActiveBits = InitVal.getActiveBits(); +NumPositiveBits = std::max({NumPositiveBits, ActiveBits, 1u}); + } else { +NumNegativeBits = +std::max(NumNegativeBits, (unsigned)InitVal.getSignificantBits()); + } } } + + /// The following code follows the same logic as in Sema::ActOnEnumBody + /// clang/lib/Sema/SemaDecl.cpp + // If we have an empty set of enumerators we still need one bit. + // From [dcl.enum]p8 + // If the enumerator-list is empty, the values of the enumeration are as if + // the enumeration had a single enumerator with value 0 + if (!NumPositiveBits && !NumNegativeBits) +NumPositiveBits = 1; + + clang::EnumDecl *enum_decl = + ClangUtil::GetQualType(clang_type)->getAs()->getDecl(); + enum_decl->setNumPositiveBits(NumPositiveBits); + enum_decl->setNumNegativeBits(NumNegativeBits); + + // C++0x N3000 [conv.prom]p3: + // An rvalue of an unscoped enumeration type whose underlying + // type is not fixed can be converted to an rvalue of the first + // of the following types that can represent all the values of + // the enumeration: int, unsigned int, long int, unsigned long + // int, long long int, or unsigned long long int. + // C99 6.4.4.3p2: + // An identifier declared as an enumeration constant has type int. + // The C99 rule is modified by C23. + clang::QualType BestPromotionType; + unsigned BestWidth; + + auto &Context = m_ast.getASTContext(); + unsigned LongWidth = Context.getTargetInfo().getLongWidth(); + unsigned IntWidth = Context.getTargetInfo().getIntWidth(); + unsigned CharWidth = Context.getTargetInfo().getCharWidth(); + unsigned ShortWidth = Context.getTargetInfo().getShortWidth(); + + bool is_cpp = Language::LanguageIsCPlusPlus( + SymbolFileDWARF::GetLanguage(*parent_die.GetCU())); + + if (NumNegativeBits) { +// If there is a negative value, figure out t
[Lldb-commits] [clang] [lldb] [lldb] Analyze enum promotion type during parsing (PR #115005)
https://github.com/kuilpd created https://github.com/llvm/llvm-project/pull/115005 The information about an enum's best promotion type is discarded after compilation and is not present in debug info. This patch repeats the same analysis of each enum value as in the front-end to determine the best promotion type during DWARF info parsing. Fixes #86989 >From 5290832b802d98b9d293b6910c0837911ec490c4 Mon Sep 17 00:00:00 2001 From: Ilia Kuklin Date: Mon, 4 Nov 2024 14:33:45 +0500 Subject: [PATCH 1/3] [lldb] Analyze enum promotion type during parsing --- clang/include/clang/AST/Decl.h| 2 +- .../SymbolFile/DWARF/DWARFASTParserClang.cpp | 95 ++- .../TypeSystem/Clang/TypeSystemClang.cpp | 29 +- 3 files changed, 100 insertions(+), 26 deletions(-) diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h index 8c39ef3d5a9fa6..a78e6c92abb22b 100644 --- a/clang/include/clang/AST/Decl.h +++ b/clang/include/clang/AST/Decl.h @@ -3891,6 +3891,7 @@ class EnumDecl : public TagDecl { void setInstantiationOfMemberEnum(ASTContext &C, EnumDecl *ED, TemplateSpecializationKind TSK); +public: /// Sets the width in bits required to store all the /// non-negative enumerators of this enum. void setNumPositiveBits(unsigned Num) { @@ -3902,7 +3903,6 @@ class EnumDecl : public TagDecl { /// negative enumerators of this enum. (see getNumNegativeBits) void setNumNegativeBits(unsigned Num) { EnumDeclBits.NumNegativeBits = Num; } -public: /// True if this tag declaration is a scoped enumeration. Only /// possible in C++11 mode. void setScoped(bool Scoped = true) { EnumDeclBits.IsScoped = Scoped; } diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index a30d898a93cc4d..a21607c2020161 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -2248,6 +2248,8 @@ size_t DWARFASTParserClang::ParseChildEnumerators( return 0; size_t enumerators_added = 0; + unsigned NumNegativeBits = 0; + unsigned NumPositiveBits = 0; for (DWARFDIE die : parent_die.children()) { const dw_tag_t tag = die.Tag(); @@ -2299,11 +2301,102 @@ size_t DWARFASTParserClang::ParseChildEnumerators( } if (name && name[0] && got_value) { - m_ast.AddEnumerationValueToEnumerationType( + auto ECD = m_ast.AddEnumerationValueToEnumerationType( clang_type, decl, name, enum_value, enumerator_byte_size * 8); ++enumerators_added; + + llvm::APSInt InitVal = ECD->getInitVal(); + // Keep track of the size of positive and negative values. + if (InitVal.isUnsigned() || InitVal.isNonNegative()) { +// If the enumerator is zero that should still be counted as a positive +// bit since we need a bit to store the value zero. +unsigned ActiveBits = InitVal.getActiveBits(); +NumPositiveBits = std::max({NumPositiveBits, ActiveBits, 1u}); + } else { +NumNegativeBits = +std::max(NumNegativeBits, (unsigned)InitVal.getSignificantBits()); + } } } + + /// The following code follows the same logic as in Sema::ActOnEnumBody + /// clang/lib/Sema/SemaDecl.cpp + // If we have an empty set of enumerators we still need one bit. + // From [dcl.enum]p8 + // If the enumerator-list is empty, the values of the enumeration are as if + // the enumeration had a single enumerator with value 0 + if (!NumPositiveBits && !NumNegativeBits) +NumPositiveBits = 1; + + clang::QualType qual_type(ClangUtil::GetQualType(clang_type)); + clang::EnumDecl *enum_decl = qual_type->getAs()->getDecl(); + enum_decl->setNumPositiveBits(NumPositiveBits); + enum_decl->setNumNegativeBits(NumNegativeBits); + + // C++0x N3000 [conv.prom]p3: + // An rvalue of an unscoped enumeration type whose underlying + // type is not fixed can be converted to an rvalue of the first + // of the following types that can represent all the values of + // the enumeration: int, unsigned int, long int, unsigned long + // int, long long int, or unsigned long long int. + // C99 6.4.4.3p2: + // An identifier declared as an enumeration constant has type int. + // The C99 rule is modified by C23. + clang::QualType BestPromotionType; + unsigned BestWidth; + + auto &Context = m_ast.getASTContext(); + unsigned LongWidth = Context.getTargetInfo().getLongWidth(); + unsigned IntWidth = Context.getTargetInfo().getIntWidth(); + unsigned CharWidth = Context.getTargetInfo().getCharWidth(); + unsigned ShortWidth = Context.getTargetInfo().getShortWidth(); + + bool is_cpp = Language::LanguageIsCPlusPlus( + SymbolFileDWARF::GetLanguage(*parent_die.GetCU())); + + if (NumNegativeBits) { +// If there is a negative value, figure out the smallest integer type (of +// int/long/longl
[Lldb-commits] [clang] [lldb] [lldb] Analyze enum promotion type during parsing (PR #115005)
kuilpd wrote: My thought process for this patch: https://github.com/llvm/llvm-project/issues/86989#issuecomment-2438116468 https://github.com/llvm/llvm-project/pull/115005 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [lldb] Analyze enum promotion type during parsing (PR #115005)
github-actions[bot] wrote: :warning: Python code formatter, darker found issues in your code. :warning: You can test this locally with the following command: ``bash darker --check --diff -r b2d2494731976ab7aa9702f3134472db694b9332...62c801145a2312d2c9339d30cf116fc2e709d630 lldb/test/API/lang/cpp/enum_promotion/TestCPPEnumPromotion.py `` View the diff from darker here. ``diff --- TestCPPEnumPromotion.py 2024-11-05 14:20:51.00 + +++ TestCPPEnumPromotion.py 2024-11-05 15:10:22.755529 + @@ -5,12 +5,12 @@ import lldb from lldbsuite.test.decorators import * from lldbsuite.test.lldbtest import * from lldbsuite.test import lldbutil + class TestCPPEnumPromotion(TestBase): - @skipIf(debug_info=no_match(["dwarf", "dwo"])) def test(self): self.build() lldbutil.run_to_source_breakpoint( self, "// break here", lldb.SBFileSpec("main.cpp") `` https://github.com/llvm/llvm-project/pull/115005 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [lldb] Analyze enum promotion type during parsing (PR #115005)
github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff b2d2494731976ab7aa9702f3134472db694b9332 62c801145a2312d2c9339d30cf116fc2e709d630 --extensions h,cpp -- lldb/test/API/lang/cpp/enum_promotion/main.cpp clang/include/clang/AST/Decl.h lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp `` View the diff from clang-format here. ``diff diff --git a/lldb/test/API/lang/cpp/enum_promotion/main.cpp b/lldb/test/API/lang/cpp/enum_promotion/main.cpp index fafb96f6c7..bcdb0adff5 100644 --- a/lldb/test/API/lang/cpp/enum_promotion/main.cpp +++ b/lldb/test/API/lang/cpp/enum_promotion/main.cpp @@ -4,7 +4,7 @@ enum EnumUInt { UInt = 0x10001 }; enum EnumSLong { SLong = 0x10001 }; enum EnumULong { ULong = 0xFFF0 }; enum EnumNChar { NChar = -1 }; -enum EnumNShort { NShort= -0x101 }; +enum EnumNShort { NShort = -0x101 }; enum EnumNInt { NInt = -0x10001 }; enum EnumNLong { NLong = -0x10001 }; `` https://github.com/llvm/llvm-project/pull/115005 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [lldb] Analyze enum promotion type during parsing (PR #115005)
https://github.com/kuilpd updated https://github.com/llvm/llvm-project/pull/115005 >From 5290832b802d98b9d293b6910c0837911ec490c4 Mon Sep 17 00:00:00 2001 From: Ilia Kuklin Date: Mon, 4 Nov 2024 14:33:45 +0500 Subject: [PATCH 1/4] [lldb] Analyze enum promotion type during parsing --- clang/include/clang/AST/Decl.h| 2 +- .../SymbolFile/DWARF/DWARFASTParserClang.cpp | 95 ++- .../TypeSystem/Clang/TypeSystemClang.cpp | 29 +- 3 files changed, 100 insertions(+), 26 deletions(-) diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h index 8c39ef3d5a9fa6..a78e6c92abb22b 100644 --- a/clang/include/clang/AST/Decl.h +++ b/clang/include/clang/AST/Decl.h @@ -3891,6 +3891,7 @@ class EnumDecl : public TagDecl { void setInstantiationOfMemberEnum(ASTContext &C, EnumDecl *ED, TemplateSpecializationKind TSK); +public: /// Sets the width in bits required to store all the /// non-negative enumerators of this enum. void setNumPositiveBits(unsigned Num) { @@ -3902,7 +3903,6 @@ class EnumDecl : public TagDecl { /// negative enumerators of this enum. (see getNumNegativeBits) void setNumNegativeBits(unsigned Num) { EnumDeclBits.NumNegativeBits = Num; } -public: /// True if this tag declaration is a scoped enumeration. Only /// possible in C++11 mode. void setScoped(bool Scoped = true) { EnumDeclBits.IsScoped = Scoped; } diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index a30d898a93cc4d..a21607c2020161 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -2248,6 +2248,8 @@ size_t DWARFASTParserClang::ParseChildEnumerators( return 0; size_t enumerators_added = 0; + unsigned NumNegativeBits = 0; + unsigned NumPositiveBits = 0; for (DWARFDIE die : parent_die.children()) { const dw_tag_t tag = die.Tag(); @@ -2299,11 +2301,102 @@ size_t DWARFASTParserClang::ParseChildEnumerators( } if (name && name[0] && got_value) { - m_ast.AddEnumerationValueToEnumerationType( + auto ECD = m_ast.AddEnumerationValueToEnumerationType( clang_type, decl, name, enum_value, enumerator_byte_size * 8); ++enumerators_added; + + llvm::APSInt InitVal = ECD->getInitVal(); + // Keep track of the size of positive and negative values. + if (InitVal.isUnsigned() || InitVal.isNonNegative()) { +// If the enumerator is zero that should still be counted as a positive +// bit since we need a bit to store the value zero. +unsigned ActiveBits = InitVal.getActiveBits(); +NumPositiveBits = std::max({NumPositiveBits, ActiveBits, 1u}); + } else { +NumNegativeBits = +std::max(NumNegativeBits, (unsigned)InitVal.getSignificantBits()); + } } } + + /// The following code follows the same logic as in Sema::ActOnEnumBody + /// clang/lib/Sema/SemaDecl.cpp + // If we have an empty set of enumerators we still need one bit. + // From [dcl.enum]p8 + // If the enumerator-list is empty, the values of the enumeration are as if + // the enumeration had a single enumerator with value 0 + if (!NumPositiveBits && !NumNegativeBits) +NumPositiveBits = 1; + + clang::QualType qual_type(ClangUtil::GetQualType(clang_type)); + clang::EnumDecl *enum_decl = qual_type->getAs()->getDecl(); + enum_decl->setNumPositiveBits(NumPositiveBits); + enum_decl->setNumNegativeBits(NumNegativeBits); + + // C++0x N3000 [conv.prom]p3: + // An rvalue of an unscoped enumeration type whose underlying + // type is not fixed can be converted to an rvalue of the first + // of the following types that can represent all the values of + // the enumeration: int, unsigned int, long int, unsigned long + // int, long long int, or unsigned long long int. + // C99 6.4.4.3p2: + // An identifier declared as an enumeration constant has type int. + // The C99 rule is modified by C23. + clang::QualType BestPromotionType; + unsigned BestWidth; + + auto &Context = m_ast.getASTContext(); + unsigned LongWidth = Context.getTargetInfo().getLongWidth(); + unsigned IntWidth = Context.getTargetInfo().getIntWidth(); + unsigned CharWidth = Context.getTargetInfo().getCharWidth(); + unsigned ShortWidth = Context.getTargetInfo().getShortWidth(); + + bool is_cpp = Language::LanguageIsCPlusPlus( + SymbolFileDWARF::GetLanguage(*parent_die.GetCU())); + + if (NumNegativeBits) { +// If there is a negative value, figure out the smallest integer type (of +// int/long/longlong) that fits. +if (NumNegativeBits <= CharWidth && NumPositiveBits < CharWidth) { + BestWidth = CharWidth; +} else if (NumNegativeBits <= ShortWidth && NumPositiveBits < ShortWidth) { + BestWidth = ShortWidth; +} else if (NumNegativeBits <= IntWidth &
[Lldb-commits] [lldb] [llvm] [lldb][dwarf] Compute fully qualified names on simplified template names with DWARFTypePrinter (PR #112811)
@@ -46,6 +48,7 @@ class DWARFBaseDIE { explicit operator bool() const { return IsValid(); } bool IsValid() const { return m_cu && m_die; } + bool isValid() const { return IsValid(); } labath wrote: How about using `operator bool` as the common api ? https://github.com/llvm/llvm-project/pull/112811 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Index static const members of classes, structs and unions as global variables in DWARF 4 and earlier (PR #111859)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/111859 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 1a68459 - Revert "Fix pointer to reference type (#113596)" (#114831)
Author: Pavel Labath Date: 2024-11-05T16:39:31+01:00 New Revision: 1a684591da84dea644de6524a1d8646b245f636b URL: https://github.com/llvm/llvm-project/commit/1a684591da84dea644de6524a1d8646b245f636b DIFF: https://github.com/llvm/llvm-project/commit/1a684591da84dea644de6524a1d8646b245f636b.diff LOG: Revert "Fix pointer to reference type (#113596)" (#114831) This reverts commit 25909b811a7ddc983d042b15cb54ec271a673d63 due to unresolved questions about the behavior of "frame var" and ValueObject in the presence of references (see the original patch for discussion). Added: Modified: lldb/source/ValueObject/ValueObject.cpp lldb/test/API/lang/cpp/dereferencing_references/TestCPPDereferencingReferences.py lldb/test/API/lang/cpp/dereferencing_references/main.cpp Removed: diff --git a/lldb/source/ValueObject/ValueObject.cpp b/lldb/source/ValueObject/ValueObject.cpp index aca43175d12fac..4006f6e6fd0a5e 100644 --- a/lldb/source/ValueObject/ValueObject.cpp +++ b/lldb/source/ValueObject/ValueObject.cpp @@ -2907,15 +2907,6 @@ ValueObjectSP ValueObject::AddressOf(Status &error) { AddressType address_type = eAddressTypeInvalid; const bool scalar_is_load_address = false; - - // For reference type we need to get the address of the object that - // it refers to. - if (GetCompilerType().IsReferenceType()) { -ValueObjectSP deref_obj = Dereference(error); -if (error.Fail() || !deref_obj) - return ValueObjectSP(); -return deref_obj->AddressOf(error); - } addr_t addr = GetAddressOf(scalar_is_load_address, &address_type); error.Clear(); if (addr != LLDB_INVALID_ADDRESS && address_type != eAddressTypeHost) { diff --git a/lldb/test/API/lang/cpp/dereferencing_references/TestCPPDereferencingReferences.py b/lldb/test/API/lang/cpp/dereferencing_references/TestCPPDereferencingReferences.py index 1374d4e1ec67ab..938fb1a6edf32c 100644 --- a/lldb/test/API/lang/cpp/dereferencing_references/TestCPPDereferencingReferences.py +++ b/lldb/test/API/lang/cpp/dereferencing_references/TestCPPDereferencingReferences.py @@ -25,24 +25,3 @@ def test(self): # Typedef to a reference should dereference to the underlying type. td_val = self.expect_var_path("td_to_ref_type", type="td_int_ref") self.assertEqual(td_val.Dereference().GetType().GetName(), "int") - -def test_take_address_of_reference(self): -"""Tests taking address of lvalue/rvalue references in lldb works correctly.""" -self.build() -lldbutil.run_to_source_breakpoint( -self, "// break here", lldb.SBFileSpec("main.cpp") -) - -plref_val_from_code = self.expect_var_path("pl_ref", type="TTT *") -plref_val_from_expr_path = self.expect_var_path("&l_ref", type="TTT *") -self.assertEqual( -plref_val_from_code.GetValueAsAddress(), -plref_val_from_expr_path.GetValueAsAddress(), -) - -prref_val_from_code = self.expect_var_path("pr_ref", type="TTT *") -prref_val_from_expr_path = self.expect_var_path("&r_ref", type="TTT *") -self.assertEqual( -prref_val_from_code.GetValueAsAddress(), -prref_val_from_expr_path.GetValueAsAddress(), -) diff --git a/lldb/test/API/lang/cpp/dereferencing_references/main.cpp b/lldb/test/API/lang/cpp/dereferencing_references/main.cpp index 4ddffd167ddeed..b64978a9029f81 100644 --- a/lldb/test/API/lang/cpp/dereferencing_references/main.cpp +++ b/lldb/test/API/lang/cpp/dereferencing_references/main.cpp @@ -9,7 +9,5 @@ int main() { // typedef of a reference td_int_ref td_to_ref_type = i; - TTT *pl_ref = &l_ref; - TTT *pr_ref = &r_ref; return l_ref; // break here } ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [lldb][dwarf] Compute fully qualified names on simplified template names with DWARFTypePrinter (PR #112811)
https://github.com/labath commented: I'd say this looks very good. Given this is a relatively novel approach, I'd suggest we give folks a bit more time to take a look at this. https://github.com/llvm/llvm-project/pull/112811 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [lldb] Analyze enum promotion type during parsing (PR #115005)
kuilpd wrote: > Not that this shouldn't be fixed, just weighing of the amount of complexity > added here versus the benefit. I don't really know how useful it is in general to know the actual promotion type of the enum, I guess only for using enum values in expressions without explicit casting. > Also, is this promotion-type something that can be encoded in DWARF? GCC > generates the same DWARF as Clang here. >From what I could tell there is nothing in the DWARF spec that allows to hold >a promotion type of an enum. We could just use some undocumented field, but >then it wouldn't work with GCC binaries. > My question is: if the promotion type can be computed from the information in > dwarf (can it always?), and clang already has code to compute it (not from > DWARF, but from.. clang AST I guess), can we refactor that code somehow so > that it is usable from lldb as well? It can be computed if all the enum values are present in DWARF. I thought about reusing the code from Sema as well, but it uses different interface to iterate through every value, and does other analysis along the way which is not needed during debugging anymore. Plus the patch allows to remove some of the erroneous analysis in [TypeSystemClang.cpp](https://github.com/llvm/llvm-project/pull/115005/files#diff-3a759ae917207e962f79e48fa280277099a587c11bc1f1385e64000c03d6b6a2) https://github.com/llvm/llvm-project/pull/115005 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Index static const members of classes, structs and unions as global variables in DWARF 4 and earlier (PR #111859)
https://github.com/Michael137 approved this pull request. LGTM too, thanks! Given the pre-DWARFv4 spec isn't very specific about how to differentiate static vs non-static members, leaving out the check/assert for `DW_TAG_data_member_location` as Pavel suggested seems correct. https://github.com/llvm/llvm-project/pull/111859 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [lldb] Analyze enum promotion type during parsing (PR #115005)
labath wrote: I'm worried about the same thing as Michael. My question is: if the promotion type can be computed from the information in dwarf (can it always?), and clang already has code to compute it (not from DWARF, but from.. clang AST I guess), can we refactor that code somehow so that it is usable from lldb as well? https://github.com/llvm/llvm-project/pull/115005 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Index static const members of classes, structs and unions as global variables in DWARF 4 and earlier (PR #111859)
@@ -362,6 +369,18 @@ void ManualDWARFIndex::IndexUnitImpl(DWARFUnit &unit, set.namespaces.Insert(ConstString(name), ref); break; +case DW_TAG_member: { + // In DWARF 4 and earlier `static const` members of a struct, a class or a + // union have an entry tag `DW_TAG_member`, and are also tagged as + // `DW_AT_declaration`, but otherwise follow the same rules as + // `DW_TAG_variable`. + bool parent_is_class_type = false; + if (auto parent = die.GetParent()) +parent_is_class_type = DWARFDIE(&unit, parent).IsStructUnionOrClass(); Michael137 wrote: Ah right, yea that's unfortunate https://github.com/llvm/llvm-project/pull/111859 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [lldb] Analyze enum promotion type during parsing (PR #115005)
Michael137 wrote: I haven't done an in-depth review of the patch yet but my first instinct here is that this looks like a lot of work for LLDB which the compiler has already done, so we ideally don't want to repeat. Where is this actually an issue from a user perspective? In the example you gave one could just cast to the appropriate enum type and get the bit-mask style presentation: ``` (lldb) expr (UnscopedEnum) (-enum_one) (UnscopedEnum) $0 = One | 0xfffe ``` Not that this shouldn't be fixed, just weighing of the amount of complexity added here versus the benefit. Also, is this promotion-type something that can be encoded in DWARF? GCC generates the same DWARF as Clang here. What happens if we make the type of the `DW_TAG_variable`s be `UnscopedEnum` instead of `int`? I guess that wouldn't help with ``` (lldb) expr UnscopedEnum::One + 1 ``` https://github.com/llvm/llvm-project/pull/115005 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Revert "Fix pointer to reference type (#113596)" (PR #114831)
https://github.com/labath closed https://github.com/llvm/llvm-project/pull/114831 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix a thinko in the CallSite handling code: (PR #114896)
https://github.com/labath approved this pull request. https://github.com/llvm/llvm-project/pull/114896 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap to support running in a server mode, allowing multiple connections. (PR #114881)
labath wrote: > This is a fairly large refactor of the lldb-dap to allow multiple DAP > sessions from a single lldb-dap server. The obvious question after an opening like this is whether the patch can be split into multiple smaller changes. I would expect that at least the "remove g_dap global" part can be removed from the addition new functionality (listen for multiple connections). https://llvm.org/docs/DeveloperPolicy.html#incremental-development https://github.com/llvm/llvm-project/pull/114881 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Index static const members of classes, structs and unions as global variables in DWARF 4 and earlier (PR #111859)
https://github.com/labath commented: I think this looks good now. Is there anything you'd like to add, Michael? https://github.com/llvm/llvm-project/pull/111859 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Index static const members of classes, structs and unions as global variables in DWARF 4 and earlier (PR #111859)
@@ -362,6 +369,18 @@ void ManualDWARFIndex::IndexUnitImpl(DWARFUnit &unit, set.namespaces.Insert(ConstString(name), ref); break; +case DW_TAG_member: { + // In DWARF 4 and earlier `static const` members of a struct, a class or a + // union have an entry tag `DW_TAG_member`, and are also tagged as + // `DW_AT_declaration`, but otherwise follow the same rules as + // `DW_TAG_variable`. + bool parent_is_class_type = false; + if (auto parent = die.GetParent()) +parent_is_class_type = DWARFDIE(&unit, parent).IsStructUnionOrClass(); labath wrote: No, because `parent` is a DWARFDebugInfoEntry, not a DWARFDIE :P It's a (not entirely fortunate) result of trying to avoid any high level (expensive) operations, and of the fact that lldb's DWARFDebugInfoEntry (unlike the llvm) fork actually provides some functionality instead of being a simple data holder. https://github.com/llvm/llvm-project/pull/111859 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Index static const members of classes, structs and unions as global variables in DWARF 4 and earlier (PR #111859)
https://github.com/labath edited https://github.com/llvm/llvm-project/pull/111859 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [lldb-dap] Support column breakpoints (PR #113787)
@@ -910,6 +911,183 @@ void request_attach(const llvm::json::Object &request) { } } +// "BreakpointLocationsRequest": { +// "allOf": [ { "$ref": "#/definitions/Request" }, { +// "type": "object", +// "description": "The `breakpointLocations` request returns all possible +// locations for source breakpoints in a given range.\nClients should only +// call this request if the corresponding capability +// `supportsBreakpointLocationsRequest` is true.", +// "properties": { +// "command": { +// "type": "string", +// "enum": [ "breakpointLocations" ] +// }, +// "arguments": { +// "$ref": "#/definitions/BreakpointLocationsArguments" +// } +// }, +// "required": [ "command" ] +// }] +// }, +// "BreakpointLocationsArguments": { +// "type": "object", +// "description": "Arguments for `breakpointLocations` request.", +// "properties": { +// "source": { +// "$ref": "#/definitions/Source", +// "description": "The source location of the breakpoints; either +// `source.path` or `source.sourceReference` must be specified." +// }, +// "line": { +// "type": "integer", +// "description": "Start line of range to search possible breakpoint +// locations in. If only the line is specified, the request returns all +// possible locations in that line." +// }, +// "column": { +// "type": "integer", +// "description": "Start position within `line` to search possible +// breakpoint locations in. It is measured in UTF-16 code units and the +// client capability `columnsStartAt1` determines whether it is 0- or +// 1-based. If no column is given, the first position in the start line is +// assumed." +// }, +// "endLine": { +// "type": "integer", +// "description": "End line of range to search possible breakpoint +// locations in. If no end line is given, then the end line is assumed to +// be the start line." +// }, +// "endColumn": { +// "type": "integer", +// "description": "End position within `endLine` to search possible +// breakpoint locations in. It is measured in UTF-16 code units and the +// client capability `columnsStartAt1` determines whether it is 0- or +// 1-based. If no end column is given, the last position in the end line +// is assumed." +// } +// }, +// "required": [ "source", "line" ] +// }, +// "BreakpointLocationsResponse": { +// "allOf": [ { "$ref": "#/definitions/Response" }, { +// "type": "object", +// "description": "Response to `breakpointLocations` request.\nContains +// possible locations for source breakpoints.", +// "properties": { +// "body": { +// "type": "object", +// "properties": { +// "breakpoints": { +// "type": "array", +// "items": { +// "$ref": "#/definitions/BreakpointLocation" +// }, +// "description": "Sorted set of possible breakpoint locations." +// } +// }, +// "required": [ "breakpoints" ] +// } +// }, +// "required": [ "body" ] +// }] +// }, +// "BreakpointLocation": { +// "type": "object", +// "description": "Properties of a breakpoint location returned from the +// `breakpointLocations` request.", +// "properties": { +// "line": { +// "type": "integer", +// "description": "Start line of breakpoint location." +// }, +// "column": { +// "type": "integer", +// "description": "The start position of a breakpoint location. Position +// is measured in UTF-16 code units and the client capability +// `columnsStartAt1` determines whether it is 0- or 1-based." +// }, +// "endLine": { +// "type": "integer", +// "description": "The end line of breakpoint location if the location +// covers a range." +// }, +// "endColumn": { +// "type": "integer", +// "description": "The end position of a breakpoint location (if the +// location covers a range). Position is measured in UTF-16 code units and +// the client capability `columnsStartAt1` determines whether it is 0- or +// 1-based." +// } +// }, +// "required": [ "line" ] +// }, +void request_breakpointLocations(const llvm::json::Object &request) { + llvm::json::Object response; + FillResponse(request, response); + auto *arguments = request.getObject("arguments"); + auto *source = arguments->getObject("source"); + std::string path = GetString(source, "path").str(); + uint64_t start_line = GetUnsigned(arguments, "line", 0); + uint64_t start_column = GetUnsigned(arguments, "column", 0); + uint64_t end_line = GetUnsigned(arguments, "endLine", start_line); + uint64_t end_column = + GetUnsigned(arguments, "endColumn", std::numeric_limits::max()); + + lldb::SBFileSpec file_spec
[Lldb-commits] [lldb] 6620cd2 - [LLDB] Add a target.launch-working-dir setting (#113521)
Author: Walter Erquinigo Date: 2024-11-05T06:33:25-05:00 New Revision: 6620cd25234a42ca4b51490627afcb93fa443dc3 URL: https://github.com/llvm/llvm-project/commit/6620cd25234a42ca4b51490627afcb93fa443dc3 DIFF: https://github.com/llvm/llvm-project/commit/6620cd25234a42ca4b51490627afcb93fa443dc3.diff LOG: [LLDB] Add a target.launch-working-dir setting (#113521) Internally we use bazel in a way in which it can drop you in a LLDB session with the target launched in a particular cwd, which is needed for things to work. We've been making this automation work via `process launch -w`. However, if later the user wants to restart the process with `r`, then they end up using a different cwd for relaunching the process. As a way to fix this, I'm adding a target-level setting that allows configuring a default cwd used for launching the process without needing the user to specify it manually. Added: Modified: lldb/include/lldb/Target/Target.h lldb/source/Commands/CommandObjectProcess.cpp lldb/source/Commands/Options.td lldb/source/Target/Target.cpp lldb/source/Target/TargetProperties.td lldb/test/API/commands/process/launch/TestProcessLaunch.py llvm/docs/ReleaseNotes.md Removed: diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index e4848f19e64d62..cab21c29a7486f 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -37,6 +37,7 @@ #include "lldb/Utility/RealpathPrefixes.h" #include "lldb/Utility/Timeout.h" #include "lldb/lldb-public.h" +#include "llvm/ADT/StringRef.h" namespace lldb_private { @@ -114,6 +115,8 @@ class TargetProperties : public Properties { void SetDisableSTDIO(bool b); + llvm::StringRef GetLaunchWorkingDirectory() const; + const char *GetDisassemblyFlavor() const; InlineStrategy GetInlineStrategy() const; diff --git a/lldb/source/Commands/CommandObjectProcess.cpp b/lldb/source/Commands/CommandObjectProcess.cpp index e7c7d07ad47722..7444e46aa729e7 100644 --- a/lldb/source/Commands/CommandObjectProcess.cpp +++ b/lldb/source/Commands/CommandObjectProcess.cpp @@ -201,6 +201,13 @@ class CommandObjectProcessLaunch : public CommandObjectProcessLaunchOrAttach { if (target->GetDisableSTDIO()) m_options.launch_info.GetFlags().Set(eLaunchFlagDisableSTDIO); +if (!m_options.launch_info.GetWorkingDirectory()) { + if (llvm::StringRef wd = target->GetLaunchWorkingDirectory(); + !wd.empty()) { +m_options.launch_info.SetWorkingDirectory(FileSpec(wd)); + } +} + // Merge the launch info environment with the target environment. Environment target_env = target->GetEnvironment(); m_options.launch_info.GetEnvironment().insert(target_env.begin(), diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td index 4276d9e7f9c8b0..9d8d45d083eca4 100644 --- a/lldb/source/Commands/Options.td +++ b/lldb/source/Commands/Options.td @@ -691,7 +691,10 @@ let Command = "process launch" in { def process_launch_plugin : Option<"plugin", "P">, Arg<"Plugin">, Desc<"Name of the process plugin you want to use.">; def process_launch_working_dir : Option<"working-dir", "w">, Arg<"DirectoryName">, -Desc<"Set the current working directory to when running the inferior.">; +Desc<"Set the current working directory to when running the inferior. This option " + "applies only to the current `process launch` invocation. If " + "`target.launch-working-dir` is set and this option is given, the value of this " + "option will be used instead of the setting.">; def process_launch_arch : Option<"arch", "a">, Arg<"Architecture">, Desc<"Set the architecture for the process to launch when ambiguous.">; def process_launch_environment : Option<"environment", "E">, diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index 8cd3fa8af6bae1..242d2eaec2a15a 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -4471,6 +4471,11 @@ void TargetProperties::SetDisableSTDIO(bool b) { const uint32_t idx = ePropertyDisableSTDIO; SetPropertyAtIndex(idx, b); } +llvm::StringRef TargetProperties::GetLaunchWorkingDirectory() const { + const uint32_t idx = ePropertyLaunchWorkingDir; + return GetPropertyAtIndexAs( + idx, g_target_properties[idx].default_cstr_value); +} const char *TargetProperties::GetDisassemblyFlavor() const { const uint32_t idx = ePropertyDisassemblyFlavor; diff --git a/lldb/source/Target/TargetProperties.td b/lldb/source/Target/TargetProperties.td index fb61478fb752dc..00ad8dd2a9f7f9 100644 --- a/lldb/source/Target/TargetProperties.td +++ b/lldb/source/Target/TargetProperties.td @@ -201,6 +201,13 @@ let Definition = "target" in { def DebugUtilityExpression: Property<"debug-utility-expression", "Boolean">, DefaultFal
[Lldb-commits] [lldb] [lldb][breakpoint] Grey out disabled breakpoints (PR #91404)
@@ -0,0 +1,32 @@ +""" +Test that disabling breakpoints and viewing them in a list uses the correct ANSI color settings when colors are enabled and disabled. +""" +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil +from lldbsuite.test.lldbpexpect import PExpectTest + +import re +import io + + +class DisabledBreakpointsTest(PExpectTest): +@add_test_categories(["pexpect"]) +def test_disabling_breakpoints_with_color(self): +"""Test that disabling a breakpoint and viewing the breakpoints list uses the specified ANSI color prefix.""" +import pexpect + +self.child = pexpect.spawn("expect", encoding="utf-8") + +ansi_red_color_code = "\x1b[31m" + +self.launch(use_colors=True, dimensions=(100, 100)) +self.child.sendline( labath wrote: Yes, that's what I meant. The `expect` function was meant to be used to inject lldb commands, some tests use the pexpect API directly because they're doing something more complicated (or because the thing they're driving is not lldb), but that's not the case here. https://github.com/llvm/llvm-project/pull/91404 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][breakpoint] Grey out disabled breakpoints (PR #91404)
@@ -0,0 +1,26 @@ +""" +Test that disabling breakpoints and viewing them in a list uses the correct ANSI color settings when colors are enabled and disabled. +""" + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil +from lldbsuite.test.lldbpexpect import PExpectTest + +import io + + +class DisabledBreakpointsTest(PExpectTest): +@add_test_categories(["pexpect"]) +def test_disabling_breakpoints_with_color(self): +"""Test that disabling a breakpoint and viewing the breakpoints list uses the specified ANSI color prefix.""" +ansi_red_color_code = "\x1b[31m" + +self.launch(use_colors=True, dimensions=(100, 100)) +self.expect('settings set disable-breakpoint-ansi-prefix "${ansi.fg.red}"') +self.expect("b main") +self.expect("br dis") +self.expect("br l") +self.child.expect_exact(ansi_red_color_code + "1:") labath wrote: ```suggestion self.expect("br l", substrs=[ansi_red_color_code + "1:"]) ``` The idea is to make this mostly look like the existing SB API tests. Kinda surprised it works without it. If I had to guess, I'd say its because of the prompt coloring code causes the prompt substring to be present more than once (which then means the substring search loses synch). (That's one more reason to add some sort of expectations to the previous commands. The other reason is that they give you an earlier indication if something goes wrong during the test -- the pexpect error messages aren't of the most readable type) https://github.com/llvm/llvm-project/pull/91404 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][breakpoint] Grey out disabled breakpoints (PR #91404)
@@ -0,0 +1,32 @@ +""" +Test that disabling breakpoints and viewing them in a list uses the correct ANSI color settings when colors are enabled and disabled. +""" +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil +from lldbsuite.test.lldbpexpect import PExpectTest + +import re +import io + + +class DisabledBreakpointsTest(PExpectTest): +@add_test_categories(["pexpect"]) +def test_disabling_breakpoints_with_color(self): +"""Test that disabling a breakpoint and viewing the breakpoints list uses the specified ANSI color prefix.""" +import pexpect + +self.child = pexpect.spawn("expect", encoding="utf-8") + labath wrote: I'm pretty sure this was doing nothing as `self.launch` overwrites the `child` member. I recall we have one test which uses `pexpect` (the python module) to drive `expect` (the unix utility which lent its name to the former) to drive `lldb`. We definitely don't need that here (and tbh, I'm not sure the original test does either). https://github.com/llvm/llvm-project/pull/91404 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][breakpoint] Grey out disabled breakpoints (PR #91404)
https://github.com/labath commented: I'm kinda late to the party (particularly as there's an analogous "progress" setting), but I have to say I was surprised to see a `disable-breakpoint-ansi-suffix` setting. That sounds like too low level of a model. The model I would expect/find more natural is if there was a single setting describing the "normal" text colour, and then several (not necessarily one each item that needs colouring) settings defining the colour of items that need colouring. I find it hard to imagine why one would ever want to set different values for the "suffix" colours, as that means the colour of random text would depend on what was printed before it. I don't want to make this a blocking objection, but I would urge you to consider if this is the behavior you want. https://github.com/llvm/llvm-project/pull/91404 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][breakpoint] Grey out disabled breakpoints (PR #91404)
https://github.com/labath edited https://github.com/llvm/llvm-project/pull/91404 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [lldb] Analyze enum promotion type during parsing (PR #115005)
https://github.com/kuilpd updated https://github.com/llvm/llvm-project/pull/115005 >From 5290832b802d98b9d293b6910c0837911ec490c4 Mon Sep 17 00:00:00 2001 From: Ilia Kuklin Date: Mon, 4 Nov 2024 14:33:45 +0500 Subject: [PATCH 1/4] [lldb] Analyze enum promotion type during parsing --- clang/include/clang/AST/Decl.h| 2 +- .../SymbolFile/DWARF/DWARFASTParserClang.cpp | 95 ++- .../TypeSystem/Clang/TypeSystemClang.cpp | 29 +- 3 files changed, 100 insertions(+), 26 deletions(-) diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h index 8c39ef3d5a9fa6..a78e6c92abb22b 100644 --- a/clang/include/clang/AST/Decl.h +++ b/clang/include/clang/AST/Decl.h @@ -3891,6 +3891,7 @@ class EnumDecl : public TagDecl { void setInstantiationOfMemberEnum(ASTContext &C, EnumDecl *ED, TemplateSpecializationKind TSK); +public: /// Sets the width in bits required to store all the /// non-negative enumerators of this enum. void setNumPositiveBits(unsigned Num) { @@ -3902,7 +3903,6 @@ class EnumDecl : public TagDecl { /// negative enumerators of this enum. (see getNumNegativeBits) void setNumNegativeBits(unsigned Num) { EnumDeclBits.NumNegativeBits = Num; } -public: /// True if this tag declaration is a scoped enumeration. Only /// possible in C++11 mode. void setScoped(bool Scoped = true) { EnumDeclBits.IsScoped = Scoped; } diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index a30d898a93cc4d..a21607c2020161 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -2248,6 +2248,8 @@ size_t DWARFASTParserClang::ParseChildEnumerators( return 0; size_t enumerators_added = 0; + unsigned NumNegativeBits = 0; + unsigned NumPositiveBits = 0; for (DWARFDIE die : parent_die.children()) { const dw_tag_t tag = die.Tag(); @@ -2299,11 +2301,102 @@ size_t DWARFASTParserClang::ParseChildEnumerators( } if (name && name[0] && got_value) { - m_ast.AddEnumerationValueToEnumerationType( + auto ECD = m_ast.AddEnumerationValueToEnumerationType( clang_type, decl, name, enum_value, enumerator_byte_size * 8); ++enumerators_added; + + llvm::APSInt InitVal = ECD->getInitVal(); + // Keep track of the size of positive and negative values. + if (InitVal.isUnsigned() || InitVal.isNonNegative()) { +// If the enumerator is zero that should still be counted as a positive +// bit since we need a bit to store the value zero. +unsigned ActiveBits = InitVal.getActiveBits(); +NumPositiveBits = std::max({NumPositiveBits, ActiveBits, 1u}); + } else { +NumNegativeBits = +std::max(NumNegativeBits, (unsigned)InitVal.getSignificantBits()); + } } } + + /// The following code follows the same logic as in Sema::ActOnEnumBody + /// clang/lib/Sema/SemaDecl.cpp + // If we have an empty set of enumerators we still need one bit. + // From [dcl.enum]p8 + // If the enumerator-list is empty, the values of the enumeration are as if + // the enumeration had a single enumerator with value 0 + if (!NumPositiveBits && !NumNegativeBits) +NumPositiveBits = 1; + + clang::QualType qual_type(ClangUtil::GetQualType(clang_type)); + clang::EnumDecl *enum_decl = qual_type->getAs()->getDecl(); + enum_decl->setNumPositiveBits(NumPositiveBits); + enum_decl->setNumNegativeBits(NumNegativeBits); + + // C++0x N3000 [conv.prom]p3: + // An rvalue of an unscoped enumeration type whose underlying + // type is not fixed can be converted to an rvalue of the first + // of the following types that can represent all the values of + // the enumeration: int, unsigned int, long int, unsigned long + // int, long long int, or unsigned long long int. + // C99 6.4.4.3p2: + // An identifier declared as an enumeration constant has type int. + // The C99 rule is modified by C23. + clang::QualType BestPromotionType; + unsigned BestWidth; + + auto &Context = m_ast.getASTContext(); + unsigned LongWidth = Context.getTargetInfo().getLongWidth(); + unsigned IntWidth = Context.getTargetInfo().getIntWidth(); + unsigned CharWidth = Context.getTargetInfo().getCharWidth(); + unsigned ShortWidth = Context.getTargetInfo().getShortWidth(); + + bool is_cpp = Language::LanguageIsCPlusPlus( + SymbolFileDWARF::GetLanguage(*parent_die.GetCU())); + + if (NumNegativeBits) { +// If there is a negative value, figure out the smallest integer type (of +// int/long/longlong) that fits. +if (NumNegativeBits <= CharWidth && NumPositiveBits < CharWidth) { + BestWidth = CharWidth; +} else if (NumNegativeBits <= ShortWidth && NumPositiveBits < ShortWidth) { + BestWidth = ShortWidth; +} else if (NumNegativeBits <= IntWidth &
[Lldb-commits] [lldb] [llvm] [lldb][dwarf] Compute fully qualified names on simplified template names with DWARFTypePrinter (PR #112811)
https://github.com/labath edited https://github.com/llvm/llvm-project/pull/112811 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] More refinement of call site handling in stepping. (PR #114628)
DavidSpickett wrote: Intentionally or not, this also fixes TestInlineStepping.py `test_with_python_api` on Arm 32 bit. https://github.com/llvm/llvm-project/pull/114628 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] e952728 - [LLDB] Retry Add a target.launch-working-dir setting
Author: walter erquinigo Date: 2024-11-05T13:29:51-05:00 New Revision: e952728f88c8b0e0208dc991dd9a04fe8c211cfb URL: https://github.com/llvm/llvm-project/commit/e952728f88c8b0e0208dc991dd9a04fe8c211cfb DIFF: https://github.com/llvm/llvm-project/commit/e952728f88c8b0e0208dc991dd9a04fe8c211cfb.diff LOG: [LLDB] Retry Add a target.launch-working-dir setting This retries the PR 113521 skipping a test in a remote environment. Added: Modified: lldb/include/lldb/Target/Target.h lldb/source/Commands/CommandObjectProcess.cpp lldb/source/Commands/Options.td lldb/source/Target/Target.cpp lldb/source/Target/TargetProperties.td lldb/test/API/commands/process/launch/TestProcessLaunch.py llvm/docs/ReleaseNotes.md Removed: diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index e4848f19e64d62..cab21c29a7486f 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -37,6 +37,7 @@ #include "lldb/Utility/RealpathPrefixes.h" #include "lldb/Utility/Timeout.h" #include "lldb/lldb-public.h" +#include "llvm/ADT/StringRef.h" namespace lldb_private { @@ -114,6 +115,8 @@ class TargetProperties : public Properties { void SetDisableSTDIO(bool b); + llvm::StringRef GetLaunchWorkingDirectory() const; + const char *GetDisassemblyFlavor() const; InlineStrategy GetInlineStrategy() const; diff --git a/lldb/source/Commands/CommandObjectProcess.cpp b/lldb/source/Commands/CommandObjectProcess.cpp index e7c7d07ad47722..7444e46aa729e7 100644 --- a/lldb/source/Commands/CommandObjectProcess.cpp +++ b/lldb/source/Commands/CommandObjectProcess.cpp @@ -201,6 +201,13 @@ class CommandObjectProcessLaunch : public CommandObjectProcessLaunchOrAttach { if (target->GetDisableSTDIO()) m_options.launch_info.GetFlags().Set(eLaunchFlagDisableSTDIO); +if (!m_options.launch_info.GetWorkingDirectory()) { + if (llvm::StringRef wd = target->GetLaunchWorkingDirectory(); + !wd.empty()) { +m_options.launch_info.SetWorkingDirectory(FileSpec(wd)); + } +} + // Merge the launch info environment with the target environment. Environment target_env = target->GetEnvironment(); m_options.launch_info.GetEnvironment().insert(target_env.begin(), diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td index 4276d9e7f9c8b0..9d8d45d083eca4 100644 --- a/lldb/source/Commands/Options.td +++ b/lldb/source/Commands/Options.td @@ -691,7 +691,10 @@ let Command = "process launch" in { def process_launch_plugin : Option<"plugin", "P">, Arg<"Plugin">, Desc<"Name of the process plugin you want to use.">; def process_launch_working_dir : Option<"working-dir", "w">, Arg<"DirectoryName">, -Desc<"Set the current working directory to when running the inferior.">; +Desc<"Set the current working directory to when running the inferior. This option " + "applies only to the current `process launch` invocation. If " + "`target.launch-working-dir` is set and this option is given, the value of this " + "option will be used instead of the setting.">; def process_launch_arch : Option<"arch", "a">, Arg<"Architecture">, Desc<"Set the architecture for the process to launch when ambiguous.">; def process_launch_environment : Option<"environment", "E">, diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index 8cd3fa8af6bae1..242d2eaec2a15a 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -4471,6 +4471,11 @@ void TargetProperties::SetDisableSTDIO(bool b) { const uint32_t idx = ePropertyDisableSTDIO; SetPropertyAtIndex(idx, b); } +llvm::StringRef TargetProperties::GetLaunchWorkingDirectory() const { + const uint32_t idx = ePropertyLaunchWorkingDir; + return GetPropertyAtIndexAs( + idx, g_target_properties[idx].default_cstr_value); +} const char *TargetProperties::GetDisassemblyFlavor() const { const uint32_t idx = ePropertyDisassemblyFlavor; diff --git a/lldb/source/Target/TargetProperties.td b/lldb/source/Target/TargetProperties.td index fb61478fb752dc..00ad8dd2a9f7f9 100644 --- a/lldb/source/Target/TargetProperties.td +++ b/lldb/source/Target/TargetProperties.td @@ -201,6 +201,13 @@ let Definition = "target" in { def DebugUtilityExpression: Property<"debug-utility-expression", "Boolean">, DefaultFalse, Desc<"Enable debugging of LLDB-internal utility expressions.">; + def LaunchWorkingDir: Property<"launch-working-dir", "String">, +DefaultStringValue<"">, +Desc<"A default value for the working directory to use when launching processes. " + "It is ignored when empty. This setting is only used when the target is " + "launched. If you change this setting, the new value will only apply to " + "subsequent launches. Comman
[Lldb-commits] [lldb] More refinement of call site handling in stepping. (PR #114628)
https://github.com/jimingham updated https://github.com/llvm/llvm-project/pull/114628 >From 042ac07ed67a5465aaf5c2dc8c4396adf5da2948 Mon Sep 17 00:00:00 2001 From: Jim Ingham Date: Fri, 1 Nov 2024 17:23:12 -0700 Subject: [PATCH 1/5] More refinement of call site handling in stepping. When you set a "next branch breakpoint" and run to it while stepping, you have to claim the stop at that breakpoint to be the top of the inlined call stack, or you will seem to "step in" and then plans might try to step back out again. This records the PrefferedLineEntry for next branch breakpoints and adds a test to make sure this works. --- lldb/source/Target/ThreadPlanStepRange.cpp| 42 +-- .../inline-stepping/TestInlineStepping.py | 20 - 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/lldb/source/Target/ThreadPlanStepRange.cpp b/lldb/source/Target/ThreadPlanStepRange.cpp index 3c825058b8c375..e53b189d49be44 100644 --- a/lldb/source/Target/ThreadPlanStepRange.cpp +++ b/lldb/source/Target/ThreadPlanStepRange.cpp @@ -379,10 +379,9 @@ bool ThreadPlanStepRange::SetNextBranchBreakpoint() { !m_next_branch_bp_sp->HasResolvedLocations()) m_could_not_resolve_hw_bp = true; +BreakpointLocationSP bp_loc = m_next_branch_bp_sp->GetLocationAtIndex(0); if (log) { lldb::break_id_t bp_site_id = LLDB_INVALID_BREAK_ID; - BreakpointLocationSP bp_loc = - m_next_branch_bp_sp->GetLocationAtIndex(0); if (bp_loc) { BreakpointSiteSP bp_site = bp_loc->GetBreakpointSite(); if (bp_site) { @@ -395,7 +394,44 @@ bool ThreadPlanStepRange::SetNextBranchBreakpoint() { m_next_branch_bp_sp->GetID(), bp_site_id, run_to_address.GetLoadAddress(&m_process.GetTarget())); } - +// The "next branch breakpoint might land on an virtual inlined call +// stack. If that's true, we should always stop at the top of the +// inlined call stack. Only virtual steps should walk deeper into the +// inlined call stack. +Block *block = run_to_address.CalculateSymbolContextBlock(); +if (bp_loc && block) { + LineEntry top_most_line_entry; + lldb::addr_t run_to_addr = run_to_address.GetFileAddress(); + for (Block *inlined_parent = block->GetContainingInlinedBlock(); inlined_parent; + inlined_parent = inlined_parent->GetInlinedParent()) { +AddressRange range; +if (!inlined_parent->GetRangeContainingAddress(run_to_address, range)) + break; +Address range_start_address = range.GetBaseAddress(); +// Only compare addresses here, we may have different symbol +// contexts (for virtual inlined stacks), but we just want to know +// that they are all at the same address. +if (range_start_address.GetFileAddress() != run_to_addr) + break; +const InlineFunctionInfo *inline_info = inlined_parent->GetInlinedFunctionInfo(); +if (!inline_info) + break; +const Declaration &call_site = inline_info->GetCallSite(); +top_most_line_entry.line = call_site.GetLine(); +top_most_line_entry.column = call_site.GetColumn(); +FileSpec call_site_file_spec = call_site.GetFile(); +top_most_line_entry.original_file_sp.reset(new SupportFile(call_site_file_spec)); +top_most_line_entry.range = range; +top_most_line_entry.file_sp.reset(); + top_most_line_entry.ApplyFileMappings(GetThread().CalculateTarget()); +if (!top_most_line_entry.file_sp) + top_most_line_entry.file_sp = top_most_line_entry.original_file_sp; + } + if (top_most_line_entry.IsValid()) { +LLDB_LOG(log, "Setting preferred line entry: {0}:{1}", top_most_line_entry.GetFile(), top_most_line_entry.line); +bp_loc->SetPreferredLineEntry(top_most_line_entry); + } +} m_next_branch_bp_sp->SetThreadID(m_tid); m_next_branch_bp_sp->SetBreakpointKind("next-branch-location"); diff --git a/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py b/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py index 4e2d908e63b81c..b43bc71243f259 100644 --- a/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py +++ b/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py @@ -291,7 +291,7 @@ def inline_stepping_step_over(self): break_1_in_main = target.BreakpointCreateBySourceRegex( "// At second call of caller_ref_1 in main.", self.main_source_spec ) -self.assertTrue(break_1_in_main, VALID_BREAKPOINT) +self.assertGreater(break_1_in_main.GetNumLocations(), 0, VALID_BREAKPOINT) # Now launch the process, and do not stop a
[Lldb-commits] [lldb] More refinement of call site handling in stepping. (PR #114628)
https://github.com/jimingham closed https://github.com/llvm/llvm-project/pull/114628 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 23a01a4 - More refinement of call site handling in stepping. (#114628)
Author: jimingham Date: 2024-11-05T10:33:24-08:00 New Revision: 23a01a413d29f2d5b1f6204d0237e3884ae0231e URL: https://github.com/llvm/llvm-project/commit/23a01a413d29f2d5b1f6204d0237e3884ae0231e DIFF: https://github.com/llvm/llvm-project/commit/23a01a413d29f2d5b1f6204d0237e3884ae0231e.diff LOG: More refinement of call site handling in stepping. (#114628) When you set a "next branch breakpoint" and run to it while stepping, you have to claim the stop at that breakpoint to be the top of the inlined call stack, or you will seem to "step in" and then plans might try to step back out again. This records the PrefferedLineEntry for next branch breakpoints and adds a test to make sure this works. Added: Modified: lldb/source/Target/ThreadPlanStepRange.cpp lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py Removed: diff --git a/lldb/source/Target/ThreadPlanStepRange.cpp b/lldb/source/Target/ThreadPlanStepRange.cpp index 3c825058b8c375..b64ce6f0dc9c28 100644 --- a/lldb/source/Target/ThreadPlanStepRange.cpp +++ b/lldb/source/Target/ThreadPlanStepRange.cpp @@ -379,10 +379,10 @@ bool ThreadPlanStepRange::SetNextBranchBreakpoint() { !m_next_branch_bp_sp->HasResolvedLocations()) m_could_not_resolve_hw_bp = true; +BreakpointLocationSP bp_loc = +m_next_branch_bp_sp->GetLocationAtIndex(0); if (log) { lldb::break_id_t bp_site_id = LLDB_INVALID_BREAK_ID; - BreakpointLocationSP bp_loc = - m_next_branch_bp_sp->GetLocationAtIndex(0); if (bp_loc) { BreakpointSiteSP bp_site = bp_loc->GetBreakpointSite(); if (bp_site) { @@ -395,7 +395,51 @@ bool ThreadPlanStepRange::SetNextBranchBreakpoint() { m_next_branch_bp_sp->GetID(), bp_site_id, run_to_address.GetLoadAddress(&m_process.GetTarget())); } - +// The "next branch breakpoint might land on a virtual inlined call +// stack. If that's true, we should always stop at the top of the +// inlined call stack. Only virtual steps should walk deeper into the +// inlined call stack. +Block *block = run_to_address.CalculateSymbolContextBlock(); +if (bp_loc && block) { + LineEntry top_most_line_entry; + lldb::addr_t run_to_addr = run_to_address.GetFileAddress(); + for (Block *inlined_parent = block->GetContainingInlinedBlock(); + inlined_parent; + inlined_parent = inlined_parent->GetInlinedParent()) { +AddressRange range; +if (!inlined_parent->GetRangeContainingAddress(run_to_address, + range)) + break; +Address range_start_address = range.GetBaseAddress(); +// Only compare addresses here, we may have diff erent symbol +// contexts (for virtual inlined stacks), but we just want to know +// that they are all at the same address. +if (range_start_address.GetFileAddress() != run_to_addr) + break; +const InlineFunctionInfo *inline_info = +inlined_parent->GetInlinedFunctionInfo(); +if (!inline_info) + break; +const Declaration &call_site = inline_info->GetCallSite(); +top_most_line_entry.line = call_site.GetLine(); +top_most_line_entry.column = call_site.GetColumn(); +FileSpec call_site_file_spec = call_site.GetFile(); +top_most_line_entry.original_file_sp.reset( +new SupportFile(call_site_file_spec)); +top_most_line_entry.range = range; +top_most_line_entry.file_sp.reset(); +top_most_line_entry.ApplyFileMappings( +GetThread().CalculateTarget()); +if (!top_most_line_entry.file_sp) + top_most_line_entry.file_sp = + top_most_line_entry.original_file_sp; + } + if (top_most_line_entry.IsValid()) { +LLDB_LOG(log, "Setting preferred line entry: {0}:{1}", + top_most_line_entry.GetFile(), top_most_line_entry.line); +bp_loc->SetPreferredLineEntry(top_most_line_entry); + } +} m_next_branch_bp_sp->SetThreadID(m_tid); m_next_branch_bp_sp->SetBreakpointKind("next-branch-location"); diff --git a/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py b/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py index 4e2d908e63b81c..bb5606260e15f7 100644 --- a/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py +++ b/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py @@ -13,7 +13,6 @@ class TestInlineStepping(TestBase): compiler="icc", bugnumber="# Not r
[Lldb-commits] [lldb] More refinement of call site handling in stepping. (PR #114628)
jimingham wrote: > Intentionally or not, this also fixes TestInlineStepping.py > `test_with_python_api` on Arm 32 bit. That's plausible. I removed the skip along with this patch, let's see if that holds on the bots. https://github.com/llvm/llvm-project/pull/114628 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix a thinko in the CallSite handling code: (PR #114896)
https://github.com/jimingham closed https://github.com/llvm/llvm-project/pull/114896 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 803f957 - Fix a thinko in the CallSite handling code: (#114896)
Author: jimingham Date: 2024-11-05T11:23:23-08:00 New Revision: 803f957e87e4083f6d61c8991171eeeaf0e6bd61 URL: https://github.com/llvm/llvm-project/commit/803f957e87e4083f6d61c8991171eeeaf0e6bd61 DIFF: https://github.com/llvm/llvm-project/commit/803f957e87e4083f6d61c8991171eeeaf0e6bd61.diff LOG: Fix a thinko in the CallSite handling code: (#114896) I have to check for the sc list size being changed by the call-site search, not just that it had more than one element. Added a test for multiple CU's with the same name in a given module, which would have caught this mistake. We were also doing all the work to find call sites when the found decl and specified decl's only difference was a column, but the incoming specification hadn't specified a column (column number == 0). Added: lldb/test/API/functionalities/breakpoint/same_cu_name/Makefile lldb/test/API/functionalities/breakpoint/same_cu_name/TestFileBreakpoinsSameCUName.py lldb/test/API/functionalities/breakpoint/same_cu_name/common.cpp lldb/test/API/functionalities/breakpoint/same_cu_name/main.cpp Modified: lldb/source/Symbol/CompileUnit.cpp Removed: diff --git a/lldb/source/Symbol/CompileUnit.cpp b/lldb/source/Symbol/CompileUnit.cpp index 73389b2e8479b3..166f111ef62207 100644 --- a/lldb/source/Symbol/CompileUnit.cpp +++ b/lldb/source/Symbol/CompileUnit.cpp @@ -328,14 +328,17 @@ void CompileUnit::ResolveSymbolContext( // We will miss functions that ONLY exist as a call site entry. if (line_entry.IsValid() && - (line_entry.line != line || line_entry.column != column_num) && - resolve_scope & eSymbolContextLineEntry && check_inlines) { + (line_entry.line != line || + (column_num != 0 && line_entry.column != column_num)) && + (resolve_scope & eSymbolContextLineEntry) && check_inlines) { // We don't move lines over function boundaries, so the address in the // line entry will be the in function that contained the line that might // be a CallSite, and we can just iterate over that function to find any // inline records, and dig up their call sites. Address start_addr = line_entry.range.GetBaseAddress(); Function *function = start_addr.CalculateSymbolContextFunction(); +// Record the size of the list to see if we added to it: +size_t old_sc_list_size = sc_list.GetSize(); Declaration sought_decl(file_spec, line, column_num); // We use this recursive function to descend the block structure looking @@ -417,7 +420,7 @@ void CompileUnit::ResolveSymbolContext( // FIXME: Should I also do this for "call site line exists between the // given line number and the later line we found in the line table"? That's // a closer approximation to our general sliding algorithm. -if (sc_list.GetSize()) +if (sc_list.GetSize() > old_sc_list_size) return; } diff --git a/lldb/test/API/functionalities/breakpoint/same_cu_name/Makefile b/lldb/test/API/functionalities/breakpoint/same_cu_name/Makefile new file mode 100644 index 00..4bfdb15e777d99 --- /dev/null +++ b/lldb/test/API/functionalities/breakpoint/same_cu_name/Makefile @@ -0,0 +1,19 @@ +CXX_SOURCES := main.cpp +LD_EXTRAS := ns1.o ns2.o ns3.o ns4.o + +a.out: main.o ns1.o ns2.o ns3.o ns4.o + +ns1.o: common.cpp + $(CC) -g -c -DNAMESPACE=ns1 -o $@ $< + +ns2.o: common.cpp + $(CC) -g -c -DNAMESPACE=ns2 -o $@ $< + +ns3.o: common.cpp + $(CC) -g -c -DNAMESPACE=ns3 -o $@ $< + +ns4.o: common.cpp + $(CC) -g -c -DNAMESPACE=ns4 -o $@ $< + + +include Makefile.rules diff --git a/lldb/test/API/functionalities/breakpoint/same_cu_name/TestFileBreakpoinsSameCUName.py b/lldb/test/API/functionalities/breakpoint/same_cu_name/TestFileBreakpoinsSameCUName.py new file mode 100644 index 00..dc10d407d72302 --- /dev/null +++ b/lldb/test/API/functionalities/breakpoint/same_cu_name/TestFileBreakpoinsSameCUName.py @@ -0,0 +1,32 @@ +""" +Test setting a breakpoint by file and line when many instances of the +same file name exist in the CU list. +""" + + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class TestBreakpointSameCU(TestBase): +def test_breakpoint_same_cu(self): +self.build() +target = self.createTestTarget() + +# Break both on the line before the code: +comment_line = line_number("common.cpp", "// A comment here") +self.assertNotEqual(comment_line, 0, "line_number worked") +bkpt = target.BreakpointCreateByLocation("common.cpp", comment_line) +self.assertEqual( +bkpt.GetNumLocations(), 4, "Got the right number of breakpoints" +) + +# And break on the code, both should work: +code_line = line_number("common.cpp", "// The line with code") +self.assertNotEqual(comment_line, 0,
[Lldb-commits] [lldb] Fix a thinko in the CallSite handling code: (PR #114896)
jimingham wrote: This patch seems to fail on x86_64 Linux, but I don't understand how this patch could cause the reported failure. The failure is in a lldb-dap test, here: File "/var/lib/buildkite-agent/builds/linux-56-59b8f5d88-9dkps-1/llvm-project/github-pull-requests/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py", line 104, in run_test_evaluate_expressions Comparing the log to a successful one, all the breakpoints got set on the same lines. Then we ran and are stopped at the first breakpoint - which parallels a successful test. We also passed a bunch of self.assertEvaluate's to print various variables. Then we go to run the "var" command in "repl" mode: ``` if context == "repl": # In the repl context expressions may be interpreted as lldb # commands since no variables have the same name as the command. self.assertEvaluate("var", r"\(lldb\) var\n.*") ``` and get no reply. Then we try to kill the DAP process, and that also seems to fail. I can't reproduce this locally, and I can't see how this failure could result from this patch... I'm going to apply this patch and keep an eye on the x86_64 bots to see if this was just a glitch or not. https://github.com/llvm/llvm-project/pull/114896 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap to support running in a server mode, allowing multiple connections. (PR #114881)
@@ -9,21 +9,28 @@ #ifndef LLDB_TOOLS_LLDB_DAP_BREAKPOINTBASE_H #define LLDB_TOOLS_LLDB_DAP_BREAKPOINTBASE_H -#include "llvm/Support/JSON.h" #include +#include "llvm/Support/JSON.h" + +#include "DAPForward.h" + namespace lldb_dap { struct BreakpointBase { + // Associated DAP session. + DAP *dap; // An optional expression for conditional breakpoints. std::string condition; // An optional expression that controls how many hits of the breakpoint are // ignored. The backend is expected to interpret the expression as needed std::string hitCondition; + BreakpointBase(DAP *d) : dap(d) {} + BreakpointBase(DAP *d, const llvm::json::Object &obj); ashgti wrote: I initially used `DAP&` but I couldn't copy the a Breakpoint as a result. I switched from the pointer to a reference and corrected usage of the breakpoint types to make sure we either construct entries in place or move them correctly. https://github.com/llvm/llvm-project/pull/114881 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap to support running in a server mode, allowing multiple connections. (PR #114881)
github-actions[bot] wrote: :warning: Python code formatter, darker found issues in your code. :warning: You can test this locally with the following command: ``bash darker --check --diff -r d2d1b5897e871f7b4873befbe2b85db58744e42b...ec85f0f7397c4790442cf99cd51ce61510da498f lldb/test/API/tools/lldb-dap/server/TestDAP_server.py lldb/packages/Python/lldbsuite/test/lldbtest.py lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py `` View the diff from darker here. ``diff --- packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py 2024-11-06 01:56:19.00 + +++ packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py 2024-11-06 01:59:53.834195 + @@ -1162,11 +1162,15 @@ env=None, ): self.process = None if launch: self.process = DebugAdaptorServer.launch( -executable, port=port, unix_socket=unix_socket, log_file=log_file, env=env +executable, +port=port, +unix_socket=unix_socket, +log_file=log_file, +env=env, ) if port: s = socket.socket(socket.AF_INET, socket.SOCK_STREAM) s.connect(("127.0.0.1", port)) --- packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py 2024-11-06 01:56:19.00 + +++ packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py 2024-11-06 01:59:54.000710 + @@ -28,15 +28,22 @@ log_file=log_file_path, env=env, ) def build_and_create_debug_adaptor( -self, lldbDAPEnv=None, lldbDAPLaunch=True, lldbDAPPort=None, lldbDAPUnixSocket=None +self, +lldbDAPEnv=None, +lldbDAPLaunch=True, +lldbDAPPort=None, +lldbDAPUnixSocket=None, ): self.build() self.create_debug_adaptor( -lldbDAPEnv, launch=lldbDAPLaunch, port=lldbDAPPort, unix_socket=lldbDAPUnixSocket +lldbDAPEnv, +launch=lldbDAPLaunch, +port=lldbDAPPort, +unix_socket=lldbDAPUnixSocket, ) def set_source_breakpoints(self, source_path, lines, data=None): """Sets source breakpoints and returns an array of strings containing the breakpoint IDs ("1", "2") for each breakpoint that was set. @@ -487,11 +494,16 @@ lldbDAPLaunch=True, ): """Build the default Makefile target, create the DAP debug adaptor, and launch the process. """ -self.build_and_create_debug_adaptor(lldbDAPEnv=lldbDAPEnv, lldbDAPLaunch=lldbDAPLaunch, lldbDAPPort=lldbDAPPort, lldbDAPUnixSocket=lldbDAPUnixSocket) +self.build_and_create_debug_adaptor( +lldbDAPEnv=lldbDAPEnv, +lldbDAPLaunch=lldbDAPLaunch, +lldbDAPPort=lldbDAPPort, +lldbDAPUnixSocket=lldbDAPUnixSocket, +) self.assertTrue(os.path.exists(program), "executable must exist") return self.launch( program, args, `` https://github.com/llvm/llvm-project/pull/114881 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap to support running in a server mode, allowing multiple connections. (PR #114881)
walter-erquinigo wrote: Yeah, splitting it would be a nice idea https://github.com/llvm/llvm-project/pull/114881 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap to support running in a server mode, allowing multiple connections. (PR #114881)
@@ -12,15 +12,19 @@ namespace lldb_dap { -SourceBreakpoint::SourceBreakpoint(const llvm::json::Object &obj) -: Breakpoint(obj), logMessage(std::string(GetString(obj, "logMessage"))), +SourceBreakpoint::SourceBreakpoint(DAP *dap, const llvm::json::Object &obj) +: Breakpoint(dap, obj), + logMessage(std::string(GetString(obj, "logMessage"))), line(GetUnsigned(obj, "line", 0)), column(GetUnsigned(obj, "column", 0)) { } void SourceBreakpoint::SetBreakpoint(const llvm::StringRef source_path) { + if (!dap) ashgti wrote: It would happen if a breakpoint outlived the dap or was incorrectly initialized. However, I replaced the `DAP*` with a `DAP&`, which means this shouldn't happen anymore. https://github.com/llvm/llvm-project/pull/114881 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap to support running in a server mode, allowing multiple connections. (PR #114881)
@@ -9,21 +9,28 @@ #ifndef LLDB_TOOLS_LLDB_DAP_BREAKPOINTBASE_H #define LLDB_TOOLS_LLDB_DAP_BREAKPOINTBASE_H -#include "llvm/Support/JSON.h" #include +#include "llvm/Support/JSON.h" + +#include "DAPForward.h" + namespace lldb_dap { struct BreakpointBase { + // Associated DAP session. + DAP *dap; ashgti wrote: I updated this to use a `DAP&` everywhere. The DAP objects should live as long as the associated connection and will be released once the connection closes. https://github.com/llvm/llvm-project/pull/114881 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap to support running in a server mode, allowing multiple connections. (PR #114881)
@@ -173,7 +171,7 @@ def do_test_scopes_variables_setVariable_evaluate( "value": "1", }, "declaration": { -"equals": {"line": 12, "column": 14}, +"equals": {"line": 15}, ashgti wrote: Reverted this change https://github.com/llvm/llvm-project/pull/114881 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 7c20bdf - [lldb] Fix synchronization in AlarmTest (NFC)
Author: Jonas Devlieghere Date: 2024-11-05T18:05:33-08:00 New Revision: 7c20bdf373d6cd7f35dee5c71cf94f0eb1be3200 URL: https://github.com/llvm/llvm-project/commit/7c20bdf373d6cd7f35dee5c71cf94f0eb1be3200 DIFF: https://github.com/llvm/llvm-project/commit/7c20bdf373d6cd7f35dee5c71cf94f0eb1be3200.diff LOG: [lldb] Fix synchronization in AlarmTest (NFC) ThreadSanitizer detected a data race as if synchronized via sleep. Added: Modified: lldb/unittests/Host/AlarmTest.cpp Removed: diff --git a/lldb/unittests/Host/AlarmTest.cpp b/lldb/unittests/Host/AlarmTest.cpp index 9f6ad189dee970..94e72af3ffe8d1 100644 --- a/lldb/unittests/Host/AlarmTest.cpp +++ b/lldb/unittests/Host/AlarmTest.cpp @@ -56,6 +56,9 @@ TEST(AlarmTest, Create) { // Leave plenty of time for all the alarms to fire. std::this_thread::sleep_for(TEST_TIMEOUT); + // Acquire the lock to check the callbacks. + std::lock_guard guard(m); + // Make sure all the alarms fired around the expected time. for (size_t i = 0; i < 5; ++i) EXPECT_GE(callbacks_actual[i], callbacks_expected[i]); @@ -83,6 +86,9 @@ TEST(AlarmTest, Exit) { // Let the alarm go out of scope before any alarm had a chance to fire. } + // Acquire the lock to check the callbacks. + std::lock_guard guard(m); + // Make sure none of the alarms fired. for (bool callback : callbacks) EXPECT_TRUE(callback); @@ -113,6 +119,9 @@ TEST(AlarmTest, Cancel) { // Leave plenty of time for all the alarms to fire. std::this_thread::sleep_for(TEST_TIMEOUT); + // Acquire the lock to check the callbacks. + std::lock_guard guard(m); + // Make sure none of the first 4 alarms fired. for (size_t i = 0; i < 4; ++i) EXPECT_FALSE(callbacks[i]); @@ -146,6 +155,7 @@ TEST(AlarmTest, Restart) { // Update the last 2 alarms. for (size_t i = 3; i < 5; ++i) { +std::lock_guard guard(m); callbacks_expected[i] = std::chrono::system_clock::now() + ALARM_TIMEOUT; EXPECT_TRUE(alarm.Restart(handles[i])); } @@ -153,6 +163,9 @@ TEST(AlarmTest, Restart) { // Leave plenty of time for all the alarms to fire. std::this_thread::sleep_for(TEST_TIMEOUT); + // Acquire the lock to check the callbacks. + std::lock_guard guard(m); + // Make sure all the alarms around the expected time. for (size_t i = 0; i < 5; ++i) EXPECT_GE(callbacks_actual[i], callbacks_expected[i]); ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits