[Lldb-commits] [lldb] [lldb] Add constant value mode for RegisterLocation in UnwindPlans (PR #100624)
https://github.com/felipepiovezan created
https://github.com/llvm/llvm-project/pull/100624
This is useful for language runtimes that compute register values by inspecting
the state of the currently running process. Currently, there are no mechanisms
enabling these runtimes to set register values to arbitrary values.
The alternative considered would involve creating a dwarf expression that
produces an arbitrary integer (e.g. using OP_constu). However, the current data
structure for Rows is such that they do not own any memory associated with
dwarf expressions, which implies any such expression would need to have static
storage and therefore could not contain a runtime value.
Adding a new rule for constants leads to a simpler implementation. It's also
worth noting that this does not make the "Location" union any bigger, since it
already contains a pointer+size pair.
>From 223075d6f036537a7edec10b370f6d922135cb79 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan
Date: Tue, 23 Jul 2024 13:30:40 -0700
Subject: [PATCH] [lldb] Add constant value mode for RegisterLocation in
UnwindPlans
This is useful for language runtimes that compute register values by inspecting
the state of the currently running process. Currently, there are no mechanisms
enabling these runtimes to set register values to arbitrary values.
The alternative considered would involve creating a dwarf expression that
produces an arbitrary integer (e.g. using OP_constu). However, the current data
structure for Rows is such that they do not own any memory associated with dwarf
expressions, which implies any such expression would need to have static storage
and therefore could not contain a runtime value.
Adding a new rule for constants leads to a simpler implementation. It's also
worth noting that this does not make the "Location" union any bigger, since it
already contains a pointer+size pair.
---
lldb/include/lldb/Symbol/UnwindPlan.h| 17 -
lldb/source/Symbol/UnwindPlan.cpp| 17 +
lldb/source/Target/RegisterContextUnwind.cpp | 9 +
3 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/lldb/include/lldb/Symbol/UnwindPlan.h
b/lldb/include/lldb/Symbol/UnwindPlan.h
index ebb0ec421da72..a9e8406608ff3 100644
--- a/lldb/include/lldb/Symbol/UnwindPlan.h
+++ b/lldb/include/lldb/Symbol/UnwindPlan.h
@@ -68,7 +68,8 @@ class UnwindPlan {
isAFAPlusOffset, // reg = AFA + offset
inOtherRegister, // reg = other reg
atDWARFExpression, // reg = deref(eval(dwarf_expr))
-isDWARFExpression // reg = eval(dwarf_expr)
+isDWARFExpression, // reg = eval(dwarf_expr)
+isConstant // reg = constant
};
RegisterLocation() : m_location() {}
@@ -105,6 +106,15 @@ class UnwindPlan {
bool IsDWARFExpression() const { return m_type == isDWARFExpression; }
+ bool IsConstant() const { return m_type == isConstant; }
+
+ void SetIsConstant(uint64_t value) {
+m_type = isConstant;
+m_location.constant_value = value;
+ }
+
+ uint64_t GetConstant() const { return m_location.constant_value; }
+
void SetAtCFAPlusOffset(int32_t offset) {
m_type = atCFAPlusOffset;
m_location.offset = offset;
@@ -192,6 +202,8 @@ class UnwindPlan {
const uint8_t *opcodes;
uint16_t length;
} expr;
+// For m_type == isConstant
+uint64_t constant_value;
} m_location;
};
@@ -358,6 +370,9 @@ class UnwindPlan {
bool SetRegisterLocationToSame(uint32_t reg_num, bool must_replace);
+bool SetRegisterLocationToIsConstant(uint32_t reg_num, uint64_t constant,
+ bool can_replace);
+
// When this UnspecifiedRegistersAreUndefined mode is
// set, any register that is not specified by this Row will
// be described as Undefined.
diff --git a/lldb/source/Symbol/UnwindPlan.cpp
b/lldb/source/Symbol/UnwindPlan.cpp
index e258a4e3d82f2..fcd3154a01d82 100644
--- a/lldb/source/Symbol/UnwindPlan.cpp
+++ b/lldb/source/Symbol/UnwindPlan.cpp
@@ -46,6 +46,8 @@ operator==(const UnwindPlan::Row::RegisterLocation &rhs)
const {
return !memcmp(m_location.expr.opcodes, rhs.m_location.expr.opcodes,
m_location.expr.length);
break;
+case isConstant:
+ return m_location.constant_value == rhs.m_location.constant_value;
}
}
return false;
@@ -153,6 +155,9 @@ void UnwindPlan::Row::RegisterLocation::Dump(Stream &s,
if (m_type == atDWARFExpression)
s.PutChar(']');
} break;
+ case isConstant:
+s.Printf("=%x", m_location.offset);
+break;
}
}
@@ -351,6 +356,18 @@ bool UnwindPlan::Row::SetRegisterLocationToSame(uint32_t
reg_num,
return true;
}
+bool UnwindPlan::Row::SetRegisterLocationToIsConstant(uint32_t reg_num,
+ uint64_t constant,
+
[Lldb-commits] [lldb] [lldb] Add constant value mode for RegisterLocation in UnwindPlans (PR #100624)
@@ -153,6 +155,9 @@ void UnwindPlan::Row::RegisterLocation::Dump(Stream &s,
if (m_type == atDWARFExpression)
s.PutChar(']');
} break;
+ case isConstant:
+s.Printf("=%x", m_location.offset);
felipepiovezan wrote:
Ohh oops, you're totally right! Fixed!
https://github.com/llvm/llvm-project/pull/100624
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add constant value mode for RegisterLocation in UnwindPlans (PR #100624)
https://github.com/felipepiovezan updated
https://github.com/llvm/llvm-project/pull/100624
>From 223075d6f036537a7edec10b370f6d922135cb79 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan
Date: Tue, 23 Jul 2024 13:30:40 -0700
Subject: [PATCH 1/2] [lldb] Add constant value mode for RegisterLocation in
UnwindPlans
This is useful for language runtimes that compute register values by inspecting
the state of the currently running process. Currently, there are no mechanisms
enabling these runtimes to set register values to arbitrary values.
The alternative considered would involve creating a dwarf expression that
produces an arbitrary integer (e.g. using OP_constu). However, the current data
structure for Rows is such that they do not own any memory associated with dwarf
expressions, which implies any such expression would need to have static storage
and therefore could not contain a runtime value.
Adding a new rule for constants leads to a simpler implementation. It's also
worth noting that this does not make the "Location" union any bigger, since it
already contains a pointer+size pair.
---
lldb/include/lldb/Symbol/UnwindPlan.h| 17 -
lldb/source/Symbol/UnwindPlan.cpp| 17 +
lldb/source/Target/RegisterContextUnwind.cpp | 9 +
3 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/lldb/include/lldb/Symbol/UnwindPlan.h
b/lldb/include/lldb/Symbol/UnwindPlan.h
index ebb0ec421da72..a9e8406608ff3 100644
--- a/lldb/include/lldb/Symbol/UnwindPlan.h
+++ b/lldb/include/lldb/Symbol/UnwindPlan.h
@@ -68,7 +68,8 @@ class UnwindPlan {
isAFAPlusOffset, // reg = AFA + offset
inOtherRegister, // reg = other reg
atDWARFExpression, // reg = deref(eval(dwarf_expr))
-isDWARFExpression // reg = eval(dwarf_expr)
+isDWARFExpression, // reg = eval(dwarf_expr)
+isConstant // reg = constant
};
RegisterLocation() : m_location() {}
@@ -105,6 +106,15 @@ class UnwindPlan {
bool IsDWARFExpression() const { return m_type == isDWARFExpression; }
+ bool IsConstant() const { return m_type == isConstant; }
+
+ void SetIsConstant(uint64_t value) {
+m_type = isConstant;
+m_location.constant_value = value;
+ }
+
+ uint64_t GetConstant() const { return m_location.constant_value; }
+
void SetAtCFAPlusOffset(int32_t offset) {
m_type = atCFAPlusOffset;
m_location.offset = offset;
@@ -192,6 +202,8 @@ class UnwindPlan {
const uint8_t *opcodes;
uint16_t length;
} expr;
+// For m_type == isConstant
+uint64_t constant_value;
} m_location;
};
@@ -358,6 +370,9 @@ class UnwindPlan {
bool SetRegisterLocationToSame(uint32_t reg_num, bool must_replace);
+bool SetRegisterLocationToIsConstant(uint32_t reg_num, uint64_t constant,
+ bool can_replace);
+
// When this UnspecifiedRegistersAreUndefined mode is
// set, any register that is not specified by this Row will
// be described as Undefined.
diff --git a/lldb/source/Symbol/UnwindPlan.cpp
b/lldb/source/Symbol/UnwindPlan.cpp
index e258a4e3d82f2..fcd3154a01d82 100644
--- a/lldb/source/Symbol/UnwindPlan.cpp
+++ b/lldb/source/Symbol/UnwindPlan.cpp
@@ -46,6 +46,8 @@ operator==(const UnwindPlan::Row::RegisterLocation &rhs)
const {
return !memcmp(m_location.expr.opcodes, rhs.m_location.expr.opcodes,
m_location.expr.length);
break;
+case isConstant:
+ return m_location.constant_value == rhs.m_location.constant_value;
}
}
return false;
@@ -153,6 +155,9 @@ void UnwindPlan::Row::RegisterLocation::Dump(Stream &s,
if (m_type == atDWARFExpression)
s.PutChar(']');
} break;
+ case isConstant:
+s.Printf("=%x", m_location.offset);
+break;
}
}
@@ -351,6 +356,18 @@ bool UnwindPlan::Row::SetRegisterLocationToSame(uint32_t
reg_num,
return true;
}
+bool UnwindPlan::Row::SetRegisterLocationToIsConstant(uint32_t reg_num,
+ uint64_t constant,
+ bool can_replace) {
+ if (!can_replace &&
+ m_register_locations.find(reg_num) != m_register_locations.end())
+return false;
+ RegisterLocation reg_loc;
+ reg_loc.SetIsConstant(constant);
+ m_register_locations[reg_num] = reg_loc;
+ return true;
+}
+
bool UnwindPlan::Row::operator==(const UnwindPlan::Row &rhs) const {
return m_offset == rhs.m_offset && m_cfa_value == rhs.m_cfa_value &&
m_afa_value == rhs.m_afa_value &&
diff --git a/lldb/source/Target/RegisterContextUnwind.cpp
b/lldb/source/Target/RegisterContextUnwind.cpp
index 95e8abd763d53..f74f1dc0e1b80 100644
--- a/lldb/source/Target/RegisterContextUnwind.cpp
+++ b/lldb/source/Target/RegisterContextUnwind.cpp
@@ -1694,6 +1694,15 @@ RegisterContextUnwind::Sav
[Lldb-commits] [lldb] [lldb] Add constant value mode for RegisterLocation in UnwindPlans (PR #100624)
https://github.com/felipepiovezan closed https://github.com/llvm/llvm-project/pull/100624 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][ClangExpressionParser][NFC] Factor LangOptions logic out of ClangExpressionParser constructor (PR #101669)
@@ -351,136 +352,32 @@ static void
SetupDefaultClangDiagnostics(CompilerInstance &compiler) {
}
}
-//===--===//
-// Implementation of ClangExpressionParser
-//===--===//
-
-ClangExpressionParser::ClangExpressionParser(
-ExecutionContextScope *exe_scope, Expression &expr,
-bool generate_debug_info, std::vector include_directories,
-std::string filename)
-: ExpressionParser(exe_scope, expr, generate_debug_info), m_compiler(),
- m_pp_callbacks(nullptr),
- m_include_directories(std::move(include_directories)),
- m_filename(std::move(filename)) {
+static void SetupLangOpts(CompilerInstance &compiler,
+ ExecutionContextScope &exe_scope,
+ Expression const &expr) {
felipepiovezan wrote:
the LLVM codebase is more of a west const type of codebase ;)
https://github.com/llvm/llvm-project/pull/101669
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Move definition of SBSaveCoreOptions dtor out of header (PR #102539)
https://github.com/felipepiovezan approved this pull request. https://github.com/llvm/llvm-project/pull/102539 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Move definition of SBSaveCoreOptions dtor out of header (PR #102539)
felipepiovezan wrote: LGTM! Good catch! https://github.com/llvm/llvm-project/pull/102539 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add the word "backtrace" to bt help string (PR #92618)
https://github.com/felipepiovezan approved this pull request. LGTM! https://github.com/llvm/llvm-project/pull/92618 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/dwarf] Fix DW_IDX_parent processing for split dwarf (PR #92745)
https://github.com/felipepiovezan edited https://github.com/llvm/llvm-project/pull/92745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/dwarf] Fix DW_IDX_parent processing for split dwarf (PR #92745)
@@ -9,7 +9,7 @@ #include "Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h" #include "Plugins/SymbolFile/DWARF/DWARFDebugInfo.h" #include "Plugins/SymbolFile/DWARF/DWARFDeclContext.h" -#include "Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h" +#include "Plugins/SymbolFile/DWARF/LogChannelDWARF.h" felipepiovezan wrote: Is this actually needed? https://github.com/llvm/llvm-project/pull/92745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/dwarf] Fix DW_IDX_parent processing for split dwarf (PR #92745)
https://github.com/felipepiovezan approved this pull request. Great catch and fix! Thanks for doing this https://github.com/llvm/llvm-project/pull/92745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Improve performance of .debug_names lookups when DW_IDX_parent attributes are used (PR #91808)
felipepiovezan wrote:
> I think #77696 is justification enough to add this check, but I don't think
> we should be implicating DW_IDX_parent_entries until we know how those came
> about.
This is pretty much my point, the workaround is fine, but I don't think the
source-code comment in this PR is. The suggestion provided LGTM.
My guess is that the examples Greg is encountering are just hitting the
slowness of DWARF parsing through other code paths (anything that causes too
many calls to "ProcessEntries", and IDX_parent addressed _one_ of those,
@Michael137 was just asking me the other day if we can address some of the
other queries as well).
> I believe we could add a debug_names entry for std::ios_base (and refer to it
> from the entry for std::ios_base::seekdir), as long as the ios_base entry was
> not present in any of the name tables.
Yup, this is very doable!
> I was not able to produce a DW_IDX_parent_entries referring to any kind of
> class type
When I compile the following with `clang++ test.cpp -c -gdwarf-5 -O0 -o - |
dwarfdump --debug-names -`, I get the right parent chain. Is this not true when
you try it?
```
namespace A { namespace B { struct State { class InnerState{}; }; } }
A::B::State::InnerState get_state() { return A::B::State::InnerState(); }
```
```
String: 0x00c4 "InnerState"
Entry @ 0xbe {
Abbrev: 0x138
Tag: DW_TAG_class_type
DW_IDX_die_offset: 0x003f
DW_IDX_parent: Entry @ 0xde
}
String: 0x00be "State"
Entry @ 0xde {
Abbrev: 0x9b8
Tag: DW_TAG_structure_type
DW_IDX_die_offset: 0x0039
DW_IDX_parent: Entry @ 0xe9
}
String: 0x00bc "B"
Entry @ 0xe9 {
Abbrev: 0x1cb8
Tag: DW_TAG_namespace
DW_IDX_die_offset: 0x0037
DW_IDX_parent: Entry @ 0xd7
}
String: 0x00ba "A"
Entry @ 0xd7 {
Abbrev: 0x1c98
Tag: DW_TAG_namespace
DW_IDX_die_offset: 0x0035
DW_IDX_parent:
}
```
https://github.com/llvm/llvm-project/pull/91808
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Make sure bad abbreviation codes do not crash lldb (PR #93006)
https://github.com/felipepiovezan approved this pull request. LGTM! Thanks for catching this https://github.com/llvm/llvm-project/pull/93006 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Remove DWARFDebugInfo DIERef footguns (PR #92894)
@@ -195,17 +195,17 @@ void DebugNamesDWARFIndex::GetCompleteObjCClass(
if (!ref)
continue;
-DWARFUnit *cu = m_debug_info.GetUnit(*ref);
-if (!cu || !cu->Supports_DW_AT_APPLE_objc_complete_type()) {
- incomplete_types.push_back(*ref);
- continue;
-}
-
-DWARFDIE die = m_debug_info.GetDIE(*ref);
+SymbolFileDWARF &dwarf = *llvm::cast(
+m_module.GetSymbolFile()->GetBackingSymbolFile());
felipepiovezan wrote:
This concerns me a little because `GetSymbolFile()` can fail (and so does
GetBackingSymbolFile), and we're doing this somewhat expensive operation inside
a loop, even though they are loop invariant.
We can probably address both of these issues by hoisting this outside the loop,
computing the range, early exiting if the range is empty, and then setting up
the symbol file. Something like:
```
auto range = m_debug_names_up->equal_range(class_name.GetStringRef()
if (range.empty()) return;
auto *symbolfile = m_module.GetSymbolFile();
if (!symbolfile) ...
if (!backing symbol file)...
// maybe just wrap all the lines above into a helper function
for (const DebugNames::Entry &entry :
m_debug_names_up->equal_range(class_name.GetStringRef())) {
...
```
```
https://github.com/llvm/llvm-project/pull/92894
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Remove DWARFDebugInfo DIERef footguns (PR #92894)
https://github.com/felipepiovezan edited https://github.com/llvm/llvm-project/pull/92894 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Remove DWARFDebugInfo DIERef footguns (PR #92894)
https://github.com/felipepiovezan edited https://github.com/llvm/llvm-project/pull/92894 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Remove DWARFDebugInfo DIERef footguns (PR #92894)
https://github.com/felipepiovezan edited https://github.com/llvm/llvm-project/pull/92894 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Improve performance of .debug_names lookups when DW_IDX_parent attributes are used (PR #91808)
felipepiovezan wrote: >Since clang no longer emits entry for: But what does the `debug_info` section look like? In particular, what is the _parent_ of the class DIE? If the parent of `InnerState` is not some kind of entry for `State` (either a declaration or a definition), IMO Clang is generating incorrect information for the type. What caused Clang to stop emitting these entries? > This gets a bit fuzzy, I think. The spec appears to allow this behavior (_In > such a case, a parent attribute may point to a nameless index entry (...), or > it may point to the **nearest ancestor that does have an index entry**._), > but I don't think this is particularly useful. I think it would be better to > have the parent point to the definition in the type unit (streching the > meaning of "parent" in the spec), or use one of those nameless entries to > point to the physical (declaration) parent) > > (IANAL YMMV) We can discuss this, but I think the point is going to be moot given what I mentioned about. The debug_names section is reflecting the state of `debug_info`. If the `debug_info` is saying that `State` is not a parent of `InnerState`, the `debug_names` section is correct in the literal sense, but will produce incorrect results for the query: "find A::B::State::InnerState". In the case where the declaration is there, debug_names will have correct info for `InnerState`: it will just say the parent is not indexed and things work out just fine. > have the parent point to the definition in the type unit (streching the > meaning of "parent" in the spec), Why do you say this is stretching the meaning of parent? This looks fine to me, but it seems impossible to emit such debug_names section if the compiler is no longer emitting the declaration with the type signature. (we'd need to check if the emitter code has some way of finding the definition, but also how could it possibly know there is a type between any two Parent-Child nodes? It really feels like we can't just elide the definition). https://github.com/llvm/llvm-project/pull/91808 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Improve performance of .debug_names lookups when DW_IDX_parent attributes are used (PR #91808)
felipepiovezan wrote: Ohhh ok, I just inspected the debug_names/debug_info sections; there is no parent at all for the InnerState debug_names entry. In this case, the debug_names section is valid. Unfortunate, as this may cause entire CUs/TUs to be parsed (because of all the calls to ProcessEntry, which is what the idx_parent project was addressing), but still valid. https://github.com/llvm/llvm-project/pull/91808 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Improve performance of .debug_names lookups when DW_IDX_parent attributes are used (PR #91808)
felipepiovezan wrote: > Current BOLT behavior is to skip that DIE and reference it's parent: This, on the other hand, is concerning. It needs to reflect the debug_info section, otherwise we can't use debug_names to answer queries like "find A::B::C::D". It would be better if the BOLT compiler produced either a "parent_not_indexed" entry, or no parent at all, or implement the nameless parent. All of these allow LLDB to correctly answer the query above, but what BOLT is currently doing does not https://github.com/llvm/llvm-project/pull/91808 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [llvm] Remove some `try_compile` CMake checks for compiler flags (PR #92953)
felipepiovezan wrote: These types of changes touching a lot of projects at once can benefit from multiple PRs, one per project, as it makes partial reverts a lot easier and doesn't cause as much churn downstream (plus we can get more targeted comments from individual project owners) https://github.com/llvm/llvm-project/pull/92953 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Bypass the compres^Wconstruction of DIERefs in debug_names (PR #93296)
@@ -183,27 +181,22 @@ void DebugNamesDWARFIndex::GetCompleteObjCClass(
llvm::function_ref callback) {
// Keep a list of incomplete types as fallback for when we don't find the
// complete type.
- DIEArray incomplete_types;
+ std::vector incomplete_types;
felipepiovezan wrote:
Let me try to run at least one basic experiment here
https://github.com/llvm/llvm-project/pull/93296
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Bypass the compres^Wconstruction of DIERefs in debug_names (PR #93296)
https://github.com/felipepiovezan approved this pull request. I think this should probably be fine (perf-wise)! https://github.com/llvm/llvm-project/pull/93296 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Improve performance of .debug_names lookups when DW_IDX_parent attributes are used (PR #91808)
felipepiovezan wrote: > > Discussed with Pavel, I applied this change to #92328 so we can ensure the > > DIEs from the index is always definition DIEs and avoid duplicate/expensive > > checks later. > > To elaborate, I suggested Zequan does this, because I think there's consensus > that this is a good way to filter out (incorrect) declaration dies from the > index, and it's a better (faster) fix than what Zequan had in his PR. It's > still worthwhile to figure out where those entries are coming from. We know > of one case with type units and that has now been fixed (thanks to David), if > there are more, we should try to understand where they are coming from). Sounds good! https://github.com/llvm/llvm-project/pull/91808 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Improve performance of .debug_names lookups when DW_IDX_parent attributes are used (PR #91808)
felipepiovezan wrote: thanks for well-written summary @dwblaikie ! https://github.com/llvm/llvm-project/pull/91808 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Remove DWARFDebugInfo DIERef footguns (PR #92894)
https://github.com/felipepiovezan approved this pull request. LGTM! https://github.com/llvm/llvm-project/pull/92894 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Fix adding children to clang type that hasn't started definition. (PR #93839)
@@ -13,12 +13,18 @@
using namespace lldb_private::dwarf;
using namespace lldb_private::plugin::dwarf;
+static bool IsStructOrClassTag(llvm::dwarf::Tag Tag) {
felipepiovezan wrote:
Yeah, we definitely need to clean all of these up.
There might be one inside llvm/ too..
https://github.com/llvm/llvm-project/pull/93839
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Improve performance of .debug_names lookups when DW_IDX_parent attributes are used (PR #91808)
felipepiovezan wrote:
> @felipepiovezan I have another question. For the same example. I see:
>
> ```
> Name 4 {
> Hash: 0x2F94396D
> String: 0x0049 "_Z9get_statev"
> Entry @ 0x112 {
> Abbrev: 0x2
> Tag: DW_TAG_subprogram
> DW_IDX_die_offset: 0x0023
> DW_IDX_parent:
> }
> ...
> Name 8 {
> Hash: 0x2B607
> String: 0x0041 "B"
> Entry @ 0x13b {
> Abbrev: 0x7
> Tag: DW_TAG_namespace
> DW_IDX_type_unit: 0x00
> DW_IDX_die_offset: 0x0025
> DW_IDX_parent: Entry @ 0x112
> }
> ```
>
> This seems like a bug no? Looks like it's confusing
>
> ```
> 0x0023: DW_TAG_namespace
> DW_AT_name ("A")
> ```
>
> In TU 0 With Subprogram at the same (relative offset) in the CU
>
> ```
> 0x006a: Compile Unit: length = 0x005c, format = DWARF32, version =
> 0x0005, unit_type = DW_UT_compile, abbr_offset = 0x, addr_size = 0x08
> (next unit at 0x00ca)
> ...
> 0x008d: DW_TAG_subprogram
> ```
>
> I think it should be pointing to:
>
> ```
> String: 0x0023 "A"
> Entry @ 0x11e {
> Abbrev: 0x4
> Tag: DW_TAG_namespace
> DW_IDX_type_unit: 0x00
> DW_IDX_die_offset: 0x0023
> DW_IDX_parent:
> }
> ```
You are right. The fact that they have the same relative offset tells me that
some part of the code is failing to account for TUs. I just checked the
printing code in the hope that it was a mistake while dumping, but it doesn't
seem to be...
https://github.com/llvm/llvm-project/pull/91808
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #92328)
felipepiovezan wrote: >Should we allow such entries in the index? And does this warrant rephrasing in >the DWARF spec? It's fine to have those as _nameless_ entries (which we don't emit today), but the entries that are reaching "process entry" are not nameless by definition. That if statement is a workaround for incorrect dwarf generation. It seems like this test is providing a case where incorrect dwarf is generated? 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][DebugNames] Only skip processing of DW_AT_declarations for class/union types (PR #94400)
https://github.com/felipepiovezan approved this pull request. https://github.com/llvm/llvm-project/pull/94400 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][lldb-dap] Cleanup breakpoint filters. (PR #87550)
felipepiovezan wrote: @oontvoo I think this broke the bots again: https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake/5341/console https://github.com/llvm/llvm-project/pull/87550 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][lldb-dap] Cleanup breakpoint filters. (PR #87550)
felipepiovezan wrote: Would you mind reverting it until you have a chance to look at the failures? https://github.com/llvm/llvm-project/pull/87550 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reapply PR/87550 (PR #94625)
felipepiovezan wrote: Oops, I commented on the old PR instead of the new one, so let me copy paste it here: @oontvoo I think this broke the bots again: https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake/5341/console Would you mind reverting it until you have a chance to look at the failures? https://github.com/llvm/llvm-project/pull/94625 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reapply PR/87550 (PR #94625)
felipepiovezan wrote: @oontvoo I'm going to revert this for now as the incremental bots are our first line of defense against failures https://github.com/llvm/llvm-project/pull/94625 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] adcf33f - Revert "Reapply PR/87550 (#94625)"
Author: Felipe de Azevedo Piovezan
Date: 2024-06-07T10:14:58-07:00
New Revision: adcf33f8fbcc0f068bd4b8254994b16dda525009
URL:
https://github.com/llvm/llvm-project/commit/adcf33f8fbcc0f068bd4b8254994b16dda525009
DIFF:
https://github.com/llvm/llvm-project/commit/adcf33f8fbcc0f068bd4b8254994b16dda525009.diff
LOG: Revert "Reapply PR/87550 (#94625)"
This reverts commit 35fa2ded2ac52151be22c206fc92b983d1fd8e30.
It broke the LLDB bots on green dragon
Added:
Modified:
lldb/include/lldb/API/SBDebugger.h
lldb/include/lldb/Symbol/TypeSystem.h
lldb/source/API/SBDebugger.cpp
lldb/source/Symbol/TypeSystem.cpp
lldb/tools/lldb-dap/DAP.cpp
lldb/tools/lldb-dap/DAP.h
lldb/tools/lldb-dap/lldb-dap.cpp
Removed:
diff --git a/lldb/include/lldb/API/SBDebugger.h
b/lldb/include/lldb/API/SBDebugger.h
index 84ea9c0f772e1..af19b1faf3bf5 100644
--- a/lldb/include/lldb/API/SBDebugger.h
+++ b/lldb/include/lldb/API/SBDebugger.h
@@ -57,8 +57,6 @@ class LLDB_API SBDebugger {
static const char *GetBroadcasterClass();
- static bool SupportsLanguage(lldb::LanguageType language);
-
lldb::SBBroadcaster GetBroadcaster();
/// Get progress data from a SBEvent whose type is eBroadcastBitProgress.
diff --git a/lldb/include/lldb/Symbol/TypeSystem.h
b/lldb/include/lldb/Symbol/TypeSystem.h
index 7d48f9b316138..b4025c173a186 100644
--- a/lldb/include/lldb/Symbol/TypeSystem.h
+++ b/lldb/include/lldb/Symbol/TypeSystem.h
@@ -209,7 +209,6 @@ class TypeSystem : public PluginInterface,
// TypeSystems can support more than one language
virtual bool SupportsLanguage(lldb::LanguageType language) = 0;
- static bool SupportsLanguageStatic(lldb::LanguageType language);
// Type Completion
virtual bool GetCompleteType(lldb::opaque_compiler_type_t type) = 0;
diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index 29da7d33dd80b..7ef0d6efd4aaa 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -1742,7 +1742,3 @@ bool SBDebugger::InterruptRequested() {
return m_opaque_sp->InterruptRequested();
return false;
}
-
-bool SBDebugger::SupportsLanguage(lldb::LanguageType language) {
- return TypeSystem::SupportsLanguageStatic(language);
-}
diff --git a/lldb/source/Symbol/TypeSystem.cpp
b/lldb/source/Symbol/TypeSystem.cpp
index 5d56d9b1829da..4956f10a0b0a7 100644
--- a/lldb/source/Symbol/TypeSystem.cpp
+++ b/lldb/source/Symbol/TypeSystem.cpp
@@ -335,14 +335,3 @@ TypeSystemMap::GetTypeSystemForLanguage(lldb::LanguageType
language,
}
return GetTypeSystemForLanguage(language);
}
-
-bool TypeSystem::SupportsLanguageStatic(lldb::LanguageType language) {
- if (language == eLanguageTypeUnknown)
-return false;
-
- LanguageSet languages =
- PluginManager::GetAllTypeSystemSupportedLanguagesForTypes();
- if (languages.Empty())
-return false;
- return languages[language];
-}
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 263e841b7254d..d419f821999e6 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -32,7 +32,14 @@ namespace lldb_dap {
DAP g_dap;
DAP::DAP()
-: broadcaster("lldb-dap"), exception_breakpoints(),
+: broadcaster("lldb-dap"),
+ exception_breakpoints(
+ {{"cpp_catch", "C++ Catch", lldb::eLanguageTypeC_plus_plus},
+ {"cpp_throw", "C++ Throw", lldb::eLanguageTypeC_plus_plus},
+ {"objc_catch", "Objective-C Catch", lldb::eLanguageTypeObjC},
+ {"objc_throw", "Objective-C Throw", lldb::eLanguageTypeObjC},
+ {"swift_catch", "Swift Catch", lldb::eLanguageTypeSwift},
+ {"swift_throw", "Swift Throw", lldb::eLanguageTypeSwift}}),
focus_tid(LLDB_INVALID_THREAD_ID), stop_at_entry(false),
is_attach(false),
enable_auto_variable_summaries(false),
enable_synthetic_child_debugging(false),
@@ -58,51 +65,8 @@ DAP::DAP()
DAP::~DAP() = default;
-void DAP::PopulateExceptionBreakpoints() {
- llvm::call_once(initExceptionBreakpoints, [this]() {
-exception_breakpoints = {};
-if (lldb::SBDebugger::SupportsLanguage(lldb::eLanguageTypeC_plus_plus)) {
- exception_breakpoints->emplace_back("cpp_catch", "C++ Catch",
- lldb::eLanguageTypeC_plus_plus);
- exception_breakpoints->emplace_back("cpp_throw", "C++ Throw",
- lldb::eLanguageTypeC_plus_plus);
-}
-if (lldb::SBDebugger::SupportsLanguage(lldb::eLanguageTypeObjC)) {
- exception_breakpoints->emplace_back("objc_catch", "Objective-C Catch",
- lldb::eLanguageTypeObjC);
- exception_breakpoints->emplace_back("objc_throw", "Objective-C Throw",
- lldb::eLanguageTypeObjC);
-}
-if (lldb::SBDebugger::SupportsLanguage(lldb::eLanguageTypeSwif
[Lldb-commits] [lldb] [lldb] Tighten ABI assert in `StopInfoMachException::DeterminePtrauthFailure` (NFC) (PR #95015)
https://github.com/felipepiovezan approved this pull request. https://github.com/llvm/llvm-project/pull/95015 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParser][NFC] Factor out CXX/ObjC method specifics out of ParseSubroutine (PR #95078)
@@ -975,6 +975,219 @@ ConvertDWARFCallingConventionToClang(const
ParsedDWARFTypeAttributes &attrs) {
return clang::CC_C;
}
+bool DWARFASTParserClang::ParseObjCMethod(
+const ObjCLanguage::MethodName &objc_method, const DWARFDIE &die,
+CompilerType clang_type, const ParsedDWARFTypeAttributes &attrs,
+bool is_variadic) {
+ SymbolFileDWARF *dwarf = die.GetDWARF();
+ const auto tag = die.Tag();
+ ConstString class_name(objc_method.GetClassName());
+ if (!class_name)
+return false;
+
+ TypeSP complete_objc_class_type_sp(
+ dwarf->FindCompleteObjCDefinitionTypeForDIE(DWARFDIE(), class_name,
+ false));
+
+ if (!complete_objc_class_type_sp)
+return false;
+
+ CompilerType type_clang_forward_type =
+ complete_objc_class_type_sp->GetForwardCompilerType();
+
+ if (!type_clang_forward_type)
+return false;
+
+ if (!TypeSystemClang::IsObjCObjectOrInterfaceType(type_clang_forward_type))
+return false;
+
+ clang::ObjCMethodDecl *objc_method_decl = m_ast.AddMethodToObjCObjectType(
+ type_clang_forward_type, attrs.name.GetCString(), clang_type,
+ attrs.is_artificial, is_variadic, attrs.is_objc_direct_call);
+
+ if (!objc_method_decl) {
+dwarf->GetObjectFile()->GetModule()->ReportError(
+"[{0:x16}]: invalid Objective-C method {1:x4} ({2}), "
+"please file a bug and attach the file at the start of "
+"this error message",
+die.GetOffset(), tag, DW_TAG_value_to_name(tag));
+return false;
+ }
+
+ LinkDeclContextToDIE(objc_method_decl, die);
+ m_ast.SetMetadataAsUserID(objc_method_decl, die.GetID());
+
+ return true;
+}
+
+std::pair DWARFASTParserClang::ParseCXXMethod(
+const DWARFDIE &die, CompilerType clang_type,
+const ParsedDWARFTypeAttributes &attrs, const DWARFDIE &decl_ctx_die,
+bool is_static, bool &ignore_containing_context) {
+ Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups);
+ SymbolFileDWARF *dwarf = die.GetDWARF();
+
+ Type *class_type = dwarf->ResolveType(decl_ctx_die);
+ if (!class_type)
+return {};
+
+ if (class_type->GetID() != decl_ctx_die.GetID() ||
+ IsClangModuleFwdDecl(decl_ctx_die)) {
+
+// We uniqued the parent class of this function to another
+// class so we now need to associate all dies under
+// "decl_ctx_die" to DIEs in the DIE for "class_type"...
+DWARFDIE class_type_die = dwarf->GetDIE(class_type->GetID());
+
+if (class_type_die) {
+ std::vector failures;
+
+ CopyUniqueClassMethodTypes(decl_ctx_die, class_type_die, class_type,
+ failures);
+
+ // FIXME do something with these failures that's
+ // smarter than just dropping them on the ground.
+ // Unfortunately classes don't like having stuff added
+ // to them after their definitions are complete...
+
+ Type *type_ptr = dwarf->GetDIEToType()[die.GetDIE()];
+ if (type_ptr && type_ptr != DIE_IS_BEING_PARSED) {
+return {true, type_ptr->shared_from_this()};
+ }
felipepiovezan wrote:
all the other single-statement if's here are without {}
https://github.com/llvm/llvm-project/pull/95078
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParser][NFC] Factor out CXX/ObjC method specifics out of ParseSubroutine (PR #95078)
@@ -975,6 +975,219 @@ ConvertDWARFCallingConventionToClang(const
ParsedDWARFTypeAttributes &attrs) {
return clang::CC_C;
}
+bool DWARFASTParserClang::ParseObjCMethod(
+const ObjCLanguage::MethodName &objc_method, const DWARFDIE &die,
+CompilerType clang_type, const ParsedDWARFTypeAttributes &attrs,
+bool is_variadic) {
+ SymbolFileDWARF *dwarf = die.GetDWARF();
+ const auto tag = die.Tag();
+ ConstString class_name(objc_method.GetClassName());
+ if (!class_name)
+return false;
+
+ TypeSP complete_objc_class_type_sp(
+ dwarf->FindCompleteObjCDefinitionTypeForDIE(DWARFDIE(), class_name,
+ false));
+
+ if (!complete_objc_class_type_sp)
+return false;
+
+ CompilerType type_clang_forward_type =
+ complete_objc_class_type_sp->GetForwardCompilerType();
+
+ if (!type_clang_forward_type)
+return false;
+
+ if (!TypeSystemClang::IsObjCObjectOrInterfaceType(type_clang_forward_type))
+return false;
+
+ clang::ObjCMethodDecl *objc_method_decl = m_ast.AddMethodToObjCObjectType(
+ type_clang_forward_type, attrs.name.GetCString(), clang_type,
+ attrs.is_artificial, is_variadic, attrs.is_objc_direct_call);
+
+ if (!objc_method_decl) {
+dwarf->GetObjectFile()->GetModule()->ReportError(
+"[{0:x16}]: invalid Objective-C method {1:x4} ({2}), "
+"please file a bug and attach the file at the start of "
+"this error message",
+die.GetOffset(), tag, DW_TAG_value_to_name(tag));
+return false;
+ }
+
+ LinkDeclContextToDIE(objc_method_decl, die);
+ m_ast.SetMetadataAsUserID(objc_method_decl, die.GetID());
+
+ return true;
+}
+
+std::pair DWARFASTParserClang::ParseCXXMethod(
+const DWARFDIE &die, CompilerType clang_type,
+const ParsedDWARFTypeAttributes &attrs, const DWARFDIE &decl_ctx_die,
+bool is_static, bool &ignore_containing_context) {
+ Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups);
+ SymbolFileDWARF *dwarf = die.GetDWARF();
+
+ Type *class_type = dwarf->ResolveType(decl_ctx_die);
+ if (!class_type)
+return {};
+
+ if (class_type->GetID() != decl_ctx_die.GetID() ||
+ IsClangModuleFwdDecl(decl_ctx_die)) {
+
+// We uniqued the parent class of this function to another
+// class so we now need to associate all dies under
+// "decl_ctx_die" to DIEs in the DIE for "class_type"...
+DWARFDIE class_type_die = dwarf->GetDIE(class_type->GetID());
felipepiovezan wrote:
we can reduce the scope of this variable by folding its declaration inside the
if statement, it's not used outside
https://github.com/llvm/llvm-project/pull/95078
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParser][NFC] Factor out CXX/ObjC method specifics out of ParseSubroutine (PR #95078)
https://github.com/felipepiovezan approved this pull request. It's say something that, even after this refactor, the methods are still pretty big. But this is still a win https://github.com/llvm/llvm-project/pull/95078 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParser][NFC] Factor out CXX/ObjC method specifics out of ParseSubroutine (PR #95078)
@@ -975,6 +975,219 @@ ConvertDWARFCallingConventionToClang(const
ParsedDWARFTypeAttributes &attrs) {
return clang::CC_C;
}
+bool DWARFASTParserClang::ParseObjCMethod(
+const ObjCLanguage::MethodName &objc_method, const DWARFDIE &die,
+CompilerType clang_type, const ParsedDWARFTypeAttributes &attrs,
+bool is_variadic) {
+ SymbolFileDWARF *dwarf = die.GetDWARF();
+ const auto tag = die.Tag();
+ ConstString class_name(objc_method.GetClassName());
+ if (!class_name)
+return false;
+
+ TypeSP complete_objc_class_type_sp(
+ dwarf->FindCompleteObjCDefinitionTypeForDIE(DWARFDIE(), class_name,
+ false));
+
+ if (!complete_objc_class_type_sp)
+return false;
+
+ CompilerType type_clang_forward_type =
+ complete_objc_class_type_sp->GetForwardCompilerType();
+
+ if (!type_clang_forward_type)
+return false;
+
+ if (!TypeSystemClang::IsObjCObjectOrInterfaceType(type_clang_forward_type))
+return false;
+
+ clang::ObjCMethodDecl *objc_method_decl = m_ast.AddMethodToObjCObjectType(
+ type_clang_forward_type, attrs.name.GetCString(), clang_type,
+ attrs.is_artificial, is_variadic, attrs.is_objc_direct_call);
+
+ if (!objc_method_decl) {
+dwarf->GetObjectFile()->GetModule()->ReportError(
+"[{0:x16}]: invalid Objective-C method {1:x4} ({2}), "
+"please file a bug and attach the file at the start of "
+"this error message",
+die.GetOffset(), tag, DW_TAG_value_to_name(tag));
+return false;
+ }
+
+ LinkDeclContextToDIE(objc_method_decl, die);
+ m_ast.SetMetadataAsUserID(objc_method_decl, die.GetID());
+
+ return true;
+}
+
+std::pair DWARFASTParserClang::ParseCXXMethod(
+const DWARFDIE &die, CompilerType clang_type,
+const ParsedDWARFTypeAttributes &attrs, const DWARFDIE &decl_ctx_die,
+bool is_static, bool &ignore_containing_context) {
+ Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups);
+ SymbolFileDWARF *dwarf = die.GetDWARF();
felipepiovezan wrote:
I'd also assert for this
https://github.com/llvm/llvm-project/pull/95078
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParser][NFC] Factor out CXX/ObjC method specifics out of ParseSubroutine (PR #95078)
@@ -975,6 +975,219 @@ ConvertDWARFCallingConventionToClang(const
ParsedDWARFTypeAttributes &attrs) {
return clang::CC_C;
}
+bool DWARFASTParserClang::ParseObjCMethod(
+const ObjCLanguage::MethodName &objc_method, const DWARFDIE &die,
+CompilerType clang_type, const ParsedDWARFTypeAttributes &attrs,
+bool is_variadic) {
+ SymbolFileDWARF *dwarf = die.GetDWARF();
felipepiovezan wrote:
I know the previous code was not doing this, but let's take this chance to
assert this is not null.
https://github.com/llvm/llvm-project/pull/95078
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParser][NFC] Factor out CXX/ObjC method specifics out of ParseSubroutine (PR #95078)
https://github.com/felipepiovezan edited https://github.com/llvm/llvm-project/pull/95078 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParser][NFC] Factor out CXX/ObjC method specifics out of ParseSubroutine (PR #95078)
@@ -975,6 +975,219 @@ ConvertDWARFCallingConventionToClang(const
ParsedDWARFTypeAttributes &attrs) {
return clang::CC_C;
}
+bool DWARFASTParserClang::ParseObjCMethod(
+const ObjCLanguage::MethodName &objc_method, const DWARFDIE &die,
+CompilerType clang_type, const ParsedDWARFTypeAttributes &attrs,
+bool is_variadic) {
+ SymbolFileDWARF *dwarf = die.GetDWARF();
+ const auto tag = die.Tag();
+ ConstString class_name(objc_method.GetClassName());
+ if (!class_name)
+return false;
+
+ TypeSP complete_objc_class_type_sp(
+ dwarf->FindCompleteObjCDefinitionTypeForDIE(DWARFDIE(), class_name,
felipepiovezan wrote:
should we write this as an assignment instead? It is the same thing, but at
least the code is consistent with how other variables are initialized
https://github.com/llvm/llvm-project/pull/95078
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParser][NFC] Factor out CXX/ObjC method specifics out of ParseSubroutine (PR #95078)
https://github.com/felipepiovezan edited https://github.com/llvm/llvm-project/pull/95078 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 2e007b8 - [lldb] Skip TestAttachDenied under asan
Author: Felipe de Azevedo Piovezan Date: 2024-06-11T09:28:10-07:00 New Revision: 2e007b89c65eeb33baf1b40103284d8937700cf0 URL: https://github.com/llvm/llvm-project/commit/2e007b89c65eeb33baf1b40103284d8937700cf0 DIFF: https://github.com/llvm/llvm-project/commit/2e007b89c65eeb33baf1b40103284d8937700cf0.diff LOG: [lldb] Skip TestAttachDenied under asan Like many other tests, this one times out when run under the address sanitizer. To reduce noise, this commit skips it in those builds. Added: Modified: lldb/test/API/commands/process/attach/attach_denied/TestAttachDenied.py Removed: diff --git a/lldb/test/API/commands/process/attach/attach_denied/TestAttachDenied.py b/lldb/test/API/commands/process/attach/attach_denied/TestAttachDenied.py index 22dca62045022..d72a710e8127b 100644 --- a/lldb/test/API/commands/process/attach/attach_denied/TestAttachDenied.py +++ b/lldb/test/API/commands/process/attach/attach_denied/TestAttachDenied.py @@ -18,6 +18,7 @@ class AttachDeniedTestCase(TestBase): @skipIfWindows @skipIfiOSSimulator @skipIfDarwinEmbedded # ptrace(ATTACH_REQUEST...) won't work on ios/tvos/etc +@skipIfAsan # Times out inconsistently under asan def test_attach_to_process_by_id_denied(self): """Test attach by process id denied""" self.build() ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Disable PIE for some API tests (PR #93808)
felipepiovezan wrote: hi @dzhidzhoev, I think this commit is causing the arm lldb incremental bots to fail. Could you have a look or revert if you think the fix is not obvious? This is our most important bot, so it would be nice to get it green again https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake/5651/console https://github.com/llvm/llvm-project/pull/93808 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Disable PIE for some API tests (PR #93808)
felipepiovezan wrote: @dzhidzhoev I think @labath 's suggestion might fix this (LD_EXTRAS) https://github.com/llvm/llvm-project/pull/93808 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] c579020 - [lldb] Fix linker flags in lldb tests
Author: Felipe de Azevedo Piovezan Date: 2024-06-12T09:32:54-07:00 New Revision: c5790206f719c8fac168ae488420f31800d55cf0 URL: https://github.com/llvm/llvm-project/commit/c5790206f719c8fac168ae488420f31800d55cf0 DIFF: https://github.com/llvm/llvm-project/commit/c5790206f719c8fac168ae488420f31800d55cf0.diff LOG: [lldb] Fix linker flags in lldb tests This is a fixup to https://github.com/llvm/llvm-project/pull/93808, which used LDFLAGS instead of the correct LD_EXTRAS Added: Modified: lldb/test/API/commands/target/basic/Makefile lldb/test/API/lang/c/global_variables/Makefile lldb/test/API/lang/cpp/char8_t/Makefile Removed: diff --git a/lldb/test/API/commands/target/basic/Makefile b/lldb/test/API/commands/target/basic/Makefile index e66971834b689..1f1b61dbd6316 100644 --- a/lldb/test/API/commands/target/basic/Makefile +++ b/lldb/test/API/commands/target/basic/Makefile @@ -4,7 +4,7 @@ # EXE := b.out ifndef PIE - LDFLAGS := -no-pie + LD_EXTRAS := -no-pie endif include Makefile.rules diff --git a/lldb/test/API/lang/c/global_variables/Makefile b/lldb/test/API/lang/c/global_variables/Makefile index 00c2557033d81..acd6c56470b6f 100644 --- a/lldb/test/API/lang/c/global_variables/Makefile +++ b/lldb/test/API/lang/c/global_variables/Makefile @@ -3,7 +3,7 @@ C_SOURCES := main.c DYLIB_NAME := a DYLIB_C_SOURCES := a.c ifndef PIE - LDFLAGS := -no-pie + LD_EXTRAS := -no-pie endif include Makefile.rules diff --git a/lldb/test/API/lang/cpp/char8_t/Makefile b/lldb/test/API/lang/cpp/char8_t/Makefile index 28f982a0078d8..7ae9c7189298c 100644 --- a/lldb/test/API/lang/cpp/char8_t/Makefile +++ b/lldb/test/API/lang/cpp/char8_t/Makefile @@ -1,7 +1,7 @@ CXX_SOURCES := main.cpp CXXFLAGS_EXTRAS := -std=c++2a -fchar8_t ifndef PIE - LDFLAGS := -no-pie + LD_EXTRAS := -no-pie endif include Makefile.rules ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Disable PIE for some API tests (PR #93808)
felipepiovezan wrote: Pushed a fix https://github.com/llvm/llvm-project/pull/93808 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Support case-insensitive regex matches (PR #95350)
felipepiovezan wrote: > LGTM, may be we could also support this for the command line Just keep in mind that those are very different queries w.r.t. speed: one goes through a fast path in the accelerator table, the other one doesn't. https://github.com/llvm/llvm-project/pull/95350 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Force dwarf4 usage in test requiring it (PR #95449)
https://github.com/felipepiovezan created https://github.com/llvm/llvm-project/pull/95449 This test is explicitly checking for dwarf 4 behavior on Apple platforms, so we should explicitly use the dwarf4 flag. Related to https://github.com/llvm/llvm-project/pull/95164 >From d4915d55cfbef6f9d4183db648725b3edf5e8024 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Thu, 13 Jun 2024 11:16:45 -0700 Subject: [PATCH] [lldb][test] Force dwarf4 usage in test requiring it This test is explicitly checking for dwarf 4 behavior on Apple platforms, so we should explicitly use the dwarf4 flag. --- lldb/test/Shell/SymbolFile/DWARF/x86/apple-index-is-used.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/apple-index-is-used.cpp b/lldb/test/Shell/SymbolFile/DWARF/x86/apple-index-is-used.cpp index 00440531e99f7..5bcb2cbcbbe29 100644 --- a/lldb/test/Shell/SymbolFile/DWARF/x86/apple-index-is-used.cpp +++ b/lldb/test/Shell/SymbolFile/DWARF/x86/apple-index-is-used.cpp @@ -1,5 +1,5 @@ // Test that we use the apple indexes. -// RUN: %clang %s -g -c -o %t --target=x86_64-apple-macosx +// RUN: %clang %s -g -c -o %t --target=x86_64-apple-macosx -gdwarf-4 // RUN: lldb-test symbols %t | FileCheck %s // CHECK: .apple_names index present ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Tweak Python interpreter workaround on macOS (PR #95582)
https://github.com/felipepiovezan approved this pull request. https://github.com/llvm/llvm-project/pull/95582 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Force dwarf4 usage in test requiring it (PR #95449)
https://github.com/felipepiovezan closed https://github.com/llvm/llvm-project/pull/95449 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Search fallback to the manual index in GetFullyQualified… (PR #102123)
felipepiovezan wrote: Isn't this going to degrade the performance of _all_ negative queries? I don't remember off the top of my head what is "m_fallback" when we have an accelerator table, but this worries me. https://github.com/llvm/llvm-project/pull/102123 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Search fallback to the manual index in GetFullyQualified… (PR #102123)
felipepiovezan wrote: > The fallback index will only index those units that aren't covered by the > debug_names index, so if debug_names covers everything, the fallback is a noop Got it, this is what I was hoping that would happen! Thanks for explaining it. (future ideas) This makes me wonder if a better design would have been to make whoever owns this instances of Index classes have instead a collection of these indices, instead of having an _implementation_ of an index have to remember to call a fallback mechanism. To answer this question with another question: since Index classes are not allowed to have false negatives / positives, we would never need more than one index (for a give CU). Which begs the question of why we have a design that allows us to reach the problem this PR is fixing? It feels off that we have two objects: a manual index and a dwarf index, and that the dwarf index has to remember to call the manual index when the dwarf index is empty. https://github.com/llvm/llvm-project/pull/102123 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Search fallback to the manual index in GetFullyQualified… (PR #102123)
felipepiovezan wrote:
> The current setup makes sense to me, but I guess that's expected as I'm the
> one who created it. I can also imagine something like what you propose, but
> it doesn't seem like a clear win to me. These objects are owned by
> SymbolFileDWARF, and we probably don't want to have it do the work of
> juggling these (it has a lot on its plate already), so it would probably have
> to be a separate object (basically another implementation of the "dwarf
> index" interface). That would be a lot of boilerplate (though maybe we could
> use some template trickery to reduce it). We'd also need to come up with a
> less ad-hoc way communicate which things are supposed to be indexed by who,
> but we also wouldn't want to make it too generic (== more code), since there
> are basically only three index configurations we care about (and these could
> easily be reduced to two).
I guess what I was proposing was more about deleting code than adding it. We
must always have at most _one_ index, but we have a lot of code allowing for
the situation of two indices (manual & {apple, dwarf}).
https://github.com/llvm/llvm-project/pull/102123
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [GDBRemote] Fix processing of comma-separated memory region entries (PR #105873)
https://github.com/felipepiovezan created
https://github.com/llvm/llvm-project/pull/105873
The existing algorithm was performing the following comparisons for an
`aaa,bbb,ccc,ddd`:
aaa\0bbb,ccc,ddd == "stack"
aaa\0bbb\0ccc,ddd == "stack"
aaa\0bbb\0ccc\0ddd == "stack"
Which wouldn't work. This commit just dispatches to a known algorithm
implementation.
>From 8ef8ecdc8d87d474b906835d04ffdd9a19216406 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan
Date: Fri, 23 Aug 2024 11:43:16 -0700
Subject: [PATCH] [GDBRemote] Fix processing of comma-separated memory region
entries
The existing algorithm was performing the following comparisons for an
`aaa,bbb,ccc,ddd`:
aaa\0bbb,ccc,ddd == "stack"
aaa\0bbb\0ccc,ddd == "stack"
aaa\0bbb\0ccc\0ddd == "stack"
Which wouldn't work. This commit just dispatches to a known algorithm
implementation.
---
.../gdb-remote/GDBRemoteCommunicationClient.cpp | 12 ++--
.../gdb-remote/GDBRemoteCommunicationClientTest.cpp | 7 +--
2 files changed, 7 insertions(+), 12 deletions(-)
diff --git
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 83ba27783da471..d80ccae0518088 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1632,17 +1632,9 @@ Status GDBRemoteCommunicationClient::GetMemoryRegionInfo(
}
}
} else if (name == "type") {
- std::string comma_sep_str = value.str();
- size_t comma_pos;
- while ((comma_pos = comma_sep_str.find(',')) != std::string::npos) {
-comma_sep_str[comma_pos] = '\0';
-if (comma_sep_str == "stack") {
+ for (llvm::StringRef entry: llvm::split(value, ',')) {
+if (entry == "stack")
region_info.SetIsStackMemory(MemoryRegionInfo::eYes);
-}
- }
- // handle final (or only) type of "stack"
- if (comma_sep_str == "stack") {
-region_info.SetIsStackMemory(MemoryRegionInfo::eYes);
}
} else if (name == "error") {
StringExtractorGDBRemote error_extractor(value);
diff --git
a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
index 11e14f9472164d..18020c8e43fe06 100644
--- a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -343,24 +343,27 @@ TEST_F(GDBRemoteCommunicationClientTest,
GetMemoryRegionInfo) {
EXPECT_EQ(MemoryRegionInfo::eYes, region_info.GetExecutable());
EXPECT_EQ("/foo/bar.so", region_info.GetName().GetStringRef());
EXPECT_EQ(MemoryRegionInfo::eDontKnow, region_info.GetMemoryTagged());
+ EXPECT_EQ(MemoryRegionInfo::eDontKnow, region_info.IsStackMemory());
result = std::async(std::launch::async, [&] {
return client.GetMemoryRegionInfo(addr, region_info);
});
HandlePacket(server, "qMemoryRegionInfo:a000",
- "start:a000;size:2000;flags:;");
+ "start:a000;size:2000;flags:;type:stack;");
EXPECT_TRUE(result.get().Success());
EXPECT_EQ(MemoryRegionInfo::eNo, region_info.GetMemoryTagged());
+ EXPECT_EQ(MemoryRegionInfo::eYes, region_info.IsStackMemory());
result = std::async(std::launch::async, [&] {
return client.GetMemoryRegionInfo(addr, region_info);
});
HandlePacket(server, "qMemoryRegionInfo:a000",
- "start:a000;size:2000;flags: mt zz mt ;");
+ "start:a000;size:2000;flags: mt zz mt ;type:ha,ha,stack;");
EXPECT_TRUE(result.get().Success());
EXPECT_EQ(MemoryRegionInfo::eYes, region_info.GetMemoryTagged());
+ EXPECT_EQ(MemoryRegionInfo::eYes, region_info.IsStackMemory());
}
TEST_F(GDBRemoteCommunicationClientTest, GetMemoryRegionInfoInvalidResponse) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [GDBRemote] Fix processing of comma-separated memory region entries (PR #105873)
felipepiovezan wrote: Since this is mostly uncontroversial, I am going to go ahead and merge it now. Happy to address feedback in post! https://github.com/llvm/llvm-project/pull/105873 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [GDBRemote] Fix processing of comma-separated memory region entries (PR #105873)
https://github.com/felipepiovezan updated
https://github.com/llvm/llvm-project/pull/105873
>From fbe4fdfb0e804c281ed5c52382ef4bb16ec9fce5 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan
Date: Fri, 23 Aug 2024 11:43:16 -0700
Subject: [PATCH] [GDBRemote] Fix processing of comma-separated memory region
entries
The existing algorithm was performing the following comparisons for an
`aaa,bbb,ccc,ddd`:
aaa\0bbb,ccc,ddd == "stack"
aaa\0bbb\0ccc,ddd == "stack"
aaa\0bbb\0ccc\0ddd == "stack"
Which wouldn't work. This commit just dispatches to a known algorithm
implementation.
---
.../gdb-remote/GDBRemoteCommunicationClient.cpp | 12 ++--
.../gdb-remote/GDBRemoteCommunicationClientTest.cpp | 7 +--
2 files changed, 7 insertions(+), 12 deletions(-)
diff --git
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 83ba27783da471..d7a0baa488edc5 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1632,17 +1632,9 @@ Status GDBRemoteCommunicationClient::GetMemoryRegionInfo(
}
}
} else if (name == "type") {
- std::string comma_sep_str = value.str();
- size_t comma_pos;
- while ((comma_pos = comma_sep_str.find(',')) != std::string::npos) {
-comma_sep_str[comma_pos] = '\0';
-if (comma_sep_str == "stack") {
+ for (llvm::StringRef entry : llvm::split(value, ',')) {
+if (entry == "stack")
region_info.SetIsStackMemory(MemoryRegionInfo::eYes);
-}
- }
- // handle final (or only) type of "stack"
- if (comma_sep_str == "stack") {
-region_info.SetIsStackMemory(MemoryRegionInfo::eYes);
}
} else if (name == "error") {
StringExtractorGDBRemote error_extractor(value);
diff --git
a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
index 11e14f9472164d..18020c8e43fe06 100644
--- a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -343,24 +343,27 @@ TEST_F(GDBRemoteCommunicationClientTest,
GetMemoryRegionInfo) {
EXPECT_EQ(MemoryRegionInfo::eYes, region_info.GetExecutable());
EXPECT_EQ("/foo/bar.so", region_info.GetName().GetStringRef());
EXPECT_EQ(MemoryRegionInfo::eDontKnow, region_info.GetMemoryTagged());
+ EXPECT_EQ(MemoryRegionInfo::eDontKnow, region_info.IsStackMemory());
result = std::async(std::launch::async, [&] {
return client.GetMemoryRegionInfo(addr, region_info);
});
HandlePacket(server, "qMemoryRegionInfo:a000",
- "start:a000;size:2000;flags:;");
+ "start:a000;size:2000;flags:;type:stack;");
EXPECT_TRUE(result.get().Success());
EXPECT_EQ(MemoryRegionInfo::eNo, region_info.GetMemoryTagged());
+ EXPECT_EQ(MemoryRegionInfo::eYes, region_info.IsStackMemory());
result = std::async(std::launch::async, [&] {
return client.GetMemoryRegionInfo(addr, region_info);
});
HandlePacket(server, "qMemoryRegionInfo:a000",
- "start:a000;size:2000;flags: mt zz mt ;");
+ "start:a000;size:2000;flags: mt zz mt ;type:ha,ha,stack;");
EXPECT_TRUE(result.get().Success());
EXPECT_EQ(MemoryRegionInfo::eYes, region_info.GetMemoryTagged());
+ EXPECT_EQ(MemoryRegionInfo::eYes, region_info.IsStackMemory());
}
TEST_F(GDBRemoteCommunicationClientTest, GetMemoryRegionInfoInvalidResponse) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [GDBRemote] Fix processing of comma-separated memory region entries (PR #105873)
felipepiovezan wrote: Fixed clang format issue https://github.com/llvm/llvm-project/pull/105873 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [GDBRemote] Fix processing of comma-separated memory region entries (PR #105873)
https://github.com/felipepiovezan closed https://github.com/llvm/llvm-project/pull/105873 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [GDBRemote] Handle 'heap' memory region info type (PR #105883)
https://github.com/felipepiovezan created
https://github.com/llvm/llvm-project/pull/105883
This should cause the memory region info "is stack" field to be set to "no".
>From 0730cd7f48dd4b80fba0275fed9beadcf6193987 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan
Date: Fri, 23 Aug 2024 13:10:00 -0700
Subject: [PATCH] [GDBRemote] Handle 'heap' memory region info type
This should cause the memory region info "is stack" field to be set to "no".
---
.../Process/gdb-remote/GDBRemoteCommunicationClient.cpp | 2 ++
.../gdb-remote/GDBRemoteCommunicationClientTest.cpp | 9 +
2 files changed, 11 insertions(+)
diff --git
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index d7a0baa488edc5..6fbbfb03ed1176 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1635,6 +1635,8 @@ Status GDBRemoteCommunicationClient::GetMemoryRegionInfo(
for (llvm::StringRef entry : llvm::split(value, ',')) {
if (entry == "stack")
region_info.SetIsStackMemory(MemoryRegionInfo::eYes);
+if (entry == "heap")
+ region_info.SetIsStackMemory(MemoryRegionInfo::eNo);
}
} else if (name == "error") {
StringExtractorGDBRemote error_extractor(value);
diff --git
a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
index 18020c8e43fe06..ce5ab2cf508293 100644
--- a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -364,6 +364,15 @@ TEST_F(GDBRemoteCommunicationClientTest,
GetMemoryRegionInfo) {
EXPECT_TRUE(result.get().Success());
EXPECT_EQ(MemoryRegionInfo::eYes, region_info.GetMemoryTagged());
EXPECT_EQ(MemoryRegionInfo::eYes, region_info.IsStackMemory());
+
+ result = std::async(std::launch::async, [&] {
+return client.GetMemoryRegionInfo(addr, region_info);
+ });
+
+ HandlePacket(server, "qMemoryRegionInfo:a000",
+ "start:a000;size:2000;type:heap;");
+ EXPECT_TRUE(result.get().Success());
+ EXPECT_EQ(MemoryRegionInfo::eNo, region_info.IsStackMemory());
}
TEST_F(GDBRemoteCommunicationClientTest, GetMemoryRegionInfoInvalidResponse) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [GDBRemote] Handle 'heap' memory region info type (PR #105883)
https://github.com/felipepiovezan updated
https://github.com/llvm/llvm-project/pull/105883
>From e91140e2c059df5f65ad907e9c93329c75e36dc4 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan
Date: Fri, 23 Aug 2024 13:10:00 -0700
Subject: [PATCH 1/2] [GDBRemote] Handle 'heap' memory region info type
This should cause the memory region info "is stack" field to be set to "no".
---
.../Process/gdb-remote/GDBRemoteCommunicationClient.cpp | 2 ++
.../gdb-remote/GDBRemoteCommunicationClientTest.cpp | 9 +
2 files changed, 11 insertions(+)
diff --git
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 0297fe363f69e1..ac04049548e437 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1638,6 +1638,8 @@ Status GDBRemoteCommunicationClient::GetMemoryRegionInfo(
for (llvm::StringRef entry : llvm::split(value, ',')) {
if (entry == "stack")
region_info.SetIsStackMemory(MemoryRegionInfo::eYes);
+if (entry == "heap")
+ region_info.SetIsStackMemory(MemoryRegionInfo::eNo);
}
} else if (name == "error") {
StringExtractorGDBRemote error_extractor(value);
diff --git
a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
index 18020c8e43fe06..ce5ab2cf508293 100644
--- a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -364,6 +364,15 @@ TEST_F(GDBRemoteCommunicationClientTest,
GetMemoryRegionInfo) {
EXPECT_TRUE(result.get().Success());
EXPECT_EQ(MemoryRegionInfo::eYes, region_info.GetMemoryTagged());
EXPECT_EQ(MemoryRegionInfo::eYes, region_info.IsStackMemory());
+
+ result = std::async(std::launch::async, [&] {
+return client.GetMemoryRegionInfo(addr, region_info);
+ });
+
+ HandlePacket(server, "qMemoryRegionInfo:a000",
+ "start:a000;size:2000;type:heap;");
+ EXPECT_TRUE(result.get().Success());
+ EXPECT_EQ(MemoryRegionInfo::eNo, region_info.IsStackMemory());
}
TEST_F(GDBRemoteCommunicationClientTest, GetMemoryRegionInfoInvalidResponse) {
>From 4f874c829643c12283afd87c52b762a897c93d11 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan
Date: Wed, 4 Sep 2024 07:02:16 -0700
Subject: [PATCH 2/2] Fixup! address review comments
---
.../Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index ac04049548e437..55d76ca8532d39 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1638,7 +1638,7 @@ Status GDBRemoteCommunicationClient::GetMemoryRegionInfo(
for (llvm::StringRef entry : llvm::split(value, ',')) {
if (entry == "stack")
region_info.SetIsStackMemory(MemoryRegionInfo::eYes);
-if (entry == "heap")
+else if (entry == "heap")
region_info.SetIsStackMemory(MemoryRegionInfo::eNo);
}
} else if (name == "error") {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [GDBRemote] Handle 'heap' memory region info type (PR #105883)
https://github.com/felipepiovezan closed https://github.com/llvm/llvm-project/pull/105883 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make deep copies of Status explicit (NFC) (PR #107170)
felipepiovezan wrote: I'm not if this is your intention, but the way LLVM has configured their github, your two nicely separated commits will be squashed :( https://github.com/llvm/llvm-project/pull/107170 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make deep copies of Status explicit (NFC) (PR #107170)
felipepiovezan wrote: Ah I've just noticed the other PR https://github.com/llvm/llvm-project/pull/107170 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make deep copies of Status explicit (NFC) (PR #107170)
felipepiovezan wrote: > > Ah I've just noticed the other PR > > Do we actually have abetter solution for stacked commits than what I'm doing > here? short answer is "maybe": https://llvm.org/docs/GitHub.html#using-graphite-for-stacked-pull-requests https://github.com/llvm/llvm-project/pull/107170 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix some tests that fail with system libstdc++ (PR #106885)
felipepiovezan wrote: > @felipepiovezan Are you OK if we push this as-is, or would you like smarter > logic for detecting libc++ on the system? I'm fine with this; in hindsight, I think the original workaround was not good enough. The longer term fix would be set both "use system std library" "use libcxx" and skip the test if it's impossible to satisfy both requirements at the same time. But we can do that later if this becomes a problem. https://github.com/llvm/llvm-project/pull/106885 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix some tests that fail with system libstdc++ (PR #106885)
https://github.com/felipepiovezan approved this pull request. https://github.com/llvm/llvm-project/pull/106885 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] TestDbgInfoContentVectorFromStdModule.py: skip test on Darwin (PR #108003)
https://github.com/felipepiovezan approved this pull request. LGTM! Not sure why the formatter is complaining about a single line deletion https://github.com/llvm/llvm-project/pull/108003 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][testing] Check all stop reasons in get_threads_stopped_at_breakpoint_id (PR #108281)
https://github.com/felipepiovezan created https://github.com/llvm/llvm-project/pull/108281 If multiple breakpoints are hit at the same time, multiple stop reasons are reported, one per breakpoint. Currently, `get_threads_stopped_at_breakpoint_id` only checks the first such reason. >From 09a4e6192345541c5c3ce7c3a78a64e8a29e3c64 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Wed, 11 Sep 2024 12:45:18 -0700 Subject: [PATCH] [lldb][testing] Check all stop reasons in get_threads_stopped_at_breakpoint_id If multiple breakpoints are hit at the same time, multiple stop reasons are reported, one per breakpoint. Currently, `get_threads_stopped_at_breakpoint_id` only checks the first such reason. --- lldb/packages/Python/lldbsuite/test/lldbutil.py | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/lldb/packages/Python/lldbsuite/test/lldbutil.py b/lldb/packages/Python/lldbsuite/test/lldbutil.py index 629565b38ca1e6..660a3c085a908a 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbutil.py +++ b/lldb/packages/Python/lldbsuite/test/lldbutil.py @@ -773,9 +773,16 @@ def get_threads_stopped_at_breakpoint_id(process, bpid): return threads for thread in stopped_threads: -# Make sure we've hit our breakpoint... -break_id = thread.GetStopReasonDataAtIndex(0) -if break_id == bpid: +# Make sure we've hit our breakpoint. +# From the docs of GetStopReasonDataAtIndex: "Breakpoint stop reasons +# will have data that consists of pairs of breakpoint IDs followed by +# the breakpoint location IDs". +# Iterate over all such pairs looking for `bpid`. +break_ids = [ +thread.GetStopReasonDataAtIndex(idx) +for idx in range(0, thread.GetStopReasonDataCount(), 2) +] +if bpid in break_ids: threads.append(thread) return threads ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][testing] Check all stop reasons in get_threads_stopped_at_breakpoint_id (PR #108281)
felipepiovezan wrote: I only hit this bug in a downstream test. It seems we don't directly test lldbutil, but none of the tests regress with this change, so we should be confident it is ok. https://github.com/llvm/llvm-project/pull/108281 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][testing] Check all stop reasons in get_threads_stopped_at_breakpoint_id (PR #108281)
https://github.com/felipepiovezan closed https://github.com/llvm/llvm-project/pull/108281 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add more helper functions to CompilerType class (second try). (PR #73472)
@@ -436,8 +482,8 @@ class CompilerType {
ExecutionContextScope *exe_scope);
/// Dump to stdout.
- void DumpTypeDescription(lldb::DescriptionLevel level =
- lldb::eDescriptionLevelFull) const;
felipepiovezan wrote:
I understand the desire to fix formatting issues, but let's avoid doing them in
a big patch like this. If things needs to be reverted, as this patch was, such
a formatting change should not need to be reverted.
This can also cause unnecessary work for downstream forks in case of merge
conflicts (the first patch, the revert, the second patch, etc).
https://github.com/llvm/llvm-project/pull/73472
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add more helper functions to CompilerType class (second try). (PR #73472)
@@ -249,7 +250,7 @@ bool CompilerType::IsPossibleDynamicType(CompilerType *dynamic_pointee_type, if (IsValid()) if (auto type_system_sp = GetTypeSystem()) return type_system_sp->IsPossibleDynamicType(m_type, dynamic_pointee_type, -check_cplusplus, check_objc); + check_cplusplus, check_objc); felipepiovezan wrote: same issue with formatting as mentioned before https://github.com/llvm/llvm-project/pull/73472 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add more helper functions to CompilerType class (second try). (PR #73472)
@@ -302,6 +303,256 @@ bool CompilerType::IsBeingDefined() const {
return false;
}
+bool CompilerType::IsSmartPtrType() const {
+ // These regular expressions cover shared, unique and weak pointers both from
+ // stdlibc++ and libc+++.
+
+ static llvm::Regex k_libcxx_std_unique_ptr_regex(
+ "^std::__[[:alnum:]]+::unique_ptr<.+>(( )?&)?$");
+ static llvm::Regex k_libcxx_std_shared_ptr_regex(
+ "^std::__[[:alnum:]]+::shared_ptr<.+>(( )?&)?$");
+ static llvm::Regex k_libcxx_std_weak_ptr_regex(
+ "^std::__[[:alnum:]]+::weak_ptr<.+>(( )?&)?$");
+ //
+ static llvm::Regex k_libcxx_std_unique_ptr_regex_2(
+ "^std::unique_ptr<.+>(( )?&)?$");
+ static llvm::Regex k_libcxx_std_shared_ptr_regex_2(
+ "^std::shared_ptr<.+>(( )?&)?$");
+ static llvm::Regex k_libcxx_std_weak_ptr_regex_2(
+ "^std::weak_ptr<.+>(( )?&)?$");
+ //
+ llvm::StringRef name = GetTypeName();
+ return k_libcxx_std_unique_ptr_regex.match(name) ||
+ k_libcxx_std_shared_ptr_regex.match(name) ||
+ k_libcxx_std_weak_ptr_regex.match(name) ||
+ k_libcxx_std_unique_ptr_regex_2.match(name) ||
+ k_libcxx_std_shared_ptr_regex_2.match(name) ||
+ k_libcxx_std_weak_ptr_regex_2.match(name);
+}
+
+bool CompilerType::IsInteger() const {
+ // This is used when you don't care about the signedness of the integer.
+ bool is_signed;
+ return IsIntegerType(is_signed);
+}
+
+bool CompilerType::IsFloat() const {
+ uint32_t count = 0;
+ bool is_complex = false;
+ return IsFloatingPointType(count, is_complex);
+}
+
+bool CompilerType::IsEnumerationType() const {
+ // This is used when you don't care about the signedness of the enum.
+ bool is_signed;
+ return IsEnumerationType(is_signed);
+}
+
+bool CompilerType::IsUnscopedEnumerationType() const {
+ return IsEnumerationType() && !IsScopedEnumerationType();
+}
+
+bool CompilerType::IsIntegerOrUnscopedEnumerationType() const {
+ return IsInteger() || IsUnscopedEnumerationType();
+}
+
+bool CompilerType::IsSigned() const {
+ if (IsEnumerationType()) {
+return IsEnumerationIntegerTypeSigned();
+ }
+ return GetTypeInfo() & lldb::eTypeIsSigned;
+}
+
+bool CompilerType::IsNullPtrType() const {
+ return GetCanonicalType().GetBasicTypeEnumeration() ==
+ lldb::eBasicTypeNullPtr;
+}
+
+bool CompilerType::IsBoolean() const {
+ return GetCanonicalType().GetBasicTypeEnumeration() == lldb::eBasicTypeBool;
+}
+
+bool CompilerType::IsEnumerationIntegerTypeSigned() const {
+ if (IsValid()) {
+return GetEnumerationIntegerType().GetTypeInfo() & lldb::eTypeIsSigned;
+ }
+ return false;
+}
+
+bool CompilerType::IsScalarOrUnscopedEnumerationType() const {
+ return IsScalarType() || IsUnscopedEnumerationType();
+}
+
+bool CompilerType::IsPromotableIntegerType() const {
+ // Unscoped enums are always considered as promotable, even if their
+ // underlying type does not need to be promoted (e.g. "int").
+ if (IsUnscopedEnumerationType()) {
+return true;
+ }
+
+ switch (GetCanonicalType().GetBasicTypeEnumeration()) {
+ case lldb::eBasicTypeBool:
+ case lldb::eBasicTypeChar:
+ case lldb::eBasicTypeSignedChar:
+ case lldb::eBasicTypeUnsignedChar:
+ case lldb::eBasicTypeShort:
+ case lldb::eBasicTypeUnsignedShort:
+ case lldb::eBasicTypeWChar:
+ case lldb::eBasicTypeSignedWChar:
+ case lldb::eBasicTypeUnsignedWChar:
+ case lldb::eBasicTypeChar16:
+ case lldb::eBasicTypeChar32:
+return true;
+
+ default:
+return false;
+ }
+}
+
+bool CompilerType::IsPointerToVoid() const {
+ if (!IsValid())
+return false;
+
+ return IsPointerType() &&
+ GetPointeeType().GetBasicTypeEnumeration() == lldb::eBasicTypeVoid;
+}
+
+bool CompilerType::IsRecordType() const {
+ if (!IsValid())
+return false;
+
+ return GetCanonicalType().GetTypeClass() &
+ (lldb::eTypeClassClass | lldb::eTypeClassStruct |
+ lldb::eTypeClassUnion);
+}
+
+// Checks whether `target_base` is a virtual base of `type` (direct or
+// indirect). If it is, stores the first virtual base type on the path from
felipepiovezan wrote:
+1
https://github.com/llvm/llvm-project/pull/73472
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add more helper functions to CompilerType class (second try). (PR #73472)
@@ -302,6 +303,256 @@ bool CompilerType::IsBeingDefined() const {
return false;
}
+bool CompilerType::IsSmartPtrType() const {
+ // These regular expressions cover shared, unique and weak pointers both from
+ // stdlibc++ and libc+++.
+
+ static llvm::Regex k_libcxx_std_unique_ptr_regex(
+ "^std::__[[:alnum:]]+::unique_ptr<.+>(( )?&)?$");
+ static llvm::Regex k_libcxx_std_shared_ptr_regex(
+ "^std::__[[:alnum:]]+::shared_ptr<.+>(( )?&)?$");
+ static llvm::Regex k_libcxx_std_weak_ptr_regex(
+ "^std::__[[:alnum:]]+::weak_ptr<.+>(( )?&)?$");
+ //
+ static llvm::Regex k_libcxx_std_unique_ptr_regex_2(
+ "^std::unique_ptr<.+>(( )?&)?$");
+ static llvm::Regex k_libcxx_std_shared_ptr_regex_2(
+ "^std::shared_ptr<.+>(( )?&)?$");
+ static llvm::Regex k_libcxx_std_weak_ptr_regex_2(
felipepiovezan wrote:
Let's make these `const`s
https://github.com/llvm/llvm-project/pull/73472
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add more helper functions to CompilerType class (second try). (PR #73472)
@@ -302,6 +303,256 @@ bool CompilerType::IsBeingDefined() const {
return false;
}
+bool CompilerType::IsSmartPtrType() const {
+ // These regular expressions cover shared, unique and weak pointers both from
+ // stdlibc++ and libc+++.
+
+ static llvm::Regex k_libcxx_std_unique_ptr_regex(
+ "^std::__[[:alnum:]]+::unique_ptr<.+>(( )?&)?$");
+ static llvm::Regex k_libcxx_std_shared_ptr_regex(
+ "^std::__[[:alnum:]]+::shared_ptr<.+>(( )?&)?$");
+ static llvm::Regex k_libcxx_std_weak_ptr_regex(
+ "^std::__[[:alnum:]]+::weak_ptr<.+>(( )?&)?$");
+ //
+ static llvm::Regex k_libcxx_std_unique_ptr_regex_2(
+ "^std::unique_ptr<.+>(( )?&)?$");
+ static llvm::Regex k_libcxx_std_shared_ptr_regex_2(
+ "^std::shared_ptr<.+>(( )?&)?$");
+ static llvm::Regex k_libcxx_std_weak_ptr_regex_2(
+ "^std::weak_ptr<.+>(( )?&)?$");
+ //
+ llvm::StringRef name = GetTypeName();
+ return k_libcxx_std_unique_ptr_regex.match(name) ||
+ k_libcxx_std_shared_ptr_regex.match(name) ||
+ k_libcxx_std_weak_ptr_regex.match(name) ||
+ k_libcxx_std_unique_ptr_regex_2.match(name) ||
+ k_libcxx_std_shared_ptr_regex_2.match(name) ||
+ k_libcxx_std_weak_ptr_regex_2.match(name);
+}
+
+bool CompilerType::IsInteger() const {
+ // This is used when you don't care about the signedness of the integer.
+ bool is_signed;
felipepiovezan wrote:
+1
https://github.com/llvm/llvm-project/pull/73472
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add more helper functions to CompilerType class (second try). (PR #73472)
@@ -54,7 +54,7 @@ bool CompilerType::IsArrayType(CompilerType *element_type_ptr, uint64_t *size, if (IsValid()) if (auto type_system_sp = GetTypeSystem()) return type_system_sp->IsArrayType(m_type, element_type_ptr, size, - is_incomplete); + is_incomplete); felipepiovezan wrote: same issue with formatting as mentioned before https://github.com/llvm/llvm-project/pull/73472 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add more helper functions to CompilerType class (second try). (PR #73472)
@@ -302,6 +303,256 @@ bool CompilerType::IsBeingDefined() const {
return false;
}
+bool CompilerType::IsSmartPtrType() const {
+ // These regular expressions cover shared, unique and weak pointers both from
+ // stdlibc++ and libc+++.
+
+ static llvm::Regex k_libcxx_std_unique_ptr_regex(
+ "^std::__[[:alnum:]]+::unique_ptr<.+>(( )?&)?$");
+ static llvm::Regex k_libcxx_std_shared_ptr_regex(
+ "^std::__[[:alnum:]]+::shared_ptr<.+>(( )?&)?$");
+ static llvm::Regex k_libcxx_std_weak_ptr_regex(
+ "^std::__[[:alnum:]]+::weak_ptr<.+>(( )?&)?$");
+ //
+ static llvm::Regex k_libcxx_std_unique_ptr_regex_2(
+ "^std::unique_ptr<.+>(( )?&)?$");
+ static llvm::Regex k_libcxx_std_shared_ptr_regex_2(
+ "^std::shared_ptr<.+>(( )?&)?$");
+ static llvm::Regex k_libcxx_std_weak_ptr_regex_2(
+ "^std::weak_ptr<.+>(( )?&)?$");
+ //
+ llvm::StringRef name = GetTypeName();
+ return k_libcxx_std_unique_ptr_regex.match(name) ||
+ k_libcxx_std_shared_ptr_regex.match(name) ||
+ k_libcxx_std_weak_ptr_regex.match(name) ||
+ k_libcxx_std_unique_ptr_regex_2.match(name) ||
+ k_libcxx_std_shared_ptr_regex_2.match(name) ||
+ k_libcxx_std_weak_ptr_regex_2.match(name);
+}
+
+bool CompilerType::IsInteger() const {
+ // This is used when you don't care about the signedness of the integer.
+ bool is_signed;
+ return IsIntegerType(is_signed);
+}
+
+bool CompilerType::IsFloat() const {
+ uint32_t count = 0;
+ bool is_complex = false;
+ return IsFloatingPointType(count, is_complex);
+}
+
+bool CompilerType::IsEnumerationType() const {
+ // This is used when you don't care about the signedness of the enum.
+ bool is_signed;
+ return IsEnumerationType(is_signed);
+}
+
+bool CompilerType::IsUnscopedEnumerationType() const {
+ return IsEnumerationType() && !IsScopedEnumerationType();
+}
+
+bool CompilerType::IsIntegerOrUnscopedEnumerationType() const {
+ return IsInteger() || IsUnscopedEnumerationType();
+}
+
+bool CompilerType::IsSigned() const {
+ if (IsEnumerationType()) {
+return IsEnumerationIntegerTypeSigned();
+ }
+ return GetTypeInfo() & lldb::eTypeIsSigned;
+}
+
+bool CompilerType::IsNullPtrType() const {
+ return GetCanonicalType().GetBasicTypeEnumeration() ==
+ lldb::eBasicTypeNullPtr;
+}
+
+bool CompilerType::IsBoolean() const {
+ return GetCanonicalType().GetBasicTypeEnumeration() == lldb::eBasicTypeBool;
+}
+
+bool CompilerType::IsEnumerationIntegerTypeSigned() const {
+ if (IsValid()) {
+return GetEnumerationIntegerType().GetTypeInfo() & lldb::eTypeIsSigned;
+ }
+ return false;
+}
+
+bool CompilerType::IsScalarOrUnscopedEnumerationType() const {
+ return IsScalarType() || IsUnscopedEnumerationType();
+}
+
+bool CompilerType::IsPromotableIntegerType() const {
+ // Unscoped enums are always considered as promotable, even if their
+ // underlying type does not need to be promoted (e.g. "int").
+ if (IsUnscopedEnumerationType()) {
+return true;
+ }
+
+ switch (GetCanonicalType().GetBasicTypeEnumeration()) {
+ case lldb::eBasicTypeBool:
+ case lldb::eBasicTypeChar:
+ case lldb::eBasicTypeSignedChar:
+ case lldb::eBasicTypeUnsignedChar:
+ case lldb::eBasicTypeShort:
+ case lldb::eBasicTypeUnsignedShort:
+ case lldb::eBasicTypeWChar:
+ case lldb::eBasicTypeSignedWChar:
+ case lldb::eBasicTypeUnsignedWChar:
+ case lldb::eBasicTypeChar16:
+ case lldb::eBasicTypeChar32:
+return true;
+
+ default:
+return false;
+ }
felipepiovezan wrote:
Rationale:
https://llvm.org/docs/CodingStandards.html#don-t-use-default-labels-in-fully-covered-switches-over-enumerations
https://github.com/llvm/llvm-project/pull/73472
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add more helper functions to CompilerType class (second try). (PR #73472)
@@ -302,6 +303,256 @@ bool CompilerType::IsBeingDefined() const {
return false;
}
+bool CompilerType::IsSmartPtrType() const {
+ // These regular expressions cover shared, unique and weak pointers both from
+ // stdlibc++ and libc+++.
+
+ static llvm::Regex k_libcxx_std_unique_ptr_regex(
+ "^std::__[[:alnum:]]+::unique_ptr<.+>(( )?&)?$");
+ static llvm::Regex k_libcxx_std_shared_ptr_regex(
+ "^std::__[[:alnum:]]+::shared_ptr<.+>(( )?&)?$");
+ static llvm::Regex k_libcxx_std_weak_ptr_regex(
+ "^std::__[[:alnum:]]+::weak_ptr<.+>(( )?&)?$");
+ //
+ static llvm::Regex k_libcxx_std_unique_ptr_regex_2(
+ "^std::unique_ptr<.+>(( )?&)?$");
+ static llvm::Regex k_libcxx_std_shared_ptr_regex_2(
+ "^std::shared_ptr<.+>(( )?&)?$");
+ static llvm::Regex k_libcxx_std_weak_ptr_regex_2(
+ "^std::weak_ptr<.+>(( )?&)?$");
+ //
+ llvm::StringRef name = GetTypeName();
+ return k_libcxx_std_unique_ptr_regex.match(name) ||
+ k_libcxx_std_shared_ptr_regex.match(name) ||
+ k_libcxx_std_weak_ptr_regex.match(name) ||
+ k_libcxx_std_unique_ptr_regex_2.match(name) ||
+ k_libcxx_std_shared_ptr_regex_2.match(name) ||
+ k_libcxx_std_weak_ptr_regex_2.match(name);
+}
+
+bool CompilerType::IsInteger() const {
+ // This is used when you don't care about the signedness of the integer.
+ bool is_signed;
+ return IsIntegerType(is_signed);
+}
+
+bool CompilerType::IsFloat() const {
+ uint32_t count = 0;
+ bool is_complex = false;
+ return IsFloatingPointType(count, is_complex);
+}
+
+bool CompilerType::IsEnumerationType() const {
+ // This is used when you don't care about the signedness of the enum.
+ bool is_signed;
+ return IsEnumerationType(is_signed);
+}
+
+bool CompilerType::IsUnscopedEnumerationType() const {
+ return IsEnumerationType() && !IsScopedEnumerationType();
+}
+
+bool CompilerType::IsIntegerOrUnscopedEnumerationType() const {
+ return IsInteger() || IsUnscopedEnumerationType();
+}
+
+bool CompilerType::IsSigned() const {
+ if (IsEnumerationType()) {
+return IsEnumerationIntegerTypeSigned();
+ }
+ return GetTypeInfo() & lldb::eTypeIsSigned;
+}
+
+bool CompilerType::IsNullPtrType() const {
+ return GetCanonicalType().GetBasicTypeEnumeration() ==
+ lldb::eBasicTypeNullPtr;
+}
+
+bool CompilerType::IsBoolean() const {
+ return GetCanonicalType().GetBasicTypeEnumeration() == lldb::eBasicTypeBool;
+}
+
+bool CompilerType::IsEnumerationIntegerTypeSigned() const {
+ if (IsValid()) {
+return GetEnumerationIntegerType().GetTypeInfo() & lldb::eTypeIsSigned;
+ }
+ return false;
+}
+
+bool CompilerType::IsScalarOrUnscopedEnumerationType() const {
+ return IsScalarType() || IsUnscopedEnumerationType();
+}
+
+bool CompilerType::IsPromotableIntegerType() const {
+ // Unscoped enums are always considered as promotable, even if their
+ // underlying type does not need to be promoted (e.g. "int").
+ if (IsUnscopedEnumerationType()) {
+return true;
+ }
+
+ switch (GetCanonicalType().GetBasicTypeEnumeration()) {
+ case lldb::eBasicTypeBool:
+ case lldb::eBasicTypeChar:
+ case lldb::eBasicTypeSignedChar:
+ case lldb::eBasicTypeUnsignedChar:
+ case lldb::eBasicTypeShort:
+ case lldb::eBasicTypeUnsignedShort:
+ case lldb::eBasicTypeWChar:
+ case lldb::eBasicTypeSignedWChar:
+ case lldb::eBasicTypeUnsignedWChar:
+ case lldb::eBasicTypeChar16:
+ case lldb::eBasicTypeChar32:
+return true;
+
+ default:
+return false;
+ }
+}
+
+bool CompilerType::IsPointerToVoid() const {
+ if (!IsValid())
+return false;
+
+ return IsPointerType() &&
+ GetPointeeType().GetBasicTypeEnumeration() == lldb::eBasicTypeVoid;
+}
+
+bool CompilerType::IsRecordType() const {
+ if (!IsValid())
+return false;
+
+ return GetCanonicalType().GetTypeClass() &
+ (lldb::eTypeClassClass | lldb::eTypeClassStruct |
+ lldb::eTypeClassUnion);
+}
+
+// Checks whether `target_base` is a virtual base of `type` (direct or
+// indirect). If it is, stores the first virtual base type on the path from
+// `type` to `target_type`.
+bool CompilerType::IsVirtualBase(CompilerType target_base,
+ CompilerType *virtual_base,
+ bool carry_virtual) const {
+ if (CompareTypes(target_base)) {
+return carry_virtual;
+ }
felipepiovezan wrote:
There are many other instances of this in the code below
https://github.com/llvm/llvm-project/pull/73472
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add more helper functions to CompilerType class (second try). (PR #73472)
@@ -302,6 +303,256 @@ bool CompilerType::IsBeingDefined() const {
return false;
}
+bool CompilerType::IsSmartPtrType() const {
+ // These regular expressions cover shared, unique and weak pointers both from
+ // stdlibc++ and libc+++.
+
+ static llvm::Regex k_libcxx_std_unique_ptr_regex(
+ "^std::__[[:alnum:]]+::unique_ptr<.+>(( )?&)?$");
+ static llvm::Regex k_libcxx_std_shared_ptr_regex(
+ "^std::__[[:alnum:]]+::shared_ptr<.+>(( )?&)?$");
+ static llvm::Regex k_libcxx_std_weak_ptr_regex(
+ "^std::__[[:alnum:]]+::weak_ptr<.+>(( )?&)?$");
+ //
+ static llvm::Regex k_libcxx_std_unique_ptr_regex_2(
+ "^std::unique_ptr<.+>(( )?&)?$");
+ static llvm::Regex k_libcxx_std_shared_ptr_regex_2(
+ "^std::shared_ptr<.+>(( )?&)?$");
+ static llvm::Regex k_libcxx_std_weak_ptr_regex_2(
+ "^std::weak_ptr<.+>(( )?&)?$");
+ //
+ llvm::StringRef name = GetTypeName();
+ return k_libcxx_std_unique_ptr_regex.match(name) ||
+ k_libcxx_std_shared_ptr_regex.match(name) ||
+ k_libcxx_std_weak_ptr_regex.match(name) ||
+ k_libcxx_std_unique_ptr_regex_2.match(name) ||
+ k_libcxx_std_shared_ptr_regex_2.match(name) ||
+ k_libcxx_std_weak_ptr_regex_2.match(name);
+}
+
+bool CompilerType::IsInteger() const {
+ // This is used when you don't care about the signedness of the integer.
+ bool is_signed;
+ return IsIntegerType(is_signed);
+}
+
+bool CompilerType::IsFloat() const {
+ uint32_t count = 0;
+ bool is_complex = false;
+ return IsFloatingPointType(count, is_complex);
+}
+
+bool CompilerType::IsEnumerationType() const {
+ // This is used when you don't care about the signedness of the enum.
+ bool is_signed;
+ return IsEnumerationType(is_signed);
+}
+
+bool CompilerType::IsUnscopedEnumerationType() const {
+ return IsEnumerationType() && !IsScopedEnumerationType();
+}
+
+bool CompilerType::IsIntegerOrUnscopedEnumerationType() const {
+ return IsInteger() || IsUnscopedEnumerationType();
+}
+
+bool CompilerType::IsSigned() const {
+ if (IsEnumerationType()) {
+return IsEnumerationIntegerTypeSigned();
+ }
+ return GetTypeInfo() & lldb::eTypeIsSigned;
+}
+
+bool CompilerType::IsNullPtrType() const {
+ return GetCanonicalType().GetBasicTypeEnumeration() ==
+ lldb::eBasicTypeNullPtr;
+}
+
+bool CompilerType::IsBoolean() const {
+ return GetCanonicalType().GetBasicTypeEnumeration() == lldb::eBasicTypeBool;
+}
+
+bool CompilerType::IsEnumerationIntegerTypeSigned() const {
+ if (IsValid()) {
+return GetEnumerationIntegerType().GetTypeInfo() & lldb::eTypeIsSigned;
+ }
+ return false;
+}
+
+bool CompilerType::IsScalarOrUnscopedEnumerationType() const {
+ return IsScalarType() || IsUnscopedEnumerationType();
+}
+
+bool CompilerType::IsPromotableIntegerType() const {
+ // Unscoped enums are always considered as promotable, even if their
+ // underlying type does not need to be promoted (e.g. "int").
+ if (IsUnscopedEnumerationType()) {
+return true;
+ }
+
+ switch (GetCanonicalType().GetBasicTypeEnumeration()) {
+ case lldb::eBasicTypeBool:
+ case lldb::eBasicTypeChar:
+ case lldb::eBasicTypeSignedChar:
+ case lldb::eBasicTypeUnsignedChar:
+ case lldb::eBasicTypeShort:
+ case lldb::eBasicTypeUnsignedShort:
+ case lldb::eBasicTypeWChar:
+ case lldb::eBasicTypeSignedWChar:
+ case lldb::eBasicTypeUnsignedWChar:
+ case lldb::eBasicTypeChar16:
+ case lldb::eBasicTypeChar32:
+return true;
+
+ default:
+return false;
+ }
+}
+
+bool CompilerType::IsPointerToVoid() const {
+ if (!IsValid())
+return false;
+
+ return IsPointerType() &&
+ GetPointeeType().GetBasicTypeEnumeration() == lldb::eBasicTypeVoid;
+}
+
+bool CompilerType::IsRecordType() const {
+ if (!IsValid())
+return false;
+
+ return GetCanonicalType().GetTypeClass() &
+ (lldb::eTypeClassClass | lldb::eTypeClassStruct |
+ lldb::eTypeClassUnion);
+}
+
+// Checks whether `target_base` is a virtual base of `type` (direct or
+// indirect). If it is, stores the first virtual base type on the path from
felipepiovezan wrote:
This comment should also explain what `virtual_base` and `carry_virtual` are
https://github.com/llvm/llvm-project/pull/73472
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add more helper functions to CompilerType class (second try). (PR #73472)
@@ -157,7 +157,8 @@ bool CompilerType::IsBlockPointerType(
CompilerType *function_pointer_type_ptr) const {
if (IsValid())
if (auto type_system_sp = GetTypeSystem())
- return type_system_sp->IsBlockPointerType(m_type,
function_pointer_type_ptr);
+ return type_system_sp->IsBlockPointerType(m_type,
felipepiovezan wrote:
same issue with formatting as mentioned before
https://github.com/llvm/llvm-project/pull/73472
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] TestConstStaticIntegralMember: relax assertion on number of global variables (PR #73707)
https://github.com/felipepiovezan closed https://github.com/llvm/llvm-project/pull/73707 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add more helper functions to CompilerType class (second try). (PR #73472)
@@ -436,8 +482,8 @@ class CompilerType {
ExecutionContextScope *exe_scope);
/// Dump to stdout.
- void DumpTypeDescription(lldb::DescriptionLevel level =
- lldb::eDescriptionLevelFull) const;
felipepiovezan wrote:
I could be wrong here, but AFAICT the bot should _only_ check for formatting
changes inside the diff of the commit, not inside the entire file. If this is
not what the bot is doing, it is something we can probably improve upon.
In the meantime though, my suggestion is to manually inspect the bot output: if
it is complaining about lines not touched by this commit, just ignore that.
This is favorable to editing lines that are not related to the patch
https://github.com/llvm/llvm-project/pull/73472
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add more helper functions to CompilerType class (second try). (PR #73472)
@@ -436,8 +482,8 @@ class CompilerType {
ExecutionContextScope *exe_scope);
/// Dump to stdout.
- void DumpTypeDescription(lldb::DescriptionLevel level =
- lldb::eDescriptionLevelFull) const;
felipepiovezan wrote:
I see this comment in [another
PR](https://github.com/llvm/llvm-project/pull/73252#issuecomment-1832242278)
> NB: not applying all the clang-format recommendations as it affects lines I'm
> not editing, and I don't want to pollute git-blame
That makes me think the bot is not running clang-format-diff?
https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting
https://github.com/llvm/llvm-project/pull/73472
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add option to pass thread ID to thread select command (PR #73596)
@@ -807,6 +808,23 @@ void
CommandCompletions::TypeCategoryNames(CommandInterpreter &interpreter,
});
}
+void CommandCompletions::ThreadIDs(CommandInterpreter &interpreter,
+ CompletionRequest &request,
+ SearchFilter *searcher) {
+ const ExecutionContext &exe_ctx = interpreter.GetExecutionContext();
+ if (!exe_ctx.HasProcessScope())
+return;
+
+ ThreadList &threads = exe_ctx.GetProcessPtr()->GetThreadList();
+ lldb::ThreadSP thread_sp;
+ for (uint32_t idx = 0; (thread_sp = threads.GetThreadAtIndex(idx)); ++idx) {
felipepiovezan wrote:
minor nit, don't block the review on this, but why do you have ()s here?
https://github.com/llvm/llvm-project/pull/73596
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add more helper functions to CompilerType class (second try). (PR #73472)
@@ -436,8 +482,8 @@ class CompilerType {
ExecutionContextScope *exe_scope);
/// Dump to stdout.
- void DumpTypeDescription(lldb::DescriptionLevel level =
- lldb::eDescriptionLevelFull) const;
felipepiovezan wrote:
Yes, please :)
https://github.com/llvm/llvm-project/pull/73472
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add option to pass thread ID to thread select command (PR #73596)
https://github.com/felipepiovezan edited https://github.com/llvm/llvm-project/pull/73596 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Delete unreachable code and unncesary string conversions (PR #74119)
https://github.com/felipepiovezan created
https://github.com/llvm/llvm-project/pull/74119
This PR cleans up OptionArgParser in a couple of ways:
1. We remove unnecessary std::string temporaries
2. Through else-after-return elimination, we prove the existence of unreachable
code
>From 6f502ee61bae59a16f90f30357eb72698ad577d0 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan
Date: Fri, 1 Dec 2023 09:07:11 -0800
Subject: [PATCH 1/3] [lldb][NFC] Remove unnecessary std::string temporaries
The existing code was taking three substrings from a regex match and converting
to std::strings prior to using them. This may have been done to address
null-termination concerns, but this is not the case:
1. `name` was being used to call `c_str()` and then implicitly converted back to
a `StringRef` on the call to `ToAddress`. While the path `const char *` ->
`StringRef` requires null-termination, we can simply use the original StringRef.
2. `str_offset` was being converted back to a StringRef in order to call a
member method. Member methods can't handle non-null termination.
3. `sign` simply had it's 0-th element accessed.
---
lldb/source/Interpreter/OptionArgParser.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lldb/source/Interpreter/OptionArgParser.cpp
b/lldb/source/Interpreter/OptionArgParser.cpp
index ba2d3416e1838a9..9e53261ac885436 100644
--- a/lldb/source/Interpreter/OptionArgParser.cpp
+++ b/lldb/source/Interpreter/OptionArgParser.cpp
@@ -243,9 +243,9 @@ OptionArgParser::DoToAddress(const ExecutionContext
*exe_ctx, llvm::StringRef s,
llvm::SmallVector matches;
if (g_symbol_plus_offset_regex.Execute(sref, &matches)) {
uint64_t offset = 0;
- std::string name = matches[1].str();
- std::string sign = matches[2].str();
- std::string str_offset = matches[3].str();
+ llvm::StringRef name = matches[1];
+ llvm::StringRef sign = matches[2];
+ llvm::StringRef str_offset = matches[3];
if (!llvm::StringRef(str_offset).getAsInteger(0, offset)) {
Status error;
addr = ToAddress(exe_ctx, name.c_str(), LLDB_INVALID_ADDRESS, &error);
>From f7b4bece1f40ffbb22cffb1ef25d56fb34bfedb2 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan
Date: Fri, 1 Dec 2023 09:44:28 -0800
Subject: [PATCH 2/3] [lldb][NFC] Remove else after return in OptionArgParse
This will enable us to prove that there is unreachable code in this function in
a subsequent commit.
---
lldb/source/Interpreter/OptionArgParser.cpp | 21 +
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/lldb/source/Interpreter/OptionArgParser.cpp
b/lldb/source/Interpreter/OptionArgParser.cpp
index 9e53261ac885436..8698f1de224de20 100644
--- a/lldb/source/Interpreter/OptionArgParser.cpp
+++ b/lldb/source/Interpreter/OptionArgParser.cpp
@@ -168,7 +168,6 @@ lldb::addr_t OptionArgParser::ToAddress(const
ExecutionContext *exe_ctx,
std::optional
OptionArgParser::DoToAddress(const ExecutionContext *exe_ctx, llvm::StringRef
s,
Status *error_ptr) {
- bool error_set = false;
if (s.empty()) {
if (error_ptr)
error_ptr->SetErrorStringWithFormat("invalid address expression \"%s\"",
@@ -212,6 +211,7 @@ OptionArgParser::DoToAddress(const ExecutionContext
*exe_ctx, llvm::StringRef s,
target->EvaluateExpression(s, exe_ctx->GetFramePtr(), valobj_sp,
options);
bool success = false;
+ bool error_set = false;
if (expr_result == eExpressionCompleted) {
if (valobj_sp)
valobj_sp = valobj_sp->GetQualifiedRepresentationIfAvailable(
@@ -223,16 +223,14 @@ OptionArgParser::DoToAddress(const ExecutionContext
*exe_ctx, llvm::StringRef s,
if (error_ptr)
error_ptr->Clear();
return addr;
-} else {
- if (error_ptr) {
-error_set = true;
-error_ptr->SetErrorStringWithFormat(
-"address expression \"%s\" resulted in a value whose type "
-"can't be converted to an address: %s",
-s.str().c_str(), valobj_sp->GetTypeName().GetCString());
- }
}
-
+if (error_ptr) {
+ error_set = true;
+ error_ptr->SetErrorStringWithFormat(
+ "address expression \"%s\" resulted in a value whose type "
+ "can't be converted to an address: %s",
+ s.str().c_str(), valobj_sp->GetTypeName().GetCString());
+}
} else {
// Since the compiler can't handle things like "main + 12" we should try to
// do this for now. The compiler doesn't like adding offsets to function
@@ -252,8 +250,7 @@ OptionArgParser::DoToAddress(const ExecutionContext
*exe_ctx, llvm::StringRef s,
if (addr != LLDB_INVALID_ADDRESS) {
if (sign[0] == '+')
return addr + offset;
- else
-return addr - offset;
+ return addr - offset;
}
}
}
>From 25decf3ecd15f3782ffe8e5666423d8a750ff14d Mon Sep 17 00:00:00 2001
From: Felipe de Az
[Lldb-commits] [lldb] [lldb] Delete unreachable code and unnecessary string conversions (PR #74119)
https://github.com/felipepiovezan edited https://github.com/llvm/llvm-project/pull/74119 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Delete unreachable code and unnecessary string conversions (PR #74119)
https://github.com/felipepiovezan updated
https://github.com/llvm/llvm-project/pull/74119
>From d4143eea927eed4f4f0efaa9e1657de5f452a257 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan
Date: Fri, 1 Dec 2023 09:07:11 -0800
Subject: [PATCH 1/3] [lldb][NFC] Remove unnecessary std::string temporaries
The existing code was taking three substrings from a regex match and converting
to std::strings prior to using them. This may have been done to address
null-termination concerns, but this is not the case:
1. `name` was being used to call `c_str()` and then implicitly converted back to
a `StringRef` on the call to `ToAddress`. While the path `const char *` ->
`StringRef` requires null-termination, we can simply use the original StringRef.
2. `str_offset` was being converted back to a StringRef in order to call a
member method. Member methods can't handle non-null termination.
3. `sign` simply had it's 0-th element accessed.
---
lldb/source/Interpreter/OptionArgParser.cpp | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/lldb/source/Interpreter/OptionArgParser.cpp
b/lldb/source/Interpreter/OptionArgParser.cpp
index ba2d3416e1838a9..933bc6514ca2418 100644
--- a/lldb/source/Interpreter/OptionArgParser.cpp
+++ b/lldb/source/Interpreter/OptionArgParser.cpp
@@ -243,12 +243,12 @@ OptionArgParser::DoToAddress(const ExecutionContext
*exe_ctx, llvm::StringRef s,
llvm::SmallVector matches;
if (g_symbol_plus_offset_regex.Execute(sref, &matches)) {
uint64_t offset = 0;
- std::string name = matches[1].str();
- std::string sign = matches[2].str();
- std::string str_offset = matches[3].str();
- if (!llvm::StringRef(str_offset).getAsInteger(0, offset)) {
+ llvm::StringRef name = matches[1];
+ llvm::StringRef sign = matches[2];
+ llvm::StringRef str_offset = matches[3];
+ if (!str_offset.getAsInteger(0, offset)) {
Status error;
-addr = ToAddress(exe_ctx, name.c_str(), LLDB_INVALID_ADDRESS, &error);
+addr = ToAddress(exe_ctx, name, LLDB_INVALID_ADDRESS, &error);
if (addr != LLDB_INVALID_ADDRESS) {
if (sign[0] == '+')
return addr + offset;
>From 58283e15fd2ac6ecb29d24a77b99e883b3a9831a Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan
Date: Fri, 1 Dec 2023 09:44:28 -0800
Subject: [PATCH 2/3] [lldb][NFC] Remove else after return in OptionArgParse
This will enable us to prove that there is unreachable code in this function in
a subsequent commit.
---
lldb/source/Interpreter/OptionArgParser.cpp | 21 +
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/lldb/source/Interpreter/OptionArgParser.cpp
b/lldb/source/Interpreter/OptionArgParser.cpp
index 933bc6514ca2418..c61072dfafc19f2 100644
--- a/lldb/source/Interpreter/OptionArgParser.cpp
+++ b/lldb/source/Interpreter/OptionArgParser.cpp
@@ -168,7 +168,6 @@ lldb::addr_t OptionArgParser::ToAddress(const
ExecutionContext *exe_ctx,
std::optional
OptionArgParser::DoToAddress(const ExecutionContext *exe_ctx, llvm::StringRef
s,
Status *error_ptr) {
- bool error_set = false;
if (s.empty()) {
if (error_ptr)
error_ptr->SetErrorStringWithFormat("invalid address expression \"%s\"",
@@ -212,6 +211,7 @@ OptionArgParser::DoToAddress(const ExecutionContext
*exe_ctx, llvm::StringRef s,
target->EvaluateExpression(s, exe_ctx->GetFramePtr(), valobj_sp,
options);
bool success = false;
+ bool error_set = false;
if (expr_result == eExpressionCompleted) {
if (valobj_sp)
valobj_sp = valobj_sp->GetQualifiedRepresentationIfAvailable(
@@ -223,16 +223,14 @@ OptionArgParser::DoToAddress(const ExecutionContext
*exe_ctx, llvm::StringRef s,
if (error_ptr)
error_ptr->Clear();
return addr;
-} else {
- if (error_ptr) {
-error_set = true;
-error_ptr->SetErrorStringWithFormat(
-"address expression \"%s\" resulted in a value whose type "
-"can't be converted to an address: %s",
-s.str().c_str(), valobj_sp->GetTypeName().GetCString());
- }
}
-
+if (error_ptr) {
+ error_set = true;
+ error_ptr->SetErrorStringWithFormat(
+ "address expression \"%s\" resulted in a value whose type "
+ "can't be converted to an address: %s",
+ s.str().c_str(), valobj_sp->GetTypeName().GetCString());
+}
} else {
// Since the compiler can't handle things like "main + 12" we should try to
// do this for now. The compiler doesn't like adding offsets to function
@@ -252,8 +250,7 @@ OptionArgParser::DoToAddress(const ExecutionContext
*exe_ctx, llvm::StringRef s,
if (addr != LLDB_INVALID_ADDRESS) {
if (sign[0] == '+')
return addr + offset;
- else
-return addr - offset;
+ return addr - offset;
}
}
}
>From d6a4756ca3d65382a532c59ff89ddc6d57fc
[Lldb-commits] [lldb] [lldb] Delete unreachable code and unnecessary string conversions (PR #74119)
felipepiovezan wrote: > Github makes this look extremely complicated but in my editor this all looks > pretty obvious. LGTM. Yeah, that's part of the reason I tried to break the changes into smaller bits, but it still doesn't look as obvious as I wish it did https://github.com/llvm/llvm-project/pull/74119 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 7a86cc6 - [lldb][NFC] Remove unnecessary std::string temporaries
Author: Felipe de Azevedo Piovezan
Date: 2023-12-04T10:23:04-08:00
New Revision: 7a86cc6c4ca11e37d5985d4fc902658ab6ad0e22
URL:
https://github.com/llvm/llvm-project/commit/7a86cc6c4ca11e37d5985d4fc902658ab6ad0e22
DIFF:
https://github.com/llvm/llvm-project/commit/7a86cc6c4ca11e37d5985d4fc902658ab6ad0e22.diff
LOG: [lldb][NFC] Remove unnecessary std::string temporaries
The existing code was taking three substrings from a regex match and converting
to std::strings prior to using them. This may have been done to address
null-termination concerns, but this is not the case:
1. `name` was being used to call `c_str()` and then implicitly converted back to
a `StringRef` on the call to `ToAddress`. While the path `const char *` ->
`StringRef` requires null-termination, we can simply use the original StringRef.
2. `str_offset` was being converted back to a StringRef in order to call a
member method. Member methods can't handle non-null termination.
3. `sign` simply had it's 0-th element accessed.
Added:
Modified:
lldb/source/Interpreter/OptionArgParser.cpp
Removed:
diff --git a/lldb/source/Interpreter/OptionArgParser.cpp
b/lldb/source/Interpreter/OptionArgParser.cpp
index ba2d3416e1838..933bc6514ca24 100644
--- a/lldb/source/Interpreter/OptionArgParser.cpp
+++ b/lldb/source/Interpreter/OptionArgParser.cpp
@@ -243,12 +243,12 @@ OptionArgParser::DoToAddress(const ExecutionContext
*exe_ctx, llvm::StringRef s,
llvm::SmallVector matches;
if (g_symbol_plus_offset_regex.Execute(sref, &matches)) {
uint64_t offset = 0;
- std::string name = matches[1].str();
- std::string sign = matches[2].str();
- std::string str_offset = matches[3].str();
- if (!llvm::StringRef(str_offset).getAsInteger(0, offset)) {
+ llvm::StringRef name = matches[1];
+ llvm::StringRef sign = matches[2];
+ llvm::StringRef str_offset = matches[3];
+ if (!str_offset.getAsInteger(0, offset)) {
Status error;
-addr = ToAddress(exe_ctx, name.c_str(), LLDB_INVALID_ADDRESS, &error);
+addr = ToAddress(exe_ctx, name, LLDB_INVALID_ADDRESS, &error);
if (addr != LLDB_INVALID_ADDRESS) {
if (sign[0] == '+')
return addr + offset;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 3e98a28 - [lldb][NFC] Remove else after return in OptionArgParse
Author: Felipe de Azevedo Piovezan
Date: 2023-12-04T10:23:05-08:00
New Revision: 3e98a285138a517fd918ec0ac8397dc56330d8e7
URL:
https://github.com/llvm/llvm-project/commit/3e98a285138a517fd918ec0ac8397dc56330d8e7
DIFF:
https://github.com/llvm/llvm-project/commit/3e98a285138a517fd918ec0ac8397dc56330d8e7.diff
LOG: [lldb][NFC] Remove else after return in OptionArgParse
This will enable us to prove that there is unreachable code in this function in
a subsequent commit.
Added:
Modified:
lldb/source/Interpreter/OptionArgParser.cpp
Removed:
diff --git a/lldb/source/Interpreter/OptionArgParser.cpp
b/lldb/source/Interpreter/OptionArgParser.cpp
index 933bc6514ca24..c61072dfafc19 100644
--- a/lldb/source/Interpreter/OptionArgParser.cpp
+++ b/lldb/source/Interpreter/OptionArgParser.cpp
@@ -168,7 +168,6 @@ lldb::addr_t OptionArgParser::ToAddress(const
ExecutionContext *exe_ctx,
std::optional
OptionArgParser::DoToAddress(const ExecutionContext *exe_ctx, llvm::StringRef
s,
Status *error_ptr) {
- bool error_set = false;
if (s.empty()) {
if (error_ptr)
error_ptr->SetErrorStringWithFormat("invalid address expression \"%s\"",
@@ -212,6 +211,7 @@ OptionArgParser::DoToAddress(const ExecutionContext
*exe_ctx, llvm::StringRef s,
target->EvaluateExpression(s, exe_ctx->GetFramePtr(), valobj_sp,
options);
bool success = false;
+ bool error_set = false;
if (expr_result == eExpressionCompleted) {
if (valobj_sp)
valobj_sp = valobj_sp->GetQualifiedRepresentationIfAvailable(
@@ -223,16 +223,14 @@ OptionArgParser::DoToAddress(const ExecutionContext
*exe_ctx, llvm::StringRef s,
if (error_ptr)
error_ptr->Clear();
return addr;
-} else {
- if (error_ptr) {
-error_set = true;
-error_ptr->SetErrorStringWithFormat(
-"address expression \"%s\" resulted in a value whose type "
-"can't be converted to an address: %s",
-s.str().c_str(), valobj_sp->GetTypeName().GetCString());
- }
}
-
+if (error_ptr) {
+ error_set = true;
+ error_ptr->SetErrorStringWithFormat(
+ "address expression \"%s\" resulted in a value whose type "
+ "can't be converted to an address: %s",
+ s.str().c_str(), valobj_sp->GetTypeName().GetCString());
+}
} else {
// Since the compiler can't handle things like "main + 12" we should try to
// do this for now. The compiler doesn't like adding offsets to function
@@ -252,8 +250,7 @@ OptionArgParser::DoToAddress(const ExecutionContext
*exe_ctx, llvm::StringRef s,
if (addr != LLDB_INVALID_ADDRESS) {
if (sign[0] == '+')
return addr + offset;
- else
-return addr - offset;
+ return addr - offset;
}
}
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] d24d7ed - [lldb][NFC] Delete unreachable code and dead variable in OptionArgParser
Author: Felipe de Azevedo Piovezan
Date: 2023-12-04T10:23:05-08:00
New Revision: d24d7edaef9517edd9eb2ab26b02969e201bbcad
URL:
https://github.com/llvm/llvm-project/commit/d24d7edaef9517edd9eb2ab26b02969e201bbcad
DIFF:
https://github.com/llvm/llvm-project/commit/d24d7edaef9517edd9eb2ab26b02969e201bbcad.diff
LOG: [lldb][NFC] Delete unreachable code and dead variable in OptionArgParser
With the combination of an early return and removing an else-after-return, it
becomes evident that there is unreachable code in the function being changed.
Added:
Modified:
lldb/source/Interpreter/OptionArgParser.cpp
Removed:
diff --git a/lldb/source/Interpreter/OptionArgParser.cpp
b/lldb/source/Interpreter/OptionArgParser.cpp
index c61072dfafc19..8a92c7d08c476 100644
--- a/lldb/source/Interpreter/OptionArgParser.cpp
+++ b/lldb/source/Interpreter/OptionArgParser.cpp
@@ -211,7 +211,6 @@ OptionArgParser::DoToAddress(const ExecutionContext
*exe_ctx, llvm::StringRef s,
target->EvaluateExpression(s, exe_ctx->GetFramePtr(), valobj_sp,
options);
bool success = false;
- bool error_set = false;
if (expr_result == eExpressionCompleted) {
if (valobj_sp)
valobj_sp = valobj_sp->GetQualifiedRepresentationIfAvailable(
@@ -224,48 +223,39 @@ OptionArgParser::DoToAddress(const ExecutionContext
*exe_ctx, llvm::StringRef s,
error_ptr->Clear();
return addr;
}
-if (error_ptr) {
- error_set = true;
+if (error_ptr)
error_ptr->SetErrorStringWithFormat(
"address expression \"%s\" resulted in a value whose type "
"can't be converted to an address: %s",
s.str().c_str(), valobj_sp->GetTypeName().GetCString());
-}
- } else {
-// Since the compiler can't handle things like "main + 12" we should try to
-// do this for now. The compiler doesn't like adding offsets to function
-// pointer types.
-static RegularExpression g_symbol_plus_offset_regex(
-"^(.*)([-\\+])[[:space:]]*(0x[0-9A-Fa-f]+|[0-9]+)[[:space:]]*$");
-
-llvm::SmallVector matches;
-if (g_symbol_plus_offset_regex.Execute(sref, &matches)) {
- uint64_t offset = 0;
- llvm::StringRef name = matches[1];
- llvm::StringRef sign = matches[2];
- llvm::StringRef str_offset = matches[3];
- if (!str_offset.getAsInteger(0, offset)) {
-Status error;
-addr = ToAddress(exe_ctx, name, LLDB_INVALID_ADDRESS, &error);
-if (addr != LLDB_INVALID_ADDRESS) {
- if (sign[0] == '+')
-return addr + offset;
- return addr - offset;
-}
- }
-}
+return {};
+ }
-if (error_ptr) {
- error_set = true;
- error_ptr->SetErrorStringWithFormat(
- "address expression \"%s\" evaluation failed", s.str().c_str());
+ // Since the compiler can't handle things like "main + 12" we should try to
+ // do this for now. The compiler doesn't like adding offsets to function
+ // pointer types.
+ static RegularExpression g_symbol_plus_offset_regex(
+ "^(.*)([-\\+])[[:space:]]*(0x[0-9A-Fa-f]+|[0-9]+)[[:space:]]*$");
+
+ llvm::SmallVector matches;
+ if (g_symbol_plus_offset_regex.Execute(sref, &matches)) {
+uint64_t offset = 0;
+llvm::StringRef name = matches[1];
+llvm::StringRef sign = matches[2];
+llvm::StringRef str_offset = matches[3];
+if (!str_offset.getAsInteger(0, offset)) {
+ Status error;
+ addr = ToAddress(exe_ctx, name, LLDB_INVALID_ADDRESS, &error);
+ if (addr != LLDB_INVALID_ADDRESS) {
+if (sign[0] == '+')
+ return addr + offset;
+return addr - offset;
+ }
}
}
- if (error_ptr) {
-if (!error_set)
- error_ptr->SetErrorStringWithFormat("invalid address expression \"%s\"",
- s.str().c_str());
- }
+ if (error_ptr)
+error_ptr->SetErrorStringWithFormat(
+"address expression \"%s\" evaluation failed", s.str().c_str());
return {};
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Delete unreachable code and unnecessary string conversions (PR #74119)
https://github.com/felipepiovezan closed https://github.com/llvm/llvm-project/pull/74119 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Delete unreachable code and unnecessary string conversions (PR #74119)
felipepiovezan wrote: Pushed as ``` * d24d7edaef95 (2 minu..) fpiove.. [lldb][NFC] Delete unreachable code and dead │ variable in OptionArgParser │ * 3e98a285138a (2 minu..) fpiove.. [lldb][NFC] Remove else after return in OptionArgParse │ * 7a86cc6c4ca1 (2 minu..) fpiove.. [lldb][NFC] Remove unnecessary std::string temporaries ``` https://github.com/llvm/llvm-project/pull/74119 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Correctly check and report error states in StackFrame.cpp (PR #74414)
felipepiovezan wrote: It is wild that all of these have gone unnoticed for such a long time. I guess it makes sense, as these are more of sanity checks that we never expect to fire? https://github.com/llvm/llvm-project/pull/74414 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Return index of element in ValueObject path instead of the element's value (PR #74413)
felipepiovezan wrote: Should we change the documentation of these functions? https://github.com/llvm/llvm-project/pull/74413 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
