[Lldb-commits] [lldb] [lldb][DWARFASTParser][NFC] Factor out CXX/ObjC method specifics out of ParseSubroutine (PR #95078)
https://github.com/Michael137 closed 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] 1efd5c2 - [lldb][DWARFASTParser][NFC] Factor out CXX/ObjC method specifics out of ParseSubroutine (#95078)
Author: Michael Buch
Date: 2024-06-12T09:33:35+01:00
New Revision: 1efd5c22893e4a186453f6aaf44fee747f1d63bf
URL:
https://github.com/llvm/llvm-project/commit/1efd5c22893e4a186453f6aaf44fee747f1d63bf
DIFF:
https://github.com/llvm/llvm-project/commit/1efd5c22893e4a186453f6aaf44fee747f1d63bf.diff
LOG: [lldb][DWARFASTParser][NFC] Factor out CXX/ObjC method specifics out of
ParseSubroutine (#95078)
This patch moves some of the `is_cxx_method`/`objc_method` logic out of
`DWARFASTParserClang::ParseSubroutine` into their own functions. Mainly
the purpose of this is to (hopefully) make this function more readable
by turning the deeply nested if-statements into early-returns. This will
be useful in an upcoming change where we remove some of the branches of
said if-statement.
Considerations:
* Would be nice to make them into static helpers in
`DWARFASTParserClang.cpp`. That would require them take few more
arguments which seemed to get unwieldy.
* `HandleCXXMethod` can return three states: (1) found a `TypeSP` we
previously parsed (2) successfully set a link between the DIE and
DeclContext (3) failure. One could express this with
`std::optional`, but then returning `std::nullopt` vs `nullptr`
becomes hard to reason about. So I opted to return `std::pair`, where the `bool` indicates success and the `TypeSP` the cached
type.
* `HandleCXXMethod` takes `ignore_containing_context` as an output
parameter. Haven't found a great way to do this differently
Added:
Modified:
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
Removed:
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 579a538af3634..df1794c5d1b0c 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -975,6 +975,216 @@ 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();
+ assert(dwarf);
+
+ 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();
+ assert(dwarf);
+
+ 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"...
+if (DWARFDIE class_type_die = dwarf->GetDIE(class_type->GetID())) {
+ 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(
[Lldb-commits] [lldb] 860f0b5 - [lldb][ConstString] Prevent GetString from constructing a std::string with a nullptr (#95175)
Author: Michael Buch
Date: 2024-06-12T09:34:12+01:00
New Revision: 860f0b542ae32c507959201146242cd716222041
URL:
https://github.com/llvm/llvm-project/commit/860f0b542ae32c507959201146242cd716222041
DIFF:
https://github.com/llvm/llvm-project/commit/860f0b542ae32c507959201146242cd716222041.diff
LOG: [lldb][ConstString] Prevent GetString from constructing a std::string with
a nullptr (#95175)
This patch prevents passing a `nullptr` to the `std::string` constructor
in `GetString`. This prevents UB arising from calling `GetString` on a
default-constructed `ConstString`.
Added:
Modified:
lldb/include/lldb/Utility/ConstString.h
lldb/unittests/Utility/ConstStringTest.cpp
Removed:
diff --git a/lldb/include/lldb/Utility/ConstString.h
b/lldb/include/lldb/Utility/ConstString.h
index f7f7ec7605eba..692ecb63bd763 100644
--- a/lldb/include/lldb/Utility/ConstString.h
+++ b/lldb/include/lldb/Utility/ConstString.h
@@ -199,7 +199,9 @@ class ConstString {
}
/// Get the string value as a std::string
- std::string GetString() const { return std::string(m_string, GetLength()); }
+ std::string GetString() const {
+return std::string(AsCString(""), GetLength());
+ }
/// Get the string value as a C string.
///
diff --git a/lldb/unittests/Utility/ConstStringTest.cpp
b/lldb/unittests/Utility/ConstStringTest.cpp
index 716f2d8d6c804..7018248991b6e 100644
--- a/lldb/unittests/Utility/ConstStringTest.cpp
+++ b/lldb/unittests/Utility/ConstStringTest.cpp
@@ -88,6 +88,7 @@ TEST(ConstStringTest, NullAndEmptyStates) {
EXPECT_TRUE(!null);
EXPECT_TRUE(null.IsEmpty());
EXPECT_TRUE(null.IsNull());
+ EXPECT_TRUE(null.GetString().empty());
}
TEST(ConstStringTest, CompareConstString) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][ConstString] Prevent GetString from constructing a std::string with a nullptr (PR #95175)
https://github.com/Michael137 closed https://github.com/llvm/llvm-project/pull/95175 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [WIP] [lldb][Progress] Report progress when completing types from DWARF (PR #91452)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/91452
>From d9d0e0de9d57cefc8be78efa5ba9254127c68521 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Wed, 8 May 2024 10:47:54 +0100
Subject: [PATCH 1/2] [lldb][DWARFASTParserClang] Report progress when parsing
types from DWARF
---
.../source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp | 6 ++
1 file changed, 6 insertions(+)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 579a538af3634..c701fa230bb79 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -23,6 +23,7 @@
#include "Plugins/ExpressionParser/Clang/ClangUtil.h"
#include "Plugins/Language/ObjC/ObjCLanguage.h"
#include "lldb/Core/Module.h"
+#include "lldb/Core/Progress.h"
#include "lldb/Core/Value.h"
#include "lldb/Host/Host.h"
#include "lldb/Symbol/CompileUnit.h"
@@ -1758,6 +1759,11 @@ DWARFASTParserClang::ParseStructureLikeDIE(const
SymbolContext &sc,
}
if (attrs.is_forward_declaration) {
+Progress progress(llvm::formatv(
+"Parsing forward declaration {0}: {1}",
+dwarf->GetObjectFile()->GetFileSpec().GetFilename().GetString(),
+attrs.name.GetString()));
+
// We have a forward declaration to a type and we need to try and
// find a full declaration. We look in the current type index just in
// case we have a forward declaration followed by an actual
>From 5dc83694d3a92769a825a73f03ba85cd772b84e6 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Tue, 11 Jun 2024 23:27:40 +0100
Subject: [PATCH 2/2] fixup! rephrase progress message
---
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index c701fa230bb79..3255144da6d9b 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1760,7 +1760,7 @@ DWARFASTParserClang::ParseStructureLikeDIE(const
SymbolContext &sc,
if (attrs.is_forward_declaration) {
Progress progress(llvm::formatv(
-"Parsing forward declaration {0}: {1}",
+"Parsing type in {0}: '{1}'",
dwarf->GetObjectFile()->GetFileSpec().GetFilename().GetString(),
attrs.name.GetString()));
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [lldb][FreeBSD][AArch64] Enable register field detection (PR #85058)
https://github.com/DavidSpickett updated
https://github.com/llvm/llvm-project/pull/85058
>From 60086c78d9be91704158f45e19813217289757b7 Mon Sep 17 00:00:00 2001
From: David Spickett
Date: Tue, 11 Jun 2024 13:41:18 +
Subject: [PATCH 1/2] [lldb] Do not produce field information for registers
known not to exist
Currently the logic is generate field information for all registers in
LinuxArm64RegisterFlags and then as we walk the existing register info,
only those that are in that existing info will get the new fields patched in.
This works fine but on a review for FreeBSD support it was pointed out
that this is not obvious from the source code.
So instead I've allowed the construction of empty lists of fields,
and field detection methods can return an empty field list if they
think that the register will never exist.
Then the pre-existing code will see the empty field list, and never
look for that register in the register info.
I think removing the assert is ok because the GDB classes filter
out empty field lists at runtime, and anyone updating the built in
field information would presumably notice if none of the fields they
intended to add were displayed.
mte_ctrl and svcr are the only registers that need this so far.
There is no extra testing here as the behaviour is the same, it
doesn't add field information to regiters that don't exist. The
mechanism is just clearer now.
---
.../Process/Utility/RegisterFlagsLinux_arm64.cpp | 11 +--
.../Process/Utility/RegisterFlagsLinux_arm64.h| 6 +++---
lldb/source/Target/RegisterFlags.cpp | 4
3 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
index 51553817921f3..8ed75d700f225 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
@@ -20,6 +20,7 @@
#define HWCAP2_BTI (1ULL << 17)
#define HWCAP2_MTE (1ULL << 18)
#define HWCAP2_AFP (1ULL << 20)
+#define HWCAP2_SME (1ULL << 23)
#define HWCAP2_EBF16 (1ULL << 32)
using namespace lldb_private;
@@ -27,7 +28,10 @@ using namespace lldb_private;
LinuxArm64RegisterFlags::Fields
LinuxArm64RegisterFlags::DetectSVCRFields(uint64_t hwcap, uint64_t hwcap2) {
(void)hwcap;
- (void)hwcap2;
+
+ if (!(hwcap2 & HWCAP2_SME))
+return {};
+
// Represents the pseudo register that lldb-server builds, which itself
// matches the architectural register SCVR. The fields match SVCR in the Arm
// manual.
@@ -40,7 +44,10 @@ LinuxArm64RegisterFlags::DetectSVCRFields(uint64_t hwcap,
uint64_t hwcap2) {
LinuxArm64RegisterFlags::Fields
LinuxArm64RegisterFlags::DetectMTECtrlFields(uint64_t hwcap, uint64_t hwcap2) {
(void)hwcap;
- (void)hwcap2;
+
+ if (!(hwcap2 & HWCAP2_MTE))
+return {};
+
// Represents the contents of NT_ARM_TAGGED_ADDR_CTRL and the value passed
// to prctl(PR_TAGGED_ADDR_CTRL...). Fields are derived from the defines
// used to build the value.
diff --git a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h
b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h
index 660bef08700f4..49b1d90db64f6 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h
+++ b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h
@@ -38,8 +38,8 @@ class LinuxArm64RegisterFlags {
/// For the registers listed in this class, detect which fields are
/// present. Must be called before UpdateRegisterInfos.
/// If called more than once, fields will be redetected each time from
- /// scratch. If you do not have access to hwcap, just pass 0 for each one,
you
- /// will only get unconditional fields.
+ /// scratch. If the target would not have this register at all, the list of
+ /// fields will be left empty.
void DetectFields(uint64_t hwcap, uint64_t hwcap2);
/// Add the field information of any registers named in this class,
@@ -63,7 +63,7 @@ class LinuxArm64RegisterFlags {
struct RegisterEntry {
RegisterEntry(llvm::StringRef name, unsigned size, DetectorFn detector)
-: m_name(name), m_flags(std::string(name) + "_flags", size, {{"", 0}}),
+: m_name(name), m_flags(std::string(name) + "_flags", size, {}),
m_detector(detector) {}
llvm::StringRef m_name;
diff --git a/lldb/source/Target/RegisterFlags.cpp
b/lldb/source/Target/RegisterFlags.cpp
index 5274960587bf3..de9f12649e2ec 100644
--- a/lldb/source/Target/RegisterFlags.cpp
+++ b/lldb/source/Target/RegisterFlags.cpp
@@ -54,10 +54,6 @@ unsigned RegisterFlags::Field::PaddingDistance(const Field
&other) const {
}
void RegisterFlags::SetFields(const std::vector &fields) {
- // We expect that the XML processor will discard anything describing flags
but
- // with no fields.
- assert(fields.size() && "Some fields must be provided.");
-
// We expect that th
[Lldb-commits] [lldb] [llvm] [lldb][FreeBSD][AArch64] Enable register field detection (PR #85058)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/85058 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [lldb][FreeBSD][AArch64] Enable register field detection (PR #85058)
DavidSpickett wrote: Rebased so that commit #1 here is https://github.com/llvm/llvm-project/pull/95125. > It looks like svcr is for SME & mte_ctrl is for MTE. Neither of these are > currently supported in FreeBSD. When they are supported the appropriate HWCAP > flags (and ID_AA64* registers) will be set to tell userspace it can use them. Which will address this point. So the second commit is now the changes for review. https://github.com/llvm/llvm-project/pull/85058 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [lldb][FreeBSD][AArch64] Enable register field detection (PR #85058)
DavidSpickett wrote: Eventually we'll need to pass the OS type to the detector, but this is not needed now and quite easy to add when it is needed. https://github.com/llvm/llvm-project/pull/85058 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] d32afb3 - [lldb][Progress] Report progress when parsing forward declarations from DWARF (#91452)
Author: Michael Buch
Date: 2024-06-12T10:33:03+01:00
New Revision: d32afb39fd90a305fc116a7161a2b4c4556117d4
URL:
https://github.com/llvm/llvm-project/commit/d32afb39fd90a305fc116a7161a2b4c4556117d4
DIFF:
https://github.com/llvm/llvm-project/commit/d32afb39fd90a305fc116a7161a2b4c4556117d4.diff
LOG: [lldb][Progress] Report progress when parsing forward declarations from
DWARF (#91452)
This is an attempt at displaying the work that's being done by LLDB when
waiting on type-completion events, e.g., when running an expression. This patch
adds a single new progress event for cases where we search for the definition
DIE of a forward declaration, which can be an expensive operation in the
presence of many object files.
Added:
Modified:
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
Removed:
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index df1794c5d1b0c..b5d8cf8b2d480 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -23,6 +23,7 @@
#include "Plugins/ExpressionParser/Clang/ClangUtil.h"
#include "Plugins/Language/ObjC/ObjCLanguage.h"
#include "lldb/Core/Module.h"
+#include "lldb/Core/Progress.h"
#include "lldb/Core/Value.h"
#include "lldb/Host/Host.h"
#include "lldb/Symbol/CompileUnit.h"
@@ -1773,6 +1774,11 @@ DWARFASTParserClang::ParseStructureLikeDIE(const
SymbolContext &sc,
}
if (attrs.is_forward_declaration) {
+Progress progress(llvm::formatv(
+"Parsing type in {0}: '{1}'",
+dwarf->GetObjectFile()->GetFileSpec().GetFilename().GetString(),
+attrs.name.GetString()));
+
// We have a forward declaration to a type and we need to try and
// find a full declaration. We look in the current type index just in
// case we have a forward declaration followed by an actual
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [WIP] [lldb][Progress] Report progress when completing types from DWARF (PR #91452)
https://github.com/Michael137 closed https://github.com/llvm/llvm-project/pull/91452 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [WIP] [lldb][Progress] Report progress when completing types from DWARF (PR #91452)
Michael137 wrote: Only buildkite test failure was `TestConcurrentVFork`, which seemed unrelated. https://github.com/llvm/llvm-project/pull/91452 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [WIP][lldb] Use forward decls with redeclared definitions instead of minimal import for records (PR #95100)
labath wrote:
> > * when you say "slower", what exactly does that mean. How much slow down
> > are we talking about?
> > * the "increased number of DWARF searches", is that due to clang asking for
> > definitions of types more eagerly? If yes, do you have some examples of
> > where are these extra definitions coming from?
>
> For example, attaching LLDB to Clang and evaluating a method of the
> `ASTContext` class would take ~35s instead of the baseline ~6s. One of the
> main contributors to this is that completing a definition for a class would
> try to resolve all fields _and_ member functions of that class (see
> `DWARFASTParserClang::CompleteRecordType` where it resolves the member
> function DIEs). This in turn completes the parameters, etc. So we end up
> trying to pull in many more definitions than needed. The tricky part here is
> that anything that calls `getDefinition` (which we do in the `ASTImporter`
> for example) could trigger completion. So we can have situations where the
> user requests a `GetForwardCompilerType` but that ends up trying to
> `CompleteRedeclChain`. The current patch breaks the notion of
> `{Forward/Layout/Full}CompilerType`, but haven't found a good way of fixing
> that yet.
>
> Another source of overhead are cases where we try to find the definition DIE
> using a commonly used name such as `__base`, which we find many matches for
> in the index, but we fail to find a correct match. Such false positive
> lookups are quite expensive. I forget the details at the moment but IIRC this
> patch just makes those lookups more frequent. And the actual failure to
> choose the correct name for lookup is a consequence of how templates are
> represented in the LLDB AST.
Ack. Thanks for explaining.
>
> If we restrict LLDB to only complete fields (but complete methods only as
> needed), we get much closer to the baseline. Though "lazily" completing
> methods has other consequences (some features in LLDB rely on all methods
> being available, e.g., auto-complete, etc.). Apart from that, there still
> seem to be some redundant global lookups for namespaces and functions that
> add to the overhead. Once we get closer to the baseline performance I can
> provide some more accurate benchmark results. Also, happy to hear other
> suggestions/concerns regarding this.
I'm not sure by what you meant by "completing methods" in this paragraph. Does
that mean "adding the methods (`CXXMethodDecl`s) to the containing class
(`CXXRecordDecl`)", "completing (getting the definition) of the method's
arguments and/or return types", or something else? My understanding is that
we're currently always adding the declaration of the methods when completing
the class, but that we don't eagerly complete the types arguments or results of
the methods (except for covariant methods). I can see how some sort of lazy
method addition could break tab completion, but I think that'd be pretty hard
to implement (this is actually the idea I was looking into, but I sort of gave
up on it because it didn't seem to bring much benefit). OTOH, completing all
argument/result types seems rather unnecessary (I don't think C++ requires that
for anything) and very expensive.
>
> > * I see one test which tries to skip itself conditionally based on a
> > setting enabling this, but I don't see the code for the handling the
> > setting itself. Is the intention to add the setting, or will this be a flag
> > day change?
>
> Good catch! I cherry-picked these commits from the `apple/llvm-project` fork
> where we put these changes behind a setting. But upstream I would like to
> avoid doing so. I'll remove the decorator from the test (we can just XPASS it)
Might I ask why? Was it too unwieldy? I can certainly see how a change like
this would be hard to keep under a flag, but on the other hand, a
risky/wide-sweeping change like this is where a flag would be most useful
(e.g., it would enable incremental implementation&bugfixes instead of
revert-reapply Yoyo-ing)
>
> > * I'm intrigued by this line "Instead of creating partially defined records
> > that have fields but possibly no definition, ...". Until last week, I had
> > assumed that types in lldb (and clang) can exist in only one of two states
> > "empty/forward declared/undefined" and "fully defined". I was intrigued to
> > find some support for loading only fields
> > (`setHasLoadedFieldsFromExternalStorage`, et al.) in clang (but not in
> > lldb, AIUI). Does this imply that the code for loading only the fields is
> > broken (is the thing you're trying to remove)?
>
> Good question, I've been having trouble getting small reproducers for this.
> We've had numerous workarounds around the ASTImporter since the original
> patch was proposed that addressed these types of issues. E.g.,
> https://reviews.llvm.org/D71378 (which isn't quite the "definition but no
> fields" situation, but wouldn't be surprised if historically this co
[Lldb-commits] [lldb] [lldb] Move DWARFDeclContext functions from DWARFDebugInfoEntry to DW… (PR #95227)
https://github.com/labath created
https://github.com/llvm/llvm-project/pull/95227
…ARFDIE
This puts them closer to the other two functions doing something very similar.
I've tried to stick to the original logic of the functions as much as possible,
though I did apply some easy simplifications.
The changes in DWARFDeclContext.h are there to make the unit tests produce more
useful error messages.
>From 7a79c68fa118f9795fc72d77dfb494b549e83af0 Mon Sep 17 00:00:00 2001
From: Pavel Labath
Date: Wed, 12 Jun 2024 09:18:35 +0200
Subject: [PATCH] [lldb] Move DWARFDeclContext functions from
DWARFDebugInfoEntry to DWARFDIE
This puts them closer to the other two functions doing something very
similar. I've tried to stick to the original logic of the functions as
much as possible, though I did apply some easy simplifications.
The changes in DWARFDeclContext.h are there to make the unit tests
produce more useful error messages.
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 2 +-
.../Plugins/SymbolFile/DWARF/DWARFDIE.cpp | 60 ++-
.../Plugins/SymbolFile/DWARF/DWARFDIE.h | 2 +
.../SymbolFile/DWARF/DWARFDebugInfoEntry.cpp | 73 ---
.../SymbolFile/DWARF/DWARFDebugInfoEntry.h| 9 ---
.../SymbolFile/DWARF/DWARFDeclContext.cpp | 18 ++---
.../SymbolFile/DWARF/DWARFDeclContext.h | 15
.../Plugins/SymbolFile/DWARF/DWARFIndex.cpp | 3 +-
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 17 ++---
.../SymbolFile/DWARF/SymbolFileDWARF.h| 2 -
.../SymbolFile/DWARF/DWARFDIETest.cpp | 5 ++
11 files changed, 93 insertions(+), 113 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index b5d8cf8b2d480..02c91ddc57f83 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2461,7 +2461,7 @@
DWARFASTParserClang::ConstructDemangledNameFromDWARF(const DWARFDIE &die) {
std::vector param_decls;
StreamString sstr;
- DWARFDeclContext decl_ctx = SymbolFileDWARF::GetDWARFDeclContext(die);
+ DWARFDeclContext decl_ctx = die.GetDWARFDeclContext();
sstr << decl_ctx.GetQualifiedName();
clang::DeclContext *containing_decl_ctx =
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
index 992d814793f9d..ada3da85112fe 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
@@ -505,12 +505,64 @@ std::vector
DWARFDIE::GetTypeLookupContext() const {
return context;
}
+static DWARFDeclContext GetDWARFDeclContextImpl(DWARFDIE die) {
+ DWARFDeclContext dwarf_decl_ctx;
+ while (die) {
+const dw_tag_t tag = die.Tag();
+if (tag == DW_TAG_compile_unit || tag == DW_TAG_partial_unit)
+ break;
+dwarf_decl_ctx.AppendDeclContext(tag, die.GetName());
+DWARFDIE parent_decl_ctx_die = die.GetParentDeclContextDIE();
+if (parent_decl_ctx_die == die)
+ break;
+die = parent_decl_ctx_die;
+ }
+ return dwarf_decl_ctx;
+}
+
+DWARFDeclContext DWARFDIE::GetDWARFDeclContext() const {
+ return GetDWARFDeclContextImpl(*this);
+}
+
+static DWARFDIE GetParentDeclContextDIEImpl(DWARFDIE die) {
+ DWARFDIE orig_die = die;
+ while (die) {
+// If this is the original DIE that we are searching for a declaration for,
+// then don't look in the cache as we don't want our own decl context to be
+// our decl context...
+if (die != orig_die) {
+ switch (die.Tag()) {
+ case DW_TAG_compile_unit:
+ case DW_TAG_partial_unit:
+ case DW_TAG_namespace:
+ case DW_TAG_structure_type:
+ case DW_TAG_union_type:
+ case DW_TAG_class_type:
+return die;
+
+ default:
+break;
+ }
+}
+
+if (DWARFDIE spec_die = die.GetReferencedDIE(DW_AT_specification)) {
+ if (DWARFDIE decl_ctx_die = spec_die.GetParentDeclContextDIE())
+return decl_ctx_die;
+}
+
+if (DWARFDIE abs_die = die.GetReferencedDIE(DW_AT_abstract_origin)) {
+ if (DWARFDIE decl_ctx_die = abs_die.GetParentDeclContextDIE())
+return decl_ctx_die;
+}
+
+die = die.GetParent();
+ }
+ return DWARFDIE();
+}
+
DWARFDIE
DWARFDIE::GetParentDeclContextDIE() const {
- if (IsValid())
-return m_die->GetParentDeclContextDIE(m_cu);
- else
-return DWARFDIE();
+ return GetParentDeclContextDIEImpl(*this);
}
bool DWARFDIE::IsStructUnionOrClass() const {
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
index c74a82061fccf..e1318953a384c 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
@@ -86,6 +86,8 @@ class DWARFDIE : public DWARFBaseDIE {
/// using a full or partial CompilerContext array.
std::vector GetTypeLookupContext() const;
[Lldb-commits] [lldb] [lldb] Move DWARFDeclContext functions from DWARFDebugInfoEntry to DW… (PR #95227)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Pavel Labath (labath)
Changes
…ARFDIE
This puts them closer to the other two functions doing something very similar.
I've tried to stick to the original logic of the functions as much as possible,
though I did apply some easy simplifications.
The changes in DWARFDeclContext.h are there to make the unit tests produce more
useful error messages.
---
Full diff: https://github.com/llvm/llvm-project/pull/95227.diff
11 Files Affected:
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
(+1-1)
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp (+56-4)
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h (+2)
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp (-73)
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h (-9)
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFDeclContext.cpp (+8-10)
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFDeclContext.h (+15)
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp (+1-2)
- (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (+5-12)
- (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h (-2)
- (modified) lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp (+5)
``diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index b5d8cf8b2d480..02c91ddc57f83 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2461,7 +2461,7 @@
DWARFASTParserClang::ConstructDemangledNameFromDWARF(const DWARFDIE &die) {
std::vector param_decls;
StreamString sstr;
- DWARFDeclContext decl_ctx = SymbolFileDWARF::GetDWARFDeclContext(die);
+ DWARFDeclContext decl_ctx = die.GetDWARFDeclContext();
sstr << decl_ctx.GetQualifiedName();
clang::DeclContext *containing_decl_ctx =
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
index 992d814793f9d..ada3da85112fe 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
@@ -505,12 +505,64 @@ std::vector
DWARFDIE::GetTypeLookupContext() const {
return context;
}
+static DWARFDeclContext GetDWARFDeclContextImpl(DWARFDIE die) {
+ DWARFDeclContext dwarf_decl_ctx;
+ while (die) {
+const dw_tag_t tag = die.Tag();
+if (tag == DW_TAG_compile_unit || tag == DW_TAG_partial_unit)
+ break;
+dwarf_decl_ctx.AppendDeclContext(tag, die.GetName());
+DWARFDIE parent_decl_ctx_die = die.GetParentDeclContextDIE();
+if (parent_decl_ctx_die == die)
+ break;
+die = parent_decl_ctx_die;
+ }
+ return dwarf_decl_ctx;
+}
+
+DWARFDeclContext DWARFDIE::GetDWARFDeclContext() const {
+ return GetDWARFDeclContextImpl(*this);
+}
+
+static DWARFDIE GetParentDeclContextDIEImpl(DWARFDIE die) {
+ DWARFDIE orig_die = die;
+ while (die) {
+// If this is the original DIE that we are searching for a declaration for,
+// then don't look in the cache as we don't want our own decl context to be
+// our decl context...
+if (die != orig_die) {
+ switch (die.Tag()) {
+ case DW_TAG_compile_unit:
+ case DW_TAG_partial_unit:
+ case DW_TAG_namespace:
+ case DW_TAG_structure_type:
+ case DW_TAG_union_type:
+ case DW_TAG_class_type:
+return die;
+
+ default:
+break;
+ }
+}
+
+if (DWARFDIE spec_die = die.GetReferencedDIE(DW_AT_specification)) {
+ if (DWARFDIE decl_ctx_die = spec_die.GetParentDeclContextDIE())
+return decl_ctx_die;
+}
+
+if (DWARFDIE abs_die = die.GetReferencedDIE(DW_AT_abstract_origin)) {
+ if (DWARFDIE decl_ctx_die = abs_die.GetParentDeclContextDIE())
+return decl_ctx_die;
+}
+
+die = die.GetParent();
+ }
+ return DWARFDIE();
+}
+
DWARFDIE
DWARFDIE::GetParentDeclContextDIE() const {
- if (IsValid())
-return m_die->GetParentDeclContextDIE(m_cu);
- else
-return DWARFDIE();
+ return GetParentDeclContextDIEImpl(*this);
}
bool DWARFDIE::IsStructUnionOrClass() const {
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
index c74a82061fccf..e1318953a384c 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
@@ -86,6 +86,8 @@ class DWARFDIE : public DWARFBaseDIE {
/// using a full or partial CompilerContext array.
std::vector GetTypeLookupContext() const;
+ DWARFDeclContext GetDWARFDeclContext() const;
+
// Getting attribute values from the DIE.
//
// GetAttributeValueAsXXX() functions should only be used if you are
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWAR
[Lldb-commits] [lldb] [lldb] Move DWARFDeclContext functions from DWARFDebugInfoEntry to DW… (PR #95227)
https://github.com/labath updated
https://github.com/llvm/llvm-project/pull/95227
>From 096b84cd00ce5858bc0562e72660ca68d17c9edc Mon Sep 17 00:00:00 2001
From: Pavel Labath
Date: Wed, 12 Jun 2024 09:18:35 +0200
Subject: [PATCH] [lldb] Move DWARFDeclContext functions from
DWARFDebugInfoEntry to DWARFDIE
This puts them closer to the other two functions doing something very
similar. I've tried to stick to the original logic of the functions as
much as possible, though I did apply some easy simplifications.
The changes in DWARFDeclContext.h are there to make the unit tests
produce more useful error messages.
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 2 +-
.../Plugins/SymbolFile/DWARF/DWARFDIE.cpp | 60 ++-
.../Plugins/SymbolFile/DWARF/DWARFDIE.h | 2 +
.../SymbolFile/DWARF/DWARFDebugInfoEntry.cpp | 73 ---
.../SymbolFile/DWARF/DWARFDebugInfoEntry.h| 9 ---
.../SymbolFile/DWARF/DWARFDeclContext.cpp | 18 ++---
.../SymbolFile/DWARF/DWARFDeclContext.h | 18 -
.../Plugins/SymbolFile/DWARF/DWARFIndex.cpp | 3 +-
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 17 ++---
.../SymbolFile/DWARF/SymbolFileDWARF.h| 2 -
.../SymbolFile/DWARF/DWARFDIETest.cpp | 5 ++
11 files changed, 95 insertions(+), 114 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index b5d8cf8b2d480..02c91ddc57f83 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2461,7 +2461,7 @@
DWARFASTParserClang::ConstructDemangledNameFromDWARF(const DWARFDIE &die) {
std::vector param_decls;
StreamString sstr;
- DWARFDeclContext decl_ctx = SymbolFileDWARF::GetDWARFDeclContext(die);
+ DWARFDeclContext decl_ctx = die.GetDWARFDeclContext();
sstr << decl_ctx.GetQualifiedName();
clang::DeclContext *containing_decl_ctx =
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
index 992d814793f9d..ada3da85112fe 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
@@ -505,12 +505,64 @@ std::vector
DWARFDIE::GetTypeLookupContext() const {
return context;
}
+static DWARFDeclContext GetDWARFDeclContextImpl(DWARFDIE die) {
+ DWARFDeclContext dwarf_decl_ctx;
+ while (die) {
+const dw_tag_t tag = die.Tag();
+if (tag == DW_TAG_compile_unit || tag == DW_TAG_partial_unit)
+ break;
+dwarf_decl_ctx.AppendDeclContext(tag, die.GetName());
+DWARFDIE parent_decl_ctx_die = die.GetParentDeclContextDIE();
+if (parent_decl_ctx_die == die)
+ break;
+die = parent_decl_ctx_die;
+ }
+ return dwarf_decl_ctx;
+}
+
+DWARFDeclContext DWARFDIE::GetDWARFDeclContext() const {
+ return GetDWARFDeclContextImpl(*this);
+}
+
+static DWARFDIE GetParentDeclContextDIEImpl(DWARFDIE die) {
+ DWARFDIE orig_die = die;
+ while (die) {
+// If this is the original DIE that we are searching for a declaration for,
+// then don't look in the cache as we don't want our own decl context to be
+// our decl context...
+if (die != orig_die) {
+ switch (die.Tag()) {
+ case DW_TAG_compile_unit:
+ case DW_TAG_partial_unit:
+ case DW_TAG_namespace:
+ case DW_TAG_structure_type:
+ case DW_TAG_union_type:
+ case DW_TAG_class_type:
+return die;
+
+ default:
+break;
+ }
+}
+
+if (DWARFDIE spec_die = die.GetReferencedDIE(DW_AT_specification)) {
+ if (DWARFDIE decl_ctx_die = spec_die.GetParentDeclContextDIE())
+return decl_ctx_die;
+}
+
+if (DWARFDIE abs_die = die.GetReferencedDIE(DW_AT_abstract_origin)) {
+ if (DWARFDIE decl_ctx_die = abs_die.GetParentDeclContextDIE())
+return decl_ctx_die;
+}
+
+die = die.GetParent();
+ }
+ return DWARFDIE();
+}
+
DWARFDIE
DWARFDIE::GetParentDeclContextDIE() const {
- if (IsValid())
-return m_die->GetParentDeclContextDIE(m_cu);
- else
-return DWARFDIE();
+ return GetParentDeclContextDIEImpl(*this);
}
bool DWARFDIE::IsStructUnionOrClass() const {
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
index c74a82061fccf..e1318953a384c 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
@@ -86,6 +86,8 @@ class DWARFDIE : public DWARFBaseDIE {
/// using a full or partial CompilerContext array.
std::vector GetTypeLookupContext() const;
+ DWARFDeclContext GetDWARFDeclContext() const;
+
// Getting attribute values from the DIE.
//
// GetAttributeValueAsXXX() functions should only be used if you are
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
index 6
[Lldb-commits] [lldb] [lldb] Adds additional fields to ProcessInfo (PR #91544)
feg208 wrote: @clayborg I don't have commit access. Can you merge this? https://github.com/llvm/llvm-project/pull/91544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Move DWARFDeclContext functions from DWARFDebugInfoEntry to DW… (PR #95227)
https://github.com/Michael137 approved this pull request. LGTM, the `GetDWARFDeclContext` implementation is definitely easier to grok now https://github.com/llvm/llvm-project/pull/95227 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [WIP][lldb] Use forward decls with redeclared definitions instead of minimal import for records (PR #95100)
Michael137 wrote: > "completing (getting the definition) of the method's arguments and/or return > types", or something else? My understanding is that we're currently always > adding the declaration of the methods when completing the class, but that we > don't eagerly complete the types arguments or results of the methods (except > for covariant methods). Sorry I was a bit too loose in my terminology. You're correct, LLDB currently adds all methods to the containing class, and it's pretty cheap, because we don't fully complete arguments/return types. With this patch it becomes expensive when the ASTImporter gets involved. The core idea here is that we no longer do minimal import for records. So when we get to importing a class, the ASTImporter imports all the methods that LLDB attached, which in turn import all the parameters and return types, fully. That's where type explosion comes in. > completing all argument/result types seems rather unnecessary (I don't think > C++ requires that for anything) and very expensive. True, and AFAICT the ASTImporter has no notion of that atm (I'd need to confirm this again), so I played around with the idea of not adding all methods to the containing class, and instead add it when the Clang parser asks us (which is tricky to do well because we usually don't know that we're parsing a method call). Another idea was to add more control over which types the ASTImporter imports fully, though I never got a great implementation going for that. > Might I ask why? Was it too unwieldy? I can certainly see how a change like > this would be hard to keep under a flag, but on the other hand, a > risky/wide-sweeping change like this is where a flag would be most useful > (e.g., it would enable incremental implementation&bugfixes instead of > revert-reapply Yoyo-ing) It isn't too bad, but having two implementations of this mechanism seemed like a maintenance burden (e.g., runs the risk of not being removed in the long run, need to run the test-suite with and without the setting, etc.). If the community is open to it, I don't mind. Incremental bugfixing/improvements is a big plus. I guess it might be easier to decide on this once we ironed out the remaining TODOs. For reference, here's what it looks like with the setting: https://github.com/apple/llvm-project/pull/8222 (approximately 50 references to `UseRedeclCompletion()` when I check my local branch) https://github.com/llvm/llvm-project/pull/95100 ___ 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)
https://github.com/dzhidzhoev updated https://github.com/llvm/llvm-project/pull/93808 >From eaf50dc4a1f5285676c16b59c4149483d94fb986 Mon Sep 17 00:00:00 2001 From: Vladislav Dzhidzhoev Date: Thu, 9 May 2024 04:53:23 + Subject: [PATCH] [lldb][test] Disable PIE for some API tests These tests fail when PIE is enabled by default on a platform since `target variable` can't read global string variable value before running inferior. --- lldb/test/API/commands/target/basic/Makefile | 4 lldb/test/API/lang/c/global_variables/Makefile | 3 +++ lldb/test/API/lang/cpp/char8_t/Makefile| 3 +++ 3 files changed, 10 insertions(+) diff --git a/lldb/test/API/commands/target/basic/Makefile b/lldb/test/API/commands/target/basic/Makefile index b31e594019f6f..e66971834b689 100644 --- a/lldb/test/API/commands/target/basic/Makefile +++ b/lldb/test/API/commands/target/basic/Makefile @@ -3,4 +3,8 @@ # C_SOURCES := b.c # EXE := b.out +ifndef PIE + LDFLAGS := -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 7b94b6556f254..00c2557033d81 100644 --- a/lldb/test/API/lang/c/global_variables/Makefile +++ b/lldb/test/API/lang/c/global_variables/Makefile @@ -2,5 +2,8 @@ C_SOURCES := main.c DYLIB_NAME := a DYLIB_C_SOURCES := a.c +ifndef PIE + LDFLAGS := -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 e7c9938b5a85e..28f982a0078d8 100644 --- a/lldb/test/API/lang/cpp/char8_t/Makefile +++ b/lldb/test/API/lang/cpp/char8_t/Makefile @@ -1,4 +1,7 @@ CXX_SOURCES := main.cpp CXXFLAGS_EXTRAS := -std=c++2a -fchar8_t +ifndef PIE + LDFLAGS := -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] f6a2ca4 - [lldb][test] Disable PIE for some API tests (#93808)
Author: Vladislav Dzhidzhoev Date: 2024-06-12T17:10:20+02:00 New Revision: f6a2ca4f22e3d737a7aa488a4edde88d53dc8b26 URL: https://github.com/llvm/llvm-project/commit/f6a2ca4f22e3d737a7aa488a4edde88d53dc8b26 DIFF: https://github.com/llvm/llvm-project/commit/f6a2ca4f22e3d737a7aa488a4edde88d53dc8b26.diff LOG: [lldb][test] Disable PIE for some API tests (#93808) When PIE is enabled on a platform by default, these tests fail since the `target variable` command can't read a global string variable value before running an inferior process. It fixes the following tests when built with clang on Ubuntu aarch64: ``` commands/target/basic/TestTargetCommand.py lang/c/global_variables/TestGlobalVariables.py lang/cpp/char8_t/TestCxxChar8_t.py ``` 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 b31e594019f6f..e66971834b689 100644 --- a/lldb/test/API/commands/target/basic/Makefile +++ b/lldb/test/API/commands/target/basic/Makefile @@ -3,4 +3,8 @@ # C_SOURCES := b.c # EXE := b.out +ifndef PIE + LDFLAGS := -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 7b94b6556f254..00c2557033d81 100644 --- a/lldb/test/API/lang/c/global_variables/Makefile +++ b/lldb/test/API/lang/c/global_variables/Makefile @@ -2,5 +2,8 @@ C_SOURCES := main.c DYLIB_NAME := a DYLIB_C_SOURCES := a.c +ifndef PIE + LDFLAGS := -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 e7c9938b5a85e..28f982a0078d8 100644 --- a/lldb/test/API/lang/cpp/char8_t/Makefile +++ b/lldb/test/API/lang/cpp/char8_t/Makefile @@ -1,4 +1,7 @@ CXX_SOURCES := main.cpp CXXFLAGS_EXTRAS := -std=c++2a -fchar8_t +ifndef PIE + LDFLAGS := -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)
https://github.com/dzhidzhoev closed 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][API] Add Find(Ranges)InMemory() to Process SB API (PR #95007)
@@ -810,6 +809,65 @@ const char *SBProcess::GetBroadcasterClass() {
return ConstString(Process::GetStaticBroadcasterClass()).AsCString();
}
+lldb::SBAddressRangeList
+SBProcess::FindRangesInMemory(const void *buf, uint64_t size,
+ SBAddressRangeList &ranges, uint32_t alignment,
+ uint32_t max_matches, SBError &error) {
+ LLDB_INSTRUMENT_VA(this, buf, size, ranges, alignment, max_matches, error);
+
+ Log *log = GetLog(LLDBLog::Process);
+ lldb::SBAddressRangeList matches;
+
+ ProcessSP process_sp(GetSP());
+ if (!process_sp) {
+LLDB_LOGF(log, "SBProcess::%s SBProcess is invalid.", __FUNCTION__);
+return matches;
+ }
+ Process::StopLocker stop_locker;
+ if (!stop_locker.TryLock(&process_sp->GetRunLock())) {
+LLDB_LOGF(
+log,
+"SBProcess::%s Cannot find process in memory while process is
running.",
+__FUNCTION__);
+return matches;
+ }
+ std::lock_guard guard(
+ process_sp->GetTarget().GetAPIMutex());
+ matches.m_opaque_up->ref() = process_sp->FindRangesInMemory(
+ reinterpret_cast(buf), size, ranges.m_opaque_up->ref(),
+ alignment, max_matches, error.ref());
+ return matches;
+}
+
+lldb::addr_t SBProcess::FindInMemory(const void *buf, uint64_t size,
+ SBAddressRange &range, uint32_t alignment,
+ SBError &error) {
+ LLDB_INSTRUMENT_VA(this, buf, size, range, alignment, error);
+
+ if (!range.IsValid()) {
+error.SetErrorStringWithFormat("range is invalid");
+return LLDB_INVALID_ADDRESS;
+ }
+
mbucko wrote:
I'm calling `*range.m_opaque_up` further down so I left this check here
https://github.com/llvm/llvm-project/pull/95007
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][API] Add Find(Ranges)InMemory() to Process SB API (PR #95007)
@@ -2007,6 +2007,124 @@ size_t Process::ReadMemory(addr_t addr, void *buf,
size_t size, Status &error) {
}
}
+void Process::DoFindInMemory(lldb::addr_t start_addr, lldb::addr_t end_addr,
+ const uint8_t *buf, size_t size,
+ AddressRanges &matches, size_t alignment,
+ size_t max_count) {
+ // Inputs are already validated in FindInMemory() functions.
+ assert(buf != nullptr);
+ assert(size > 0);
+ assert(alignment > 0);
+ assert(max_count > 0);
+ assert(start_addr != LLDB_INVALID_ADDRESS);
+ assert(end_addr != LLDB_INVALID_ADDRESS);
+ assert(start_addr < end_addr);
+
+ lldb::addr_t start = start_addr;
+ while (matches.size() < max_count && (start + size) < end_addr) {
+const lldb::addr_t found_addr = FindInMemory(start, end_addr, buf, size);
+if (found_addr == LLDB_INVALID_ADDRESS)
+ break;
+matches.emplace_back(found_addr, size);
+start = found_addr + alignment;
+ }
+}
+
+AddressRanges Process::FindRangesInMemory(const uint8_t *buf, uint64_t size,
+ const AddressRanges &ranges,
+ size_t alignment, size_t max_count,
+ Status &error) {
+ AddressRanges matches;
+ if (buf == nullptr) {
+error.SetErrorStringWithFormat("buffer is null");
+return matches;
+ }
+ if (size == 0) {
+error.SetErrorStringWithFormat("size is zero");
+return matches;
+ }
+ if (ranges.empty()) {
+error.SetErrorStringWithFormat("ranges in empty");
+return matches;
+ }
+ if (alignment == 0) {
+error.SetErrorStringWithFormat(
+"invalid alignment %zu, must be greater than 0", alignment);
+return matches;
+ }
+ if (max_count == 0) {
+error.SetErrorStringWithFormat(
+"invalid max_count %zu, must be greater than 0", max_count);
+return matches;
+ }
+
+ Target &target = GetTarget();
+ Log *log = GetLog(LLDBLog::Process);
+ for (size_t i = 0; i < ranges.size(); ++i) {
+if (matches.size() >= max_count) {
+ break;
+}
+const AddressRange &range = ranges[i];
+if (range.IsValid() == false) {
+ LLDB_LOGF(log, "Process::%s range is invalid", __FUNCTION__);
mbucko wrote:
If we set the error but return successfully the ranges then the caller might
discard the returned ranges no?
https://github.com/llvm/llvm-project/pull/95007
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [libcxx] [lldb] [llvm] Add clang basic module directory (PR #93388)
ldionne wrote: For tidying up the libc++ review queue, please remove the `libc++` tag once you rebase this (the small changes in `libcxx` will go away). https://github.com/llvm/llvm-project/pull/93388 ___ 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][API] Add Find(Ranges)InMemory() to Process SB API (PR #95007)
@@ -0,0 +1,31 @@ +import lldb + +SINGLE_INSTANCE_PATTERN = "there_is_only_one_of_me" +DOUBLE_INSTANCE_PATTERN = "there_is_exactly_two_of_me" + + +def GetAddressRanges(test_base): mbucko wrote: Why not just store them all on stack and only use one region to search through? https://github.com/llvm/llvm-project/pull/95007 ___ 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 ValueObject class. (PR #87197)
cmtice wrote: > There was one comment that should probably be removed. This looks okay to me > at this stage. Jim, If this looks ok to you, could you officially mark it as approved please? https://github.com/llvm/llvm-project/pull/87197 ___ 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 ValueObject class. (PR #87197)
cmtice wrote: Review ping? Does this look ok to everyone now? I would really like to get this PR committed (I have more waiting behind it). Thanks in advance! https://github.com/llvm/llvm-project/pull/87197 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][API] Add Find(Ranges)InMemory() to Process SB API (PR #95007)
@@ -0,0 +1,31 @@ +import lldb + +SINGLE_INSTANCE_PATTERN = "there_is_only_one_of_me" +DOUBLE_INSTANCE_PATTERN = "there_is_exactly_two_of_me" + + +def GetAddressRanges(test_base): clayborg wrote: You will want to check that multiple memory ranges works, so you need at least two right? https://github.com/llvm/llvm-project/pull/95007 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][API] Add Find(Ranges)InMemory() to Process SB API (PR #95007)
@@ -2007,6 +2007,124 @@ size_t Process::ReadMemory(addr_t addr, void *buf,
size_t size, Status &error) {
}
}
+void Process::DoFindInMemory(lldb::addr_t start_addr, lldb::addr_t end_addr,
+ const uint8_t *buf, size_t size,
+ AddressRanges &matches, size_t alignment,
+ size_t max_count) {
+ // Inputs are already validated in FindInMemory() functions.
+ assert(buf != nullptr);
+ assert(size > 0);
+ assert(alignment > 0);
+ assert(max_count > 0);
+ assert(start_addr != LLDB_INVALID_ADDRESS);
+ assert(end_addr != LLDB_INVALID_ADDRESS);
+ assert(start_addr < end_addr);
+
+ lldb::addr_t start = start_addr;
+ while (matches.size() < max_count && (start + size) < end_addr) {
+const lldb::addr_t found_addr = FindInMemory(start, end_addr, buf, size);
+if (found_addr == LLDB_INVALID_ADDRESS)
+ break;
+matches.emplace_back(found_addr, size);
+start = found_addr + alignment;
+ }
+}
+
+AddressRanges Process::FindRangesInMemory(const uint8_t *buf, uint64_t size,
+ const AddressRanges &ranges,
+ size_t alignment, size_t max_count,
+ Status &error) {
+ AddressRanges matches;
+ if (buf == nullptr) {
+error.SetErrorStringWithFormat("buffer is null");
+return matches;
+ }
+ if (size == 0) {
+error.SetErrorStringWithFormat("size is zero");
+return matches;
+ }
+ if (ranges.empty()) {
+error.SetErrorStringWithFormat("ranges in empty");
+return matches;
+ }
+ if (alignment == 0) {
+error.SetErrorStringWithFormat(
+"invalid alignment %zu, must be greater than 0", alignment);
+return matches;
+ }
+ if (max_count == 0) {
+error.SetErrorStringWithFormat(
+"invalid max_count %zu, must be greater than 0", max_count);
+return matches;
+ }
+
+ Target &target = GetTarget();
+ Log *log = GetLog(LLDBLog::Process);
+ for (size_t i = 0; i < ranges.size(); ++i) {
+if (matches.size() >= max_count) {
+ break;
+}
+const AddressRange &range = ranges[i];
+if (range.IsValid() == false) {
+ LLDB_LOGF(log, "Process::%s range is invalid", __FUNCTION__);
clayborg wrote:
We can document it in the header documentation if we want to allow returning an
error but still have results. We can also just ignore invalid address ranges,
or we can add a counter to count the number of invalid address ranges, and if
it is equal to the number of ranges passed in, then we return an error like
"all address ranges are invalid"
https://github.com/llvm/llvm-project/pull/95007
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][API] Add Find(Ranges)InMemory() to Process SB API (PR #95007)
@@ -810,6 +809,65 @@ const char *SBProcess::GetBroadcasterClass() {
return ConstString(Process::GetStaticBroadcasterClass()).AsCString();
}
+lldb::SBAddressRangeList
+SBProcess::FindRangesInMemory(const void *buf, uint64_t size,
+ SBAddressRangeList &ranges, uint32_t alignment,
+ uint32_t max_matches, SBError &error) {
+ LLDB_INSTRUMENT_VA(this, buf, size, ranges, alignment, max_matches, error);
+
+ Log *log = GetLog(LLDBLog::Process);
+ lldb::SBAddressRangeList matches;
+
+ ProcessSP process_sp(GetSP());
+ if (!process_sp) {
+LLDB_LOGF(log, "SBProcess::%s SBProcess is invalid.", __FUNCTION__);
+return matches;
+ }
+ Process::StopLocker stop_locker;
+ if (!stop_locker.TryLock(&process_sp->GetRunLock())) {
+LLDB_LOGF(
+log,
+"SBProcess::%s Cannot find process in memory while process is
running.",
+__FUNCTION__);
+return matches;
+ }
+ std::lock_guard guard(
+ process_sp->GetTarget().GetAPIMutex());
+ matches.m_opaque_up->ref() = process_sp->FindRangesInMemory(
+ reinterpret_cast(buf), size, ranges.m_opaque_up->ref(),
+ alignment, max_matches, error.ref());
+ return matches;
+}
+
+lldb::addr_t SBProcess::FindInMemory(const void *buf, uint64_t size,
+ SBAddressRange &range, uint32_t alignment,
+ SBError &error) {
+ LLDB_INSTRUMENT_VA(this, buf, size, range, alignment, error);
+
+ if (!range.IsValid()) {
+error.SetErrorStringWithFormat("range is invalid");
+return LLDB_INVALID_ADDRESS;
+ }
+
clayborg wrote:
I would still just handle it in one place. Why? Because it makes sure the error
is consistent between the internal API and the external API. Lets say we change
the error above to "invalid address range", but the internal version returns
"range is invalid". Then we have a difference between the internal and external
APIs.
https://github.com/llvm/llvm-project/pull/95007
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][API] Add Find(Ranges)InMemory() to Process SB API (PR #95007)
@@ -810,6 +809,65 @@ const char *SBProcess::GetBroadcasterClass() {
return ConstString(Process::GetStaticBroadcasterClass()).AsCString();
}
+lldb::SBAddressRangeList
+SBProcess::FindRangesInMemory(const void *buf, uint64_t size,
+ SBAddressRangeList &ranges, uint32_t alignment,
+ uint32_t max_matches, SBError &error) {
+ LLDB_INSTRUMENT_VA(this, buf, size, ranges, alignment, max_matches, error);
+
+ Log *log = GetLog(LLDBLog::Process);
+ lldb::SBAddressRangeList matches;
+
+ ProcessSP process_sp(GetSP());
+ if (!process_sp) {
+LLDB_LOGF(log, "SBProcess::%s SBProcess is invalid.", __FUNCTION__);
clayborg wrote:
We should fill in the error here with `"invalid process"` (check other API
calls to make sure we use the same error string.
https://github.com/llvm/llvm-project/pull/95007
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][API] Add Find(Ranges)InMemory() to Process SB API (PR #95007)
@@ -810,6 +809,65 @@ const char *SBProcess::GetBroadcasterClass() {
return ConstString(Process::GetStaticBroadcasterClass()).AsCString();
}
+lldb::SBAddressRangeList
+SBProcess::FindRangesInMemory(const void *buf, uint64_t size,
+ SBAddressRangeList &ranges, uint32_t alignment,
+ uint32_t max_matches, SBError &error) {
+ LLDB_INSTRUMENT_VA(this, buf, size, ranges, alignment, max_matches, error);
+
+ Log *log = GetLog(LLDBLog::Process);
+ lldb::SBAddressRangeList matches;
+
+ ProcessSP process_sp(GetSP());
+ if (!process_sp) {
+LLDB_LOGF(log, "SBProcess::%s SBProcess is invalid.", __FUNCTION__);
+return matches;
+ }
+ Process::StopLocker stop_locker;
+ if (!stop_locker.TryLock(&process_sp->GetRunLock())) {
+LLDB_LOGF(
+log,
+"SBProcess::%s Cannot find process in memory while process is
running.",
+__FUNCTION__);
+return matches;
+ }
+ std::lock_guard guard(
+ process_sp->GetTarget().GetAPIMutex());
+ matches.m_opaque_up->ref() = process_sp->FindRangesInMemory(
+ reinterpret_cast(buf), size, ranges.m_opaque_up->ref(),
+ alignment, max_matches, error.ref());
+ return matches;
+}
+
+lldb::addr_t SBProcess::FindInMemory(const void *buf, uint64_t size,
+ SBAddressRange &range, uint32_t alignment,
+ SBError &error) {
+ LLDB_INSTRUMENT_VA(this, buf, size, range, alignment, error);
+
+ if (!range.IsValid()) {
+error.SetErrorStringWithFormat("range is invalid");
+return LLDB_INVALID_ADDRESS;
+ }
+
+ ProcessSP process_sp(GetSP());
+
+ if (!process_sp) {
+error.SetErrorStringWithFormat("SBProcess is invalid");
clayborg wrote:
No need for the `WithFormat` variant:
```
error.SetErrorString("SBProcess is invalid");
```
https://github.com/llvm/llvm-project/pull/95007
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][API] Add Find(Ranges)InMemory() to Process SB API (PR #95007)
@@ -810,6 +809,65 @@ const char *SBProcess::GetBroadcasterClass() {
return ConstString(Process::GetStaticBroadcasterClass()).AsCString();
}
+lldb::SBAddressRangeList
+SBProcess::FindRangesInMemory(const void *buf, uint64_t size,
+ SBAddressRangeList &ranges, uint32_t alignment,
+ uint32_t max_matches, SBError &error) {
+ LLDB_INSTRUMENT_VA(this, buf, size, ranges, alignment, max_matches, error);
+
+ Log *log = GetLog(LLDBLog::Process);
+ lldb::SBAddressRangeList matches;
+
+ ProcessSP process_sp(GetSP());
+ if (!process_sp) {
+LLDB_LOGF(log, "SBProcess::%s SBProcess is invalid.", __FUNCTION__);
+return matches;
+ }
+ Process::StopLocker stop_locker;
+ if (!stop_locker.TryLock(&process_sp->GetRunLock())) {
+LLDB_LOGF(
+log,
+"SBProcess::%s Cannot find process in memory while process is
running.",
+__FUNCTION__);
+return matches;
+ }
+ std::lock_guard guard(
+ process_sp->GetTarget().GetAPIMutex());
+ matches.m_opaque_up->ref() = process_sp->FindRangesInMemory(
+ reinterpret_cast(buf), size, ranges.m_opaque_up->ref(),
+ alignment, max_matches, error.ref());
+ return matches;
+}
+
+lldb::addr_t SBProcess::FindInMemory(const void *buf, uint64_t size,
+ SBAddressRange &range, uint32_t alignment,
+ SBError &error) {
+ LLDB_INSTRUMENT_VA(this, buf, size, range, alignment, error);
+
+ if (!range.IsValid()) {
+error.SetErrorStringWithFormat("range is invalid");
+return LLDB_INVALID_ADDRESS;
+ }
+
+ ProcessSP process_sp(GetSP());
+
+ if (!process_sp) {
+error.SetErrorStringWithFormat("SBProcess is invalid");
+return LLDB_INVALID_ADDRESS;
+ }
+
+ Process::StopLocker stop_locker;
+ if (!stop_locker.TryLock(&process_sp->GetRunLock())) {
+error.SetErrorStringWithFormat("process is running");
clayborg wrote:
No need to use the `WithFormat` variant, and match other errors when the
process isn't stopped:
```
error.SetErrorString("process is running");
```
https://github.com/llvm/llvm-project/pull/95007
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][API] Add Find(Ranges)InMemory() to Process SB API (PR #95007)
@@ -810,6 +809,65 @@ const char *SBProcess::GetBroadcasterClass() {
return ConstString(Process::GetStaticBroadcasterClass()).AsCString();
}
+lldb::SBAddressRangeList
+SBProcess::FindRangesInMemory(const void *buf, uint64_t size,
+ SBAddressRangeList &ranges, uint32_t alignment,
+ uint32_t max_matches, SBError &error) {
+ LLDB_INSTRUMENT_VA(this, buf, size, ranges, alignment, max_matches, error);
+
+ Log *log = GetLog(LLDBLog::Process);
+ lldb::SBAddressRangeList matches;
+
+ ProcessSP process_sp(GetSP());
+ if (!process_sp) {
+LLDB_LOGF(log, "SBProcess::%s SBProcess is invalid.", __FUNCTION__);
+return matches;
+ }
+ Process::StopLocker stop_locker;
+ if (!stop_locker.TryLock(&process_sp->GetRunLock())) {
+LLDB_LOGF(
+log,
+"SBProcess::%s Cannot find process in memory while process is
running.",
clayborg wrote:
Fill in error as well here.
https://github.com/llvm/llvm-project/pull/95007
___
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 ValueObject class. (PR #87197)
https://github.com/jimingham approved this pull request. LGTM at this point. https://github.com/llvm/llvm-project/pull/87197 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
https://github.com/Jlalond created
https://github.com/llvm/llvm-project/pull/95312
Currently, LLDB does not support taking a minidump over the 4.2gb limit imposed
by uint32. In fact, currently it writes the RVA's and the headers to the end of
the file, which can become corrupted due to the header offset only supporting a
32b offset.
This change reorganizes how the file structure is laid out. LLDB will
precalculate the number of directories required and preallocate space at the
top of the file to fill in later. Additionally, thread stacks require a 32b
offset, and we provision empty descriptors and keep track of them to clean up
once we write the 32b memory list.
For
[MemoryList64](https://learn.microsoft.com/en-us/windows/win32/api/minidumpapiset/ns-minidumpapiset-minidump_memory64_list),
the RVA to the start of the section itself will remain in a 32b addressable
space. We achieve this by predetermining the space the memory regions will
take, and only writing up to 4.2 gb of data with some buffer to allow all the
MemoryDescriptor64s to also still be 32b addressable.
I did not add any explicit tests to this PR because allocating 4.2gb+ to test
is very expensive. However, we have 32b automation tests and I validated with
in several ways, including with 5gb+ array/object and would be willing to add
this as a test case.
>From 810865580a4d8ea3f48e56f3964602fba6087ddf Mon Sep 17 00:00:00 2001
From: Jacob Lalonde
Date: Wed, 12 Jun 2024 13:57:10 -0700
Subject: [PATCH 1/5] Squash minidump-64-protoype.
This is a change to reorder LLDB's minidumps to support 64b memory
lists/minidumps while still keeping all directories under the 32b limit.
Better O(N), not N^2 cleanup code
---
.../Minidump/MinidumpFileBuilder.cpp | 520 ++
.../ObjectFile/Minidump/MinidumpFileBuilder.h | 55 +-
.../Minidump/ObjectFileMinidump.cpp | 38 +-
llvm/include/llvm/BinaryFormat/Minidump.h | 11 +
llvm/include/llvm/Object/Minidump.h | 7 +
llvm/lib/ObjectYAML/MinidumpEmitter.cpp | 7 +
6 files changed, 482 insertions(+), 156 deletions(-)
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 7231433619ffb..c08b5206720ab 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -20,35 +20,106 @@
#include "lldb/Target/RegisterContext.h"
#include "lldb/Target/StopInfo.h"
#include "lldb/Target/ThreadList.h"
+#include "lldb/Utility/DataBufferHeap.h"
#include "lldb/Utility/DataExtractor.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Log.h"
+#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/RegisterValue.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/BinaryFormat/Minidump.h"
#include "llvm/Support/ConvertUTF.h"
+#include "llvm/Support/Endian.h"
#include "llvm/Support/Error.h"
#include "Plugins/Process/minidump/MinidumpTypes.h"
+#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-forward.h"
+#include "lldb/lldb-types.h"
#include
+#include
+#include
+#include
+#include
+#include
using namespace lldb;
using namespace lldb_private;
using namespace llvm::minidump;
-void MinidumpFileBuilder::AddDirectory(StreamType type, size_t stream_size) {
+Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories(const
lldb_private::Target &target, const lldb::ProcessSP &process_sp) {
+ // First set the offset on the file, and on the bytes saved
+ //TODO: advance the core file.
+ m_saved_data_size += header_size;
+ // We know we will have at least Misc, SystemInfo, Modules, and ThreadList
(corresponding memory list for stacks)
+ // And an additional memory list for non-stacks.
+ m_expected_directories = 6;
+ // Check if OS is linux
+ if (target.GetArchitecture().GetTriple().getOS() ==
+ llvm::Triple::OSType::Linux)
+m_expected_directories += 9;
+
+ // Go through all of the threads and check for exceptions.
+ lldb_private::ThreadList thread_list = process_sp->GetThreadList();
+ const uint32_t num_threads = thread_list.GetSize();
+ for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
+ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx));
+StopInfoSP stop_info_sp = thread_sp->GetStopInfo();
+if (stop_info_sp && stop_info_sp->GetStopReason() ==
StopReason::eStopReasonException) {
+ m_expected_directories++;
+}
+ }
+
+ // Now offset the file by the directores so we can write them in later.
+ offset_t directory_offset = m_expected_directories * directory_size;
+ //TODO: advance the core file.
+ m_saved_data_size += directory_offset;
+ // Replace this when we make a better way to do this.
+ Status error;
+ Header empty_header;
+ size_t bytes_written;
+ bytes_written = header_size;
+ error = m_core_file->Write(&empty_header, bytes_written);
+ if (error.Fail()
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
llvmbot wrote:
@llvm/pr-subscribers-llvm-binary-utilities
Author: Jacob Lalonde (Jlalond)
Changes
Currently, LLDB does not support taking a minidump over the 4.2gb limit imposed
by uint32. In fact, currently it writes the RVA's and the headers to the end of
the file, which can become corrupted due to the header offset only supporting a
32b offset.
This change reorganizes how the file structure is laid out. LLDB will
precalculate the number of directories required and preallocate space at the
top of the file to fill in later. Additionally, thread stacks require a 32b
offset, and we provision empty descriptors and keep track of them to clean up
once we write the 32b memory list.
For
[MemoryList64](https://learn.microsoft.com/en-us/windows/win32/api/minidumpapiset/ns-minidumpapiset-minidump_memory64_list),
the RVA to the start of the section itself will remain in a 32b addressable
space. We achieve this by predetermining the space the memory regions will
take, and only writing up to 4.2 gb of data with some buffer to allow all the
MemoryDescriptor64s to also still be 32b addressable.
I did not add any explicit tests to this PR because allocating 4.2gb+ to test
is very expensive. However, we have 32b automation tests and I validated with
in several ways, including with 5gb+ array/object and would be willing to add
this as a test case.
---
Patch is 36.01 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/95312.diff
5 Files Affected:
- (modified) lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
(+412-118)
- (modified) lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
(+52-15)
- (modified) lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
(+23-25)
- (modified) llvm/include/llvm/BinaryFormat/Minidump.h (+5)
- (modified) llvm/lib/ObjectYAML/MinidumpEmitter.cpp (+2)
``diff
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 7231433619ffb..3c1d88652cc92 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -20,25 +20,98 @@
#include "lldb/Target/RegisterContext.h"
#include "lldb/Target/StopInfo.h"
#include "lldb/Target/ThreadList.h"
+#include "lldb/Utility/DataBufferHeap.h"
#include "lldb/Utility/DataExtractor.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Log.h"
+#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/RegisterValue.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/BinaryFormat/Minidump.h"
#include "llvm/Support/ConvertUTF.h"
+#include "llvm/Support/Endian.h"
#include "llvm/Support/Error.h"
#include "Plugins/Process/minidump/MinidumpTypes.h"
+#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-forward.h"
+#include "lldb/lldb-types.h"
+#include
#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
using namespace lldb;
using namespace lldb_private;
using namespace llvm::minidump;
-void MinidumpFileBuilder::AddDirectory(StreamType type, size_t stream_size) {
+Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories(
+const lldb_private::Target &target, const lldb::ProcessSP &process_sp) {
+ // First set the offset on the file, and on the bytes saved
+ m_saved_data_size += header_size;
+ // We know we will have at least Misc, SystemInfo, Modules, and ThreadList
+ // (corresponding memory list for stacks) And an additional memory list for
+ // non-stacks.
+ m_expected_directories = 6;
+ // Check if OS is linux
+ if (target.GetArchitecture().GetTriple().getOS() ==
+ llvm::Triple::OSType::Linux)
+m_expected_directories += 9;
+
+ // Go through all of the threads and check for exceptions.
+ lldb_private::ThreadList thread_list = process_sp->GetThreadList();
+ const uint32_t num_threads = thread_list.GetSize();
+ for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
+ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx));
+StopInfoSP stop_info_sp = thread_sp->GetStopInfo();
+if (stop_info_sp &&
+stop_info_sp->GetStopReason() == StopReason::eStopReasonException) {
+ m_expected_directories++;
+}
+ }
+
+ // Now offset the file by the directores so we can write them in later.
+ offset_t directory_offset = m_expected_directories * directory_size;
+ m_saved_data_size += directory_offset;
+ // Replace this when we make a better way to do this.
+ Status error;
+ Header empty_header;
+ size_t bytes_written;
+ bytes_written = header_size;
+ error = m_core_file->Write(&empty_header, bytes_written);
+ if (error.Fail() || bytes_written != header_size) {
+if (bytes_written != header_size)
+ error.SetErrorStringWithFormat(
+ "unable to write the header (written %zd/%zd)", bytes_written,
+ header_size);
+re
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff d6bbe2e20ff21cc2d31106742dfe1711ae5c641e
c592936dedb0dc494b03144d741ce57ac0b27809 --
lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
llvm/include/llvm/BinaryFormat/Minidump.h
llvm/lib/ObjectYAML/MinidumpEmitter.cpp
``
View the diff from clang-format here.
``diff
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 3c1d88652c..ca09e89cd6 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -24,7 +24,6 @@
#include "lldb/Utility/DataExtractor.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Log.h"
-#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/RegisterValue.h"
#include "llvm/ADT/StringRef.h"
@@ -311,10 +310,12 @@ Status MinidumpFileBuilder::AddModuleList(Target &target)
{
auto maybe_mod_size = getModuleFileSize(target, mod);
if (!maybe_mod_size) {
llvm::Error mod_size_err = maybe_mod_size.takeError();
- llvm::handleAllErrors(std::move(mod_size_err), [&](const
llvm::ErrorInfoBase &E) {
-error.SetErrorStringWithFormat("Unable to get the size of module %s:
%s.",
- module_name.c_str(), E.message().c_str());
- });
+ llvm::handleAllErrors(std::move(mod_size_err),
+[&](const llvm::ErrorInfoBase &E) {
+ error.SetErrorStringWithFormat(
+ "Unable to get the size of module %s: %s.",
+ module_name.c_str(), E.message().c_str());
+});
return error;
}
@@ -950,10 +951,10 @@ Status MinidumpFileBuilder::AddMemoryList_32(
const addr_t size = core_range.range.size();
auto data_up = std::make_unique(size, 0);
-LLDB_LOGF(
-log,
-"AddMemoryList %zu/%zu reading memory for region (%zu bytes) [%zx,
%zx)",
-region_index, ranges.size(), size, addr, addr + size);
+LLDB_LOGF(log,
+ "AddMemoryList %zu/%zu reading memory for region (%zu bytes) "
+ "[%zx, %zx)",
+ region_index, ranges.size(), size, addr, addr + size);
++region_index;
const size_t bytes_read =
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
index 178318a2d6..441eef388b 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -46,7 +46,8 @@ lldb_private::Status WriteString(const std::string &to_write,
/// the data on heap.
class MinidumpFileBuilder {
public:
- MinidumpFileBuilder(lldb::FileUP&& core_file):
m_core_file(std::move(core_file)) {};
+ MinidumpFileBuilder(lldb::FileUP &&core_file)
+ : m_core_file(std::move(core_file)) {};
MinidumpFileBuilder(const MinidumpFileBuilder &) = delete;
MinidumpFileBuilder &operator=(const MinidumpFileBuilder &) = delete;
``
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][swig] Refactor callback typemaps and functions (PR #95318)
https://github.com/chelcassanova created
https://github.com/llvm/llvm-project/pull/95318
This commit refactors the typemaps and static functions used in the SWIG
typemaps and wrappers to be in their own SWIG files that are included in the
main `python.swig` file.
>From 51ea2794e3ea5369d5197103775476819154b019 Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova
Date: Wed, 12 Jun 2024 14:42:39 -0700
Subject: [PATCH] [lldb][swig] Refactor callback typemaps and functions
This commit refactors the typemaps and static functions used in the SWIG
typemaps and wrappers to be in their own SWIG files that are included in
the main `python.swig` file.
---
.../python/python-callbacks-typemaps.swig | 119 ++
.../python/python-callbacks-wrapper.swig | 101 +++
lldb/bindings/python/python-typemaps.swig | 116 -
lldb/bindings/python/python-wrapper.swig | 96 --
lldb/bindings/python/python.swig | 2 +
5 files changed, 222 insertions(+), 212 deletions(-)
create mode 100644 lldb/bindings/python/python-callbacks-typemaps.swig
create mode 100644 lldb/bindings/python/python-callbacks-wrapper.swig
diff --git a/lldb/bindings/python/python-callbacks-typemaps.swig
b/lldb/bindings/python/python-callbacks-typemaps.swig
new file mode 100644
index 0..71f01c1144557
--- /dev/null
+++ b/lldb/bindings/python/python-callbacks-typemaps.swig
@@ -0,0 +1,119 @@
+/*
+ Typemaps specific to callback functions in LLDB. If editing this file
+ use the Python C API to access Python objects instead of using
PythonDataObjects.
+*/
+
+// For Log::LogOutputCallback
+%typemap(in) (lldb::LogOutputCallback log_callback, void *baton) {
+ if (!($input == Py_None ||
+PyCallable_Check(reinterpret_cast($input {
+PyErr_SetString(PyExc_TypeError, "Need a callable object or None!");
+SWIG_fail;
+ }
+
+ // FIXME (filcab): We can't currently check if our callback is already
+ // LLDBSwigPythonCallPythonLogOutputCallback (to DECREF the previous
+ // baton) nor can we just remove all traces of a callback, if we want to
+ // revert to a file logging mechanism.
+
+ // Don't lose the callback reference
+ Py_INCREF($input);
+ $1 = LLDBSwigPythonCallPythonLogOutputCallback;
+ $2 = $input;
+}
+
+%typemap(typecheck) (lldb::LogOutputCallback log_callback, void *baton) {
+ $1 = $input == Py_None;
+ $1 = $1 || PyCallable_Check(reinterpret_cast($input));
+}
+
+// For lldb::SBDebuggerDestroyCallback
+%typemap(in) (lldb::SBDebuggerDestroyCallback destroy_callback, void *baton) {
+ if (!($input == Py_None ||
+PyCallable_Check(reinterpret_cast($input {
+PyErr_SetString(PyExc_TypeError, "Need a callable object or None!");
+SWIG_fail;
+ }
+
+ // FIXME (filcab): We can't currently check if our callback is already
+ // LLDBSwigPythonCallPythonSBDebuggerTerminateCallback (to DECREF the
previous
+ // baton) nor can we just remove all traces of a callback, if we want to
+ // revert to a file logging mechanism.
+
+ // Don't lose the callback reference
+ Py_INCREF($input);
+ $1 = LLDBSwigPythonCallPythonSBDebuggerTerminateCallback;
+ $2 = $input;
+}
+
+%typemap(typecheck) (lldb::SBDebuggerDestroyCallback destroy_callback, void
*baton) {
+ $1 = $input == Py_None;
+ $1 = $1 || PyCallable_Check(reinterpret_cast($input));
+}
+
+%typemap(in) (lldb::CommandOverrideCallback callback, void *baton) {
+ if (!($input == Py_None ||
+PyCallable_Check(reinterpret_cast($input {
+PyErr_SetString(PyExc_TypeError, "Need a callable object or None!");
+SWIG_fail;
+ }
+
+ // Don't lose the callback reference
+ Py_INCREF($input);
+ $1 = LLDBSwigPythonCallPythonSBCommandInterpreterSetCommandOverrideCallback;
+ $2 = $input;
+}
+%typemap(typecheck) (lldb::CommandOverrideCallback callback, void *baton) {
+ $1 = $input == Py_None;
+ $1 = $1 || PyCallable_Check(reinterpret_cast($input));
+}
+// For lldb::SBPlatformLocateModuleCallback
+%typemap(in) (lldb::SBPlatformLocateModuleCallback callback,
+ void *callback_baton) {
+ if (!($input == Py_None ||
+PyCallable_Check(reinterpret_cast($input {
+PyErr_SetString(PyExc_TypeError, "Need a callable object or None!");
+SWIG_fail;
+ }
+
+ if ($input == Py_None) {
+$1 = nullptr;
+$2 = nullptr;
+ } else {
+PythonCallable callable = Retain($input);
+if (!callable.IsValid()) {
+ PyErr_SetString(PyExc_TypeError, "Need a valid callable object");
+ SWIG_fail;
+}
+
+llvm::Expected arg_info = callable.GetArgInfo();
+if (!arg_info) {
+ PyErr_SetString(PyExc_TypeError,
+ ("Could not get arguments: " +
+ llvm::toString(arg_info.takeError())).c_str());
+ SWIG_fail;
+}
+
+if (arg_info.get().max_positional_args != 3) {
+ PyErr_SetString(PyExc_TypeError, "Expected 3 argument callable object");
+ SWIG_fail;
+}
+
+// NOTE: When this is calle
[Lldb-commits] [lldb] [lldb][swig] Refactor callback typemaps and functions (PR #95318)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Chelsea Cassanova (chelcassanova)
Changes
This commit refactors the typemaps and static functions used in the SWIG
typemaps and wrappers to be in their own SWIG files that are included in the
main `python.swig` file.
---
Full diff: https://github.com/llvm/llvm-project/pull/95318.diff
5 Files Affected:
- (added) lldb/bindings/python/python-callbacks-typemaps.swig (+119)
- (added) lldb/bindings/python/python-callbacks-wrapper.swig (+101)
- (modified) lldb/bindings/python/python-typemaps.swig (-116)
- (modified) lldb/bindings/python/python-wrapper.swig (-96)
- (modified) lldb/bindings/python/python.swig (+2)
``diff
diff --git a/lldb/bindings/python/python-callbacks-typemaps.swig
b/lldb/bindings/python/python-callbacks-typemaps.swig
new file mode 100644
index 0..71f01c1144557
--- /dev/null
+++ b/lldb/bindings/python/python-callbacks-typemaps.swig
@@ -0,0 +1,119 @@
+/*
+ Typemaps specific to callback functions in LLDB. If editing this file
+ use the Python C API to access Python objects instead of using
PythonDataObjects.
+*/
+
+// For Log::LogOutputCallback
+%typemap(in) (lldb::LogOutputCallback log_callback, void *baton) {
+ if (!($input == Py_None ||
+PyCallable_Check(reinterpret_cast($input {
+PyErr_SetString(PyExc_TypeError, "Need a callable object or None!");
+SWIG_fail;
+ }
+
+ // FIXME (filcab): We can't currently check if our callback is already
+ // LLDBSwigPythonCallPythonLogOutputCallback (to DECREF the previous
+ // baton) nor can we just remove all traces of a callback, if we want to
+ // revert to a file logging mechanism.
+
+ // Don't lose the callback reference
+ Py_INCREF($input);
+ $1 = LLDBSwigPythonCallPythonLogOutputCallback;
+ $2 = $input;
+}
+
+%typemap(typecheck) (lldb::LogOutputCallback log_callback, void *baton) {
+ $1 = $input == Py_None;
+ $1 = $1 || PyCallable_Check(reinterpret_cast($input));
+}
+
+// For lldb::SBDebuggerDestroyCallback
+%typemap(in) (lldb::SBDebuggerDestroyCallback destroy_callback, void *baton) {
+ if (!($input == Py_None ||
+PyCallable_Check(reinterpret_cast($input {
+PyErr_SetString(PyExc_TypeError, "Need a callable object or None!");
+SWIG_fail;
+ }
+
+ // FIXME (filcab): We can't currently check if our callback is already
+ // LLDBSwigPythonCallPythonSBDebuggerTerminateCallback (to DECREF the
previous
+ // baton) nor can we just remove all traces of a callback, if we want to
+ // revert to a file logging mechanism.
+
+ // Don't lose the callback reference
+ Py_INCREF($input);
+ $1 = LLDBSwigPythonCallPythonSBDebuggerTerminateCallback;
+ $2 = $input;
+}
+
+%typemap(typecheck) (lldb::SBDebuggerDestroyCallback destroy_callback, void
*baton) {
+ $1 = $input == Py_None;
+ $1 = $1 || PyCallable_Check(reinterpret_cast($input));
+}
+
+%typemap(in) (lldb::CommandOverrideCallback callback, void *baton) {
+ if (!($input == Py_None ||
+PyCallable_Check(reinterpret_cast($input {
+PyErr_SetString(PyExc_TypeError, "Need a callable object or None!");
+SWIG_fail;
+ }
+
+ // Don't lose the callback reference
+ Py_INCREF($input);
+ $1 = LLDBSwigPythonCallPythonSBCommandInterpreterSetCommandOverrideCallback;
+ $2 = $input;
+}
+%typemap(typecheck) (lldb::CommandOverrideCallback callback, void *baton) {
+ $1 = $input == Py_None;
+ $1 = $1 || PyCallable_Check(reinterpret_cast($input));
+}
+// For lldb::SBPlatformLocateModuleCallback
+%typemap(in) (lldb::SBPlatformLocateModuleCallback callback,
+ void *callback_baton) {
+ if (!($input == Py_None ||
+PyCallable_Check(reinterpret_cast($input {
+PyErr_SetString(PyExc_TypeError, "Need a callable object or None!");
+SWIG_fail;
+ }
+
+ if ($input == Py_None) {
+$1 = nullptr;
+$2 = nullptr;
+ } else {
+PythonCallable callable = Retain($input);
+if (!callable.IsValid()) {
+ PyErr_SetString(PyExc_TypeError, "Need a valid callable object");
+ SWIG_fail;
+}
+
+llvm::Expected arg_info = callable.GetArgInfo();
+if (!arg_info) {
+ PyErr_SetString(PyExc_TypeError,
+ ("Could not get arguments: " +
+ llvm::toString(arg_info.takeError())).c_str());
+ SWIG_fail;
+}
+
+if (arg_info.get().max_positional_args != 3) {
+ PyErr_SetString(PyExc_TypeError, "Expected 3 argument callable object");
+ SWIG_fail;
+}
+
+// NOTE: When this is called multiple times, this will leak the Python
+// callable object as other callbacks, because this does not call Py_DECREF
+// the object. But it should be almost zero impact since this method is
+// expected to be called only once.
+
+// Don't lose the callback reference
+Py_INCREF($input);
+
+$1 = LLDBSwigPythonCallLocateModuleCallback;
+$2 = $input;
+ }
+}
+
+%typemap(typecheck) (lldb::SBPlatformLocateModuleCallback callback,
+ void *ca
[Lldb-commits] [lldb] [lldb][swig] Refactor callback typemaps and functions (PR #95318)
https://github.com/medismailben requested changes to this pull request. https://github.com/llvm/llvm-project/pull/95318 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][swig] Refactor callback typemaps and functions (PR #95318)
https://github.com/medismailben edited https://github.com/llvm/llvm-project/pull/95318 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][swig] Refactor callback typemaps and functions (PR #95318)
@@ -0,0 +1,119 @@ +/* + Typemaps specific to callback functions in LLDB. If editing this file + use the Python C API to access Python objects instead of using PythonDataObjects. medismailben wrote: nit: it would be nice to put the Python C API documentation link. https://github.com/llvm/llvm-project/pull/95318 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][swig] Refactor callback typemaps and functions (PR #95318)
@@ -0,0 +1,119 @@
+/*
+ Typemaps specific to callback functions in LLDB. If editing this file
+ use the Python C API to access Python objects instead of using
PythonDataObjects.
+*/
+
+// For Log::LogOutputCallback
+%typemap(in) (lldb::LogOutputCallback log_callback, void *baton) {
+ if (!($input == Py_None ||
+PyCallable_Check(reinterpret_cast($input {
+PyErr_SetString(PyExc_TypeError, "Need a callable object or None!");
+SWIG_fail;
+ }
+
+ // FIXME (filcab): We can't currently check if our callback is already
+ // LLDBSwigPythonCallPythonLogOutputCallback (to DECREF the previous
+ // baton) nor can we just remove all traces of a callback, if we want to
+ // revert to a file logging mechanism.
+
+ // Don't lose the callback reference
+ Py_INCREF($input);
+ $1 = LLDBSwigPythonCallPythonLogOutputCallback;
+ $2 = $input;
+}
+
+%typemap(typecheck) (lldb::LogOutputCallback log_callback, void *baton) {
+ $1 = $input == Py_None;
+ $1 = $1 || PyCallable_Check(reinterpret_cast($input));
+}
+
+// For lldb::SBDebuggerDestroyCallback
+%typemap(in) (lldb::SBDebuggerDestroyCallback destroy_callback, void *baton) {
+ if (!($input == Py_None ||
+PyCallable_Check(reinterpret_cast($input {
+PyErr_SetString(PyExc_TypeError, "Need a callable object or None!");
+SWIG_fail;
+ }
+
+ // FIXME (filcab): We can't currently check if our callback is already
+ // LLDBSwigPythonCallPythonSBDebuggerTerminateCallback (to DECREF the
previous
+ // baton) nor can we just remove all traces of a callback, if we want to
+ // revert to a file logging mechanism.
+
+ // Don't lose the callback reference
+ Py_INCREF($input);
+ $1 = LLDBSwigPythonCallPythonSBDebuggerTerminateCallback;
+ $2 = $input;
+}
+
+%typemap(typecheck) (lldb::SBDebuggerDestroyCallback destroy_callback, void
*baton) {
+ $1 = $input == Py_None;
+ $1 = $1 || PyCallable_Check(reinterpret_cast($input));
+}
+
+%typemap(in) (lldb::CommandOverrideCallback callback, void *baton) {
+ if (!($input == Py_None ||
+PyCallable_Check(reinterpret_cast($input {
+PyErr_SetString(PyExc_TypeError, "Need a callable object or None!");
+SWIG_fail;
+ }
+
+ // Don't lose the callback reference
+ Py_INCREF($input);
+ $1 = LLDBSwigPythonCallPythonSBCommandInterpreterSetCommandOverrideCallback;
+ $2 = $input;
+}
+%typemap(typecheck) (lldb::CommandOverrideCallback callback, void *baton) {
+ $1 = $input == Py_None;
+ $1 = $1 || PyCallable_Check(reinterpret_cast($input));
+}
+// For lldb::SBPlatformLocateModuleCallback
+%typemap(in) (lldb::SBPlatformLocateModuleCallback callback,
+ void *callback_baton) {
+ if (!($input == Py_None ||
+PyCallable_Check(reinterpret_cast($input {
+PyErr_SetString(PyExc_TypeError, "Need a callable object or None!");
+SWIG_fail;
+ }
+
+ if ($input == Py_None) {
+$1 = nullptr;
+$2 = nullptr;
+ } else {
+PythonCallable callable = Retain($input);
+if (!callable.IsValid()) {
+ PyErr_SetString(PyExc_TypeError, "Need a valid callable object");
+ SWIG_fail;
+}
+
+llvm::Expected arg_info = callable.GetArgInfo();
medismailben wrote:
Also you can't use llvm here.
https://github.com/llvm/llvm-project/pull/95318
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][swig] Refactor callback typemaps and functions (PR #95318)
@@ -0,0 +1,119 @@
+/*
+ Typemaps specific to callback functions in LLDB. If editing this file
+ use the Python C API to access Python objects instead of using
PythonDataObjects.
+*/
+
+// For Log::LogOutputCallback
+%typemap(in) (lldb::LogOutputCallback log_callback, void *baton) {
+ if (!($input == Py_None ||
+PyCallable_Check(reinterpret_cast($input {
+PyErr_SetString(PyExc_TypeError, "Need a callable object or None!");
+SWIG_fail;
+ }
+
+ // FIXME (filcab): We can't currently check if our callback is already
+ // LLDBSwigPythonCallPythonLogOutputCallback (to DECREF the previous
+ // baton) nor can we just remove all traces of a callback, if we want to
+ // revert to a file logging mechanism.
+
+ // Don't lose the callback reference
+ Py_INCREF($input);
+ $1 = LLDBSwigPythonCallPythonLogOutputCallback;
+ $2 = $input;
+}
+
+%typemap(typecheck) (lldb::LogOutputCallback log_callback, void *baton) {
+ $1 = $input == Py_None;
+ $1 = $1 || PyCallable_Check(reinterpret_cast($input));
+}
+
+// For lldb::SBDebuggerDestroyCallback
+%typemap(in) (lldb::SBDebuggerDestroyCallback destroy_callback, void *baton) {
+ if (!($input == Py_None ||
+PyCallable_Check(reinterpret_cast($input {
+PyErr_SetString(PyExc_TypeError, "Need a callable object or None!");
+SWIG_fail;
+ }
+
+ // FIXME (filcab): We can't currently check if our callback is already
+ // LLDBSwigPythonCallPythonSBDebuggerTerminateCallback (to DECREF the
previous
+ // baton) nor can we just remove all traces of a callback, if we want to
+ // revert to a file logging mechanism.
+
+ // Don't lose the callback reference
+ Py_INCREF($input);
+ $1 = LLDBSwigPythonCallPythonSBDebuggerTerminateCallback;
+ $2 = $input;
+}
+
+%typemap(typecheck) (lldb::SBDebuggerDestroyCallback destroy_callback, void
*baton) {
+ $1 = $input == Py_None;
+ $1 = $1 || PyCallable_Check(reinterpret_cast($input));
+}
+
+%typemap(in) (lldb::CommandOverrideCallback callback, void *baton) {
+ if (!($input == Py_None ||
+PyCallable_Check(reinterpret_cast($input {
+PyErr_SetString(PyExc_TypeError, "Need a callable object or None!");
+SWIG_fail;
+ }
+
+ // Don't lose the callback reference
+ Py_INCREF($input);
+ $1 = LLDBSwigPythonCallPythonSBCommandInterpreterSetCommandOverrideCallback;
+ $2 = $input;
+}
+%typemap(typecheck) (lldb::CommandOverrideCallback callback, void *baton) {
+ $1 = $input == Py_None;
+ $1 = $1 || PyCallable_Check(reinterpret_cast($input));
+}
+// For lldb::SBPlatformLocateModuleCallback
+%typemap(in) (lldb::SBPlatformLocateModuleCallback callback,
+ void *callback_baton) {
+ if (!($input == Py_None ||
+PyCallable_Check(reinterpret_cast($input {
+PyErr_SetString(PyExc_TypeError, "Need a callable object or None!");
+SWIG_fail;
+ }
+
+ if ($input == Py_None) {
+$1 = nullptr;
+$2 = nullptr;
+ } else {
+PythonCallable callable = Retain($input);
medismailben wrote:
This is a PythonDataObject type
https://github.com/llvm/llvm-project/pull/95318
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 45927d7 - [lldb] Skip delay-init test when run on pre-macOS 15
Author: Jason Molenda Date: 2024-06-12T15:58:03-07:00 New Revision: 45927d730bcd2aa3380834ca8db96e32a8b2f2b1 URL: https://github.com/llvm/llvm-project/commit/45927d730bcd2aa3380834ca8db96e32a8b2f2b1 DIFF: https://github.com/llvm/llvm-project/commit/45927d730bcd2aa3380834ca8db96e32a8b2f2b1.diff LOG: [lldb] Skip delay-init test when run on pre-macOS 15 The test has a check that the static linker supports the new option, but it assumed the Xcode 16 linker also meant it was running on macOS 15 and the dynamic linker would honor dependencies flagged this way. But Xcode 16 can be run on macOS 14.5, so we need to skip the test in that combination. Added: Modified: lldb/test/API/macosx/delay-init-dependency/TestDelayInitDependency.py Removed: diff --git a/lldb/test/API/macosx/delay-init-dependency/TestDelayInitDependency.py b/lldb/test/API/macosx/delay-init-dependency/TestDelayInitDependency.py index 44ed2b1d21f18..090a3554a9207 100644 --- a/lldb/test/API/macosx/delay-init-dependency/TestDelayInitDependency.py +++ b/lldb/test/API/macosx/delay-init-dependency/TestDelayInitDependency.py @@ -11,6 +11,7 @@ class TestDelayInitDependencies(TestBase): NO_DEBUG_INFO_TESTCASE = True @skipUnlessDarwin +@skipIf(macos_version=["<", "15.0"]) def test_delay_init_dependency(self): TestBase.setUp(self) out = subprocess.run( ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][swig] Refactor callback typemaps and functions (PR #95318)
https://github.com/bulbazord commented: Is this just moving code around? The summary says what you're doing but not why, what's the motivation? https://github.com/llvm/llvm-project/pull/95318 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
https://github.com/clayborg edited https://github.com/llvm/llvm-project/pull/95312 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
https://github.com/clayborg requested changes to this pull request. https://github.com/llvm/llvm-project/pull/95312 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -797,20 +822,75 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
}
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP &core_file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemory(const ProcessSP &process_sp,
+ SaveCoreStyle core_style) {
+ Status error;
+
+ Process::CoreFileMemoryRanges ranges_for_memory_list;
+ error = process_sp->CalculateCoreFileSaveRanges(
+ SaveCoreStyle::eSaveCoreStackOnly, ranges_for_memory_list);
+ if (error.Fail()) {
+return error;
+ }
+
+ std::set stack_ranges;
+ for (const auto &core_range : ranges_for_memory_list) {
+stack_ranges.insert(core_range.range.start());
+ }
+ // We leave a little padding for dictionary and any other metadata we would
+ // want. Also so that we can put the header of the memory list 64 in 32b
land,
+ // because the directory requires a 32b RVA.
+ Process::CoreFileMemoryRanges ranges;
+ error = process_sp->CalculateCoreFileSaveRanges(core_style, ranges);
+ if (error.Fail()) {
+return error;
+ }
clayborg wrote:
single line if statements should have not `{}`:
```
if (error.Fail())
return error;
```
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -797,20 +822,75 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
}
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP &core_file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemory(const ProcessSP &process_sp,
+ SaveCoreStyle core_style) {
+ Status error;
+
+ Process::CoreFileMemoryRanges ranges_for_memory_list;
+ error = process_sp->CalculateCoreFileSaveRanges(
+ SaveCoreStyle::eSaveCoreStackOnly, ranges_for_memory_list);
+ if (error.Fail()) {
+return error;
+ }
+
+ std::set stack_ranges;
+ for (const auto &core_range : ranges_for_memory_list) {
+stack_ranges.insert(core_range.range.start());
+ }
+ // We leave a little padding for dictionary and any other metadata we would
+ // want. Also so that we can put the header of the memory list 64 in 32b
land,
+ // because the directory requires a 32b RVA.
+ Process::CoreFileMemoryRanges ranges;
+ error = process_sp->CalculateCoreFileSaveRanges(core_style, ranges);
+ if (error.Fail()) {
+return error;
+ }
+
+ uint64_t total_size =
+ 256 + (ranges.size() * sizeof(llvm::minidump::MemoryDescriptor_64));
+ // Take all the memory that will fit in the 32b range.
+ for (int i = ranges.size() - 1; i >= 0; i--) {
clayborg wrote:
Why are we iterating over the ranges in reverse order? It might be easier to
iterate over the ranges in the normal order and then adding the right ranges to
either the `ranges_32` or `ranges_64` list as needed? So this could be just:
```
for (const auto &range: all_ranges) {
lldb::offset_t range_32_size = range.range.size() +
sizeof(llvm::minidump::MemoryDescriptor);
if (stack_start_addresses.count(ranges[i].range.start()) > 0)
continue; // Stack range that was already added to range_32
if (total_size + range_32_size < UINT32_MAX) {
ranges_32.push_back(range);
total_size += range_32_size;
} else {
ranges_64.push_back(range);
}
}
```
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -797,20 +822,75 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
}
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP &core_file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemory(const ProcessSP &process_sp,
+ SaveCoreStyle core_style) {
+ Status error;
+
+ Process::CoreFileMemoryRanges ranges_for_memory_list;
clayborg wrote:
It might be useful to create two Process::CoreFileMemoryRanges lists: one for
32 bit and one for 64 bit and have a variable that keeps track of what was
already added:
```
uint64_t ranges_32_size = 0;
Process::CoreFileMemoryRanges ranges_32;
Process::CoreFileMemoryRanges ranges_64;
```
Then we can add regions to each list as needed below without having to remove
anything from the `ranges` list below?
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -59,39 +68,67 @@ class MinidumpFileBuilder {
// Add ThreadList stream, containing information about all threads running
// at the moment of core saving. Contains information about thread
// contexts.
- lldb_private::Status AddThreadList(const lldb::ProcessSP &process_sp);
- // Add Exception streams for any threads that stopped with exceptions.
- void AddExceptions(const lldb::ProcessSP &process_sp);
- // Add MemoryList stream, containing dumps of important memory segments
- lldb_private::Status AddMemoryList(const lldb::ProcessSP &process_sp,
- lldb::SaveCoreStyle core_style);
// Add MiscInfo stream, mainly providing ProcessId
void AddMiscInfo(const lldb::ProcessSP &process_sp);
// Add informative files about a Linux process
void AddLinuxFileStreams(const lldb::ProcessSP &process_sp);
+ // Add Exception streams for any threads that stopped with exceptions.
+ void AddExceptions(const lldb::ProcessSP &process_sp);
clayborg wrote:
Revert to be the same as the lines removed from above to minimize changes to
this file
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -40,7 +46,7 @@ lldb_private::Status WriteString(const std::string &to_write,
/// the data on heap.
class MinidumpFileBuilder {
public:
- MinidumpFileBuilder() = default;
+ MinidumpFileBuilder(lldb::FileUP&& core_file):
m_core_file(std::move(core_file)) {};
clayborg wrote:
We should a `const lldb::ProcessSP &process_sp` to the constructor and remove
all `const lldb::ProcessSP &process_sp` arguemnts from the methods below. We
never want to change processes while creating a minidump. We should store the
process shared pointer as an instance variable and then the methods that used
to take the `const lldb::ProcessSP &process_sp` as an argument now use the
instance variable:
```
lldb::ProcessSP m_process_sp;
```
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -797,20 +822,75 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
}
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP &core_file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemory(const ProcessSP &process_sp,
+ SaveCoreStyle core_style) {
+ Status error;
+
+ Process::CoreFileMemoryRanges ranges_for_memory_list;
+ error = process_sp->CalculateCoreFileSaveRanges(
+ SaveCoreStyle::eSaveCoreStackOnly, ranges_for_memory_list);
+ if (error.Fail()) {
+return error;
+ }
+
+ std::set stack_ranges;
+ for (const auto &core_range : ranges_for_memory_list) {
+stack_ranges.insert(core_range.range.start());
+ }
+ // We leave a little padding for dictionary and any other metadata we would
+ // want. Also so that we can put the header of the memory list 64 in 32b
land,
+ // because the directory requires a 32b RVA.
+ Process::CoreFileMemoryRanges ranges;
+ error = process_sp->CalculateCoreFileSaveRanges(core_style, ranges);
+ if (error.Fail()) {
+return error;
+ }
+
+ uint64_t total_size =
+ 256 + (ranges.size() * sizeof(llvm::minidump::MemoryDescriptor_64));
clayborg wrote:
What is 256 here? And why do we care about `ranges.size() *
sizeof(llvm::minidump::MemoryDescriptor_64))`?
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -59,39 +68,67 @@ class MinidumpFileBuilder {
// Add ThreadList stream, containing information about all threads running
// at the moment of core saving. Contains information about thread
// contexts.
- lldb_private::Status AddThreadList(const lldb::ProcessSP &process_sp);
- // Add Exception streams for any threads that stopped with exceptions.
- void AddExceptions(const lldb::ProcessSP &process_sp);
- // Add MemoryList stream, containing dumps of important memory segments
- lldb_private::Status AddMemoryList(const lldb::ProcessSP &process_sp,
- lldb::SaveCoreStyle core_style);
// Add MiscInfo stream, mainly providing ProcessId
void AddMiscInfo(const lldb::ProcessSP &process_sp);
// Add informative files about a Linux process
void AddLinuxFileStreams(const lldb::ProcessSP &process_sp);
+ // Add Exception streams for any threads that stopped with exceptions.
+ void AddExceptions(const lldb::ProcessSP &process_sp);
// Dump the prepared data into file. In case of the failure data are
// intact.
- lldb_private::Status Dump(lldb::FileUP &core_file) const;
- // Returns the current number of directories(streams) that have been so far
- // created. This number of directories will be dumped when calling Dump()
- size_t GetDirectoriesNum() const;
+ lldb_private::Status AddThreadList(const lldb::ProcessSP &process_sp);
+
clayborg wrote:
Revert these two lines and use the old ones that were removed above.
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -797,20 +822,75 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
}
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP &core_file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemory(const ProcessSP &process_sp,
+ SaveCoreStyle core_style) {
+ Status error;
+
+ Process::CoreFileMemoryRanges ranges_for_memory_list;
+ error = process_sp->CalculateCoreFileSaveRanges(
+ SaveCoreStyle::eSaveCoreStackOnly, ranges_for_memory_list);
+ if (error.Fail()) {
+return error;
+ }
+
+ std::set stack_ranges;
+ for (const auto &core_range : ranges_for_memory_list) {
+stack_ranges.insert(core_range.range.start());
+ }
clayborg wrote:
remove `{}` from single line `for` statement and use `ranges_32` and rename
`stack_ranges` to `stack_start_addresses` since that is what we are storing:
```
std::set stack_start_addresses;
for (const auto &core_range : ranges_32)
stack_start_addresses.insert(core_range.range.start());
```
We also need to make sure the current `ranges_32` doesn't exceed 4GB or we must
error out since the thread stacks must be under 4GB. Or we can avoid fixing the
stack memory descriptor for any threads that exceed the 4GB barrier
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -797,20 +822,75 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
}
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP &core_file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemory(const ProcessSP &process_sp,
+ SaveCoreStyle core_style) {
+ Status error;
+
+ Process::CoreFileMemoryRanges ranges_for_memory_list;
+ error = process_sp->CalculateCoreFileSaveRanges(
+ SaveCoreStyle::eSaveCoreStackOnly, ranges_for_memory_list);
+ if (error.Fail()) {
+return error;
+ }
+
+ std::set stack_ranges;
+ for (const auto &core_range : ranges_for_memory_list) {
+stack_ranges.insert(core_range.range.start());
+ }
+ // We leave a little padding for dictionary and any other metadata we would
+ // want. Also so that we can put the header of the memory list 64 in 32b
land,
+ // because the directory requires a 32b RVA.
+ Process::CoreFileMemoryRanges ranges;
+ error = process_sp->CalculateCoreFileSaveRanges(core_style, ranges);
+ if (error.Fail()) {
+return error;
+ }
+
+ uint64_t total_size =
+ 256 + (ranges.size() * sizeof(llvm::minidump::MemoryDescriptor_64));
+ // Take all the memory that will fit in the 32b range.
+ for (int i = ranges.size() - 1; i >= 0; i--) {
+addr_t size_to_add =
+ranges[i].range.size() + sizeof(llvm::minidump::MemoryDescriptor);
+// filter out the stacks and check if it's below 32b max.
+if (stack_ranges.count(ranges[i].range.start()) > 0) {
+ ranges.erase(ranges.begin() + i);
+} else if (total_size + size_to_add < UINT32_MAX) {
+ ranges_for_memory_list.push_back(ranges[i]);
+ total_size += ranges[i].range.size();
+ total_size += sizeof(llvm::minidump::MemoryDescriptor);
+ ranges.erase(ranges.begin() + i);
+} else {
+ break;
+}
+ }
+ error = AddMemoryList_32(process_sp, ranges_for_memory_list);
+ if (error.Fail())
+return error;
+
+ // Add the remaining memory as a 64b range.
+ if (ranges.size() > 0) {
+error = AddMemoryList_64(process_sp, ranges);
clayborg wrote:
```
if (!ranges_64.empty()) {
error = AddMemoryList_64(process_sp, ranges_64);
```
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -59,39 +68,67 @@ class MinidumpFileBuilder {
// Add ThreadList stream, containing information about all threads running
// at the moment of core saving. Contains information about thread
// contexts.
- lldb_private::Status AddThreadList(const lldb::ProcessSP &process_sp);
- // Add Exception streams for any threads that stopped with exceptions.
- void AddExceptions(const lldb::ProcessSP &process_sp);
clayborg wrote:
These haven't changed at all, so I would move them back to this line. No need
to make changes to this header file if things haven't changed (`AddThreadList`
and `AddExceptions`). So make as much of this the same as it was before by
reverting the above 3 lines and removing them from below.
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -797,20 +822,75 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
}
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP &core_file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemory(const ProcessSP &process_sp,
+ SaveCoreStyle core_style) {
+ Status error;
+
+ Process::CoreFileMemoryRanges ranges_for_memory_list;
+ error = process_sp->CalculateCoreFileSaveRanges(
+ SaveCoreStyle::eSaveCoreStackOnly, ranges_for_memory_list);
+ if (error.Fail()) {
+return error;
+ }
+
+ std::set stack_ranges;
+ for (const auto &core_range : ranges_for_memory_list) {
+stack_ranges.insert(core_range.range.start());
+ }
+ // We leave a little padding for dictionary and any other metadata we would
+ // want. Also so that we can put the header of the memory list 64 in 32b
land,
+ // because the directory requires a 32b RVA.
clayborg wrote:
These 3 lines of comments seem out of place for the code that comes beneath it,
place closer to the code that enforces this
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -59,39 +68,67 @@ class MinidumpFileBuilder {
// Add ThreadList stream, containing information about all threads running
// at the moment of core saving. Contains information about thread
// contexts.
- lldb_private::Status AddThreadList(const lldb::ProcessSP &process_sp);
- // Add Exception streams for any threads that stopped with exceptions.
- void AddExceptions(const lldb::ProcessSP &process_sp);
- // Add MemoryList stream, containing dumps of important memory segments
- lldb_private::Status AddMemoryList(const lldb::ProcessSP &process_sp,
- lldb::SaveCoreStyle core_style);
// Add MiscInfo stream, mainly providing ProcessId
void AddMiscInfo(const lldb::ProcessSP &process_sp);
// Add informative files about a Linux process
void AddLinuxFileStreams(const lldb::ProcessSP &process_sp);
+ // Add Exception streams for any threads that stopped with exceptions.
+ void AddExceptions(const lldb::ProcessSP &process_sp);
// Dump the prepared data into file. In case of the failure data are
// intact.
- lldb_private::Status Dump(lldb::FileUP &core_file) const;
- // Returns the current number of directories(streams) that have been so far
- // created. This number of directories will be dumped when calling Dump()
- size_t GetDirectoriesNum() const;
+ lldb_private::Status AddThreadList(const lldb::ProcessSP &process_sp);
+
+ lldb_private::Status AddMemory(const lldb::ProcessSP &process_sp,
+ lldb::SaveCoreStyle core_style);
+
+ lldb_private::Status DumpToFile();
private:
+ // Add data to the end of the buffer, if the buffer exceeds the flush level,
+ // trigger a flush.
+ lldb_private::Status AddData(const void *data, size_t size);
+ // Add MemoryList stream, containing dumps of important memory segments
+ lldb_private::Status
+ AddMemoryList_64(const lldb::ProcessSP &process_sp,
+ const lldb_private::Process::CoreFileMemoryRanges &ranges);
+ lldb_private::Status
+ AddMemoryList_32(const lldb::ProcessSP &process_sp,
+ const lldb_private::Process::CoreFileMemoryRanges &ranges);
+ lldb_private::Status FixThreads();
+ lldb_private::Status FlushToDisk();
+
+ lldb_private::Status DumpHeader() const;
+ lldb_private::Status DumpDirectories() const;
+ bool CheckIf_64Bit(const size_t size);
// Add directory of StreamType pointing to the current end of the prepared
// file with the specified size.
- void AddDirectory(llvm::minidump::StreamType type, size_t stream_size);
- size_t GetCurrentDataEndOffset() const;
-
- // Stores directories to later put them at the end of minidump file
+ void AddDirectory(llvm::minidump::StreamType type, uint64_t stream_size);
+ lldb::addr_t GetCurrentDataEndOffset() const;
clayborg wrote:
return `lldb::offset_t` instead of `lldb::addr_t`.
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
@@ -797,20 +822,75 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
}
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP &core_file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemory(const ProcessSP &process_sp,
+ SaveCoreStyle core_style) {
+ Status error;
+
+ Process::CoreFileMemoryRanges ranges_for_memory_list;
+ error = process_sp->CalculateCoreFileSaveRanges(
+ SaveCoreStyle::eSaveCoreStackOnly, ranges_for_memory_list);
clayborg wrote:
We know stacks need to go into the `ranges_32` list so this could be:
```
// Thread stacks must use 32 bit memory ranges as that is all that
// the thread structure allows
error = process_sp->CalculateCoreFileSaveRanges(
SaveCoreStyle::eSaveCoreStackOnly, ranges_32);
```
https://github.com/llvm/llvm-project/pull/95312
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Support case-insensitive regex matches (PR #95350)
https://github.com/JDevlieghere created
https://github.com/llvm/llvm-project/pull/95350
Support case-insensitive regex matches for `SBTarget::FindGlobalFunctions` and
`SBTarget::FindGlobalVariables`.
>From a2a362aeaf3d091c04a2eaefc604962730aa483c Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere
Date: Wed, 12 Jun 2024 21:44:02 -0700
Subject: [PATCH] [lldb] Support case-insensitive regex matches
Support case-insensitive regex matches for SBTarget::FindGlobalFunctions
and SBTarget::FindGlobalVariables.
---
lldb/include/lldb/Utility/RegularExpression.h | 8 +++-
lldb/include/lldb/lldb-enumerations.h | 7 ++-
lldb/source/API/SBTarget.cpp | 10 ++
lldb/source/Utility/RegularExpression.cpp | 5 +++--
.../API/lang/cpp/class_static/TestStaticVariables.py | 4
5 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/lldb/include/lldb/Utility/RegularExpression.h
b/lldb/include/lldb/Utility/RegularExpression.h
index 415f1b58b1110..e8bd47bb53c29 100644
--- a/lldb/include/lldb/Utility/RegularExpression.h
+++ b/lldb/include/lldb/Utility/RegularExpression.h
@@ -31,7 +31,13 @@ class RegularExpression {
/// \param[in] string
/// An llvm::StringRef that represents the regular expression to compile.
// String is not referenced anymore after the object is constructed.
- explicit RegularExpression(llvm::StringRef string);
+ //
+ /// \param[in] flags
+ /// An llvm::Regex::RegexFlags that modifies the matching behavior. The
+ /// default is NoFlags.
+ explicit RegularExpression(
+ llvm::StringRef string,
+ llvm::Regex::RegexFlags flags = llvm::Regex::NoFlags);
~RegularExpression() = default;
diff --git a/lldb/include/lldb/lldb-enumerations.h
b/lldb/include/lldb/lldb-enumerations.h
index 8e05f6ba9c876..74ff458bf87de 100644
--- a/lldb/include/lldb/lldb-enumerations.h
+++ b/lldb/include/lldb/lldb-enumerations.h
@@ -1107,7 +1107,12 @@ enum MemberFunctionKind {
};
/// String matching algorithm used by SBTarget.
-enum MatchType { eMatchTypeNormal, eMatchTypeRegex, eMatchTypeStartsWith };
+enum MatchType {
+ eMatchTypeNormal,
+ eMatchTypeRegex,
+ eMatchTypeStartsWith,
+ eMatchTypeRegexInsensitive
+};
/// Bitmask that describes details about a type.
FLAGS_ENUM(TypeFlags){
diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index adb9e645610ba..2c336296f0b8d 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -1789,6 +1789,11 @@ lldb::SBSymbolContextList
SBTarget::FindGlobalFunctions(const char *name,
target_sp->GetImages().FindFunctions(RegularExpression(name_ref),
function_options, *sb_sc_list);
break;
+ case eMatchTypeRegexInsensitive:
+target_sp->GetImages().FindFunctions(
+RegularExpression(name_ref, llvm::Regex::RegexFlags::IgnoreCase),
+function_options, *sb_sc_list);
+break;
case eMatchTypeStartsWith:
regexstr = llvm::Regex::escape(name) + ".*";
target_sp->GetImages().FindFunctions(RegularExpression(regexstr),
@@ -1936,6 +1941,11 @@ SBValueList SBTarget::FindGlobalVariables(const char
*name,
target_sp->GetImages().FindGlobalVariables(RegularExpression(name_ref),
max_matches, variable_list);
break;
+case eMatchTypeRegexInsensitive:
+ target_sp->GetImages().FindGlobalVariables(
+ RegularExpression(name_ref, llvm::Regex::IgnoreCase), max_matches,
+ variable_list);
+ break;
case eMatchTypeStartsWith:
regexstr = "^" + llvm::Regex::escape(name) + ".*";
target_sp->GetImages().FindGlobalVariables(RegularExpression(regexstr),
diff --git a/lldb/source/Utility/RegularExpression.cpp
b/lldb/source/Utility/RegularExpression.cpp
index 20bebbfe15f27..026793462221c 100644
--- a/lldb/source/Utility/RegularExpression.cpp
+++ b/lldb/source/Utility/RegularExpression.cpp
@@ -12,10 +12,11 @@
using namespace lldb_private;
-RegularExpression::RegularExpression(llvm::StringRef str)
+RegularExpression::RegularExpression(llvm::StringRef str,
+ llvm::Regex::RegexFlags flags)
: m_regex_text(std::string(str)),
// m_regex does not reference str anymore after it is constructed.
- m_regex(llvm::Regex(str)) {}
+ m_regex(llvm::Regex(str, flags)) {}
RegularExpression::RegularExpression(const RegularExpression &rhs)
: RegularExpression(rhs.GetText()) {}
diff --git a/lldb/test/API/lang/cpp/class_static/TestStaticVariables.py
b/lldb/test/API/lang/cpp/class_static/TestStaticVariables.py
index 8211d532b2638..dea7bff3a3d03 100644
--- a/lldb/test/API/lang/cpp/class_static/TestStaticVariables.py
+++ b/lldb/test/API/lang/cpp/class_static/TestStaticVariables.py
@@ -263,6 +263,10 @@ def test_with_python_FindGlobalVariables(self):
self.a
[Lldb-commits] [lldb] [lldb] Support case-insensitive regex matches (PR #95350)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Jonas Devlieghere (JDevlieghere)
Changes
Support case-insensitive regex matches for `SBTarget::FindGlobalFunctions` and
`SBTarget::FindGlobalVariables`.
---
Full diff: https://github.com/llvm/llvm-project/pull/95350.diff
5 Files Affected:
- (modified) lldb/include/lldb/Utility/RegularExpression.h (+7-1)
- (modified) lldb/include/lldb/lldb-enumerations.h (+6-1)
- (modified) lldb/source/API/SBTarget.cpp (+10)
- (modified) lldb/source/Utility/RegularExpression.cpp (+3-2)
- (modified) lldb/test/API/lang/cpp/class_static/TestStaticVariables.py (+4)
``diff
diff --git a/lldb/include/lldb/Utility/RegularExpression.h
b/lldb/include/lldb/Utility/RegularExpression.h
index 415f1b58b1110..e8bd47bb53c29 100644
--- a/lldb/include/lldb/Utility/RegularExpression.h
+++ b/lldb/include/lldb/Utility/RegularExpression.h
@@ -31,7 +31,13 @@ class RegularExpression {
/// \param[in] string
/// An llvm::StringRef that represents the regular expression to compile.
// String is not referenced anymore after the object is constructed.
- explicit RegularExpression(llvm::StringRef string);
+ //
+ /// \param[in] flags
+ /// An llvm::Regex::RegexFlags that modifies the matching behavior. The
+ /// default is NoFlags.
+ explicit RegularExpression(
+ llvm::StringRef string,
+ llvm::Regex::RegexFlags flags = llvm::Regex::NoFlags);
~RegularExpression() = default;
diff --git a/lldb/include/lldb/lldb-enumerations.h
b/lldb/include/lldb/lldb-enumerations.h
index 8e05f6ba9c876..74ff458bf87de 100644
--- a/lldb/include/lldb/lldb-enumerations.h
+++ b/lldb/include/lldb/lldb-enumerations.h
@@ -1107,7 +1107,12 @@ enum MemberFunctionKind {
};
/// String matching algorithm used by SBTarget.
-enum MatchType { eMatchTypeNormal, eMatchTypeRegex, eMatchTypeStartsWith };
+enum MatchType {
+ eMatchTypeNormal,
+ eMatchTypeRegex,
+ eMatchTypeStartsWith,
+ eMatchTypeRegexInsensitive
+};
/// Bitmask that describes details about a type.
FLAGS_ENUM(TypeFlags){
diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index adb9e645610ba..2c336296f0b8d 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -1789,6 +1789,11 @@ lldb::SBSymbolContextList
SBTarget::FindGlobalFunctions(const char *name,
target_sp->GetImages().FindFunctions(RegularExpression(name_ref),
function_options, *sb_sc_list);
break;
+ case eMatchTypeRegexInsensitive:
+target_sp->GetImages().FindFunctions(
+RegularExpression(name_ref, llvm::Regex::RegexFlags::IgnoreCase),
+function_options, *sb_sc_list);
+break;
case eMatchTypeStartsWith:
regexstr = llvm::Regex::escape(name) + ".*";
target_sp->GetImages().FindFunctions(RegularExpression(regexstr),
@@ -1936,6 +1941,11 @@ SBValueList SBTarget::FindGlobalVariables(const char
*name,
target_sp->GetImages().FindGlobalVariables(RegularExpression(name_ref),
max_matches, variable_list);
break;
+case eMatchTypeRegexInsensitive:
+ target_sp->GetImages().FindGlobalVariables(
+ RegularExpression(name_ref, llvm::Regex::IgnoreCase), max_matches,
+ variable_list);
+ break;
case eMatchTypeStartsWith:
regexstr = "^" + llvm::Regex::escape(name) + ".*";
target_sp->GetImages().FindGlobalVariables(RegularExpression(regexstr),
diff --git a/lldb/source/Utility/RegularExpression.cpp
b/lldb/source/Utility/RegularExpression.cpp
index 20bebbfe15f27..026793462221c 100644
--- a/lldb/source/Utility/RegularExpression.cpp
+++ b/lldb/source/Utility/RegularExpression.cpp
@@ -12,10 +12,11 @@
using namespace lldb_private;
-RegularExpression::RegularExpression(llvm::StringRef str)
+RegularExpression::RegularExpression(llvm::StringRef str,
+ llvm::Regex::RegexFlags flags)
: m_regex_text(std::string(str)),
// m_regex does not reference str anymore after it is constructed.
- m_regex(llvm::Regex(str)) {}
+ m_regex(llvm::Regex(str, flags)) {}
RegularExpression::RegularExpression(const RegularExpression &rhs)
: RegularExpression(rhs.GetText()) {}
diff --git a/lldb/test/API/lang/cpp/class_static/TestStaticVariables.py
b/lldb/test/API/lang/cpp/class_static/TestStaticVariables.py
index 8211d532b2638..dea7bff3a3d03 100644
--- a/lldb/test/API/lang/cpp/class_static/TestStaticVariables.py
+++ b/lldb/test/API/lang/cpp/class_static/TestStaticVariables.py
@@ -263,6 +263,10 @@ def test_with_python_FindGlobalVariables(self):
self.assertTrue(found_a, "Regex search found A::g_points")
self.assertTrue(found_aa, "Regex search found AA::g_points")
+# Regex lowercase should find both as well.
+val_list = target.FindGlobalVariables("a::g_points", 10,
lldb.eMatchTypeRegexInsensitive)
[Lldb-commits] [lldb] [lldb] Support case-insensitive regex matches (PR #95350)
github-actions[bot] wrote:
:warning: Python code formatter, darker found issues in your code. :warning:
You can test this locally with the following command:
``bash
darker --check --diff -r
3fce14569fc3611eddca41db055143285244736a...a2a362aeaf3d091c04a2eaefc604962730aa483c
lldb/test/API/lang/cpp/class_static/TestStaticVariables.py
``
View the diff from darker here.
``diff
--- TestStaticVariables.py 2024-06-13 04:44:06.00 +
+++ TestStaticVariables.py 2024-06-13 04:48:00.279771 +
@@ -262,11 +262,13 @@
self.assertTrue(found_a, "Regex search found A::g_points")
self.assertTrue(found_aa, "Regex search found AA::g_points")
# Regex lowercase should find both as well.
-val_list = target.FindGlobalVariables("a::g_points", 10,
lldb.eMatchTypeRegexInsensitive)
+val_list = target.FindGlobalVariables(
+"a::g_points", 10, lldb.eMatchTypeRegexInsensitive
+)
self.assertEqual(val_list.GetSize(), 2, "Found A & AA")
# Normal search for full name should find one, but it looks like we
don't match
# on identifier boundaries here yet:
val_list = target.FindGlobalVariables("A::g_points", 10,
lldb.eMatchTypeNormal)
``
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] Support case-insensitive regex matches (PR #95350)
https://github.com/JDevlieghere updated
https://github.com/llvm/llvm-project/pull/95350
>From a2a362aeaf3d091c04a2eaefc604962730aa483c Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere
Date: Wed, 12 Jun 2024 21:44:02 -0700
Subject: [PATCH 1/2] [lldb] Support case-insensitive regex matches
Support case-insensitive regex matches for SBTarget::FindGlobalFunctions
and SBTarget::FindGlobalVariables.
---
lldb/include/lldb/Utility/RegularExpression.h | 8 +++-
lldb/include/lldb/lldb-enumerations.h | 7 ++-
lldb/source/API/SBTarget.cpp | 10 ++
lldb/source/Utility/RegularExpression.cpp | 5 +++--
.../API/lang/cpp/class_static/TestStaticVariables.py | 4
5 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/lldb/include/lldb/Utility/RegularExpression.h
b/lldb/include/lldb/Utility/RegularExpression.h
index 415f1b58b1110..e8bd47bb53c29 100644
--- a/lldb/include/lldb/Utility/RegularExpression.h
+++ b/lldb/include/lldb/Utility/RegularExpression.h
@@ -31,7 +31,13 @@ class RegularExpression {
/// \param[in] string
/// An llvm::StringRef that represents the regular expression to compile.
// String is not referenced anymore after the object is constructed.
- explicit RegularExpression(llvm::StringRef string);
+ //
+ /// \param[in] flags
+ /// An llvm::Regex::RegexFlags that modifies the matching behavior. The
+ /// default is NoFlags.
+ explicit RegularExpression(
+ llvm::StringRef string,
+ llvm::Regex::RegexFlags flags = llvm::Regex::NoFlags);
~RegularExpression() = default;
diff --git a/lldb/include/lldb/lldb-enumerations.h
b/lldb/include/lldb/lldb-enumerations.h
index 8e05f6ba9c876..74ff458bf87de 100644
--- a/lldb/include/lldb/lldb-enumerations.h
+++ b/lldb/include/lldb/lldb-enumerations.h
@@ -1107,7 +1107,12 @@ enum MemberFunctionKind {
};
/// String matching algorithm used by SBTarget.
-enum MatchType { eMatchTypeNormal, eMatchTypeRegex, eMatchTypeStartsWith };
+enum MatchType {
+ eMatchTypeNormal,
+ eMatchTypeRegex,
+ eMatchTypeStartsWith,
+ eMatchTypeRegexInsensitive
+};
/// Bitmask that describes details about a type.
FLAGS_ENUM(TypeFlags){
diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index adb9e645610ba..2c336296f0b8d 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -1789,6 +1789,11 @@ lldb::SBSymbolContextList
SBTarget::FindGlobalFunctions(const char *name,
target_sp->GetImages().FindFunctions(RegularExpression(name_ref),
function_options, *sb_sc_list);
break;
+ case eMatchTypeRegexInsensitive:
+target_sp->GetImages().FindFunctions(
+RegularExpression(name_ref, llvm::Regex::RegexFlags::IgnoreCase),
+function_options, *sb_sc_list);
+break;
case eMatchTypeStartsWith:
regexstr = llvm::Regex::escape(name) + ".*";
target_sp->GetImages().FindFunctions(RegularExpression(regexstr),
@@ -1936,6 +1941,11 @@ SBValueList SBTarget::FindGlobalVariables(const char
*name,
target_sp->GetImages().FindGlobalVariables(RegularExpression(name_ref),
max_matches, variable_list);
break;
+case eMatchTypeRegexInsensitive:
+ target_sp->GetImages().FindGlobalVariables(
+ RegularExpression(name_ref, llvm::Regex::IgnoreCase), max_matches,
+ variable_list);
+ break;
case eMatchTypeStartsWith:
regexstr = "^" + llvm::Regex::escape(name) + ".*";
target_sp->GetImages().FindGlobalVariables(RegularExpression(regexstr),
diff --git a/lldb/source/Utility/RegularExpression.cpp
b/lldb/source/Utility/RegularExpression.cpp
index 20bebbfe15f27..026793462221c 100644
--- a/lldb/source/Utility/RegularExpression.cpp
+++ b/lldb/source/Utility/RegularExpression.cpp
@@ -12,10 +12,11 @@
using namespace lldb_private;
-RegularExpression::RegularExpression(llvm::StringRef str)
+RegularExpression::RegularExpression(llvm::StringRef str,
+ llvm::Regex::RegexFlags flags)
: m_regex_text(std::string(str)),
// m_regex does not reference str anymore after it is constructed.
- m_regex(llvm::Regex(str)) {}
+ m_regex(llvm::Regex(str, flags)) {}
RegularExpression::RegularExpression(const RegularExpression &rhs)
: RegularExpression(rhs.GetText()) {}
diff --git a/lldb/test/API/lang/cpp/class_static/TestStaticVariables.py
b/lldb/test/API/lang/cpp/class_static/TestStaticVariables.py
index 8211d532b2638..dea7bff3a3d03 100644
--- a/lldb/test/API/lang/cpp/class_static/TestStaticVariables.py
+++ b/lldb/test/API/lang/cpp/class_static/TestStaticVariables.py
@@ -263,6 +263,10 @@ def test_with_python_FindGlobalVariables(self):
self.assertTrue(found_a, "Regex search found A::g_points")
self.assertTrue(found_aa, "Regex search found AA
[Lldb-commits] [lldb] [lldb] Support case-insensitive regex matches (PR #95350)
medismailben wrote: LGTM, may be we could also support this for the command line ``` (lldb) target modules lookup -F square 1 match found in /tmp/step: Address: step[0x00013ee8] (step.__TEXT.__text + 0) Summary: step`square at step.c:3 (lldb) target modules lookup -F Square ``` 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] Support case-insensitive regex matches (PR #95350)
https://github.com/medismailben approved this pull request. 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/crashlog] Make interactive mode the new default (PR #94575)
https://github.com/medismailben updated https://github.com/llvm/llvm-project/pull/94575 >From 3b239e683362e8f9a1a9f9b9904ef4f77f4a4b3a Mon Sep 17 00:00:00 2001 From: Med Ismail Bennani Date: Wed, 12 Jun 2024 22:11:12 -0700 Subject: [PATCH] [lldb/crashlog] Make interactive mode the new default This patch makes interactive mode as the default when using the crashlog command. It replaces the existing `-i|--interactive` flag with a new `-m|--mode` option, that can either be `interactive` or `batch`. By default, when the option is not explicitely set by the user, the interactive mode is selected, however, lldb will fallback to batch mode if the command interpreter is not interactive or if stdout is not a tty. This also adds some railguards to prevent users from using interactive only options with the batch mode and updates the tests accordingly. rdar://97801509 Differential Revision: https://reviews.llvm.org/D141658 Signed-off-by: Med Ismail Bennani --- lldb/examples/python/crashlog.py | 124 +++--- .../Python/Crashlog/altered_threadState.test | 2 +- .../Python/Crashlog/json.test | 6 +- .../Python/Crashlog/no_threadState.test | 2 +- .../skipped_status_interactive_crashlog.test | 2 +- .../Python/Crashlog/text.test | 2 +- 6 files changed, 83 insertions(+), 55 deletions(-) diff --git a/lldb/examples/python/crashlog.py b/lldb/examples/python/crashlog.py index 1c0d717ce455c..d3952e377c657 100755 --- a/lldb/examples/python/crashlog.py +++ b/lldb/examples/python/crashlog.py @@ -31,6 +31,7 @@ import concurrent.futures import contextlib import datetime +import enum import json import os import platform @@ -45,7 +46,6 @@ import time import uuid - print_lock = threading.RLock() try: @@ -1582,9 +1582,12 @@ def synchronous(debugger): debugger.RunCommandInterpreter(True, False, run_options, 0, False, True) -def CreateSymbolicateCrashLogOptions( -command_name, description, add_interactive_options -): +class CrashLogLoadingMode(str, enum.Enum): +batch = "batch" +interactive = "interactive" + + +def CreateSymbolicateCrashLogOptions(command_name, description): usage = "crashlog [options] [FILE ...]" arg_parser = argparse.ArgumentParser( description=description, @@ -1600,6 +1603,12 @@ def CreateSymbolicateCrashLogOptions( help="crash report(s) to symbolicate", ) +arg_parser.add_argument( +"-m", +"--mode", +choices=[mode.value for mode in CrashLogLoadingMode], +help="change how the symbolicated process and threads are displayed to the user (default: interactive)", +) arg_parser.add_argument( "--version", "-V", @@ -1736,36 +1745,35 @@ def CreateSymbolicateCrashLogOptions( help=argparse.SUPPRESS, default=False, ) -if add_interactive_options: -arg_parser.add_argument( -"-i", -"--interactive", -action="store_true", -help="parse a crash log and load it in a ScriptedProcess", -default=False, -) -arg_parser.add_argument( -"-b", -"--batch", -action="store_true", -help="dump symbolicated stackframes without creating a debug session", -default=True, -) -arg_parser.add_argument( -"--target", -"-t", -dest="target_path", -help="the target binary path that should be used for interactive crashlog (optional)", -default=None, -) -arg_parser.add_argument( -"--skip-status", -"-s", -dest="skip_status", -action="store_true", -help="prevent the interactive crashlog to dump the process status and thread backtrace at launch", -default=False, -) +arg_parser.add_argument( +"--target", +"-t", +dest="target_path", +help="the target binary path that should be used for interactive crashlog (optional)", +default=None, +) +arg_parser.add_argument( +"--skip-status", +"-s", +dest="skip_status", +action="store_true", +help="prevent the interactive crashlog to dump the process status and thread backtrace at launch", +default=False, +) +legacy_group = arg_parser.add_mutually_exclusive_group() +legacy_group.add_argument( +"-i", +"--interactive", +action="store_true", +help=argparse.SUPPRESS, +) +legacy_group.add_argument( +"-b", +"--batch", +action="store_true", +help=argparse.SUPPRESS, +) + return arg_parser @@ -1778,7 +1786,7 @@ def CrashLogOptionParser(): created that has all of the shared libraries loaded at the load addresses found in the crash log file. This allows you to explore the prog
