[Lldb-commits] [lldb] [LLDB][SBSaveCore] Fix bug where default values are not propagated. (PR #101770)
hokein wrote: I saw the msan failure when integrating LLVM into our internal codebase, it is a msan-build lldb. The second error `AssertionError: launching (4) != stopped (5)` occurred in a non-msan-build lldb from the upstream repository. And I ran `llvm-lit -sv lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py` https://github.com/llvm/llvm-project/pull/101770 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add `` to AddressableBits (PR #102110)
https://github.com/thesamesam created https://github.com/llvm/llvm-project/pull/102110 AddressableBits uses `uint32_t` without including `` which fails to build w/ GCC 15 after a change in libstdc++ [0] [0] https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=3a817a4a5a6d94da9127af3be9f84a74e3076ee2 >From bff13abff5c8e477cd282a7b1b8c418a5baf7239 Mon Sep 17 00:00:00 2001 From: Sam James Date: Tue, 6 Aug 2024 09:40:55 +0100 Subject: [PATCH] [LLDB] Add `` to AddressableBits AddressableBits uses `uint32_t` without including `` which fails to build w/ GCC 15 after a change in libstdc++ [0] [0] https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=3a817a4a5a6d94da9127af3be9f84a74e3076ee2 --- lldb/include/lldb/Utility/AddressableBits.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lldb/include/lldb/Utility/AddressableBits.h b/lldb/include/lldb/Utility/AddressableBits.h index 0d27c3561ec27..8c7a1ec5f52c0 100644 --- a/lldb/include/lldb/Utility/AddressableBits.h +++ b/lldb/include/lldb/Utility/AddressableBits.h @@ -12,6 +12,8 @@ #include "lldb/lldb-forward.h" #include "lldb/lldb-public.h" +#include + namespace lldb_private { /// \class AddressableBits AddressableBits.h "lldb/Core/AddressableBits.h" ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add `` to AddressableBits (PR #102110)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Sam James (thesamesam) Changes AddressableBits uses `uint32_t` without including `` which fails to build w/ GCC 15 after a change in libstdc++ [0] [0] https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=3a817a4a5a6d94da9127af3be9f84a74e3076ee2 --- Full diff: https://github.com/llvm/llvm-project/pull/102110.diff 1 Files Affected: - (modified) lldb/include/lldb/Utility/AddressableBits.h (+2) ``diff diff --git a/lldb/include/lldb/Utility/AddressableBits.h b/lldb/include/lldb/Utility/AddressableBits.h index 0d27c3561ec27..8c7a1ec5f52c0 100644 --- a/lldb/include/lldb/Utility/AddressableBits.h +++ b/lldb/include/lldb/Utility/AddressableBits.h @@ -12,6 +12,8 @@ #include "lldb/lldb-forward.h" #include "lldb/lldb-public.h" +#include + namespace lldb_private { /// \class AddressableBits AddressableBits.h "lldb/Core/AddressableBits.h" `` https://github.com/llvm/llvm-project/pull/102110 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Better matching of types in anonymous namespaces (PR #102111)
https://github.com/labath created https://github.com/llvm/llvm-project/pull/102111 This patch extends TypeQuery matching to support anonymous namespaces. A new flag is added to control the behavior. In the "strict" mode, the query must match the type exactly -- all anonymous namespaces included. The dynamic type resolver in the itanium abi (the motivating use case for this) uses this flag, as it queries using the name from the demangles, which includes anonymous namespaces. This ensures we don't confuse a type with a same-named type in an anonymous namespace. However, this does *not* ensure we don't confuse two types in anonymous namespacs (in different CUs). To resolve this, we would need to use a completely different lookup algorithm, which probably also requires a DWARF extension. In the "lax" mode (the default), the anonymous namespaces in the query are optional, and this allows one search for the type using the usual language rules (`::A` matches `::(anonymous namespace)::A`). This patch also changes the type context computation algorithm in DWARFDIE, so that it includes anonymous namespace information. This causes a slight change in behavior: the algorithm previously stopped computing the context after encountering an anonymous namespace, which caused the outer namespaces to be ignored. This meant that a type like `NS::(anonymous namespace)::A` would be (incorrectly) recognized as `::A`). This can cause code depending on the old behavior to misbehave. The fix is to specify all the enclosing namespaces in the query, or use a non-exact match. >From c6936f3d5d0592babe9082e867b179af594c447b Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Tue, 6 Aug 2024 09:51:20 +0200 Subject: [PATCH] [lldb] Better matching of types in anonymous namespaces This patch extends TypeQuery matching to support anonymous namespaces. A new flag is added to control the behavior. In the "strict" mode, the query must match the type exactly -- all anonymous namespaces included. The dynamic type resolver in the itanium abi (the motivating use case for this) uses this flag, as it queries using the name from the demangles, which includes anonymous namespaces. This ensures we don't confuse a type with a same-named type in an anonymous namespace. However, this does *not* ensure we don't confuse two types in anonymous namespacs (in different CUs). To resolve this, we would need to use a completely different lookup algorithm, which probably also requires a DWARF extension. In the "lax" mode (the default), the anonymous namespaces in the query are optional, and this allows one search for the type using the usual language rules (`::A` matches `::(anonymous namespace)::A`). This patch also changes the type context computation algorithm in DWARFDIE, so that it includes anonymous namespace information. This causes a slight change in behavior: the algorithm previously stopped computing the context after encountering an anonymous namespace, which caused the outer namespaces to be ignored. This meant that a type like `NS::(anonymous namespace)::A` would be (incorrectly) recognized as `::A`). This can cause code depending on the old behavior to misbehave. The fix is to specify all the enclosing namespaces in the query, or use a non-exact match. --- lldb/include/lldb/Symbol/Type.h | 10 - .../ItaniumABI/ItaniumABILanguageRuntime.cpp | 1 + .../Plugins/SymbolFile/DWARF/DWARFDIE.cpp | 8 +--- lldb/source/Symbol/Type.cpp | 33 +--- lldb/test/API/lang/cpp/dynamic-value/Makefile | 2 +- .../cpp/dynamic-value/TestDynamicValue.py | 17 - lldb/test/API/lang/cpp/dynamic-value/a.h | 25 .../lang/cpp/dynamic-value/anonymous-b.cpp| 15 .../lang/cpp/dynamic-value/pass-to-base.cpp | 38 +-- lldb/unittests/Symbol/TestType.cpp| 22 +++ .../SymbolFile/DWARF/DWARFDIETest.cpp | 24 +++- 11 files changed, 149 insertions(+), 46 deletions(-) create mode 100644 lldb/test/API/lang/cpp/dynamic-value/a.h create mode 100644 lldb/test/API/lang/cpp/dynamic-value/anonymous-b.cpp diff --git a/lldb/include/lldb/Symbol/Type.h b/lldb/include/lldb/Symbol/Type.h index 420307e0dbcf0..021e27b52fca7 100644 --- a/lldb/include/lldb/Symbol/Type.h +++ b/lldb/include/lldb/Symbol/Type.h @@ -77,10 +77,13 @@ FLAGS_ENUM(TypeQueryOptions){ /// If set, the query will ignore all Module entries in the type context, /// even for exact matches. e_ignore_modules = (1u << 2), +/// If set, all anonymous namespaces in the context must be matched exactly +/// by the pattern. Otherwise, superfluous namespaces are skipped. +e_strict_namespaces = (1u << 3), /// When true, the find types call should stop the query as soon as a single /// matching type is found. When false, the type query should find all /// matching types. -e_find_one = (1u << 3), +e_find_one = (1u << 4), };
[Lldb-commits] [lldb] [lldb] Better matching of types in anonymous namespaces (PR #102111)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Pavel Labath (labath) Changes This patch extends TypeQuery matching to support anonymous namespaces. A new flag is added to control the behavior. In the "strict" mode, the query must match the type exactly -- all anonymous namespaces included. The dynamic type resolver in the itanium abi (the motivating use case for this) uses this flag, as it queries using the name from the demangles, which includes anonymous namespaces. This ensures we don't confuse a type with a same-named type in an anonymous namespace. However, this does *not* ensure we don't confuse two types in anonymous namespacs (in different CUs). To resolve this, we would need to use a completely different lookup algorithm, which probably also requires a DWARF extension. In the "lax" mode (the default), the anonymous namespaces in the query are optional, and this allows one search for the type using the usual language rules (`::A` matches `::(anonymous namespace)::A`). This patch also changes the type context computation algorithm in DWARFDIE, so that it includes anonymous namespace information. This causes a slight change in behavior: the algorithm previously stopped computing the context after encountering an anonymous namespace, which caused the outer namespaces to be ignored. This meant that a type like `NS::(anonymous namespace)::A` would be (incorrectly) recognized as `::A`). This can cause code depending on the old behavior to misbehave. The fix is to specify all the enclosing namespaces in the query, or use a non-exact match. --- Full diff: https://github.com/llvm/llvm-project/pull/102111.diff 11 Files Affected: - (modified) lldb/include/lldb/Symbol/Type.h (+9-1) - (modified) lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp (+1) - (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp (+1-7) - (modified) lldb/source/Symbol/Type.cpp (+28-5) - (modified) lldb/test/API/lang/cpp/dynamic-value/Makefile (+1-1) - (modified) lldb/test/API/lang/cpp/dynamic-value/TestDynamicValue.py (+16-1) - (added) lldb/test/API/lang/cpp/dynamic-value/a.h (+25) - (added) lldb/test/API/lang/cpp/dynamic-value/anonymous-b.cpp (+15) - (modified) lldb/test/API/lang/cpp/dynamic-value/pass-to-base.cpp (+9-29) - (modified) lldb/unittests/Symbol/TestType.cpp (+22) - (modified) lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp (+22-2) ``diff diff --git a/lldb/include/lldb/Symbol/Type.h b/lldb/include/lldb/Symbol/Type.h index 420307e0dbcf0..021e27b52fca7 100644 --- a/lldb/include/lldb/Symbol/Type.h +++ b/lldb/include/lldb/Symbol/Type.h @@ -77,10 +77,13 @@ FLAGS_ENUM(TypeQueryOptions){ /// If set, the query will ignore all Module entries in the type context, /// even for exact matches. e_ignore_modules = (1u << 2), +/// If set, all anonymous namespaces in the context must be matched exactly +/// by the pattern. Otherwise, superfluous namespaces are skipped. +e_strict_namespaces = (1u << 3), /// When true, the find types call should stop the query as soon as a single /// matching type is found. When false, the type query should find all /// matching types. -e_find_one = (1u << 3), +e_find_one = (1u << 4), }; LLDB_MARK_AS_BITMASK_ENUM(TypeQueryOptions) @@ -266,6 +269,11 @@ class TypeQuery { bool GetIgnoreModules() const { return (m_options & e_ignore_modules) != 0; } void SetIgnoreModules() { m_options &= ~e_ignore_modules; } + bool GetStrictNamespaces() const { +return (m_options & e_strict_namespaces) != 0; + } + void SetStrictNamespaces() { m_options &= ~e_strict_namespaces; } + /// The \a m_context can be used in two ways: normal types searching with /// the context containing a stanadard declaration context for a type, or /// with the context being more complete for exact matches in clang modules. diff --git a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp index 7af768aad0bc1..4c547afe30fe8 100644 --- a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp +++ b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp @@ -90,6 +90,7 @@ TypeAndOrName ItaniumABILanguageRuntime::GetTypeInfo( TypeResults results; TypeQuery query(const_lookup_name.GetStringRef(), TypeQueryOptions::e_exact_match | + TypeQueryOptions::e_strict_namespaces | TypeQueryOptions::e_find_one); if (module_sp) { module_sp->FindTypes(query, results); diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp index fb32e2adeb3fe..0a13c457a307a 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp +++ b/lldb/source/Plugin
[Lldb-commits] [lldb] [LLDB] Add `` to AddressableBits (PR #102110)
https://github.com/labath approved this pull request. https://github.com/llvm/llvm-project/pull/102110 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Better matching of types in anonymous namespaces (PR #102111)
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 d337f5aa59fecd2413b076ed9573e378c57c1307...c6936f3d5d0592babe9082e867b179af594c447b lldb/test/API/lang/cpp/dynamic-value/TestDynamicValue.py `` View the diff from darker here. ``diff --- TestDynamicValue.py 2024-08-06 08:42:30.00 + +++ TestDynamicValue.py 2024-08-06 08:49:54.831969 + @@ -182,11 +182,10 @@ self.assertTrue(anotherA_value) anotherA_loc = int(anotherA_value.GetValue(), 16) self.assertEqual(anotherA_loc, reallyA_loc) self.assertEqual(anotherA_value.GetTypeName().find("B"), -1) - # Finally do the same with a B in an anonymous namespace. threads = lldbutil.continue_to_breakpoint(process, do_something_bpt) self.assertEqual(len(threads), 1) thread = threads[0] @@ -195,11 +194,10 @@ self.assertTrue(anotherA_value) self.assertIn("B", anotherA_value.GetTypeName()) anon_b_value = anotherA_value.GetChildMemberWithName("m_anon_b_value") self.assertTrue(anon_b_value) self.assertEqual(anon_b_value.GetValueAsSigned(), 47) - def examine_value_object_of_this_ptr( self, this_static, this_dynamic, dynamic_location ): # Get "this" as its static value `` https://github.com/llvm/llvm-project/pull/102111 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Better matching of types in anonymous namespaces (PR #102111)
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 d337f5aa59fecd2413b076ed9573e378c57c1307 c6936f3d5d0592babe9082e867b179af594c447b --extensions h,cpp -- lldb/test/API/lang/cpp/dynamic-value/a.h lldb/test/API/lang/cpp/dynamic-value/anonymous-b.cpp lldb/include/lldb/Symbol/Type.h lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp lldb/source/Symbol/Type.cpp lldb/test/API/lang/cpp/dynamic-value/pass-to-base.cpp lldb/unittests/Symbol/TestType.cpp lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp `` View the diff from clang-format here. ``diff diff --git a/lldb/test/API/lang/cpp/dynamic-value/anonymous-b.cpp b/lldb/test/API/lang/cpp/dynamic-value/anonymous-b.cpp index 3bfd4f4b96..755afcbf12 100644 --- a/lldb/test/API/lang/cpp/dynamic-value/anonymous-b.cpp +++ b/lldb/test/API/lang/cpp/dynamic-value/anonymous-b.cpp @@ -10,6 +10,4 @@ private: }; } // namespace -A *make_anonymous_B() { - return new B(); -} +A *make_anonymous_B() { return new B(); } diff --git a/lldb/test/API/lang/cpp/dynamic-value/pass-to-base.cpp b/lldb/test/API/lang/cpp/dynamic-value/pass-to-base.cpp index be763390cc..74bea2214a 100644 --- a/lldb/test/API/lang/cpp/dynamic-value/pass-to-base.cpp +++ b/lldb/test/API/lang/cpp/dynamic-value/pass-to-base.cpp @@ -3,7 +3,8 @@ void A::doSomething(A &anotherA) { printf("In A %p doing something with %d.\n", this, m_a_value); int tmp_value = anotherA.Value(); - printf("Also have another A at %p: %d.\n", &anotherA, tmp_value); // Break here in doSomething. + printf("Also have another A at %p: %d.\n", &anotherA, + tmp_value); // Break here in doSomething. } class Extra diff --git a/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp b/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp index 07e6cf78b7..1e4c8f3ba0 100644 --- a/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp +++ b/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp @@ -275,10 +275,12 @@ DWARF: {DW_TAG_namespace, "NAMESPACE"}})); EXPECT_THAT(anon_struct_die.GetDeclContext(), testing::ElementsAre(make_namespace("NAMESPACE"), - make_namespace(nullptr), make_struct("STRUCT"))); + make_namespace(nullptr), + make_struct("STRUCT"))); EXPECT_THAT(anon_struct_die.GetTypeLookupContext(), testing::ElementsAre(make_namespace("NAMESPACE"), - make_namespace(nullptr), make_struct("STRUCT"))); + make_namespace(nullptr), + make_struct("STRUCT"))); EXPECT_THAT(anon_struct_die.GetDWARFDeclContext(), DWARFDeclContext({{DW_TAG_structure_type, "STRUCT"}, {DW_TAG_namespace, nullptr}, `` https://github.com/llvm/llvm-project/pull/102111 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Better matching of types in anonymous namespaces (PR #102111)
https://github.com/labath updated https://github.com/llvm/llvm-project/pull/102111 >From c6936f3d5d0592babe9082e867b179af594c447b Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Tue, 6 Aug 2024 09:51:20 +0200 Subject: [PATCH 1/2] [lldb] Better matching of types in anonymous namespaces This patch extends TypeQuery matching to support anonymous namespaces. A new flag is added to control the behavior. In the "strict" mode, the query must match the type exactly -- all anonymous namespaces included. The dynamic type resolver in the itanium abi (the motivating use case for this) uses this flag, as it queries using the name from the demangles, which includes anonymous namespaces. This ensures we don't confuse a type with a same-named type in an anonymous namespace. However, this does *not* ensure we don't confuse two types in anonymous namespacs (in different CUs). To resolve this, we would need to use a completely different lookup algorithm, which probably also requires a DWARF extension. In the "lax" mode (the default), the anonymous namespaces in the query are optional, and this allows one search for the type using the usual language rules (`::A` matches `::(anonymous namespace)::A`). This patch also changes the type context computation algorithm in DWARFDIE, so that it includes anonymous namespace information. This causes a slight change in behavior: the algorithm previously stopped computing the context after encountering an anonymous namespace, which caused the outer namespaces to be ignored. This meant that a type like `NS::(anonymous namespace)::A` would be (incorrectly) recognized as `::A`). This can cause code depending on the old behavior to misbehave. The fix is to specify all the enclosing namespaces in the query, or use a non-exact match. --- lldb/include/lldb/Symbol/Type.h | 10 - .../ItaniumABI/ItaniumABILanguageRuntime.cpp | 1 + .../Plugins/SymbolFile/DWARF/DWARFDIE.cpp | 8 +--- lldb/source/Symbol/Type.cpp | 33 +--- lldb/test/API/lang/cpp/dynamic-value/Makefile | 2 +- .../cpp/dynamic-value/TestDynamicValue.py | 17 - lldb/test/API/lang/cpp/dynamic-value/a.h | 25 .../lang/cpp/dynamic-value/anonymous-b.cpp| 15 .../lang/cpp/dynamic-value/pass-to-base.cpp | 38 +-- lldb/unittests/Symbol/TestType.cpp| 22 +++ .../SymbolFile/DWARF/DWARFDIETest.cpp | 24 +++- 11 files changed, 149 insertions(+), 46 deletions(-) create mode 100644 lldb/test/API/lang/cpp/dynamic-value/a.h create mode 100644 lldb/test/API/lang/cpp/dynamic-value/anonymous-b.cpp diff --git a/lldb/include/lldb/Symbol/Type.h b/lldb/include/lldb/Symbol/Type.h index 420307e0dbcf0..021e27b52fca7 100644 --- a/lldb/include/lldb/Symbol/Type.h +++ b/lldb/include/lldb/Symbol/Type.h @@ -77,10 +77,13 @@ FLAGS_ENUM(TypeQueryOptions){ /// If set, the query will ignore all Module entries in the type context, /// even for exact matches. e_ignore_modules = (1u << 2), +/// If set, all anonymous namespaces in the context must be matched exactly +/// by the pattern. Otherwise, superfluous namespaces are skipped. +e_strict_namespaces = (1u << 3), /// When true, the find types call should stop the query as soon as a single /// matching type is found. When false, the type query should find all /// matching types. -e_find_one = (1u << 3), +e_find_one = (1u << 4), }; LLDB_MARK_AS_BITMASK_ENUM(TypeQueryOptions) @@ -266,6 +269,11 @@ class TypeQuery { bool GetIgnoreModules() const { return (m_options & e_ignore_modules) != 0; } void SetIgnoreModules() { m_options &= ~e_ignore_modules; } + bool GetStrictNamespaces() const { +return (m_options & e_strict_namespaces) != 0; + } + void SetStrictNamespaces() { m_options &= ~e_strict_namespaces; } + /// The \a m_context can be used in two ways: normal types searching with /// the context containing a stanadard declaration context for a type, or /// with the context being more complete for exact matches in clang modules. diff --git a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp index 7af768aad0bc1..4c547afe30fe8 100644 --- a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp +++ b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp @@ -90,6 +90,7 @@ TypeAndOrName ItaniumABILanguageRuntime::GetTypeInfo( TypeResults results; TypeQuery query(const_lookup_name.GetStringRef(), TypeQueryOptions::e_exact_match | + TypeQueryOptions::e_strict_namespaces | TypeQueryOptions::e_find_one); if (module_sp) { module_sp->FindTypes(query, results); diff --git a/lldb/source/Plugins/SymbolFile
[Lldb-commits] [lldb] [LLDB] Add `` to AddressableBits (PR #102110)
thesamesam wrote: Thanks! https://github.com/llvm/llvm-project/pull/102110 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] bb59f04 - [LLDB] Add `` to AddressableBits (#102110)
Author: Sam James Date: 2024-08-06T09:58:36+01:00 New Revision: bb59f04e7e75dcbe39f1bf952304a157f0035314 URL: https://github.com/llvm/llvm-project/commit/bb59f04e7e75dcbe39f1bf952304a157f0035314 DIFF: https://github.com/llvm/llvm-project/commit/bb59f04e7e75dcbe39f1bf952304a157f0035314.diff LOG: [LLDB] Add `` to AddressableBits (#102110) Added: Modified: lldb/include/lldb/Utility/AddressableBits.h Removed: diff --git a/lldb/include/lldb/Utility/AddressableBits.h b/lldb/include/lldb/Utility/AddressableBits.h index 0d27c3561ec27..8c7a1ec5f52c0 100644 --- a/lldb/include/lldb/Utility/AddressableBits.h +++ b/lldb/include/lldb/Utility/AddressableBits.h @@ -12,6 +12,8 @@ #include "lldb/lldb-forward.h" #include "lldb/lldb-public.h" +#include + namespace lldb_private { /// \class AddressableBits AddressableBits.h "lldb/Core/AddressableBits.h" ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add `` to AddressableBits (PR #102110)
https://github.com/thesamesam closed https://github.com/llvm/llvm-project/pull/102110 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add `` to AddressableBits (PR #102110)
https://github.com/thesamesam milestoned https://github.com/llvm/llvm-project/pull/102110 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add `` to AddressableBits (PR #102110)
thesamesam wrote: /cherry-pick bb59f04e7e75dcbe39f1bf952304a157f0035314 https://github.com/llvm/llvm-project/pull/102110 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add `` to AddressableBits (PR #102110)
llvmbot wrote: /pull-request llvm/llvm-project#102112 https://github.com/llvm/llvm-project/pull/102110 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix crash when adding members to an "incomplete" type (PR #102116)
https://github.com/labath created https://github.com/llvm/llvm-project/pull/102116 This fixes a regression caused by delayed type definition searching (#96755 and friends): If we end up adding a member (e.g. a typedef) to a type that we've already attempted to complete (and failed), the resulting AST would end up inconsistent (we would start to "forcibly" complete it, but never finish it), and importing it into an expression AST would crash. This patch fixes this by detecting the situation and finishing the definition as well. >From c5373215793f744ab80b80e394354e455b415fcb Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Tue, 6 Aug 2024 11:36:31 +0200 Subject: [PATCH] [lldb] Fix crash when adding members to an "incomplete" type This fixes a regression caused by delayed type definition searching (#96755 and friends): If we end up adding a member (e.g. a typedef) to a type that we've already attempted to complete (and failed), the resulting AST would end up inconsistent (we would start to "forcibly" complete it, but never finish it), and importing it into an expression AST would crash. This patch fixes this by detecting the situation and finishing the definition as well. --- .../SymbolFile/DWARF/DWARFASTParserClang.cpp | 11 +++-- .../DWARF/x86/typedef-in-incomplete-type.cpp | 23 +++ 2 files changed, 32 insertions(+), 2 deletions(-) create mode 100644 lldb/test/Shell/SymbolFile/DWARF/x86/typedef-in-incomplete-type.cpp diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index a4dcde1629fc2..3968d5bf71e1c 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -269,8 +269,15 @@ static void PrepareContextToReceiveMembers(TypeSystemClang &ast, } // We don't have a type definition and/or the import failed, but we need to - // add members to it. Start the definition to make that possible. - tag_decl_ctx->startDefinition(); + // add members to it. Start the definition to make that possible. If the type + // has no external storage we also have to complete the definition. Otherwise, + // that will happen when we are asked to complete the type + // (CompleteTypeFromDWARF). + ast.StartTagDeclarationDefinition(type); + if (!tag_decl_ctx->hasExternalLexicalStorage()) { +ast.SetDeclIsForcefullyCompleted(tag_decl_ctx); +ast.CompleteTagDeclarationDefinition(type); + } } ParsedDWARFTypeAttributes::ParsedDWARFTypeAttributes(const DWARFDIE &die) { diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/typedef-in-incomplete-type.cpp b/lldb/test/Shell/SymbolFile/DWARF/x86/typedef-in-incomplete-type.cpp new file mode 100644 index 0..47ea1e2639c6e --- /dev/null +++ b/lldb/test/Shell/SymbolFile/DWARF/x86/typedef-in-incomplete-type.cpp @@ -0,0 +1,23 @@ +// RUN: %clangxx --target=x86_64-pc-linux -o %t -c %s -g +// RUN: %lldb %t -o "target var a" -o "expr -- var" -o exit | FileCheck %s + +// This forces lldb to attempt to complete the type A. Since it has no +// definition it will fail. +// CHECK: target var a +// CHECK: (A) a = + +// Now attempt to display the second variable, which will try to add a typedef +// to the incomplete type. Make sure that succeeds. Use the expression command +// to make sure the resulting AST can be imported correctly. +// CHECK: expr -- var +// CHECK: (A::X) $0 = 0 + +struct A { + // Declare the constructor, but don't define it to avoid emitting the + // definition in the debug info. + A(); + using X = int; +}; + +A a; +A::X var; ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix crash when adding members to an "incomplete" type (PR #102116)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Pavel Labath (labath) Changes This fixes a regression caused by delayed type definition searching (#96755 and friends): If we end up adding a member (e.g. a typedef) to a type that we've already attempted to complete (and failed), the resulting AST would end up inconsistent (we would start to "forcibly" complete it, but never finish it), and importing it into an expression AST would crash. This patch fixes this by detecting the situation and finishing the definition as well. --- Full diff: https://github.com/llvm/llvm-project/pull/102116.diff 2 Files Affected: - (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp (+9-2) - (added) lldb/test/Shell/SymbolFile/DWARF/x86/typedef-in-incomplete-type.cpp (+23) ``diff diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index a4dcde1629fc2..3968d5bf71e1c 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -269,8 +269,15 @@ static void PrepareContextToReceiveMembers(TypeSystemClang &ast, } // We don't have a type definition and/or the import failed, but we need to - // add members to it. Start the definition to make that possible. - tag_decl_ctx->startDefinition(); + // add members to it. Start the definition to make that possible. If the type + // has no external storage we also have to complete the definition. Otherwise, + // that will happen when we are asked to complete the type + // (CompleteTypeFromDWARF). + ast.StartTagDeclarationDefinition(type); + if (!tag_decl_ctx->hasExternalLexicalStorage()) { +ast.SetDeclIsForcefullyCompleted(tag_decl_ctx); +ast.CompleteTagDeclarationDefinition(type); + } } ParsedDWARFTypeAttributes::ParsedDWARFTypeAttributes(const DWARFDIE &die) { diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/typedef-in-incomplete-type.cpp b/lldb/test/Shell/SymbolFile/DWARF/x86/typedef-in-incomplete-type.cpp new file mode 100644 index 0..47ea1e2639c6e --- /dev/null +++ b/lldb/test/Shell/SymbolFile/DWARF/x86/typedef-in-incomplete-type.cpp @@ -0,0 +1,23 @@ +// RUN: %clangxx --target=x86_64-pc-linux -o %t -c %s -g +// RUN: %lldb %t -o "target var a" -o "expr -- var" -o exit | FileCheck %s + +// This forces lldb to attempt to complete the type A. Since it has no +// definition it will fail. +// CHECK: target var a +// CHECK: (A) a = + +// Now attempt to display the second variable, which will try to add a typedef +// to the incomplete type. Make sure that succeeds. Use the expression command +// to make sure the resulting AST can be imported correctly. +// CHECK: expr -- var +// CHECK: (A::X) $0 = 0 + +struct A { + // Declare the constructor, but don't define it to avoid emitting the + // definition in the debug info. + A(); + using X = int; +}; + +A a; +A::X var; `` https://github.com/llvm/llvm-project/pull/102116 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][ClangExpressionParser] Set BuiltinHeadersInSystemModules depending on SDK version (PR #101778)
https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/101778 >From 3d7c2b87ec73a48de30e1c5387a7f0d8d817b0f4 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Fri, 2 Aug 2024 23:38:18 +0100 Subject: [PATCH 1/6] [lldb][ClangExpressionParser] Set BuiltinHeadersInSystemModules depending on SDK version This patch addresses an issue that will arise with future SDKs. The ClangExpressionParser currently unconditionally sets `-fbuiltin-headers-in-system-modules` when evaluating expressions with the `target.import-std-module` setting. This flag should, however, be set depending on the SDK version (which is what the Clang Darwin toolchain does). Unfortunately, the compiler instance that we create with `ClangExpressionParser` never consults the Clang driver, and thus doesn't correctly infer `BuiltinHeadersInSystemModules`. Note, this isn't an issue with the `CompilerInstance` that the `ClangModulesDeclVendor` creates because it uses the `createInovcation` API, which calls into `Darwin::addClangTargetOptions`. This patch mimicks how `sdkSupportsBuiltinModules` is used in `Darwin::addClangTargetOptions`. This ensures that the `import-std-module` API tests run cleanly regardless of SDK version. The plan is to make the `CompilerInstance` construction in `ClangExpressionParser` go through the driver, so we can avoid duplicating the logic in LLDB. --- .../Clang/ClangExpressionParser.cpp | 69 ++- 1 file changed, 66 insertions(+), 3 deletions(-) diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp index 2a8bdf29314e4..c6217e2f13394 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp @@ -11,6 +11,7 @@ #include "clang/AST/ExternalASTSource.h" #include "clang/AST/PrettyPrinter.h" #include "clang/Basic/Builtins.h" +#include "clang/Basic/DarwinSDKInfo.h" #include "clang/Basic/DiagnosticIDs.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/TargetInfo.h" @@ -561,7 +562,62 @@ static void SetupLangOpts(CompilerInstance &compiler, lang_opts.NoBuiltin = true; } -static void SetupImportStdModuleLangOpts(CompilerInstance &compiler) { +// NOTE: should be kept in sync with sdkSupportsBuiltinModules in +// Toolchains/Darwin.cpp +static bool +sdkSupportsBuiltinModulesImpl(const llvm::Triple &triple, + const std::optional &SDKInfo) { + if (!SDKInfo) +return false; + + VersionTuple SDKVersion = SDKInfo->getVersion(); + switch (triple.getOS()) { + case Triple::OSType::MacOSX: +return SDKVersion >= VersionTuple(15U); + case Triple::OSType::IOS: +return SDKVersion >= VersionTuple(18U); + case Triple::OSType::TvOS: +return SDKVersion >= VersionTuple(18U); + case Triple::OSType::WatchOS: +return SDKVersion >= VersionTuple(11U); + case Triple::OSType::XROS: +return SDKVersion >= VersionTuple(2U); + default: +// New SDKs support builtin modules from the start. +return true; + } +} + +static bool +sdkSupportsBuiltinModules(llvm::Triple const &triple, + std::vector const &include_dirs) { + static constexpr std::string_view s_sdk_suffix = ".sdk"; + auto it = llvm::find_if(include_dirs, [](std::string const &path) { +return path.find(s_sdk_suffix) != std::string::npos; + }); + if (it == include_dirs.end()) +return false; + + size_t suffix = it->find(s_sdk_suffix); + if (suffix == std::string::npos) +return false; + + auto VFS = FileSystem::Instance().GetVirtualFileSystem(); + if (!VFS) +return false; + + std::string sdk_path = it->substr(0, suffix + s_sdk_suffix.size()); + auto parsed = clang::parseDarwinSDKInfo(*VFS, sdk_path); + if (!parsed) +return false; + + return sdkSupportsBuiltinModulesImpl(triple, *parsed); +} + +static void +SetupImportStdModuleLangOpts(CompilerInstance &compiler, + llvm::Triple const &triple, + std::vector const &include_dirs) { LangOptions &lang_opts = compiler.getLangOpts(); lang_opts.Modules = true; // We want to implicitly build modules. @@ -578,7 +634,12 @@ static void SetupImportStdModuleLangOpts(CompilerInstance &compiler) { lang_opts.GNUMode = true; lang_opts.GNUKeywords = true; lang_opts.CPlusPlus11 = true; - lang_opts.BuiltinHeadersInSystemModules = true; + + // FIXME: We should use the driver to derive this for use. + // ClangModulesDeclVendor already parses the SDKSettings for the purposes of + // this check. + lang_opts.BuiltinHeadersInSystemModules = + !sdkSupportsBuiltinModules(triple, include_dirs); // The Darwin libc expects this macro to be set. lang_opts.GNUCVersion = 40201; @@ -663,7 +724,9 @@ ClangExpressionParser::ClangExpressionParser( if (auto *clang_expr = dyn_cast(&m_expr); clang_ex
[Lldb-commits] [lldb] [lldb/DWARF] Search fallback to the manual index in GetFullyQualified… (PR #102123)
https://github.com/labath created https://github.com/llvm/llvm-project/pull/102123 …Type This is needed to ensure we find a type if its definition is in a CU that wasn't indexed. This can happen if the definition is in some precompiled code (e.g. the c++ standard library) which was built with different flags than the rest of the binary. >From cde7cd17f63585cf823e5b26cf38bcecc1c300f4 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Tue, 6 Aug 2024 13:08:32 +0200 Subject: [PATCH] [lldb/DWARF] Search fallback to the manual index in GetFullyQualifiedType This is needed to ensure we find a type if its definition is in a CU that wasn't indexed. This can happen if the definition is in some precompiled code (e.g. the c++ standard library) which was built with different flags than the rest of the binary. --- .../SymbolFile/DWARF/DebugNamesDWARFIndex.cpp | 1 + ...ixed-debug-names-complete-type-search.test | 35 +++ 2 files changed, 36 insertions(+) create mode 100644 lldb/test/Shell/SymbolFile/DWARF/x86/mixed-debug-names-complete-type-search.test diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp index 7e66b3dccf97f..32d8a92305aaf 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp @@ -371,6 +371,7 @@ void DebugNamesDWARFIndex::GetFullyQualifiedType( !ProcessEntry(entry, callback)) return; } + m_fallback.GetFullyQualifiedType(context, callback); } bool DebugNamesDWARFIndex::SameParentChain( diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/mixed-debug-names-complete-type-search.test b/lldb/test/Shell/SymbolFile/DWARF/x86/mixed-debug-names-complete-type-search.test new file mode 100644 index 0..71da8fad00165 --- /dev/null +++ b/lldb/test/Shell/SymbolFile/DWARF/x86/mixed-debug-names-complete-type-search.test @@ -0,0 +1,35 @@ +REQUIRES: lld, python + +RUN: split-file %s %t +RUN: %clang --target=x86_64-pc-linux -g -gpubnames -c %t/file1.c -o %t-1.o +RUN: %clang --target=x86_64-pc-linux -g -gno-pubnames -c %t/file2.c -o %t-2.o +RUN: llvm-dwarfdump %t-1.o --debug-names | FileCheck %s --check-prefix=PUBNAMES +RUN: llvm-dwarfdump %t-2.o --debug-names | FileCheck %s --check-prefix=NOPUBNAMES +RUN: ld.lld %t-1.o %t-2.o -o %t.out +RUN: %lldb %t.out -s %t/commands -o exit | FileCheck %s + +// Precondition check: The first file should contain a debug_names index, but no +// entries for MYSTRUCT. +PUBNAMES: Name Index @ 0x0 { +PUBNAMES-NOT: MYSTRUCT + +// The second file should not contain an index. +NOPUBNAMES-NOT: Name Index + +// Starting from the variable in the first file, we should be able to find the +// declaration of the type in the first unit, and then match that with the +// definition in the second unit. +CHECK: (lldb) script +CHECK: struct MYSTRUCT { +CHECK-NEXT: int x; +CHECK-NEXT: } + +#--- commands +script lldb.target.FindFirstGlobalVariable("struct_ptr").GetType().GetPointeeType() +#--- file1.c +struct MYSTRUCT *struct_ptr; +#--- file2.c +struct MYSTRUCT { + int x; +}; +struct MYSTRUCT struct_; ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Search fallback to the manual index in GetFullyQualified… (PR #102123)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Pavel Labath (labath) Changes …Type This is needed to ensure we find a type if its definition is in a CU that wasn't indexed. This can happen if the definition is in some precompiled code (e.g. the c++ standard library) which was built with different flags than the rest of the binary. --- Full diff: https://github.com/llvm/llvm-project/pull/102123.diff 2 Files Affected: - (modified) lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp (+1) - (added) lldb/test/Shell/SymbolFile/DWARF/x86/mixed-debug-names-complete-type-search.test (+35) ``diff diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp index 7e66b3dccf97f..32d8a92305aaf 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp @@ -371,6 +371,7 @@ void DebugNamesDWARFIndex::GetFullyQualifiedType( !ProcessEntry(entry, callback)) return; } + m_fallback.GetFullyQualifiedType(context, callback); } bool DebugNamesDWARFIndex::SameParentChain( diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/mixed-debug-names-complete-type-search.test b/lldb/test/Shell/SymbolFile/DWARF/x86/mixed-debug-names-complete-type-search.test new file mode 100644 index 0..71da8fad00165 --- /dev/null +++ b/lldb/test/Shell/SymbolFile/DWARF/x86/mixed-debug-names-complete-type-search.test @@ -0,0 +1,35 @@ +REQUIRES: lld, python + +RUN: split-file %s %t +RUN: %clang --target=x86_64-pc-linux -g -gpubnames -c %t/file1.c -o %t-1.o +RUN: %clang --target=x86_64-pc-linux -g -gno-pubnames -c %t/file2.c -o %t-2.o +RUN: llvm-dwarfdump %t-1.o --debug-names | FileCheck %s --check-prefix=PUBNAMES +RUN: llvm-dwarfdump %t-2.o --debug-names | FileCheck %s --check-prefix=NOPUBNAMES +RUN: ld.lld %t-1.o %t-2.o -o %t.out +RUN: %lldb %t.out -s %t/commands -o exit | FileCheck %s + +// Precondition check: The first file should contain a debug_names index, but no +// entries for MYSTRUCT. +PUBNAMES: Name Index @ 0x0 { +PUBNAMES-NOT: MYSTRUCT + +// The second file should not contain an index. +NOPUBNAMES-NOT: Name Index + +// Starting from the variable in the first file, we should be able to find the +// declaration of the type in the first unit, and then match that with the +// definition in the second unit. +CHECK: (lldb) script +CHECK: struct MYSTRUCT { +CHECK-NEXT: int x; +CHECK-NEXT: } + +#--- commands +script lldb.target.FindFirstGlobalVariable("struct_ptr").GetType().GetPointeeType() +#--- file1.c +struct MYSTRUCT *struct_ptr; +#--- file2.c +struct MYSTRUCT { + int x; +}; +struct MYSTRUCT struct_; `` https://github.com/llvm/llvm-project/pull/102123 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Better matching of types in anonymous namespaces (PR #102111)
https://github.com/Michael137 approved this pull request. LGTM, thanks! Left some stylistic comments/clarifications https://github.com/llvm/llvm-project/pull/102111 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Better matching of types in anonymous namespaces (PR #102111)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/102111 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Better matching of types in anonymous namespaces (PR #102111)
@@ -788,7 +808,10 @@ Type::GetTypeScopeAndBasename(llvm::StringRef name) { switch (pos.value()) { case ':': if (prev_is_colon && template_depth == 0) { -result.scope.push_back(name.slice(name_begin, pos.index() - 1)); +llvm::StringRef scope_name = name.slice(name_begin, pos.index() - 1); Michael137 wrote: Do I understand correctly that we could've kept this as-is, but would then have to check for the `(anonymous namespace)` string in the `ContextMatches` function? Could we leave a comment on why this is necessary? E.g., how this string might end up in the parsed name (aka the demangler). https://github.com/llvm/llvm-project/pull/102111 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Better matching of types in anonymous namespaces (PR #102111)
@@ -111,4 +115,22 @@ TEST(Type, CompilerContextPattern) { Matches(std::vector{make_class("C")})); EXPECT_THAT((std::vector{make_namespace("NS"), make_class("C")}), Not(Matches(std::vector{make_any_type("C")}))); + + EXPECT_THAT((std::vector{make_namespace(""), make_class("C")}), + Matches(std::vector{make_class("C")})); + EXPECT_THAT((std::vector{make_namespace(""), make_class("C")}), + Not(MatchesWithStrictNamespaces(std::vector{make_class("C")}))); + EXPECT_THAT((std::vector{make_namespace(""), make_class("C")}), + Matches(std::vector{make_namespace(""), make_class("C")})); + EXPECT_THAT((std::vector{make_namespace(""), make_class("C")}), + MatchesWithStrictNamespaces( + std::vector{make_namespace(""), make_class("C")})); + EXPECT_THAT((std::vector{make_class("C")}), + Not(Matches(std::vector{make_namespace(""), make_class("C")}))); + EXPECT_THAT((std::vector{make_class("C")}), + Not(MatchesWithStrictNamespaces( + std::vector{make_namespace(""), make_class("C")}))); + EXPECT_THAT((std::vector{make_namespace(""), make_namespace("NS"), + make_namespace(""), make_class("C")}), + Matches(std::vector{make_namespace("NS"), make_class("C")})); Michael137 wrote: These are great! Could also add a case for: * consecutive empty anonymous namespace (not sure why that would happen but clang happily generates symbols such as `_ZN2ns12_GLOBAL__N_112_GLOBAL__N_14funcEv` 🤷) * mix modules and anonymous namespaces https://github.com/llvm/llvm-project/pull/102111 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Better matching of types in anonymous namespaces (PR #102111)
@@ -145,10 +159,16 @@ bool TypeQuery::ContextMatches( ++pat; } - // Skip over any remaining module entries if we were asked to do that. - while (GetIgnoreModules() && ctx != ctx_end && - ctx->kind == CompilerContextKind::Module) -++ctx; + // Skip over any remaining module and anonymous namespace entries if we were + // asked to do that. + auto should_skip = [this](const CompilerContext &ctx) { +if (ctx.kind == CompilerContextKind::Module) + return GetIgnoreModules(); +if (ctx.kind == CompilerContextKind::Namespace && ctx.name.IsEmpty()) Michael137 wrote: Almost wonder if this check should be a `IsAnonymousNamespace` helper function on `CompilerContext`. But maybe that'd make it too C++-specific? Since it's just in this function, feel free to ignore https://github.com/llvm/llvm-project/pull/102111 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Better matching of types in anonymous namespaces (PR #102111)
@@ -266,6 +269,11 @@ class TypeQuery { bool GetIgnoreModules() const { return (m_options & e_ignore_modules) != 0; } void SetIgnoreModules() { m_options &= ~e_ignore_modules; } + bool GetStrictNamespaces() const { +return (m_options & e_strict_namespaces) != 0; + } + void SetStrictNamespaces() { m_options &= ~e_strict_namespaces; } Michael137 wrote: Wouldn't this clear the bit? https://github.com/llvm/llvm-project/pull/102111 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Better matching of types in anonymous namespaces (PR #102111)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/102111 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Better matching of types in anonymous namespaces (PR #102111)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/102111 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix crash when adding members to an "incomplete" type (PR #102116)
@@ -0,0 +1,23 @@ +// RUN: %clangxx --target=x86_64-pc-linux -o %t -c %s -g Michael137 wrote: This would only crash with `-flimit-debug-info` right? Could we add that here? Also should we move it to the non-x86 directory? Not sure what the intent behind the x86 directory is but it would be nice to get the coverage when running this on ARM machines too https://github.com/llvm/llvm-project/pull/102116 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/101283 >From 6b2a41ba3d71270e81e24a42d3b4f5dc2f96b939 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Wed, 31 Jul 2024 05:41:21 +0400 Subject: [PATCH 1/2] [lldb] Updated lldb-server to spawn the child process and share socket on Windows `lldb-server platform --server` works on Windows now w/o multithreading. The rest functionality remains unchanged. Depends on #101383. Fixes #90923, fixes #56346. This is the part 1 of the replacement of #100670. In the part 2 I plan to switch `lldb-server gdbserver` to use `--fd` and listen a common gdb port for all gdbserver connections. Then we can remove gdb port mapping to fix #97537. --- lldb/tools/lldb-server/lldb-platform.cpp | 347 --- 1 file changed, 306 insertions(+), 41 deletions(-) diff --git a/lldb/tools/lldb-server/lldb-platform.cpp b/lldb/tools/lldb-server/lldb-platform.cpp index 7148a1d2a3094..e23237ef574c3 100644 --- a/lldb/tools/lldb-server/lldb-platform.cpp +++ b/lldb/tools/lldb-server/lldb-platform.cpp @@ -22,6 +22,7 @@ #include #include "llvm/Support/FileSystem.h" +#include "llvm/Support/ScopedPrinter.h" #include "llvm/Support/WithColor.h" #include "llvm/Support/raw_ostream.h" @@ -32,8 +33,10 @@ #include "lldb/Host/ConnectionFileDescriptor.h" #include "lldb/Host/HostGetOpt.h" #include "lldb/Host/OptionParser.h" +#include "lldb/Host/Socket.h" #include "lldb/Host/common/TCPSocket.h" #include "lldb/Utility/FileSpec.h" +#include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/Status.h" using namespace lldb; @@ -60,6 +63,9 @@ static struct option g_long_options[] = { {"max-gdbserver-port", required_argument, nullptr, 'M'}, {"socket-file", required_argument, nullptr, 'f'}, {"server", no_argument, &g_server, 1}, +#if defined(_WIN32) +{"accept", required_argument, nullptr, 'a'}, +#endif {nullptr, 0, nullptr, 0}}; #if defined(__APPLE__) @@ -114,6 +120,249 @@ static Status save_socket_id_to_file(const std::string &socket_id, return status; } +static void client_handle(GDBRemoteCommunicationServerPlatform &platform, + const lldb_private::Args &args) { + if (!platform.IsConnected()) +return; + + if (args.GetArgumentCount() > 0) { +lldb::pid_t pid = LLDB_INVALID_PROCESS_ID; +std::optional port; +std::string socket_name; +Status error = platform.LaunchGDBServer(args, +"", // hostname +pid, port, socket_name); +if (error.Success()) + platform.SetPendingGdbServer(pid, *port, socket_name); +else + fprintf(stderr, "failed to start gdbserver: %s\n", error.AsCString()); + } + + bool interrupt = false; + bool done = false; + Status error; + while (!interrupt && !done) { +if (platform.GetPacketAndSendResponse(std::nullopt, error, interrupt, + done) != +GDBRemoteCommunication::PacketResult::Success) + break; + } + + if (error.Fail()) +WithColor::error() << error.AsCString() << '\n'; +} + +static GDBRemoteCommunicationServerPlatform::PortMap gdbserver_portmap; +static std::mutex gdbserver_portmap_mutex; + +#if defined(_WIN32) +static void spawn_process_reaped(lldb::pid_t pid, int signal, int status) { + std::lock_guard guard(gdbserver_portmap_mutex); + gdbserver_portmap.FreePortForProcess(pid); +} + +static bool spawn_process_parent(const char *progname, Connection *conn, + uint16_t gdb_port, uint16_t port_offset, + const lldb_private::Args &args, + const std::string &log_file, + const StringRef log_channels) { + Log *log = GetLog(LLDBLog::Platform); + Pipe socket_pipe; + Status error = socket_pipe.CreateNew(true); + if (error.Fail()) { +LLDB_LOGF(log, + "lldb-platform parent: " + "cannot create pipe: %s", + error.AsCString()); +return false; + } + + ProcessLaunchInfo launch_info; + FileSpec self_spec(progname, FileSpec::Style::native); + launch_info.SetExecutableFile(self_spec, true); + Args &self_args = launch_info.GetArguments(); + self_args.AppendArgument(llvm::StringRef("platform")); + self_args.AppendArgument(llvm::StringRef("--accept")); + self_args.AppendArgument(llvm::to_string(socket_pipe.GetReadPipe())); + if (gdb_port) { +self_args.AppendArgument(llvm::StringRef("--gdbserver-port")); +self_args.AppendArgument(llvm::to_string(gdb_port)); + } + if (port_offset > 0) { +self_args.AppendArgument(llvm::StringRef("--port-offset")); +self_args.AppendArgument(llvm::to_string(port_offset)); + } + if (!log_file.empty()) { +self_args.AppendArgument(llvm::StringRef("--log-file")); +self_args.AppendArgument(log_file); + } + if (!log_channels.empty()) { +self_args.AppendArg
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket (PR #101283)
https://github.com/slydiman edited https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket (PR #101283)
slydiman wrote: I have updated the patch to spawn the child process on non-Windows too and minimized the number of `#ifdef _WIN32`. https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket (PR #101283)
https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/101283 >From 6b2a41ba3d71270e81e24a42d3b4f5dc2f96b939 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Wed, 31 Jul 2024 05:41:21 +0400 Subject: [PATCH 1/2] [lldb] Updated lldb-server to spawn the child process and share socket on Windows `lldb-server platform --server` works on Windows now w/o multithreading. The rest functionality remains unchanged. Depends on #101383. Fixes #90923, fixes #56346. This is the part 1 of the replacement of #100670. In the part 2 I plan to switch `lldb-server gdbserver` to use `--fd` and listen a common gdb port for all gdbserver connections. Then we can remove gdb port mapping to fix #97537. --- lldb/tools/lldb-server/lldb-platform.cpp | 347 --- 1 file changed, 306 insertions(+), 41 deletions(-) diff --git a/lldb/tools/lldb-server/lldb-platform.cpp b/lldb/tools/lldb-server/lldb-platform.cpp index 7148a1d2a3094..e23237ef574c3 100644 --- a/lldb/tools/lldb-server/lldb-platform.cpp +++ b/lldb/tools/lldb-server/lldb-platform.cpp @@ -22,6 +22,7 @@ #include #include "llvm/Support/FileSystem.h" +#include "llvm/Support/ScopedPrinter.h" #include "llvm/Support/WithColor.h" #include "llvm/Support/raw_ostream.h" @@ -32,8 +33,10 @@ #include "lldb/Host/ConnectionFileDescriptor.h" #include "lldb/Host/HostGetOpt.h" #include "lldb/Host/OptionParser.h" +#include "lldb/Host/Socket.h" #include "lldb/Host/common/TCPSocket.h" #include "lldb/Utility/FileSpec.h" +#include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/Status.h" using namespace lldb; @@ -60,6 +63,9 @@ static struct option g_long_options[] = { {"max-gdbserver-port", required_argument, nullptr, 'M'}, {"socket-file", required_argument, nullptr, 'f'}, {"server", no_argument, &g_server, 1}, +#if defined(_WIN32) +{"accept", required_argument, nullptr, 'a'}, +#endif {nullptr, 0, nullptr, 0}}; #if defined(__APPLE__) @@ -114,6 +120,249 @@ static Status save_socket_id_to_file(const std::string &socket_id, return status; } +static void client_handle(GDBRemoteCommunicationServerPlatform &platform, + const lldb_private::Args &args) { + if (!platform.IsConnected()) +return; + + if (args.GetArgumentCount() > 0) { +lldb::pid_t pid = LLDB_INVALID_PROCESS_ID; +std::optional port; +std::string socket_name; +Status error = platform.LaunchGDBServer(args, +"", // hostname +pid, port, socket_name); +if (error.Success()) + platform.SetPendingGdbServer(pid, *port, socket_name); +else + fprintf(stderr, "failed to start gdbserver: %s\n", error.AsCString()); + } + + bool interrupt = false; + bool done = false; + Status error; + while (!interrupt && !done) { +if (platform.GetPacketAndSendResponse(std::nullopt, error, interrupt, + done) != +GDBRemoteCommunication::PacketResult::Success) + break; + } + + if (error.Fail()) +WithColor::error() << error.AsCString() << '\n'; +} + +static GDBRemoteCommunicationServerPlatform::PortMap gdbserver_portmap; +static std::mutex gdbserver_portmap_mutex; + +#if defined(_WIN32) +static void spawn_process_reaped(lldb::pid_t pid, int signal, int status) { + std::lock_guard guard(gdbserver_portmap_mutex); + gdbserver_portmap.FreePortForProcess(pid); +} + +static bool spawn_process_parent(const char *progname, Connection *conn, + uint16_t gdb_port, uint16_t port_offset, + const lldb_private::Args &args, + const std::string &log_file, + const StringRef log_channels) { + Log *log = GetLog(LLDBLog::Platform); + Pipe socket_pipe; + Status error = socket_pipe.CreateNew(true); + if (error.Fail()) { +LLDB_LOGF(log, + "lldb-platform parent: " + "cannot create pipe: %s", + error.AsCString()); +return false; + } + + ProcessLaunchInfo launch_info; + FileSpec self_spec(progname, FileSpec::Style::native); + launch_info.SetExecutableFile(self_spec, true); + Args &self_args = launch_info.GetArguments(); + self_args.AppendArgument(llvm::StringRef("platform")); + self_args.AppendArgument(llvm::StringRef("--accept")); + self_args.AppendArgument(llvm::to_string(socket_pipe.GetReadPipe())); + if (gdb_port) { +self_args.AppendArgument(llvm::StringRef("--gdbserver-port")); +self_args.AppendArgument(llvm::to_string(gdb_port)); + } + if (port_offset > 0) { +self_args.AppendArgument(llvm::StringRef("--port-offset")); +self_args.AppendArgument(llvm::to_string(port_offset)); + } + if (!log_file.empty()) { +self_args.AppendArgument(llvm::StringRef("--log-file")); +self_args.AppendArgument(log_file); + } + if (!log_channels.empty()) { +self_args.AppendArg
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket (PR #101283)
labath wrote: I would start looking at this from the other end. Let's identify logical pieces of functionality that can be shared between the various modes of operation (fork/exec/singleserver), and try to refactor to code to fit that. While doing that, I think an important criteria should be whether some of the code is platform-specific. If it is, the function containing that should be as small as possible so that the ifdef doesn't occur in a deeply nested code (deeply nested ifdefs are much harder to reason about than one that occurs at the top level. The first obvious piece of functionality I see is the one which serves a single connection. This is something we need to execute in all three code paths. It should definitely be a function of its own (I'm going to call it `ServeSingleConnection` here, but there may be a name that better fits local conventions), so that we can just call it, instead of reaching it through a web of compile and runtime conditionals. The second one is the piece that serves multiple connections (let's call it `ServeManyConnections`) and handles handing them off to subprocesses. This one is tricky because it going to be platform specific. I'm not clear on whether it should be a single function with embedded ifdefs or two implementations of a single function, but I think it should be as small as possible so that the platform specificities really stand out. I would try to implement it as something like this (where the two functions in the while loop are platform-specific code): ``` void ServeManyConnections(...) { while (conn = AcceptNewConnection()) { ServeInSubprocess(conn); ReapSubprocessesIfNeeded(); } } ``` ... but I can also image other schemes. (BTW, I don't think you can get rid of the waitpid calls completely, as you need to reap the zombies even if you're not going to do anything with them). Then I think we can structure the main (or something main-like) function as something like: ``` int main() { ... if (getting_connection_from_parent) return AcceptConnectionFromParent(...); // retrieves the connection and calls ServeSingleConnection if (!server_mode) // no --server flag return ServeSingleConnection(...) return ServeManyConnections(...); } ``` I know this is handwavy, but that's sort of the goal I think we should aim for. It will get messier once all of the arguments (which I've conveniently ignored here) get added, but feel free to add structs or classes to group things as they make sense. Once we have this, I think it will also be clearer which code (if any) we want to put in some common libraries. Doing everything at once also may not be the best idea -- I can totally see how a preparatory patch which just refactors things without adding windows support would make sense here. (Also @DavidSpickett, feel free to chime in, I shouldn't be the only one dictating conditions here.:) https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket (PR #101283)
labath wrote: We raced updating the PR, so I didn't see the new version. I think the new version is looking better. I'm going to comment in more detail soon. https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket (PR #101283)
slydiman wrote: @labath Please look at the updated patch. ServeSingleConnection() = client_handle() AcceptConnectionFromParent() = some code + client_handle() ServeInSubprocess() = spawn_process() lldb-platform.cpp is simple. Currently we don't see the whole picture before the part 2 of this patch. The socket sharing for `lldb-server gdbserver` from GDBRemoteCommunication::StartDebugserverProcess() is much more complicated. And we need to update the acceptor to listen 2 ports in the same thread. I think we should implement the new functionality first and then we can optimize it. https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [clang][AST] fix ast-print of extern with >=2 declarators, fixed, fixed (PR #98795)
AaronBallman wrote: Ping @JDevlieghere for lldb test suite assistance. As for ORC JIT, [the compiler-rt project](https://compiler-rt.llvm.org/) does not claim to support Windows to begin with, so it's unclear whether the bot should not be testing on Windows or whether compiler-rt needs to update their support claims. CC @vitalybuka for more opinions/help on this one (my preference is for Windows to be a first-class citizen with compiler-rt, but if nobody is willing to do the maintenance work, we should not let bots failing when testing compiler-rt on Windows be a blocking concern). https://github.com/llvm/llvm-project/pull/98795 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][test] Update Makefile.rules to support Windows host+Linux target (PR #99266)
dzhidzhoev wrote: Thank you! https://github.com/llvm/llvm-project/pull/99266 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] a0fa9a3 - [LLDB][test] Update Makefile.rules to support Windows host+Linux target (#99266)
Author: Vladislav Dzhidzhoev Date: 2024-08-06T15:07:12+02:00 New Revision: a0fa9a308d20786ceb63b5d021c7f643ea2ef1c2 URL: https://github.com/llvm/llvm-project/commit/a0fa9a308d20786ceb63b5d021c7f643ea2ef1c2 DIFF: https://github.com/llvm/llvm-project/commit/a0fa9a308d20786ceb63b5d021c7f643ea2ef1c2.diff LOG: [LLDB][test] Update Makefile.rules to support Windows host+Linux target (#99266) These changes aim to support cross-compilation build on Windows host for Linux target for API tests execution. They're not final: changes will follow for refactoring and adjustments to make all tests pass. Chocolatey make is recommended to be used since it is maintained better than GnuWin32 mentioned here https://lldb.llvm.org/resources/build.html#windows (latest GnuWin32 release is dated by 2010) and helps to avoid problems with building tests (for example, GnuWin32 make doesn't support long paths and there are some other failures with building for Linux with it). Co-authored-by: Pavel Labath Added: Modified: lldb/packages/Python/lldbsuite/test/make/Makefile.rules Removed: diff --git a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules index be3ad684dd736..629ccee32e840 100644 --- a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules +++ b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules @@ -112,7 +112,7 @@ $(error "C compiler is not specified. Please run tests through lldb-dotest or li endif #-- -# Handle SDKROOT on Darwin +# Handle SDKROOT for the cross platform builds. #-- ifeq "$(OS)" "Darwin" @@ -120,6 +120,18 @@ ifeq "$(OS)" "Darwin" # We haven't otherwise set the SDKROOT, so set it now to macosx SDKROOT := $(shell xcrun --sdk macosx --show-sdk-path) endif +SYSROOT_FLAGS := -isysroot "$(SDKROOT)" +GCC_TOOLCHAIN_FLAGS := +else +ifneq "$(SDKROOT)" "" +SYSROOT_FLAGS := --sysroot "$(SDKROOT)" +GCC_TOOLCHAIN_FLAGS := --gcc-toolchain="$(SDKROOT)/usr" +else +# Do not set up these options if SDKROOT was not specified. +# This is a regular build in that case (or Android). +SYSROOT_FLAGS := +GCC_TOOLCHAIN_FLAGS := +endif endif #-- @@ -210,20 +222,15 @@ endif DEBUG_INFO_FLAG ?= -g CFLAGS ?= $(DEBUG_INFO_FLAG) -O0 - -ifeq "$(OS)" "Darwin" - ifneq "$(SDKROOT)" "" - CFLAGS += -isysroot "$(SDKROOT)" - endif -endif +CFLAGS += $(SYSROOT_FLAGS) ifeq "$(OS)" "Darwin" CFLAGS += $(ARCHFLAG) $(ARCH) $(FRAMEWORK_INCLUDES) else CFLAGS += $(ARCHFLAG)$(ARCH) endif -CFLAGS += -I$(LLDB_BASE_DIR)include -I$(LLDB_OBJ_ROOT)/include +CFLAGS += -I$(LLDB_BASE_DIR)/include -I$(LLDB_OBJ_ROOT)/include CFLAGS += -I$(SRCDIR) -I$(THIS_FILE_DIR) ifndef NO_TEST_COMMON_H @@ -234,9 +241,9 @@ CFLAGS += $(NO_LIMIT_DEBUG_INFO_FLAGS) $(ARCH_CFLAGS) # Use this one if you want to build one part of the result without debug information: ifeq "$(OS)" "Darwin" - CFLAGS_NO_DEBUG = -O0 $(ARCHFLAG) $(ARCH) $(FRAMEWORK_INCLUDES) $(ARCH_CFLAGS) $(CFLAGS_EXTRAS) -isysroot "$(SDKROOT)" + CFLAGS_NO_DEBUG = -O0 $(ARCHFLAG) $(ARCH) $(FRAMEWORK_INCLUDES) $(ARCH_CFLAGS) $(CFLAGS_EXTRAS) $(SYSROOT_FLAGS) else - CFLAGS_NO_DEBUG = -O0 $(ARCHFLAG)$(ARCH) $(FRAMEWORK_INCLUDES) $(ARCH_CFLAGS) $(CFLAGS_EXTRAS) + CFLAGS_NO_DEBUG = -O0 $(ARCHFLAG)$(ARCH) $(FRAMEWORK_INCLUDES) $(ARCH_CFLAGS) $(CFLAGS_EXTRAS) $(SYSROOT_FLAGS) endif ifeq "$(MAKE_DWO)" "YES" @@ -267,7 +274,9 @@ endif CFLAGS += $(CFLAGS_EXTRAS) CXXFLAGS += -std=c++11 $(CFLAGS) $(ARCH_CXXFLAGS) LD = $(CC) -LDFLAGS ?= $(CFLAGS) +# Copy common options to the linker flags (dwarf, arch. & etc). +# Note: we get some 'garbage' options for linker here (such as -I, --isystem & etc). +LDFLAGS += $(CFLAGS) LDFLAGS += $(LD_EXTRAS) $(ARCH_LDFLAGS) ifeq (,$(filter $(OS), Windows_NT Android Darwin)) ifneq (,$(filter YES,$(ENABLE_THREADS))) @@ -378,11 +387,26 @@ ifeq (1, $(USE_SYSTEM_STDLIB)) endif endif +ifeq (,$(filter 1, $(USE_LIBSTDCPP) $(USE_LIBCPP) $(USE_SYSTEM_STDLIB))) + # If no explicit C++ library request was made, but we have paths to a custom libcxx, use + # them. Otherwise, use the system library by default. + ifneq ($(and $(LIBCPP_INCLUDE_DIR), $(LIBCPP_LIBRARY_DIR)),) +CXXFLAGS += -nostdlib++ -nostdinc++ -cxx-isystem $(LIBCPP_INCLUDE_DIR) +ifneq "$(LIBCPP_INCLUDE_TARGET_DIR)" "" + CXXFLAGS += -cxx-isystem $(LIBCPP_INCLUDE_TARGET_DIR) +endif +LDFLAGS += -L$(LIBCPP_LIBRARY_DIR) -Wl,-rpath,$(LIBCPP_LIBRARY_DIR) -lc++ + else +USE_SYSTEM_STDLIB := 1 + endif +endif + ifeq (1,$(USE_LIBSTDCPP)) #
[Lldb-commits] [lldb] [LLDB][test] Update Makefile.rules to support Windows host+Linux target (PR #99266)
https://github.com/dzhidzhoev closed https://github.com/llvm/llvm-project/pull/99266 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix crash when adding members to an "incomplete" type (PR #102116)
@@ -0,0 +1,23 @@ +// RUN: %clangxx --target=x86_64-pc-linux -o %t -c %s -g labath wrote: > This would only crash with -flimit-debug-info right? Correct. The type definition needs to be absent from the binary and the ~only way to achieve that is to compile at least a part of it with -flimit-debug-info. I will add that explicitly. > Not sure what the intent behind the x86 directory is but it would be nice to > get the coverage when running this on ARM machines too The x86 directory is conditionalized on the x86 target, so it will run whenever the local build supports the x86 architecture (ie `X86` is in `LLVM_TARGETS_TO_BUILD`), regardless of the actual host architecture. (i.e., it works the same as as `x86` directories in llvm) The goal here isn't so much to hardcode the architecture (to x86), as much it is to hardcode the file format (to non-windows). We don't support working with unlinked object files on windows, and if I tried to link things then I'd need to depend on the linker and pull in other platform-specific stuff, which would make the test non-minimal. The thing I like about a test like this is that it (again, like the llvm tests) runs the same way everywhere, so there's little risk of needing to scrable to find a freebsd-mips (I'm exaggerating, I know) machine to reproduce a problem somewhere. (And I would totally love if we had more tests hardcoding aarch64-macosx triples so I can get more macos coverage without switching to a mac machine) https://github.com/llvm/llvm-project/pull/102116 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix crash when adding members to an "incomplete" type (PR #102116)
https://github.com/labath edited https://github.com/llvm/llvm-project/pull/102116 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix crash when adding members to an "incomplete" type (PR #102116)
https://github.com/labath updated https://github.com/llvm/llvm-project/pull/102116 >From c5373215793f744ab80b80e394354e455b415fcb Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Tue, 6 Aug 2024 11:36:31 +0200 Subject: [PATCH 1/2] [lldb] Fix crash when adding members to an "incomplete" type This fixes a regression caused by delayed type definition searching (#96755 and friends): If we end up adding a member (e.g. a typedef) to a type that we've already attempted to complete (and failed), the resulting AST would end up inconsistent (we would start to "forcibly" complete it, but never finish it), and importing it into an expression AST would crash. This patch fixes this by detecting the situation and finishing the definition as well. --- .../SymbolFile/DWARF/DWARFASTParserClang.cpp | 11 +++-- .../DWARF/x86/typedef-in-incomplete-type.cpp | 23 +++ 2 files changed, 32 insertions(+), 2 deletions(-) create mode 100644 lldb/test/Shell/SymbolFile/DWARF/x86/typedef-in-incomplete-type.cpp diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index a4dcde1629fc2..3968d5bf71e1c 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -269,8 +269,15 @@ static void PrepareContextToReceiveMembers(TypeSystemClang &ast, } // We don't have a type definition and/or the import failed, but we need to - // add members to it. Start the definition to make that possible. - tag_decl_ctx->startDefinition(); + // add members to it. Start the definition to make that possible. If the type + // has no external storage we also have to complete the definition. Otherwise, + // that will happen when we are asked to complete the type + // (CompleteTypeFromDWARF). + ast.StartTagDeclarationDefinition(type); + if (!tag_decl_ctx->hasExternalLexicalStorage()) { +ast.SetDeclIsForcefullyCompleted(tag_decl_ctx); +ast.CompleteTagDeclarationDefinition(type); + } } ParsedDWARFTypeAttributes::ParsedDWARFTypeAttributes(const DWARFDIE &die) { diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/typedef-in-incomplete-type.cpp b/lldb/test/Shell/SymbolFile/DWARF/x86/typedef-in-incomplete-type.cpp new file mode 100644 index 0..47ea1e2639c6e --- /dev/null +++ b/lldb/test/Shell/SymbolFile/DWARF/x86/typedef-in-incomplete-type.cpp @@ -0,0 +1,23 @@ +// RUN: %clangxx --target=x86_64-pc-linux -o %t -c %s -g +// RUN: %lldb %t -o "target var a" -o "expr -- var" -o exit | FileCheck %s + +// This forces lldb to attempt to complete the type A. Since it has no +// definition it will fail. +// CHECK: target var a +// CHECK: (A) a = + +// Now attempt to display the second variable, which will try to add a typedef +// to the incomplete type. Make sure that succeeds. Use the expression command +// to make sure the resulting AST can be imported correctly. +// CHECK: expr -- var +// CHECK: (A::X) $0 = 0 + +struct A { + // Declare the constructor, but don't define it to avoid emitting the + // definition in the debug info. + A(); + using X = int; +}; + +A a; +A::X var; >From 1a0a69810c2329fcb558592637bd39ee82262942 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Tue, 6 Aug 2024 15:11:14 +0200 Subject: [PATCH 2/2] add -flimit-debug-info --- .../Shell/SymbolFile/DWARF/x86/typedef-in-incomplete-type.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/typedef-in-incomplete-type.cpp b/lldb/test/Shell/SymbolFile/DWARF/x86/typedef-in-incomplete-type.cpp index 47ea1e2639c6e..591607784b0a9 100644 --- a/lldb/test/Shell/SymbolFile/DWARF/x86/typedef-in-incomplete-type.cpp +++ b/lldb/test/Shell/SymbolFile/DWARF/x86/typedef-in-incomplete-type.cpp @@ -1,4 +1,4 @@ -// RUN: %clangxx --target=x86_64-pc-linux -o %t -c %s -g +// RUN: %clangxx --target=x86_64-pc-linux -flimit-debug-info -o %t -c %s -g // RUN: %lldb %t -o "target var a" -o "expr -- var" -o exit | FileCheck %s // This forces lldb to attempt to complete the type A. Since it has no ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [clang][AST] fix ast-print of extern with >=2 declarators, fixed, fixed (PR #98795)
labath wrote: I don't have the full context but I can definitely run the lldb test suite for you. I'm building now. (BTW, I see there are some merge conflicts on the PR.) https://github.com/llvm/llvm-project/pull/98795 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [lldb] [clang] Reland: Instantiate alias templates with sugar (PR #101858)
gribozavr wrote: > So from my undertstanding, IWYU only needs the SubstTemplateTypeParmType for > resugaring purposes, in order to recover the type as written by the user. > > But with this patch we are doing the substitution already with sugar, so > there is no need to resugar, so IWYU shouldn't need the > SubstTemplateTypeParmType here anymore. I agree that we don't need `SubstTemplateTypeParmType` nodes if all resuraging that we ever do is related to types that the Clang frontend itself knows. However that is not universally true. For example, we (Google) have a tool for inferring and checking nullability contracts (https://github.com/google/crubit/tree/main/nullability). It relies on `SubstTemplateTypeParmType` nodes to propagate nullability annotations through template signatures. We can't rely on the Clang frontend doing this propagation because this logic is in an external tool. So for all practical purposes, the removal of `SubstTemplateTypeParmType` is losing relevant information, and it means that no other tool can do the things that Clang frontend can. https://github.com/llvm/llvm-project/pull/101858 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [clang][AST] fix ast-print of extern with >=2 declarators, fixed, fixed (PR #98795)
labath wrote: > I don't have the full context but I can definitely run the lldb test suite > for you. I'm building now. (BTW, I see there are some merge conflicts on the > PR.) I have the results now. The patch is based on a fairly old revision, so it didn't work for me without a python 3.12 fix (22ea97d7bfd65abf68a68b13bf96ad69be23df54). After patching that it, I got a clean check-lldb run (on x86_64-linux). https://github.com/llvm/llvm-project/pull/98795 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix crash when adding members to an "incomplete" type (PR #102116)
@@ -0,0 +1,23 @@ +// RUN: %clangxx --target=x86_64-pc-linux -o %t -c %s -g Michael137 wrote: Ahh got it I saw the tests in this directory weren't run on my M1 and misinterpreted that. ``` The thing I like about a test like this is that it (again, like the llvm tests) runs the same way everywhere, so there's little risk of needing to scrable to find a freebsd-mips (I'm exaggerating, I know) machine to reproduce a problem somewhere. ``` Agreed https://github.com/llvm/llvm-project/pull/102116 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix crash when adding members to an "incomplete" type (PR #102116)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/102116 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Docs] Add edit link to docs pages (PR #102144)
https://github.com/DavidSpickett created https://github.com/llvm/llvm-project/pull/102144 That aren't the generated `python_api/` pages. This button is a pencil icon at the top right of the page and takes you to a GitHub page where you can edit the content, assuming you have a fork already. If not it tells you how to make one. This is hardcoded to the llvm-project URL and main branch. So folks will need a downstream patch if they want to change that. For the upstream repo, main is right because even if a release branch was open for PRs, it would only be for cherry picks from main. The icon isn't as obvious as the "edit on GitHub" icons seen elsewhere but it's built in, and we could change it later if we wanted to. >From 8fddf6f57dcb4bcdef78139f49718fcc2ea0fd20 Mon Sep 17 00:00:00 2001 From: David Spickett Date: Tue, 6 Aug 2024 13:25:32 +0100 Subject: [PATCH] [lldb][Docs] Add edit link to docs pages That aren't the generated `python_api/` pages. This button is a pencil icon at the top right of the page and takes you to a GitHub page where you can edit the content, assuming you have a fork already. If not it tells you how to make one. This is hardcoded to the llvm-project URL and main branch. So folks will need a downstream patch if they want to change that. For the upstream repo, main is right because even if a release branch was open for PRs, it would only be for cherry picks from main. The icon isn't as obvious as the "edit on GitHub" icons seen elsewhere but it's built in, and we could change it later if we wanted to. --- lldb/docs/_templates/components/edit-this-page.html | 9 + lldb/docs/conf.py | 6 +- 2 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 lldb/docs/_templates/components/edit-this-page.html diff --git a/lldb/docs/_templates/components/edit-this-page.html b/lldb/docs/_templates/components/edit-this-page.html new file mode 100644 index 0..671caeac52f6d --- /dev/null +++ b/lldb/docs/_templates/components/edit-this-page.html @@ -0,0 +1,9 @@ +{% extends "furo/components/edit-this-page.html" %} + +{% block link_available -%} +{%- if pagename.startswith("python_api") -%} + +{%- else -%} + {{ furo_edit_button(determine_page_edit_link()) }} +{%- endif -%} +{%- endblock %} \ No newline at end of file diff --git a/lldb/docs/conf.py b/lldb/docs/conf.py index 9b005c9537f23..7c9e2acd41457 100644 --- a/lldb/docs/conf.py +++ b/lldb/docs/conf.py @@ -166,7 +166,11 @@ # Theme options are theme-specific and customize the look and feel of a theme # further. For a list of options available for each theme, see the # documentation. -html_theme_options = {} +html_theme_options = { + "source_repository": "https://github.com/llvm/llvm-project";, + "source_branch": "main", + "source_directory": "lldb/docs/", +} # Add any paths that contain custom themes here, relative to this directory. # html_theme_path = [] ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Docs] Add edit link to docs pages (PR #102144)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: David Spickett (DavidSpickett) Changes That aren't the generated `python_api/` pages. This button is a pencil icon at the top right of the page and takes you to a GitHub page where you can edit the content, assuming you have a fork already. If not it tells you how to make one. This is hardcoded to the llvm-project URL and main branch. So folks will need a downstream patch if they want to change that. For the upstream repo, main is right because even if a release branch was open for PRs, it would only be for cherry picks from main. The icon isn't as obvious as the "edit on GitHub" icons seen elsewhere but it's built in, and we could change it later if we wanted to. --- Full diff: https://github.com/llvm/llvm-project/pull/102144.diff 2 Files Affected: - (added) lldb/docs/_templates/components/edit-this-page.html (+9) - (modified) lldb/docs/conf.py (+5-1) ``diff diff --git a/lldb/docs/_templates/components/edit-this-page.html b/lldb/docs/_templates/components/edit-this-page.html new file mode 100644 index 0..671caeac52f6d --- /dev/null +++ b/lldb/docs/_templates/components/edit-this-page.html @@ -0,0 +1,9 @@ +{% extends "furo/components/edit-this-page.html" %} + +{% block link_available -%} +{%- if pagename.startswith("python_api") -%} + +{%- else -%} + {{ furo_edit_button(determine_page_edit_link()) }} +{%- endif -%} +{%- endblock %} \ No newline at end of file diff --git a/lldb/docs/conf.py b/lldb/docs/conf.py index 9b005c9537f23..7c9e2acd41457 100644 --- a/lldb/docs/conf.py +++ b/lldb/docs/conf.py @@ -166,7 +166,11 @@ # Theme options are theme-specific and customize the look and feel of a theme # further. For a list of options available for each theme, see the # documentation. -html_theme_options = {} +html_theme_options = { + "source_repository": "https://github.com/llvm/llvm-project";, + "source_branch": "main", + "source_directory": "lldb/docs/", +} # Add any paths that contain custom themes here, relative to this directory. # html_theme_path = [] `` https://github.com/llvm/llvm-project/pull/102144 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Docs] Add edit link to docs pages (PR #102144)
DavidSpickett wrote: https://github.com/pradyunsg/furo/blob/main/src/furo/theme/furo/components/edit-this-page.html is the file that gets extended. https://github.com/llvm/llvm-project/pull/102144 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Docs] Add edit link to docs pages (PR #102144)
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 4c670b266a10d613a58b71cc9c3d3a21cb6dc36b...8fddf6f57dcb4bcdef78139f49718fcc2ea0fd20 lldb/docs/conf.py `` View the diff from darker here. ``diff --- conf.py 2024-08-06 13:47:48.00 + +++ conf.py 2024-08-06 13:58:14.091861 + @@ -165,13 +165,13 @@ # Theme options are theme-specific and customize the look and feel of a theme # further. For a list of options available for each theme, see the # documentation. html_theme_options = { - "source_repository": "https://github.com/llvm/llvm-project";, - "source_branch": "main", - "source_directory": "lldb/docs/", +"source_repository": "https://github.com/llvm/llvm-project";, +"source_branch": "main", +"source_directory": "lldb/docs/", } # Add any paths that contain custom themes here, relative to this directory. # html_theme_path = [] `` https://github.com/llvm/llvm-project/pull/102144 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Docs] Add edit link to docs pages (PR #102144)
https://github.com/DavidSpickett updated https://github.com/llvm/llvm-project/pull/102144 >From 8fddf6f57dcb4bcdef78139f49718fcc2ea0fd20 Mon Sep 17 00:00:00 2001 From: David Spickett Date: Tue, 6 Aug 2024 13:25:32 +0100 Subject: [PATCH 1/2] [lldb][Docs] Add edit link to docs pages That aren't the generated `python_api/` pages. This button is a pencil icon at the top right of the page and takes you to a GitHub page where you can edit the content, assuming you have a fork already. If not it tells you how to make one. This is hardcoded to the llvm-project URL and main branch. So folks will need a downstream patch if they want to change that. For the upstream repo, main is right because even if a release branch was open for PRs, it would only be for cherry picks from main. The icon isn't as obvious as the "edit on GitHub" icons seen elsewhere but it's built in, and we could change it later if we wanted to. --- lldb/docs/_templates/components/edit-this-page.html | 9 + lldb/docs/conf.py | 6 +- 2 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 lldb/docs/_templates/components/edit-this-page.html diff --git a/lldb/docs/_templates/components/edit-this-page.html b/lldb/docs/_templates/components/edit-this-page.html new file mode 100644 index 0..671caeac52f6d --- /dev/null +++ b/lldb/docs/_templates/components/edit-this-page.html @@ -0,0 +1,9 @@ +{% extends "furo/components/edit-this-page.html" %} + +{% block link_available -%} +{%- if pagename.startswith("python_api") -%} + +{%- else -%} + {{ furo_edit_button(determine_page_edit_link()) }} +{%- endif -%} +{%- endblock %} \ No newline at end of file diff --git a/lldb/docs/conf.py b/lldb/docs/conf.py index 9b005c9537f23..7c9e2acd41457 100644 --- a/lldb/docs/conf.py +++ b/lldb/docs/conf.py @@ -166,7 +166,11 @@ # Theme options are theme-specific and customize the look and feel of a theme # further. For a list of options available for each theme, see the # documentation. -html_theme_options = {} +html_theme_options = { + "source_repository": "https://github.com/llvm/llvm-project";, + "source_branch": "main", + "source_directory": "lldb/docs/", +} # Add any paths that contain custom themes here, relative to this directory. # html_theme_path = [] >From f50d20f27194497cd520940319384ed540c7278e Mon Sep 17 00:00:00 2001 From: David Spickett Date: Tue, 6 Aug 2024 14:59:26 +0100 Subject: [PATCH 2/2] format --- lldb/docs/conf.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lldb/docs/conf.py b/lldb/docs/conf.py index 7c9e2acd41457..50f4ab84f1ca6 100644 --- a/lldb/docs/conf.py +++ b/lldb/docs/conf.py @@ -167,9 +167,9 @@ # further. For a list of options available for each theme, see the # documentation. html_theme_options = { - "source_repository": "https://github.com/llvm/llvm-project";, - "source_branch": "main", - "source_directory": "lldb/docs/", +"source_repository": "https://github.com/llvm/llvm-project";, +"source_branch": "main", +"source_directory": "lldb/docs/", } # Add any paths that contain custom themes here, relative to this directory. ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Docs] Add edit link to docs pages (PR #102144)
https://github.com/medismailben approved this pull request. https://github.com/llvm/llvm-project/pull/102144 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystemClang] Pass ClangASTMetadata around by value (PR #102161)
https://github.com/Michael137 created https://github.com/llvm/llvm-project/pull/102161 This patch changes the return type of `GetMetadata` from a `ClangASTMetadata*` to a `std::optional`. Except for one call-site (`SetDeclIsForcefullyCompleted`), we never actually make use of the mutability of the returned metadata. And we never make use of the pointer-identity. By passing `ClangASTMetadata` by-value (the type is fairly small, size of 2 64-bit pointers) we'll avoid some questions surrounding the lifetimes/ownership/mutability of this metadata. For consistency, we also change the parameter to `SetMetadata` from `ClangASTMetadata&` to `ClangASTMetadata` (which is an NFC since we copy the data anyway). This came up after some changes we plan to make where we [create redeclaration chains for decls in the LLDB AST](https://github.com/llvm/llvm-project/pull/95100). We want to avoid having to dig out the canonical decl of the declaration chain for retrieving/setting the metadata. It should just be copied across all decls in the chain. This is easier to guarantee when everything is done by-value. >From 624022e5737e7c51aa4976928183fa099503437c Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Tue, 6 Aug 2024 15:16:56 +0100 Subject: [PATCH] [lldb][TypeSystemClang] Pass ClangASTMetadata around by value --- .../Clang/ClangASTImporter.cpp| 11 ++-- .../ExpressionParser/Clang/ClangASTImporter.h | 2 +- .../Clang/ClangUserExpression.cpp | 6 +-- .../AppleObjCRuntime/AppleObjCDeclVendor.cpp | 6 +-- .../TypeSystem/Clang/TypeSystemClang.cpp | 50 ++- .../TypeSystem/Clang/TypeSystemClang.h| 13 ++--- .../unittests/Symbol/TestClangASTImporter.cpp | 6 +-- 7 files changed, 48 insertions(+), 46 deletions(-) diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp index 44071d1ea71c7..f6fb446634b18 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp @@ -84,8 +84,7 @@ clang::Decl *ClangASTImporter::CopyDecl(clang::ASTContext *dst_ast, LLDB_LOG_ERROR(log, result.takeError(), "Couldn't import decl: {0}"); if (log) { lldb::user_id_t user_id = LLDB_INVALID_UID; - ClangASTMetadata *metadata = GetDeclMetadata(decl); - if (metadata) + if (auto metadata = GetDeclMetadata(decl)) user_id = metadata->GetUserID(); if (NamedDecl *named_decl = dyn_cast(decl)) @@ -950,7 +949,8 @@ bool ClangASTImporter::RequireCompleteType(clang::QualType type) { return true; } -ClangASTMetadata *ClangASTImporter::GetDeclMetadata(const clang::Decl *decl) { +std::optional +ClangASTImporter::GetDeclMetadata(const clang::Decl *decl) { DeclOrigin decl_origin = GetDeclOrigin(decl); if (decl_origin.Valid()) { @@ -1105,7 +1105,7 @@ ClangASTImporter::ASTImporterDelegate::ImportImpl(Decl *From) { // If we have a forcefully completed type, try to find an actual definition // for it in other modules. - const ClangASTMetadata *md = m_main.GetDeclMetadata(From); + auto md = m_main.GetDeclMetadata(From); auto *td = dyn_cast(From); if (td && md && md->IsForcefullyCompleted()) { Log *log = GetLog(LLDBLog::Expressions); @@ -1284,8 +1284,7 @@ void ClangASTImporter::ASTImporterDelegate::Imported(clang::Decl *from, } lldb::user_id_t user_id = LLDB_INVALID_UID; - ClangASTMetadata *metadata = m_main.GetDeclMetadata(from); - if (metadata) + if (auto metadata = m_main.GetDeclMetadata(from)) user_id = metadata->GetUserID(); if (log) { diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.h index bc962e544d2f1..6231f0fb25939 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.h +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.h @@ -189,7 +189,7 @@ class ClangASTImporter { /// is only a shallow clone that lacks any contents. void SetDeclOrigin(const clang::Decl *decl, clang::Decl *original_decl); - ClangASTMetadata *GetDeclMetadata(const clang::Decl *decl); + std::optional GetDeclMetadata(const clang::Decl *decl); // // Namespace maps diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp index 35038a56440d5..2933d90550ffe 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp @@ -219,9 +219,9 @@ void ClangUserExpression::ScanContext(ExecutionContext &exe_ctx, Status &err) { // whatever runtime the debug info says the object pointer belongs to. Do // that here. -ClangASTMetadata *metadata = -TypeSystemClang::DeclContextGetMetaData(decl_context, function_decl); -
[Lldb-commits] [lldb] [lldb][TypeSystemClang] Pass ClangASTMetadata around by value (PR #102161)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) Changes This patch changes the return type of `GetMetadata` from a `ClangASTMetadata*` to a `std::optional`. Except for one call-site (`SetDeclIsForcefullyCompleted`), we never actually make use of the mutability of the returned metadata. And we never make use of the pointer-identity. By passing `ClangASTMetadata` by-value (the type is fairly small, size of 2 64-bit pointers) we'll avoid some questions surrounding the lifetimes/ownership/mutability of this metadata. For consistency, we also change the parameter to `SetMetadata` from `ClangASTMetadata&` to `ClangASTMetadata` (which is an NFC since we copy the data anyway). This came up after some changes we plan to make where we [create redeclaration chains for decls in the LLDB AST](https://github.com/llvm/llvm-project/pull/95100). We want to avoid having to dig out the canonical decl of the declaration chain for retrieving/setting the metadata. It should just be copied across all decls in the chain. This is easier to guarantee when everything is done by-value. --- Full diff: https://github.com/llvm/llvm-project/pull/102161.diff 7 Files Affected: - (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp (+5-6) - (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.h (+1-1) - (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp (+3-3) - (modified) lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp (+2-4) - (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp (+27-23) - (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h (+7-6) - (modified) lldb/unittests/Symbol/TestClangASTImporter.cpp (+3-3) ``diff diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp index 44071d1ea71c7..f6fb446634b18 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp @@ -84,8 +84,7 @@ clang::Decl *ClangASTImporter::CopyDecl(clang::ASTContext *dst_ast, LLDB_LOG_ERROR(log, result.takeError(), "Couldn't import decl: {0}"); if (log) { lldb::user_id_t user_id = LLDB_INVALID_UID; - ClangASTMetadata *metadata = GetDeclMetadata(decl); - if (metadata) + if (auto metadata = GetDeclMetadata(decl)) user_id = metadata->GetUserID(); if (NamedDecl *named_decl = dyn_cast(decl)) @@ -950,7 +949,8 @@ bool ClangASTImporter::RequireCompleteType(clang::QualType type) { return true; } -ClangASTMetadata *ClangASTImporter::GetDeclMetadata(const clang::Decl *decl) { +std::optional +ClangASTImporter::GetDeclMetadata(const clang::Decl *decl) { DeclOrigin decl_origin = GetDeclOrigin(decl); if (decl_origin.Valid()) { @@ -1105,7 +1105,7 @@ ClangASTImporter::ASTImporterDelegate::ImportImpl(Decl *From) { // If we have a forcefully completed type, try to find an actual definition // for it in other modules. - const ClangASTMetadata *md = m_main.GetDeclMetadata(From); + auto md = m_main.GetDeclMetadata(From); auto *td = dyn_cast(From); if (td && md && md->IsForcefullyCompleted()) { Log *log = GetLog(LLDBLog::Expressions); @@ -1284,8 +1284,7 @@ void ClangASTImporter::ASTImporterDelegate::Imported(clang::Decl *from, } lldb::user_id_t user_id = LLDB_INVALID_UID; - ClangASTMetadata *metadata = m_main.GetDeclMetadata(from); - if (metadata) + if (auto metadata = m_main.GetDeclMetadata(from)) user_id = metadata->GetUserID(); if (log) { diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.h index bc962e544d2f1..6231f0fb25939 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.h +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.h @@ -189,7 +189,7 @@ class ClangASTImporter { /// is only a shallow clone that lacks any contents. void SetDeclOrigin(const clang::Decl *decl, clang::Decl *original_decl); - ClangASTMetadata *GetDeclMetadata(const clang::Decl *decl); + std::optional GetDeclMetadata(const clang::Decl *decl); // // Namespace maps diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp index 35038a56440d5..2933d90550ffe 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp @@ -219,9 +219,9 @@ void ClangUserExpression::ScanContext(ExecutionContext &exe_ctx, Status &err) { // whatever runtime the debug info says the object pointer belongs to. Do // that here. -ClangASTMetadata *metadata = -TypeSystemClang::DeclContextGetMeta
[Lldb-commits] [lldb] [lldb][TypeSystemClang] Pass ClangASTMetadata around by value (PR #102161)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/102161 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystemClang] Pass ClangASTMetadata around by value (PR #102161)
https://github.com/JDevlieghere edited https://github.com/llvm/llvm-project/pull/102161 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystemClang] Pass ClangASTMetadata around by value (PR #102161)
https://github.com/JDevlieghere approved this pull request. Makes sense. https://github.com/llvm/llvm-project/pull/102161 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystemClang] Pass ClangASTMetadata around by value (PR #102161)
@@ -219,9 +219,9 @@ void ClangUserExpression::ScanContext(ExecutionContext &exe_ctx, Status &err) { // whatever runtime the debug info says the object pointer belongs to. Do // that here. -ClangASTMetadata *metadata = -TypeSystemClang::DeclContextGetMetaData(decl_context, function_decl); -if (metadata && metadata->HasObjectPtr()) { +if (auto metadata = TypeSystemClang::DeclContextGetMetaData(decl_context, JDevlieghere wrote: Nit: I know everyone has a different opinion on `auto`, but to me it's not obvious from the method name what the return type is, let alone that it's wrapped in an optional. https://github.com/llvm/llvm-project/pull/102161 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Make sure that a `Progress` "completed" update is always reported at destruction (PR #102097)
https://github.com/JDevlieghere approved this pull request. LGTM. Could we cover this scenario by the existing unit test? https://github.com/llvm/llvm-project/pull/102097 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][ClangExpressionParser] Set BuiltinHeadersInSystemModules depending on SDK version (PR #101778)
@@ -561,7 +562,85 @@ static void SetupLangOpts(CompilerInstance &compiler, lang_opts.NoBuiltin = true; } -static void SetupImportStdModuleLangOpts(CompilerInstance &compiler) { +// NOTE: should be kept in sync with sdkSupportsBuiltinModules in +// Toolchains/Darwin.cpp +static bool +sdkSupportsBuiltinModulesImpl(const llvm::Triple &triple, JDevlieghere wrote: Yeah, even if it's a static method it'd be good to centralize that logic. If someone needs something similar in the future, it's the first place they'd look. https://github.com/llvm/llvm-project/pull/101778 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystemClang] Pass ClangASTMetadata around by value (PR #102161)
@@ -219,9 +219,9 @@ void ClangUserExpression::ScanContext(ExecutionContext &exe_ctx, Status &err) { // whatever runtime the debug info says the object pointer belongs to. Do // that here. -ClangASTMetadata *metadata = -TypeSystemClang::DeclContextGetMetaData(decl_context, function_decl); -if (metadata && metadata->HasObjectPtr()) { +if (auto metadata = TypeSystemClang::DeclContextGetMetaData(decl_context, Michael137 wrote: Yea that's fair, i don't mind adding back the typenames https://github.com/llvm/llvm-project/pull/102161 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][ClangExpressionParser] Set BuiltinHeadersInSystemModules depending on SDK version (PR #101778)
@@ -561,7 +562,85 @@ static void SetupLangOpts(CompilerInstance &compiler, lang_opts.NoBuiltin = true; } -static void SetupImportStdModuleLangOpts(CompilerInstance &compiler) { +// NOTE: should be kept in sync with sdkSupportsBuiltinModules in +// Toolchains/Darwin.cpp +static bool +sdkSupportsBuiltinModulesImpl(const llvm::Triple &triple, Michael137 wrote: Tbh I'd prefer keeping this as an implementation detail. We really don't want to duplicate this logic. And can hopefully remove it eventually. If it lives as an API in a header that has a slight chance of making it harder to remove. But lmk if either of you feel strongly about it. https://github.com/llvm/llvm-project/pull/101778 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][ClangExpressionParser] Set BuiltinHeadersInSystemModules depending on SDK version (PR #101778)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/101778 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystemClang] Pass ClangASTMetadata around by value (PR #102161)
https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/102161 >From 624022e5737e7c51aa4976928183fa099503437c Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Tue, 6 Aug 2024 15:16:56 +0100 Subject: [PATCH 1/2] [lldb][TypeSystemClang] Pass ClangASTMetadata around by value --- .../Clang/ClangASTImporter.cpp| 11 ++-- .../ExpressionParser/Clang/ClangASTImporter.h | 2 +- .../Clang/ClangUserExpression.cpp | 6 +-- .../AppleObjCRuntime/AppleObjCDeclVendor.cpp | 6 +-- .../TypeSystem/Clang/TypeSystemClang.cpp | 50 ++- .../TypeSystem/Clang/TypeSystemClang.h| 13 ++--- .../unittests/Symbol/TestClangASTImporter.cpp | 6 +-- 7 files changed, 48 insertions(+), 46 deletions(-) diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp index 44071d1ea71c7..f6fb446634b18 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp @@ -84,8 +84,7 @@ clang::Decl *ClangASTImporter::CopyDecl(clang::ASTContext *dst_ast, LLDB_LOG_ERROR(log, result.takeError(), "Couldn't import decl: {0}"); if (log) { lldb::user_id_t user_id = LLDB_INVALID_UID; - ClangASTMetadata *metadata = GetDeclMetadata(decl); - if (metadata) + if (auto metadata = GetDeclMetadata(decl)) user_id = metadata->GetUserID(); if (NamedDecl *named_decl = dyn_cast(decl)) @@ -950,7 +949,8 @@ bool ClangASTImporter::RequireCompleteType(clang::QualType type) { return true; } -ClangASTMetadata *ClangASTImporter::GetDeclMetadata(const clang::Decl *decl) { +std::optional +ClangASTImporter::GetDeclMetadata(const clang::Decl *decl) { DeclOrigin decl_origin = GetDeclOrigin(decl); if (decl_origin.Valid()) { @@ -1105,7 +1105,7 @@ ClangASTImporter::ASTImporterDelegate::ImportImpl(Decl *From) { // If we have a forcefully completed type, try to find an actual definition // for it in other modules. - const ClangASTMetadata *md = m_main.GetDeclMetadata(From); + auto md = m_main.GetDeclMetadata(From); auto *td = dyn_cast(From); if (td && md && md->IsForcefullyCompleted()) { Log *log = GetLog(LLDBLog::Expressions); @@ -1284,8 +1284,7 @@ void ClangASTImporter::ASTImporterDelegate::Imported(clang::Decl *from, } lldb::user_id_t user_id = LLDB_INVALID_UID; - ClangASTMetadata *metadata = m_main.GetDeclMetadata(from); - if (metadata) + if (auto metadata = m_main.GetDeclMetadata(from)) user_id = metadata->GetUserID(); if (log) { diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.h index bc962e544d2f1..6231f0fb25939 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.h +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.h @@ -189,7 +189,7 @@ class ClangASTImporter { /// is only a shallow clone that lacks any contents. void SetDeclOrigin(const clang::Decl *decl, clang::Decl *original_decl); - ClangASTMetadata *GetDeclMetadata(const clang::Decl *decl); + std::optional GetDeclMetadata(const clang::Decl *decl); // // Namespace maps diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp index 35038a56440d5..2933d90550ffe 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp @@ -219,9 +219,9 @@ void ClangUserExpression::ScanContext(ExecutionContext &exe_ctx, Status &err) { // whatever runtime the debug info says the object pointer belongs to. Do // that here. -ClangASTMetadata *metadata = -TypeSystemClang::DeclContextGetMetaData(decl_context, function_decl); -if (metadata && metadata->HasObjectPtr()) { +if (auto metadata = TypeSystemClang::DeclContextGetMetaData(decl_context, +function_decl); +metadata && metadata->HasObjectPtr()) { lldb::LanguageType language = metadata->GetObjectPtrLanguage(); if (language == lldb::eLanguageTypeC_plus_plus) { if (m_enforce_valid_object) { diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp index 6894cdccaf95a..2626c86a8cbf9 100644 --- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp +++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp @@ -398,9 +398,8 @@ bool AppleObjCDeclVendor::FinishDecl(clang::ObjCInterfaceDecl *interface_decl) { Log *log( GetLog(LLDBLog::Expressions)); // FIXME - a more appropriate log channel? - ClangAS
[Lldb-commits] [lldb] [lldb][debuginfod] Fix the DebugInfoD PR that caused issues when working with stripped binaries (PR #99362)
kevinfrei wrote: Ping @JDevlieghere https://github.com/llvm/llvm-project/pull/99362 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [lldb] [clang] Reland: Instantiate alias templates with sugar (PR #101858)
mizvekov wrote: > I agree that we don't need `SubstTemplateTypeParmType` nodes if all > resuraging that we ever do is related to types that the Clang frontend itself > knows. However that is not universally true. > > For example, we (Google) have a tool for inferring and checking nullability > contracts (https://github.com/google/crubit/tree/main/nullability), and a > tool for lifetime contracts > (https://github.com/google/crubit/tree/main/lifetime_annotations). These tool > rely on `SubstTemplateTypeParmType` nodes to propagate nullability and > lifetime annotations through template signatures. We can't rely on the Clang > frontend doing this propagation because this logic is in an external tool, > and the whole notion of nullability and lifetimes is absent in Clang's > upstream repository. > Can we extend Clang in other ways where this propagation can happen within our transforms? > To be clear: our tool right now is partially broken (when alias templates are > involved), and I am not sure how to fix it when `SubstTemplateTypeParmType` > nodes are not going to be available. To be clear, it has been broken as of a couple of years already for some cases, we already perform this 'Final' substitution when instantiating default arguments for templates, for example. This patch extends that to alias templates. As for my first question, can you think of an alternative way we can go about this? What would you need to be upstreamed in order for our own transforms to handle this for you? It sounds like it will be a lot of waste if we have our own resugarer within clang, but then you would be forced to implement your own external resugarer anyway. https://github.com/llvm/llvm-project/pull/101858 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [clang][AST] fix ast-print of extern with >=2 declarators, fixed, fixed (PR #98795)
temyurchenko wrote: > > I don't have the full context but I can definitely run the lldb test suite > > for you. I'm building now. (BTW, I see there are some merge conflicts on > > the PR.) > > I have the results now. The patch is based on a fairly old revision, so it > didn't work for me without a python 3.12 fix > ([22ea97d](https://github.com/llvm/llvm-project/commit/22ea97d7bfd65abf68a68b13bf96ad69be23df54)). > After patching that it, I got a clean check-lldb run (on x86_64-linux). Thank you so much! Glad to hear that. The PR hasn't been updated for a while, but I will fix the conflicts now https://github.com/llvm/llvm-project/pull/98795 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [clang][AST] fix ast-print of extern with >=2 declarators, fixed, fixed (PR #98795)
https://github.com/temyurchenko updated https://github.com/llvm/llvm-project/pull/98795 >From 24ee56b5a104bdc8434413458f23b2d271764c78 Mon Sep 17 00:00:00 2001 From: Artem Yurchenko <44875844+temyurche...@users.noreply.github.com> Date: Thu, 30 May 2024 16:18:47 -0400 Subject: [PATCH 1/4] [clang][AST] fix ast-print of `extern ` with >=2 declarators (#93131) Problem: the printer used to ignore all but the first declarator for unbraced language linkage declarators. Furthemore, that one would be printed without the final semicolon. Solution: for unbraced case we traverse all declarators via `VisitDeclContext`. Furthermore, in appropriate visitors we query for whether they are a part of the unbraced extern language linkage spec, and if so, print appropriately. --- clang/lib/AST/DeclPrinter.cpp | 55 ++- clang/test/AST/ast-print-language-linkage.cpp | 31 +++ 2 files changed, 72 insertions(+), 14 deletions(-) create mode 100644 clang/test/AST/ast-print-language-linkage.cpp diff --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp index 26773a69ab9acf..b8e0ef1b403587 100644 --- a/clang/lib/AST/DeclPrinter.cpp +++ b/clang/lib/AST/DeclPrinter.cpp @@ -633,7 +633,7 @@ static void printExplicitSpecifier(ExplicitSpecifier ES, llvm::raw_ostream &Out, Out << Proto; } -static void MaybePrintTagKeywordIfSupressingScopes(PrintingPolicy &Policy, +static void maybePrintTagKeywordIfSupressingScopes(PrintingPolicy &Policy, QualType T, llvm::raw_ostream &Out) { StringRef prefix = T->isClassType() ? "class " @@ -643,6 +643,22 @@ static void MaybePrintTagKeywordIfSupressingScopes(PrintingPolicy &Policy, Out << prefix; } +/// Return the language of the linkage spec of `D`, if applicable. +/// +/// \Return - "C" if `D` has been declared with unbraced `extern "C"` +/// - "C++" if `D` has been declared with unbraced `extern "C++"` +/// - nullptr in any other case +static const char *tryGetUnbracedLinkageLanguage(const Decl *D) { + const auto *SD = dyn_cast(D->getDeclContext()); + if (!SD || SD->hasBraces()) +return nullptr; + if (SD->getLanguage() == LinkageSpecLanguageIDs::C) +return "C"; + assert(SD->getLanguage() == LinkageSpecLanguageIDs::CXX && + "unknown language in linkage specification"); + return "C++"; +} + void DeclPrinter::VisitFunctionDecl(FunctionDecl *D) { if (!D->getDescribedFunctionTemplate() && !D->isFunctionTemplateSpecialization()) { @@ -662,6 +678,11 @@ void DeclPrinter::VisitFunctionDecl(FunctionDecl *D) { CXXConversionDecl *ConversionDecl = dyn_cast(D); CXXDeductionGuideDecl *GuideDecl = dyn_cast(D); if (!Policy.SuppressSpecifiers) { +if (const char *Lang = tryGetUnbracedLinkageLanguage(D)) { + // the "extern" specifier is implicit + assert(D->getStorageClass() == SC_None); + Out << "extern \"" << Lang << "\" "; +} switch (D->getStorageClass()) { case SC_None: break; case SC_Extern: Out << "extern "; break; @@ -807,7 +828,7 @@ void DeclPrinter::VisitFunctionDecl(FunctionDecl *D) { } if (!Policy.SuppressTagKeyword && Policy.SuppressScope && !Policy.SuppressUnwrittenScope) -MaybePrintTagKeywordIfSupressingScopes(Policy, AFT->getReturnType(), +maybePrintTagKeywordIfSupressingScopes(Policy, AFT->getReturnType(), Out); AFT->getReturnType().print(Out, Policy, Proto); Proto.clear(); @@ -932,6 +953,11 @@ void DeclPrinter::VisitVarDecl(VarDecl *D) { : D->getASTContext().getUnqualifiedObjCPointerType(D->getType()); if (!Policy.SuppressSpecifiers) { +if (const char *Lang = tryGetUnbracedLinkageLanguage(D)) { + // the "extern" specifier is implicit + assert(D->getStorageClass() == SC_None); + Out << "extern \"" << Lang << "\" "; +} StorageClass SC = D->getStorageClass(); if (SC != SC_None) Out << VarDecl::getStorageClassSpecifierString(SC) << " "; @@ -961,7 +987,7 @@ void DeclPrinter::VisitVarDecl(VarDecl *D) { if (!Policy.SuppressTagKeyword && Policy.SuppressScope && !Policy.SuppressUnwrittenScope) -MaybePrintTagKeywordIfSupressingScopes(Policy, T, Out); +maybePrintTagKeywordIfSupressingScopes(Policy, T, Out); printDeclType(T, (isa(D) && Policy.CleanUglifiedParameters && D->getIdentifier()) @@ -1064,6 +1090,8 @@ void DeclPrinter::VisitNamespaceAliasDecl(NamespaceAliasDecl *D) { void DeclPrinter::VisitEmptyDecl(EmptyDecl *D) { prettyPrintAttributes(D); + if (const char *Lang = tryGetUnbracedLinkageLanguage(D)) +Out << "extern \"" << Lang << "\";"; } void DeclPrinter::VisitCXXRecordDecl(CXXRecordDecl *D) { @@ -1136,22 +1164,21 @@ void DeclPrinter::VisitCXXRecordDecl(CXXRecordDecl *D) { } void DeclPrinter::VisitLinkageSpecDecl(
[Lldb-commits] [lldb] [lldb][TypeSystemClang] Pass ClangASTMetadata around by value (PR #102161)
https://github.com/adrian-prantl approved this pull request. https://github.com/llvm/llvm-project/pull/102161 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Make sure that a `Progress` "completed" update is always reported at destruction (PR #102097)
royitaqi wrote: > LGTM. Could we cover this scenario by the existing unit test? Yes, definitely. At the time of the patch I didn't find any existing unit test files by searching for "ProgressTest.cpp". Just now I realize there is the [ProgressReportTest.cpp](https://github.com/llvm/llvm-project/blob/main/lldb/unittests/Core/ProgressReportTest.cpp). I will read it and try to add a test for this scenario. https://github.com/llvm/llvm-project/pull/102097 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][ClangExpressionParser] Set BuiltinHeadersInSystemModules depending on SDK version (PR #101778)
Michael137 wrote: Talked to Adrian and we decided it's best if we landed this on the apple/llvm-project fork 6.0 branch. There's a larger refactor we'll do on llvm.org main instead (mainly re-using the PlatformDarwin and HostInfo utilities to get the correct SDK directory, instead of the DWARF support files). https://github.com/llvm/llvm-project/pull/101778 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][ClangExpressionParser] Set BuiltinHeadersInSystemModules depending on SDK version (PR #101778)
https://github.com/Michael137 closed https://github.com/llvm/llvm-project/pull/101778 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] New ThreadPlanSingleThreadTimeout to resolve potential deadlock in single thread stepping (PR #90930)
jimingham wrote: This patch almost certainly caused this asan failure: https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake-sanitized/581/consoleText Can you take a look? https://github.com/llvm/llvm-project/pull/90930 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] New ThreadPlanSingleThreadTimeout to resolve potential deadlock in single thread stepping (PR #90930)
adrian-prantl wrote: @jimingham Should we revert the patch until this is fixed or are there more patches depending on this? https://github.com/llvm/llvm-project/pull/90930 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Improve toolchain detection in Makefile.rules (PR #102185)
https://github.com/dzhidzhoev created https://github.com/llvm/llvm-project/pull/102185 This fix is based on a problem with cxx_compiler and cxx_linker macros on Windows. There was an issue with compiler detection in paths containing "icc". In such case, Makefile.rules thought it was provided with ICC compiler. To solve that, quotes from paths are removed, which may be added when arguments are passed from Python to make, the last element of the compiler's path is separated, taking into account platform path delimiter, and compiler type is extracted, taking into account possible cross-toolchain prefix. Paths for additional tools, like OBJCOPY, are initialized with tools built with LLVM if USE_LLVM_TOOLS is on, to achieve better portability for Windows. >From 5b3f0df1c35dc2f02d15da0e3222d6f5388e6f92 Mon Sep 17 00:00:00 2001 From: Vladislav Dzhidzhoev Date: Sat, 27 Jul 2024 02:39:32 +0200 Subject: [PATCH] [lldb][test] Improve toolchain detection in Makefile.rules This fix is based on a problem with cxx_compiler and cxx_linker macros on Windows. There was an issue with compiler detection in paths containing "icc". In such case, Makefile.rules thought it was provided with icc compiler. To solve that, quotes from paths are removed, that may be added when arguments are passed from Python to make, the last element of compiler's path is separated, taking into account platform path delimiter, and compiler type is extracted, with regard of possible cross-toolchain prefix. Paths for additional tools, like OBJCOPY, are initialized with tools built with LLVM if USE_LLVM_TOOLS is on, to achieve better portability for Windows. --- .../Python/lldbsuite/test/make/Makefile.rules | 168 -- 1 file changed, 115 insertions(+), 53 deletions(-) diff --git a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules index 629ccee32e840..8676531a8a5d9 100644 --- a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules +++ b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules @@ -55,7 +55,10 @@ LLDB_BASE_DIR := $(THIS_FILE_DIR)/../../../../../ # Also reset BUILDDIR value because "pwd" returns cygwin or msys path # which needs to be converted to windows path. #-- +path_wrapper = $(1) ifeq "$(HOST_OS)" "Windows_NT" + path_wrapper = $(subst \,/,$(1)) + # MinGW make gets $(windir) variable if launched from cmd.exe # and $(WINDIR) if launched from MSYS2. SHELL := $(or $(windir),$(WINDIR),C:\WINDOWS)\system32\cmd.exe @@ -111,6 +114,110 @@ ifeq "$(CC)" "" $(error "C compiler is not specified. Please run tests through lldb-dotest or lit") endif +# Remove all " and ' characters from the path. Also remove surrounding space chars. +cstrip = $(strip $(subst ",,$(subst ',,$(1 + +# We can get CC compiler string in the following formats: +# [] - such as 'xrun clang', 'xrun /usr/bin/clang' & etc +# +# Where could contain the following parts: +# [.] - sucn as 'clang', 'clang.exe' ('clamg-cl.exe'?) +# -[.] - such as 'armv7-linux-gnueabi-gcc' +# /[.]- such as '/usr/bin/clang', 'c:\path\to\compiler\clang,exe' +# /-[.]- such as '/usr/bin/clang', 'c:\path\to\compiler\clang,exe' + +# We need CC without anything around. +override CC := $(call path_wrapper,$(call cstrip,$(CC))) + +CC_EXT := $(suffix $(CC)) +# Compiler name without extension +CC_NAME := $(basename $(lastword $(subst /, ,$(CC +# A kind of compiler (canonical name): clang, gcc, cc & etc. +ifeq (,$(filter $(CC_NAME), clang-cl llvm-gcc)) + CCC := $(or $(lastword $(subst -, ,$(CC_NAME))), $(CC_NAME)) + # A triple prefix of compiler name: gcc + CC_PREFIX := $(if $(findstring -,$(CC_NAME)),$(subst -$(CCC),,$(CC_NAME)),) +else + CCC := $(CC_NAME) + CC_PREFIX := +endif + +# Any trace of path within CC? +# $(dir ) function returns './' for the empty strings, but we need to keep path empty. +# Use this function on CC only if a path was specified with CC. +ifneq (,$(findstring /,$(CC))) + CC_PATH := $(dir $(lastword $(CC))) + replace_cc_with = $(patsubst %/$(CC_NAME)$(CC_EXT),%/$(1)$(CC_EXT),$(CC)) +else + CC_PATH := + replace_cc_with = $(patsubst $(CC_NAME)$(CC_EXT),$(1)$(CC_EXT),$(CC)) +endif + +# Function returns the tool/compiler name including the triple (or whatnever) prefix +# if it is present in the original CC. +cname_with_prefix = $(if $(CC_PREFIX),$(CC_PREFIX)-$(1),$(1)) + +# These functions return the fully qualified tool name (compiler or whatnevet) based on the previously parsed CC, +# given a simple tool (clang, gcc, objcopy & etc) name as arg. +# The first arg is the simplyfied tool name +# The second arg is a path to the tool (CC_PATH otherwise) +toolpath_base = $(join $(or $(2),$(CC_PATH)),$(addsuffix $(EXE_EXT),$(1))) + +# A kind of C++ compiler.
[Lldb-commits] [lldb] [lldb][test] Improve toolchain detection in Makefile.rules (PR #102185)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Vladislav Dzhidzhoev (dzhidzhoev) Changes This fix is based on a problem with cxx_compiler and cxx_linker macros on Windows. There was an issue with compiler detection in paths containing "icc". In such case, Makefile.rules thought it was provided with ICC compiler. To solve that, quotes from paths are removed, which may be added when arguments are passed from Python to make, the last element of the compiler's path is separated, taking into account platform path delimiter, and compiler type is extracted, taking into account possible cross-toolchain prefix. Paths for additional tools, like OBJCOPY, are initialized with tools built with LLVM if USE_LLVM_TOOLS is on, to achieve better portability for Windows. --- Full diff: https://github.com/llvm/llvm-project/pull/102185.diff 1 Files Affected: - (modified) lldb/packages/Python/lldbsuite/test/make/Makefile.rules (+115-53) ``diff diff --git a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules index 629ccee32e8404..8676531a8a5d93 100644 --- a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules +++ b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules @@ -55,7 +55,10 @@ LLDB_BASE_DIR := $(THIS_FILE_DIR)/../../../../../ # Also reset BUILDDIR value because "pwd" returns cygwin or msys path # which needs to be converted to windows path. #-- +path_wrapper = $(1) ifeq "$(HOST_OS)" "Windows_NT" + path_wrapper = $(subst \,/,$(1)) + # MinGW make gets $(windir) variable if launched from cmd.exe # and $(WINDIR) if launched from MSYS2. SHELL := $(or $(windir),$(WINDIR),C:\WINDOWS)\system32\cmd.exe @@ -111,6 +114,110 @@ ifeq "$(CC)" "" $(error "C compiler is not specified. Please run tests through lldb-dotest or lit") endif +# Remove all " and ' characters from the path. Also remove surrounding space chars. +cstrip = $(strip $(subst ",,$(subst ',,$(1 + +# We can get CC compiler string in the following formats: +# [] - such as 'xrun clang', 'xrun /usr/bin/clang' & etc +# +# Where could contain the following parts: +# [.] - sucn as 'clang', 'clang.exe' ('clamg-cl.exe'?) +# -[.] - such as 'armv7-linux-gnueabi-gcc' +# /[.]- such as '/usr/bin/clang', 'c:\path\to\compiler\clang,exe' +# /-[.]- such as '/usr/bin/clang', 'c:\path\to\compiler\clang,exe' + +# We need CC without anything around. +override CC := $(call path_wrapper,$(call cstrip,$(CC))) + +CC_EXT := $(suffix $(CC)) +# Compiler name without extension +CC_NAME := $(basename $(lastword $(subst /, ,$(CC +# A kind of compiler (canonical name): clang, gcc, cc & etc. +ifeq (,$(filter $(CC_NAME), clang-cl llvm-gcc)) + CCC := $(or $(lastword $(subst -, ,$(CC_NAME))), $(CC_NAME)) + # A triple prefix of compiler name: gcc + CC_PREFIX := $(if $(findstring -,$(CC_NAME)),$(subst -$(CCC),,$(CC_NAME)),) +else + CCC := $(CC_NAME) + CC_PREFIX := +endif + +# Any trace of path within CC? +# $(dir ) function returns './' for the empty strings, but we need to keep path empty. +# Use this function on CC only if a path was specified with CC. +ifneq (,$(findstring /,$(CC))) + CC_PATH := $(dir $(lastword $(CC))) + replace_cc_with = $(patsubst %/$(CC_NAME)$(CC_EXT),%/$(1)$(CC_EXT),$(CC)) +else + CC_PATH := + replace_cc_with = $(patsubst $(CC_NAME)$(CC_EXT),$(1)$(CC_EXT),$(CC)) +endif + +# Function returns the tool/compiler name including the triple (or whatnever) prefix +# if it is present in the original CC. +cname_with_prefix = $(if $(CC_PREFIX),$(CC_PREFIX)-$(1),$(1)) + +# These functions return the fully qualified tool name (compiler or whatnevet) based on the previously parsed CC, +# given a simple tool (clang, gcc, objcopy & etc) name as arg. +# The first arg is the simplyfied tool name +# The second arg is a path to the tool (CC_PATH otherwise) +toolpath_base = $(join $(or $(2),$(CC_PATH)),$(addsuffix $(EXE_EXT),$(1))) + +# A kind of C++ compiler. Get the counterpart C++ compiler based on CC/CCC. +CXXC := $(strip $(if $(filter $(CCC), icc),icpc,\ +$(if $(filter $(CCC), llvm-gcc),llvm-g++,\ +$(if $(filter $(CCC), gcc),g++,\ +$(if $(filter $(CCC), cc),c++,\ +$(if $(filter $(CCC), clang),clang++,\ +$(CCC))) +# A name of C++ compiler including the prefix. +CXX_NAME := $(call cname_with_prefix,$(CXXC)) + +ifneq "$(LLVM_TOOLS_DIR)" "" +override LLVM_TOOLS_DIR := $(call path_wrapper,$(call cstrip,$(LLVM_TOOLS_DIR)))/ +endif + +ifeq "$(HOST_OS)" "Windows_NT" + wrap_quotes = $(QUOTE)$(1)$(QUOTE) + # This function enframes the full path with the platform specific quotes. This is necessary to run the c++ executable + # properly under 'sh' on Wind
[Lldb-commits] [lldb] [lldb][test] Improve toolchain detection in Makefile.rules (PR #102185)
https://github.com/dzhidzhoev edited https://github.com/llvm/llvm-project/pull/102185 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Improve toolchain detection in Makefile.rules (PR #102185)
https://github.com/dzhidzhoev edited https://github.com/llvm/llvm-project/pull/102185 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/Plugins] Introduce Scripted Platform Plugin (PR #99814)
@@ -1003,6 +1010,21 @@ class Platform : public PluginInterface { FileSpec GetModuleCacheRoot(); }; +class PlatformMetadata { +public: + PlatformMetadata(Debugger &debugger, const ScriptedMetadata metadata); + ~PlatformMetadata() = default; + + Debugger &GetDebugger() const { return m_debugger; } + const ScriptedMetadata GetScriptedMetadata() const { +return m_scripted_metadata; + } + +protected: + Debugger &m_debugger; jimingham wrote: I don't like that idea, a scripted platform is really really going to want to create scripted processes. That will be super awkward when the scripted platform and the scripted process can't share code because they are in different interpreters. I don't want to have to explain this to people trying to use the lldb extension points. I think a much simpler solution is that Debuggers have a list of Platforms which is where dynamically created platforms are registered, and which it consults first when resolving "platform for name". Debuggers really should be independent of one another or things get hard to reason about. For instance, it would be very strange to have `command script import` in debugger A make a new Platform show up in debugger B. So you want to keep platforms made by a debugger confined to that debugger, and having an "dynamic platforms" or whatever list in the debugger seems a pretty straightforward way of doing that. https://github.com/llvm/llvm-project/pull/99814 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/Plugins] Introduce Scripted Platform Plugin (PR #99814)
https://github.com/jimingham edited https://github.com/llvm/llvm-project/pull/99814 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Improve toolchain detection in Makefile.rules (PR #102185)
https://github.com/dzhidzhoev updated https://github.com/llvm/llvm-project/pull/102185 >From 5b3f0df1c35dc2f02d15da0e3222d6f5388e6f92 Mon Sep 17 00:00:00 2001 From: Vladislav Dzhidzhoev Date: Sat, 27 Jul 2024 02:39:32 +0200 Subject: [PATCH 1/2] [lldb][test] Improve toolchain detection in Makefile.rules This fix is based on a problem with cxx_compiler and cxx_linker macros on Windows. There was an issue with compiler detection in paths containing "icc". In such case, Makefile.rules thought it was provided with icc compiler. To solve that, quotes from paths are removed, that may be added when arguments are passed from Python to make, the last element of compiler's path is separated, taking into account platform path delimiter, and compiler type is extracted, with regard of possible cross-toolchain prefix. Paths for additional tools, like OBJCOPY, are initialized with tools built with LLVM if USE_LLVM_TOOLS is on, to achieve better portability for Windows. --- .../Python/lldbsuite/test/make/Makefile.rules | 168 -- 1 file changed, 115 insertions(+), 53 deletions(-) diff --git a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules index 629ccee32e8404..8676531a8a5d93 100644 --- a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules +++ b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules @@ -55,7 +55,10 @@ LLDB_BASE_DIR := $(THIS_FILE_DIR)/../../../../../ # Also reset BUILDDIR value because "pwd" returns cygwin or msys path # which needs to be converted to windows path. #-- +path_wrapper = $(1) ifeq "$(HOST_OS)" "Windows_NT" + path_wrapper = $(subst \,/,$(1)) + # MinGW make gets $(windir) variable if launched from cmd.exe # and $(WINDIR) if launched from MSYS2. SHELL := $(or $(windir),$(WINDIR),C:\WINDOWS)\system32\cmd.exe @@ -111,6 +114,110 @@ ifeq "$(CC)" "" $(error "C compiler is not specified. Please run tests through lldb-dotest or lit") endif +# Remove all " and ' characters from the path. Also remove surrounding space chars. +cstrip = $(strip $(subst ",,$(subst ',,$(1 + +# We can get CC compiler string in the following formats: +# [] - such as 'xrun clang', 'xrun /usr/bin/clang' & etc +# +# Where could contain the following parts: +# [.] - sucn as 'clang', 'clang.exe' ('clamg-cl.exe'?) +# -[.] - such as 'armv7-linux-gnueabi-gcc' +# /[.]- such as '/usr/bin/clang', 'c:\path\to\compiler\clang,exe' +# /-[.]- such as '/usr/bin/clang', 'c:\path\to\compiler\clang,exe' + +# We need CC without anything around. +override CC := $(call path_wrapper,$(call cstrip,$(CC))) + +CC_EXT := $(suffix $(CC)) +# Compiler name without extension +CC_NAME := $(basename $(lastword $(subst /, ,$(CC +# A kind of compiler (canonical name): clang, gcc, cc & etc. +ifeq (,$(filter $(CC_NAME), clang-cl llvm-gcc)) + CCC := $(or $(lastword $(subst -, ,$(CC_NAME))), $(CC_NAME)) + # A triple prefix of compiler name: gcc + CC_PREFIX := $(if $(findstring -,$(CC_NAME)),$(subst -$(CCC),,$(CC_NAME)),) +else + CCC := $(CC_NAME) + CC_PREFIX := +endif + +# Any trace of path within CC? +# $(dir ) function returns './' for the empty strings, but we need to keep path empty. +# Use this function on CC only if a path was specified with CC. +ifneq (,$(findstring /,$(CC))) + CC_PATH := $(dir $(lastword $(CC))) + replace_cc_with = $(patsubst %/$(CC_NAME)$(CC_EXT),%/$(1)$(CC_EXT),$(CC)) +else + CC_PATH := + replace_cc_with = $(patsubst $(CC_NAME)$(CC_EXT),$(1)$(CC_EXT),$(CC)) +endif + +# Function returns the tool/compiler name including the triple (or whatnever) prefix +# if it is present in the original CC. +cname_with_prefix = $(if $(CC_PREFIX),$(CC_PREFIX)-$(1),$(1)) + +# These functions return the fully qualified tool name (compiler or whatnevet) based on the previously parsed CC, +# given a simple tool (clang, gcc, objcopy & etc) name as arg. +# The first arg is the simplyfied tool name +# The second arg is a path to the tool (CC_PATH otherwise) +toolpath_base = $(join $(or $(2),$(CC_PATH)),$(addsuffix $(EXE_EXT),$(1))) + +# A kind of C++ compiler. Get the counterpart C++ compiler based on CC/CCC. +CXXC := $(strip $(if $(filter $(CCC), icc),icpc,\ +$(if $(filter $(CCC), llvm-gcc),llvm-g++,\ +$(if $(filter $(CCC), gcc),g++,\ +$(if $(filter $(CCC), cc),c++,\ +$(if $(filter $(CCC), clang),clang++,\ +$(CCC))) +# A name of C++ compiler including the prefix. +CXX_NAME := $(call cname_with_prefix,$(CXXC)) + +ifneq "$(LLVM_TOOLS_DIR)" "" +override LLVM_TOOLS_DIR := $(call path_wrapper,$(call cstrip,$(LLVM_TOOLS_DIR)))/ +endif + +ifeq "$(HOST_OS)" "Windows_NT" + wrap_quotes = $(QUOTE)$(1)$(QUOTE) + # This function enfram
[Lldb-commits] [lldb] New ThreadPlanSingleThreadTimeout to resolve potential deadlock in single thread stepping (PR #90930)
jimingham wrote: This is a pretty big patch, since it's only failing on ASAN bots, I'd give @jeffreytan81 a bit to see if it's something obvious before reverting that. reverts aren't 100% free, they can make cherrypicking a hassle, so... https://github.com/llvm/llvm-project/pull/90930 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][debuginfod] Fix the DebugInfoD PR that caused issues when working with stripped binaries (PR #99362)
https://github.com/JDevlieghere approved this pull request. https://github.com/llvm/llvm-project/pull/99362 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] e77ac42 - [lldb][debuginfod] Fix the DebugInfoD PR that caused issues when working with stripped binaries (#99362)
Author: Kevin Frei Date: 2024-08-06T11:06:04-07:00 New Revision: e77ac42bccb8c26bbf4b74d8e92eb09e7fa1b218 URL: https://github.com/llvm/llvm-project/commit/e77ac42bccb8c26bbf4b74d8e92eb09e7fa1b218 DIFF: https://github.com/llvm/llvm-project/commit/e77ac42bccb8c26bbf4b74d8e92eb09e7fa1b218.diff LOG: [lldb][debuginfod] Fix the DebugInfoD PR that caused issues when working with stripped binaries (#99362) @walter-erquinigo found the the [PR with testing and a fix for DebugInfoD](https://github.com/llvm/llvm-project/pull/98344) caused an issue when working with stripped binaries. The issue is that when you're working with split-dwarf, there are *3* possible files: The stripped binary the user is debugging, the "only-keep-debug" *or* unstripped binary, plus the `.dwp` file. The debuginfod plugin should provide the unstripped/OKD binary. However, if the debuginfod plugin fails, the default symbol locator plugin will just return the stripped binary, which doesn't help. So, to address that, the SymbolVendorELF code checks to see if the SymbolLocator's ExecutableObjectFile request returned the same file, and bails if that's the case. You can see the specific diff as the second commit in the PR. I'm investigating adding a test: I can't quite get a simple repro, and I'm unwilling to make any additional changes to Makefile.rules to this diff, for Pavlovian reasons. Added: lldb/test/API/debuginfod/Normal/Makefile lldb/test/API/debuginfod/Normal/TestDebuginfod.py lldb/test/API/debuginfod/Normal/main.c lldb/test/API/debuginfod/SplitDWARF/Makefile lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py lldb/test/API/debuginfod/SplitDWARF/main.c Modified: lldb/include/lldb/Host/Config.h.cmake lldb/packages/Python/lldbsuite/test/decorators.py lldb/packages/Python/lldbsuite/test/make/Makefile.rules lldb/source/API/SBDebugger.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp lldb/source/Plugins/SymbolLocator/CMakeLists.txt lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp Removed: diff --git a/lldb/include/lldb/Host/Config.h.cmake b/lldb/include/lldb/Host/Config.h.cmake index 3defa454f6d420..9e538534086a2b 100644 --- a/lldb/include/lldb/Host/Config.h.cmake +++ b/lldb/include/lldb/Host/Config.h.cmake @@ -33,6 +33,8 @@ #cmakedefine01 LLDB_ENABLE_LZMA +#cmakedefine01 LLVM_ENABLE_CURL + #cmakedefine01 LLDB_ENABLE_CURSES #cmakedefine01 CURSES_HAVE_NCURSES_CURSES_H diff --git a/lldb/packages/Python/lldbsuite/test/decorators.py b/lldb/packages/Python/lldbsuite/test/decorators.py index ecc7b81035f11f..0e8ca159efd55d 100644 --- a/lldb/packages/Python/lldbsuite/test/decorators.py +++ b/lldb/packages/Python/lldbsuite/test/decorators.py @@ -1053,6 +1053,10 @@ def _get_bool_config_skip_if_decorator(key): return unittest.skipIf(not have, "requires " + key) +def skipIfCurlSupportMissing(func): +return _get_bool_config_skip_if_decorator("curl")(func) + + def skipIfCursesSupportMissing(func): return _get_bool_config_skip_if_decorator("curses")(func) diff --git a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules index 629ccee32e8404..1ba3f843e87cf3 100644 --- a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules +++ b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules @@ -202,6 +202,12 @@ else ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES" DSYM = $(EXE).debug endif + + ifeq "$(MAKE_DWP)" "YES" + MAKE_DWO := YES + DWP_NAME = $(EXE).dwp + DYLIB_DWP_NAME = $(DYLIB_NAME).dwp + endif endif LIMIT_DEBUG_INFO_FLAGS = @@ -347,6 +353,17 @@ ifneq "$(OS)" "Darwin" OBJCOPY ?= $(call replace_cc_with,objcopy) ARCHIVER ?= $(call replace_cc_with,ar) + # Look for llvm-dwp or gnu dwp + DWP ?= $(call replace_cc_with,llvm-dwp) + ifeq ($(wildcard $(DWP)),) + DWP = $(call replace_cc_with,dwp) + ifeq ($(wildcard $(DWP)),) + DWP = $(shell command -v llvm-dwp 2> /dev/null) + ifeq ($(wildcard $(DWP)),) + DWP = $(shell command -v dwp 2> /dev/null) + endif + endif + endif override AR = $(ARCHIVER) endif @@ -526,6 +543,10 @@ ifneq "$(CXX)" "" endif endif +ifeq "$(GEN_GNU_BUILD_ID)" "YES" + LDFLAGS += -Wl,--build-id +endif + #-- # DYLIB_ONLY variable can be used to skip the building of a.out. # See the sections below regarding dSYM file as well as the building of @@ -564,11 +585,18 @@ else endif else ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES" +ifeq "$(SAVE_FULL_DEBUG_BINARY)" "YES" + cp "$(EXE)" "$(EXE).unstripped" +endif $(OBJCOPY) --on
[Lldb-commits] [lldb] [lldb][debuginfod] Fix the DebugInfoD PR that caused issues when working with stripped binaries (PR #99362)
https://github.com/JDevlieghere closed https://github.com/llvm/llvm-project/pull/99362 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] New ThreadPlanSingleThreadTimeout to resolve potential deadlock in single thread stepping (PR #90930)
jeffreytan81 wrote: @jimingham, @adrian-prantl , thanks for letting me know. Surprised that I did not get failure email. Do you know how I can reproduce the failure? https://github.com/llvm/llvm-project/pull/90930 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/101272 >From 78ab13e3da15832117a7e2f8f85090a63482ca41 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Tue, 30 Jul 2024 11:04:45 -0700 Subject: [PATCH 01/10] Squash 64b-memory-regions-minidump and take only llvm-changes --- llvm/include/llvm/BinaryFormat/Minidump.h | 12 llvm/include/llvm/Object/Minidump.h | 16 -- llvm/include/llvm/ObjectYAML/MinidumpYAML.h | 28 + llvm/lib/Object/Minidump.cpp| 19 ++-- llvm/lib/ObjectYAML/MinidumpEmitter.cpp | 11 +++ llvm/lib/ObjectYAML/MinidumpYAML.cpp| 34 + 6 files changed, 115 insertions(+), 5 deletions(-) diff --git a/llvm/include/llvm/BinaryFormat/Minidump.h b/llvm/include/llvm/BinaryFormat/Minidump.h index 9669303252615..8054e81322a92 100644 --- a/llvm/include/llvm/BinaryFormat/Minidump.h +++ b/llvm/include/llvm/BinaryFormat/Minidump.h @@ -74,6 +74,18 @@ struct MemoryDescriptor_64 { support::ulittle64_t StartOfMemoryRange; support::ulittle64_t DataSize; }; +static_assert(sizeof(MemoryDescriptor_64) == 16); + +struct MemoryListHeader { + support::ulittle32_t NumberOfMemoryRanges; +}; +static_assert(sizeof(MemoryListHeader) == 4); + +struct Memory64ListHeader { + support::ulittle64_t NumberOfMemoryRanges; + support::ulittle64_t BaseRVA; +}; +static_assert(sizeof(Memory64ListHeader) == 16); struct MemoryInfoListHeader { support::ulittle32_t SizeOfHeader; diff --git a/llvm/include/llvm/Object/Minidump.h b/llvm/include/llvm/Object/Minidump.h index e45d4de0090de..6ca9eb2afe150 100644 --- a/llvm/include/llvm/Object/Minidump.h +++ b/llvm/include/llvm/Object/Minidump.h @@ -103,6 +103,15 @@ class MinidumpFile : public Binary { minidump::StreamType::MemoryList); } + /// Returns the header to the memory 64 list stream. An error is returned if + /// the file does not contain this stream. + Expected getMemoryList64Header() const { +return getStream( +minidump::StreamType::Memory64List); + } + + Expected> getMemory64List() const; + class MemoryInfoIterator : public iterator_facade_base> getDataSlice(ArrayRef Data, - size_t Offset, size_t Size); + static Expected> + getDataSlice(ArrayRef Data, uint64_t Offset, uint64_t Size); /// Return the slice of the given data array as an array of objects of the /// given type. The function checks that the input array is large enough to /// contain the correct number of objects of the given type. template static Expected> getDataSliceAs(ArrayRef Data, - size_t Offset, size_t Count); + uint64_t Offset, uint64_t Count); MinidumpFile(MemoryBufferRef Source, const minidump::Header &Header, ArrayRef Streams, @@ -208,6 +217,7 @@ Expected> MinidumpFile::getDataSliceAs(ArrayRef Data, getDataSlice(Data, Offset, sizeof(T) * Count); if (!Slice) return Slice.takeError(); + return ArrayRef(reinterpret_cast(Slice->data()), Count); } diff --git a/llvm/include/llvm/ObjectYAML/MinidumpYAML.h b/llvm/include/llvm/ObjectYAML/MinidumpYAML.h index b0cee541cef20..2cccf7fba5853 100644 --- a/llvm/include/llvm/ObjectYAML/MinidumpYAML.h +++ b/llvm/include/llvm/ObjectYAML/MinidumpYAML.h @@ -29,6 +29,7 @@ struct Stream { Exception, MemoryInfoList, MemoryList, +Memory64List, ModuleList, RawContent, SystemInfo, @@ -104,6 +105,25 @@ using ModuleListStream = detail::ListStream; using ThreadListStream = detail::ListStream; using MemoryListStream = detail::ListStream; +/// Memory64ListStream minidump stream. +struct Memory64ListStream : public Stream { + minidump::Memory64ListHeader Header; + std::vector Entries; + yaml::BinaryRef Content; + + Memory64ListStream() + : Stream(StreamKind::Memory64List, minidump::StreamType::Memory64List) {} + + explicit Memory64ListStream( + std::vector Entries) + : Stream(StreamKind::Memory64List, minidump::StreamType::Memory64List), +Entries(Entries) {} + + static bool classof(const Stream *S) { +return S->Kind == StreamKind::Memory64List; + } +}; + /// ExceptionStream minidump stream. struct ExceptionStream : public Stream { minidump::ExceptionStream MDExceptionStream; @@ -244,6 +264,12 @@ template <> struct MappingContextTraits { BinaryRef &Content); }; +template <> +struct MappingContextTraits { + static void mapping(IO &IO, minidump::MemoryDescriptor_64 &Memory, + BinaryRef &Content); +}; + } // namespace yaml } // namespace llvm @@ -262,6 +288,7 @@ LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::minidump::CPUInfo::X86Info) LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::minidump::Exception) LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::minidump::MemoryInfo) LLVM_YAML_DECLARE_MAPPING_TRA
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -132,6 +140,79 @@ class MinidumpFile : public Binary { size_t Stride; }; +class Memory64Iterator { + public: +static Memory64Iterator begin(ArrayRef Storage, ArrayRef Descriptors, uint64_t BaseRVA) { + return Memory64Iterator(Storage, Descriptors, BaseRVA); +} + +static Memory64Iterator end() { + return Memory64Iterator(); +} + +std::pair> Current; + +bool operator==(const Memory64Iterator &R) const { + return isEnd == R.isEnd; +} + +bool operator!=(const Memory64Iterator &R) const { return !(*this == R); } + +const std::pair> &operator*() const { + return Current; +} + +const std::pair> *operator->() const { + return &Current; +} + +Error inc() { + if (Storage.size() == 0 || Descriptors.size() == 0) Jlalond wrote: @labath This is my last concern. I tried to base this off of `Archive.cpp`'s implementation of Fallible iterator, but if the iterator is dereferenced before being advanced we'll get a default value of {0, 0}. Currently you just have to use the `++It` operator in both classes, but I'm not sure if this is best practice. https://github.com/llvm/llvm-project/pull/101272 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] New ThreadPlanSingleThreadTimeout to resolve potential deadlock in single thread stepping (PR #90930)
jimingham wrote: The console logs for the bots usually have the CMake command used to configure the tree for the test. The one I referred to above has: '/usr/local/bin/cmake' '-G' 'Ninja' '/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-sanitized/llvm-project/llvm' '-DCMAKE_BUILD_TYPE=Release' '-DCMAKE_EXPORT_COMPILE_COMMANDS=ON' '-DLLVM_TARGETS_TO_BUILD=X86;ARM;AArch64' '-DCMAKE_INSTALL_PREFIX=/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-sanitized/lldb-install' '-DCMAKE_MAKE_PROGRAM=/usr/local/bin/ninja' '-DLLDB_TEST_USER_ARGS=--build-dir;/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-sanitized/lldb-build/lldb-test-build.noindex;-t;--env;TERM=vt100' '-DLLDB_ENFORCE_STRICT_TEST_REQUIREMENTS=On' '-DLLDB_ENABLE_PYTHON=On' '-DLLDB_ENABLE_LZMA=Off' '-DLIBCXX_HARDENING_MODE=none' '-DLLVM_ENABLE_ASSERTIONS:BOOL=TRUE' '-DLLVM_ENABLE_MODULES=Off' '-DLLVM_ENABLE_PROJECTS=clang;lld;lldb' '-DLLVM_LIT_ARGS=-v --time-tests --shuffle --xunit-xml-output=/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-sanitized/test/results.xml -v -j 3 --timeout 1200' '-DLLVM_VERSION_PATCH=99' '-DLLVM_ENABLE_RUNTIMES=libcxx;libcxxabi;libunwind' '-DLLVM_TARGETS_TO_BUILD=X86' '-DLLVM_USE_SANITIZER=Address;Undefined' I think the only part of that that's relevant for you is `-DLLVM_USE_SANITIZER=Address;Undefined` - you should be able to just add that to your CMakery, then run the test. https://github.com/llvm/llvm-project/pull/90930 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/Plugins] Introduce Scripted Platform Plugin (PR #99814)
JDevlieghere wrote: I haven't had a chance to review this, but I would appreciate if we can keep in mind how this might affect breaking up the platforms. Right now, the Platform abstraction covers two concepts: 1. How you connect to a platform and do things like transfer files, list processes, etc. 2. How to debug on a certain platform, which includes things like SDK, shared cache, etc We ran into this problem when adding the (downstream) `device` command. We had new way to talk to a device (using a private framework) and I really didn't want to create a new macOS, iOS, watchOS, etc platform just for that. So for Xcode 16 we worked around it, but I really want to go back and do it the right way in the near future. I think it should be possible to detangle these two concepts so I want to make sure this patch doesn't make that harder. https://github.com/llvm/llvm-project/pull/99814 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/101272 >From d28a238367aebb814be76749a3422a49963da8ac Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Tue, 30 Jul 2024 11:04:45 -0700 Subject: [PATCH 01/11] Squash 64b-memory-regions-minidump and take only llvm-changes --- llvm/include/llvm/BinaryFormat/Minidump.h | 12 llvm/include/llvm/Object/Minidump.h | 16 -- llvm/include/llvm/ObjectYAML/MinidumpYAML.h | 28 + llvm/lib/Object/Minidump.cpp| 19 ++-- llvm/lib/ObjectYAML/MinidumpEmitter.cpp | 11 +++ llvm/lib/ObjectYAML/MinidumpYAML.cpp| 34 + 6 files changed, 115 insertions(+), 5 deletions(-) diff --git a/llvm/include/llvm/BinaryFormat/Minidump.h b/llvm/include/llvm/BinaryFormat/Minidump.h index 9669303252615..8054e81322a92 100644 --- a/llvm/include/llvm/BinaryFormat/Minidump.h +++ b/llvm/include/llvm/BinaryFormat/Minidump.h @@ -74,6 +74,18 @@ struct MemoryDescriptor_64 { support::ulittle64_t StartOfMemoryRange; support::ulittle64_t DataSize; }; +static_assert(sizeof(MemoryDescriptor_64) == 16); + +struct MemoryListHeader { + support::ulittle32_t NumberOfMemoryRanges; +}; +static_assert(sizeof(MemoryListHeader) == 4); + +struct Memory64ListHeader { + support::ulittle64_t NumberOfMemoryRanges; + support::ulittle64_t BaseRVA; +}; +static_assert(sizeof(Memory64ListHeader) == 16); struct MemoryInfoListHeader { support::ulittle32_t SizeOfHeader; diff --git a/llvm/include/llvm/Object/Minidump.h b/llvm/include/llvm/Object/Minidump.h index e45d4de0090de..6ca9eb2afe150 100644 --- a/llvm/include/llvm/Object/Minidump.h +++ b/llvm/include/llvm/Object/Minidump.h @@ -103,6 +103,15 @@ class MinidumpFile : public Binary { minidump::StreamType::MemoryList); } + /// Returns the header to the memory 64 list stream. An error is returned if + /// the file does not contain this stream. + Expected getMemoryList64Header() const { +return getStream( +minidump::StreamType::Memory64List); + } + + Expected> getMemory64List() const; + class MemoryInfoIterator : public iterator_facade_base> getDataSlice(ArrayRef Data, - size_t Offset, size_t Size); + static Expected> + getDataSlice(ArrayRef Data, uint64_t Offset, uint64_t Size); /// Return the slice of the given data array as an array of objects of the /// given type. The function checks that the input array is large enough to /// contain the correct number of objects of the given type. template static Expected> getDataSliceAs(ArrayRef Data, - size_t Offset, size_t Count); + uint64_t Offset, uint64_t Count); MinidumpFile(MemoryBufferRef Source, const minidump::Header &Header, ArrayRef Streams, @@ -208,6 +217,7 @@ Expected> MinidumpFile::getDataSliceAs(ArrayRef Data, getDataSlice(Data, Offset, sizeof(T) * Count); if (!Slice) return Slice.takeError(); + return ArrayRef(reinterpret_cast(Slice->data()), Count); } diff --git a/llvm/include/llvm/ObjectYAML/MinidumpYAML.h b/llvm/include/llvm/ObjectYAML/MinidumpYAML.h index b0cee541cef20..2cccf7fba5853 100644 --- a/llvm/include/llvm/ObjectYAML/MinidumpYAML.h +++ b/llvm/include/llvm/ObjectYAML/MinidumpYAML.h @@ -29,6 +29,7 @@ struct Stream { Exception, MemoryInfoList, MemoryList, +Memory64List, ModuleList, RawContent, SystemInfo, @@ -104,6 +105,25 @@ using ModuleListStream = detail::ListStream; using ThreadListStream = detail::ListStream; using MemoryListStream = detail::ListStream; +/// Memory64ListStream minidump stream. +struct Memory64ListStream : public Stream { + minidump::Memory64ListHeader Header; + std::vector Entries; + yaml::BinaryRef Content; + + Memory64ListStream() + : Stream(StreamKind::Memory64List, minidump::StreamType::Memory64List) {} + + explicit Memory64ListStream( + std::vector Entries) + : Stream(StreamKind::Memory64List, minidump::StreamType::Memory64List), +Entries(Entries) {} + + static bool classof(const Stream *S) { +return S->Kind == StreamKind::Memory64List; + } +}; + /// ExceptionStream minidump stream. struct ExceptionStream : public Stream { minidump::ExceptionStream MDExceptionStream; @@ -244,6 +264,12 @@ template <> struct MappingContextTraits { BinaryRef &Content); }; +template <> +struct MappingContextTraits { + static void mapping(IO &IO, minidump::MemoryDescriptor_64 &Memory, + BinaryRef &Content); +}; + } // namespace yaml } // namespace llvm @@ -262,6 +288,7 @@ LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::minidump::CPUInfo::X86Info) LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::minidump::Exception) LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::minidump::MemoryInfo) LLVM_YAML_DECLARE_MAPPING_TRA
[Lldb-commits] [lldb] New ThreadPlanSingleThreadTimeout to resolve potential deadlock in single thread stepping (PR #90930)
jeffreytan81 wrote: I think I understand the lifetime issue that caused the ASAN failure. I will fix it. https://github.com/llvm/llvm-project/pull/90930 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/101272 >From d28a238367aebb814be76749a3422a49963da8ac Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Tue, 30 Jul 2024 11:04:45 -0700 Subject: [PATCH 01/12] Squash 64b-memory-regions-minidump and take only llvm-changes --- llvm/include/llvm/BinaryFormat/Minidump.h | 12 llvm/include/llvm/Object/Minidump.h | 16 -- llvm/include/llvm/ObjectYAML/MinidumpYAML.h | 28 + llvm/lib/Object/Minidump.cpp| 19 ++-- llvm/lib/ObjectYAML/MinidumpEmitter.cpp | 11 +++ llvm/lib/ObjectYAML/MinidumpYAML.cpp| 34 + 6 files changed, 115 insertions(+), 5 deletions(-) diff --git a/llvm/include/llvm/BinaryFormat/Minidump.h b/llvm/include/llvm/BinaryFormat/Minidump.h index 9669303252615..8054e81322a92 100644 --- a/llvm/include/llvm/BinaryFormat/Minidump.h +++ b/llvm/include/llvm/BinaryFormat/Minidump.h @@ -74,6 +74,18 @@ struct MemoryDescriptor_64 { support::ulittle64_t StartOfMemoryRange; support::ulittle64_t DataSize; }; +static_assert(sizeof(MemoryDescriptor_64) == 16); + +struct MemoryListHeader { + support::ulittle32_t NumberOfMemoryRanges; +}; +static_assert(sizeof(MemoryListHeader) == 4); + +struct Memory64ListHeader { + support::ulittle64_t NumberOfMemoryRanges; + support::ulittle64_t BaseRVA; +}; +static_assert(sizeof(Memory64ListHeader) == 16); struct MemoryInfoListHeader { support::ulittle32_t SizeOfHeader; diff --git a/llvm/include/llvm/Object/Minidump.h b/llvm/include/llvm/Object/Minidump.h index e45d4de0090de..6ca9eb2afe150 100644 --- a/llvm/include/llvm/Object/Minidump.h +++ b/llvm/include/llvm/Object/Minidump.h @@ -103,6 +103,15 @@ class MinidumpFile : public Binary { minidump::StreamType::MemoryList); } + /// Returns the header to the memory 64 list stream. An error is returned if + /// the file does not contain this stream. + Expected getMemoryList64Header() const { +return getStream( +minidump::StreamType::Memory64List); + } + + Expected> getMemory64List() const; + class MemoryInfoIterator : public iterator_facade_base> getDataSlice(ArrayRef Data, - size_t Offset, size_t Size); + static Expected> + getDataSlice(ArrayRef Data, uint64_t Offset, uint64_t Size); /// Return the slice of the given data array as an array of objects of the /// given type. The function checks that the input array is large enough to /// contain the correct number of objects of the given type. template static Expected> getDataSliceAs(ArrayRef Data, - size_t Offset, size_t Count); + uint64_t Offset, uint64_t Count); MinidumpFile(MemoryBufferRef Source, const minidump::Header &Header, ArrayRef Streams, @@ -208,6 +217,7 @@ Expected> MinidumpFile::getDataSliceAs(ArrayRef Data, getDataSlice(Data, Offset, sizeof(T) * Count); if (!Slice) return Slice.takeError(); + return ArrayRef(reinterpret_cast(Slice->data()), Count); } diff --git a/llvm/include/llvm/ObjectYAML/MinidumpYAML.h b/llvm/include/llvm/ObjectYAML/MinidumpYAML.h index b0cee541cef20..2cccf7fba5853 100644 --- a/llvm/include/llvm/ObjectYAML/MinidumpYAML.h +++ b/llvm/include/llvm/ObjectYAML/MinidumpYAML.h @@ -29,6 +29,7 @@ struct Stream { Exception, MemoryInfoList, MemoryList, +Memory64List, ModuleList, RawContent, SystemInfo, @@ -104,6 +105,25 @@ using ModuleListStream = detail::ListStream; using ThreadListStream = detail::ListStream; using MemoryListStream = detail::ListStream; +/// Memory64ListStream minidump stream. +struct Memory64ListStream : public Stream { + minidump::Memory64ListHeader Header; + std::vector Entries; + yaml::BinaryRef Content; + + Memory64ListStream() + : Stream(StreamKind::Memory64List, minidump::StreamType::Memory64List) {} + + explicit Memory64ListStream( + std::vector Entries) + : Stream(StreamKind::Memory64List, minidump::StreamType::Memory64List), +Entries(Entries) {} + + static bool classof(const Stream *S) { +return S->Kind == StreamKind::Memory64List; + } +}; + /// ExceptionStream minidump stream. struct ExceptionStream : public Stream { minidump::ExceptionStream MDExceptionStream; @@ -244,6 +264,12 @@ template <> struct MappingContextTraits { BinaryRef &Content); }; +template <> +struct MappingContextTraits { + static void mapping(IO &IO, minidump::MemoryDescriptor_64 &Memory, + BinaryRef &Content); +}; + } // namespace yaml } // namespace llvm @@ -262,6 +288,7 @@ LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::minidump::CPUInfo::X86Info) LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::minidump::Exception) LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::minidump::MemoryInfo) LLVM_YAML_DECLARE_MAPPING_TRA
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Support minidumps where there are multiple exception streams (PR #97470)
https://github.com/Jlalond closed https://github.com/llvm/llvm-project/pull/97470 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [llvm]Added lib/Telemetry (PR #98528)
@@ -0,0 +1,619 @@ + +//===-- Telemetry.cpp -===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// +#include "lldb/Core/Telemetry.h" + +#include +#include + +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "lldb/API/SBDebugger.h" +#include "lldb/API/SBProcess.h" +#include "lldb/Core/Debugger.h" +#include "lldb/Core/Module.h" +#include "lldb/Host/FileSystem.h" +#include "lldb/Host/HostInfo.h" +#include "lldb/Interpreter/CommandInterpreter.h" +#include "lldb/Target/Process.h" +#include "lldb/Target/Statistics.h" +#include "lldb/Utility/ConstString.h" +#include "lldb/Utility/FileSpec.h" +#include "lldb/Utility/UUID.h" +#include "lldb/Version/Version.h" +#include "lldb/lldb-enumerations.h" +#include "lldb/lldb-forward.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallString.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/ADT/Twine.h" +#include "llvm/Support/FileSystem.h" +#include "llvm/Support/LineIterator.h" +#include "llvm/Support/MemoryBuffer.h" +#include "llvm/Support/Path.h" +#include "llvm/Support/RandomNumberGenerator.h" +#include "llvm/Telemetry/Telemetry.h" + +#ifdef HAS_VENDOR_TELEMETRY_PLUGINS +// TODO: could make this path a build-variable rather than hard-coded +#include "lldb/Core/VendorTelemetryPlugin.h" + +// This must include definitions for these functions +extern void ApplyVendorSpecificConfigs( +llvm::telemetry::TelemetryConfig *config) /* __attribute__((weak))*/; +extern std::shared_ptr SanitizeSensitiveFields( +const llvm::telemetry::TelemetryInfo *entry) /*__attribute__((weak))*/; +extern std::shared_ptr +CreateVendorSpecificTelemeter( +llvm::telemetry::TelemetryConfig *config) /*__attribute__((weak))*/; +#endif + +namespace lldb_private { + +static std::string GetDuration(const TelemetryEventStats &stats) { + if (stats.m_end.has_value()) +return std::to_string((stats.m_end.value() - stats.m_start).count()) + + "(nanosec)"; + return ""; +} + +std::string DebuggerTelemetryInfo::ToString() const { + std::string duration_desc = + (exit_description.has_value() ? " lldb session duration: " +: " lldb startup duration: ") + + std::to_string((stats.m_end.value() - stats.m_start).count()) + + "(nanosec)\n"; + + return TelemetryInfo::ToString() + "\n" + ("[DebuggerTelemetryInfo]\n") + + (" username: " + username + "\n") + + (" lldb_git_sha: " + lldb_git_sha + "\n") + + (" lldb_path: " + lldb_path + "\n") + (" cwd: " + cwd + "\n") + + duration_desc + "\n"; +} + +std::string ClientTelemetryInfo::ToString() const { + return TelemetryInfo::ToString() + "\n" + ("[DapRequestInfoEntry]\n") + + (" request_name: " + request_name + "\n") + + (" request_duration: " + GetDuration(stats) + "(nanosec)\n") + + (" error_msg: " + error_msg + "\n"); +} + +std::string TargetTelemetryInfo::ToString() const { + std::string exit_or_load_desc; + if (exit_description.has_value()) { +// If this entry was emitted for an exit +exit_or_load_desc = " process_duration: " + GetDuration(stats) + +" exit: " + exit_description->ToString() + "\n"; + } else { +// This was emitted for a load event. +// See if it was the start-load or end-load entry +if (stats.m_end.has_value()) { + exit_or_load_desc = + " startup_init_duration: " + GetDuration(stats) + "\n"; +} else { + exit_or_load_desc = " startup_init_start\n"; +} + } + return TelemetryInfo::ToString() + "\n" + ("[TargetTelemetryInfo]\n") + + (" target_uuid: " + target_uuid + "\n") + + (" file_format: " + file_format + "\n") + + (" binary_path: " + binary_path + "\n") + + (" binary_size: " + std::to_string(binary_size) + "\n") + + exit_or_load_desc; +} + +static std::string StatusToString(CommandReturnObject *result) { + std::string msg; + switch (result->GetStatus()) { + case lldb::eReturnStatusInvalid: +msg = "invalid"; +break; + case lldb::eReturnStatusSuccessFinishNoResult: +msg = "success_finish_no_result"; +break; + case lldb::eReturnStatusSuccessFinishResult: +msg = "success_finish_result"; +break; + case lldb::eReturnStatusSuccessContinuingNoResult: +msg = "success_continuing_no_result"; +break; + case lldb::eReturnStatusSuccessContinuingResult: +msg = "success_continuing_result"; +break; + case lldb::eReturnStatusStarted: +msg = "started"; +break; + case lldb::eReturnStatusFailed: +msg = "failed"; +break; + case lldb::eReturnStatusQuit: +
[Lldb-commits] [lldb] [llvm] [llvm]Added lib/Telemetry (PR #98528)
@@ -0,0 +1,619 @@ + +//===-- Telemetry.cpp -===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// +#include "lldb/Core/Telemetry.h" + +#include +#include + +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "lldb/API/SBDebugger.h" +#include "lldb/API/SBProcess.h" +#include "lldb/Core/Debugger.h" +#include "lldb/Core/Module.h" +#include "lldb/Host/FileSystem.h" +#include "lldb/Host/HostInfo.h" +#include "lldb/Interpreter/CommandInterpreter.h" +#include "lldb/Target/Process.h" +#include "lldb/Target/Statistics.h" +#include "lldb/Utility/ConstString.h" +#include "lldb/Utility/FileSpec.h" +#include "lldb/Utility/UUID.h" +#include "lldb/Version/Version.h" +#include "lldb/lldb-enumerations.h" +#include "lldb/lldb-forward.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallString.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/ADT/Twine.h" +#include "llvm/Support/FileSystem.h" +#include "llvm/Support/LineIterator.h" +#include "llvm/Support/MemoryBuffer.h" +#include "llvm/Support/Path.h" +#include "llvm/Support/RandomNumberGenerator.h" +#include "llvm/Telemetry/Telemetry.h" + +#ifdef HAS_VENDOR_TELEMETRY_PLUGINS +// TODO: could make this path a build-variable rather than hard-coded +#include "lldb/Core/VendorTelemetryPlugin.h" + +// This must include definitions for these functions +extern void ApplyVendorSpecificConfigs( +llvm::telemetry::TelemetryConfig *config) /* __attribute__((weak))*/; +extern std::shared_ptr SanitizeSensitiveFields( +const llvm::telemetry::TelemetryInfo *entry) /*__attribute__((weak))*/; +extern std::shared_ptr +CreateVendorSpecificTelemeter( +llvm::telemetry::TelemetryConfig *config) /*__attribute__((weak))*/; +#endif + +namespace lldb_private { + +static std::string GetDuration(const TelemetryEventStats &stats) { + if (stats.m_end.has_value()) +return std::to_string((stats.m_end.value() - stats.m_start).count()) + + "(nanosec)"; + return ""; +} + +std::string DebuggerTelemetryInfo::ToString() const { + std::string duration_desc = + (exit_description.has_value() ? " lldb session duration: " +: " lldb startup duration: ") + + std::to_string((stats.m_end.value() - stats.m_start).count()) + + "(nanosec)\n"; + + return TelemetryInfo::ToString() + "\n" + ("[DebuggerTelemetryInfo]\n") + + (" username: " + username + "\n") + + (" lldb_git_sha: " + lldb_git_sha + "\n") + + (" lldb_path: " + lldb_path + "\n") + (" cwd: " + cwd + "\n") + + duration_desc + "\n"; +} + +std::string ClientTelemetryInfo::ToString() const { + return TelemetryInfo::ToString() + "\n" + ("[DapRequestInfoEntry]\n") + + (" request_name: " + request_name + "\n") + + (" request_duration: " + GetDuration(stats) + "(nanosec)\n") + + (" error_msg: " + error_msg + "\n"); +} + +std::string TargetTelemetryInfo::ToString() const { + std::string exit_or_load_desc; + if (exit_description.has_value()) { +// If this entry was emitted for an exit +exit_or_load_desc = " process_duration: " + GetDuration(stats) + +" exit: " + exit_description->ToString() + "\n"; + } else { +// This was emitted for a load event. +// See if it was the start-load or end-load entry +if (stats.m_end.has_value()) { + exit_or_load_desc = + " startup_init_duration: " + GetDuration(stats) + "\n"; +} else { + exit_or_load_desc = " startup_init_start\n"; +} + } + return TelemetryInfo::ToString() + "\n" + ("[TargetTelemetryInfo]\n") + + (" target_uuid: " + target_uuid + "\n") + + (" file_format: " + file_format + "\n") + + (" binary_path: " + binary_path + "\n") + + (" binary_size: " + std::to_string(binary_size) + "\n") + + exit_or_load_desc; +} + +static std::string StatusToString(CommandReturnObject *result) { + std::string msg; + switch (result->GetStatus()) { + case lldb::eReturnStatusInvalid: +msg = "invalid"; +break; + case lldb::eReturnStatusSuccessFinishNoResult: +msg = "success_finish_no_result"; +break; + case lldb::eReturnStatusSuccessFinishResult: +msg = "success_finish_result"; +break; + case lldb::eReturnStatusSuccessContinuingNoResult: +msg = "success_continuing_no_result"; +break; + case lldb::eReturnStatusSuccessContinuingResult: +msg = "success_continuing_result"; +break; + case lldb::eReturnStatusStarted: +msg = "started"; +break; + case lldb::eReturnStatusFailed: +msg = "failed"; +break; + case lldb::eReturnStatusQuit: +
[Lldb-commits] [lldb] Fix ASAN failure in TestSingleThreadStepTimeout.py (PR #102208)
https://github.com/jeffreytan81 created https://github.com/llvm/llvm-project/pull/102208 This PR fixes the ASAN failure in https://github.com/llvm/llvm-project/pull/90930. The original PR made the assumption that parent `ThreadPlanStepOverRange`'s lifetime will always be longer than `ThreadPlanSingleThreadTimeout` leaf plan so it passes the `m_timeout_info` as reference to it. >From the ASAN failure, it seems that this assumption may not be true (likely >the thread stack is holding a strong reference to the leaf plan). This PR fixes this lifetime issue by using shared pointer instead of passing by reference. >From 03e22727aa470b60762baa25e20a36f875d451c3 Mon Sep 17 00:00:00 2001 From: jeffreytan81 Date: Tue, 6 Aug 2024 12:47:50 -0700 Subject: [PATCH] Fix ASAN failure in TestSingleThreadStepTimeout.py --- .../Target/ThreadPlanSingleThreadTimeout.h| 15 ++--- lldb/include/lldb/Target/TimeoutResumeAll.h | 7 ++-- .../Target/ThreadPlanSingleThreadTimeout.cpp | 33 ++- 3 files changed, 34 insertions(+), 21 deletions(-) diff --git a/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h b/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h index 1d88d6a51260b..87e6ba6d76695 100644 --- a/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h +++ b/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h @@ -54,11 +54,15 @@ class ThreadPlanSingleThreadTimeout : public ThreadPlan { // state. The reference of \param info is passed in so that when // ThreadPlanSingleThreadTimeout got popped its last state can be stored // in it for future resume. - static void PushNewWithTimeout(Thread &thread, TimeoutInfo &info); + static void PushNewWithTimeout( + Thread &thread, + std::shared_ptr &info); // Push a new ThreadPlanSingleThreadTimeout by restoring state from // input \param info and resume execution. - static void ResumeFromPrevState(Thread &thread, TimeoutInfo &info); + static void ResumeFromPrevState( + Thread &thread, + std::shared_ptr &info); void GetDescription(Stream *s, lldb::DescriptionLevel level) override; bool ValidatePlan(Stream *error) override { return true; } @@ -78,7 +82,9 @@ class ThreadPlanSingleThreadTimeout : public ThreadPlan { bool StopOthers() override; private: - ThreadPlanSingleThreadTimeout(Thread &thread, TimeoutInfo &info); + ThreadPlanSingleThreadTimeout( + Thread &thread, + std::shared_ptr &info); bool IsTimeoutAsyncInterrupt(Event *event_ptr); bool HandleEvent(Event *event_ptr); @@ -91,7 +97,8 @@ class ThreadPlanSingleThreadTimeout : public ThreadPlan { const ThreadPlanSingleThreadTimeout & operator=(const ThreadPlanSingleThreadTimeout &) = delete; - TimeoutInfo &m_info; // Reference to controlling ThreadPlan's TimeoutInfo. + std::shared_ptr + m_info; // Reference to controlling ThreadPlan's TimeoutInfo. State m_state; // Lock for m_wakeup_cv and m_exit_flag between thread plan thread and timer diff --git a/lldb/include/lldb/Target/TimeoutResumeAll.h b/lldb/include/lldb/Target/TimeoutResumeAll.h index d484ea02bcce9..30e923f30602a 100644 --- a/lldb/include/lldb/Target/TimeoutResumeAll.h +++ b/lldb/include/lldb/Target/TimeoutResumeAll.h @@ -19,7 +19,10 @@ namespace lldb_private { // ResumeWithTimeout() during DoWillResume(). class TimeoutResumeAll { public: - TimeoutResumeAll(Thread &thread) : m_thread(thread) {} + TimeoutResumeAll(Thread &thread) + : m_thread(thread), +m_timeout_info( +std::make_shared()) {} void PushNewTimeout() { ThreadPlanSingleThreadTimeout::PushNewWithTimeout(m_thread, m_timeout_info); @@ -32,7 +35,7 @@ class TimeoutResumeAll { private: Thread &m_thread; - ThreadPlanSingleThreadTimeout::TimeoutInfo m_timeout_info; + std::shared_ptr m_timeout_info; }; } // namespace lldb_private diff --git a/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp b/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp index 9cc21bfbb1952..17e6befb7d388 100644 --- a/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp +++ b/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp @@ -24,19 +24,20 @@ using namespace lldb_private; using namespace lldb; -ThreadPlanSingleThreadTimeout::ThreadPlanSingleThreadTimeout(Thread &thread, - TimeoutInfo &info) +ThreadPlanSingleThreadTimeout::ThreadPlanSingleThreadTimeout( +Thread &thread, +std::shared_ptr &info) : ThreadPlan(ThreadPlan::eKindSingleThreadTimeout, "Single thread timeout", thread, eVoteNo, eVoteNoOpinion), m_info(info), m_state(State::WaitTimeout) { // TODO: reuse m_timer_thread without recreation. m_timer_thread = std::thread(TimeoutThreadFunc, this); - m_info.m_isAlive = true; - m_state = m_info.m_last_state; + m_info->m_isAlive = true; + m_state = m_info->m_last_state; } ThreadPlanSingleThreadTimeout::~ThreadPlanSingleTh
[Lldb-commits] [lldb] Fix ASAN failure in TestSingleThreadStepTimeout.py (PR #102208)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: None (jeffreytan81) Changes This PR fixes the ASAN failure in https://github.com/llvm/llvm-project/pull/90930. The original PR made the assumption that parent `ThreadPlanStepOverRange`'s lifetime will always be longer than `ThreadPlanSingleThreadTimeout` leaf plan so it passes the `m_timeout_info` as reference to it. >From the ASAN failure, it seems that this assumption may not be true (likely >the thread stack is holding a strong reference to the leaf plan). This PR fixes this lifetime issue by using shared pointer instead of passing by reference. --- Full diff: https://github.com/llvm/llvm-project/pull/102208.diff 3 Files Affected: - (modified) lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h (+11-4) - (modified) lldb/include/lldb/Target/TimeoutResumeAll.h (+5-2) - (modified) lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp (+18-15) ``diff diff --git a/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h b/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h index 1d88d6a51260b1..87e6ba6d766955 100644 --- a/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h +++ b/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h @@ -54,11 +54,15 @@ class ThreadPlanSingleThreadTimeout : public ThreadPlan { // state. The reference of \param info is passed in so that when // ThreadPlanSingleThreadTimeout got popped its last state can be stored // in it for future resume. - static void PushNewWithTimeout(Thread &thread, TimeoutInfo &info); + static void PushNewWithTimeout( + Thread &thread, + std::shared_ptr &info); // Push a new ThreadPlanSingleThreadTimeout by restoring state from // input \param info and resume execution. - static void ResumeFromPrevState(Thread &thread, TimeoutInfo &info); + static void ResumeFromPrevState( + Thread &thread, + std::shared_ptr &info); void GetDescription(Stream *s, lldb::DescriptionLevel level) override; bool ValidatePlan(Stream *error) override { return true; } @@ -78,7 +82,9 @@ class ThreadPlanSingleThreadTimeout : public ThreadPlan { bool StopOthers() override; private: - ThreadPlanSingleThreadTimeout(Thread &thread, TimeoutInfo &info); + ThreadPlanSingleThreadTimeout( + Thread &thread, + std::shared_ptr &info); bool IsTimeoutAsyncInterrupt(Event *event_ptr); bool HandleEvent(Event *event_ptr); @@ -91,7 +97,8 @@ class ThreadPlanSingleThreadTimeout : public ThreadPlan { const ThreadPlanSingleThreadTimeout & operator=(const ThreadPlanSingleThreadTimeout &) = delete; - TimeoutInfo &m_info; // Reference to controlling ThreadPlan's TimeoutInfo. + std::shared_ptr + m_info; // Reference to controlling ThreadPlan's TimeoutInfo. State m_state; // Lock for m_wakeup_cv and m_exit_flag between thread plan thread and timer diff --git a/lldb/include/lldb/Target/TimeoutResumeAll.h b/lldb/include/lldb/Target/TimeoutResumeAll.h index d484ea02bcce94..30e923f30602a3 100644 --- a/lldb/include/lldb/Target/TimeoutResumeAll.h +++ b/lldb/include/lldb/Target/TimeoutResumeAll.h @@ -19,7 +19,10 @@ namespace lldb_private { // ResumeWithTimeout() during DoWillResume(). class TimeoutResumeAll { public: - TimeoutResumeAll(Thread &thread) : m_thread(thread) {} + TimeoutResumeAll(Thread &thread) + : m_thread(thread), +m_timeout_info( +std::make_shared()) {} void PushNewTimeout() { ThreadPlanSingleThreadTimeout::PushNewWithTimeout(m_thread, m_timeout_info); @@ -32,7 +35,7 @@ class TimeoutResumeAll { private: Thread &m_thread; - ThreadPlanSingleThreadTimeout::TimeoutInfo m_timeout_info; + std::shared_ptr m_timeout_info; }; } // namespace lldb_private diff --git a/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp b/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp index 9cc21bfbb1952d..17e6befb7d388c 100644 --- a/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp +++ b/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp @@ -24,19 +24,20 @@ using namespace lldb_private; using namespace lldb; -ThreadPlanSingleThreadTimeout::ThreadPlanSingleThreadTimeout(Thread &thread, - TimeoutInfo &info) +ThreadPlanSingleThreadTimeout::ThreadPlanSingleThreadTimeout( +Thread &thread, +std::shared_ptr &info) : ThreadPlan(ThreadPlan::eKindSingleThreadTimeout, "Single thread timeout", thread, eVoteNo, eVoteNoOpinion), m_info(info), m_state(State::WaitTimeout) { // TODO: reuse m_timer_thread without recreation. m_timer_thread = std::thread(TimeoutThreadFunc, this); - m_info.m_isAlive = true; - m_state = m_info.m_last_state; + m_info->m_isAlive = true; + m_state = m_info->m_last_state; } ThreadPlanSingleThreadTimeout::~ThreadPlanSingleThreadTimeout() { - m_info.m_isAlive = false; + m_info->m_isAlive = false; } uint64_t ThreadPlanSingleThreadTimeout