[Lldb-commits] [lldb] [lldb][test][win][x86_64] XFAIL already failing Shell tests (PR #100476)
labath wrote: One of the problems here is that we actually have two implementations of pdb readers in lldb (SymbolFilePDB, and SymbolFileNativePDB). PDB is the default (kind of -- if you are on windows, and cmake manages to find the DIA SDK), while the majority of users (my non-scientific sample consists of the two of you plus the chromium team (cc @ZequanWu)) appear to use/prefer the NativePDB implementation. Maybe it would be a good idea to switch the default to use NativePDB, or even consider deleting the PDB implementation entirely? https://github.com/llvm/llvm-project/pull/100476 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBSaveCore] Fix bug where default values are not propagated. (PR #101770)
hokein wrote: The newly-added test causes a msan failure: ``` ==1960==WARNING: MemorySanitizer: use-of-uninitialized-value #0 0x55e25271b29e in RetryAfterSignal llvm-project/llvm/include/llvm/Support/Errno.h:37:11 #1 0x55e25271b29e in lldb_private::NativeFile::Write(void const*, unsigned long&) llvm-project/lldb/source/Host/common/File.cpp:618:9 #2 0x55e25223793f in MinidumpFileBuilder::FlushBufferToDisk() llvm-project/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:1165:26 #3 0x55e252237548 in MinidumpFileBuilder::FixThreadStacks() llvm-project/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:568:3 #4 0x55e25223ba52 in MinidumpFileBuilder::AddMemoryList() llvm-project/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:890:10 #5 0x55e2522438ad in ObjectFileMinidump::SaveCore(std::__msan::shared_ptr const&, lldb_private::SaveCoreOptions&, lldb_private::Status&) llvm-project/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp:124:19 #6 0x55e251e1f482 in lldb_private::PluginManager::SaveCore(std::__msan::shared_ptr const&, lldb_private::SaveCoreOptions&) llvm-project/lldb/source/Core/PluginManager.cpp:736:33 ``` https://github.com/llvm/llvm-project/pull/101770 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 86f7374 - Revert "[LLDB][SBSaveCore] Fix bug where default values are not propagated. (#101770)"
Author: Haojian Wu
Date: 2024-08-05T09:37:36+02:00
New Revision: 86f7374078288e2b3d3d0fd66428f7752e2319e6
URL:
https://github.com/llvm/llvm-project/commit/86f7374078288e2b3d3d0fd66428f7752e2319e6
DIFF:
https://github.com/llvm/llvm-project/commit/86f7374078288e2b3d3d0fd66428f7752e2319e6.diff
LOG: Revert "[LLDB][SBSaveCore] Fix bug where default values are not
propagated. (#101770)"
This reverts commit 34766d0d488ba2fbefa80dcd0cc8720a0e753448 which
caused a msan failure, see comment
https://github.com/llvm/llvm-project/pull/101770#issuecomment-2268373325 for
details.
Added:
Modified:
lldb/include/lldb/Core/PluginManager.h
lldb/include/lldb/lldb-private-interfaces.h
lldb/source/Core/PluginManager.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp
lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.h
lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py
Removed:
diff --git a/lldb/include/lldb/Core/PluginManager.h
b/lldb/include/lldb/Core/PluginManager.h
index e4e0c3eea67f8..a23f834f471fb 100644
--- a/lldb/include/lldb/Core/PluginManager.h
+++ b/lldb/include/lldb/Core/PluginManager.h
@@ -194,7 +194,7 @@ class PluginManager {
GetObjectFileCreateMemoryCallbackForPluginName(llvm::StringRef name);
static Status SaveCore(const lldb::ProcessSP &process_sp,
- lldb_private::SaveCoreOptions &core_options);
+ const lldb_private::SaveCoreOptions &core_options);
// ObjectContainer
static bool RegisterPlugin(
diff --git a/lldb/include/lldb/lldb-private-interfaces.h
b/lldb/include/lldb/lldb-private-interfaces.h
index b3c8cda899b95..87c5ff8d22fb6 100644
--- a/lldb/include/lldb/lldb-private-interfaces.h
+++ b/lldb/include/lldb/lldb-private-interfaces.h
@@ -57,7 +57,7 @@ typedef ObjectFile *(*ObjectFileCreateMemoryInstance)(
const lldb::ModuleSP &module_sp, lldb::WritableDataBufferSP data_sp,
const lldb::ProcessSP &process_sp, lldb::addr_t offset);
typedef bool (*ObjectFileSaveCore)(const lldb::ProcessSP &process_sp,
- lldb_private::SaveCoreOptions &options,
+ const lldb_private::SaveCoreOptions
&options,
Status &error);
typedef EmulateInstruction *(*EmulateInstructionCreateInstance)(
const ArchSpec &arch, InstructionType inst_type);
diff --git a/lldb/source/Core/PluginManager.cpp
b/lldb/source/Core/PluginManager.cpp
index f243807df509e..fbd78a7780578 100644
--- a/lldb/source/Core/PluginManager.cpp
+++ b/lldb/source/Core/PluginManager.cpp
@@ -702,7 +702,7 @@
PluginManager::GetObjectFileCreateMemoryCallbackForPluginName(
}
Status PluginManager::SaveCore(const lldb::ProcessSP &process_sp,
- lldb_private::SaveCoreOptions &options) {
+ const lldb_private::SaveCoreOptions &options) {
Status error;
if (!options.GetOutputFile()) {
error.SetErrorString("No output file specified");
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 22ece4f4dacf7..4322bd7e2674f 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -6351,7 +6351,7 @@ static offset_t
CreateAllImageInfosPayload(const lldb::ProcessSP &process_sp,
offset_t initial_file_offset,
StreamString &all_image_infos_payload,
- lldb_private::SaveCoreOptions &options) {
+ const lldb_private::SaveCoreOptions &options) {
Target &target = process_sp->GetTarget();
ModuleList modules = target.GetImages();
@@ -6522,17 +6522,16 @@ struct page_object {
};
bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp,
- lldb_private::SaveCoreOptions &options,
+ const lldb_private::SaveCoreOptions &options,
Status &error) {
+ auto core_style = options.GetStyle();
+ if (core_style == SaveCoreStyle::eSaveCoreUnspecified)
+core_style = SaveCoreStyle::eSaveCoreDirtyOnly;
// The FileSpec and Process are already checked in PluginManager::SaveCore.
assert(options.GetOutputFile().has_value());
assert(process_sp);
const FileSpec outfile = options.G
[Lldb-commits] [lldb] [lldb/API] Fix SBStructuredData JSON Array parsing (PR #101929)
https://github.com/medismailben created
https://github.com/llvm/llvm-project/pull/101929
This patch loosen the parsing requirement to allow parsing not only JSON
dictionaries but also JSON arrays.
>From 54081f56181d954365f9ab762ca6715e28119ad4 Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani
Date: Mon, 5 Aug 2024 00:37:51 -0700
Subject: [PATCH] [lldb/API] Fix SBStructuredData JSON Array parsing
This patch loosen the parsing requirement to allow parsing not only
JSON dictionaries but also JSON arrays.
Signed-off-by: Med Ismail Bennani
---
lldb/source/API/SBStructuredData.cpp | 8 ++--
.../sbstructureddata/TestStructuredDataAPI.py | 18 ++
2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/lldb/source/API/SBStructuredData.cpp
b/lldb/source/API/SBStructuredData.cpp
index b18fc5655fc81..32fa5d7bb30f9 100644
--- a/lldb/source/API/SBStructuredData.cpp
+++ b/lldb/source/API/SBStructuredData.cpp
@@ -86,7 +86,11 @@ lldb::SBError SBStructuredData::SetFromJSON(lldb::SBStream
&stream) {
StructuredData::ParseJSON(stream.GetData());
m_impl_up->SetObjectSP(json_obj);
- if (!json_obj || json_obj->GetType() != eStructuredDataTypeDictionary)
+ static constexpr StructuredDataType structured_data_record_type[] = {
+ eStructuredDataTypeArray, eStructuredDataTypeDictionary};
+
+ if (!json_obj ||
+ !llvm::is_contained(structured_data_record_type, json_obj->GetType()))
error.SetErrorString("Invalid Syntax");
return error;
}
@@ -106,7 +110,7 @@ bool SBStructuredData::IsValid() const {
SBStructuredData::operator bool() const {
LLDB_INSTRUMENT_VA(this);
- return m_impl_up->IsValid();
+ return m_impl_up && m_impl_up->IsValid();
}
void SBStructuredData::Clear() {
diff --git a/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
b/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
index b3db3bc61e4dc..c0ce482860c85 100644
--- a/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
+++ b/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
@@ -110,6 +110,24 @@ class MyRandomClass:
self.assertTrue(my_random_class)
self.assertEqual(my_random_class.payload, MyRandomClass.payload)
+example_arr = [1, 2.3, "4", {"5": False}]
+arr_str = json.dumps(example_arr)
+s.Clear()
+s.Print(arr_str)
+example = lldb.SBStructuredData()
+
+# Check SetFromJSON API for dictionaries, integers, floating point
+# values, strings and arrays
+error = example.SetFromJSON(s)
+if not error.Success():
+self.fail("FAILED: " + error.GetCString())
+
+s.Clear()
+self.assertSuccess(example.GetAsJSON(s))
+sb_data = json.loads(s.GetData())
+self.assertEqual(sb_data, example_arr)
+
+
def invalid_struct_test(self, example):
invalid_struct = lldb.SBStructuredData()
invalid_struct = example.GetValueForKey("invalid_key")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/Plugins] Introduce Scripted Platform Plugin (PR #99814)
@@ -86,7 +86,11 @@ lldb::SBError SBStructuredData::SetFromJSON(lldb::SBStream
&stream) {
StructuredData::ParseJSON(stream.GetData());
m_impl_up->SetObjectSP(json_obj);
- if (!json_obj || json_obj->GetType() != eStructuredDataTypeDictionary)
+ static constexpr StructuredDataType structured_data_record_type[] = {
medismailben wrote:
Here: #101929
https://github.com/llvm/llvm-project/pull/99814
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/API] Fix SBStructuredData JSON Array parsing (PR #101929)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Med Ismail Bennani (medismailben)
Changes
This patch loosen the parsing requirement to allow parsing not only JSON
dictionaries but also JSON arrays.
---
Full diff: https://github.com/llvm/llvm-project/pull/101929.diff
2 Files Affected:
- (modified) lldb/source/API/SBStructuredData.cpp (+6-2)
- (modified) lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
(+18)
``diff
diff --git a/lldb/source/API/SBStructuredData.cpp
b/lldb/source/API/SBStructuredData.cpp
index b18fc5655fc81..32fa5d7bb30f9 100644
--- a/lldb/source/API/SBStructuredData.cpp
+++ b/lldb/source/API/SBStructuredData.cpp
@@ -86,7 +86,11 @@ lldb::SBError SBStructuredData::SetFromJSON(lldb::SBStream
&stream) {
StructuredData::ParseJSON(stream.GetData());
m_impl_up->SetObjectSP(json_obj);
- if (!json_obj || json_obj->GetType() != eStructuredDataTypeDictionary)
+ static constexpr StructuredDataType structured_data_record_type[] = {
+ eStructuredDataTypeArray, eStructuredDataTypeDictionary};
+
+ if (!json_obj ||
+ !llvm::is_contained(structured_data_record_type, json_obj->GetType()))
error.SetErrorString("Invalid Syntax");
return error;
}
@@ -106,7 +110,7 @@ bool SBStructuredData::IsValid() const {
SBStructuredData::operator bool() const {
LLDB_INSTRUMENT_VA(this);
- return m_impl_up->IsValid();
+ return m_impl_up && m_impl_up->IsValid();
}
void SBStructuredData::Clear() {
diff --git a/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
b/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
index b3db3bc61e4dc..c0ce482860c85 100644
--- a/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
+++ b/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
@@ -110,6 +110,24 @@ class MyRandomClass:
self.assertTrue(my_random_class)
self.assertEqual(my_random_class.payload, MyRandomClass.payload)
+example_arr = [1, 2.3, "4", {"5": False}]
+arr_str = json.dumps(example_arr)
+s.Clear()
+s.Print(arr_str)
+example = lldb.SBStructuredData()
+
+# Check SetFromJSON API for dictionaries, integers, floating point
+# values, strings and arrays
+error = example.SetFromJSON(s)
+if not error.Success():
+self.fail("FAILED: " + error.GetCString())
+
+s.Clear()
+self.assertSuccess(example.GetAsJSON(s))
+sb_data = json.loads(s.GetData())
+self.assertEqual(sb_data, example_arr)
+
+
def invalid_struct_test(self, example):
invalid_struct = lldb.SBStructuredData()
invalid_struct = example.GetValueForKey("invalid_key")
``
https://github.com/llvm/llvm-project/pull/101929
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBSaveCore] Fix bug where default values are not propagated. (PR #101770)
hokein wrote: @Jlalond, I revert it in 86f7374078288e2b3d3d0fd66428f7752e2319e6 to keep the trunk green. Feel free to reland the patch with the fix. https://github.com/llvm/llvm-project/pull/101770 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/Plugins] Introduce Scripted Platform Plugin (PR #99814)
https://github.com/medismailben updated
https://github.com/llvm/llvm-project/pull/99814
>From 54081f56181d954365f9ab762ca6715e28119ad4 Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani
Date: Mon, 5 Aug 2024 00:37:51 -0700
Subject: [PATCH 1/2] [lldb/API] Fix SBStructuredData JSON Array parsing
This patch loosen the parsing requirement to allow parsing not only
JSON dictionaries but also JSON arrays.
Signed-off-by: Med Ismail Bennani
---
lldb/source/API/SBStructuredData.cpp | 8 ++--
.../sbstructureddata/TestStructuredDataAPI.py | 18 ++
2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/lldb/source/API/SBStructuredData.cpp
b/lldb/source/API/SBStructuredData.cpp
index b18fc5655fc81..32fa5d7bb30f9 100644
--- a/lldb/source/API/SBStructuredData.cpp
+++ b/lldb/source/API/SBStructuredData.cpp
@@ -86,7 +86,11 @@ lldb::SBError SBStructuredData::SetFromJSON(lldb::SBStream
&stream) {
StructuredData::ParseJSON(stream.GetData());
m_impl_up->SetObjectSP(json_obj);
- if (!json_obj || json_obj->GetType() != eStructuredDataTypeDictionary)
+ static constexpr StructuredDataType structured_data_record_type[] = {
+ eStructuredDataTypeArray, eStructuredDataTypeDictionary};
+
+ if (!json_obj ||
+ !llvm::is_contained(structured_data_record_type, json_obj->GetType()))
error.SetErrorString("Invalid Syntax");
return error;
}
@@ -106,7 +110,7 @@ bool SBStructuredData::IsValid() const {
SBStructuredData::operator bool() const {
LLDB_INSTRUMENT_VA(this);
- return m_impl_up->IsValid();
+ return m_impl_up && m_impl_up->IsValid();
}
void SBStructuredData::Clear() {
diff --git a/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
b/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
index b3db3bc61e4dc..c0ce482860c85 100644
--- a/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
+++ b/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
@@ -110,6 +110,24 @@ class MyRandomClass:
self.assertTrue(my_random_class)
self.assertEqual(my_random_class.payload, MyRandomClass.payload)
+example_arr = [1, 2.3, "4", {"5": False}]
+arr_str = json.dumps(example_arr)
+s.Clear()
+s.Print(arr_str)
+example = lldb.SBStructuredData()
+
+# Check SetFromJSON API for dictionaries, integers, floating point
+# values, strings and arrays
+error = example.SetFromJSON(s)
+if not error.Success():
+self.fail("FAILED: " + error.GetCString())
+
+s.Clear()
+self.assertSuccess(example.GetAsJSON(s))
+sb_data = json.loads(s.GetData())
+self.assertEqual(sb_data, example_arr)
+
+
def invalid_struct_test(self, example):
invalid_struct = lldb.SBStructuredData()
invalid_struct = example.GetValueForKey("invalid_key")
>From 4d1b7aaa5916417ad6882075a06bd1ae4b8b6d30 Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani
Date: Mon, 5 Aug 2024 00:03:33 -0700
Subject: [PATCH 2/2] [lldb/Plugins] Introduce Scripted Platform Plugin
This patch introduces Scripted Platform, a new platform plugin that can
be customized with a python script.
For now this can list processes described in the python script file but
eventually, it will be used to spawn scripted processes and act as an
interface between them.
This patch is also a less intrusive implementation of 2d53527.
It introduces a new PlatformMetadata held by every platform that
contains various objects that might be used by a platform instance.
In its current form, the PlatformMetadata holds a reference to the
Debugger and a ScriptedMetadata pointer. These are necessary in other to
instanciate the scripted object that the ScriptedPlatform interacts with.
In order to make it less introsive with the rest of lldb's platform
creation code, platform metadata are set after the platform creation,
and requires to platform to reload them (using `Platform::ReloadMetadata`).
This approach has the tradeoff that the ScriptedPlaform instance is
technically invalid and useless right after its creation. However, the user
should never be in that situation, since we reload the platform metadata
everytime with create or select the platform.
This work was previously reviewed in:
- https://reviews.llvm.org/D139252
- https://reviews.llvm.org/D142266
Signed-off-by: Med Ismail Bennani
---
.../bindings/interface/SBPlatformDocstrings.i | 12 +
lldb/bindings/python/python-swigsafecast.swig | 10 +
lldb/bindings/python/python-wrapper.swig | 36 ++
.../python/templates/scripted_platform.py | 28 +-
lldb/include/lldb/API/SBDebugger.h| 2 +
lldb/include/lldb/API/SBPlatform.h| 4 +-
lldb/include/lldb/API/SBProcess.h | 2 +
lldb/include/lldb/API/SBStructuredData.h | 1 +
lldb/include/lldb/API/SBTarget.h | 2 +
.../Interfaces/ScriptedPlatformInterface.h| 9 +-
.../lldb
[Lldb-commits] [lldb] [lldb/API] Fix SBStructuredData JSON Array parsing (PR #101929)
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
8d1b17b6623742ec4454f5bae2e23f8b30124314...54081f56181d954365f9ab762ca6715e28119ad4
lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
``
View the diff from darker here.
``diff
--- TestStructuredDataAPI.py2024-08-05 07:37:51.00 +
+++ TestStructuredDataAPI.py2024-08-05 07:43:53.182694 +
@@ -125,11 +125,10 @@
s.Clear()
self.assertSuccess(example.GetAsJSON(s))
sb_data = json.loads(s.GetData())
self.assertEqual(sb_data, example_arr)
-
def invalid_struct_test(self, example):
invalid_struct = lldb.SBStructuredData()
invalid_struct = example.GetValueForKey("invalid_key")
if invalid_struct.IsValid():
self.fail("An invalid object should have been returned")
``
https://github.com/llvm/llvm-project/pull/101929
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/API] Fix SBStructuredData JSON Array parsing (PR #101929)
https://github.com/medismailben updated
https://github.com/llvm/llvm-project/pull/101929
>From 18159bbcf6418d18ceb2a77e464e98e349225c1c Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani
Date: Mon, 5 Aug 2024 00:48:58 -0700
Subject: [PATCH] [lldb/API] Fix SBStructuredData JSON Array parsing
This patch loosen the parsing requirement to allow parsing not only
JSON dictionaries but also JSON arrays.
Signed-off-by: Med Ismail Bennani
---
lldb/source/API/SBStructuredData.cpp| 8 ++--
.../sbstructureddata/TestStructuredDataAPI.py | 17 +
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/lldb/source/API/SBStructuredData.cpp
b/lldb/source/API/SBStructuredData.cpp
index b18fc5655fc81..32fa5d7bb30f9 100644
--- a/lldb/source/API/SBStructuredData.cpp
+++ b/lldb/source/API/SBStructuredData.cpp
@@ -86,7 +86,11 @@ lldb::SBError SBStructuredData::SetFromJSON(lldb::SBStream
&stream) {
StructuredData::ParseJSON(stream.GetData());
m_impl_up->SetObjectSP(json_obj);
- if (!json_obj || json_obj->GetType() != eStructuredDataTypeDictionary)
+ static constexpr StructuredDataType structured_data_record_type[] = {
+ eStructuredDataTypeArray, eStructuredDataTypeDictionary};
+
+ if (!json_obj ||
+ !llvm::is_contained(structured_data_record_type, json_obj->GetType()))
error.SetErrorString("Invalid Syntax");
return error;
}
@@ -106,7 +110,7 @@ bool SBStructuredData::IsValid() const {
SBStructuredData::operator bool() const {
LLDB_INSTRUMENT_VA(this);
- return m_impl_up->IsValid();
+ return m_impl_up && m_impl_up->IsValid();
}
void SBStructuredData::Clear() {
diff --git a/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
b/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
index b3db3bc61e4dc..01e5a76d1a3a6 100644
--- a/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
+++ b/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
@@ -110,6 +110,23 @@ class MyRandomClass:
self.assertTrue(my_random_class)
self.assertEqual(my_random_class.payload, MyRandomClass.payload)
+example_arr = [1, 2.3, "4", {"5": False}]
+arr_str = json.dumps(example_arr)
+s.Clear()
+s.Print(arr_str)
+example = lldb.SBStructuredData()
+
+# Check SetFromJSON API for dictionaries, integers, floating point
+# values, strings and arrays
+error = example.SetFromJSON(s)
+if not error.Success():
+self.fail("FAILED: " + error.GetCString())
+
+s.Clear()
+self.assertSuccess(example.GetAsJSON(s))
+sb_data = json.loads(s.GetData())
+self.assertEqual(sb_data, example_arr)
+
def invalid_struct_test(self, example):
invalid_struct = lldb.SBStructuredData()
invalid_struct = example.GetValueForKey("invalid_key")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/Plugins] Introduce Scripted Platform Plugin (PR #99814)
https://github.com/medismailben updated
https://github.com/llvm/llvm-project/pull/99814
>From 4b8ccab40914a9a6ee32939911b66fb701605c96 Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani
Date: Mon, 5 Aug 2024 00:49:55 -0700
Subject: [PATCH 1/2] [lldb/API] Fix SBStructuredData JSON Array parsing
This patch loosen the parsing requirement to allow parsing not only
JSON dictionaries but also JSON arrays.
Signed-off-by: Med Ismail Bennani
---
lldb/source/API/SBStructuredData.cpp| 8 ++--
.../sbstructureddata/TestStructuredDataAPI.py | 17 +
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/lldb/source/API/SBStructuredData.cpp
b/lldb/source/API/SBStructuredData.cpp
index b18fc5655fc81..32fa5d7bb30f9 100644
--- a/lldb/source/API/SBStructuredData.cpp
+++ b/lldb/source/API/SBStructuredData.cpp
@@ -86,7 +86,11 @@ lldb::SBError SBStructuredData::SetFromJSON(lldb::SBStream
&stream) {
StructuredData::ParseJSON(stream.GetData());
m_impl_up->SetObjectSP(json_obj);
- if (!json_obj || json_obj->GetType() != eStructuredDataTypeDictionary)
+ static constexpr StructuredDataType structured_data_record_type[] = {
+ eStructuredDataTypeArray, eStructuredDataTypeDictionary};
+
+ if (!json_obj ||
+ !llvm::is_contained(structured_data_record_type, json_obj->GetType()))
error.SetErrorString("Invalid Syntax");
return error;
}
@@ -106,7 +110,7 @@ bool SBStructuredData::IsValid() const {
SBStructuredData::operator bool() const {
LLDB_INSTRUMENT_VA(this);
- return m_impl_up->IsValid();
+ return m_impl_up && m_impl_up->IsValid();
}
void SBStructuredData::Clear() {
diff --git a/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
b/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
index b3db3bc61e4dc..01e5a76d1a3a6 100644
--- a/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
+++ b/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
@@ -110,6 +110,23 @@ class MyRandomClass:
self.assertTrue(my_random_class)
self.assertEqual(my_random_class.payload, MyRandomClass.payload)
+example_arr = [1, 2.3, "4", {"5": False}]
+arr_str = json.dumps(example_arr)
+s.Clear()
+s.Print(arr_str)
+example = lldb.SBStructuredData()
+
+# Check SetFromJSON API for dictionaries, integers, floating point
+# values, strings and arrays
+error = example.SetFromJSON(s)
+if not error.Success():
+self.fail("FAILED: " + error.GetCString())
+
+s.Clear()
+self.assertSuccess(example.GetAsJSON(s))
+sb_data = json.loads(s.GetData())
+self.assertEqual(sb_data, example_arr)
+
def invalid_struct_test(self, example):
invalid_struct = lldb.SBStructuredData()
invalid_struct = example.GetValueForKey("invalid_key")
>From 6f076dc50577938647d00ee895e07e44afe0f0ea Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani
Date: Mon, 5 Aug 2024 00:03:33 -0700
Subject: [PATCH 2/2] [lldb/Plugins] Introduce Scripted Platform Plugin
This patch introduces Scripted Platform, a new platform plugin that can
be customized with a python script.
For now this can list processes described in the python script file but
eventually, it will be used to spawn scripted processes and act as an
interface between them.
This patch is also a less intrusive implementation of 2d53527.
It introduces a new PlatformMetadata held by every platform that
contains various objects that might be used by a platform instance.
In its current form, the PlatformMetadata holds a reference to the
Debugger and a ScriptedMetadata pointer. These are necessary in other to
instanciate the scripted object that the ScriptedPlatform interacts with.
In order to make it less introsive with the rest of lldb's platform
creation code, platform metadata are set after the platform creation,
and requires to platform to reload them (using `Platform::ReloadMetadata`).
This approach has the tradeoff that the ScriptedPlaform instance is
technically invalid and useless right after its creation. However, the user
should never be in that situation, since we reload the platform metadata
everytime with create or select the platform.
This work was previously reviewed in:
- https://reviews.llvm.org/D139252
- https://reviews.llvm.org/D142266
Signed-off-by: Med Ismail Bennani
---
.../bindings/interface/SBPlatformDocstrings.i | 12 +
lldb/bindings/python/python-swigsafecast.swig | 10 +
lldb/bindings/python/python-wrapper.swig | 36 ++
.../python/templates/scripted_platform.py | 28 +-
lldb/include/lldb/API/SBDebugger.h| 2 +
lldb/include/lldb/API/SBPlatform.h| 4 +-
lldb/include/lldb/API/SBProcess.h | 2 +
lldb/include/lldb/API/SBStructuredData.h | 1 +
lldb/include/lldb/API/SBTarget.h | 2 +
.../Interfaces/ScriptedPlatformInterface.h| 9 +-
.../lldb/
[Lldb-commits] [lldb] [lldb/Plugins] Introduce Scripted Platform Plugin (PR #99814)
https://github.com/medismailben updated
https://github.com/llvm/llvm-project/pull/99814
>From 4b8ccab40914a9a6ee32939911b66fb701605c96 Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani
Date: Mon, 5 Aug 2024 00:49:55 -0700
Subject: [PATCH 1/2] [lldb/API] Fix SBStructuredData JSON Array parsing
This patch loosen the parsing requirement to allow parsing not only
JSON dictionaries but also JSON arrays.
Signed-off-by: Med Ismail Bennani
---
lldb/source/API/SBStructuredData.cpp| 8 ++--
.../sbstructureddata/TestStructuredDataAPI.py | 17 +
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/lldb/source/API/SBStructuredData.cpp
b/lldb/source/API/SBStructuredData.cpp
index b18fc5655fc81..32fa5d7bb30f9 100644
--- a/lldb/source/API/SBStructuredData.cpp
+++ b/lldb/source/API/SBStructuredData.cpp
@@ -86,7 +86,11 @@ lldb::SBError SBStructuredData::SetFromJSON(lldb::SBStream
&stream) {
StructuredData::ParseJSON(stream.GetData());
m_impl_up->SetObjectSP(json_obj);
- if (!json_obj || json_obj->GetType() != eStructuredDataTypeDictionary)
+ static constexpr StructuredDataType structured_data_record_type[] = {
+ eStructuredDataTypeArray, eStructuredDataTypeDictionary};
+
+ if (!json_obj ||
+ !llvm::is_contained(structured_data_record_type, json_obj->GetType()))
error.SetErrorString("Invalid Syntax");
return error;
}
@@ -106,7 +110,7 @@ bool SBStructuredData::IsValid() const {
SBStructuredData::operator bool() const {
LLDB_INSTRUMENT_VA(this);
- return m_impl_up->IsValid();
+ return m_impl_up && m_impl_up->IsValid();
}
void SBStructuredData::Clear() {
diff --git a/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
b/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
index b3db3bc61e4dc..01e5a76d1a3a6 100644
--- a/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
+++ b/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
@@ -110,6 +110,23 @@ class MyRandomClass:
self.assertTrue(my_random_class)
self.assertEqual(my_random_class.payload, MyRandomClass.payload)
+example_arr = [1, 2.3, "4", {"5": False}]
+arr_str = json.dumps(example_arr)
+s.Clear()
+s.Print(arr_str)
+example = lldb.SBStructuredData()
+
+# Check SetFromJSON API for dictionaries, integers, floating point
+# values, strings and arrays
+error = example.SetFromJSON(s)
+if not error.Success():
+self.fail("FAILED: " + error.GetCString())
+
+s.Clear()
+self.assertSuccess(example.GetAsJSON(s))
+sb_data = json.loads(s.GetData())
+self.assertEqual(sb_data, example_arr)
+
def invalid_struct_test(self, example):
invalid_struct = lldb.SBStructuredData()
invalid_struct = example.GetValueForKey("invalid_key")
>From 69812dd8a4f0feef416cae44f7949c2a35237b1a Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani
Date: Mon, 5 Aug 2024 00:51:34 -0700
Subject: [PATCH 2/2] [lldb/Plugins] Introduce Scripted Platform Plugin
This patch introduces Scripted Platform, a new platform plugin that can
be customized with a python script.
For now this can list processes described in the python script file but
eventually, it will be used to spawn scripted processes and act as an
interface between them.
This patch is also a less intrusive implementation of 2d53527.
It introduces a new PlatformMetadata held by every platform that
contains various objects that might be used by a platform instance.
In its current form, the PlatformMetadata holds a reference to the
Debugger and a ScriptedMetadata pointer. These are necessary in other to
instanciate the scripted object that the ScriptedPlatform interacts with.
In order to make it less introsive with the rest of lldb's platform
creation code, platform metadata are set after the platform creation,
and requires to platform to reload them (using `Platform::ReloadMetadata`).
This approach has the tradeoff that the ScriptedPlaform instance is
technically invalid and useless right after its creation. However, the user
should never be in that situation, since we reload the platform metadata
everytime with create or select the platform.
This work was previously reviewed in:
- https://reviews.llvm.org/D139252
- https://reviews.llvm.org/D142266
Signed-off-by: Med Ismail Bennani
---
.../bindings/interface/SBPlatformDocstrings.i | 12 +
lldb/bindings/python/python-swigsafecast.swig | 10 +
lldb/bindings/python/python-wrapper.swig | 36 ++
.../python/templates/scripted_platform.py | 28 +-
lldb/include/lldb/API/SBDebugger.h| 2 +
lldb/include/lldb/API/SBPlatform.h| 4 +-
lldb/include/lldb/API/SBProcess.h | 2 +
lldb/include/lldb/API/SBStructuredData.h | 1 +
lldb/include/lldb/API/SBTarget.h | 2 +
.../Interfaces/ScriptedPlatformInterface.h| 9 +-
.../lldb/
[Lldb-commits] [lldb] [lldb/Target] Rename ThreadPlanPython into ScriptedThreadPlan (PR #101931)
https://github.com/medismailben created
https://github.com/llvm/llvm-project/pull/101931
Following 9a9ec228cdcf, since the ThreadPlanPython class started making use of
the Scripted Interface instead of calling directly into the python methods,
this class can work with other scripting languages (as long as someone add the
interfact for that language ;p).
So it doesn't make sense anymore for it to keep this name and also we should
avoid having language specific related classes outside the plugin directory.
This patch renames the internal class from `ThreadPlanPython` to
`ScriptedThreadPlan` as its advertised externally, and also updates the various
log messages.
This should hopefully make the codebase more coherent.
>From 3a294342de12aa769799cbc197edfe9328f9473e Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani
Date: Sun, 4 Aug 2024 21:18:56 -0700
Subject: [PATCH] [lldb/Target] Rename ThreadPlanPython into ScriptedThreadPlan
Following 9a9ec22, since the ThreadPlanPython class started making use
of the Scripted Interface instead of calling directly into the python
methods, this class can work with other scripting languages (as long as
someone add the interfact for that language ;p).
So it doesn't make sense anymore for it to keep this name and also we
should avoid having language specific related classes outside the plugin
directory.
This patch renames the internal class from `ThreadPlanPython` to
`ScriptedThreadPlan` as its advertised externally, and also updates the
various log messages.
This should hopefully make the codebase more coherent.
Signed-off-by: Med Ismail Bennani
---
...hreadPlanPython.h => ScriptedThreadPlan.h} | 27 +++
lldb/source/API/SBThreadPlan.cpp | 10 +--
lldb/source/Target/CMakeLists.txt | 2 +-
...dPlanPython.cpp => ScriptedThreadPlan.cpp} | 81 +--
lldb/source/Target/Thread.cpp | 4 +-
5 files changed, 59 insertions(+), 65 deletions(-)
rename lldb/include/lldb/Target/{ThreadPlanPython.h => ScriptedThreadPlan.h}
(77%)
rename lldb/source/Target/{ThreadPlanPython.cpp => ScriptedThreadPlan.cpp}
(70%)
diff --git a/lldb/include/lldb/Target/ThreadPlanPython.h
b/lldb/include/lldb/Target/ScriptedThreadPlan.h
similarity index 77%
rename from lldb/include/lldb/Target/ThreadPlanPython.h
rename to lldb/include/lldb/Target/ScriptedThreadPlan.h
index da106faf951db..7532b6b0a41c5 100644
--- a/lldb/include/lldb/Target/ThreadPlanPython.h
+++ b/lldb/include/lldb/Target/ScriptedThreadPlan.h
@@ -1,5 +1,4 @@
-//===-- ThreadPlanPython.h *- C++
-//-*-===//
+//===-- ScriptedThreadPlan.h *- C++
-*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -7,8 +6,8 @@
//
//===--===//
-#ifndef LLDB_TARGET_THREADPLANPYTHON_H
-#define LLDB_TARGET_THREADPLANPYTHON_H
+#ifndef LLDB_TARGET_SCRIPTEDTHREADPLAN_H
+#define LLDB_TARGET_SCRIPTEDTHREADPLAN_H
#include
@@ -27,13 +26,10 @@
namespace lldb_private {
-// ThreadPlanPython:
-//
-
-class ThreadPlanPython : public ThreadPlan {
+class ScriptedThreadPlan : public ThreadPlan {
public:
- ThreadPlanPython(Thread &thread, const char *class_name,
- const StructuredDataImpl &args_data);
+ ScriptedThreadPlan(Thread &thread, const char *class_name,
+ const StructuredDataImpl &args_data);
void GetDescription(Stream *s, lldb::DescriptionLevel level) override;
@@ -52,15 +48,14 @@ class ThreadPlanPython : public ThreadPlan {
void DidPush() override;
bool IsPlanStale() override;
-
- bool DoWillResume(lldb::StateType resume_state, bool current_plan) override;
+ bool DoWillResume(lldb::StateType resume_state, bool current_plan) override;
protected:
bool DoPlanExplainsStop(Event *event_ptr) override;
lldb::StateType GetPlanRunState() override;
-
+
ScriptInterpreter *GetScriptInterpreter();
private:
@@ -73,10 +68,10 @@ class ThreadPlanPython : public ThreadPlan {
bool m_stop_others;
lldb::ScriptedThreadPlanInterfaceSP m_interface;
- ThreadPlanPython(const ThreadPlanPython &) = delete;
- const ThreadPlanPython &operator=(const ThreadPlanPython &) = delete;
+ ScriptedThreadPlan(const ScriptedThreadPlan &) = delete;
+ const ScriptedThreadPlan &operator=(const ScriptedThreadPlan &) = delete;
};
} // namespace lldb_private
-#endif // LLDB_TARGET_THREADPLANPYTHON_H
+#endif // LLDB_TARGET_SCRIPTEDTHREADPLAN_H
diff --git a/lldb/source/API/SBThreadPlan.cpp b/lldb/source/API/SBThreadPlan.cpp
index f756bca3176ed..083a050de8a38 100644
--- a/lldb/source/API/SBThreadPlan.cpp
+++ b/lldb/source/API/SBThreadPlan.cpp
@@ -21,12 +21,12 @@
#include "lldb/Symbol/SymbolContext.h"
#include "lldb/Target/Process.h"
#include "lldb/Target/Queue.h"
+#include "lldb/
[Lldb-commits] [lldb] [lldb][test] Support remote run of Shell tests (PR #95986)
https://github.com/labath commented: > Looks good to me. @labath should give this another look as the biggest > skeptic here :) My position remains unchanged. I don't think we should have this functionality. I'm not going to block you adding it either.. https://github.com/llvm/llvm-project/pull/95986 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Support remote run of Shell tests (PR #95986)
@@ -165,6 +235,14 @@ def use_support_substitutions(config):
if config.cmake_sysroot:
host_flags += ["--sysroot={}".format(config.cmake_sysroot)]
+if config.enable_remote and config.has_libcxx:
+host_flags += [
+"-L{}".format(config.libcxx_libs_dir),
+"-Wl,-rpath,{}".format(config.libcxx_libs_dir),
+"-lc++",
+"-lc++abi",
labath wrote:
These flags don't seem very consistent. -rpath is only useful for dynamic links
but I think you're using static links now. And since we removed -lc++abi (which
is only needed for *some* static links) from the makefile, I don't think it
should be used here as well.
https://github.com/llvm/llvm-project/pull/95986
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Support remote run of Shell tests (PR #95986)
https://github.com/labath edited https://github.com/llvm/llvm-project/pull/95986 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/Target] Rename ThreadPlanPython into ScriptedThreadPlan (PR #101931)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Med Ismail Bennani (medismailben)
Changes
Following 9a9ec228cdcf, since the ThreadPlanPython class started making use of
the Scripted Interface instead of calling directly into the python methods,
this class can work with other scripting languages (as long as someone add the
interfact for that language ;p).
So it doesn't make sense anymore for it to keep this name and also we should
avoid having language specific related classes outside the plugin directory.
This patch renames the internal class from `ThreadPlanPython` to
`ScriptedThreadPlan` as its advertised externally, and also updates the various
log messages.
This should hopefully make the codebase more coherent.
---
Full diff: https://github.com/llvm/llvm-project/pull/101931.diff
5 Files Affected:
- (renamed) lldb/include/lldb/Target/ScriptedThreadPlan.h (+11-16)
- (modified) lldb/source/API/SBThreadPlan.cpp (+5-5)
- (modified) lldb/source/Target/CMakeLists.txt (+1-1)
- (renamed) lldb/source/Target/ScriptedThreadPlan.cpp (+40-41)
- (modified) lldb/source/Target/Thread.cpp (+2-2)
``diff
diff --git a/lldb/include/lldb/Target/ThreadPlanPython.h
b/lldb/include/lldb/Target/ScriptedThreadPlan.h
similarity index 77%
rename from lldb/include/lldb/Target/ThreadPlanPython.h
rename to lldb/include/lldb/Target/ScriptedThreadPlan.h
index da106faf951db..7532b6b0a41c5 100644
--- a/lldb/include/lldb/Target/ThreadPlanPython.h
+++ b/lldb/include/lldb/Target/ScriptedThreadPlan.h
@@ -1,5 +1,4 @@
-//===-- ThreadPlanPython.h *- C++
-//-*-===//
+//===-- ScriptedThreadPlan.h *- C++
-*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -7,8 +6,8 @@
//
//===--===//
-#ifndef LLDB_TARGET_THREADPLANPYTHON_H
-#define LLDB_TARGET_THREADPLANPYTHON_H
+#ifndef LLDB_TARGET_SCRIPTEDTHREADPLAN_H
+#define LLDB_TARGET_SCRIPTEDTHREADPLAN_H
#include
@@ -27,13 +26,10 @@
namespace lldb_private {
-// ThreadPlanPython:
-//
-
-class ThreadPlanPython : public ThreadPlan {
+class ScriptedThreadPlan : public ThreadPlan {
public:
- ThreadPlanPython(Thread &thread, const char *class_name,
- const StructuredDataImpl &args_data);
+ ScriptedThreadPlan(Thread &thread, const char *class_name,
+ const StructuredDataImpl &args_data);
void GetDescription(Stream *s, lldb::DescriptionLevel level) override;
@@ -52,15 +48,14 @@ class ThreadPlanPython : public ThreadPlan {
void DidPush() override;
bool IsPlanStale() override;
-
- bool DoWillResume(lldb::StateType resume_state, bool current_plan) override;
+ bool DoWillResume(lldb::StateType resume_state, bool current_plan) override;
protected:
bool DoPlanExplainsStop(Event *event_ptr) override;
lldb::StateType GetPlanRunState() override;
-
+
ScriptInterpreter *GetScriptInterpreter();
private:
@@ -73,10 +68,10 @@ class ThreadPlanPython : public ThreadPlan {
bool m_stop_others;
lldb::ScriptedThreadPlanInterfaceSP m_interface;
- ThreadPlanPython(const ThreadPlanPython &) = delete;
- const ThreadPlanPython &operator=(const ThreadPlanPython &) = delete;
+ ScriptedThreadPlan(const ScriptedThreadPlan &) = delete;
+ const ScriptedThreadPlan &operator=(const ScriptedThreadPlan &) = delete;
};
} // namespace lldb_private
-#endif // LLDB_TARGET_THREADPLANPYTHON_H
+#endif // LLDB_TARGET_SCRIPTEDTHREADPLAN_H
diff --git a/lldb/source/API/SBThreadPlan.cpp b/lldb/source/API/SBThreadPlan.cpp
index f756bca3176ed..083a050de8a38 100644
--- a/lldb/source/API/SBThreadPlan.cpp
+++ b/lldb/source/API/SBThreadPlan.cpp
@@ -21,12 +21,12 @@
#include "lldb/Symbol/SymbolContext.h"
#include "lldb/Target/Process.h"
#include "lldb/Target/Queue.h"
+#include "lldb/Target/ScriptedThreadPlan.h"
#include "lldb/Target/StopInfo.h"
#include "lldb/Target/SystemRuntime.h"
#include "lldb/Target/Target.h"
#include "lldb/Target/Thread.h"
#include "lldb/Target/ThreadPlan.h"
-#include "lldb/Target/ThreadPlanPython.h"
#include "lldb/Target/ThreadPlanStepInRange.h"
#include "lldb/Target/ThreadPlanStepInstruction.h"
#include "lldb/Target/ThreadPlanStepOut.h"
@@ -66,8 +66,8 @@ SBThreadPlan::SBThreadPlan(lldb::SBThread &sb_thread, const
char *class_name) {
Thread *thread = sb_thread.get();
if (thread)
-m_opaque_wp = std::make_shared(*thread, class_name,
- StructuredDataImpl());
+m_opaque_wp = std::make_shared(*thread, class_name,
+ StructuredDataImpl());
}
SBThreadPlan::SBThreadPlan(lldb::SBThread &sb_thread, const char *class_name,
@@ -76,8 +76,8 @@ SBThreadPlan::SBThreadPlan(lldb::SBThread &sb_thread, const
char *class_name,
[Lldb-commits] [lldb] [lldb][TypeSystemClang] Create VLAs of explicitly 0-size as ConstantArrayType (PR #100710)
labath wrote: > Out of curiosity, what's stopping us from using `lld` on Windows? It's not the default configuration of clang -- anywhere. The difference is that on linux/elf, the choice of linker has very little impact on the debug info. https://github.com/llvm/llvm-project/pull/100710 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [WIP] [lldb][ClangExpressionParser] Set BuiltinHeadersInSystemModules depending on SDK version (PR #101778)
https://github.com/Michael137 ready_for_review https://github.com/llvm/llvm-project/pull/101778 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [WIP] [lldb][ClangExpressionParser] Set BuiltinHeadersInSystemModules depending on SDK version (PR #101778)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Michael Buch (Michael137)
Changes
This patch addresses an issue that will arise with future SDKs. The
ClangExpressionParser currently unconditionally sets
`-fbuiltin-headers-in-system-modules` when evaluating expressions with the
`target.import-std-module` setting. This flag should, however, be set depending
on the SDK version (which is what the Clang Darwin toolchain does).
Unfortunately, the compiler instance that we create with
`ClangExpressionParser` never consults the Clang driver, and thus doesn't
correctly infer `BuiltinHeadersInSystemModules`. Note, this isn't an issue with
the `CompilerInstance` that the `ClangModulesDeclVendor` creates because it
uses the `createInovcation` API, which calls into
`Darwin::addClangTargetOptions`.
This patch mimicks how `sdkSupportsBuiltinModules` is used in
`Darwin::addClangTargetOptions`.
This ensures that the `import-std-module` API tests run cleanly regardless of
SDK version.
The plan is to eventually make the `CompilerInstance` construction in
`ClangExpressionParser` go through the driver, so we can avoid duplicating the
logic in LLDB. But we aren't there yet.
**Implementation**
* We look for the `SDKSettings.json` in the sysroot directory that we found in
DWARF (via `DW_AT_LLVM_sysroot`)
* Then parse this file and extract the SDK version number out of it
* Then mimick `sdkSupportsBuiltinModules` from `Toolchains/Darwin.cpp` and set
`BuiltinHeadersInSystemModules` based on it
rdar://116490281
---
Full diff: https://github.com/llvm/llvm-project/pull/101778.diff
1 Files Affected:
- (modified)
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp (+89-3)
``diff
diff --git
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index 2a8bdf29314e4..e05cd649bb044 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -11,6 +11,7 @@
#include "clang/AST/ExternalASTSource.h"
#include "clang/AST/PrettyPrinter.h"
#include "clang/Basic/Builtins.h"
+#include "clang/Basic/DarwinSDKInfo.h"
#include "clang/Basic/DiagnosticIDs.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/TargetInfo.h"
@@ -561,7 +562,85 @@ static void SetupLangOpts(CompilerInstance &compiler,
lang_opts.NoBuiltin = true;
}
-static void SetupImportStdModuleLangOpts(CompilerInstance &compiler) {
+// NOTE: should be kept in sync with sdkSupportsBuiltinModules in
+// Toolchains/Darwin.cpp
+static bool
+sdkSupportsBuiltinModulesImpl(const llvm::Triple &triple,
+ const std::optional &SDKInfo) {
+ if (!SDKInfo)
+return false;
+
+ VersionTuple SDKVersion = SDKInfo->getVersion();
+ switch (triple.getOS()) {
+ case Triple::OSType::MacOSX:
+return SDKVersion >= VersionTuple(15U);
+ case Triple::OSType::IOS:
+return SDKVersion >= VersionTuple(18U);
+ case Triple::OSType::TvOS:
+return SDKVersion >= VersionTuple(18U);
+ case Triple::OSType::WatchOS:
+return SDKVersion >= VersionTuple(11U);
+ case Triple::OSType::XROS:
+return SDKVersion >= VersionTuple(2U);
+ default:
+// New SDKs support builtin modules from the start.
+return true;
+ }
+}
+
+/// Returns true if the SDK for the specified triple supports
+/// builtin modules in system headers. This is used to decide
+/// whether to pass -fbuiltin-headers-in-system-modules to
+/// the compiler instance when compiling the `std` module.
+///
+/// This function assumes one of the directories in \ref include_dirs
+/// is an SDK path that exists on the host. The SDK version is
+/// read from the SDKSettings.json in that directory.
+///
+/// \param[in] triple The target triple.
+/// \param[in] include_dirs The include directories the compiler will use
+/// during header search.
+///
+static bool
+sdkSupportsBuiltinModules(llvm::Triple const &triple,
+ std::vector const &include_dirs) {
+ // Find an SDK directory.
+ static constexpr std::string_view s_sdk_suffix = ".sdk";
+ auto it = llvm::find_if(include_dirs, [](std::string const &path) {
+return path.find(s_sdk_suffix) != std::string::npos;
+ });
+ if (it == include_dirs.end())
+return false;
+
+ auto VFS = FileSystem::Instance().GetVirtualFileSystem();
+ if (!VFS)
+return false;
+
+ // Extract: /path/to/some.sdk
+ size_t suffix = it->find(s_sdk_suffix);
+ assert(suffix != std::string::npos);
+ std::string sdk_path = it->substr(0, suffix + s_sdk_suffix.size());
+
+ // Extract SDK version from the /path/to/some.sdk/SDKSettings.json
+ auto parsed = clang::parseDarwinSDKInfo(*VFS, sdk_path);
+ if (!parsed)
+return false;
+
+ return sdkSupportsBuiltinModulesImpl(triple, *parsed);
+}
+
+/// Sets the LangOptions to prepare for running with `import-std-module`
[Lldb-commits] [lldb] [lldb][ClangExpressionParser] Set BuiltinHeadersInSystemModules depending on SDK version (PR #101778)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/101778 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/Interpreter] Fix ambiguous partial command resolution (PR #101934)
https://github.com/medismailben created
https://github.com/llvm/llvm-project/pull/101934
This patch is a follow-up to #97263 that fix ambigous abbreviated command
resolution.
When multiple commands are resolved, instead of failing to pick a command to
run, this patch changes to resolution logic to check if there is either a
single user command match or a single alias match and if so, it will run that
command instead of the others.
This has as a side-effect that we don't need to make aliases for every
substring of aliases to support abbrivated alias resolution.
>From 8c8607d723ce5ebe9f37484dd26be38aa513fa7d Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani
Date: Sun, 4 Aug 2024 00:41:31 -0700
Subject: [PATCH] [lldb/Interpreter] Fix ambiguous partial command resolution
This patch is a follow-up to #97263 that fix ambigous abbreviated
command resolution.
When multiple commands are resolved, instead of failing to pick a
command to run, this patch changes to resolution logic to check if there
is either a single user command match or a single alias match and if so,
it will run that command instead of the others.
This has as a side-effect that we don't need to make aliases for every
substring of aliases to support abbrivated alias resolution.
Signed-off-by: Med Ismail Bennani
---
.../lldb/Interpreter/CommandInterpreter.h | 4 +
.../source/Interpreter/CommandInterpreter.cpp | 87 +++
2 files changed, 72 insertions(+), 19 deletions(-)
diff --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h
b/lldb/include/lldb/Interpreter/CommandInterpreter.h
index 48f6618ab0e39..2bafc30cc8e23 100644
--- a/lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -295,6 +295,10 @@ class CommandInterpreter : public Broadcaster,
StringList *matches = nullptr,
StringList *descriptions = nullptr)
const;
+ CommandObject *
+ GetAliasCommandObject(llvm::StringRef cmd, StringList *matches = nullptr,
+StringList *descriptions = nullptr) const;
+
/// Determine whether a root level, built-in command with this name exists.
bool CommandExists(llvm::StringRef cmd) const;
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp
b/lldb/source/Interpreter/CommandInterpreter.cpp
index fc07168b6c0ac..0fd3dfcb73ed1 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -520,10 +520,6 @@ void CommandInterpreter::Initialize() {
cmd_obj_sp = GetCommandSPExact("scripting run");
if (cmd_obj_sp) {
-AddAlias("sc", cmd_obj_sp);
-AddAlias("scr", cmd_obj_sp);
-AddAlias("scri", cmd_obj_sp);
-AddAlias("scrip", cmd_obj_sp);
AddAlias("script", cmd_obj_sp);
}
@@ -1302,6 +1298,36 @@ CommandObject *CommandInterpreter::GetUserCommandObject(
return {};
}
+CommandObject *CommandInterpreter::GetAliasCommandObject(
+llvm::StringRef cmd, StringList *matches, StringList *descriptions) const {
+ std::string cmd_str(cmd);
+ auto find_exact = [&](const CommandObject::CommandMap &map) {
+auto found_elem = map.find(std::string(cmd));
+if (found_elem == map.end())
+ return (CommandObject *)nullptr;
+CommandObject *exact_cmd = found_elem->second.get();
+if (exact_cmd) {
+ if (matches)
+matches->AppendString(exact_cmd->GetCommandName());
+ if (descriptions)
+descriptions->AppendString(exact_cmd->GetHelp());
+ return exact_cmd;
+}
+return (CommandObject *)nullptr;
+ };
+
+ CommandObject *exact_cmd = find_exact(GetAliases());
+ if (exact_cmd)
+return exact_cmd;
+
+ // We didn't have an exact command, so now look for partial matches.
+ StringList tmp_list;
+ StringList *matches_ptr = matches ? matches : &tmp_list;
+ AddNamesMatchingPartialString(GetAliases(), cmd_str, *matches_ptr);
+
+ return {};
+}
+
bool CommandInterpreter::CommandExists(llvm::StringRef cmd) const {
return m_command_dict.find(std::string(cmd)) != m_command_dict.end();
}
@@ -3421,6 +3447,19 @@ CommandInterpreter::ResolveCommandImpl(std::string
&command_line,
std::string next_word;
StringList matches;
bool done = false;
+
+ auto build_alias_cmd = [&](std::string &full_name) {
+revised_command_line.Clear();
+matches.Clear();
+std::string alias_result;
+cmd_obj =
+BuildAliasResult(full_name, scratch_command, alias_result, result);
+revised_command_line.Printf("%s", alias_result.c_str());
+if (cmd_obj) {
+ wants_raw_input = cmd_obj->WantsRawCommandString();
+}
+ };
+
while (!done) {
char quote_char = '\0';
std::string suffix;
@@ -3432,14 +3471,7 @@ CommandInterpreter::ResolveCommandImpl(std::string
&command_line,
bool is_real_command =
(!is_alias) || (cmd_obj != nullptr && !cmd_obj->IsAlias());
if (!is_real_command) {
-matches
[Lldb-commits] [lldb] [lldb/Interpreter] Fix ambiguous partial command resolution (PR #101934)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Med Ismail Bennani (medismailben)
Changes
This patch is a follow-up to #97263 that fix ambigous abbreviated
command resolution.
When multiple commands are resolved, instead of failing to pick a command to
run, this patch changes to resolution logic to check if there is either a
single user command match or a single alias match and if so, it will run that
command instead of the others.
This has as a side-effect that we don't need to make aliases for every
substring of aliases to support abbrivated alias resolution.
---
Full diff: https://github.com/llvm/llvm-project/pull/101934.diff
2 Files Affected:
- (modified) lldb/include/lldb/Interpreter/CommandInterpreter.h (+4)
- (modified) lldb/source/Interpreter/CommandInterpreter.cpp (+68-19)
``diff
diff --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h
b/lldb/include/lldb/Interpreter/CommandInterpreter.h
index 48f6618ab0e39..2bafc30cc8e23 100644
--- a/lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -295,6 +295,10 @@ class CommandInterpreter : public Broadcaster,
StringList *matches = nullptr,
StringList *descriptions = nullptr)
const;
+ CommandObject *
+ GetAliasCommandObject(llvm::StringRef cmd, StringList *matches = nullptr,
+StringList *descriptions = nullptr) const;
+
/// Determine whether a root level, built-in command with this name exists.
bool CommandExists(llvm::StringRef cmd) const;
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp
b/lldb/source/Interpreter/CommandInterpreter.cpp
index fc07168b6c0ac..0fd3dfcb73ed1 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -520,10 +520,6 @@ void CommandInterpreter::Initialize() {
cmd_obj_sp = GetCommandSPExact("scripting run");
if (cmd_obj_sp) {
-AddAlias("sc", cmd_obj_sp);
-AddAlias("scr", cmd_obj_sp);
-AddAlias("scri", cmd_obj_sp);
-AddAlias("scrip", cmd_obj_sp);
AddAlias("script", cmd_obj_sp);
}
@@ -1302,6 +1298,36 @@ CommandObject *CommandInterpreter::GetUserCommandObject(
return {};
}
+CommandObject *CommandInterpreter::GetAliasCommandObject(
+llvm::StringRef cmd, StringList *matches, StringList *descriptions) const {
+ std::string cmd_str(cmd);
+ auto find_exact = [&](const CommandObject::CommandMap &map) {
+auto found_elem = map.find(std::string(cmd));
+if (found_elem == map.end())
+ return (CommandObject *)nullptr;
+CommandObject *exact_cmd = found_elem->second.get();
+if (exact_cmd) {
+ if (matches)
+matches->AppendString(exact_cmd->GetCommandName());
+ if (descriptions)
+descriptions->AppendString(exact_cmd->GetHelp());
+ return exact_cmd;
+}
+return (CommandObject *)nullptr;
+ };
+
+ CommandObject *exact_cmd = find_exact(GetAliases());
+ if (exact_cmd)
+return exact_cmd;
+
+ // We didn't have an exact command, so now look for partial matches.
+ StringList tmp_list;
+ StringList *matches_ptr = matches ? matches : &tmp_list;
+ AddNamesMatchingPartialString(GetAliases(), cmd_str, *matches_ptr);
+
+ return {};
+}
+
bool CommandInterpreter::CommandExists(llvm::StringRef cmd) const {
return m_command_dict.find(std::string(cmd)) != m_command_dict.end();
}
@@ -3421,6 +3447,19 @@ CommandInterpreter::ResolveCommandImpl(std::string
&command_line,
std::string next_word;
StringList matches;
bool done = false;
+
+ auto build_alias_cmd = [&](std::string &full_name) {
+revised_command_line.Clear();
+matches.Clear();
+std::string alias_result;
+cmd_obj =
+BuildAliasResult(full_name, scratch_command, alias_result, result);
+revised_command_line.Printf("%s", alias_result.c_str());
+if (cmd_obj) {
+ wants_raw_input = cmd_obj->WantsRawCommandString();
+}
+ };
+
while (!done) {
char quote_char = '\0';
std::string suffix;
@@ -3432,14 +3471,7 @@ CommandInterpreter::ResolveCommandImpl(std::string
&command_line,
bool is_real_command =
(!is_alias) || (cmd_obj != nullptr && !cmd_obj->IsAlias());
if (!is_real_command) {
-matches.Clear();
-std::string alias_result;
-cmd_obj =
-BuildAliasResult(full_name, scratch_command, alias_result, result);
-revised_command_line.Printf("%s", alias_result.c_str());
-if (cmd_obj) {
- wants_raw_input = cmd_obj->WantsRawCommandString();
-}
+build_alias_cmd(full_name);
} else {
if (cmd_obj) {
llvm::StringRef cmd_name = cmd_obj->GetCommandName();
@@ -3486,21 +3518,38 @@ CommandInterpreter::ResolveCommandImpl(std::string
&command_line,
if (cmd_obj == nullptr) {
const size_t num_matches = matches.GetSize();
if (matches.GetSize() >
[Lldb-commits] [lldb] [lldb] Rename `scripting template` to `scripting extension` (NFC) (PR #101935)
https://github.com/medismailben created
https://github.com/llvm/llvm-project/pull/101935
This patch renames the `scripting template` subcommand to be `scripting
extension` instead since that would make more sense for upcoming commands.
>From e398844b5482d097250f0f8b96adf23e548f2927 Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani
Date: Sat, 3 Aug 2024 22:28:23 -0700
Subject: [PATCH] [lldb] Rename `scripting template` to `scripting extension`
(NFC)
This patch renames the `scripting template` subcommand to be `scripting
extension` instead since that would make more sense for upcoming
commands.
Signed-off-by: Med Ismail Bennani
---
.../Commands/CommandObjectScripting.cpp | 38 +--
lldb/source/Commands/Options.td | 4 +-
... => command-scripting-extension-list.test} | 4 +-
3 files changed, 23 insertions(+), 23 deletions(-)
rename lldb/test/Shell/Commands/{command-scripting-template-list.test =>
command-scripting-extension-list.test} (96%)
diff --git a/lldb/source/Commands/CommandObjectScripting.cpp
b/lldb/source/Commands/CommandObjectScripting.cpp
index 730a190a6e891..698e297520020 100644
--- a/lldb/source/Commands/CommandObjectScripting.cpp
+++ b/lldb/source/Commands/CommandObjectScripting.cpp
@@ -129,18 +129,18 @@ class CommandObjectScriptingRun : public CommandObjectRaw
{
CommandOptions m_options;
};
-#define LLDB_OPTIONS_scripting_template_list
+#define LLDB_OPTIONS_scripting_extension_list
#include "CommandOptions.inc"
-class CommandObjectScriptingTemplateList : public CommandObjectParsed {
+class CommandObjectScriptingExtensionList : public CommandObjectParsed {
public:
- CommandObjectScriptingTemplateList(CommandInterpreter &interpreter)
+ CommandObjectScriptingExtensionList(CommandInterpreter &interpreter)
: CommandObjectParsed(
-interpreter, "scripting template list",
+interpreter, "scripting extension list",
"List all the available scripting extension templates. ",
"scripting template list [--language --]") {}
- ~CommandObjectScriptingTemplateList() override = default;
+ ~CommandObjectScriptingExtensionList() override = default;
Options *GetOptions() override { return &m_options; }
@@ -174,7 +174,7 @@ class CommandObjectScriptingTemplateList : public
CommandObjectParsed {
}
llvm::ArrayRef GetDefinitions() override {
- return llvm::ArrayRef(g_scripting_template_list_options);
+ return llvm::ArrayRef(g_scripting_extension_list_options);
}
lldb::ScriptLanguage m_language = lldb::eScriptLanguageDefault;
@@ -195,8 +195,8 @@ class CommandObjectScriptingTemplateList : public
CommandObjectParsed {
};
size_t num_listed_interface = 0;
-size_t num_templates = PluginManager::GetNumScriptedInterfaces();
-for (size_t i = 0; i < num_templates; i++) {
+size_t num_extensions = PluginManager::GetNumScriptedInterfaces();
+for (size_t i = 0; i < num_extensions; i++) {
llvm::StringRef plugin_name =
PluginManager::GetScriptedInterfaceNameAtIndex(i);
if (plugin_name.empty())
@@ -223,7 +223,7 @@ class CommandObjectScriptingTemplateList : public
CommandObjectParsed {
usages.Dump(s, ScriptedInterfaceUsages::UsageKind::API);
usages.Dump(s, ScriptedInterfaceUsages::UsageKind::CommandInterpreter);
- if (i != num_templates - 1)
+ if (i != num_extensions - 1)
s.EOL();
}
@@ -235,19 +235,19 @@ class CommandObjectScriptingTemplateList : public
CommandObjectParsed {
CommandOptions m_options;
};
-class CommandObjectMultiwordScriptingTemplate : public CommandObjectMultiword {
+class CommandObjectMultiwordScriptingExtension : public CommandObjectMultiword
{
public:
- CommandObjectMultiwordScriptingTemplate(CommandInterpreter &interpreter)
+ CommandObjectMultiwordScriptingExtension(CommandInterpreter &interpreter)
: CommandObjectMultiword(
-interpreter, "scripting template",
-"Commands for operating on the scripting templates.",
-"scripting template []") {
+interpreter, "scripting extension",
+"Commands for operating on the scripting extensions.",
+"scripting extension []") {
LoadSubCommand(
"list",
-CommandObjectSP(new CommandObjectScriptingTemplateList(interpreter)));
+CommandObjectSP(new CommandObjectScriptingExtensionList(interpreter)));
}
- ~CommandObjectMultiwordScriptingTemplate() override = default;
+ ~CommandObjectMultiwordScriptingExtension() override = default;
};
CommandObjectMultiwordScripting::CommandObjectMultiwordScripting(
@@ -258,9 +258,9 @@
CommandObjectMultiwordScripting::CommandObjectMultiwordScripting(
"scripting []") {
LoadSubCommand("run",
CommandObjectSP(new CommandObjectScriptingRun(interpreter)));
- LoadSubCommand("template",
- CommandObjectSP(
- new
[Lldb-commits] [lldb] [lldb] Rename `scripting template` to `scripting extension` (NFC) (PR #101935)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Med Ismail Bennani (medismailben)
Changes
This patch renames the `scripting template` subcommand to be `scripting
extension` instead since that would make more sense for upcoming commands.
---
Full diff: https://github.com/llvm/llvm-project/pull/101935.diff
3 Files Affected:
- (modified) lldb/source/Commands/CommandObjectScripting.cpp (+19-19)
- (modified) lldb/source/Commands/Options.td (+2-2)
- (renamed) lldb/test/Shell/Commands/command-scripting-extension-list.test
(+2-2)
``diff
diff --git a/lldb/source/Commands/CommandObjectScripting.cpp
b/lldb/source/Commands/CommandObjectScripting.cpp
index 730a190a6e891..698e297520020 100644
--- a/lldb/source/Commands/CommandObjectScripting.cpp
+++ b/lldb/source/Commands/CommandObjectScripting.cpp
@@ -129,18 +129,18 @@ class CommandObjectScriptingRun : public CommandObjectRaw
{
CommandOptions m_options;
};
-#define LLDB_OPTIONS_scripting_template_list
+#define LLDB_OPTIONS_scripting_extension_list
#include "CommandOptions.inc"
-class CommandObjectScriptingTemplateList : public CommandObjectParsed {
+class CommandObjectScriptingExtensionList : public CommandObjectParsed {
public:
- CommandObjectScriptingTemplateList(CommandInterpreter &interpreter)
+ CommandObjectScriptingExtensionList(CommandInterpreter &interpreter)
: CommandObjectParsed(
-interpreter, "scripting template list",
+interpreter, "scripting extension list",
"List all the available scripting extension templates. ",
"scripting template list [--language --]") {}
- ~CommandObjectScriptingTemplateList() override = default;
+ ~CommandObjectScriptingExtensionList() override = default;
Options *GetOptions() override { return &m_options; }
@@ -174,7 +174,7 @@ class CommandObjectScriptingTemplateList : public
CommandObjectParsed {
}
llvm::ArrayRef GetDefinitions() override {
- return llvm::ArrayRef(g_scripting_template_list_options);
+ return llvm::ArrayRef(g_scripting_extension_list_options);
}
lldb::ScriptLanguage m_language = lldb::eScriptLanguageDefault;
@@ -195,8 +195,8 @@ class CommandObjectScriptingTemplateList : public
CommandObjectParsed {
};
size_t num_listed_interface = 0;
-size_t num_templates = PluginManager::GetNumScriptedInterfaces();
-for (size_t i = 0; i < num_templates; i++) {
+size_t num_extensions = PluginManager::GetNumScriptedInterfaces();
+for (size_t i = 0; i < num_extensions; i++) {
llvm::StringRef plugin_name =
PluginManager::GetScriptedInterfaceNameAtIndex(i);
if (plugin_name.empty())
@@ -223,7 +223,7 @@ class CommandObjectScriptingTemplateList : public
CommandObjectParsed {
usages.Dump(s, ScriptedInterfaceUsages::UsageKind::API);
usages.Dump(s, ScriptedInterfaceUsages::UsageKind::CommandInterpreter);
- if (i != num_templates - 1)
+ if (i != num_extensions - 1)
s.EOL();
}
@@ -235,19 +235,19 @@ class CommandObjectScriptingTemplateList : public
CommandObjectParsed {
CommandOptions m_options;
};
-class CommandObjectMultiwordScriptingTemplate : public CommandObjectMultiword {
+class CommandObjectMultiwordScriptingExtension : public CommandObjectMultiword
{
public:
- CommandObjectMultiwordScriptingTemplate(CommandInterpreter &interpreter)
+ CommandObjectMultiwordScriptingExtension(CommandInterpreter &interpreter)
: CommandObjectMultiword(
-interpreter, "scripting template",
-"Commands for operating on the scripting templates.",
-"scripting template []") {
+interpreter, "scripting extension",
+"Commands for operating on the scripting extensions.",
+"scripting extension []") {
LoadSubCommand(
"list",
-CommandObjectSP(new CommandObjectScriptingTemplateList(interpreter)));
+CommandObjectSP(new CommandObjectScriptingExtensionList(interpreter)));
}
- ~CommandObjectMultiwordScriptingTemplate() override = default;
+ ~CommandObjectMultiwordScriptingExtension() override = default;
};
CommandObjectMultiwordScripting::CommandObjectMultiwordScripting(
@@ -258,9 +258,9 @@
CommandObjectMultiwordScripting::CommandObjectMultiwordScripting(
"scripting []") {
LoadSubCommand("run",
CommandObjectSP(new CommandObjectScriptingRun(interpreter)));
- LoadSubCommand("template",
- CommandObjectSP(
- new
CommandObjectMultiwordScriptingTemplate(interpreter)));
+ LoadSubCommand("extension",
+ CommandObjectSP(new CommandObjectMultiwordScriptingExtension(
+ interpreter)));
}
CommandObjectMultiwordScripting::~CommandObjectMultiwordScripting() = default;
diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td
index 6e5ed21b22ad8..f050cd2ebb5ae 100644
--- a/lldb/source/Commands
[Lldb-commits] [lldb] [lldb/API] Fix SBStructuredData JSON Array parsing (PR #101929)
https://github.com/DavidSpickett approved this pull request. LGTM. https://github.com/llvm/llvm-project/pull/101929 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/Interpreter] Fix ambiguous partial command resolution (PR #101934)
DavidSpickett wrote: This is for this sort of lookup: ``` (lldb) reg read pc ``` Right? As opposed to tab completion. This needs a test case or at least a clear example as a comment in the code if a test is not possible. It seems fine from how you've described it but these things come across better with a clear example. https://github.com/llvm/llvm-project/pull/101934 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Rename `scripting template` to `scripting extension` (NFC) (PR #101935)
https://github.com/DavidSpickett approved this pull request. So the things listed are "scripting extensions" and then there are templates to get started using those extensions. Makes sense to me. https://github.com/llvm/llvm-project/pull/101935 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
https://github.com/labath commented: Better, but I still have some comments on the implementation. I see you ended up not using the fallible_iterator thing in the end. I'm sort of ok with that, though I think it'd be better to do it that way, as we wouldn't need the upfront array bounds check and we could return partial data where it made sense, https://github.com/llvm/llvm-project/pull/101272 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
https://github.com/labath edited https://github.com/llvm/llvm-project/pull/101272 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -136,6 +136,22 @@ static size_t layout(BlobAllocator &File,
MinidumpYAML::ExceptionStream &S) {
return DataEnd;
}
+static size_t layout(BlobAllocator &File, MinidumpYAML::Memory64ListStream &S)
{
+ size_t BaseRVA = File.tell() + sizeof(minidump::Memory64ListHeader);
labath wrote:
Changing this to 64-bit makes sense to me, but I'd rather do it wholesale and
as a separate patch.
https://github.com/llvm/llvm-project/pull/101272
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -132,6 +140,71 @@ class MinidumpFile : public Binary {
size_t Stride;
};
+ class Memory64ListFacade {
labath wrote:
I don't think you need this facade class (particularly if we can avoid storing
the of the vector thing). llvm APIs of this kind will
typically return `iterator_range` and put all of the logic into
the iterator class.
https://github.com/llvm/llvm-project/pull/101272
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -373,7 +373,6 @@ void yaml::MappingContextTraits::mapping(
void yaml::MappingContextTraits::mapping(
IO &IO, MemoryDescriptor_64 &Memory, BinaryRef &Content) {
mapRequiredHex(IO, "Start of Memory Range", Memory.StartOfMemoryRange);
- mapRequiredHex(IO, "Data Size", Memory.DataSize);
labath wrote:
> Content isn't initialized when the mapping is being configured, so
> binary_size() always resolves to 0.
Are you sure about that? Did you put this line after the line that initializes
the `Content` field?
This pattern really should work, as that's the same one used for
RawContentStream. And now that I've looked into it more closely, I think it'd
be best to do exactly the same thing as RawContentStream does, where a smaller
DataSize field is an error, and a larger value just pads the output with zeroes
(so if you just want a field of a certain size, and don't care about the
contents, you can just put `DataSize: 47` and omit the Content entirely:
```
$ yaml2obj
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -152,21 +225,21 @@ class MinidumpFile : public Binary {
}
/// Return a slice of the given data array, with bounds checking.
- static Expected> getDataSlice(ArrayRef Data,
- size_t Offset, size_t Size);
+ static Expected>
+ getDataSlice(ArrayRef Data, uint64_t Offset, uint64_t Size);
/// Return the slice of the given data array as an array of objects of the
/// given type. The function checks that the input array is large enough to
/// contain the correct number of objects of the given type.
template
static Expected> getDataSliceAs(ArrayRef Data,
- size_t Offset, size_t Count);
+ uint64_t Offset, uint64_t Count);
MinidumpFile(MemoryBufferRef Source, const minidump::Header &Header,
ArrayRef Streams,
DenseMap StreamMap)
: Binary(ID_Minidump, Source), Header(Header), Streams(Streams),
-StreamMap(std::move(StreamMap)) {}
+StreamMap(std::move(StreamMap)) {};
labath wrote:
superfluous semicolon
https://github.com/llvm/llvm-project/pull/101272
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -132,6 +140,71 @@ class MinidumpFile : public Binary {
size_t Stride;
};
+ class Memory64ListFacade {
+struct Memory64Iterator {
+public:
+ Memory64Iterator(size_t Count, uint64_t BaseRVA,
+ const Memory64ListFacade *Parent)
+ : Parent(Parent), BaseRVA(BaseRVA), Count(Count) {};
+
+ const std::pair>
+ operator*() {
+return Parent->Next(this);
+ }
+
+ bool operator==(const Memory64Iterator &R) const {
+return Parent == R.Parent && Count == R.Count;
+ }
+
+ bool operator!=(const Memory64Iterator &R) const { return !(*this == R);
}
+
+private:
+ friend class Memory64ListFacade;
+ const Memory64ListFacade *Parent;
+ uint64_t BaseRVA;
+ size_t Count;
+};
+
+ public:
+Memory64ListFacade(ArrayRef Storage,
+ std::vector Descriptors,
labath wrote:
I don't think this needs to be a vector. The memory for these is held by the
MinidumpFile object, right?
https://github.com/llvm/llvm-project/pull/101272
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -336,3 +336,52 @@ TEST(MinidumpYAML, ExceptionStream_ExtraParameter) {
0xab, 0xad, 0xca, 0xfe}),
*ExpectedContext);
}
+
+TEST(MinidumpYAML, MemoryRegion_64bit) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Memory64List
+Memory Ranges:
+ - Start of Memory Range: 0x7FCF0818283
+Content: '68656c6c6f'
+ - Start of Memory Range: 0x7FFF0818283
+Content: '776f726c64'
+)");
+
+ ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+ object::MinidumpFile &File = **ExpectedFile;
+
+ ASSERT_EQ(1u, File.streams().size());
+
+ Expected> ExpectedMemoryList =
+ File.getMemory64List();
+
+ ASSERT_THAT_EXPECTED(ExpectedMemoryList, Succeeded());
+
+ ArrayRef MemoryList = *ExpectedMemoryList;
+ ASSERT_EQ(2u, MemoryList.size());
+
+ const minidump::MemoryDescriptor_64 &DescOne = MemoryList[0];
+ ASSERT_EQ(0x7FCF0818283u, DescOne.StartOfMemoryRange);
+ ASSERT_EQ(5u, DescOne.DataSize);
+
+ const minidump::MemoryDescriptor_64 &DescTwo = MemoryList[1];
+ ASSERT_EQ(0x7FFF0818283u, DescTwo.StartOfMemoryRange);
+ ASSERT_EQ(5u, DescTwo.DataSize);
+
+ const std::optional> ExpectedContent =
+ File.getRawStream(StreamType::Memory64List);
+ ASSERT_TRUE(ExpectedContent);
+ const size_t ExpectedStreamSize = sizeof(Memory64ListHeader) +
(sizeof(MemoryDescriptor_64) * 2);
+ ASSERT_EQ(ExpectedStreamSize, ExpectedContent->size());
+
+ Expected> DescOneExpectedContentSlice =
File.getRawData(DescOne);
+ ASSERT_THAT_EXPECTED(DescOneExpectedContentSlice, Succeeded());
+ ASSERT_EQ("hello", reinterpret_cast(DescOneExpectedContentSlice->data()));
labath wrote:
For `ASSERT_THAT` assertions, the order isn't interchangeable, and its always
`ASSERT_THAT(Actual, ExpressionMatchingExpected)`. For `ASSERT_EQ` (and
friends), the order is interchangable. LLVM doesn't have strict rules around
that or uses them consistently, but I think the `(Actual, Expected)` order is
more common, and that's what I'd recommend -- if nothing else, then because
it's more consistent with the ASSERT_THAT form.
The reason I'm suggesting ASSERT_THAT is because it's more concise and often
produces better error messages. For example, if you test that a vector of two
elements, and the vector ends up having three, with a sequence like
```
ASSERT_EQ(vector.size(), 2);
ASSERT_EQ(vector[0], something);
ASSERT_EQ(vector[1], something_else);
```
you will only get an error like `2!=3`, whereas `ASSERT_THAT(vector,
ElementsAre(something, something_else))` is much shorter and will print the
full contents of actual and expected vectors (if the elements have a reasonable
operator<<, but the code works even if they don't)
https://github.com/llvm/llvm-project/pull/101272
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -132,6 +140,71 @@ class MinidumpFile : public Binary {
size_t Stride;
};
+ class Memory64ListFacade {
+struct Memory64Iterator {
+public:
+ Memory64Iterator(size_t Count, uint64_t BaseRVA,
+ const Memory64ListFacade *Parent)
+ : Parent(Parent), BaseRVA(BaseRVA), Count(Count) {};
+
+ const std::pair>
+ operator*() {
+return Parent->Next(this);
+ }
+
+ bool operator==(const Memory64Iterator &R) const {
+return Parent == R.Parent && Count == R.Count;
+ }
+
+ bool operator!=(const Memory64Iterator &R) const { return !(*this == R);
}
+
+private:
+ friend class Memory64ListFacade;
+ const Memory64ListFacade *Parent;
+ uint64_t BaseRVA;
+ size_t Count;
+};
+
+ public:
+Memory64ListFacade(ArrayRef Storage,
+ std::vector Descriptors,
+ uint64_t BaseRVA)
+: BaseRVA(BaseRVA), Storage(Storage),
+ Descriptors(std::move(Descriptors)) {};
+
+Memory64Iterator begin() const {
+ return Memory64Iterator(0, BaseRVA, this);
+}
+
+Memory64Iterator end() const {
+ return Memory64Iterator(Descriptors.size(), BaseRVA, this);
+}
+
+size_t size() const { return Descriptors.size(); }
+
+ private:
+uint64_t BaseRVA;
+ArrayRef Storage;
+std::vector Descriptors;
+
+const std::pair>
+Next(Memory64Iterator *Iterator) const {
+ assert(Descriptors.size() > Iterator->Count);
+ minidump::MemoryDescriptor_64 Descriptor = Descriptors[Iterator->Count];
+ ArrayRef Content =
+ Storage.slice(Iterator->BaseRVA, Descriptor.DataSize);
labath wrote:
I think it's worth dropping a short comment saying that the bounds of this
array were checked when this iterator was constructed.
https://github.com/llvm/llvm-project/pull/101272
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -11,6 +11,7 @@ #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/StringExtras.h" +#include "llvm/ADT/fallible_iterator.h" labath wrote: You don't actually use this in the current implementation, right? https://github.com/llvm/llvm-project/pull/101272 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Support remote run of Shell tests (PR #95986)
labath wrote: > > Looks good to me. @labath should give this another look as the biggest > > skeptic here :) > > My position remains unchanged. I don't think we should have this > functionality. I'm not going to block you adding it either.. I was going to leave it at that, but seeing https://github.com/llvm/llvm-project/issues/101710, I have to add this as it illustrates my point brilliantly. Overall, I am more worried about the reproducibility of lldb tests, then their universality. That bug shows we're not able to create a test for a very simple thing (an unaligned stack pointer) that works reliably for everyone. What makes us think we'll be able to do that for tests that execute remotely? This is important as it slows down the velocity of all developers (old and new). In the end, that also reduces test coverage, because people will start to (even proactively) add `REQUIRES: my-system` to their tests to avoid breaking other people's CI. I believe that the loss of test coverage due to these proactive annotations is much bigger than anything that can be gained by running these tests remotely, and we'd be better off focusing on making Shell tests as hermetic/reproducible/etc. as possible rather making them generic so they can run remotely. For me, this just sends the wrong message/sets the wrong incentive. https://github.com/llvm/llvm-project/pull/95986 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
labath wrote: > I don't know what I'm going to do here, and besides armv7 this same problem > will occur for a target that has no instruction-step primitive. Another > common trick is to have enough instruction emulation that you can tell what > instruction will execute next by decoding the current instruction, and > putting a breakpoint there. That has the same effect as armv7's > instruction-step-by-breakpoints -- the reported stop reason from the stub > will be "breakpoint hit". FWIW, lldb-server already does this breakpoint-on-next-instruction dance for arm32 (and other architectures), but I'm not sure what kind of stop reason is reported in this case. Maybe @DavidSpickett has an arm32 machine around for a quick test? I think it'd be pretty easy to change the reported reason to something else in this case, if that helped. https://github.com/llvm/llvm-project/pull/96260 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add a StackFrameRecognizer for the Darwin specific abort_with_payload… (PR #101365)
Michael137 wrote: FYI, this is failing on x86 and matrix macOS bots: https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake/4544/execution/node/97/log/?consoleFull ``` FAIL: LLDB (/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake/lldb-build/bin/clang-x86_64) :: test_abort_with_reason (TestAbortWithPayload.TestAbortWithPayload) Restore dir to: /Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake/lldb-build/tools/lldb/test == FAIL: test_abort_with_payload (TestAbortWithPayload.TestAbortWithPayload) There can be many tests in a test case - describe this test here. -- Traceback (most recent call last): File "/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake/llvm-project/lldb/test/API/macosx/abort_with_payload/TestAbortWithPayload.py", line 20, in test_abort_with_payload self.abort_with_test(True) File "/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake/llvm-project/lldb/test/API/macosx/abort_with_payload/TestAbortWithPayload.py", line 129, in abort_with_test self.assertEqual( AssertionError: 140702053824792 != 46 : payload size value correct Config=x86_64-/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake/lldb-build/bin/clang == FAIL: test_abort_with_reason (TestAbortWithPayload.TestAbortWithPayload) There can be many tests in a test case - describe this test here. -- Traceback (most recent call last): File "/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake/llvm-project/lldb/test/API/macosx/abort_with_payload/TestAbortWithPayload.py", line 26, in test_abort_with_reason self.abort_with_test(False) File "/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake/llvm-project/lldb/test/API/macosx/abort_with_payload/TestAbortWithPayload.py", line 138, in abort_with_test self.assertEqual( AssertionError: 140702053824776 != 0 : Got 0 payload size for reason call Config=x86_64-/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake/lldb-build/bin/clang -- Ran 2 tests in 3.083s FAILED (failures=2) ``` @jimingham could you take a look? https://github.com/llvm/llvm-project/pull/101365 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/Interpreter] Fix ambiguous partial command resolution (PR #101934)
@@ -1302,6 +1298,36 @@ CommandObject *CommandInterpreter::GetUserCommandObject(
return {};
}
+CommandObject *CommandInterpreter::GetAliasCommandObject(
+llvm::StringRef cmd, StringList *matches, StringList *descriptions) const {
+ std::string cmd_str(cmd);
+ auto find_exact = [&](const CommandObject::CommandMap &map) {
+auto found_elem = map.find(std::string(cmd));
+if (found_elem == map.end())
+ return (CommandObject *)nullptr;
+CommandObject *exact_cmd = found_elem->second.get();
+if (exact_cmd) {
+ if (matches)
+matches->AppendString(exact_cmd->GetCommandName());
+ if (descriptions)
+descriptions->AppendString(exact_cmd->GetHelp());
+ return exact_cmd;
+}
+return (CommandObject *)nullptr;
Michael137 wrote:
```suggestion
if (!exact_cmd)
return nullptr;
if (matches)
matches->AppendString(exact_cmd->GetCommandName());
if (descriptions)
descriptions->AppendString(exact_cmd->GetHelp());
return exact_cmd;
```
https://github.com/llvm/llvm-project/pull/101934
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/Interpreter] Fix ambiguous partial command resolution (PR #101934)
@@ -1302,6 +1298,36 @@ CommandObject *CommandInterpreter::GetUserCommandObject(
return {};
}
+CommandObject *CommandInterpreter::GetAliasCommandObject(
+llvm::StringRef cmd, StringList *matches, StringList *descriptions) const {
+ std::string cmd_str(cmd);
+ auto find_exact = [&](const CommandObject::CommandMap &map) {
+auto found_elem = map.find(std::string(cmd));
+if (found_elem == map.end())
+ return (CommandObject *)nullptr;
+CommandObject *exact_cmd = found_elem->second.get();
+if (exact_cmd) {
+ if (matches)
+matches->AppendString(exact_cmd->GetCommandName());
+ if (descriptions)
+descriptions->AppendString(exact_cmd->GetHelp());
+ return exact_cmd;
+}
+return (CommandObject *)nullptr;
+ };
+
+ CommandObject *exact_cmd = find_exact(GetAliases());
+ if (exact_cmd)
+return exact_cmd;
+
+ // We didn't have an exact command, so now look for partial matches.
+ StringList tmp_list;
+ StringList *matches_ptr = matches ? matches : &tmp_list;
+ AddNamesMatchingPartialString(GetAliases(), cmd_str, *matches_ptr);
Michael137 wrote:
```suggestion
AddNamesMatchingPartialString(GetAliases(), cmd, *matches_ptr);
```
https://github.com/llvm/llvm-project/pull/101934
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/Interpreter] Fix ambiguous partial command resolution (PR #101934)
@@ -1302,6 +1298,36 @@ CommandObject *CommandInterpreter::GetUserCommandObject(
return {};
}
+CommandObject *CommandInterpreter::GetAliasCommandObject(
+llvm::StringRef cmd, StringList *matches, StringList *descriptions) const {
+ std::string cmd_str(cmd);
+ auto find_exact = [&](const CommandObject::CommandMap &map) {
+auto found_elem = map.find(std::string(cmd));
Michael137 wrote:
```suggestion
auto find_exact = [&](const CommandObject::CommandMap &map) {
auto found_elem = map.find(cmd.str());
```
(in C++20 we could just pass the StringRef to `find`, but not sure that's the
default version yet)
https://github.com/llvm/llvm-project/pull/101934
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/Interpreter] Fix ambiguous partial command resolution (PR #101934)
@@ -1302,6 +1298,36 @@ CommandObject *CommandInterpreter::GetUserCommandObject(
return {};
}
+CommandObject *CommandInterpreter::GetAliasCommandObject(
+llvm::StringRef cmd, StringList *matches, StringList *descriptions) const {
+ std::string cmd_str(cmd);
+ auto find_exact = [&](const CommandObject::CommandMap &map) {
Michael137 wrote:
Nit:
```suggestion
auto find_exact = [&](const CommandObject::CommandMap &map) -> CommandObject
* {
```
Then you don't need the casts below.
https://github.com/llvm/llvm-project/pull/101934
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
DavidSpickett wrote: I'll apply this PR locally and see what I get. https://github.com/llvm/llvm-project/pull/96260 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed PipeWindows bugs; added Pipe::WriteWithTimeout() (PR #101383)
https://github.com/labath commented: Thanks for doing this. Unsurprisingly, I have inline comments :) https://github.com/llvm/llvm-project/pull/101383 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed PipeWindows bugs; added Pipe::WriteWithTimeout() (PR #101383)
https://github.com/labath edited https://github.com/llvm/llvm-project/pull/101383 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed PipeWindows bugs; added Pipe::WriteWithTimeout() (PR #101383)
@@ -44,8 +42,70 @@ TEST_F(PipeTest, OpenAsReader) {
size_t name_len = name.size();
name += "foobar";
llvm::StringRef name_ref(name.data(), name_len);
+ // Note OpenAsReader() do nothing on Windows, the pipe is already opened for
+ // read and write.
ASSERT_THAT_ERROR(
pipe.OpenAsReader(name_ref, /*child_process_inherit=*/false).ToError(),
llvm::Succeeded());
+
+ ASSERT_TRUE(pipe.CanRead());
+}
+
+TEST_F(PipeTest, WriteWithTimeout) {
+ Pipe pipe;
+ ASSERT_THAT_ERROR(pipe.CreateNew(false).ToError(), llvm::Succeeded());
+ // Note write_chunk_size must be less than the pipe buffer.
+ // The pipe buffer is 1024 for PipeWindows and 4096 for PipePosix.
+ const size_t buf_size = 8192;
+ const size_t write_chunk_size = 256;
+ const size_t read_chunk_size = 300;
+ std::unique_ptr write_buf_ptr(
+ new int32_t[buf_size / sizeof(int32_t)]);
+ int32_t *write_buf = write_buf_ptr.get();
+ std::unique_ptr read_buf_ptr(
+ new int32_t[(buf_size + 100) / sizeof(int32_t)]);
+ int32_t *read_buf = read_buf_ptr.get();
+ for (int i = 0; i < buf_size / sizeof(int32_t); ++i) {
+write_buf[i] = i;
+read_buf[i] = -i;
+ }
+
+ char *write_ptr = (char *)write_buf;
+ size_t write_bytes = 0;
+ char *read_ptr = (char *)read_buf;
+ size_t read_bytes = 0;
+ size_t num_bytes = 0;
+ Status error;
+ while (write_bytes < buf_size) {
+error = pipe.WriteWithTimeout(write_ptr + write_bytes, write_chunk_size,
+ std::chrono::milliseconds(10), num_bytes);
+if (error.Fail()) {
+ ASSERT_TRUE(read_bytes < buf_size);
+ error = pipe.ReadWithTimeout(read_ptr + read_bytes, read_chunk_size,
+ std::chrono::milliseconds(10), num_bytes);
+ if (error.Fail())
+FAIL();
+ else
+read_bytes += num_bytes;
+} else
+ write_bytes += num_bytes;
+ }
+ // Read the rest data.
+ while (read_bytes < buf_size) {
+error = pipe.ReadWithTimeout(read_ptr + read_bytes, buf_size - read_bytes,
+ std::chrono::milliseconds(10), num_bytes);
+if (error.Fail())
+ FAIL();
+else
labath wrote:
ASSERT(error.Success()) << error.AsCString();
https://github.com/llvm/llvm-project/pull/101383
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed PipeWindows bugs; added Pipe::WriteWithTimeout() (PR #101383)
@@ -44,8 +42,70 @@ TEST_F(PipeTest, OpenAsReader) {
size_t name_len = name.size();
name += "foobar";
llvm::StringRef name_ref(name.data(), name_len);
+ // Note OpenAsReader() do nothing on Windows, the pipe is already opened for
labath wrote:
Let's keep this change out of the patch. I think it'd be better to redesign
this API to provide a better abstraction over the differences in system
behavior.
https://github.com/llvm/llvm-project/pull/101383
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed PipeWindows bugs; added Pipe::WriteWithTimeout() (PR #101383)
@@ -105,12 +90,19 @@ Status PipeWindows::CreateNew(llvm::StringRef name,
std::string pipe_path = g_pipe_name_prefix.str();
pipe_path.append(name.str());
+ SECURITY_ATTRIBUTES sa{sizeof(SECURITY_ATTRIBUTES), 0,
+ child_process_inherit ? TRUE : FALSE};
+
// Always open for overlapped i/o. We implement blocking manually in Read
// and Write.
DWORD read_mode = FILE_FLAG_OVERLAPPED;
- m_read = ::CreateNamedPipeA(
- pipe_path.c_str(), PIPE_ACCESS_INBOUND | read_mode,
- PIPE_TYPE_BYTE | PIPE_WAIT, 1, 1024, 1024, 120 * 1000, NULL);
+ m_read =
+ ::CreateNamedPipeA(pipe_path.c_str(), PIPE_ACCESS_INBOUND | read_mode,
+ PIPE_TYPE_BYTE | PIPE_WAIT, 1,
+ 1024, // Out buffer size
+ 1024, // In buffer size
+ 0,// Default timeout in ms, 0 means 50ms
labath wrote:
```suggestion
PIPE_TYPE_BYTE | PIPE_WAIT, /*nMaxInstances=*/1,
/*nOutBufferSize=*/1024,
/*nInBufferSize=*/1024,
/*nDefaultTimeOut=*/0,
```
This is the [llvm
style](https://llvm.org/docs/CodingStandards.html#comment-formatting).
https://github.com/llvm/llvm-project/pull/101383
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed PipeWindows bugs; added Pipe::WriteWithTimeout() (PR #101383)
@@ -44,8 +42,70 @@ TEST_F(PipeTest, OpenAsReader) {
size_t name_len = name.size();
name += "foobar";
llvm::StringRef name_ref(name.data(), name_len);
+ // Note OpenAsReader() do nothing on Windows, the pipe is already opened for
+ // read and write.
ASSERT_THAT_ERROR(
pipe.OpenAsReader(name_ref, /*child_process_inherit=*/false).ToError(),
llvm::Succeeded());
+
+ ASSERT_TRUE(pipe.CanRead());
+}
+
+TEST_F(PipeTest, WriteWithTimeout) {
+ Pipe pipe;
+ ASSERT_THAT_ERROR(pipe.CreateNew(false).ToError(), llvm::Succeeded());
+ // Note write_chunk_size must be less than the pipe buffer.
+ // The pipe buffer is 1024 for PipeWindows and 4096 for PipePosix.
+ const size_t buf_size = 8192;
+ const size_t write_chunk_size = 256;
+ const size_t read_chunk_size = 300;
+ std::unique_ptr write_buf_ptr(
+ new int32_t[buf_size / sizeof(int32_t)]);
+ int32_t *write_buf = write_buf_ptr.get();
+ std::unique_ptr read_buf_ptr(
+ new int32_t[(buf_size + 100) / sizeof(int32_t)]);
+ int32_t *read_buf = read_buf_ptr.get();
+ for (int i = 0; i < buf_size / sizeof(int32_t); ++i) {
+write_buf[i] = i;
+read_buf[i] = -i;
+ }
labath wrote:
```
std::vector write_buf(buf_size / sizeof(int32_t));
std:iota(write_buf.begin(), write_buf.end(), 0);
std::vector read_buf(write_buf.size()+100, -1);
```
or something similar, depending on how the rest of the test ends up looking
like.
The result is shorter, and there's only `/sizeof` in the entire snippet.
https://github.com/llvm/llvm-project/pull/101383
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed PipeWindows bugs; added Pipe::WriteWithTimeout() (PR #101383)
@@ -44,8 +42,70 @@ TEST_F(PipeTest, OpenAsReader) {
size_t name_len = name.size();
name += "foobar";
llvm::StringRef name_ref(name.data(), name_len);
+ // Note OpenAsReader() do nothing on Windows, the pipe is already opened for
+ // read and write.
ASSERT_THAT_ERROR(
pipe.OpenAsReader(name_ref, /*child_process_inherit=*/false).ToError(),
llvm::Succeeded());
+
+ ASSERT_TRUE(pipe.CanRead());
+}
+
+TEST_F(PipeTest, WriteWithTimeout) {
+ Pipe pipe;
+ ASSERT_THAT_ERROR(pipe.CreateNew(false).ToError(), llvm::Succeeded());
+ // Note write_chunk_size must be less than the pipe buffer.
+ // The pipe buffer is 1024 for PipeWindows and 4096 for PipePosix.
labath wrote:
The 4096 is actually system-dependent. It appears to be 512 on darwin, but I
could imagine it can be bigger on some other system. Ideally the test would
actually verify that the assumption is true. This is why I like the test plan I
proposed in the other thread better. An assertion like this can be placed
naturally within the code when it is structured as a sequence of steps, rather
than a loop. E.g. something like this
1. write to the pipe until it is full -- this automatically ensures the above
condition is met
2. attempt a write with a long(ish) timeout, check that it fails, and that it
waits (a sufficient amount of time has passed)
3. attempt a write with a short timeout, check that it does not wait too long
4. drain the pipe
5. check that we got what we wrote
6. write to the pipe again and check that it succeeds
I hate to be a sore, but I think this provides better test coverage, and makes
it clearer about what is being tested.
https://github.com/llvm/llvm-project/pull/101383
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed PipeWindows bugs; added Pipe::WriteWithTimeout() (PR #101383)
@@ -44,8 +42,70 @@ TEST_F(PipeTest, OpenAsReader) {
size_t name_len = name.size();
name += "foobar";
llvm::StringRef name_ref(name.data(), name_len);
+ // Note OpenAsReader() do nothing on Windows, the pipe is already opened for
+ // read and write.
ASSERT_THAT_ERROR(
pipe.OpenAsReader(name_ref, /*child_process_inherit=*/false).ToError(),
llvm::Succeeded());
+
+ ASSERT_TRUE(pipe.CanRead());
+}
+
+TEST_F(PipeTest, WriteWithTimeout) {
+ Pipe pipe;
+ ASSERT_THAT_ERROR(pipe.CreateNew(false).ToError(), llvm::Succeeded());
+ // Note write_chunk_size must be less than the pipe buffer.
+ // The pipe buffer is 1024 for PipeWindows and 4096 for PipePosix.
+ const size_t buf_size = 8192;
+ const size_t write_chunk_size = 256;
+ const size_t read_chunk_size = 300;
+ std::unique_ptr write_buf_ptr(
+ new int32_t[buf_size / sizeof(int32_t)]);
+ int32_t *write_buf = write_buf_ptr.get();
+ std::unique_ptr read_buf_ptr(
+ new int32_t[(buf_size + 100) / sizeof(int32_t)]);
+ int32_t *read_buf = read_buf_ptr.get();
+ for (int i = 0; i < buf_size / sizeof(int32_t); ++i) {
+write_buf[i] = i;
+read_buf[i] = -i;
+ }
+
+ char *write_ptr = (char *)write_buf;
+ size_t write_bytes = 0;
+ char *read_ptr = (char *)read_buf;
+ size_t read_bytes = 0;
+ size_t num_bytes = 0;
+ Status error;
+ while (write_bytes < buf_size) {
+error = pipe.WriteWithTimeout(write_ptr + write_bytes, write_chunk_size,
+ std::chrono::milliseconds(10), num_bytes);
+if (error.Fail()) {
+ ASSERT_TRUE(read_bytes < buf_size);
+ error = pipe.ReadWithTimeout(read_ptr + read_bytes, read_chunk_size,
+ std::chrono::milliseconds(10), num_bytes);
+ if (error.Fail())
+FAIL();
+ else
+read_bytes += num_bytes;
+} else
+ write_bytes += num_bytes;
+ }
+ // Read the rest data.
+ while (read_bytes < buf_size) {
+error = pipe.ReadWithTimeout(read_ptr + read_bytes, buf_size - read_bytes,
+ std::chrono::milliseconds(10), num_bytes);
+if (error.Fail())
+ FAIL();
+else
+ read_bytes += num_bytes;
+ }
+
+ // Be sure the pipe is empty.
+ error = pipe.ReadWithTimeout(read_ptr + read_bytes, 100,
+ std::chrono::milliseconds(10), num_bytes);
+ ASSERT_TRUE(error.Fail());
+
+ // Compare the data.
+ ASSERT_EQ(write_bytes, read_bytes);
+ ASSERT_EQ(memcmp(write_buf, read_buf, buf_size), 0);
labath wrote:
```
read_buf.resize(read_bytes/sizeof)
ASSERT_EQ(read_bytes, write_bytes);
```
point being it avoids low-level operations and has a better chance of producing
a reasonable error msg.
https://github.com/llvm/llvm-project/pull/101383
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/Plugins] Introduce Scripted Platform Plugin (PR #99814)
@@ -0,0 +1,108 @@
+"""
+Test python scripted platform in lldb
+"""
+
+import os, shutil
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbsuite.test import lldbtest
+
+
+class ScriptedPlatformTestCase(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+@skipUnlessDarwin
+def test_python_plugin_package(self):
+"""Test that the lldb python module has a `plugins.scripted_platform`
+package."""
+self.expect(
+"script import lldb.plugins",
+substrs=["ModuleNotFoundError"],
+matching=False,
+)
+
+self.expect("script dir(lldb.plugins)", substrs=["scripted_platform"])
+
+self.expect(
+"script import lldb.plugins.scripted_platform",
+substrs=["ModuleNotFoundError"],
+matching=False,
+)
+
+self.expect(
+"script dir(lldb.plugins.scripted_platform)",
substrs=["ScriptedPlatform"]
+)
+
+self.expect(
+"script from lldb.plugins.scripted_platform import
ScriptedPlatform",
+substrs=["ImportError"],
+matching=False,
+)
+
+self.expect(
+"script dir(ScriptedPlatform)",
+substrs=[
+"attach_to_process",
+"kill_process",
+"launch_process",
+"list_processes",
+],
+)
+
+@skipUnlessDarwin
+def test_list_processes(self):
+"""Test that we can load and select an lldb scripted platform using the
+SBAPI, check its process ID, parent, name & triple.
+"""
+os.environ["SKIP_SCRIPTED_PLATFORM_SELECT"] = "1"
+
+def cleanup():
+del os.environ["SKIP_SCRIPTED_PLATFORM_SELECT"]
+
+self.addTearDownHook(cleanup)
labath wrote:
Sounds good. (you could also do it before hand, as I think this patch is going
to take a couple more iterations).
https://github.com/llvm/llvm-project/pull/99814
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/Plugins] Introduce Scripted Platform Plugin (PR #99814)
@@ -1003,6 +1010,21 @@ class Platform : public PluginInterface {
FileSpec GetModuleCacheRoot();
};
+class PlatformMetadata {
+public:
+ PlatformMetadata(Debugger &debugger, const ScriptedMetadata metadata);
+ ~PlatformMetadata() = default;
+
+ Debugger &GetDebugger() const { return m_debugger; }
+ const ScriptedMetadata GetScriptedMetadata() const {
+return m_scripted_metadata;
+ }
+
+protected:
+ Debugger &m_debugger;
labath wrote:
> I don't see how passing the debugger vs. the script interpreter would make a
> difference if the scripted platform was created with one debugger and copied
> to another debugger platform's list. Even if I held on the script interpreter
> from the first debugger, once copied to the other one, the scripted platform
> will still interact with the script interpreter from the first one, which is
> basically the same as holding to the debugger.
It's basically the same, but it makes the intent clearer, I think.
> I'm not sure if this is a scenario we want to support, may be @jimingham
> would have some opinions about this.
> @labath if you have some ideas how to support this scenario, I'd love to be
> convinced :)
I am not saying we should support this scenario (I'm also not saying we should
not). What I'm saying is that this (creating a platform with one debugger, but
then adding it to another one) is something that can happen, so we should
figure out what to do about it. Do we want to support it or not? Do we want to
prevent it from happening or not?
I don't really have an answer to those, but I think we should have one..
https://github.com/llvm/llvm-project/pull/99814
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/Plugins] Introduce Scripted Platform Plugin (PR #99814)
@@ -180,7 +184,19 @@ class CommandObjectPlatformSelect : public
CommandObjectParsed {
m_interpreter, ArchSpec(), select, error, platform_arch));
if (platform_sp) {
GetDebugger().GetPlatformList().SetSelectedPlatform(platform_sp);
-
+ OptionGroupPythonClassWithDict &script_class_opts =
labath wrote:
Okay, this is better, though a more user-focused error message would be even
better ("platform XXX does not support scripting metadata" ?) I'm less worried
about what happens in the SB API version, though if the operation can fail for
more than one reason, then it might be a good idea to add an `SBError&` to the
appropriate constructor.
https://github.com/llvm/llvm-project/pull/99814
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/API] Fix SBStructuredData JSON Array parsing (PR #101929)
@@ -110,6 +110,23 @@ class MyRandomClass:
self.assertTrue(my_random_class)
self.assertEqual(my_random_class.payload, MyRandomClass.payload)
+example_arr = [1, 2.3, "4", {"5": False}]
+arr_str = json.dumps(example_arr)
+s.Clear()
+s.Print(arr_str)
+example = lldb.SBStructuredData()
+
+# Check SetFromJSON API for dictionaries, integers, floating point
+# values, strings and arrays
+error = example.SetFromJSON(s)
+if not error.Success():
+self.fail("FAILED: " + error.GetCString())
labath wrote:
```suggestion
self.assertSuccess(example.SetFromJSON(s))
```
https://github.com/llvm/llvm-project/pull/101929
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/API] Fix SBStructuredData JSON Array parsing (PR #101929)
https://github.com/labath edited https://github.com/llvm/llvm-project/pull/101929 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/API] Fix SBStructuredData JSON Array parsing (PR #101929)
https://github.com/labath commented: How is the operator bool change related to this change? AFAICT, the pimpl pointer should always be valid. https://github.com/llvm/llvm-project/pull/101929 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/API] Fix SBStructuredData JSON Array parsing (PR #101929)
https://github.com/labath edited https://github.com/llvm/llvm-project/pull/101929 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
labath wrote: > I'll apply this PR locally and see what I get. I think that would be definitely appreciated, but I'll just note that this isn't necessary to answer my question, since the patch doesn't contain any changes to lldb-server. (My question basically was: what is the stop-reply packet for a single step (vCont:s) operation on arm32?) https://github.com/llvm/llvm-project/pull/96260 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed PipeWindows bugs; added Pipe::WriteWithTimeout() (PR #101383)
@@ -44,8 +42,70 @@ TEST_F(PipeTest, OpenAsReader) {
size_t name_len = name.size();
name += "foobar";
llvm::StringRef name_ref(name.data(), name_len);
+ // Note OpenAsReader() do nothing on Windows, the pipe is already opened for
+ // read and write.
ASSERT_THAT_ERROR(
pipe.OpenAsReader(name_ref, /*child_process_inherit=*/false).ToError(),
llvm::Succeeded());
+
+ ASSERT_TRUE(pipe.CanRead());
+}
+
+TEST_F(PipeTest, WriteWithTimeout) {
+ Pipe pipe;
+ ASSERT_THAT_ERROR(pipe.CreateNew(false).ToError(), llvm::Succeeded());
+ // Note write_chunk_size must be less than the pipe buffer.
+ // The pipe buffer is 1024 for PipeWindows and 4096 for PipePosix.
+ const size_t buf_size = 8192;
+ const size_t write_chunk_size = 256;
+ const size_t read_chunk_size = 300;
+ std::unique_ptr write_buf_ptr(
+ new int32_t[buf_size / sizeof(int32_t)]);
+ int32_t *write_buf = write_buf_ptr.get();
+ std::unique_ptr read_buf_ptr(
+ new int32_t[(buf_size + 100) / sizeof(int32_t)]);
+ int32_t *read_buf = read_buf_ptr.get();
+ for (int i = 0; i < buf_size / sizeof(int32_t); ++i) {
+write_buf[i] = i;
+read_buf[i] = -i;
+ }
+
+ char *write_ptr = (char *)write_buf;
+ size_t write_bytes = 0;
+ char *read_ptr = (char *)read_buf;
+ size_t read_bytes = 0;
+ size_t num_bytes = 0;
+ Status error;
+ while (write_bytes < buf_size) {
+error = pipe.WriteWithTimeout(write_ptr + write_bytes, write_chunk_size,
+ std::chrono::milliseconds(10), num_bytes);
+if (error.Fail()) {
+ ASSERT_TRUE(read_bytes < buf_size);
+ error = pipe.ReadWithTimeout(read_ptr + read_bytes, read_chunk_size,
+ std::chrono::milliseconds(10), num_bytes);
+ if (error.Fail())
+FAIL();
+ else
+read_bytes += num_bytes;
+} else
+ write_bytes += num_bytes;
+ }
+ // Read the rest data.
+ while (read_bytes < buf_size) {
+error = pipe.ReadWithTimeout(read_ptr + read_bytes, buf_size - read_bytes,
+ std::chrono::milliseconds(10), num_bytes);
+if (error.Fail())
+ FAIL();
+else
+ read_bytes += num_bytes;
+ }
+
+ // Be sure the pipe is empty.
+ error = pipe.ReadWithTimeout(read_ptr + read_bytes, 100,
+ std::chrono::milliseconds(10), num_bytes);
+ ASSERT_TRUE(error.Fail());
+
+ // Compare the data.
+ ASSERT_EQ(write_bytes, read_bytes);
+ ASSERT_EQ(memcmp(write_buf, read_buf, buf_size), 0);
slydiman wrote:
I do not understand what is the purpose of `read_buf.resize(read_bytes/sizeof)`
you suggested?
We need to compare the content of write_buf and read_buf.
https://github.com/llvm/llvm-project/pull/101383
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Refactor TypeQuery::ContextMatches, take 2 (PR #101333)
https://github.com/labath updated
https://github.com/llvm/llvm-project/pull/101333
>From e87b2b24cd673584aabd00eaf6ad8fc4c0c52c98 Mon Sep 17 00:00:00 2001
From: Pavel Labath
Date: Tue, 16 Jul 2024 14:18:27 +
Subject: [PATCH 1/3] [lldb] Refactor TypeQuery::ContextMatches, take 2
This is an alternative, much simpler implementation of #99305. In this
version I replace the AnyModule wildcard match with a special TypeQuery
flag which achieves (mostly) the same thing.
It is a preparatory step for teaching ContextMatches about anonymous
namespaces. It started out as a way to remove the assumption that the
pattern and target contexts must be of the same length -- that's will
not be correct with anonymous namespaces, and probably isn't even
correct right now for AnyModule matches.
---
lldb/include/lldb/Symbol/Type.h | 13 +--
lldb/include/lldb/lldb-private-enumerations.h | 2 -
lldb/source/Symbol/Type.cpp | 74 ++
.../DWARF/clang-gmodules-type-lookup.c| 5 +-
.../SymbolFile/DWARF/x86/compilercontext.ll | 7 +-
lldb/tools/lldb-test/lldb-test.cpp| 18 +++-
lldb/unittests/Symbol/TestType.cpp| 97 ---
7 files changed, 119 insertions(+), 97 deletions(-)
diff --git a/lldb/include/lldb/Symbol/Type.h b/lldb/include/lldb/Symbol/Type.h
index c6f30cde81867..a35f9974fa39a 100644
--- a/lldb/include/lldb/Symbol/Type.h
+++ b/lldb/include/lldb/Symbol/Type.h
@@ -65,11 +65,6 @@ struct CompilerContext {
llvm::raw_ostream &operator<<(llvm::raw_ostream &os,
const CompilerContext &rhs);
-/// Match \p context_chain against \p pattern, which may contain "Any"
-/// kinds. The \p context_chain should *not* contain any "Any" kinds.
-bool contextMatches(llvm::ArrayRef context_chain,
-llvm::ArrayRef pattern);
-
FLAGS_ENUM(TypeQueryOptions){
e_none = 0u,
/// If set, TypeQuery::m_context contains an exact context that must match
@@ -79,10 +74,13 @@ FLAGS_ENUM(TypeQueryOptions){
/// If set, TypeQuery::m_context is a clang module compiler context. If not
/// set TypeQuery::m_context is normal type lookup context.
e_module_search = (1u << 1),
+/// If set, the query will ignore all Module entries in the type context,
+/// even for exact matches.
+e_ignore_modules = (1u << 2),
/// When true, the find types call should stop the query as soon as a
single
/// matching type is found. When false, the type query should find all
/// matching types.
-e_find_one = (1u << 2),
+e_find_one = (1u << 3),
};
LLDB_MARK_AS_BITMASK_ENUM(TypeQueryOptions)
@@ -264,6 +262,9 @@ class TypeQuery {
bool LanguageMatches(lldb::LanguageType language) const;
bool GetExactMatch() const { return (m_options & e_exact_match) != 0; }
+
+ bool GetIgnoreModules() const { return (m_options & e_ignore_modules) != 0; }
+
/// The \a m_context can be used in two ways: normal types searching with
/// the context containing a stanadard declaration context for a type, or
/// with the context being more complete for exact matches in clang modules.
diff --git a/lldb/include/lldb/lldb-private-enumerations.h
b/lldb/include/lldb/lldb-private-enumerations.h
index 9d18316dcea25..c24a3538f58da 100644
--- a/lldb/include/lldb/lldb-private-enumerations.h
+++ b/lldb/include/lldb/lldb-private-enumerations.h
@@ -207,8 +207,6 @@ enum class CompilerContextKind : uint16_t {
Builtin = 1 << 10,
Any = 1 << 15,
- /// Match 0..n nested modules.
- AnyModule = Any | Module,
/// Match any type.
AnyType = Any | ClassOrStruct | Union | Enum | Typedef | Builtin,
/// Math any declaration context.
diff --git a/lldb/source/Symbol/Type.cpp b/lldb/source/Symbol/Type.cpp
index e76574795733f..eb321407e3734 100644
--- a/lldb/source/Symbol/Type.cpp
+++ b/lldb/source/Symbol/Type.cpp
@@ -6,7 +6,9 @@
//
//===--===//
+#include
#include
+#include
#include
#include "lldb/Core/Module.h"
@@ -30,6 +32,7 @@
#include "lldb/Target/Process.h"
#include "lldb/Target/Target.h"
#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-private-enumerations.h"
#include "llvm/ADT/StringRef.h"
@@ -43,35 +46,6 @@ llvm::raw_ostream
&lldb_private::operator<<(llvm::raw_ostream &os,
return os << lldb_stream.GetString();
}
-bool lldb_private::contextMatches(llvm::ArrayRef
context_chain,
- llvm::ArrayRef pattern) {
- auto ctx = context_chain.begin();
- auto ctx_end = context_chain.end();
- for (const CompilerContext &pat : pattern) {
-// Early exit if the pattern is too long.
-if (ctx == ctx_end)
- return false;
-if (*ctx != pat) {
- // Skip any number of module matches.
- if (pat.kind == CompilerContextKind::AnyModule) {
-// Greedily match 0..n modules.
-ctx = std::find_if(ctx, ctx_end, [](const CompilerContext &ctx) {
-
[Lldb-commits] [lldb] [lldb] Refactor TypeQuery::ContextMatches, take 2 (PR #101333)
@@ -153,19 +127,36 @@ void TypeQuery::SetLanguages(LanguageSet languages) {
bool TypeQuery::ContextMatches(
llvm::ArrayRef context_chain) const {
- if (GetExactMatch() || context_chain.size() == m_context.size())
-return ::contextMatches(context_chain, m_context);
+ auto ctx = context_chain.rbegin(), ctx_end = context_chain.rend();
+ for (auto pat = m_context.rbegin(), pat_end = m_context.rend();
+ pat != pat_end;) {
+
+if (ctx == ctx_end)
+ return false; // Pattern too long.
+
+// See if there is a kind mismatch; they should have 1 bit in common.
+if ((ctx->kind & pat->kind) == CompilerContextKind())
+ return false;
+
+if (ctx->name != pat->name)
+ return false;
+
+++ctx;
+++pat;
+ }
+
+ // Skip over any remaining module entries if we were asked to do that.
+ while (GetIgnoreModules() && ctx != ctx_end &&
labath wrote:
Yeah, I've thought about that when writing this, but I figured that we
shouldn't make the compiler's job too easy :)
I also tried a version with std::find, but that ended up even longer.
https://github.com/llvm/llvm-project/pull/101333
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Refactor TypeQuery::ContextMatches, take 2 (PR #101333)
@@ -153,19 +127,36 @@ void TypeQuery::SetLanguages(LanguageSet languages) {
bool TypeQuery::ContextMatches(
llvm::ArrayRef context_chain) const {
- if (GetExactMatch() || context_chain.size() == m_context.size())
-return ::contextMatches(context_chain, m_context);
+ auto ctx = context_chain.rbegin(), ctx_end = context_chain.rend();
+ for (auto pat = m_context.rbegin(), pat_end = m_context.rend();
labath wrote:
With the modules out of the way, I guess it could be, but then I'd have to
rewrite it again to this form for anonymous namespaces.
https://github.com/llvm/llvm-project/pull/101333
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Refactor TypeQuery::ContextMatches, take 2 (PR #101333)
labath wrote: > Just applied this patch to the Swift plugin and the tests passed without much > additional work. > > I just had to add a `SetIgnoreModules` API. We don't necessarily have to have > that upstream. But it would be nice. I can add that here. Thanks for checking it out. https://github.com/llvm/llvm-project/pull/101333 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed PipeWindows bugs; added Pipe::WriteWithTimeout() (PR #101383)
@@ -44,8 +42,70 @@ TEST_F(PipeTest, OpenAsReader) {
size_t name_len = name.size();
name += "foobar";
llvm::StringRef name_ref(name.data(), name_len);
+ // Note OpenAsReader() do nothing on Windows, the pipe is already opened for
+ // read and write.
ASSERT_THAT_ERROR(
pipe.OpenAsReader(name_ref, /*child_process_inherit=*/false).ToError(),
llvm::Succeeded());
+
+ ASSERT_TRUE(pipe.CanRead());
+}
+
+TEST_F(PipeTest, WriteWithTimeout) {
+ Pipe pipe;
+ ASSERT_THAT_ERROR(pipe.CreateNew(false).ToError(), llvm::Succeeded());
+ // Note write_chunk_size must be less than the pipe buffer.
+ // The pipe buffer is 1024 for PipeWindows and 4096 for PipePosix.
+ const size_t buf_size = 8192;
+ const size_t write_chunk_size = 256;
+ const size_t read_chunk_size = 300;
+ std::unique_ptr write_buf_ptr(
+ new int32_t[buf_size / sizeof(int32_t)]);
+ int32_t *write_buf = write_buf_ptr.get();
+ std::unique_ptr read_buf_ptr(
+ new int32_t[(buf_size + 100) / sizeof(int32_t)]);
+ int32_t *read_buf = read_buf_ptr.get();
+ for (int i = 0; i < buf_size / sizeof(int32_t); ++i) {
+write_buf[i] = i;
+read_buf[i] = -i;
+ }
+
+ char *write_ptr = (char *)write_buf;
+ size_t write_bytes = 0;
+ char *read_ptr = (char *)read_buf;
+ size_t read_bytes = 0;
+ size_t num_bytes = 0;
+ Status error;
+ while (write_bytes < buf_size) {
+error = pipe.WriteWithTimeout(write_ptr + write_bytes, write_chunk_size,
+ std::chrono::milliseconds(10), num_bytes);
+if (error.Fail()) {
+ ASSERT_TRUE(read_bytes < buf_size);
+ error = pipe.ReadWithTimeout(read_ptr + read_bytes, read_chunk_size,
+ std::chrono::milliseconds(10), num_bytes);
+ if (error.Fail())
+FAIL();
+ else
+read_bytes += num_bytes;
+} else
+ write_bytes += num_bytes;
+ }
+ // Read the rest data.
+ while (read_bytes < buf_size) {
+error = pipe.ReadWithTimeout(read_ptr + read_bytes, buf_size - read_bytes,
+ std::chrono::milliseconds(10), num_bytes);
+if (error.Fail())
+ FAIL();
+else
+ read_bytes += num_bytes;
+ }
+
+ // Be sure the pipe is empty.
+ error = pipe.ReadWithTimeout(read_ptr + read_bytes, 100,
+ std::chrono::milliseconds(10), num_bytes);
+ ASSERT_TRUE(error.Fail());
+
+ // Compare the data.
+ ASSERT_EQ(write_bytes, read_bytes);
+ ASSERT_EQ(memcmp(write_buf, read_buf, buf_size), 0);
labath wrote:
Sorry, I messed that comment up. The second line was supposed to be
`ASSERT_EQ(read_buf, write_buf)`, and the purpose of the resize is to ensure
the vector contains only the data that we've actually read (so that it can then
be compared using operator==).
(if you can ensure the vector is of the same size already, then you can
obviously skip the resize step.)
https://github.com/llvm/llvm-project/pull/101383
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
labath wrote: > @labath > > > This is where I'll have to disagree with you. The exec following the fork > > is not "redundant" on linux. A naked fork is an extremely dangerous thing > > in todays (multithreaded) applications. I'm absolutely certain that the > > improper use of fork with threads (and the FD leaks that go along with it) > > is the cause of at least one of the issues (ETXTBSY) you refer to as > > "system bugs", and fairly sure it's also the cause of the other one (the > > pipe workaround) as well. In a multithreaded application (which > > lldb-platform isn't right now, but more of an accident than design, and > > which you've tried to make extremely multithreaded) the only safe use of > > fork is if it is immediately followed by an exec, and this has to be done > > in an extremely careful way (we try to do that, and we still failed). > > I meant https://bugzilla.kernel.org/show_bug.cgi?id=546 and [this > discussion](https://github.com/llvm/llvm-project/pull/100670/files#r1693201568). > I'm sad that you don't believe. I know what you're referring to. However, that's a statement that comes with a very heavy burden of proof, one that you simply have not provided. The number of users of linux kernel exceeds the users of lldb by many orders of magnitude, so it's very unlikely that the problem is on their side. It's not that it's impossible -- I personally have tracked down lldb problems down to bugs in our dependencies -- from the libstdc++ to the kernel. However, this just doesn't look like one of those cases. > I concluded that no one uses the real multithreading on Linux. That's another very bold statement. I am not sure what you meant by "real multithreading", but I know for a fact that some Linux applications run thousands of threads in one process without any issues. All of them are very careful about how they use fork though. > Someone tried and faced unsolvable problems. Therefore, #100670 is doomed. > > > But even if it weren't for all of this, I'd still think it makes sense to > > start a new process, just for symmetry with windows. That's the nature of > > portable programming. > > Not all code may be portable. It is better to use threads on Windows, but not > on Posix. fork() is missing on Windows and starting a new process is a > workaround. When we have the accepted socket and everything is ready to just > handle the connection on Posix (after fork), you suggest to drop all > initialized parameters, start a new process, pass all necessary parameters > via the command line, initialize all parameters again, go to the same point > and finally handle the connection. What is the reason? To be similar how it > will work on Windows. That is exactly what I'm suggesting. > No, I don't agree. Well, at least we can agree to disagree. ;) > You are worried about some FD. But the main process `lldb-server platform` > does not launch anything. There are no any FDs which may leak. It forks. Forking duplicates any FD opened into the parent into the child. Even those marked with CLOEXEC, which is the usual way to guard against FD leakage. If you're single-threaded than this is manageable, but as soon as you have multiple threads (believe it or not, many applications on linux use multiple threads, and a lot of code creates background threads; lldb-server platform code executed here is pretty small, but it still calls into a lot of code that could create those), fork() becomes a ticking time bomb. In a multithreaded application, the only code that can be safely run after a fork, is that which can run in a signal handler. You can't write a server like that. Using fork to serve multiple connections is a pattern from the seventies. Modern applications don't do it this way. As far as I'm concerned, this code should not have been written this way (and if the author cared about windows, they would have never done it), and I'd support removing this pattern even if it weren't for windows. > We can't unify socket sharing nicely for Windows and Posix. We can try to > combine the common code in the part 2 of this patch when the socket sharing > will be added to GDBRemoteCommunication::StartDebugserverProcess() too. That's nice, but I think we should figure out how to reduce the number of ifdefs in this patch. Porting linux away from fork may not be your concern, but figuring out how to make the code less branchy is. If you can do that without removing forks, I can take it upon myself to handle the rest. I just think it would be easier to do it both together... https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed PipeWindows bugs; added Pipe::WriteWithTimeout() (PR #101383)
https://github.com/slydiman updated
https://github.com/llvm/llvm-project/pull/101383
>From 14a653c244ea36233de288ebe67a9f42adaacfc5 Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev
Date: Wed, 31 Jul 2024 22:02:53 +0400
Subject: [PATCH 1/2] [lldb] Added Pipe::WriteWithTimeout()
Fixed few bugs in PipeWindows. Added the test for async read/write.
---
lldb/include/lldb/Host/PipeBase.h| 5 +-
lldb/include/lldb/Host/posix/PipePosix.h | 4 +-
lldb/include/lldb/Host/windows/PipeWindows.h | 5 +-
lldb/source/Host/common/PipeBase.cpp | 5 +
lldb/source/Host/posix/PipePosix.cpp | 6 +-
lldb/source/Host/windows/PipeWindows.cpp | 128 ---
lldb/unittests/Host/PipeTest.cpp | 66 +-
7 files changed, 165 insertions(+), 54 deletions(-)
diff --git a/lldb/include/lldb/Host/PipeBase.h
b/lldb/include/lldb/Host/PipeBase.h
index 48c19b899cef6..d51d0cd54e036 100644
--- a/lldb/include/lldb/Host/PipeBase.h
+++ b/lldb/include/lldb/Host/PipeBase.h
@@ -56,7 +56,10 @@ class PipeBase {
// Delete named pipe.
virtual Status Delete(llvm::StringRef name) = 0;
- virtual Status Write(const void *buf, size_t size, size_t &bytes_written) =
0;
+ virtual Status WriteWithTimeout(const void *buf, size_t size,
+ const std::chrono::microseconds &timeout,
+ size_t &bytes_written) = 0;
+ Status Write(const void *buf, size_t size, size_t &bytes_written);
virtual Status ReadWithTimeout(void *buf, size_t size,
const std::chrono::microseconds &timeout,
size_t &bytes_read) = 0;
diff --git a/lldb/include/lldb/Host/posix/PipePosix.h
b/lldb/include/lldb/Host/posix/PipePosix.h
index ec4c752a24e94..2e291160817c4 100644
--- a/lldb/include/lldb/Host/posix/PipePosix.h
+++ b/lldb/include/lldb/Host/posix/PipePosix.h
@@ -64,7 +64,9 @@ class PipePosix : public PipeBase {
Status Delete(llvm::StringRef name) override;
- Status Write(const void *buf, size_t size, size_t &bytes_written) override;
+ Status WriteWithTimeout(const void *buf, size_t size,
+ const std::chrono::microseconds &timeout,
+ size_t &bytes_written) override;
Status ReadWithTimeout(void *buf, size_t size,
const std::chrono::microseconds &timeout,
size_t &bytes_read) override;
diff --git a/lldb/include/lldb/Host/windows/PipeWindows.h
b/lldb/include/lldb/Host/windows/PipeWindows.h
index 4b5be28d7ae6c..e28d104cc60ec 100644
--- a/lldb/include/lldb/Host/windows/PipeWindows.h
+++ b/lldb/include/lldb/Host/windows/PipeWindows.h
@@ -32,7 +32,6 @@ class PipeWindows : public PipeBase {
Status CreateNew(bool child_process_inherit) override;
// Create a named pipe.
- Status CreateNewNamed(bool child_process_inherit);
Status CreateNew(llvm::StringRef name, bool child_process_inherit) override;
Status CreateWithUniqueName(llvm::StringRef prefix,
bool child_process_inherit,
@@ -60,7 +59,9 @@ class PipeWindows : public PipeBase {
Status Delete(llvm::StringRef name) override;
- Status Write(const void *buf, size_t size, size_t &bytes_written) override;
+ Status WriteWithTimeout(const void *buf, size_t size,
+ const std::chrono::microseconds &timeout,
+ size_t &bytes_written) override;
Status ReadWithTimeout(void *buf, size_t size,
const std::chrono::microseconds &timeout,
size_t &bytes_read) override;
diff --git a/lldb/source/Host/common/PipeBase.cpp
b/lldb/source/Host/common/PipeBase.cpp
index b3e0ab34a58df..904a2df12392d 100644
--- a/lldb/source/Host/common/PipeBase.cpp
+++ b/lldb/source/Host/common/PipeBase.cpp
@@ -18,6 +18,11 @@ Status PipeBase::OpenAsWriter(llvm::StringRef name,
std::chrono::microseconds::zero());
}
+Status PipeBase::Write(const void *buf, size_t size, size_t &bytes_written) {
+ return WriteWithTimeout(buf, size, std::chrono::microseconds::zero(),
+ bytes_written);
+}
+
Status PipeBase::Read(void *buf, size_t size, size_t &bytes_read) {
return ReadWithTimeout(buf, size, std::chrono::microseconds::zero(),
bytes_read);
diff --git a/lldb/source/Host/posix/PipePosix.cpp
b/lldb/source/Host/posix/PipePosix.cpp
index f35c348990df6..00c6242f3f2e8 100644
--- a/lldb/source/Host/posix/PipePosix.cpp
+++ b/lldb/source/Host/posix/PipePosix.cpp
@@ -335,7 +335,9 @@ Status PipePosix::ReadWithTimeout(void *buf, size_t size,
return error;
}
-Status PipePosix::Write(const void *buf, size_t size, size_t &bytes_written) {
+Status PipePosix::WriteWithTimeout(const void *buf, size_t size,
+ const std::chrono::microseconds &timeout,
+ size_t &bytes_writte
[Lldb-commits] [lldb] [lldb] Fixed PipeWindows bugs; added Pipe::WriteWithTimeout() (PR #101383)
slydiman wrote: @labath Thanks for the review. I have updated everything. https://github.com/llvm/llvm-project/pull/101383 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed PipeWindows bugs; added Pipe::WriteWithTimeout() (PR #101383)
https://github.com/labath edited https://github.com/llvm/llvm-project/pull/101383 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed PipeWindows bugs; added Pipe::WriteWithTimeout() (PR #101383)
@@ -47,5 +50,92 @@ TEST_F(PipeTest, OpenAsReader) {
ASSERT_THAT_ERROR(
pipe.OpenAsReader(name_ref, /*child_process_inherit=*/false).ToError(),
llvm::Succeeded());
+
+ ASSERT_TRUE(pipe.CanRead());
}
#endif
+
+TEST_F(PipeTest, WriteWithTimeout) {
+ Pipe pipe;
+ ASSERT_THAT_ERROR(pipe.CreateNew(false).ToError(), llvm::Succeeded());
+ // Note write_chunk_size must be less than the pipe buffer.
+ // The pipe buffer is 1024 for PipeWindows and at least 512 on Darwin.
+ const size_t buf_size = 8192;
+ const size_t write_chunk_size = 234;
+
+ std::vector write_buf(buf_size / sizeof(int32_t));
+ std::iota(write_buf.begin(), write_buf.end(), 0);
+ std::vector read_buf(write_buf.size() + 100, -1);
+
+ char *write_ptr = (char *)&write_buf.front();
+ char *read_ptr = (char *)&read_buf.front();
labath wrote:
```suggestion
char *write_ptr = reinterpret_cast(write_buf.data());
char *read_ptr = reinterpret_castread_buf.data());
```
https://github.com/llvm/llvm-project/pull/101383
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed PipeWindows bugs; added Pipe::WriteWithTimeout() (PR #101383)
@@ -47,5 +50,92 @@ TEST_F(PipeTest, OpenAsReader) {
ASSERT_THAT_ERROR(
pipe.OpenAsReader(name_ref, /*child_process_inherit=*/false).ToError(),
llvm::Succeeded());
+
+ ASSERT_TRUE(pipe.CanRead());
}
#endif
+
+TEST_F(PipeTest, WriteWithTimeout) {
+ Pipe pipe;
+ ASSERT_THAT_ERROR(pipe.CreateNew(false).ToError(), llvm::Succeeded());
+ // Note write_chunk_size must be less than the pipe buffer.
+ // The pipe buffer is 1024 for PipeWindows and at least 512 on Darwin.
+ const size_t buf_size = 8192;
+ const size_t write_chunk_size = 234;
+
+ std::vector write_buf(buf_size / sizeof(int32_t));
+ std::iota(write_buf.begin(), write_buf.end(), 0);
+ std::vector read_buf(write_buf.size() + 100, -1);
+
+ char *write_ptr = (char *)&write_buf.front();
+ char *read_ptr = (char *)&read_buf.front();
+ size_t write_bytes = 0;
+ size_t read_bytes = 0;
+ size_t num_bytes = 0;
+
+ // Write to the pipe until it is full.
+ while (write_bytes < buf_size) {
+Status error =
+pipe.WriteWithTimeout(write_ptr + write_bytes, write_chunk_size,
+ std::chrono::milliseconds(10), num_bytes);
+if (error.Fail())
+ break; // The write buffer is full
+write_bytes += num_bytes;
+ }
+ ASSERT_TRUE(write_bytes + write_chunk_size <= buf_size);
labath wrote:
```suggestion
ASSERT_LE(write_bytes + write_chunk_size, buf_size) << "Pipe buffer larger
than expected";
```
https://github.com/llvm/llvm-project/pull/101383
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed PipeWindows bugs; added Pipe::WriteWithTimeout() (PR #101383)
https://github.com/labath approved this pull request. Thanks. Looks good, just a couple of random improvements. https://github.com/llvm/llvm-project/pull/101383 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed PipeWindows bugs; added Pipe::WriteWithTimeout() (PR #101383)
@@ -47,5 +50,92 @@ TEST_F(PipeTest, OpenAsReader) {
ASSERT_THAT_ERROR(
pipe.OpenAsReader(name_ref, /*child_process_inherit=*/false).ToError(),
llvm::Succeeded());
+
+ ASSERT_TRUE(pipe.CanRead());
}
#endif
+
+TEST_F(PipeTest, WriteWithTimeout) {
+ Pipe pipe;
+ ASSERT_THAT_ERROR(pipe.CreateNew(false).ToError(), llvm::Succeeded());
+ // Note write_chunk_size must be less than the pipe buffer.
+ // The pipe buffer is 1024 for PipeWindows and at least 512 on Darwin.
+ const size_t buf_size = 8192;
+ const size_t write_chunk_size = 234;
+
+ std::vector write_buf(buf_size / sizeof(int32_t));
+ std::iota(write_buf.begin(), write_buf.end(), 0);
+ std::vector read_buf(write_buf.size() + 100, -1);
+
+ char *write_ptr = (char *)&write_buf.front();
+ char *read_ptr = (char *)&read_buf.front();
+ size_t write_bytes = 0;
+ size_t read_bytes = 0;
+ size_t num_bytes = 0;
+
+ // Write to the pipe until it is full.
+ while (write_bytes < buf_size) {
+Status error =
+pipe.WriteWithTimeout(write_ptr + write_bytes, write_chunk_size,
+ std::chrono::milliseconds(10), num_bytes);
+if (error.Fail())
+ break; // The write buffer is full
+write_bytes += num_bytes;
+ }
+ ASSERT_TRUE(write_bytes + write_chunk_size <= buf_size);
+
+ // Attempt a write with a long timeout.
+ auto start_time = std::chrono::steady_clock::now();
+ ASSERT_THAT_ERROR(
+ pipe.WriteWithTimeout(write_ptr + write_bytes, write_chunk_size,
+std::chrono::milliseconds(2000), num_bytes)
labath wrote:
```suggestion
std::chrono::seconds(2), num_bytes)
```
https://github.com/llvm/llvm-project/pull/101383
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed PipeWindows bugs; added Pipe::WriteWithTimeout() (PR #101383)
@@ -47,5 +50,92 @@ TEST_F(PipeTest, OpenAsReader) {
ASSERT_THAT_ERROR(
pipe.OpenAsReader(name_ref, /*child_process_inherit=*/false).ToError(),
llvm::Succeeded());
+
+ ASSERT_TRUE(pipe.CanRead());
}
#endif
+
+TEST_F(PipeTest, WriteWithTimeout) {
+ Pipe pipe;
+ ASSERT_THAT_ERROR(pipe.CreateNew(false).ToError(), llvm::Succeeded());
+ // Note write_chunk_size must be less than the pipe buffer.
+ // The pipe buffer is 1024 for PipeWindows and at least 512 on Darwin.
+ const size_t buf_size = 8192;
+ const size_t write_chunk_size = 234;
+
+ std::vector write_buf(buf_size / sizeof(int32_t));
+ std::iota(write_buf.begin(), write_buf.end(), 0);
+ std::vector read_buf(write_buf.size() + 100, -1);
+
+ char *write_ptr = (char *)&write_buf.front();
+ char *read_ptr = (char *)&read_buf.front();
+ size_t write_bytes = 0;
+ size_t read_bytes = 0;
+ size_t num_bytes = 0;
+
+ // Write to the pipe until it is full.
+ while (write_bytes < buf_size) {
+Status error =
+pipe.WriteWithTimeout(write_ptr + write_bytes, write_chunk_size,
+ std::chrono::milliseconds(10), num_bytes);
+if (error.Fail())
+ break; // The write buffer is full
+write_bytes += num_bytes;
+ }
+ ASSERT_TRUE(write_bytes + write_chunk_size <= buf_size);
+
+ // Attempt a write with a long timeout.
+ auto start_time = std::chrono::steady_clock::now();
+ ASSERT_THAT_ERROR(
+ pipe.WriteWithTimeout(write_ptr + write_bytes, write_chunk_size,
+std::chrono::milliseconds(2000), num_bytes)
+ .ToError(),
+ llvm::Failed());
+ auto dur = std::chrono::duration_cast(
+ std::chrono::steady_clock::now() - start_time)
+ .count();
+ ASSERT_GE(dur, 2000);
+
+ // Attempt a write with a short timeout
+ start_time = std::chrono::steady_clock::now();
+ ASSERT_THAT_ERROR(
+ pipe.WriteWithTimeout(write_ptr + write_bytes, write_chunk_size,
+std::chrono::milliseconds(200), num_bytes)
+ .ToError(),
+ llvm::Failed());
+ dur = std::chrono::duration_cast(
+std::chrono::steady_clock::now() - start_time)
+.count();
+ ASSERT_GE(dur, 200);
+ ASSERT_LT(dur, 300);
+
+ // Drain the pipe
+ while (read_bytes < write_bytes) {
+ASSERT_THAT_ERROR(
+pipe.ReadWithTimeout(read_ptr + read_bytes, write_bytes - read_bytes,
+ std::chrono::milliseconds(10), num_bytes)
+.ToError(),
+llvm::Succeeded());
+read_bytes += num_bytes;
+ }
+
+ // Be sure the pipe is empty.
+ ASSERT_THAT_ERROR(pipe.ReadWithTimeout(read_ptr + read_bytes, 100,
+ std::chrono::milliseconds(10),
+ num_bytes)
+.ToError(),
+llvm::Failed());
+
+ // Check that we got what we wrote.
+ ASSERT_EQ(write_bytes, read_bytes);
+ ASSERT_TRUE(std::equal(write_buf.begin(),
+ write_buf.begin() + write_bytes / sizeof(uint32_t),
+ read_buf.begin()));
labath wrote:
I can live with that :)
https://github.com/llvm/llvm-project/pull/101383
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed PipeWindows bugs; added Pipe::WriteWithTimeout() (PR #101383)
@@ -47,5 +50,92 @@ TEST_F(PipeTest, OpenAsReader) {
ASSERT_THAT_ERROR(
pipe.OpenAsReader(name_ref, /*child_process_inherit=*/false).ToError(),
llvm::Succeeded());
+
+ ASSERT_TRUE(pipe.CanRead());
}
#endif
+
+TEST_F(PipeTest, WriteWithTimeout) {
+ Pipe pipe;
+ ASSERT_THAT_ERROR(pipe.CreateNew(false).ToError(), llvm::Succeeded());
+ // Note write_chunk_size must be less than the pipe buffer.
+ // The pipe buffer is 1024 for PipeWindows and at least 512 on Darwin.
+ const size_t buf_size = 8192;
+ const size_t write_chunk_size = 234;
+
+ std::vector write_buf(buf_size / sizeof(int32_t));
+ std::iota(write_buf.begin(), write_buf.end(), 0);
+ std::vector read_buf(write_buf.size() + 100, -1);
+
+ char *write_ptr = (char *)&write_buf.front();
+ char *read_ptr = (char *)&read_buf.front();
+ size_t write_bytes = 0;
+ size_t read_bytes = 0;
+ size_t num_bytes = 0;
+
+ // Write to the pipe until it is full.
+ while (write_bytes < buf_size) {
+Status error =
+pipe.WriteWithTimeout(write_ptr + write_bytes, write_chunk_size,
+ std::chrono::milliseconds(10), num_bytes);
+if (error.Fail())
+ break; // The write buffer is full
+write_bytes += num_bytes;
+ }
+ ASSERT_TRUE(write_bytes + write_chunk_size <= buf_size);
+
+ // Attempt a write with a long timeout.
+ auto start_time = std::chrono::steady_clock::now();
+ ASSERT_THAT_ERROR(
+ pipe.WriteWithTimeout(write_ptr + write_bytes, write_chunk_size,
+std::chrono::milliseconds(2000), num_bytes)
+ .ToError(),
+ llvm::Failed());
+ auto dur = std::chrono::duration_cast(
+ std::chrono::steady_clock::now() - start_time)
+ .count();
+ ASSERT_GE(dur, 2000);
+
+ // Attempt a write with a short timeout
+ start_time = std::chrono::steady_clock::now();
+ ASSERT_THAT_ERROR(
+ pipe.WriteWithTimeout(write_ptr + write_bytes, write_chunk_size,
+std::chrono::milliseconds(200), num_bytes)
+ .ToError(),
+ llvm::Failed());
+ dur = std::chrono::duration_cast(
+std::chrono::steady_clock::now() - start_time)
+.count();
+ ASSERT_GE(dur, 200);
+ ASSERT_LT(dur, 300);
labath wrote:
This is going to be flaky when the test is run on a loaded machine. To be safe,
this needs to be at least an order of magnitude larger than the expected
timeout (so e.g. to check that this took less than 2 seconds). But that's fine,
the main thing I wanted to check by this is that it does not wait ~forever
(like with the bug you found where we multiplied by 1000 instead of dividing).
https://github.com/llvm/llvm-project/pull/101383
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed PipeWindows bugs; added Pipe::WriteWithTimeout() (PR #101383)
@@ -47,5 +50,92 @@ TEST_F(PipeTest, OpenAsReader) {
ASSERT_THAT_ERROR(
pipe.OpenAsReader(name_ref, /*child_process_inherit=*/false).ToError(),
llvm::Succeeded());
+
+ ASSERT_TRUE(pipe.CanRead());
}
#endif
+
+TEST_F(PipeTest, WriteWithTimeout) {
+ Pipe pipe;
+ ASSERT_THAT_ERROR(pipe.CreateNew(false).ToError(), llvm::Succeeded());
+ // Note write_chunk_size must be less than the pipe buffer.
+ // The pipe buffer is 1024 for PipeWindows and at least 512 on Darwin.
+ const size_t buf_size = 8192;
+ const size_t write_chunk_size = 234;
+
+ std::vector write_buf(buf_size / sizeof(int32_t));
+ std::iota(write_buf.begin(), write_buf.end(), 0);
+ std::vector read_buf(write_buf.size() + 100, -1);
+
+ char *write_ptr = (char *)&write_buf.front();
+ char *read_ptr = (char *)&read_buf.front();
+ size_t write_bytes = 0;
+ size_t read_bytes = 0;
+ size_t num_bytes = 0;
+
+ // Write to the pipe until it is full.
+ while (write_bytes < buf_size) {
+Status error =
+pipe.WriteWithTimeout(write_ptr + write_bytes, write_chunk_size,
+ std::chrono::milliseconds(10), num_bytes);
+if (error.Fail())
+ break; // The write buffer is full
+write_bytes += num_bytes;
+ }
+ ASSERT_TRUE(write_bytes + write_chunk_size <= buf_size);
+
+ // Attempt a write with a long timeout.
+ auto start_time = std::chrono::steady_clock::now();
+ ASSERT_THAT_ERROR(
+ pipe.WriteWithTimeout(write_ptr + write_bytes, write_chunk_size,
+std::chrono::milliseconds(2000), num_bytes)
+ .ToError(),
+ llvm::Failed());
+ auto dur = std::chrono::duration_cast(
+ std::chrono::steady_clock::now() - start_time)
+ .count();
+ ASSERT_GE(dur, 2000);
labath wrote:
```suggestion
auto dur = std::chrono::steady_clock::now() - start_time;
ASSERT_GE(dur, std::chrono::seconds(2));
```
If it compiles (which I think it should).
https://github.com/llvm/llvm-project/pull/101383
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Impove ObjectFileELF's .dynamic parsing and usage. (PR #101237)
@@ -402,6 +413,29 @@ class ObjectFileELF : public lldb_private::ObjectFile {
/// .gnu_debugdata section or \c nullptr if an error occured or if there's no
/// section with that name.
std::shared_ptr GetGnuDebugDataObjectFile();
+
+ /// Get the bytes that represent the .dynamic section.
+ ///
+ /// This function will fetch the data for the .dynamic section in an ELF
file.
+ /// If the ELF file is loaded from a file on disk, it will use the PT_DYNAMIC
+ /// program header to extract the data and fall back to using the section
+ /// headers. If the ELF file is loaded from memory, it will use the
PT_DYNAMIC
+ /// program header to get the information.
+ ///
+ /// \return The bytes that represent the string table data or \c std::nullopt
+ /// if an error occured.
+ std::optional GetDynamicData();
labath wrote:
I don't feel strongly, but my thinking is like:
- we should handle both the case where the dynamic section is missing (for
whatever reason) and when it is present, but empty (for whatever reason)
- we probably want to handle both cases the same way (probably by doing nothing
and bailing out)
- if we coalesce the two cases into one, the caller is more likely to get both
of them right
https://github.com/llvm/llvm-project/pull/101237
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Delete StepScope enum whose values are ignored. (PR #101981)
https://github.com/sedymrak created
https://github.com/llvm/llvm-project/pull/101981
StepScope enum is a type whose values are passed around, but they are
ultimately ignored.
>From 8abb0771e646dfc46d832f03485b288a4be08168 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Matej=20Ko=C5=A1=C3=ADk?=
Date: Mon, 5 Aug 2024 13:47:02 +0200
Subject: [PATCH] Delete StepScope enum whose values are ignored.
StepScope enum is a type whose values are passed around,
but they are ultimately ignored.
---
lldb/source/Commands/CommandObjectThread.cpp | 20
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/lldb/source/Commands/CommandObjectThread.cpp
b/lldb/source/Commands/CommandObjectThread.cpp
index 366b6dd965b38..fcf46843ba674 100644
--- a/lldb/source/Commands/CommandObjectThread.cpp
+++ b/lldb/source/Commands/CommandObjectThread.cpp
@@ -247,8 +247,6 @@ class CommandObjectThreadBacktrace : public
CommandObjectIterateOverThreads {
CommandOptions m_options;
};
-enum StepScope { eStepScopeSource, eStepScopeInstruction };
-
#define LLDB_OPTIONS_thread_step_scope
#include "CommandOptions.inc"
@@ -374,14 +372,13 @@ class CommandObjectThreadStepWithTypeAndScope : public
CommandObjectParsed {
CommandObjectThreadStepWithTypeAndScope(CommandInterpreter &interpreter,
const char *name, const char *help,
const char *syntax,
- StepType step_type,
- StepScope step_scope)
+ StepType step_type)
: CommandObjectParsed(interpreter, name, help, syntax,
eCommandRequiresProcess | eCommandRequiresThread |
eCommandTryTargetAPILock |
eCommandProcessMustBeLaunched |
eCommandProcessMustBePaused),
-m_step_type(step_type), m_step_scope(step_scope),
+m_step_type(step_type),
m_class_options("scripted step") {
AddSimpleArgumentList(eArgTypeThreadIndex, eArgRepeatOptional);
@@ -621,7 +618,6 @@ class CommandObjectThreadStepWithTypeAndScope : public
CommandObjectParsed {
}
StepType m_step_type;
- StepScope m_step_scope;
ThreadStepScopeOptionGroup m_options;
OptionGroupPythonClassWithDict m_class_options;
OptionGroupOptions m_all_options;
@@ -2561,35 +2557,35 @@
CommandObjectMultiwordThread::CommandObjectMultiwordThread(
interpreter, "thread step-in",
"Source level single step, stepping into calls. Defaults
"
"to current thread unless specified.",
- nullptr, eStepTypeInto, eStepScopeSource)));
+ nullptr, eStepTypeInto)));
LoadSubCommand("step-out",
CommandObjectSP(new CommandObjectThreadStepWithTypeAndScope(
interpreter, "thread step-out",
"Finish executing the current stack frame and stop after "
"returning. Defaults to current thread unless
specified.",
- nullptr, eStepTypeOut, eStepScopeSource)));
+ nullptr, eStepTypeOut)));
LoadSubCommand("step-over",
CommandObjectSP(new CommandObjectThreadStepWithTypeAndScope(
interpreter, "thread step-over",
"Source level single step, stepping over calls. Defaults
"
"to current thread unless specified.",
- nullptr, eStepTypeOver, eStepScopeSource)));
+ nullptr, eStepTypeOver)));
LoadSubCommand("step-inst",
CommandObjectSP(new CommandObjectThreadStepWithTypeAndScope(
interpreter, "thread step-inst",
"Instruction level single step, stepping into calls. "
"Defaults to current thread unless specified.",
- nullptr, eStepTypeTrace, eStepScopeInstruction)));
+ nullptr, eStepTypeTrace)));
LoadSubCommand("step-inst-over",
CommandObjectSP(new CommandObjectThreadStepWithTypeAndScope(
interpreter, "thread step-inst-over",
"Instruction level single step, stepping over calls. "
"Defaults to current thread unless specified.",
- nullptr, eStepTypeTraceOver, eStepScopeInstruction)));
+ nullptr, eStepTypeTraceOver)));
LoadSubCommand(
"step-scripted",
@@ -2600,7 +2596,7 @@
CommandObjectMultiwordThread::CommandObjectMultiwordThread(
"that will be used to populate an SBStructuredData Dictionary, which
"
"will be passed to the constructor of the class implementing the "
"scripted step. See the Python Reference for more details.",
-
[Lldb-commits] [lldb] [lldb] Delete StepScope enum whose values are ignored. (PR #101981)
github-actions[bot] wrote: Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using `@` followed by their GitHub username. If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the [LLVM GitHub User Guide](https://llvm.org/docs/GitHub.html). You can also ask questions in a comment on this PR, on the [LLVM Discord](https://discord.com/invite/xS7Z362) or on the [forums](https://discourse.llvm.org/). https://github.com/llvm/llvm-project/pull/101981 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Delete StepScope enum whose values are ignored. (PR #101981)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: None (sedymrak)
Changes
StepScope enum is a type whose values are passed around, but they are
ultimately ignored.
---
Full diff: https://github.com/llvm/llvm-project/pull/101981.diff
1 Files Affected:
- (modified) lldb/source/Commands/CommandObjectThread.cpp (+8-12)
``diff
diff --git a/lldb/source/Commands/CommandObjectThread.cpp
b/lldb/source/Commands/CommandObjectThread.cpp
index 366b6dd965b38..fcf46843ba674 100644
--- a/lldb/source/Commands/CommandObjectThread.cpp
+++ b/lldb/source/Commands/CommandObjectThread.cpp
@@ -247,8 +247,6 @@ class CommandObjectThreadBacktrace : public
CommandObjectIterateOverThreads {
CommandOptions m_options;
};
-enum StepScope { eStepScopeSource, eStepScopeInstruction };
-
#define LLDB_OPTIONS_thread_step_scope
#include "CommandOptions.inc"
@@ -374,14 +372,13 @@ class CommandObjectThreadStepWithTypeAndScope : public
CommandObjectParsed {
CommandObjectThreadStepWithTypeAndScope(CommandInterpreter &interpreter,
const char *name, const char *help,
const char *syntax,
- StepType step_type,
- StepScope step_scope)
+ StepType step_type)
: CommandObjectParsed(interpreter, name, help, syntax,
eCommandRequiresProcess | eCommandRequiresThread |
eCommandTryTargetAPILock |
eCommandProcessMustBeLaunched |
eCommandProcessMustBePaused),
-m_step_type(step_type), m_step_scope(step_scope),
+m_step_type(step_type),
m_class_options("scripted step") {
AddSimpleArgumentList(eArgTypeThreadIndex, eArgRepeatOptional);
@@ -621,7 +618,6 @@ class CommandObjectThreadStepWithTypeAndScope : public
CommandObjectParsed {
}
StepType m_step_type;
- StepScope m_step_scope;
ThreadStepScopeOptionGroup m_options;
OptionGroupPythonClassWithDict m_class_options;
OptionGroupOptions m_all_options;
@@ -2561,35 +2557,35 @@
CommandObjectMultiwordThread::CommandObjectMultiwordThread(
interpreter, "thread step-in",
"Source level single step, stepping into calls. Defaults
"
"to current thread unless specified.",
- nullptr, eStepTypeInto, eStepScopeSource)));
+ nullptr, eStepTypeInto)));
LoadSubCommand("step-out",
CommandObjectSP(new CommandObjectThreadStepWithTypeAndScope(
interpreter, "thread step-out",
"Finish executing the current stack frame and stop after "
"returning. Defaults to current thread unless
specified.",
- nullptr, eStepTypeOut, eStepScopeSource)));
+ nullptr, eStepTypeOut)));
LoadSubCommand("step-over",
CommandObjectSP(new CommandObjectThreadStepWithTypeAndScope(
interpreter, "thread step-over",
"Source level single step, stepping over calls. Defaults
"
"to current thread unless specified.",
- nullptr, eStepTypeOver, eStepScopeSource)));
+ nullptr, eStepTypeOver)));
LoadSubCommand("step-inst",
CommandObjectSP(new CommandObjectThreadStepWithTypeAndScope(
interpreter, "thread step-inst",
"Instruction level single step, stepping into calls. "
"Defaults to current thread unless specified.",
- nullptr, eStepTypeTrace, eStepScopeInstruction)));
+ nullptr, eStepTypeTrace)));
LoadSubCommand("step-inst-over",
CommandObjectSP(new CommandObjectThreadStepWithTypeAndScope(
interpreter, "thread step-inst-over",
"Instruction level single step, stepping over calls. "
"Defaults to current thread unless specified.",
- nullptr, eStepTypeTraceOver, eStepScopeInstruction)));
+ nullptr, eStepTypeTraceOver)));
LoadSubCommand(
"step-scripted",
@@ -2600,7 +2596,7 @@
CommandObjectMultiwordThread::CommandObjectMultiwordThread(
"that will be used to populate an SBStructuredData Dictionary, which
"
"will be passed to the constructor of the class implementing the "
"scripted step. See the Python Reference for more details.",
- nullptr, eStepTypeScripted, eStepScopeSource)));
+ nullptr, eStepTypeScripted)));
LoadSubCommand("plan", CommandObjectSP(new CommandObjectMultiwordThreadPlan(
interpreter)));
``
https://github.com/llvm/llvm-
[Lldb-commits] [lldb] [LLDB] Impove ObjectFileELF's .dynamic parsing and usage. (PR #101237)
@@ -2550,6 +2550,21 @@ ModuleSP Process::ReadModuleFromMemory(const FileSpec
&file_spec,
}
ModuleSP module_sp(new Module(file_spec, ArchSpec()));
if (module_sp) {
+if (size_to_read == 0) {
+ // Default to 8192 in case we can't find a memory region.
+ size_to_read = 0x2000;
+ MemoryRegionInfo range_info;
+ Status error(GetMemoryRegionInfo(header_addr, range_info));
+ if (error.Success()) {
+// We found a memory region, set the range of bytes ro read to read to
+// the end of the memory region. This should be enough to contain the
+// file header and important bits.
+const auto &range = range_info.GetRange();
labath wrote:
They are, but that's not my point. My point is that this first segment can be
really large (like one gigabyte or even more), and if that's the case, we
probably don't want to read all of it.
Doing the extra read in ObjectFileELF sounds good to me, as there we can look
at the elf header, and load exactly only the program header bytes we care about.
https://github.com/llvm/llvm-project/pull/101237
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
slydiman wrote: @labath >That's nice, but I think we should figure out how to reduce the number of >ifdefs in this patch. Porting linux away from fork may not be your concern, >but figuring out how to make the code less branchy is. If you can do that >without removing forks, I can take it upon myself to handle the rest. I just >think it would be easier to do it both together... `#ifdef _WIN32` is used here for the following - declare and initialize the parameter --accept (accept_fd) - call spawn_process_child() and exit if accept_fd is defined - call spawn_process_parent() instead of fork() We can unify spawn_process_child() moving it to ConnectionFileDescriptor::ConnectFD() in ConnectionFileDescriptorPosix.cpp with `#ifdef _WIN32`. Note I will remove `#ifndef _WIN32` around ::waitpid() in lldb-platform.cpp in the part 2 of this patch. We still need spawn_process_reaped() here but I will remove it in part 2 too. Any static dummy will be enough. For now we need spawn_process_reaped() here for `gdbserver_portmap.FreePortForProcess(pid);`. We can try to move spawn_process_parent() to `lldb/source/Host/windows/` and probably a part to `lldb/source/Host/posix/`. Where is the best place? HostProcess*.cpp, ProcessLauncher*.cpp? It is necessary to implement some callback for spawn_process_reaped(), which will be removed in part 2. It looks like a redundant work now. Probably it is necessary to define a new type and an invalid value for accept_fd. It is int (socket fd) on Posix and pipe_t (HANDLE) on Windows. What is the best type name and what is the best place to define it? I would be happy to keep it as is till part 2 of this patch to minimize a redundant work. https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed PipeWindows bugs; added Pipe::WriteWithTimeout() (PR #101383)
https://github.com/slydiman updated
https://github.com/llvm/llvm-project/pull/101383
>From 14a653c244ea36233de288ebe67a9f42adaacfc5 Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev
Date: Wed, 31 Jul 2024 22:02:53 +0400
Subject: [PATCH 1/3] [lldb] Added Pipe::WriteWithTimeout()
Fixed few bugs in PipeWindows. Added the test for async read/write.
---
lldb/include/lldb/Host/PipeBase.h| 5 +-
lldb/include/lldb/Host/posix/PipePosix.h | 4 +-
lldb/include/lldb/Host/windows/PipeWindows.h | 5 +-
lldb/source/Host/common/PipeBase.cpp | 5 +
lldb/source/Host/posix/PipePosix.cpp | 6 +-
lldb/source/Host/windows/PipeWindows.cpp | 128 ---
lldb/unittests/Host/PipeTest.cpp | 66 +-
7 files changed, 165 insertions(+), 54 deletions(-)
diff --git a/lldb/include/lldb/Host/PipeBase.h
b/lldb/include/lldb/Host/PipeBase.h
index 48c19b899cef6..d51d0cd54e036 100644
--- a/lldb/include/lldb/Host/PipeBase.h
+++ b/lldb/include/lldb/Host/PipeBase.h
@@ -56,7 +56,10 @@ class PipeBase {
// Delete named pipe.
virtual Status Delete(llvm::StringRef name) = 0;
- virtual Status Write(const void *buf, size_t size, size_t &bytes_written) =
0;
+ virtual Status WriteWithTimeout(const void *buf, size_t size,
+ const std::chrono::microseconds &timeout,
+ size_t &bytes_written) = 0;
+ Status Write(const void *buf, size_t size, size_t &bytes_written);
virtual Status ReadWithTimeout(void *buf, size_t size,
const std::chrono::microseconds &timeout,
size_t &bytes_read) = 0;
diff --git a/lldb/include/lldb/Host/posix/PipePosix.h
b/lldb/include/lldb/Host/posix/PipePosix.h
index ec4c752a24e94..2e291160817c4 100644
--- a/lldb/include/lldb/Host/posix/PipePosix.h
+++ b/lldb/include/lldb/Host/posix/PipePosix.h
@@ -64,7 +64,9 @@ class PipePosix : public PipeBase {
Status Delete(llvm::StringRef name) override;
- Status Write(const void *buf, size_t size, size_t &bytes_written) override;
+ Status WriteWithTimeout(const void *buf, size_t size,
+ const std::chrono::microseconds &timeout,
+ size_t &bytes_written) override;
Status ReadWithTimeout(void *buf, size_t size,
const std::chrono::microseconds &timeout,
size_t &bytes_read) override;
diff --git a/lldb/include/lldb/Host/windows/PipeWindows.h
b/lldb/include/lldb/Host/windows/PipeWindows.h
index 4b5be28d7ae6c..e28d104cc60ec 100644
--- a/lldb/include/lldb/Host/windows/PipeWindows.h
+++ b/lldb/include/lldb/Host/windows/PipeWindows.h
@@ -32,7 +32,6 @@ class PipeWindows : public PipeBase {
Status CreateNew(bool child_process_inherit) override;
// Create a named pipe.
- Status CreateNewNamed(bool child_process_inherit);
Status CreateNew(llvm::StringRef name, bool child_process_inherit) override;
Status CreateWithUniqueName(llvm::StringRef prefix,
bool child_process_inherit,
@@ -60,7 +59,9 @@ class PipeWindows : public PipeBase {
Status Delete(llvm::StringRef name) override;
- Status Write(const void *buf, size_t size, size_t &bytes_written) override;
+ Status WriteWithTimeout(const void *buf, size_t size,
+ const std::chrono::microseconds &timeout,
+ size_t &bytes_written) override;
Status ReadWithTimeout(void *buf, size_t size,
const std::chrono::microseconds &timeout,
size_t &bytes_read) override;
diff --git a/lldb/source/Host/common/PipeBase.cpp
b/lldb/source/Host/common/PipeBase.cpp
index b3e0ab34a58df..904a2df12392d 100644
--- a/lldb/source/Host/common/PipeBase.cpp
+++ b/lldb/source/Host/common/PipeBase.cpp
@@ -18,6 +18,11 @@ Status PipeBase::OpenAsWriter(llvm::StringRef name,
std::chrono::microseconds::zero());
}
+Status PipeBase::Write(const void *buf, size_t size, size_t &bytes_written) {
+ return WriteWithTimeout(buf, size, std::chrono::microseconds::zero(),
+ bytes_written);
+}
+
Status PipeBase::Read(void *buf, size_t size, size_t &bytes_read) {
return ReadWithTimeout(buf, size, std::chrono::microseconds::zero(),
bytes_read);
diff --git a/lldb/source/Host/posix/PipePosix.cpp
b/lldb/source/Host/posix/PipePosix.cpp
index f35c348990df6..00c6242f3f2e8 100644
--- a/lldb/source/Host/posix/PipePosix.cpp
+++ b/lldb/source/Host/posix/PipePosix.cpp
@@ -335,7 +335,9 @@ Status PipePosix::ReadWithTimeout(void *buf, size_t size,
return error;
}
-Status PipePosix::Write(const void *buf, size_t size, size_t &bytes_written) {
+Status PipePosix::WriteWithTimeout(const void *buf, size_t size,
+ const std::chrono::microseconds &timeout,
+ size_t &bytes_writte
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
DavidSpickett wrote:
I reduced
`lldb/test/API/functionalities/breakpoint/step_over_breakpoint/TestStepOverBreakpoint.py`
a bit to the point where it stalls, which is when it tries to step over from
breakpoint 2 to 3 in `test_step_over`:
```
int b = func(); // breakpoint_2
a = b + func(); // breakpoint_3
```
These are the placed breakpoints:
```
1: file =
'/home/david.spickett/llvm-project/lldb/test/API/functionalities/breakpoint/step_over_breakpoint/main.cpp',
line = 7, exact_match = 0
1.1:
module =
/home/david.spickett/build-llvm-arm/lldb-test-build.noindex/functionalities/breakpoint/step_over_breakpoint/TestStepOverBreakpoint.test_step_over_dwo/a.out
compile unit = main.cpp
function = main
location =
/home/david.spickett/llvm-project/lldb/test/API/functionalities/breakpoint/step_over_breakpoint/main.cpp:7:9
address = 0x0064c508
resolved = true
hardware = false
hit count = 1
2: source regex = "breakpoint_2", exact_match = 0
2.1:
module =
/home/david.spickett/build-llvm-arm/lldb-test-build.noindex/functionalities/breakpoint/step_over_breakpoint/TestStepOverBreakpoint.test_step_over_dwo/a.out
compile unit = main.cpp
function = main
location =
/home/david.spickett/llvm-project/lldb/test/API/functionalities/breakpoint/step_over_breakpoint/main.cpp:8:13
address = 0x0064c510
resolved = true
hardware = false
hit count = 0
Condition: false
3: source regex = "breakpoint_3", exact_match = 0
3.1:
module =
/home/david.spickett/build-llvm-arm/lldb-test-build.noindex/functionalities/breakpoint/step_over_breakpoint/TestStepOverBreakpoint.test_step_over_dwo/a.out
compile unit = main.cpp
function = main
location =
/home/david.spickett/llvm-project/lldb/test/API/functionalities/breakpoint/step_over_breakpoint/main.cpp:9:9
address = 0x0064c518
resolved = true
hardware = false
hit count = 0
Condition: false
```
And where they end up in the instructions:
```
04ec :
4ec: e92d4800push{fp, lr}
4f0: e1a0b00dmov fp, sp
4f4: e24dd018sub sp, sp, #24
4f8: e3002000movwr2, #0
4fc: e50b2004str r2, [fp, #-4]
500: e50b0008str r0, [fp, #-8]
504: e58d100cstr r1, [sp, #12]
508: e300movwr0, #0<< 1
50c: e58d0008str r0, [sp, #8]
510: ebf3bl 4e4 <_Z4funcv><< 2
514: e58d0004str r0, [sp, #4]
518: e59d0004ldr r0, [sp, #4] << 3
```
If I enable the `lldb state` log I see this:
```
python3.8Process::ResumeSynchronous -- locking run lock
b-remote.async> (plugin = gdb-remote, state = running)
python3.8(plugin = gdb-remote, state = running, restarted = 0)
b-remote.async> (plugin = gdb-remote, state = stopped)
b-remote.async> (plugin = gdb-remote, state = stopped, stop_id = 9
b-remote.async> (plugin = gdb-remote, state = running)
b-remote.async> (plugin = gdb-remote, state = stopped)
b-remote.async> (plugin = gdb-remote, state = stopped, stop_id = 10
b-remote.async> (plugin = gdb-remote, state = running)
b-remote.async> (plugin = gdb-remote, state = stopped)
b-remote.async> (plugin = gdb-remote, state = stopped, stop_id = 11
b-remote.async> (plugin = gdb-remote, state = running)
b-remote.async> (plugin = gdb-remote, state = stopped)
```
Then running/stopped/stopped repeats over and over.
`lldb process` log:
```
b-remote.async> (plugin = gdb-remote, state = stopped)
b-remote.async> (plugin = gdb-remote, state = stopped, stop_id = 96677
intern-state Process::ShouldBroadcastEvent: should_resume: 1 state: stopped
was_restarted: 0 report_stop_vote: -1.
intern-state Process::ShouldBroadcastEvent (0xd86029b8) Restarting process
from state: stopped
intern-state Process::PrivateResume() m_stop_id = 96677, public state:
running private state: stopped
b-remote.async> (plugin = gdb-remote, state = running)
intern-state Process thinks the process has resumed.
intern-state Process::ShouldBroadcastEvent (0xd86029b8) => new state:
stopped, last broadcast state: running - NO
intern-state Process::HandlePrivateEvent (pid = 3526293) suppressing state
stopped (old state running): should_broadcast == false
intern-state timeout = , event_sp)...
intern-state Process::ShouldBroadcastEvent (0xd8601030) => new state:
running, last broadcast state: running - NO
intern-state Process::HandlePrivateEvent (pid = 3526293) suppressing state
running (old state running): should_broadcast == false
intern-state timeout = , event_sp)...
b-remote.async> (plugin = gdb-remote, state = stopped)
b-remote.async> (plugin = gdb-remote, state = stopped, stop_id = 96678
```
Again, repeats.
Tell me if there's anything more interesting to look at.
> My question basically was: what is the stop-reply packet for a single step
> (vCont:s
[Lldb-commits] [lldb] [LLDB] Impove ObjectFileELF's .dynamic parsing and usage. (PR #101237)
@@ -0,0 +1,58 @@
+// REQUIRES: system-linux, native
labath wrote:
If it was a lot of work then sure, I'd agree with you -- but I don't think
that's the case here. I just gave it a shot, and this is the only change I
needed to make on top of your patch to be able to parse DT_NEEDED of a
section-free file (disclaimer: This code is mainly for demonstration, it's
possible there is a better way to do that):
```
diff --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
index d26215a9ce3f..5f0d2233b865 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -3810,28 +3810,26 @@ std::optional
ObjectFileELF::GetDynstrData() {
}
}
- // Every ELF file which represents an executable or shared library has
- // mandatory .dynamic entries. Two of these values are DT_STRTAB and DT_STRSZ
- // and represent the dynamic symbol tables's string table. These are needed
- // by the dynamic loader and we can read them from a process' address space.
- //
- // When loading and ELF file from memory, only the program headers end up
- // being mapped into memory, and we can find these values in the PT_DYNAMIC
- // segment.
- if (!IsInMemory())
-return std::nullopt;
- ProcessSP process_sp(m_process_wp.lock());
- if (!process_sp)
-return std::nullopt;
-
const ELFDynamic *strtab = FindDynamicSymbol(DT_STRTAB);
const ELFDynamic *strsz = FindDynamicSymbol(DT_STRSZ);
if (strtab == nullptr || strsz == nullptr)
return std::nullopt;
- if (DataBufferSP data_sp =
- ReadMemory(process_sp, strtab->d_ptr, strsz->d_val))
-return DataExtractor(data_sp, GetByteOrder(), GetAddressByteSize());
+ if (ProcessSP process_sp = m_process_wp.lock()) {
+if (DataBufferSP data_sp =
+ReadMemory(process_sp, strtab->d_ptr, strsz->d_val))
+ return DataExtractor(data_sp, GetByteOrder(), GetAddressByteSize());
+ } else {
+Address addr;
+if (addr.ResolveAddressUsingFileSections(strtab->d_ptr, GetSectionList()))
{
+ DataExtractor data;
+ addr.GetSection()->GetSectionData(data);
+ return DataExtractor(data,
+ strtab->d_ptr - addr.GetSection()->GetFileOffset(),
+ strsz->d_val);
+}
+ }
+
return std::nullopt;
}
```
And I think this goes a long way towards improving the testability of this
patch. I know section-free files aren't likely to be of much use, but they're
really great for testing, and in terms of raw LOC, such a test may cover 80% of
this patch.
https://github.com/llvm/llvm-project/pull/101237
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Impove ObjectFileELF's .dynamic parsing and usage. (PR #101237)
@@ -873,33 +873,29 @@ Address ObjectFileELF::GetImageInfoAddress(Target
*target) {
if (!section_list)
return Address();
- // Find the SHT_DYNAMIC (.dynamic) section.
- SectionSP dynsym_section_sp(
- section_list->FindSectionByType(eSectionTypeELFDynamicLinkInfo, true));
- if (!dynsym_section_sp)
-return Address();
- assert(dynsym_section_sp->GetObjectFile() == this);
-
- user_id_t dynsym_id = dynsym_section_sp->GetID();
- const ELFSectionHeaderInfo *dynsym_hdr = GetSectionHeaderByIndex(dynsym_id);
- if (!dynsym_hdr)
-return Address();
-
for (size_t i = 0; i < m_dynamic_symbols.size(); ++i) {
-ELFDynamic &symbol = m_dynamic_symbols[i];
+const ELFDynamic &symbol = m_dynamic_symbols[i].symbol;
if (symbol.d_tag == DT_DEBUG) {
// Compute the offset as the number of previous entries plus the size of
// d_tag.
- addr_t offset = i * dynsym_hdr->sh_entsize + GetAddressByteSize();
- return Address(dynsym_section_sp, offset);
+ addr_t offset = (i * 2 + 1) * GetAddressByteSize();
+ addr_t file_addr = m_dynamic_base_addr + offset;
+ Address addr;
+ if (addr.ResolveAddressUsingFileSections(file_addr, GetSectionList()))
+return addr;
}
// MIPS executables uses DT_MIPS_RLD_MAP_REL to support PIE.
DT_MIPS_RLD_MAP
// exists in non-PIE.
else if ((symbol.d_tag == DT_MIPS_RLD_MAP ||
symbol.d_tag == DT_MIPS_RLD_MAP_REL) &&
target) {
- addr_t offset = i * dynsym_hdr->sh_entsize + GetAddressByteSize();
+ SectionSP dynsym_section_sp(section_list->FindSectionByType(
+ eSectionTypeELFDynamicLinkInfo, true));
+ if (!dynsym_section_sp)
+return Address();
+
+ addr_t offset = (i * 2 + 1) * GetAddressByteSize();
addr_t dyn_base = dynsym_section_sp->GetLoadBaseAddress(target);
if (dyn_base == LLDB_INVALID_ADDRESS)
return Address();
labath wrote:
I'm not sure what you mean, the code does go about it in slightly different way
(because that was convenient at the time), but I think it should be equivalent
to:
```
addr_t offset = (i * 2 + 1) * GetAddressByteSize();
addr_t file_addr = m_dynamic_base_addr + offset;
Address addr;
if (!addr.ResolveAddressUsingFileSections(file_addr, GetSectionList()))
return Address();
Status error;
if (symbol.d_tag == DT_MIPS_RLD_MAP) {
// DT_MIPS_RLD_MAP tag stores an absolute address of the debug pointer.
Address pointer_addr;
if (target->ReadPointerFromMemory(addr, error, pointer_addr, true))
return pointer_addr;
}
```
Basically, I think it does the same thing as DT_DEBUG, but with an extra level
of indirection.
Also, this is kind of the reason why I'd really like to have a test for this
patch which does not require running an executable. I don't want the next
person to come in here to be afraid of making any changes because he can't test
his change.
https://github.com/llvm/llvm-project/pull/101237
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Impove ObjectFileELF's .dynamic parsing and usage. (PR #101237)
@@ -873,33 +873,29 @@ Address ObjectFileELF::GetImageInfoAddress(Target
*target) {
if (!section_list)
return Address();
- // Find the SHT_DYNAMIC (.dynamic) section.
- SectionSP dynsym_section_sp(
- section_list->FindSectionByType(eSectionTypeELFDynamicLinkInfo, true));
- if (!dynsym_section_sp)
-return Address();
- assert(dynsym_section_sp->GetObjectFile() == this);
-
- user_id_t dynsym_id = dynsym_section_sp->GetID();
- const ELFSectionHeaderInfo *dynsym_hdr = GetSectionHeaderByIndex(dynsym_id);
- if (!dynsym_hdr)
-return Address();
-
for (size_t i = 0; i < m_dynamic_symbols.size(); ++i) {
-ELFDynamic &symbol = m_dynamic_symbols[i];
+const ELFDynamic &symbol = m_dynamic_symbols[i].symbol;
if (symbol.d_tag == DT_DEBUG) {
// Compute the offset as the number of previous entries plus the size of
// d_tag.
- addr_t offset = i * dynsym_hdr->sh_entsize + GetAddressByteSize();
- return Address(dynsym_section_sp, offset);
+ addr_t offset = (i * 2 + 1) * GetAddressByteSize();
+ addr_t file_addr = m_dynamic_base_addr + offset;
+ Address addr;
+ if (addr.ResolveAddressUsingFileSections(file_addr, GetSectionList()))
+return addr;
}
// MIPS executables uses DT_MIPS_RLD_MAP_REL to support PIE.
DT_MIPS_RLD_MAP
// exists in non-PIE.
else if ((symbol.d_tag == DT_MIPS_RLD_MAP ||
symbol.d_tag == DT_MIPS_RLD_MAP_REL) &&
target) {
- addr_t offset = i * dynsym_hdr->sh_entsize + GetAddressByteSize();
+ SectionSP dynsym_section_sp(section_list->FindSectionByType(
+ eSectionTypeELFDynamicLinkInfo, true));
+ if (!dynsym_section_sp)
+return Address();
+
+ addr_t offset = (i * 2 + 1) * GetAddressByteSize();
addr_t dyn_base = dynsym_section_sp->GetLoadBaseAddress(target);
if (dyn_base == LLDB_INVALID_ADDRESS)
return Address();
labath wrote:
FWIW, I think noone actually uses this code. We definitely don't have any mips
buildbots, so if anything breaks, it's up to them to fix it.
https://github.com/llvm/llvm-project/pull/101237
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Impove ObjectFileELF's .dynamic parsing and usage. (PR #101237)
@@ -384,6 +392,9 @@ class ObjectFileELF : public lldb_private::ObjectFile {
/// ELF dependent module dump routine.
void DumpDependentModules(lldb_private::Stream *s);
+ /// ELF dump the .dynamic section
+ void DumpELFDynamic(lldb_private::Stream *s);
labath wrote:
I wouldn't say its required, but I'd like changing these to a reference (in
another patch). If the patch is trivial/obvious, i don't think it requires a
review.
https://github.com/llvm/llvm-project/pull/101237
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Impove ObjectFileELF's .dynamic parsing and usage. (PR #101237)
@@ -873,33 +873,29 @@ Address ObjectFileELF::GetImageInfoAddress(Target
*target) {
if (!section_list)
return Address();
- // Find the SHT_DYNAMIC (.dynamic) section.
- SectionSP dynsym_section_sp(
- section_list->FindSectionByType(eSectionTypeELFDynamicLinkInfo, true));
- if (!dynsym_section_sp)
-return Address();
- assert(dynsym_section_sp->GetObjectFile() == this);
-
- user_id_t dynsym_id = dynsym_section_sp->GetID();
- const ELFSectionHeaderInfo *dynsym_hdr = GetSectionHeaderByIndex(dynsym_id);
- if (!dynsym_hdr)
-return Address();
-
for (size_t i = 0; i < m_dynamic_symbols.size(); ++i) {
-ELFDynamic &symbol = m_dynamic_symbols[i];
+const ELFDynamic &symbol = m_dynamic_symbols[i].symbol;
if (symbol.d_tag == DT_DEBUG) {
// Compute the offset as the number of previous entries plus the size of
// d_tag.
- addr_t offset = i * dynsym_hdr->sh_entsize + GetAddressByteSize();
- return Address(dynsym_section_sp, offset);
+ addr_t offset = (i * 2 + 1) * GetAddressByteSize();
+ addr_t file_addr = m_dynamic_base_addr + offset;
+ Address addr;
labath wrote:
This is mainly useful in conjuction with the mips change below.
https://github.com/llvm/llvm-project/pull/101237
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 212950f - [lldb] Refactor TypeQuery::ContextMatches, take 2 (#101333)
Author: Pavel Labath
Date: 2024-08-05T17:06:07+02:00
New Revision: 212950fbcec3b71fa15ee49e0539333a30159c32
URL:
https://github.com/llvm/llvm-project/commit/212950fbcec3b71fa15ee49e0539333a30159c32
DIFF:
https://github.com/llvm/llvm-project/commit/212950fbcec3b71fa15ee49e0539333a30159c32.diff
LOG: [lldb] Refactor TypeQuery::ContextMatches, take 2 (#101333)
This is an alternative, much simpler implementation of #99305. In this
version I replace the AnyModule wildcard match with a special TypeQuery
flag which achieves (mostly) the same thing.
It is a preparatory step for teaching ContextMatches about anonymous
namespaces. It started out as a way to remove the assumption that the
pattern and target contexts must be of the same length -- that's will
not be correct with anonymous namespaces, and probably isn't even
correct right now for AnyModule matches.
Added:
Modified:
lldb/include/lldb/Symbol/Type.h
lldb/include/lldb/lldb-private-enumerations.h
lldb/source/Symbol/Type.cpp
lldb/test/Shell/SymbolFile/DWARF/clang-gmodules-type-lookup.c
lldb/test/Shell/SymbolFile/DWARF/x86/compilercontext.ll
lldb/tools/lldb-test/lldb-test.cpp
lldb/unittests/Symbol/TestType.cpp
Removed:
diff --git a/lldb/include/lldb/Symbol/Type.h b/lldb/include/lldb/Symbol/Type.h
index c6f30cde81867..420307e0dbcf0 100644
--- a/lldb/include/lldb/Symbol/Type.h
+++ b/lldb/include/lldb/Symbol/Type.h
@@ -65,11 +65,6 @@ struct CompilerContext {
llvm::raw_ostream &operator<<(llvm::raw_ostream &os,
const CompilerContext &rhs);
-/// Match \p context_chain against \p pattern, which may contain "Any"
-/// kinds. The \p context_chain should *not* contain any "Any" kinds.
-bool contextMatches(llvm::ArrayRef context_chain,
-llvm::ArrayRef pattern);
-
FLAGS_ENUM(TypeQueryOptions){
e_none = 0u,
/// If set, TypeQuery::m_context contains an exact context that must match
@@ -79,10 +74,13 @@ FLAGS_ENUM(TypeQueryOptions){
/// If set, TypeQuery::m_context is a clang module compiler context. If not
/// set TypeQuery::m_context is normal type lookup context.
e_module_search = (1u << 1),
+/// If set, the query will ignore all Module entries in the type context,
+/// even for exact matches.
+e_ignore_modules = (1u << 2),
/// When true, the find types call should stop the query as soon as a
single
/// matching type is found. When false, the type query should find all
/// matching types.
-e_find_one = (1u << 2),
+e_find_one = (1u << 3),
};
LLDB_MARK_AS_BITMASK_ENUM(TypeQueryOptions)
@@ -264,6 +262,10 @@ class TypeQuery {
bool LanguageMatches(lldb::LanguageType language) const;
bool GetExactMatch() const { return (m_options & e_exact_match) != 0; }
+
+ bool GetIgnoreModules() const { return (m_options & e_ignore_modules) != 0; }
+ void SetIgnoreModules() { m_options &= ~e_ignore_modules; }
+
/// The \a m_context can be used in two ways: normal types searching with
/// the context containing a stanadard declaration context for a type, or
/// with the context being more complete for exact matches in clang modules.
diff --git a/lldb/include/lldb/lldb-private-enumerations.h
b/lldb/include/lldb/lldb-private-enumerations.h
index 9d18316dcea25..c24a3538f58da 100644
--- a/lldb/include/lldb/lldb-private-enumerations.h
+++ b/lldb/include/lldb/lldb-private-enumerations.h
@@ -207,8 +207,6 @@ enum class CompilerContextKind : uint16_t {
Builtin = 1 << 10,
Any = 1 << 15,
- /// Match 0..n nested modules.
- AnyModule = Any | Module,
/// Match any type.
AnyType = Any | ClassOrStruct | Union | Enum | Typedef | Builtin,
/// Math any declaration context.
diff --git a/lldb/source/Symbol/Type.cpp b/lldb/source/Symbol/Type.cpp
index e76574795733f..eb321407e3734 100644
--- a/lldb/source/Symbol/Type.cpp
+++ b/lldb/source/Symbol/Type.cpp
@@ -6,7 +6,9 @@
//
//===--===//
+#include
#include
+#include
#include
#include "lldb/Core/Module.h"
@@ -30,6 +32,7 @@
#include "lldb/Target/Process.h"
#include "lldb/Target/Target.h"
#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-private-enumerations.h"
#include "llvm/ADT/StringRef.h"
@@ -43,35 +46,6 @@ llvm::raw_ostream
&lldb_private::operator<<(llvm::raw_ostream &os,
return os << lldb_stream.GetString();
}
-bool lldb_private::contextMatches(llvm::ArrayRef
context_chain,
- llvm::ArrayRef pattern) {
- auto ctx = context_chain.begin();
- auto ctx_end = context_chain.end();
- for (const CompilerContext &pat : pattern) {
-// Early exit if the pattern is too long.
-if (ctx == ctx_end)
- return false;
-if (*ctx != pat) {
- // Skip any number of module matches.
- if (pat.kind == CompilerContextKind::AnyModule) {
[Lldb-commits] [lldb] [lldb] Refactor TypeQuery::ContextMatches, take 2 (PR #101333)
https://github.com/labath closed https://github.com/llvm/llvm-project/pull/101333 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Tolerate multiple compile units with the same DWO ID (PR #100577)
@@ -97,12 +97,14 @@ void DWARFUnit::ExtractUnitDIEIfNeeded() {
*m_dwo_id, m_first_die.GetOffset()));
return; // Can't fetch the compile unit from the dwo file.
}
- // If the skeleton compile unit gets its unit DIE parsed first, then this
- // will fill in the DWO file's back pointer to this skeleton compile unit.
- // If the DWO files get parsed on their own first the skeleton back link
- // can be done manually in DWARFUnit::GetSkeletonCompileUnit() which will
- // do a reverse lookup and cache the result.
- dwo_cu->SetSkeletonUnit(this);
+
+ // Link the DWO unit to this object, if it hasn't been linked already (this
+ // can happen when we have an index, and the DWO unit is parsed first).
+ if (!dwo_cu->LinkToSkeletonUnit(*this)) {
+SetDwoError(Status::createWithFormat(
+"multiple compile units with Dwo ID {0:x16}", *m_dwo_id));
labath wrote:
Any thoughts on what I wrote above, Greg?
https://github.com/llvm/llvm-project/pull/100577
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Refactor TypeQuery::ContextMatches (PR #99305)
https://github.com/labath closed https://github.com/llvm/llvm-project/pull/99305 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Refactor TypeQuery::ContextMatches (PR #99305)
labath wrote: Superseeded by #101333 https://github.com/llvm/llvm-project/pull/99305 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [lldb] [clang] Reland: Instantiate alias templates with sugar (PR #101858)
bolshakov-a wrote: When instantiating such an alias: ```cpp template using Identity = T; ``` before the patch: ``` TemplateSpecializationType 0x676f0a60 'Identity' sugar alias |-name: 'Identity' qualified | `-TypeAliasTemplateDecl 0x676c91d8 Identity |-TemplateArgument type 'IndirectClass' | `-ElaboratedType 0x676c9250 'IndirectClass' sugar | `-RecordType 0x676c8430 'class IndirectClass' | `-CXXRecord 0x676c8398 'IndirectClass' `-SubstTemplateTypeParmType 0x676f0a20 'class IndirectClass' sugar typename depth 0 index 0 T |-TypeAliasTemplate 0x676c91d8 'Identity' `-RecordType 0x676c8430 'class IndirectClass' `-CXXRecord 0x676c8398 'IndirectClass' ``` After the patch: ``` TemplateSpecializationType 0x676f0a30 'Identity' sugar alias |-name: 'Identity' qualified | `-TypeAliasTemplateDecl 0x676c91d8 Identity |-TemplateArgument type 'IndirectClass' | `-ElaboratedType 0x676c9250 'IndirectClass' sugar | `-RecordType 0x676c8430 'class IndirectClass' | `-CXXRecord 0x676c8398 'IndirectClass' `-ElaboratedType 0x676c9250 'IndirectClass' sugar `-RecordType 0x676c8430 'class IndirectClass' `-CXXRecord 0x676c8398 'IndirectClass' ``` @mizvekov, do you have any idea how to get back the lost `SubstTemplateTypeParmType`? It plays an important role in the IWYU tool analysis. Thanks! https://github.com/llvm/llvm-project/pull/101858 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [lldb] [clang] Reland: Instantiate alias templates with sugar (PR #101858)
mizvekov wrote: > @mizvekov, do you have any idea how to get back the lost > `SubstTemplateTypeParmType`? It plays an important role in the IWYU tool > analysis. Thanks! So from my undertstanding, IWYU only needs the SubstTemplateTypeParmType for resugaring purposes, in order to recover the type as written by the user. But with this patch we are doing the substitution already with sugar, so there is no need to resugar, so IWYU shouldn't need the SubstTemplateTypeParmType here anymore. Does that make sense? https://github.com/llvm/llvm-project/pull/101858 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Rename `scripting template` to `scripting extension` (NFC) (PR #101935)
https://github.com/jimingham approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/101935 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
