[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)
Michael137 wrote: > I have the whole thing implemented (complete with test cases). If you really > want to dig through the complete code, you can see it at > https://github.com/cmtice/llvm-project/tree/DIL-work-new/ (note that I will > be cleaning up the Parser & Evaluator code before actually being put into a > PR). Thanks! that'll be a useful reference > Besides managing that risk (if this fades out for whatever reason, there's > less dead code around), I think this would also make it easier to review, as > we could have patches like "add operator/ to the DIL", which would focus > solely on that, and also come with associated tests. I think it would also > make it easier to discuss things like whether we want to add modifying > operations (operator++) or obviously language-specific features > (dynamic_cast), both of which I think are very good questions. Makes sense! https://github.com/llvm/llvm-project/pull/95738 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Minidump] Change expected directories to the correct type; size_t (PR #96564)
DavidSpickett wrote: `size_t` will be 32 bit on 32 bit platforms, is this ok given the expected range of this variable? I don't see any failures on 32 bit Arm right now but worth asking in case it causes flaky tests later. https://github.com/llvm/llvm-project/pull/96564 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Set target OS for API tests in case of remote testing (PR #96654)
@@ -97,6 +101,9 @@ def finalize_build_dictionary(dictionary):
dictionary = {}
dictionary["OS"] = "Android"
dictionary["PIE"] = 1
+elif target_is_remote_linux():
labath wrote:
For consistency, I think we should do this for all linux systems, not just
remote ones. (Ideally, I would do it for all systems overall, but I can
understand if you're reluctant to do that.)
You can use `getPlatform` to get the platform/os name for both local and remote
runs.
https://github.com/llvm/llvm-project/pull/96654
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix test assertions in TestDAP_stepInTargets.py (PR #96687)
labath wrote:
I'm not sure if these names come from the demangler or debug info, but windows
has different implementations for both, so it not surprising they're different.
Assuming we don't care about the precise formatting of the names, we could
just check whether the name of the function is `in` the string (and change the
name to something more unique): `assertIn("my_first_step_target",
step_in_targets[0]["label"])`
https://github.com/llvm/llvm-project/pull/96687
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix flake in TestZerothFrame.py (PR #96685)
labath wrote: The thread that you care about is stopped at a breakpoint, right? If so, then you can use the `lldbutil.get_threads_stopped_at_breakpoint` utility to find it... https://github.com/llvm/llvm-project/pull/96685 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Parse and display register field enums (PR #95768)
DavidSpickett wrote: > The error could then be surfaced back in a meaningful way, like "hey the > server gave me some bunk data so pretty-printing registers might look weird". > What do you think? IIRC GDB will do something like this, though it may just be (paraphrasing) "yo this XML be weird". Even so, we could say "... check the log for more detail". There may be a lot of log output even for relatively normal sessions, and I can tune that to some extent but even so, some users won't care. I'll see how much output there is at the moment. https://github.com/llvm/llvm-project/pull/95768 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Remove parsing recursion when searching for definition DIEs (PR #96484)
@@ -897,32 +895,39 @@ TypeSP DWARFASTParserClang::ParseEnum(const SymbolContext &sc, } CompilerType clang_type = m_ast.CreateEnumerationType( - attrs.name.GetStringRef(), GetClangDeclContextContainingDIE(die, nullptr), - GetOwningClangModule(die), attrs.decl, enumerator_clang_type, + attrs.name.GetStringRef(), GetClangDeclContextContainingDIE(def_die, nullptr), + GetOwningClangModule(def_die), attrs.decl, enumerator_clang_type, attrs.is_scoped_enum); - - LinkDeclContextToDIE(TypeSystemClang::GetDeclContextForType(clang_type), die); - - type_sp = - dwarf->MakeType(die.GetID(), attrs.name, attrs.byte_size, nullptr, + TypeSP type_sp = + dwarf->MakeType(def_die.GetID(), attrs.name, attrs.byte_size, nullptr, labath wrote: Yes, that's a very good catch. I noticed the lack of a unique map in the enum implementation, but I just assumed that means we don't care about duplicate definitions for enums. (And to a sense that's true, since it still means we will create a duplicate definition if we start with two *definition* DIEs.) This should be fairly simple to fix, I'll create a today. https://github.com/llvm/llvm-project/pull/96484 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add format eFormatEnumWithValues to ensure raw enum value is always shown (PR #90059)
DavidSpickett wrote: ping! https://github.com/llvm/llvm-project/pull/90059 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [clang][lldb] Don't assert structure layout correctness for layouts provided by LLDB (PR #93809)
Michael137 wrote:
While fixing the libc++ formatters in preparation for the [compressed_pair
change](https://github.com/llvm/llvm-project/issues/93069), i encountered
another issue which I'm not sure entirely how to best reconcile. There's [this
assumption in
`RecordLayoutBuilder`](https://github.com/llvm/llvm-project/blob/f782ff8fc6426890863be0791a9ace2394da3887/clang/lib/AST/RecordLayoutBuilder.cpp#L2258-L2264)
for external layouts, where, if we encounter overlapping field offsets, we
assume the structure is packed and set the alignment back to `1`:
```
uint64_t
ItaniumRecordLayoutBuilder::updateExternalFieldOffset(const FieldDecl *Field,
uint64_t ComputedOffset) {
uint64_t ExternalFieldOffset = External.getExternalFieldOffset(Field);
if (InferAlignment && ExternalFieldOffset < ComputedOffset) {
// The externally-supplied field offset is before the field offset we
// computed. Assume that the structure is packed.
Alignment = CharUnits::One();
PreferredAlignment = CharUnits::One();
InferAlignment = false;
}
...
```
The assumption in that comment doesn't hold for layouts where the overlap
occurred because of `[[no_unique_address]]`. In those cases, the alignment of
`1` will prevent us from aligning the `FieldOffset` correctly and cause LLDB to
read out the fields incorrectly.
We can't guard this with `Field->isZeroSize()` because we don't have the
attribute in the AST.
Can we infer packedness more accurately here?
Or maybe that's just putting a bandaid on a bigger underlying issue
https://github.com/llvm/llvm-project/pull/93809
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Unique enums parsed from declarations (PR #96751)
https://github.com/labath created
https://github.com/llvm/llvm-project/pull/96751
This is a regression from #96484 caught by @ZequanWu.
Note that we will still create separate enum types for types parsed from two
definitions. This is different from how we handle classes, but it is not a
regression.
I'm also adding the DieToType check to the class type parsing code, although in
this case, the type uniqueness should be enforced by the UniqueDWARFASTType map.
>From b8decc9a36b5f31283c088bf1783be4546fc7ab9 Mon Sep 17 00:00:00 2001
From: Pavel Labath
Date: Wed, 26 Jun 2024 12:51:35 +0200
Subject: [PATCH] [lldb/DWARF] Unique enums parsed from declarations
This is a regression from #96484 caught by @ZequanWu.
Note that we will still create separate enum types for types parsed from
two definitions. This is different from how we handle classes, but it is
not a regression.
I'm also adding the DieToType check to the class type parsing code,
although in this case, the type uniqueness should be enforced by the
UniqueDWARFASTType map.
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 14
.../DWARF/enum-declaration-uniqueness.cpp | 32 +++
2 files changed, 46 insertions(+)
create mode 100644
lldb/test/Shell/SymbolFile/DWARF/enum-declaration-uniqueness.cpp
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index f36e2af9589b8..c4a3b432ba949 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -870,6 +870,13 @@ TypeSP DWARFASTParserClang::ParseEnum(const SymbolContext
&sc,
}
}
if (def_die) {
+if (auto [it, inserted] = dwarf->GetDIEToType().try_emplace(
+def_die.GetDIE(), DIE_IS_BEING_PARSED);
+!inserted) {
+ if (it->getSecond() == nullptr || it->getSecond() == DIE_IS_BEING_PARSED)
+return nullptr;
+ return it->getSecond()->shared_from_this();
+}
attrs = ParsedDWARFTypeAttributes(def_die);
} else {
// No definition found. Proceed with the declaration die. We can use it to
@@ -1798,6 +1805,13 @@ DWARFASTParserClang::ParseStructureLikeDIE(const
SymbolContext &sc,
}
if (def_die) {
+if (auto [it, inserted] = dwarf->GetDIEToType().try_emplace(
+def_die.GetDIE(), DIE_IS_BEING_PARSED);
+!inserted) {
+ if (it->getSecond() == nullptr || it->getSecond() == DIE_IS_BEING_PARSED)
+return nullptr;
+ return it->getSecond()->shared_from_this();
+}
attrs = ParsedDWARFTypeAttributes(def_die);
} else {
// No definition found. Proceed with the declaration die. We can use it to
diff --git a/lldb/test/Shell/SymbolFile/DWARF/enum-declaration-uniqueness.cpp
b/lldb/test/Shell/SymbolFile/DWARF/enum-declaration-uniqueness.cpp
new file mode 100644
index 0..6fc47d7e4b53a
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/enum-declaration-uniqueness.cpp
@@ -0,0 +1,32 @@
+// RUN: %clangxx_host -gdwarf -c -o %t_a.o %s -DFILE_A
+// RUN: %clangxx_host -gdwarf -c -o %t_b.o %s -DFILE_B
+// RUN: %clangxx_host -o %t %t_a.o %t_b.o
+// RUN: %lldb %t \
+// RUN: -o "target variable my_enum my_enum_ref" -o "image dump ast" \
+// RUN: -o exit | FileCheck %s
+
+
+// CHECK: (lldb) target variable
+// CHECK: (MyEnum) my_enum = MyEnum_A
+// CHECK: (MyEnum &) my_enum_ref =
+// CHECK-SAME: &::my_enum_ref = MyEnum_A
+
+// CHECK: (lldb) image dump ast
+// CHECK: EnumDecl {{.*}} MyEnum
+// CHECK-NEXT: EnumConstantDecl {{.*}} MyEnum_A 'MyEnum'
+// CHECK-NOT: MyEnum
+
+enum MyEnum : int;
+
+extern MyEnum my_enum;
+
+#ifdef FILE_A
+enum MyEnum : int { MyEnum_A };
+
+MyEnum my_enum = MyEnum_A;
+
+int main() {}
+#endif
+#ifdef FILE_B
+MyEnum &my_enum_ref = my_enum;
+#endif
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Remove parsing recursion when searching for definition DIEs (PR #96484)
@@ -897,32 +895,39 @@ TypeSP DWARFASTParserClang::ParseEnum(const SymbolContext &sc, } CompilerType clang_type = m_ast.CreateEnumerationType( - attrs.name.GetStringRef(), GetClangDeclContextContainingDIE(die, nullptr), - GetOwningClangModule(die), attrs.decl, enumerator_clang_type, + attrs.name.GetStringRef(), GetClangDeclContextContainingDIE(def_die, nullptr), + GetOwningClangModule(def_die), attrs.decl, enumerator_clang_type, attrs.is_scoped_enum); - - LinkDeclContextToDIE(TypeSystemClang::GetDeclContextForType(clang_type), die); - - type_sp = - dwarf->MakeType(die.GetID(), attrs.name, attrs.byte_size, nullptr, + TypeSP type_sp = + dwarf->MakeType(def_die.GetID(), attrs.name, attrs.byte_size, nullptr, labath wrote: #96751 https://github.com/llvm/llvm-project/pull/96484 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Unique enums parsed from declarations (PR #96751)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Pavel Labath (labath)
Changes
This is a regression from #96484 caught by @ZequanWu.
Note that we will still create separate enum types for types parsed from two
definitions. This is different from how we handle classes, but it is not a
regression.
I'm also adding the DieToType check to the class type parsing code, although in
this case, the type uniqueness should be enforced by the UniqueDWARFASTType map.
---
Full diff: https://github.com/llvm/llvm-project/pull/96751.diff
2 Files Affected:
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp (+14)
- (added) lldb/test/Shell/SymbolFile/DWARF/enum-declaration-uniqueness.cpp
(+32)
``diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index f36e2af9589b8..c4a3b432ba949 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -870,6 +870,13 @@ TypeSP DWARFASTParserClang::ParseEnum(const SymbolContext
&sc,
}
}
if (def_die) {
+if (auto [it, inserted] = dwarf->GetDIEToType().try_emplace(
+def_die.GetDIE(), DIE_IS_BEING_PARSED);
+!inserted) {
+ if (it->getSecond() == nullptr || it->getSecond() == DIE_IS_BEING_PARSED)
+return nullptr;
+ return it->getSecond()->shared_from_this();
+}
attrs = ParsedDWARFTypeAttributes(def_die);
} else {
// No definition found. Proceed with the declaration die. We can use it to
@@ -1798,6 +1805,13 @@ DWARFASTParserClang::ParseStructureLikeDIE(const
SymbolContext &sc,
}
if (def_die) {
+if (auto [it, inserted] = dwarf->GetDIEToType().try_emplace(
+def_die.GetDIE(), DIE_IS_BEING_PARSED);
+!inserted) {
+ if (it->getSecond() == nullptr || it->getSecond() == DIE_IS_BEING_PARSED)
+return nullptr;
+ return it->getSecond()->shared_from_this();
+}
attrs = ParsedDWARFTypeAttributes(def_die);
} else {
// No definition found. Proceed with the declaration die. We can use it to
diff --git a/lldb/test/Shell/SymbolFile/DWARF/enum-declaration-uniqueness.cpp
b/lldb/test/Shell/SymbolFile/DWARF/enum-declaration-uniqueness.cpp
new file mode 100644
index 0..6fc47d7e4b53a
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/enum-declaration-uniqueness.cpp
@@ -0,0 +1,32 @@
+// RUN: %clangxx_host -gdwarf -c -o %t_a.o %s -DFILE_A
+// RUN: %clangxx_host -gdwarf -c -o %t_b.o %s -DFILE_B
+// RUN: %clangxx_host -o %t %t_a.o %t_b.o
+// RUN: %lldb %t \
+// RUN: -o "target variable my_enum my_enum_ref" -o "image dump ast" \
+// RUN: -o exit | FileCheck %s
+
+
+// CHECK: (lldb) target variable
+// CHECK: (MyEnum) my_enum = MyEnum_A
+// CHECK: (MyEnum &) my_enum_ref =
+// CHECK-SAME: &::my_enum_ref = MyEnum_A
+
+// CHECK: (lldb) image dump ast
+// CHECK: EnumDecl {{.*}} MyEnum
+// CHECK-NEXT: EnumConstantDecl {{.*}} MyEnum_A 'MyEnum'
+// CHECK-NOT: MyEnum
+
+enum MyEnum : int;
+
+extern MyEnum my_enum;
+
+#ifdef FILE_A
+enum MyEnum : int { MyEnum_A };
+
+MyEnum my_enum = MyEnum_A;
+
+int main() {}
+#endif
+#ifdef FILE_B
+MyEnum &my_enum_ref = my_enum;
+#endif
``
https://github.com/llvm/llvm-project/pull/96751
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Don't start class definitions in ParseStructureLikeDIE (PR #96755)
https://github.com/labath created
https://github.com/llvm/llvm-project/pull/96755
Right now, ParseStructureLikeDIE begins the class definition (which amounts to
parsing the opening "{" of a class and promising to be able to fill it in
later) if it finds a definition DIE.
This makes sense in the current setup, where we eagerly search for the
definition die (so that we will either find it in the beginning or don't find
it at all), but with delayed definition searching (#92328), this created an (in
my view, undesirable) inconsistency, where the final state of the type (whether
it has begun its definition) depended on whether we happened to start out with
a definition DIE or not.
This patch attempts to pre-emptively rectify that by establishing a new
invariant: the definition is never started eagerly. It can only be started in
one of two ways:
- we're completing the type, in which case we will start the definition, parse
everything and immediately finish it
- we need to parse a member (typedef, nested class, method) of the class
without needing the definition itself. In this case, we just start the
definition to insert the member we need.
Besides the delayed definition search, I believe this setup has a couple of
other benefits:
- It treats ObjC and C++ classes the same way (we were never starting the
definition of those)
- unifies the handling of types that types that have a definition and those
that do. When adding (e.g.) a nested class we would previously be going down a
different code path depending on whether we've found a definition DIE for that
type. Now, we're always taking the definition-not-found path (*)
- it reduces the amount of time a class spends in the funny "definition
started". Aside from the addition of stray addition of nested classes, we
always finish the definition right after we start it.
(*) Herein lies a danger, where if we're missing some calls to
PrepareContextToReceiveMembers, we could trigger a crash when
trying to add a member to the not-yet-started-to-be-defined classes.
However, this is something that could happen before as well (if we
did not have a definition for the class), and is something that
would be exacerbated by #92328 (because it could happen even if we
the definition exists, but we haven't found it yet). This way, it
will at least happen consistently, and the fix should consist of
adding a PrepareContextToReceiveMembers in the appropriate place.
>From a9076e2fe3a19b615d418eeff4e8e8b49ed66b6d Mon Sep 17 00:00:00 2001
From: Pavel Labath
Date: Tue, 25 Jun 2024 11:06:36 +0200
Subject: [PATCH] [lldb/DWARF] Don't start class definitions in
ParseStructureLikeDIE
Right now, ParseStructureLikeDIE begins the class definition (which
amounts to parsing the opening "{" of a class and promising to be able
to fill it in later) if it finds a definition DIE.
This makes sense in the current setup, where we eagerly search for the
definition die (so that we will either find it in the beginning or don't
find it at all), but with delayed definition searching (#92328), this
created an (in my view, undesirable) inconsistency, where the final
state of the type (whether it has begun its definition) depended on
whether we happened to start out with a definition DIE or not.
This patch attempts to pre-emptively rectify that by establishing a new
invariant: the definition is never started eagerly. It can only be
started in one of two ways:
- we're completing the type, in which case we will start the definition,
parse everything and immediately finish it
- we need to parse a member (typedef, nested class, method) of the class
without needing the definition itself. In this case, we just start the
definition to insert the member we need.
Besides the delayed definition search, I believe this setup has a couple
of other benefits:
- It treats ObjC and C++ classes the same way (we were never starting
the definition of those)
- unifies the handling of types that types that have a definition and
those that do. When adding (e.g.) a nested class we would previously
be going down a different code path depending on whether we've found a
definition DIE for that type. Now, we're always taking the
definition-not-found path (*)
- it reduces the amount of time a class spends in the funny "definition
started". Aside from the addition of stray addition of nested classes,
we always finish the definition right after we start it.
(*) Herein lies a danger, where if we're missing some calls to
PrepareContextToReceiveMembers, we could trigger a crash when
trying to add a member to the not-yet-started-to-be-defined classes.
However, this is something that could happen before as well (if we
did not have a definition for the class), and is something that
would be exacerbated by #92328 (because it could happen even if we
the definition exists, but we haven't found it yet). This way, it
will at least happen consist
[Lldb-commits] [lldb] [lldb/DWARF] Don't start class definitions in ParseStructureLikeDIE (PR #96755)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Pavel Labath (labath)
Changes
Right now, ParseStructureLikeDIE begins the class definition (which amounts to
parsing the opening "{" of a class and promising to be able to fill it in
later) if it finds a definition DIE.
This makes sense in the current setup, where we eagerly search for the
definition die (so that we will either find it in the beginning or don't find
it at all), but with delayed definition searching (#92328), this
created an (in my view, undesirable) inconsistency, where the final state of
the type (whether it has begun its definition) depended on whether we happened
to start out with a definition DIE or not.
This patch attempts to pre-emptively rectify that by establishing a new
invariant: the definition is never started eagerly. It can only be started in
one of two ways:
- we're completing the type, in which case we will start the definition, parse
everything and immediately finish it
- we need to parse a member (typedef, nested class, method) of the class
without needing the definition itself. In this case, we just start the
definition to insert the member we need.
Besides the delayed definition search, I believe this setup has a couple of
other benefits:
- It treats ObjC and C++ classes the same way (we were never starting the
definition of those)
- unifies the handling of types that types that have a definition and those
that do. When adding (e.g.) a nested class we would previously be going down a
different code path depending on whether we've found a definition DIE for that
type. Now, we're always taking the definition-not-found path (*)
- it reduces the amount of time a class spends in the funny "definition
started". Aside from the addition of stray addition of nested classes, we
always finish the definition right after we start it.
(*) Herein lies a danger, where if we're missing some calls to
PrepareContextToReceiveMembers, we could trigger a crash when
trying to add a member to the not-yet-started-to-be-defined classes.
However, this is something that could happen before as well (if we
did not have a definition for the class), and is something that
would be exacerbated by #92328 (because it could happen even if we
the definition exists, but we haven't found it yet). This way, it
will at least happen consistently, and the fix should consist of
adding a PrepareContextToReceiveMembers in the appropriate place.
---
Full diff: https://github.com/llvm/llvm-project/pull/96755.diff
1 Files Affected:
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
(+144-234)
``diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index f36e2af9589b8..0c3a62124eb1b 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -237,20 +237,10 @@ TypeSP
DWARFASTParserClang::ParseTypeFromClangModule(const SymbolContext &sc,
return type_sp;
}
-static void ForcefullyCompleteType(CompilerType type) {
- bool started = TypeSystemClang::StartTagDeclarationDefinition(type);
- lldbassert(started && "Unable to start a class type definition.");
- TypeSystemClang::CompleteTagDeclarationDefinition(type);
- const clang::TagDecl *td = ClangUtil::GetAsTagDecl(type);
- auto ts_sp = type.GetTypeSystem();
- auto ts = ts_sp.dyn_cast_or_null();
- if (ts)
-ts->SetDeclIsForcefullyCompleted(td);
-}
-
-/// This function serves a similar purpose as RequireCompleteType above, but it
-/// avoids completing the type if it is not immediately necessary. It only
-/// ensures we _can_ complete the type later.
+/// This function ensures we are able to add members (nested types, functions,
+/// etc.) to this type. It does so by starting its definition even if one
cannot
+/// be found in the debug info. This means the type may need to be "forcibly
+/// completed" later -- see CompleteTypeFromDWARF).
static void PrepareContextToReceiveMembers(TypeSystemClang &ast,
ClangASTImporter &ast_importer,
clang::DeclContext *decl_ctx,
@@ -260,14 +250,12 @@ static void
PrepareContextToReceiveMembers(TypeSystemClang &ast,
if (!tag_decl_ctx)
return; // Non-tag context are always ready.
- // We have already completed the type, or we have found its definition and
are
- // ready to complete it later (cf. ParseStructureLikeDIE).
+ // We have already completed the type or it is already prepared.
if (tag_decl_ctx->isCompleteDefinition() || tag_decl_ctx->isBeingDefined())
return;
- // We reach this point of the tag was present in the debug info as a
- // declaration only. If it was imported from another AST context (in the
- // gmodules case), we can complete the type by doing a full import.
+ // If this tag w
[Lldb-commits] [lldb] [lldb/DWARF] Don't start class definitions in ParseStructureLikeDIE (PR #96755)
@@ -1893,72 +1849,21 @@ DWARFASTParserClang::ParseStructureLikeDIE(const
SymbolContext &sc,
dwarf->GetUniqueDWARFASTTypeMap().Insert(unique_typename,
*unique_ast_entry_up);
- if (!attrs.is_forward_declaration) {
-// Always start the definition for a class type so that if the class
-// has child classes or types that require the class to be created
-// for use as their decl contexts the class will be ready to accept
-// these child definitions.
-if (!def_die.HasChildren()) {
- // No children for this struct/union/class, lets finish it
- if (TypeSystemClang::StartTagDeclarationDefinition(clang_type)) {
-TypeSystemClang::CompleteTagDeclarationDefinition(clang_type);
- } else {
-dwarf->GetObjectFile()->GetModule()->ReportError(
-
-"DWARF DIE {0:x16} named \"{1}\" was not able to start its "
-"definition.\nPlease file a bug and attach the file at the "
-"start of this error message",
-def_die.GetID(), attrs.name.GetCString());
- }
-
- // Setting authority byte size and alignment for empty structures.
- //
- // If the byte size or alignmenet of the record is specified then
- // overwrite the ones that would be computed by Clang.
- // This is only needed as LLDB's TypeSystemClang is always in C++ mode,
- // but some compilers such as GCC and Clang give empty structs a size of 0
- // in C mode (in contrast to the size of 1 for empty structs that would
be
- // computed in C++ mode).
- if (attrs.byte_size || attrs.alignment) {
-clang::RecordDecl *record_decl =
-TypeSystemClang::GetAsRecordDecl(clang_type);
-if (record_decl) {
labath wrote:
These lines.
https://github.com/llvm/llvm-project/pull/96755
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Don't start class definitions in ParseStructureLikeDIE (PR #96755)
@@ -2192,87 +2097,82 @@ bool DWARFASTParserClang::CompleteRecordType(const
DWARFDIE &die,
ClangASTImporter::LayoutInfo layout_info;
std::vector contained_type_dies;
- if (die.HasChildren()) {
-const bool type_is_objc_object_or_interface =
-TypeSystemClang::IsObjCObjectOrInterfaceType(clang_type);
-if (type_is_objc_object_or_interface) {
- // For objective C we don't start the definition when the class is
- // created.
- TypeSystemClang::StartTagDeclarationDefinition(clang_type);
-}
-
-AccessType default_accessibility = eAccessNone;
-if (tag == DW_TAG_structure_type) {
- default_accessibility = eAccessPublic;
-} else if (tag == DW_TAG_union_type) {
- default_accessibility = eAccessPublic;
-} else if (tag == DW_TAG_class_type) {
- default_accessibility = eAccessPrivate;
-}
-
-std::vector> bases;
-// Parse members and base classes first
-std::vector member_function_dies;
-
-DelayedPropertyList delayed_properties;
-ParseChildMembers(die, clang_type, bases, member_function_dies,
- contained_type_dies, delayed_properties,
- default_accessibility, layout_info);
-
-// Now parse any methods if there were any...
-for (const DWARFDIE &die : member_function_dies)
- dwarf->ResolveType(die);
-
-if (type_is_objc_object_or_interface) {
- ConstString class_name(clang_type.GetTypeName());
- if (class_name) {
-dwarf->GetObjCMethods(class_name, [&](DWARFDIE method_die) {
- method_die.ResolveType();
- return true;
-});
+ if (die.GetAttributeValueAsUnsigned(DW_AT_declaration, 0))
+return false; // No definition, cannot complete.
-for (DelayedAddObjCClassProperty &property : delayed_properties)
- property.Finalize();
- }
-}
+ // Start the definition if the type is not being defined already. This can
+ // happen (e.g.) when adding nested types to a class type -- see
+ // PrepareContextToReceiveMembers.
+ if (!clang_type.IsBeingDefined())
+TypeSystemClang::StartTagDeclarationDefinition(clang_type);
-if (!bases.empty()) {
- // Make sure all base classes refer to complete types and not forward
- // declarations. If we don't do this, clang will crash with an
- // assertion in the call to clang_type.TransferBaseClasses()
- for (const auto &base_class : bases) {
-clang::TypeSourceInfo *type_source_info =
-base_class->getTypeSourceInfo();
-if (type_source_info)
- TypeSystemClang::RequireCompleteType(
- m_ast.GetType(type_source_info->getType()));
- }
+ AccessType default_accessibility = eAccessNone;
+ if (tag == DW_TAG_structure_type) {
+default_accessibility = eAccessPublic;
+ } else if (tag == DW_TAG_union_type) {
+default_accessibility = eAccessPublic;
+ } else if (tag == DW_TAG_class_type) {
+default_accessibility = eAccessPrivate;
+ }
+
+ std::vector> bases;
+ // Parse members and base classes first
+ std::vector member_function_dies;
- m_ast.TransferBaseClasses(clang_type.GetOpaqueQualType(),
-std::move(bases));
+ DelayedPropertyList delayed_properties;
+ ParseChildMembers(die, clang_type, bases, member_function_dies,
+contained_type_dies, delayed_properties,
+default_accessibility, layout_info);
+
+ // Now parse any methods if there were any...
+ for (const DWARFDIE &die : member_function_dies)
+dwarf->ResolveType(die);
+
+ if (TypeSystemClang::IsObjCObjectOrInterfaceType(clang_type)) {
+ConstString class_name(clang_type.GetTypeName());
+if (class_name) {
+ dwarf->GetObjCMethods(class_name, [&](DWARFDIE method_die) {
+method_die.ResolveType();
+return true;
+ });
+
+ for (DelayedAddObjCClassProperty &property : delayed_properties)
+property.Finalize();
}
}
+ if (!bases.empty()) {
+// Make sure all base classes refer to complete types and not forward
+// declarations. If we don't do this, clang will crash with an
+// assertion in the call to clang_type.TransferBaseClasses()
+for (const auto &base_class : bases) {
+ clang::TypeSourceInfo *type_source_info =
base_class->getTypeSourceInfo();
+ if (type_source_info)
+TypeSystemClang::RequireCompleteType(
+m_ast.GetType(type_source_info->getType()));
+}
+
+m_ast.TransferBaseClasses(clang_type.GetOpaqueQualType(),
std::move(bases));
+ }
+
m_ast.AddMethodOverridesForCXXRecordType(clang_type.GetOpaqueQualType());
TypeSystemClang::BuildIndirectFields(clang_type);
TypeSystemClang::CompleteTagDeclarationDefinition(clang_type);
- if (!layout_info.field_offsets.empty() || !layout_info.base_offsets.empty()
||
- !layout_info.vbase_offsets.empty()) {
labath wrote:
The removal of these checks compensates fo
[Lldb-commits] [lldb] [lldb/DWARF] Unique enums parsed from declarations (PR #96751)
https://github.com/Michael137 approved this pull request. https://github.com/llvm/llvm-project/pull/96751 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Don't start class definitions in ParseStructureLikeDIE (PR #96755)
https://github.com/Michael137 commented: Without having reviewed this in detail yet this I like the goal, thanks for doing this. This aligns with what https://github.com/llvm/llvm-project/pull/95100 is trying to do. There we similarly want to make sure that we start and complete definitions in the same place. So I'm hoping that patch rebases more-or-less cleanly on this :) https://github.com/llvm/llvm-project/pull/96755 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add format eFormatEnumWithValues to ensure raw enum value is always shown (PR #90059)
DavidSpickett wrote: To be clear, this is not blocking register field enums work anymore. I decided to push on with that because folks can `register info` to see the values if they really need them. Having this would be nice, but there's plenty of time to consider whether it's worth it. https://github.com/llvm/llvm-project/pull/90059 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Unify Platform::ResolveExecutable (PR #96256)
slydiman wrote: It seems this patch breaks the following tests on cross setups: ``` Failed Tests (6): lldb-api :: commands/process/attach/TestProcessAttach.py lldb-api :: commands/process/detach-resumes/TestDetachResumes.py lldb-api :: functionalities/dyld-exec-linux/TestDyldExecLinux.py lldb-api :: functionalities/dyld-launch-linux/TestDyldLaunchLinux.py lldb-api :: functionalities/exec/TestExec.py lldb-api :: functionalities/thread/create_after_attach/TestCreateAfterAttach.py ``` Note the behavior is the same for Windows and Linux hosts with Linux AArch64 target. Probably removing `RemoteAwarePlatform::ResolveExecutable` causes the issue. For example here is the log for lldb-api :: commands/process/attach/TestProcessAttach.py ``` Command Output (stdout): -- Setting up remote platform 'remote-linux' Connecting to remote platform 'remote-linux' at 'connect://test1:1234'... Connected. Setting remote platform working directory to '/home/ubuntu/lldb-tests'... Skipping the following test categories: ['dsym', 'gmodules', 'debugserver', 'objc', 'lldb-dap'] -- Command Output (stderr): -- PASS: LLDB (C:\\lldb-test-scripts\build-lldb\bin\clang.exe-aarch64) :: test_attach_to_process_by_id (TestProcessAttach.ProcessAttachTestCase.test_attach_to_process_by_id) PASS: LLDB (C:\\lldb-test-scripts\build-lldb\bin\clang.exe-aarch64) :: test_attach_to_process_by_id_autocontinue (TestProcessAttach.ProcessAttachTestCase.test_attach_to_process_by_id_autocontinue) FAIL: LLDB (C:\\lldb-test-scripts\build-lldb\bin\clang.exe-aarch64) :: test_attach_to_process_by_id_correct_executable_offset (TestProcessAttach.ProcessAttachTestCase.test_attach_to_process_by_id_correct_executable_offset) PASS: LLDB (C:\lldb-test-scripts\build-lldb\bin\clang.exe-aarch64) :: test_attach_to_process_by_name (TestProcessAttach.ProcessAttachTestCase.test_attach_to_process_by_name) PASS: LLDB (C:\lldb-test-scripts\build-lldb\bin\clang.exe-aarch64) :: test_attach_to_process_from_different_dir_by_id (TestProcessAttach.ProcessAttachTestCase.test_attach_to_process_from_different_dir_by_id) == FAIL: test_attach_to_process_by_id_correct_executable_offset (TestProcessAttach.ProcessAttachTestCase.test_attach_to_process_by_id_correct_executable_offset) Test that after attaching to a process the executable offset -- Traceback (most recent call last): File "C:\lldb-test-scripts\lldb\test\API\commands\process\attach\TestProcessAttach.py", line 119, in test_attach_to_process_by_id_correct_executable_offset lldbutil.run_break_set_by_file_and_line( File "C:\\lldb-test-scripts\lldb\packages\Python\lldbsuite\test\lldbutil.py", line 378, in run_break_set_by_file_and_line check_breakpoint_result( File "C:\\lldb-test-scripts\lldb\packages\Python\lldbsuite\test\lldbutil.py", line 592, in check_breakpoint_result test.assertTrue( AssertionError: False is not true : Expecting 1 locations, got 0. Ran 5 tests in 71.809s FAILED (failures=1) ``` https://github.com/llvm/llvm-project/pull/96256 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Parse and display register field enums (PR #95768)
@@ -4179,21 +4179,124 @@ struct GdbServerTargetInfo {
RegisterSetMap reg_set_map;
};
-static std::vector ParseFlagsFields(XMLNode flags_node,
- unsigned size) {
+static FieldEnum::Enumerators ParseEnumEvalues(const XMLNode &enum_node) {
+ Log *log(GetLog(GDBRLog::Process));
+ // We will use the last instance of each value. Also we preserve the order
+ // of declaration in the XML, as it may not be numerical.
+ std::map enumerators;
DavidSpickett wrote:
In theory it's an enum so you'd start from 0 then 1, 2,3,4, etc. but this is
hardware, so it's not going to be as regular as software enums.
The other problem with IndexedMap is that you've got to have a function
uint64_t -> size_t to make the index, so for a 32 bit lldb the naive version
ends up clipping off the top half of values. Are we likely to get a 64 bit
enumerator value? No, but it's simpler to handle it than decide on another
arbitrary cut off point.
If we ignore that issue, there is the issue of order of insertion. I'm assuming
you iterate an IndexedMap using the indexes, but if the values of the
enumerator are used for the indexes we may lose the original order.
Maybe if that requirement didn't come across clearly enough, I need to expand
the comment with an example?
Or maybe I have the wrong end of the stick entirely.
https://github.com/llvm/llvm-project/pull/95768
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Parse and display register field enums (PR #95768)
https://github.com/DavidSpickett updated
https://github.com/llvm/llvm-project/pull/95768
>From a9b542a1686be0afd73ad89a504d2c66ba6c4a4f Mon Sep 17 00:00:00 2001
From: David Spickett
Date: Mon, 11 Mar 2024 10:56:29 +
Subject: [PATCH 1/5] [lldb] Parse and display register field enums
This teaches lldb to parse the enum XML elements sent by
lldb-server, and make use of the information in `register read`
and `register info`.
The format is described in
https://sourceware.org/gdb/current/onlinedocs/gdb.html/Enum-Target-Types.html.
The target XML parser will drop any invalid enum or evalue. If
we find multiple evalue for the same value, we will use the last
one we find.
The order of evalues from the XML is preserved as there may be
good reason they are not in numerical order.
---
lldb/include/lldb/Target/RegisterFlags.h | 7 +
lldb/source/Core/DumpRegisterInfo.cpp | 7 +-
.../Process/gdb-remote/ProcessGDBRemote.cpp | 188 +-
.../Process/gdb-remote/ProcessGDBRemote.h | 5 +
.../RegisterTypeBuilderClang.cpp | 30 +-
lldb/source/Target/RegisterFlags.cpp | 10 +
.../gdb_remote_client/TestXMLRegisterFlags.py | 322 ++
lldb/unittests/Core/DumpRegisterInfoTest.cpp | 26 ++
8 files changed, 573 insertions(+), 22 deletions(-)
diff --git a/lldb/include/lldb/Target/RegisterFlags.h
b/lldb/include/lldb/Target/RegisterFlags.h
index 1c6bf5dcf4a7f..628c841c10d95 100644
--- a/lldb/include/lldb/Target/RegisterFlags.h
+++ b/lldb/include/lldb/Target/RegisterFlags.h
@@ -32,10 +32,15 @@ class FieldEnum {
: m_value(value), m_name(std::move(name)) {}
void ToXML(Stream &strm) const;
+
+void log(Log *log) const;
};
typedef std::vector Enumerators;
+ // GDB also includes a "size" that is the size of the underlying register.
+ // We will not store that here but instead use the size of the register
+ // this gets attached to when emitting XML.
FieldEnum(std::string id, const Enumerators &enumerators);
const Enumerators &GetEnumerators() const { return m_enumerators; }
@@ -44,6 +49,8 @@ class FieldEnum {
void ToXML(Stream &strm, unsigned size) const;
+ void log(Log *log) const;
+
private:
std::string m_id;
Enumerators m_enumerators;
diff --git a/lldb/source/Core/DumpRegisterInfo.cpp
b/lldb/source/Core/DumpRegisterInfo.cpp
index 8334795416902..eccc6784cd497 100644
--- a/lldb/source/Core/DumpRegisterInfo.cpp
+++ b/lldb/source/Core/DumpRegisterInfo.cpp
@@ -111,6 +111,11 @@ void lldb_private::DoDumpRegisterInfo(
};
DumpList(strm, "In sets: ", in_sets, emit_set);
- if (flags_type)
+ if (flags_type) {
strm.Printf("\n\n%s", flags_type->AsTable(terminal_width).c_str());
+
+std::string enumerators = flags_type->DumpEnums(terminal_width);
+if (enumerators.size())
+ strm << "\n\n" << enumerators;
+ }
}
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index a5a731981299f..4754698e6e88f 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4179,21 +4179,124 @@ struct GdbServerTargetInfo {
RegisterSetMap reg_set_map;
};
-static std::vector ParseFlagsFields(XMLNode flags_node,
- unsigned size) {
+static FieldEnum::Enumerators ParseEnumEvalues(const XMLNode &enum_node) {
+ Log *log(GetLog(GDBRLog::Process));
+ // We will use the last instance of each value. Also we preserve the order
+ // of declaration in the XML, as it may not be numerical.
+ std::map enumerators;
+
+ enum_node.ForEachChildElementWithName(
+ "evalue", [&enumerators, &log](const XMLNode &enumerator_node) {
+std::optional name;
+std::optional value;
+
+enumerator_node.ForEachAttribute(
+[&name, &value, &log](const llvm::StringRef &attr_name,
+ const llvm::StringRef &attr_value) {
+ if (attr_name == "name") {
+if (attr_value.size())
+ name = attr_value;
+else
+ LLDB_LOG(log, "ProcessGDBRemote::ParseEnumEvalues "
+"Ignoring empty name in evalue");
+ } else if (attr_name == "value") {
+uint64_t parsed_value = 0;
+if (llvm::to_integer(attr_value, parsed_value))
+ value = parsed_value;
+else
+ LLDB_LOG(log,
+ "ProcessGDBRemote::ParseEnumEvalues "
+ "Invalid value \"{0}\" in "
+ "evalue",
+ attr_value.data());
+ } else
+LLDB_LOG(log,
+ "ProcessGDBRemote::ParseEnumEvalues Ignoring "
+ "unknown attribute "
+
[Lldb-commits] [lldb] [lldb] Parse and display register field enums (PR #95768)
@@ -4179,21 +4179,124 @@ struct GdbServerTargetInfo {
RegisterSetMap reg_set_map;
};
-static std::vector ParseFlagsFields(XMLNode flags_node,
- unsigned size) {
+static FieldEnum::Enumerators ParseEnumEvalues(const XMLNode &enum_node) {
+ Log *log(GetLog(GDBRLog::Process));
+ // We will use the last instance of each value. Also we preserve the order
+ // of declaration in the XML, as it may not be numerical.
+ std::map enumerators;
DavidSpickett wrote:
I've expanded the comment anyway.
I don't think anyone on the GDB side ever considered that order might matter,
because I don't think they have a `register info` equivalent. You can print the
type of the register but that's more of a C type, where enumerators being
sorted isn't unexpected.
https://github.com/llvm/llvm-project/pull/95768
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Parse and display register field enums (PR #95768)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/95768 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Parse and display register field enums (PR #95768)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/95768 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Use `Address` to setup breakpoint (PR #94794)
DavidSpickett wrote: I'm assuming that `Address` keeps track of the value if it changes due to position independent code, ASLR, etc. between runs. So this seems fine. Can this be tested? Not sure what coverage we have of these plugins right now. https://github.com/llvm/llvm-project/pull/94794 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix string truncation method when substring is the prefix of string (NFC) (PR #94785)
https://github.com/DavidSpickett requested changes to this pull request. https://github.com/llvm/llvm-project/pull/94785 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix string truncation method when substring is the prefix of string (NFC) (PR #94785)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/94785 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix string truncation method when substring is the prefix of string (NFC) (PR #94785)
@@ -287,7 +287,7 @@ Status PlatformAndroid::DownloadModuleSlice(const FileSpec
&src_file_spec,
static constexpr llvm::StringLiteral k_zip_separator("!/");
size_t pos = source_file.find(k_zip_separator);
if (pos != std::string::npos)
-source_file = source_file.substr(0, pos);
+source_file = source_file.resize(0, pos);
DavidSpickett wrote:
According to https://en.cppreference.com/w/cpp/string/basic_string/resize,
`resize(pos)` would be correct here. Otherwise we're resizing to 0 characters.
https://github.com/llvm/llvm-project/pull/94785
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] fd35a92 - [lldb] fix(lldb/**.py): fix comparison to True/False (#94039)
Author: Eisuke Kawashima
Date: 2024-06-26T15:55:15+01:00
New Revision: fd35a92300a00edaf56ae94176317390677569a4
URL:
https://github.com/llvm/llvm-project/commit/fd35a92300a00edaf56ae94176317390677569a4
DIFF:
https://github.com/llvm/llvm-project/commit/fd35a92300a00edaf56ae94176317390677569a4.diff
LOG: [lldb] fix(lldb/**.py): fix comparison to True/False (#94039)
from PEP8
(https://peps.python.org/pep-0008/#programming-recommendations):
> Comparisons to singletons like None should always be done with is or
is not, never the equality operators.
Co-authored-by: Eisuke Kawashima
Added:
Modified:
lldb/examples/python/crashlog.py
lldb/examples/python/disasm-stress-test.py
lldb/examples/summaries/cocoa/CFString.py
lldb/examples/summaries/pysummary.py
lldb/examples/synthetic/bitfield/example.py
lldb/packages/Python/lldbsuite/test/lldbtest.py
lldb/test/API/commands/command/script/welcome.py
lldb/test/API/commands/expression/call-throws/TestCallThatThrows.py
lldb/test/API/functionalities/disassemble/aarch64-adrp-add/TestAArch64AdrpAdd.py
lldb/test/API/functionalities/gdb_remote_client/TestNoWatchpointSupportInfo.py
lldb/test/API/tools/lldb-server/TestAppleSimulatorOSType.py
lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
Removed:
diff --git a/lldb/examples/python/crashlog.py
b/lldb/examples/python/crashlog.py
index 1c0d717ce455c..368437ed63e46 100755
--- a/lldb/examples/python/crashlog.py
+++ b/lldb/examples/python/crashlog.py
@@ -166,7 +166,7 @@ def dump_symbolicated(self, crash_log, options):
this_thread_crashed = self.app_specific_backtrace
if not this_thread_crashed:
this_thread_crashed = self.did_crash()
-if options.crashed_only and this_thread_crashed == False:
+if options.crashed_only and not this_thread_crashed:
return
print("%s" % self)
diff --git a/lldb/examples/python/disasm-stress-test.py
b/lldb/examples/python/disasm-stress-test.py
index 2d3314ee8e8ff..62b2b90a2860a 100755
--- a/lldb/examples/python/disasm-stress-test.py
+++ b/lldb/examples/python/disasm-stress-test.py
@@ -95,13 +95,13 @@ def GetLLDBFrameworkPath():
debugger = lldb.SBDebugger.Create()
-if debugger.IsValid() == False:
+if not debugger.IsValid():
print("Couldn't create an SBDebugger")
sys.exit(-1)
target = debugger.CreateTargetWithFileAndArch(None, arg_ns.arch)
-if target.IsValid() == False:
+if not target.IsValid():
print("Couldn't create an SBTarget for architecture " + arg_ns.arch)
sys.exit(-1)
diff --git a/lldb/examples/summaries/cocoa/CFString.py
b/lldb/examples/summaries/cocoa/CFString.py
index 13c294ca34122..74bd927e9db21 100644
--- a/lldb/examples/summaries/cocoa/CFString.py
+++ b/lldb/examples/summaries/cocoa/CFString.py
@@ -253,9 +253,9 @@ def get_child_at_index(self, index):
elif (
self.inline
and self.explicit
-and self.unicode == False
-and self.special == False
-and self.mutable == False
+and not self.unicode
+and not self.special
+and not self.mutable
):
return self.handle_inline_explicit()
elif self.unicode:
diff --git a/lldb/examples/summaries/pysummary.py
b/lldb/examples/summaries/pysummary.py
index e63a0bff56a13..2a05c1cbf8f28 100644
--- a/lldb/examples/summaries/pysummary.py
+++ b/lldb/examples/summaries/pysummary.py
@@ -2,7 +2,7 @@
def pyobj_summary(value, unused):
-if value is None or value.IsValid() == False or
value.GetValueAsUnsigned(0) == 0:
+if value is None or not value.IsValid() or value.GetValueAsUnsigned(0) ==
0:
return ""
refcnt = value.GetChildMemberWithName("ob_refcnt")
expr = "(char*)PyString_AsString( (PyObject*)PyObject_Str(
(PyObject*)0x%x) )" % (
diff --git a/lldb/examples/synthetic/bitfield/example.py
b/lldb/examples/synthetic/bitfield/example.py
index 2f58123268aa1..45416477bfef2 100644
--- a/lldb/examples/synthetic/bitfield/example.py
+++ b/lldb/examples/synthetic/bitfield/example.py
@@ -51,7 +51,7 @@ def get_child_at_index(self, index):
return None
if index > self.num_children():
return None
-if self.valobj.IsValid() == False:
+if not self.valobj.IsValid():
return None
if index == 0:
return self.valobj.GetChildMemberWithName("value")
diff --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 1ad8ab6e6e462..1854f6c2c2e7b 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2446,7 +2446,7 @@ def found_str(matched):
log_lines.append(pattern_li
[Lldb-commits] [lldb] [lldb] fix(lldb/**.py): fix comparison to True/False (PR #94039)
https://github.com/DavidSpickett closed https://github.com/llvm/llvm-project/pull/94039 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] fix(lldb/**.py): fix comparison to True/False (PR #94039)
github-actions[bot] wrote: @e-kwsm Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our [build bots](https://lab.llvm.org/buildbot/). If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail [here](https://llvm.org/docs/MyFirstTypoFix.html#myfirsttypofix-issues-after-landing-your-pr). If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of [LLVM development](https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy). You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! https://github.com/llvm/llvm-project/pull/94039 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix incorrect logical operator in 'if' condition check (NFC) (PR #94779)
@@ -85,13 +85,17 @@ class ScriptedPythonInterface : virtual public
ScriptedInterface {
bool has_class_name = !class_name.empty();
bool has_interpreter_dict =
!(llvm::StringRef(m_interpreter.GetDictionaryName()).empty());
-if (!has_class_name && !has_interpreter_dict && !script_obj) {
- if (!has_class_name)
-return create_error("Missing script class name.");
- else if (!has_interpreter_dict)
-return create_error("Invalid script interpreter dictionary.");
- else
-return create_error("Missing scripting object.");
+
+if (!has_class_name) {
+ return create_error("Missing script class name.");
+}
DavidSpickett wrote:
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements
https://github.com/llvm/llvm-project/pull/94779
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 5861145 - [lldb] fix(lldb/**.py): fix comparison to None (#94017)
Author: Eisuke Kawashima Date: 2024-06-26T15:59:07+01:00 New Revision: 586114510c5fa71d1377c7f53e68a3b12c472aa2 URL: https://github.com/llvm/llvm-project/commit/586114510c5fa71d1377c7f53e68a3b12c472aa2 DIFF: https://github.com/llvm/llvm-project/commit/586114510c5fa71d1377c7f53e68a3b12c472aa2.diff LOG: [lldb] fix(lldb/**.py): fix comparison to None (#94017) from PEP8 (https://peps.python.org/pep-0008/#programming-recommendations): > Comparisons to singletons like None should always be done with is or is not, never the equality operators. Co-authored-by: Eisuke Kawashima Added: Modified: lldb/bindings/interface/SBBreakpointDocstrings.i lldb/bindings/interface/SBDataExtensions.i lldb/docs/use/python.rst lldb/examples/python/armv7_cortex_m_target_defintion.py lldb/packages/Python/lldbsuite/test/dotest.py lldb/packages/Python/lldbsuite/test/lldbtest.py lldb/packages/Python/lldbsuite/test/lldbutil.py lldb/packages/Python/lldbsuite/test/tools/intelpt/intelpt_testcase.py lldb/test/API/functionalities/breakpoint/address_breakpoints/TestBadAddressBreakpoints.py lldb/test/API/functionalities/step_scripted/TestStepScripted.py lldb/test/API/functionalities/stop-on-sharedlibrary-load/TestStopOnSharedlibraryEvents.py lldb/test/API/lua_api/TestLuaAPI.py lldb/test/API/macosx/thread_suspend/TestInternalThreadSuspension.py lldb/test/API/python_api/event/TestEvents.py lldb/test/API/python_api/process/read-mem-cstring/TestReadMemCString.py lldb/test/API/python_api/type/TestTypeList.py lldb/test/API/python_api/was_interrupted/interruptible.py lldb/test/Shell/lit.cfg.py Removed: diff --git a/lldb/bindings/interface/SBBreakpointDocstrings.i b/lldb/bindings/interface/SBBreakpointDocstrings.i index 74c139d5d9fb6..dca2819a9927b 100644 --- a/lldb/bindings/interface/SBBreakpointDocstrings.i +++ b/lldb/bindings/interface/SBBreakpointDocstrings.i @@ -39,7 +39,7 @@ TestBreakpointIgnoreCount.py),:: #lldbutil.print_stacktraces(process) from lldbutil import get_stopped_thread thread = get_stopped_thread(process, lldb.eStopReasonBreakpoint) -self.assertTrue(thread != None, 'There should be a thread stopped due to breakpoint') +self.assertTrue(thread is not None, 'There should be a thread stopped due to breakpoint') frame0 = thread.GetFrameAtIndex(0) frame1 = thread.GetFrameAtIndex(1) frame2 = thread.GetFrameAtIndex(2) diff --git a/lldb/bindings/interface/SBDataExtensions.i b/lldb/bindings/interface/SBDataExtensions.i index d980e79221c6d..ddea77a088dfa 100644 --- a/lldb/bindings/interface/SBDataExtensions.i +++ b/lldb/bindings/interface/SBDataExtensions.i @@ -40,19 +40,19 @@ STRING_EXTENSION_OUTSIDE(SBData) lldbtarget = lldbdict['target'] else: lldbtarget = None -if target == None and lldbtarget != None and lldbtarget.IsValid(): +if target is None and lldbtarget is not None and lldbtarget.IsValid(): target = lldbtarget -if ptr_size == None: +if ptr_size is None: if target and target.IsValid(): ptr_size = target.addr_size else: ptr_size = 8 -if endian == None: +if endian is None: if target and target.IsValid(): endian = target.byte_order else: endian = lldbdict['eByteOrderLittle'] -if size == None: +if size is None: if value > 2147483647: size = 8 elif value < -2147483648: diff --git a/lldb/docs/use/python.rst b/lldb/docs/use/python.rst index 6183d6935d80e..d9c29d95708c1 100644 --- a/lldb/docs/use/python.rst +++ b/lldb/docs/use/python.rst @@ -75,13 +75,13 @@ later explanations: 12: if root_word == word: 13: return cur_path 14: elif word < root_word: - 15: if left_child_ptr.GetValue() == None: + 15: if left_child_ptr.GetValue() is None: 16: return "" 17: else: 18: cur_path = cur_path + "L" 19: return DFS (left_child_ptr, word, cur_path) 20: else: - 21: if right_child_ptr.GetValue() == None: + 21: if right_child_ptr.GetValue() is None: 22: return "" 23: else: 24: cur_path = cur_path + "R" diff --git a/lldb/examples/python/armv7_cortex_m_target_defintion.py b/lldb/examples/python/armv7_cortex_m_target_defintion.py index 42eaa39993dae..8225670f33e6b 100755 --- a/lldb/examples/python/armv7_cortex_m_target_defintion.py +++ b/lldb/examples/python/armv7_cortex_m_target_defintion.py @@ -222,7 +222,7 @@ def get_reg_num(reg_num_dict, reg_name): def get_ta
[Lldb-commits] [lldb] [lldb] fix(lldb/**.py): fix comparison to None (PR #94017)
https://github.com/DavidSpickett closed https://github.com/llvm/llvm-project/pull/94017 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] fix(lldb/**.py): fix comparison to None (PR #94017)
github-actions[bot] wrote: @e-kwsm Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our [build bots](https://lab.llvm.org/buildbot/). If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail [here](https://llvm.org/docs/MyFirstTypoFix.html#myfirsttypofix-issues-after-landing-your-pr). If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of [LLVM development](https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy). You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! https://github.com/llvm/llvm-project/pull/94017 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Don't start class definitions in ParseStructureLikeDIE (PR #96755)
https://github.com/Michael137 approved this pull request. This is a really nice cleanup! It actually aligns almost exactly with [our in-progress version of this](https://github.com/llvm/llvm-project/blob/caacb57a46f34bf663fa5ab2190b361ce29b255b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp). LGTM Agree with your point about `PrepareContextToReceiveMembers`. We can add those as needed. In our version of this I had to only add it in `ParseSubroutine`, as you've also effectively done. https://github.com/llvm/llvm-project/pull/96755 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Don't start class definitions in ParseStructureLikeDIE (PR #96755)
@@ -1893,72 +1849,21 @@ DWARFASTParserClang::ParseStructureLikeDIE(const
SymbolContext &sc,
dwarf->GetUniqueDWARFASTTypeMap().Insert(unique_typename,
*unique_ast_entry_up);
- if (!attrs.is_forward_declaration) {
-// Always start the definition for a class type so that if the class
-// has child classes or types that require the class to be created
-// for use as their decl contexts the class will be ready to accept
-// these child definitions.
-if (!def_die.HasChildren()) {
- // No children for this struct/union/class, lets finish it
- if (TypeSystemClang::StartTagDeclarationDefinition(clang_type)) {
-TypeSystemClang::CompleteTagDeclarationDefinition(clang_type);
- } else {
-dwarf->GetObjectFile()->GetModule()->ReportError(
-
-"DWARF DIE {0:x16} named \"{1}\" was not able to start its "
-"definition.\nPlease file a bug and attach the file at the "
-"start of this error message",
-def_die.GetID(), attrs.name.GetCString());
- }
-
- // Setting authority byte size and alignment for empty structures.
- //
- // If the byte size or alignmenet of the record is specified then
- // overwrite the ones that would be computed by Clang.
- // This is only needed as LLDB's TypeSystemClang is always in C++ mode,
- // but some compilers such as GCC and Clang give empty structs a size of 0
- // in C mode (in contrast to the size of 1 for empty structs that would
be
- // computed in C++ mode).
- if (attrs.byte_size || attrs.alignment) {
-clang::RecordDecl *record_decl =
-TypeSystemClang::GetAsRecordDecl(clang_type);
-if (record_decl) {
- ClangASTImporter::LayoutInfo layout;
- layout.bit_size = attrs.byte_size.value_or(0) * 8;
- layout.alignment = attrs.alignment.value_or(0) * 8;
- GetClangASTImporter().SetRecordLayout(record_decl, layout);
-}
- }
-} else if (clang_type_was_created) {
- // Start the definition if the class is not objective C since the
- // underlying decls respond to isCompleteDefinition(). Objective
- // C decls don't respond to isCompleteDefinition() so we can't
- // start the declaration definition right away. For C++
- // class/union/structs we want to start the definition in case the
- // class is needed as the declaration context for a contained class
- // or type without the need to complete that type..
-
- if (attrs.class_language != eLanguageTypeObjC &&
- attrs.class_language != eLanguageTypeObjC_plus_plus)
-TypeSystemClang::StartTagDeclarationDefinition(clang_type);
-
- // Leave this as a forward declaration until we need to know the
- // details of the type. lldb_private::Type will automatically call
- // the SymbolFile virtual function
- // "SymbolFileDWARF::CompleteType(Type *)" When the definition
- // needs to be defined.
- assert(!dwarf->GetForwardDeclCompilerTypeToDIE().count(
- ClangUtil::RemoveFastQualifiers(clang_type)
- .GetOpaqueQualType()) &&
- "Type already in the forward declaration map!");
- // Can't assume m_ast.GetSymbolFile() is actually a
- // SymbolFileDWARF, it can be a SymbolFileDWARFDebugMap for Apple
- // binaries.
- dwarf->GetForwardDeclCompilerTypeToDIE().try_emplace(
- ClangUtil::RemoveFastQualifiers(clang_type).GetOpaqueQualType(),
- *def_die.GetDIERef());
- m_ast.SetHasExternalStorage(clang_type.GetOpaqueQualType(), true);
-}
+ if (clang_type_was_created) {
Michael137 wrote:
Could there be any consequence of doing this for empty types now? We might end
up with some lookups into empty types? Since we're going to turn off the
external storage bit in `CompleteRecordTypeFromDWARF` this is probably fine
though?
https://github.com/llvm/llvm-project/pull/96755
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Don't start class definitions in ParseStructureLikeDIE (PR #96755)
@@ -2192,87 +2097,82 @@ bool DWARFASTParserClang::CompleteRecordType(const
DWARFDIE &die,
ClangASTImporter::LayoutInfo layout_info;
std::vector contained_type_dies;
- if (die.HasChildren()) {
-const bool type_is_objc_object_or_interface =
-TypeSystemClang::IsObjCObjectOrInterfaceType(clang_type);
-if (type_is_objc_object_or_interface) {
- // For objective C we don't start the definition when the class is
- // created.
- TypeSystemClang::StartTagDeclarationDefinition(clang_type);
-}
-
-AccessType default_accessibility = eAccessNone;
-if (tag == DW_TAG_structure_type) {
- default_accessibility = eAccessPublic;
-} else if (tag == DW_TAG_union_type) {
- default_accessibility = eAccessPublic;
-} else if (tag == DW_TAG_class_type) {
- default_accessibility = eAccessPrivate;
-}
-
-std::vector> bases;
-// Parse members and base classes first
-std::vector member_function_dies;
-
-DelayedPropertyList delayed_properties;
-ParseChildMembers(die, clang_type, bases, member_function_dies,
- contained_type_dies, delayed_properties,
- default_accessibility, layout_info);
-
-// Now parse any methods if there were any...
-for (const DWARFDIE &die : member_function_dies)
- dwarf->ResolveType(die);
-
-if (type_is_objc_object_or_interface) {
- ConstString class_name(clang_type.GetTypeName());
- if (class_name) {
-dwarf->GetObjCMethods(class_name, [&](DWARFDIE method_die) {
- method_die.ResolveType();
- return true;
-});
+ if (die.GetAttributeValueAsUnsigned(DW_AT_declaration, 0))
+return false; // No definition, cannot complete.
-for (DelayedAddObjCClassProperty &property : delayed_properties)
- property.Finalize();
- }
-}
+ // Start the definition if the type is not being defined already. This can
+ // happen (e.g.) when adding nested types to a class type -- see
+ // PrepareContextToReceiveMembers.
+ if (!clang_type.IsBeingDefined())
+TypeSystemClang::StartTagDeclarationDefinition(clang_type);
-if (!bases.empty()) {
- // Make sure all base classes refer to complete types and not forward
- // declarations. If we don't do this, clang will crash with an
- // assertion in the call to clang_type.TransferBaseClasses()
- for (const auto &base_class : bases) {
-clang::TypeSourceInfo *type_source_info =
-base_class->getTypeSourceInfo();
-if (type_source_info)
- TypeSystemClang::RequireCompleteType(
- m_ast.GetType(type_source_info->getType()));
- }
+ AccessType default_accessibility = eAccessNone;
+ if (tag == DW_TAG_structure_type) {
+default_accessibility = eAccessPublic;
+ } else if (tag == DW_TAG_union_type) {
+default_accessibility = eAccessPublic;
+ } else if (tag == DW_TAG_class_type) {
+default_accessibility = eAccessPrivate;
+ }
+
+ std::vector> bases;
+ // Parse members and base classes first
+ std::vector member_function_dies;
- m_ast.TransferBaseClasses(clang_type.GetOpaqueQualType(),
-std::move(bases));
+ DelayedPropertyList delayed_properties;
+ ParseChildMembers(die, clang_type, bases, member_function_dies,
+contained_type_dies, delayed_properties,
+default_accessibility, layout_info);
+
+ // Now parse any methods if there were any...
+ for (const DWARFDIE &die : member_function_dies)
+dwarf->ResolveType(die);
+
+ if (TypeSystemClang::IsObjCObjectOrInterfaceType(clang_type)) {
+ConstString class_name(clang_type.GetTypeName());
+if (class_name) {
+ dwarf->GetObjCMethods(class_name, [&](DWARFDIE method_die) {
+method_die.ResolveType();
+return true;
+ });
+
+ for (DelayedAddObjCClassProperty &property : delayed_properties)
+property.Finalize();
}
}
+ if (!bases.empty()) {
+// Make sure all base classes refer to complete types and not forward
+// declarations. If we don't do this, clang will crash with an
+// assertion in the call to clang_type.TransferBaseClasses()
+for (const auto &base_class : bases) {
+ clang::TypeSourceInfo *type_source_info =
base_class->getTypeSourceInfo();
+ if (type_source_info)
+TypeSystemClang::RequireCompleteType(
+m_ast.GetType(type_source_info->getType()));
+}
+
+m_ast.TransferBaseClasses(clang_type.GetOpaqueQualType(),
std::move(bases));
+ }
+
m_ast.AddMethodOverridesForCXXRecordType(clang_type.GetOpaqueQualType());
TypeSystemClang::BuildIndirectFields(clang_type);
TypeSystemClang::CompleteTagDeclarationDefinition(clang_type);
- if (!layout_info.field_offsets.empty() || !layout_info.base_offsets.empty()
||
- !layout_info.vbase_offsets.empty()) {
Michael137 wrote:
Agreed, don't really see an issue with
[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #92328)
jeffreytan81 wrote: Thanks for the update! Not sure why but the github notification system seems to fail me recently which I did not get any update emails from this thread until proactively checking now. Sounds good! Let us know the final stack/PR for the delaying forward declaration is up, we are happy to perform early testing against internal targets. cc @clayborg, @kusmour https://github.com/llvm/llvm-project/pull/92328 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix string truncation method when substring is the prefix of string (NFC) (PR #94785)
https://github.com/clayborg edited https://github.com/llvm/llvm-project/pull/94785 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix string truncation method when substring is the prefix of string (NFC) (PR #94785)
https://github.com/clayborg requested changes to this pull request. https://github.com/llvm/llvm-project/pull/94785 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix string truncation method when substring is the prefix of string (NFC) (PR #94785)
@@ -287,7 +287,7 @@ Status PlatformAndroid::DownloadModuleSlice(const FileSpec
&src_file_spec,
static constexpr llvm::StringLiteral k_zip_separator("!/");
size_t pos = source_file.find(k_zip_separator);
if (pos != std::string::npos)
-source_file = source_file.substr(0, pos);
+source_file = source_file.resize(0, pos);
clayborg wrote:
`std::string::resize()` returns `void` so this won't even compile. The right
fix is:
```
source_file.resize(pos);
```
https://github.com/llvm/llvm-project/pull/94785
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Windows Build]Removed header and validated on new windows machine (PR #96724)
https://github.com/Jlalond edited https://github.com/llvm/llvm-project/pull/96724 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Use `Address` to setup breakpoint (PR #94794)
@@ -90,17 +90,9 @@ void InstrumentationRuntimeASanLibsanitizers::Activate() {
if (!process_sp)
return;
- lldb::ModuleSP module_sp = GetRuntimeModuleSP();
-
Breakpoint *breakpoint = ReportRetriever::SetupBreakpoint(
- module_sp, process_sp, ConstString("sanitizers_address_on_report"));
-
- if (!breakpoint) {
-breakpoint = ReportRetriever::SetupBreakpoint(
-module_sp, process_sp,
-ConstString("_Z22raise_sanitizers_error23sanitizer_error_context"));
- }
-
+ GetRuntimeModuleSP(), process_sp,
+ ConstString("sanitizers_address_on_report"));
clayborg wrote:
This doesn't look right. Is this left over from the older code? Remove this?
https://github.com/llvm/llvm-project/pull/94794
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Use `Address` to setup breakpoint (PR #94794)
https://github.com/clayborg requested changes to this pull request. https://github.com/llvm/llvm-project/pull/94794 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Use `Address` to setup breakpoint (PR #94794)
https://github.com/clayborg edited https://github.com/llvm/llvm-project/pull/94794 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Minidump] Change expected directories to the correct type; size_t (PR #96564)
Jlalond wrote: @DavidSpickett We'll be fine with 32b size_t. Directories in minidumps have to all have their offsets be 32b addressable, so if you were to fill the directory list up to the `size_t` max, it would be a corrupted minidump https://github.com/llvm/llvm-project/pull/96564 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Unique enums parsed from declarations (PR #96751)
@@ -1798,6 +1805,13 @@ DWARFASTParserClang::ParseStructureLikeDIE(const
SymbolContext &sc,
}
if (def_die) {
+if (auto [it, inserted] = dwarf->GetDIEToType().try_emplace(
dwblaikie wrote:
any chance of reducing/avoiding this duplication? Looks like the same code here
as above? (refactor into a common function?)
https://github.com/llvm/llvm-project/pull/96751
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)
dwblaikie wrote: > It looks like the test is flaky: > https://lab.llvm.org/buildbot/#/builders/59/builds/535 > > It looks like the order of the types is nondeterministic. I think you should > be able to use CHECK-DAG along with [this trick to match > newlines](https://llvm.org/docs/CommandGuide/FileCheck.html#matching-newline-characters) > to make the test accept both orderings. What are lldb's guarantees in this regard? Clang/LLVM/etc require nondeterministic output - presumably the same would be valuable to lldb, but I don't know what lldb's precedents are in this regard. https://github.com/llvm/llvm-project/pull/87740 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add format eFormatEnumWithValues to ensure raw enum value is always shown (PR #90059)
clayborg wrote: Sorry for the delay, I am still wondering if we just want to have the value objects for enums return the text name as the value, and have the summary show the value as an appropriate integer if the format is eFormatEnum. When you display registers bitfields do you have a ValueObject? If so we can modify the `ValueObject::GetSummary()` to return the integer value, but only if we have the format set to `eFormatEnum`. The output for variables would be similar to what your tests expect, but we would just modify the summary string that we return and then we don't need the new format. One reason I like this way of doing things is if you call "valobj.GetValue()", and the format is set the enum with value, we will get "foo (1)" as the value back from the value object. I would rather call `valobj.GetValue()` and get `"foo"` and then call `valojb.GetSummary()` and get `"1"`, or of course you can always just call `valobj.GetValueAsUnsigned(...)` or `valobj.GetValueAsSigned(...)`. If we have special display code for the register bitfields, they can call these APIs when displaying register by default? https://github.com/llvm/llvm-project/pull/90059 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix the test to deal with non-deterministic output. (PR #96800)
https://github.com/clayborg created
https://github.com/llvm/llvm-project/pull/96800
When we check for the "struct CustomType" in the NODWP, we can just make sure
that we have both types showing up, the next tests will validate the types are
correct. Also added a "-DAG" to the integer and float types. This should fix
the flakiness in this test.
>From a874efd2b746c1f4fcf426f7d2426d28230e7714 Mon Sep 17 00:00:00 2001
From: Greg Clayton
Date: Wed, 26 Jun 2024 10:12:05 -0700
Subject: [PATCH] Fix the test to deal with non-deterministic output.
When we check for the "struct CustomType" in the NODWP, we can just make sure
that we have both types showing up, the next tests will validate the types are
correct. Also added a "-DAG" to the integer and float types. This should fix
the flakiness in this test.
---
.../DWARF/x86/dwp-foreign-type-units.cpp | 22 +--
1 file changed, 6 insertions(+), 16 deletions(-)
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp
b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp
index 8dd5a5472ed4c..4df1b33dd7d91 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp
@@ -30,24 +30,14 @@
// RUN: -o "type lookup CustomType" \
// RUN: -b %t | FileCheck %s --check-prefix=NODWP
// NODWP: (lldb) type lookup IntegerType
-// NODWP-NEXT: int
-// NODWP-NEXT: unsigned int
+// NODWP-DAG: int
+// NODWP-DAG: unsigned int
// NODWP: (lldb) type lookup FloatType
-// NODWP-NEXT: double
-// NODWP-NEXT: float
+// NODWP-DAG: double
+// NODWP-DAG: float
// NODWP: (lldb) type lookup CustomType
-// NODWP-NEXT: struct CustomType {
-// NODWP-NEXT: typedef int IntegerType;
-// NODWP-NEXT: typedef double FloatType;
-// NODWP-NEXT: CustomType::IntegerType x;
-// NODWP-NEXT: CustomType::FloatType y;
-// NODWP-NEXT: }
-// NODWP-NEXT: struct CustomType {
-// NODWP-NEXT: typedef unsigned int IntegerType;
-// NODWP-NEXT: typedef float FloatType;
-// NODWP-NEXT: CustomType::IntegerType x;
-// NODWP-NEXT: CustomType::FloatType y;
-// NODWP-NEXT: }
+// NODWP: struct CustomType {
+// NODWP: struct CustomType {
// Check when we make the .dwp file with %t.main.dwo first so it will
// pick the type unit from %t.main.dwo. Verify we find only the types from
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix the test to deal with non-deterministic output. (PR #96800)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Greg Clayton (clayborg)
Changes
When we check for the "struct CustomType" in the NODWP, we can just make sure
that we have both types showing up, the next tests will validate the types are
correct. Also added a "-DAG" to the integer and float types. This should fix
the flakiness in this test.
---
Full diff: https://github.com/llvm/llvm-project/pull/96800.diff
1 Files Affected:
- (modified) lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp
(+6-16)
``diff
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp
b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp
index 8dd5a5472ed4c..4df1b33dd7d91 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp
@@ -30,24 +30,14 @@
// RUN: -o "type lookup CustomType" \
// RUN: -b %t | FileCheck %s --check-prefix=NODWP
// NODWP: (lldb) type lookup IntegerType
-// NODWP-NEXT: int
-// NODWP-NEXT: unsigned int
+// NODWP-DAG: int
+// NODWP-DAG: unsigned int
// NODWP: (lldb) type lookup FloatType
-// NODWP-NEXT: double
-// NODWP-NEXT: float
+// NODWP-DAG: double
+// NODWP-DAG: float
// NODWP: (lldb) type lookup CustomType
-// NODWP-NEXT: struct CustomType {
-// NODWP-NEXT: typedef int IntegerType;
-// NODWP-NEXT: typedef double FloatType;
-// NODWP-NEXT: CustomType::IntegerType x;
-// NODWP-NEXT: CustomType::FloatType y;
-// NODWP-NEXT: }
-// NODWP-NEXT: struct CustomType {
-// NODWP-NEXT: typedef unsigned int IntegerType;
-// NODWP-NEXT: typedef float FloatType;
-// NODWP-NEXT: CustomType::IntegerType x;
-// NODWP-NEXT: CustomType::FloatType y;
-// NODWP-NEXT: }
+// NODWP: struct CustomType {
+// NODWP: struct CustomType {
// Check when we make the .dwp file with %t.main.dwo first so it will
// pick the type unit from %t.main.dwo. Verify we find only the types from
``
https://github.com/llvm/llvm-project/pull/96800
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)
clayborg wrote: Here is a PR to fix the flaky test: https://github.com/llvm/llvm-project/pull/96800 https://github.com/llvm/llvm-project/pull/87740 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Unique enums parsed from declarations (PR #96751)
https://github.com/clayborg approved this pull request. https://github.com/llvm/llvm-project/pull/96751 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix the test to deal with non-deterministic output. (PR #96800)
https://github.com/labath approved this pull request. https://github.com/llvm/llvm-project/pull/96800 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] DebugInfoD tests: attempt to fix Fuchsia build (PR #96802)
https://github.com/kevinfrei created
https://github.com/llvm/llvm-project/pull/96802
This is the same diff I've put up at many times before. I've been trying to add
some brand new functionality to the LLDB test infrastucture, and we all know
that no good deed goes unpunished. The last attempt was reverted because it
didn't work on the *Fuchsia* build.
My understanding of Fuchsia is based on the only thing I know about it: one of
my friends was moved from the project when it was basically killed during the
Google layoffs in January 2023. I really don't understand the value of the
LLVM/LLDB infra requiring functional Fuchsia builds when it's such a weird,
tiny, _possibly dead_ niche. But I've put this off for long enough, so I've
added a single bit of logic to the test the dwp Makefile.rules assignment to
try using llvm-dwp if it's sitting in one particular place. I would give this
particular change a chance of addressing the fuchsia problem at about 10%, but
I've got to start somewhere.
If anyone can fix the Fuchsia build, or help me figure out how to disable some
tests for Fuchsia (because it actively lies to the infra and claims it's linux
:( ) I'd really appreciate the help.
>From 5db458e34f9db981d78bdac0ba18a97d506fb404 Mon Sep 17 00:00:00 2001
From: Kevin Frei
Date: Mon, 25 Mar 2024 08:23:47 -0700
Subject: [PATCH 01/12] Trying to deal with Linux AArch64 test failures :/
---
.../SymbolVendor/ELF/SymbolVendorELF.cpp | 18 ++
.../API/debuginfod/Normal/TestDebuginfod.py | 187 +
.../SplitDWARF/TestDebuginfodDWP.py | 196 ++
3 files changed, 401 insertions(+)
create mode 100644 lldb/test/API/debuginfod/Normal/TestDebuginfod.py
create mode 100644 lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py
diff --git a/lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
b/lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
index b5fe35d71032a..a881218a56cef 100644
--- a/lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
+++ b/lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
@@ -44,6 +44,24 @@ llvm::StringRef
SymbolVendorELF::GetPluginDescriptionStatic() {
"executables.";
}
+// If this is needed elsewhere, it can be exported/moved.
+static bool IsDwpSymbolFile(const lldb::ModuleSP &module_sp,
+const FileSpec &file_spec) {
+ DataBufferSP dwp_file_data_sp;
+ lldb::offset_t dwp_file_data_offset = 0;
+ // Try to create an ObjectFile from the file_spec.
+ ObjectFileSP dwp_obj_file = ObjectFile::FindPlugin(
+ module_sp, &file_spec, 0, FileSystem::Instance().GetByteSize(file_spec),
+ dwp_file_data_sp, dwp_file_data_offset);
+ // The presence of a debug_cu_index section is the key identifying feature of
+ // a DWP file. Make sure we don't fill in the section list on dwp_obj_file
+ // (by calling GetSectionList(false)) as this is invoked before we may have
+ // all the symbol files collected and available.
+ return dwp_obj_file && ObjectFileELF::classof(dwp_obj_file.get()) &&
+ dwp_obj_file->GetSectionList(false)->FindSectionByType(
+ eSectionTypeDWARFDebugCuIndex, false);
+}
+
// CreateInstance
//
// Platforms can register a callback to use when creating symbol vendors to
diff --git a/lldb/test/API/debuginfod/Normal/TestDebuginfod.py
b/lldb/test/API/debuginfod/Normal/TestDebuginfod.py
new file mode 100644
index 0..a662fb9fc6e68
--- /dev/null
+++ b/lldb/test/API/debuginfod/Normal/TestDebuginfod.py
@@ -0,0 +1,187 @@
+import os
+import shutil
+import tempfile
+import struct
+
+import lldb
+from lldbsuite.test.decorators import *
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+def getUUID(aoutuuid):
+"""
+Pull the 20 byte UUID out of the .note.gnu.build-id section that was dumped
+to a file already, as part of the build.
+"""
+with open(aoutuuid, "rb") as f:
+data = f.read(36)
+if len(data) != 36:
+return None
+header = struct.unpack_from("<4I", data)
+if len(header) != 4:
+return None
+# 4 element 'prefix', 20 bytes of uuid, 3 byte long string: 'GNU':
+if header[0] != 4 or header[1] != 20 or header[2] != 3 or header[3] !=
0x554E47:
+return None
+return data[16:].hex()
+
+
+"""
+Test support for the DebugInfoD network symbol acquisition protocol.
+This one is for simple / no split-dwarf scenarios.
+
+For no-split-dwarf scenarios, there are 2 variations:
+1 - A stripped binary with it's corresponding unstripped binary:
+2 - A stripped binary with a corresponding --only-keep-debug symbols file
+"""
+
+
+@skipUnlessPlatform(["linux", "freebsd"])
+class DebugInfodTests(TestBase):
+# No need to try every flavor of debug inf.
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_normal_no_symbols(self):
+"""
+Validate behavior with no symbols or symbol locator.
+
[Lldb-commits] [lldb] [lldb/DWARF] Unique enums parsed from declarations (PR #96751)
@@ -1798,6 +1805,13 @@ DWARFASTParserClang::ParseStructureLikeDIE(const
SymbolContext &sc,
}
if (def_die) {
+if (auto [it, inserted] = dwarf->GetDIEToType().try_emplace(
labath wrote:
It's possible, but I am not sure if it's worth it, as this code will go away
(or at least change substantially) when we switch to lazy definition lookups.
https://github.com/llvm/llvm-project/pull/96751
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Unique enums parsed from declarations (PR #96751)
@@ -1798,6 +1805,13 @@ DWARFASTParserClang::ParseStructureLikeDIE(const
SymbolContext &sc,
}
if (def_die) {
+if (auto [it, inserted] = dwarf->GetDIEToType().try_emplace(
ZequanWu wrote:
This part seems unnecessary as `UniqueDWARFASTType` map already handles it,
right?
https://github.com/llvm/llvm-project/pull/96751
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Unique enums parsed from declarations (PR #96751)
@@ -1798,6 +1805,13 @@ DWARFASTParserClang::ParseStructureLikeDIE(const
SymbolContext &sc,
}
if (def_die) {
+if (auto [it, inserted] = dwarf->GetDIEToType().try_emplace(
labath wrote:
For uniqueness, yes the unique map should already catch that (though, like
we've seen, it can easily have bugs). However, it's still useful for preventing
(infinite) recursion.
https://github.com/llvm/llvm-project/pull/96751
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] DebugInfoD tests: attempt to fix Fuchsia build (PR #96802)
Jlalond wrote: @petrhosek Is there a way you can help @kevinfrei validate this on Fuschia? https://github.com/llvm/llvm-project/pull/96802 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Unique enums parsed from declarations (PR #96751)
https://github.com/ZequanWu approved this pull request. https://github.com/llvm/llvm-project/pull/96751 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [clang][lldb] Don't assert structure layout correctness for layouts provided by LLDB (PR #93809)
dwblaikie wrote:
Ah, I think this right solution to /this/ case is that the compiler should be
emitting the alignment for the structure?
Like there's no way for the debugger to know that this structure is
underaligned at the moment:
```
struct __attribute__((packed)) t1 {
int i;
};
```
Which means the debugger might generate code that assumes `t1` is naturally
aligned, and break (ARM crashes on underaligned operations, doesn't it? so if
you got a pointer to t1 back from some API that happened to have put it in an
underaligned location - then in your expression you tried to read `i`, this
could generate crashing code?)
Essentially the `packed` attribute on the above structure is providing the same
value/effect as `aligned(1)` and should produce the same DWARF, but it doesn't.
https://github.com/llvm/llvm-project/pull/93809
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [clang][lldb] Don't assert structure layout correctness for layouts provided by LLDB (PR #93809)
dwblaikie wrote: oh, slight misquote above - `aligned(1)` is a no-op, as that attribute can't reduce the alignment... But I think my argument still stands, roughly as - if increases in alignment are explicitly specified, decreases in alignment should be too. https://github.com/llvm/llvm-project/pull/93809 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [clang][lldb] Don't assert structure layout correctness for layouts provided by LLDB (PR #93809)
dwblaikie wrote: The other effects of `packed` are encoded (the changes to the member offsets) but that's not the only effect, and we shouldn't use that effect (which isn't guaranteed to be observable - as in the case of "packed struct with a single member/that just happens to not have padding anyway") to try to divine the alignment. https://github.com/llvm/llvm-project/pull/93809 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [clang][lldb] Don't assert structure layout correctness for layouts provided by LLDB (PR #93809)
dwblaikie wrote:
Here's the smallest patch that would put explicit alignment on any packed
structure:
```
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp
b/clang/lib/CodeGen/CGDebugInfo.cpp
index a072475ba770..bbb13ddd593b 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -64,7 +64,7 @@ static uint32_t getTypeAlignIfRequired(const Type *Ty, const
ASTContext &Ctx) {
// MaxFieldAlignmentAttr is the attribute added to types
// declared after #pragma pack(n).
if (auto *Decl = Ty->getAsRecordDecl())
-if (Decl->hasAttr())
+if (Decl->hasAttr() || Decl->hasAttr())
return TI.Align;
return 0;
```
But I don't think that's the right approach - I think what we should do is
compute the natural alignment of the structure, then compare that to the actual
alignment - and if they differ, we should put an explicit alignment on the
structure. This avoids the risk that other alignment-influencing effects might
be missed (and avoids the case of putting alignment on a structure that, when
packed, just has the same alignment anyway - which is a minor issue, but nice
to get right (eg: packed struct of a single char probably shouldn't have an
explicit alignment - since it's the same as the implicit alignment anyway))
https://github.com/llvm/llvm-project/pull/93809
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [clang][lldb] Don't assert structure layout correctness for layouts provided by LLDB (PR #93809)
dwblaikie wrote: Oh, this code was touched really recently in 66df6141659375e738d9b9b74bf79b2317576042 (@augusto2112 ) (see notes in previous comments about how this code should be generalized) https://github.com/llvm/llvm-project/pull/93809 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Don't start class definitions in ParseStructureLikeDIE (PR #96755)
https://github.com/ZequanWu approved this pull request. https://github.com/llvm/llvm-project/pull/96755 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] DebugInfoD tests: attempt to fix Fuchsia build (PR #96802)
https://github.com/kevinfrei edited https://github.com/llvm/llvm-project/pull/96802 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] DebugInfoD tests: attempt to fix Fuchsia build (PR #96802)
kevinfrei wrote: > @petrhosek Is there a way you can help @kevinfrei validate this on Fuchsia Specifically, I'm adding tests that require dwp/llvm-dwp laying around somewhere, and it looks like Fuchsia doesn't put it in the 'normal' location, and I have no idea how to simply disable an LLDB test for Fuchsia, because it lies to the infra and says it's linux (which is probably the right lie to tell, but I can't find any mechanism to see that it's a flower-flavored variant of Linux...) https://github.com/llvm/llvm-project/pull/96802 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] DebugInfoD tests: attempt to fix Fuchsia build (PR #96802)
https://github.com/kevinfrei updated
https://github.com/llvm/llvm-project/pull/96802
>From 5db458e34f9db981d78bdac0ba18a97d506fb404 Mon Sep 17 00:00:00 2001
From: Kevin Frei
Date: Mon, 25 Mar 2024 08:23:47 -0700
Subject: [PATCH 01/13] Trying to deal with Linux AArch64 test failures :/
---
.../SymbolVendor/ELF/SymbolVendorELF.cpp | 18 ++
.../API/debuginfod/Normal/TestDebuginfod.py | 187 +
.../SplitDWARF/TestDebuginfodDWP.py | 196 ++
3 files changed, 401 insertions(+)
create mode 100644 lldb/test/API/debuginfod/Normal/TestDebuginfod.py
create mode 100644 lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py
diff --git a/lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
b/lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
index b5fe35d71032a..a881218a56cef 100644
--- a/lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
+++ b/lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
@@ -44,6 +44,24 @@ llvm::StringRef
SymbolVendorELF::GetPluginDescriptionStatic() {
"executables.";
}
+// If this is needed elsewhere, it can be exported/moved.
+static bool IsDwpSymbolFile(const lldb::ModuleSP &module_sp,
+const FileSpec &file_spec) {
+ DataBufferSP dwp_file_data_sp;
+ lldb::offset_t dwp_file_data_offset = 0;
+ // Try to create an ObjectFile from the file_spec.
+ ObjectFileSP dwp_obj_file = ObjectFile::FindPlugin(
+ module_sp, &file_spec, 0, FileSystem::Instance().GetByteSize(file_spec),
+ dwp_file_data_sp, dwp_file_data_offset);
+ // The presence of a debug_cu_index section is the key identifying feature of
+ // a DWP file. Make sure we don't fill in the section list on dwp_obj_file
+ // (by calling GetSectionList(false)) as this is invoked before we may have
+ // all the symbol files collected and available.
+ return dwp_obj_file && ObjectFileELF::classof(dwp_obj_file.get()) &&
+ dwp_obj_file->GetSectionList(false)->FindSectionByType(
+ eSectionTypeDWARFDebugCuIndex, false);
+}
+
// CreateInstance
//
// Platforms can register a callback to use when creating symbol vendors to
diff --git a/lldb/test/API/debuginfod/Normal/TestDebuginfod.py
b/lldb/test/API/debuginfod/Normal/TestDebuginfod.py
new file mode 100644
index 0..a662fb9fc6e68
--- /dev/null
+++ b/lldb/test/API/debuginfod/Normal/TestDebuginfod.py
@@ -0,0 +1,187 @@
+import os
+import shutil
+import tempfile
+import struct
+
+import lldb
+from lldbsuite.test.decorators import *
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+def getUUID(aoutuuid):
+"""
+Pull the 20 byte UUID out of the .note.gnu.build-id section that was dumped
+to a file already, as part of the build.
+"""
+with open(aoutuuid, "rb") as f:
+data = f.read(36)
+if len(data) != 36:
+return None
+header = struct.unpack_from("<4I", data)
+if len(header) != 4:
+return None
+# 4 element 'prefix', 20 bytes of uuid, 3 byte long string: 'GNU':
+if header[0] != 4 or header[1] != 20 or header[2] != 3 or header[3] !=
0x554E47:
+return None
+return data[16:].hex()
+
+
+"""
+Test support for the DebugInfoD network symbol acquisition protocol.
+This one is for simple / no split-dwarf scenarios.
+
+For no-split-dwarf scenarios, there are 2 variations:
+1 - A stripped binary with it's corresponding unstripped binary:
+2 - A stripped binary with a corresponding --only-keep-debug symbols file
+"""
+
+
+@skipUnlessPlatform(["linux", "freebsd"])
+class DebugInfodTests(TestBase):
+# No need to try every flavor of debug inf.
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_normal_no_symbols(self):
+"""
+Validate behavior with no symbols or symbol locator.
+('baseline negative' behavior)
+"""
+test_root = self.config_test(["a.out"])
+self.try_breakpoint(False)
+
+def test_normal_default(self):
+"""
+Validate behavior with symbols, but no symbol locator.
+('baseline positive' behavior)
+"""
+test_root = self.config_test(["a.out", "a.out.debug"])
+self.try_breakpoint(True)
+
+def test_debuginfod_symbols(self):
+"""
+Test behavior with the full binary available from Debuginfod as
+'debuginfo' from the plug-in.
+"""
+test_root = self.config_test(["a.out"], "a.out.full")
+self.try_breakpoint(True)
+
+def test_debuginfod_executable(self):
+"""
+Test behavior with the full binary available from Debuginfod as
+'executable' from the plug-in.
+"""
+test_root = self.config_test(["a.out"], None, "a.out.full")
+self.try_breakpoint(True)
+
+def test_debuginfod_okd_symbols(self):
+"""
+Test behavior with the 'only-keep-debug' symbols available from
Debuginfod.
+"""
+
[Lldb-commits] [lldb] [LLDB] DebugInfoD tests: attempt to fix Fuchsia build (PR #96802)
petrhosek wrote: > > @petrhosek Is there a way you can help @kevinfrei validate this on Fuchsia > > Specifically, I'm adding tests that require dwp/llvm-dwp laying around > somewhere, and it looks like Fuchsia doesn't put it in the 'normal' location, > and I have no idea how to simply disable an LLDB test for Fuchsia, because it > lies to the infra and says it's linux (which is probably the right lie to > tell, but I can't find any mechanism to see that it's a flower-flavored > variant of Linux...) > > Here's the build failure, where you can see it can't find the 'dwp' tool: > https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8747217173107569041/test-results?sortby=&groupby= What do you mean by "normal" location? Our toolchain has `llvm-dwp` as `bin/llvm-dwp` which is the default location. The builder description is correct, it's a Linux distribution (Debian) but we don't install GNU toolchains, we're strict about using Clang/LLVM toolchain which is why there's no `dwp`, just `llvm-dwp`. It's not just `dwp` though, we don't install any GNU tools and instead include the LLVM counterparts, for example `llvm-readelf` instead of `readelf`, `llvm-objcopy` instead of `objcopy`, etc. It looks like `lldb/packages/Python/lldbsuite/test/make/Makefile.rules` assumes GNU toolchain, which seems a bit odd given that it's a part of LLVM. Would it be possible to make it compatible with both GNU and Clang/LLVM toolchains? https://github.com/llvm/llvm-project/pull/96802 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] DebugInfoD tests: attempt to fix Fuchsia build (PR #96802)
kevinfrei wrote: > What do you mean by "normal" location? "normal" as defined by whoever wrote Makefile.rules originally and thought that was the correct place to file tools like ar and objcopy, I guess? So, on your build machine, do you know where the llvm-dwp file might be installed? Because honestly, I'd rather just use that thing instead of assuming the GNU dwp tool is around. https://github.com/llvm/llvm-project/pull/96802 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [API] add GetSyntheticValue (PR #95959)
jimingham wrote: The one thing about this that strikes me as odd is that if you have an SBValue that doesn't have a Synthetic Value, this will return the non-synthetic value. Getting the NonSynthetic Value didn't have this problem because there's always a non-synthetic value, so provided the SBValue was valid you could always hand that out. Would it make more sense to make the ValueImpl that's going to represent the synthetic value, and return an empty SBValue if it is not? https://github.com/llvm/llvm-project/pull/95959 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix the test to deal with non-deterministic output. (PR #96800)
vvereschaka wrote:
@clayborg ,
the test is still flaky with these changes on the windows build/target host.
Here are two failed test outputs. They are a bit different.
failure 1:
```
# executed command:
'd:\projects\llvm-project\lldb\build-lldb-win\bin\filecheck.exe'
'D:\projects\llvm-project\lldb\llvm-project\lldb\test\Shell\SymbolFile\DWARF\x86\dwp-foreign-type-units.cpp'
--check-prefix=NODWP
# .---command stderr
# |
D:\projects\llvm-project\lldb\llvm-project\lldb\test\Shell\SymbolFile\DWARF\x86\dwp-foreign-type-units.cpp:35:11:
error: NODWP: expected string not found in input
# | // NODWP: (lldb) type lookup FloatType
# | ^
# | :14:22: note: scanning from here
# | typedef unsigned int IntegerType;
# | ^
# | :15:3: note: possible intended match here
# | typedef float FloatType;
# | ^
# |
# | Input file:
# | Check file:
D:\projects\llvm-project\lldb\llvm-project\lldb\test\Shell\SymbolFile\DWARF\x86\dwp-foreign-type-units.cpp
# |
# | -dump-input=help explains the following input dump.
# |
# | Input was:
# | <<
# | .
# | .
# | .
# | 9: (lldb) type lookup FloatType
# |10: double
# |11: float
# |12: (lldb) type lookup CustomType
# |13: struct CustomType {
# |14: typedef unsigned int IntegerType;
# | check:35'0 X~ error: no match found
# |15: typedef float FloatType;
# | check:35'0 ~~
# | check:35'1 ?possible intended match
# |16: CustomType::IntegerType x;
# | check:35'0
# |17: CustomType::FloatType y;
# | check:35'0 ~~
# |18: }
# | check:35'0 ~~
# |19: struct CustomType {
# | check:35'0
# |20: typedef int IntegerType;
# | check:35'0 ~~
# | .
# | .
# | .
# | >>
# `-
# error: command failed with exit status: 1
```
failure 2:
```
# executed command:
'd:\projects\llvm-project\lldb\build-lldb-win\bin\filecheck.exe'
'D:\projects\llvm-project\lldb\llvm-project\lldb\test\Shell\SymbolFile\DWARF\x86\dwp-foreign-type-units.cpp'
--check-prefix=NODWP
# .---command stderr
# |
D:\projects\llvm-project\lldb\llvm-project\lldb\test\Shell\SymbolFile\DWARF\x86\dwp-foreign-type-units.cpp:35:11:
error: NODWP: expected string not found in input
# | // NODWP: (lldb) type lookup FloatType
# | ^
# | :20:22: note: scanning from here
# | typedef unsigned int IntegerType;
# | ^
# | :21:3: note: possible intended match here
# | typedef float FloatType;
# | ^
# |
# | Input file:
# | Check file:
D:\projects\llvm-project\lldb\llvm-project\lldb\test\Shell\SymbolFile\DWARF\x86\dwp-foreign-type-units.cpp
# |
# | -dump-input=help explains the following input dump.
# |
# | Input was:
# | <<
# | .
# | .
# | .
# |15: typedef double FloatType;
# |16: CustomType::IntegerType x;
# |17: CustomType::FloatType y;
# |18: }
# |19: struct CustomType {
# |20: typedef unsigned int IntegerType;
# | check:35'0 X~ error: no match found
# |21: typedef float FloatType;
# | check:35'0 ~~
# | check:35'1 ?possible intended match
# |22: CustomType::IntegerType x;
# | check:35'0
# |23: CustomType::FloatType y;
# | check:35'0 ~~
# |24: }
# | check:35'0 ~~
# | >>
# `-
# error: command failed with exit status: 1
```
https://github.com/llvm/llvm-project/pull/96800
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add the ability for Script based commands to specify their "repeat command" (PR #94823)
https://github.com/medismailben edited https://github.com/llvm/llvm-project/pull/94823 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add the ability for Script based commands to specify their "repeat command" (PR #94823)
@@ -1827,6 +1838,15 @@ class CommandObjectScriptingObjectParsed : public
CommandObjectParsed {
ScriptedCommandSynchronicity GetSynchronicity() { return m_synchro; }
+ std::optional GetRepeatCommand(Args &args,
+ uint32_t index) override {
+ScriptInterpreter *scripter = GetDebugger().GetScriptInterpreter();
+if (!scripter)
+ return std::nullopt;
+
+return scripter->GetRepeatCommandForScriptedCommand(m_cmd_obj_sp, args);
+ }
+
medismailben wrote:
nit: This looks pretty much the same as the one in
`CommandObjectScriptingObjectRaw`. I understand that these have different base
classes but I guess we could use multiple inheritance (in a follow-up) to
refactor the 2 classes and reduce code duplication.
https://github.com/llvm/llvm-project/pull/94823
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add the ability for Script based commands to specify their "repeat command" (PR #94823)
https://github.com/medismailben approved this pull request. LGTM with nits https://github.com/llvm/llvm-project/pull/94823 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add the ability for Script based commands to specify their "repeat command" (PR #94823)
@@ -32,6 +37,39 @@ def check_help_options(self, cmd_name, opt_list, substrs=[]):
print(f"Opt Vec\n{substrs}")
self.expect("help " + cmd_name, substrs=substrs)
+def run_one_repeat(self, commands, expected_num_errors):
+with open(self.stdin_path, "w") as input_handle:
+input_handle.write(commands)
+
+in_fileH = open(self.stdin_path, "r")
+self.dbg.SetInputFileHandle(in_fileH, False)
+
+out_fileH = open(self.stdout_path, "w")
+self.dbg.SetOutputFileHandle(out_fileH, False)
+self.dbg.SetErrorFileHandle(out_fileH, False)
+
+options = lldb.SBCommandInterpreterRunOptions()
+options.SetEchoCommands(False)
+options.SetPrintResults(True)
+options.SetPrintErrors(True)
+options.SetAllowRepeats(True)
+
+n_errors, quit_requested, has_crashed = self.dbg.RunCommandInterpreter(
+True, False, options, 0, False, False
+)
+
+in_fileH.close()
+out_fileH.close()
+
+results = None
+with open(self.stdout_path, "r") as out_fileH:
+results = out_fileH.read()
+
+print(f"RESULTS:\n{results}\nDONE")
medismailben wrote:
May be we should only print if the test suite in verbose/trace mode.
https://github.com/llvm/llvm-project/pull/94823
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add the ability for Script based commands to specify their "repeat command" (PR #94823)
@@ -831,6 +831,29 @@ bool
lldb_private::python::SWIGBridge::LLDBSwigPythonCallCommandObject(
return true;
}
+std::optional
+lldb_private::python::SWIGBridge::LLDBSwigPythonGetRepeatCommandForScriptedCommand(PyObject
*implementor,
+ std::string &command) {
+ PyErr_Cleaner py_err_cleaner(true);
+
+ PythonObject self(PyRefType::Borrowed, implementor);
+ auto pfunc = self.ResolveName("get_repeat_command");
+ // If not implemented, repeat the exact command.
+ if (!pfunc.IsAllocated())
+return std::nullopt;
+
+ PythonObject result;
+ PythonString command_str(command);
+ result = pfunc(command_str);
medismailben wrote:
nit:
```suggestion
PythonString command_str(command);
PythonObject result = pfunc(command_str);
```
https://github.com/llvm/llvm-project/pull/94823
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add the ability for Script based commands to specify their "repeat command" (PR #94823)
@@ -1827,6 +1838,15 @@ class CommandObjectScriptingObjectParsed : public
CommandObjectParsed {
ScriptedCommandSynchronicity GetSynchronicity() { return m_synchro; }
+ std::optional GetRepeatCommand(Args &args,
+ uint32_t index) override {
+ScriptInterpreter *scripter = GetDebugger().GetScriptInterpreter();
+if (!scripter)
+ return std::nullopt;
+
+return scripter->GetRepeatCommandForScriptedCommand(m_cmd_obj_sp, args);
+ }
+
jimingham wrote:
I was holding off on refactoring these classes till we're switching to the
Scripted Interface infrastructure.
https://github.com/llvm/llvm-project/pull/94823
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add the ability for Script based commands to specify their "repeat command" (PR #94823)
@@ -32,6 +37,39 @@ def check_help_options(self, cmd_name, opt_list, substrs=[]):
print(f"Opt Vec\n{substrs}")
self.expect("help " + cmd_name, substrs=substrs)
+def run_one_repeat(self, commands, expected_num_errors):
+with open(self.stdin_path, "w") as input_handle:
+input_handle.write(commands)
+
+in_fileH = open(self.stdin_path, "r")
+self.dbg.SetInputFileHandle(in_fileH, False)
+
+out_fileH = open(self.stdout_path, "w")
+self.dbg.SetOutputFileHandle(out_fileH, False)
+self.dbg.SetErrorFileHandle(out_fileH, False)
+
+options = lldb.SBCommandInterpreterRunOptions()
+options.SetEchoCommands(False)
+options.SetPrintResults(True)
+options.SetPrintErrors(True)
+options.SetAllowRepeats(True)
+
+n_errors, quit_requested, has_crashed = self.dbg.RunCommandInterpreter(
+True, False, options, 0, False, False
+)
+
+in_fileH.close()
+out_fileH.close()
+
+results = None
+with open(self.stdout_path, "r") as out_fileH:
+results = out_fileH.read()
+
+print(f"RESULTS:\n{results}\nDONE")
jimingham wrote:
Doh. Probably can just get rid of this.
https://github.com/llvm/llvm-project/pull/94823
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Do not produce field information for registers known not to exist (PR #95125)
https://github.com/medismailben approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/95125 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
