[Lldb-commits] [lldb] 2af0e2f - Revert "Push down the swig module to avoid an import cycle" (#129714)
Author: David Spickett
Date: 2025-03-04T14:24:02Z
New Revision: 2af0e2f3e6c761ecd3f2dd31d0ae844572fcdce0
URL:
https://github.com/llvm/llvm-project/commit/2af0e2f3e6c761ecd3f2dd31d0ae844572fcdce0
DIFF:
https://github.com/llvm/llvm-project/commit/2af0e2f3e6c761ecd3f2dd31d0ae844572fcdce0.diff
LOG: Revert "Push down the swig module to avoid an import cycle" (#129714)
Reverts llvm/llvm-project#129135 due to buildbot test failures.
Definitely caused remote Linux to Windows failures
(https://lab.llvm.org/buildbot/#/builders/197/builds/2712), may be the
cause of Windows on Arm failures
https://lab.llvm.org/buildbot/#/builders/141/builds/6744.
Added:
Modified:
lldb/bindings/python/CMakeLists.txt
lldb/bindings/python/python.swig
Removed:
diff --git a/lldb/bindings/python/CMakeLists.txt
b/lldb/bindings/python/CMakeLists.txt
index c4bf74e094f00..69306a384e0b1 100644
--- a/lldb/bindings/python/CMakeLists.txt
+++ b/lldb/bindings/python/CMakeLists.txt
@@ -59,10 +59,8 @@ endfunction()
function(finish_swig_python swig_target lldb_python_bindings_dir
lldb_python_target_dir)
# Add a Post-Build Event to copy over Python files and create the symlink to
# liblldb.so for the Python API(hardlink on Windows).
- # Note that Swig-generated code is located one level deeper in the `native`
- # module, in order to avoid cyclic importing.
add_custom_target(${swig_target} ALL VERBATIM
-COMMAND ${CMAKE_COMMAND} -E make_directory
${lldb_python_target_dir}/native/
+COMMAND ${CMAKE_COMMAND} -E make_directory ${lldb_python_target_dir}
DEPENDS ${lldb_python_bindings_dir}/lldb.py
COMMENT "Python script sym-linking LLDB Python API")
@@ -76,8 +74,6 @@ function(finish_swig_python swig_target
lldb_python_bindings_dir lldb_python_tar
"${LLDB_SOURCE_DIR}/source/Interpreter/embedded_interpreter.py"
"${lldb_python_target_dir}")
- create_python_package(${swig_target} ${lldb_python_target_dir} "native"
FILES)
-
# Distribute the examples as python packages.
create_python_package(
${swig_target}
@@ -145,7 +141,7 @@ function(finish_swig_python swig_target
lldb_python_bindings_dir lldb_python_tar
endif()
set(LIBLLDB_SYMLINK_OUTPUT_FILE "_lldb${LLDB_PYTHON_EXT_SUFFIX}")
create_relative_symlink(${swig_target} ${LIBLLDB_SYMLINK_DEST}
- ${lldb_python_target_dir}/native/
${LIBLLDB_SYMLINK_OUTPUT_FILE})
+ ${lldb_python_target_dir}
${LIBLLDB_SYMLINK_OUTPUT_FILE})
if (NOT WIN32)
diff --git a/lldb/bindings/python/python.swig
b/lldb/bindings/python/python.swig
index f1b156624e8d8..278c0eed2bab2 100644
--- a/lldb/bindings/python/python.swig
+++ b/lldb/bindings/python/python.swig
@@ -50,12 +50,7 @@ Older swig versions will simply ignore this setting.
import $module
except ImportError:
# Relative import should work if we are being loaded by Python.
-# The cpython module built by swig is pushed one level down into
-# the native submodule, because at this point the interpreter
-# is still constructing the lldb module itself.
-# Simply importing anything using `from . import` constitutes
-# a cyclic importing.
-from .native import $module"
+from . import $module"
%enddef
// The name of the module to be created.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 7b596ce - [lldb] Fix ObjectFileJSON to section addresses. (#129648)
Author: Greg Clayton
Date: 2025-03-04T14:35:42-08:00
New Revision: 7b596ce362829281ea73b576774ce90bb212138d
URL:
https://github.com/llvm/llvm-project/commit/7b596ce362829281ea73b576774ce90bb212138d
DIFF:
https://github.com/llvm/llvm-project/commit/7b596ce362829281ea73b576774ce90bb212138d.diff
LOG: [lldb] Fix ObjectFileJSON to section addresses. (#129648)
ObjectFileJSON sections didn't work, they were set to zero all of the
time. Fixed the bug and fixed the test to ensure it was testing real
values.
Added:
Modified:
lldb/source/Core/Section.cpp
lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp
lldb/test/API/functionalities/json/object-file/TestObjectFileJSON.py
Removed:
diff --git a/lldb/source/Core/Section.cpp b/lldb/source/Core/Section.cpp
index a17f43fe89033..96410a1ad497c 100644
--- a/lldb/source/Core/Section.cpp
+++ b/lldb/source/Core/Section.cpp
@@ -690,7 +690,7 @@ bool fromJSON(const llvm::json::Value &value,
lldb_private::JSONSection §ion, llvm::json::Path path) {
llvm::json::ObjectMapper o(value, path);
return o && o.map("name", section.name) && o.map("type", section.type) &&
- o.map("size", section.address) && o.map("size", section.size);
+ o.map("address", section.address) && o.map("size", section.size);
}
bool fromJSON(const llvm::json::Value &value, lldb::SectionType &type,
diff --git a/lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp
b/lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp
index ffbd87714242c..8e90fac46f348 100644
--- a/lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp
+++ b/lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp
@@ -182,9 +182,16 @@ void ObjectFileJSON::CreateSections(SectionList
&unified_section_list) {
lldb::user_id_t id = 1;
for (const auto §ion : m_sections) {
auto section_sp = std::make_shared(
-GetModule(), this, id++, ConstString(section.name),
-section.type.value_or(eSectionTypeCode), 0, section.size.value_or(0),
0,
-section.size.value_or(0), /*log2align*/ 0, /*flags*/ 0);
+GetModule(), this,
+/*sect_id=*/id++,
+/*name=*/ConstString(section.name),
+/*sect_type=*/section.type.value_or(eSectionTypeCode),
+/*file_vm_addr=*/section.address.value_or(0),
+/*vm_size=*/section.size.value_or(0),
+/*file_offset=*/0,
+/*file_size=*/0,
+/*log2align=*/0,
+/*flags=*/0);
m_sections_up->AddSection(section_sp);
unified_section_list.AddSection(section_sp);
}
diff --git
a/lldb/test/API/functionalities/json/object-file/TestObjectFileJSON.py
b/lldb/test/API/functionalities/json/object-file/TestObjectFileJSON.py
index efb1aa2c3ad8a..eee0144ad5706 100644
--- a/lldb/test/API/functionalities/json/object-file/TestObjectFileJSON.py
+++ b/lldb/test/API/functionalities/json/object-file/TestObjectFileJSON.py
@@ -59,7 +59,15 @@ def test_module(self):
module = target.AddModule(self.toModuleSpec(json_object_file_b))
self.assertFalse(module.IsValid())
-
+TEXT_file_addr = 0x1
+DATA_file_addr = 0x11000
+foo_file_addr = TEXT_file_addr + 0x100
+bar_file_addr = DATA_file_addr + 0x10
+TEXT_size = 0x222
+DATA_size = 0x333
+foo_size = 0x11
+bar_size = 0x22
+slide = 0x1
data = {
"triple": target.GetTriple(),
"uuid": str(uuid.uuid4()),
@@ -68,16 +76,29 @@ def test_module(self):
{
"name": "__TEXT",
"type": "code",
-"address": 0,
-"size": 0x222,
+"address": TEXT_file_addr,
+"size": TEXT_size,
+},
+{
+"name": "__DATA",
+"type": "code",
+"address": DATA_file_addr,
+"size": DATA_size,
}
],
"symbols": [
{
"name": "foo",
-"address": 0x100,
-"size": 0x11,
-}
+"type": "code",
+"address": foo_file_addr,
+"size": foo_size,
+},
+{
+"name": "bar",
+"type": "data",
+"address": bar_file_addr,
+"size": bar_size,
+},
],
}
@@ -87,17 +108,31 @@ def test_module(self):
module = target.AddModule(self.toModuleSpec(json_object_file_c))
self.assertTrue(module.IsValid())
-section = module.GetSectionAtIndex(0)
-self.assertTrue(section.IsValid())
-self.assertEqual(section.GetName(), "__TEXT")
-self.assertEqual(section.file_addr, 0x0)
[Lldb-commits] [lldb] 6e28700 - [lldb-dap] Improving EOF handling on stream input and adding new unit tests (#129581)
Author: John Harrison
Date: 2025-03-04T10:09:28-08:00
New Revision: 6e28700ab1d876a9b01647782ce3c0ed4d8e0bb4
URL:
https://github.com/llvm/llvm-project/commit/6e28700ab1d876a9b01647782ce3c0ed4d8e0bb4
DIFF:
https://github.com/llvm/llvm-project/commit/6e28700ab1d876a9b01647782ce3c0ed4d8e0bb4.diff
LOG: [lldb-dap] Improving EOF handling on stream input and adding new unit
tests (#129581)
This should improve the handling of EOF on stdin and adding some new
unit tests to malformed requests.
Added:
lldb/test/API/tools/lldb-dap/io/TestDAP_io.py
Modified:
lldb/tools/lldb-dap/DAP.cpp
lldb/tools/lldb-dap/IOStream.cpp
Removed:
diff --git a/lldb/test/API/tools/lldb-dap/io/TestDAP_io.py
b/lldb/test/API/tools/lldb-dap/io/TestDAP_io.py
new file mode 100644
index 0..a39bd17ceb3b3
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/io/TestDAP_io.py
@@ -0,0 +1,72 @@
+"""
+Test lldb-dap IO handling.
+"""
+
+from lldbsuite.test.decorators import *
+import lldbdap_testcase
+import dap_server
+
+
+class TestDAP_io(lldbdap_testcase.DAPTestCaseBase):
+def launch(self):
+log_file_path = self.getBuildArtifact("dap.txt")
+process, _ = dap_server.DebugAdapterServer.launch(
+executable=self.lldbDAPExec, log_file=log_file_path
+)
+
+def cleanup():
+# If the process is still alive, terminate it.
+if process.poll() is None:
+process.terminate()
+stdout_data = process.stdout.read()
+stderr_data = process.stderr.read()
+print("= STDOUT =")
+print(stdout_data)
+print("= END =")
+print("= STDERR =")
+print(stderr_data)
+print("= END =")
+print("= DEBUG ADAPTER PROTOCOL LOGS =")
+with open(log_file_path, "r") as file:
+print(file.read())
+print("= END =")
+
+# Execute the cleanup function during test case tear down.
+self.addTearDownHook(cleanup)
+
+return process
+
+def test_eof_immediately(self):
+"""
+lldb-dap handles EOF without any other input.
+"""
+process = self.launch()
+process.stdin.close()
+self.assertEqual(process.wait(timeout=5.0), 0)
+
+def test_partial_header(self):
+"""
+lldb-dap handles parital message headers.
+"""
+process = self.launch()
+process.stdin.write(b"Content-Length: ")
+process.stdin.close()
+self.assertEqual(process.wait(timeout=5.0), 0)
+
+def test_incorrect_content_length(self):
+"""
+lldb-dap handles malformed content length headers.
+"""
+process = self.launch()
+process.stdin.write(b"Content-Length: abc")
+process.stdin.close()
+self.assertEqual(process.wait(timeout=5.0), 0)
+
+def test_partial_content_length(self):
+"""
+lldb-dap handles partial messages.
+"""
+process = self.launch()
+process.stdin.write(b"Content-Length: 10{")
+process.stdin.close()
+self.assertEqual(process.wait(timeout=5.0), 0)
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 53c514b790f38..3dc9d6f5ca0a4 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -856,7 +856,11 @@ lldb::SBError DAP::Disconnect(bool terminateDebuggee) {
}
llvm::Error DAP::Loop() {
- auto cleanup = llvm::make_scope_exit([this]() { StopEventHandlers(); });
+ auto cleanup = llvm::make_scope_exit([this]() {
+if (output.descriptor)
+ output.descriptor->Close();
+StopEventHandlers();
+ });
while (!disconnecting) {
llvm::json::Object object;
lldb_dap::PacketStatus status = GetNextObject(object);
diff --git a/lldb/tools/lldb-dap/IOStream.cpp
b/lldb/tools/lldb-dap/IOStream.cpp
index c6f1bfaf3b799..ee22a297ec248 100644
--- a/lldb/tools/lldb-dap/IOStream.cpp
+++ b/lldb/tools/lldb-dap/IOStream.cpp
@@ -35,16 +35,23 @@ bool InputStream::read_full(std::ofstream *log, size_t
length,
if (status.Fail())
return false;
- text += data;
+ text += data.substr(0, length);
return true;
}
bool InputStream::read_line(std::ofstream *log, std::string &line) {
line.clear();
while (true) {
-if (!read_full(log, 1, line))
+std::string next;
+if (!read_full(log, 1, next))
return false;
+// If EOF is encoutnered, '' is returned, break out of this loop.
+if (next.empty())
+ return false;
+
+line += next;
+
if (llvm::StringRef(line).ends_with("\r\n"))
break;
}
@@ -60,6 +67,7 @@ bool InputStream::read_expected(std::ofstream *log,
llvm::StringRef expected) {
if (log)
*log <<
[Lldb-commits] [lldb] 27901ce - Add subsection and permissions support to ObjectFileJSON. (#129801)
Author: Greg Clayton
Date: 2025-03-04T16:19:20-08:00
New Revision: 27901cec0e76d2cbf648b3b63d5b2fec1d46bb9c
URL:
https://github.com/llvm/llvm-project/commit/27901cec0e76d2cbf648b3b63d5b2fec1d46bb9c
DIFF:
https://github.com/llvm/llvm-project/commit/27901cec0e76d2cbf648b3b63d5b2fec1d46bb9c.diff
LOG: Add subsection and permissions support to ObjectFileJSON. (#129801)
This patch adds the ability to create subsections in a section and
allows permissions to be specified.
Added:
Modified:
lldb/include/lldb/Core/Section.h
lldb/source/Core/Section.cpp
lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp
lldb/test/API/functionalities/json/object-file/TestObjectFileJSON.py
Removed:
diff --git a/lldb/include/lldb/Core/Section.h
b/lldb/include/lldb/Core/Section.h
index 4bf73e2e5a068..17b3cb454949f 100644
--- a/lldb/include/lldb/Core/Section.h
+++ b/lldb/include/lldb/Core/Section.h
@@ -105,6 +105,11 @@ struct JSONSection {
std::optional type;
std::optional address;
std::optional size;
+ // Section permissions;
+ std::optional read;
+ std::optional write;
+ std::optional execute;
+ std::vector subsections;
};
class Section : public std::enable_shared_from_this,
diff --git a/lldb/source/Core/Section.cpp b/lldb/source/Core/Section.cpp
index 96410a1ad497c..608e2a5fc3093 100644
--- a/lldb/source/Core/Section.cpp
+++ b/lldb/source/Core/Section.cpp
@@ -690,7 +690,10 @@ bool fromJSON(const llvm::json::Value &value,
lldb_private::JSONSection §ion, llvm::json::Path path) {
llvm::json::ObjectMapper o(value, path);
return o && o.map("name", section.name) && o.map("type", section.type) &&
- o.map("address", section.address) && o.map("size", section.size);
+ o.map("address", section.address) && o.map("size", section.size) &&
+ o.map("read", section.read) && o.map("write", section.write) &&
+ o.map("execute", section.execute) &&
+ o.mapOptional("subsections", section.subsections);
}
bool fromJSON(const llvm::json::Value &value, lldb::SectionType &type,
diff --git a/lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp
b/lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp
index 8e90fac46f348..0f9676b836b50 100644
--- a/lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp
+++ b/lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp
@@ -180,18 +180,55 @@ void ObjectFileJSON::CreateSections(SectionList
&unified_section_list) {
m_sections_up = std::make_unique();
lldb::user_id_t id = 1;
- for (const auto §ion : m_sections) {
-auto section_sp = std::make_shared(
-GetModule(), this,
-/*sect_id=*/id++,
-/*name=*/ConstString(section.name),
-/*sect_type=*/section.type.value_or(eSectionTypeCode),
-/*file_vm_addr=*/section.address.value_or(0),
-/*vm_size=*/section.size.value_or(0),
-/*file_offset=*/0,
-/*file_size=*/0,
-/*log2align=*/0,
-/*flags=*/0);
+ for (const auto &json_section : m_sections) {
+auto make_section = [this, &id](const JSONSection §ion,
+SectionSP parent_section_sp =
+nullptr) -> SectionSP {
+ SectionSP section_sp;
+ if (parent_section_sp) {
+section_sp = std::make_shared(
+parent_section_sp, GetModule(), this,
+/*sect_id=*/id++,
+/*name=*/ConstString(section.name),
+/*sect_type=*/section.type.value_or(eSectionTypeCode),
+/*file_vm_addr=*/section.address.value_or(0) -
+parent_section_sp->GetFileAddress(),
+/*vm_size=*/section.size.value_or(0),
+/*file_offset=*/0,
+/*file_size=*/0,
+/*log2align=*/0,
+/*flags=*/0);
+
+ } else {
+section_sp = std::make_shared(
+GetModule(), this,
+/*sect_id=*/id++,
+/*name=*/ConstString(section.name),
+/*sect_type=*/section.type.value_or(eSectionTypeCode),
+/*file_vm_addr=*/section.address.value_or(0),
+/*vm_size=*/section.size.value_or(0),
+/*file_offset=*/0,
+/*file_size=*/0,
+/*log2align=*/0,
+/*flags=*/0);
+ }
+ uint32_t permissions = 0;
+ if (section.read.value_or(0))
+permissions |= lldb::ePermissionsReadable;
+ if (section.write.value_or(0))
+permissions |= lldb::ePermissionsWritable;
+ if (section.execute.value_or(0))
+permissions |= lldb::ePermissionsExecutable;
+ if (permissions)
+section_sp->SetPermissions(permissions);
+ return section_sp;
+};
+auto section_sp = make_section(json_section);
+for (const auto &subsection : json_section.subsections) {
+ SectionSP subsection_sp = make_section(subsection, section_sp);
+ section_sp->GetCh
[Lldb-commits] [lldb] [lldb] Upgrade CompilerType::GetBitSize to return llvm::Expected (PR #129601)
https://github.com/JDevlieghere edited https://github.com/llvm/llvm-project/pull/129601 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Upgrade CompilerType::GetBitSize to return llvm::Expected (PR #129601)
https://github.com/JDevlieghere approved this pull request. TIL about `llvm::expectedToOptional`. LGTM! https://github.com/llvm/llvm-project/pull/129601 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Upgrade CompilerType::GetBitSize to return llvm::Expected (PR #129601)
@@ -769,19 +771,20 @@ CompilerType::GetBasicTypeFromAST(lldb::BasicType
basic_type) const {
}
// Exploring the type
-std::optional
+llvm::Expected
CompilerType::GetBitSize(ExecutionContextScope *exe_scope) const {
if (IsValid())
if (auto type_system_sp = GetTypeSystem())
return type_system_sp->GetBitSize(m_type, exe_scope);
- return {};
+ return llvm::createStringError("invalid type; cannot determine size");
JDevlieghere wrote:
```suggestion
return llvm::createStringError("invalid type: cannot determine size");
```
We generally concatenate errors with `:` rather than `;`. Will require updating
the two tests below.
https://github.com/llvm/llvm-project/pull/129601
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Replace Get{Signed, Unsigned} with GetInteger (NFC) (PR #129823)
@@ -131,11 +131,13 @@ void BreakpointLocationsRequestHandler::operator()(
auto *arguments = request.getObject("arguments");
auto *source = arguments->getObject("source");
std::string path = GetString(source, "path").str();
- uint64_t start_line = GetUnsigned(arguments, "line", 0);
- uint64_t start_column = GetUnsigned(arguments, "column", 0);
- uint64_t end_line = GetUnsigned(arguments, "endLine", start_line);
- uint64_t end_column =
- GetUnsigned(arguments, "endColumn",
std::numeric_limits::max());
+ const auto start_line = GetInteger(arguments, "line").value_or(0);
+ const auto start_column =
+ GetInteger(arguments, "column").value_or(0);
+ const auto end_line =
+ GetInteger(arguments, "endLine").value_or(start_line);
+ const auto end_column = GetInteger(arguments, "endColumn")
+ .value_or(std::numeric_limits::max());
ashgti wrote:
Should we use `LLDB_INVALID_LINE_NUMBER` as the default for any of the 'line'
values?
And for the 'column' should default to `LLDB_INVALID_COLUMN_NUMBER`, which is
also 0, but the value makes it more explicit that we're using it as a fallback.
I suppose that may result in a change of behavior, so we may not want to but it
seems like some like we may want to have more specific fallbacks. For example,
I haven't actually tried if I can set a breakpoint on line 0 (or 1 depending on
how you count) of a file.
https://github.com/llvm/llvm-project/pull/129823
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Replace Get{Signed, Unsigned} with GetInteger (NFC) (PR #129823)
https://github.com/JDevlieghere created
https://github.com/llvm/llvm-project/pull/129823
Replace Get{Signed,Unsigned} with GetInteger and return std::optional so you
can distinguish between the value not being present and it being explicitly set
to the previous fail_value. All existing uses are replaced by calling
value_or(fail_value).
Continuation of #129818
>From ee5794a0c5cca80beabb9e9a56ec961c4d4935b4 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere
Date: Tue, 4 Mar 2025 20:41:17 -0800
Subject: [PATCH] [lldb-dap] Replace Get{Signed,Unsigned} with GetInteger
(NFC)
Replace Get{Signed,Unsigned} with GetInteger and return std::optional
so you can distinguish between the value not being present and it being
explicitly set to the previous fail_value. All existing uses are
replaced by calling value_or(fail_value).
Continuation of #129818
---
lldb/tools/lldb-dap/DAP.cpp | 8 ++--
.../lldb-dap/Handler/AttachRequestHandler.cpp | 7 +--
.../Handler/BreakpointLocationsHandler.cpp| 12 +++--
.../lldb-dap/Handler/CompletionsHandler.cpp | 5 +-
.../DataBreakpointInfoRequestHandler.cpp | 2 +-
.../Handler/DisassembleRequestHandler.cpp | 5 +-
.../Handler/LocationsRequestHandler.cpp | 3 +-
.../Handler/ReadMemoryRequestHandler.cpp | 5 +-
.../tools/lldb-dap/Handler/RequestHandler.cpp | 3 +-
.../Handler/SetVariableRequestHandler.cpp | 5 +-
.../lldb-dap/Handler/SourceRequestHandler.cpp | 6 ++-
.../Handler/StackTraceRequestHandler.cpp | 5 +-
.../lldb-dap/Handler/StepInRequestHandler.cpp | 3 +-
.../Handler/VariablesRequestHandler.cpp | 6 +--
lldb/tools/lldb-dap/InstructionBreakpoint.cpp | 2 +-
lldb/tools/lldb-dap/JSONUtils.cpp | 32 +
lldb/tools/lldb-dap/JSONUtils.h | 46 ---
lldb/tools/lldb-dap/SourceBreakpoint.cpp | 4 +-
18 files changed, 68 insertions(+), 91 deletions(-)
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 3dc9d6f5ca0a4..32e94ff9f2bb6 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -526,12 +526,14 @@ ExceptionBreakpoint
*DAP::GetExceptionBPFromStopReason(lldb::SBThread &thread) {
}
lldb::SBThread DAP::GetLLDBThread(const llvm::json::Object &arguments) {
- auto tid = GetSigned(arguments, "threadId", LLDB_INVALID_THREAD_ID);
+ auto tid = GetInteger(arguments, "threadId")
+ .value_or(LLDB_INVALID_THREAD_ID);
return target.GetProcess().GetThreadByID(tid);
}
lldb::SBFrame DAP::GetLLDBFrame(const llvm::json::Object &arguments) {
- const uint64_t frame_id = GetUnsigned(arguments, "frameId", UINT64_MAX);
+ const uint64_t frame_id =
+ GetInteger(arguments, "frameId").value_or(UINT64_MAX);
lldb::SBProcess process = target.GetProcess();
// Upper 32 bits is the thread index ID
lldb::SBThread thread =
@@ -771,7 +773,7 @@ bool DAP::HandleObject(const llvm::json::Object &object) {
}
if (packet_type == "response") {
-auto id = GetSigned(object, "request_seq", 0);
+auto id = GetInteger(object, "request_seq").value_or(0);
std::unique_ptr response_handler;
{
diff --git a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp
b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp
index 8b203e0392066..4c0970bf941e8 100644
--- a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp
@@ -54,9 +54,9 @@ void AttachRequestHandler::operator()(const
llvm::json::Object &request) const {
const int invalid_port = 0;
const auto *arguments = request.getObject("arguments");
const lldb::pid_t pid =
- GetUnsigned(arguments, "pid", LLDB_INVALID_PROCESS_ID);
+ GetInteger(arguments, "pid").value_or(LLDB_INVALID_PROCESS_ID);
const auto gdb_remote_port =
- GetUnsigned(arguments, "gdb-remote-port", invalid_port);
+ GetInteger(arguments,
"gdb-remote-port").value_or(invalid_port);
const auto gdb_remote_hostname =
GetString(arguments, "gdb-remote-hostname", "localhost");
if (pid != LLDB_INVALID_PROCESS_ID)
@@ -70,7 +70,8 @@ void AttachRequestHandler::operator()(const
llvm::json::Object &request) const {
dap.terminate_commands = GetStrings(arguments, "terminateCommands");
auto attachCommands = GetStrings(arguments, "attachCommands");
llvm::StringRef core_file = GetString(arguments, "coreFile");
- const uint64_t timeout_seconds = GetUnsigned(arguments, "timeout", 30);
+ const auto timeout_seconds =
+ GetInteger(arguments, "timeout").value_or(30);
dap.stop_at_entry =
core_file.empty() ? GetBoolean(arguments, "stopOnEntry", false) : true;
dap.post_run_commands = GetStrings(arguments, "postRunCommands");
diff --git a/lldb/tools/lldb-dap/Handler/BreakpointLocationsHandler.cpp
b/lldb/tools/lldb-dap/Handler/BreakpointLocationsHandler.cpp
index 1b5c8ba307dcb..468dacfe6737e 100644
--- a/lldb/tools/lldb-dap/Handler/BreakpointLocationsHandler.cpp
+++
[Lldb-commits] [lldb] [lldb-dap] Creating well defined structures for DAP messages. (PR #129155)
https://github.com/JDevlieghere approved this pull request. https://github.com/llvm/llvm-project/pull/129155 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Return a std::optional from GetBoolean (NFC) (PR #129818)
https://github.com/ashgti approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/129818 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Updating the logging of lldb-dap to use existing LLDBLog. (PR #129294)
JDevlieghere wrote: The more I think about it, the less I like the idea of using `LLDBLog` in `lldb-dap`. Initially, I liked the idea of reusing our existing logging implementation, although I have expressed my concern about relying on `liblldb` in other PRs. But the issues and workarounds we're talking about here sound to me like signs that we're taking it too far. I think if we want to do this right, we have two options, and they're both pretty drastic: 1. We expose `LLDBLog` from the SB API. This is something that has come up in the past in the context of people writing LLDB scripts. Similar to exposing the ability to generate progress events from the SB API, I think we need to be careful about this and clearly distinguish between unvetted logs, coming from scripts and vetted logs, coming from LLDB itself. 2. We give up on trying to build lldb-dap on top the SB API and move the DAP functionality into liblldb, allowing us to use `lldb_private` and exposing a simple interface that gets set up in the `lldb-dap` binary, similar to the command line driver. On the one hand I think it'd be sad to give up on building on top of the SB API. I always liked the idea that in theory, you could use `lldb-dap` with a different version of `liblldb`. In practice, I don't know if anyone does. We've already been cheating in that regard but we've gotten away with it because of static linking and I have a feeling this is the direction we'll likely keep on going. It's just too hard to pass up on reusing existing functionality. There is of course the third option which is designing a dedicated logging framework for `lldb-dap`. Although `LLDBLog` has is upsides, it also has its drawbacks, most notably that everything is global. If you change something in one `(SB)Debugger` instance, it is reflected in all the other ones. We'd have a similar problem for `lldb-dap` where someone changing the logging settings in one instance will affect all instances in the same process, which will be especially problematic in server mode. https://github.com/llvm/llvm-project/pull/129294 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 3f12155 - [lldb-dap] Creating well defined structures for DAP messages. (#129155)
Author: John Harrison
Date: 2025-03-04T21:45:37-08:00
New Revision: 3f121558705ac7c9ae2398baca5a2d764ce022fd
URL:
https://github.com/llvm/llvm-project/commit/3f121558705ac7c9ae2398baca5a2d764ce022fd
DIFF:
https://github.com/llvm/llvm-project/commit/3f121558705ac7c9ae2398baca5a2d764ce022fd.diff
LOG: [lldb-dap] Creating well defined structures for DAP messages. (#129155)
This adds a new `Protocol.{h,cpp}` for defining structured types that
represent Debug Adapter Protocol messages.
This adds static types to define well structure messages for the
protocol. This iteration includes only the basic `Event`, `Request` and
`Response` types.
These types help simplify and improve the validation of messages and
give us additional static type checks on the overall structure of DAP
messages, compared to today where we tend to use `llvm::json::Value`
directly.
In a follow-up patch I plan on adding more types as need to allow for
incrementally migrating raw `llvm::json::Value` usage to well defined
types.
-
Co-authored-by: Adrian Vogelsgesang
Added:
lldb/tools/lldb-dap/Protocol.cpp
lldb/tools/lldb-dap/Protocol.h
Modified:
lldb/tools/lldb-dap/CMakeLists.txt
Removed:
diff --git a/lldb/tools/lldb-dap/CMakeLists.txt
b/lldb/tools/lldb-dap/CMakeLists.txt
index 8b3c520ec4360..9a2d604f4d573 100644
--- a/lldb/tools/lldb-dap/CMakeLists.txt
+++ b/lldb/tools/lldb-dap/CMakeLists.txt
@@ -35,6 +35,7 @@ add_lldb_tool(lldb-dap
ProgressEvent.cpp
RunInTerminal.cpp
SourceBreakpoint.cpp
+ Protocol.cpp
Watchpoint.cpp
Handler/ResponseHandler.cpp
diff --git a/lldb/tools/lldb-dap/Protocol.cpp
b/lldb/tools/lldb-dap/Protocol.cpp
new file mode 100644
index 0..b516c0cb19ebf
--- /dev/null
+++ b/lldb/tools/lldb-dap/Protocol.cpp
@@ -0,0 +1,291 @@
+//===-- Protocol.cpp
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Protocol.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSwitch.h"
+#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/JSON.h"
+#include
+#include
+
+using namespace llvm;
+
+static bool mapRaw(const json::Value &Params, StringLiteral Prop,
+ std::optional &V, json::Path P) {
+ const auto *O = Params.getAsObject();
+ if (!O) {
+P.report("expected object");
+return false;
+ }
+ const json::Value *E = O->get(Prop);
+ if (E)
+V = std::move(*E);
+ return true;
+}
+
+namespace lldb_dap {
+namespace protocol {
+
+enum class MessageType { request, response, event };
+
+bool fromJSON(const json::Value &Params, MessageType &M, json::Path P) {
+ auto rawType = Params.getAsString();
+ if (!rawType) {
+P.report("expected a string");
+return false;
+ }
+ std::optional type =
+ StringSwitch>(*rawType)
+ .Case("request", MessageType::request)
+ .Case("response", MessageType::response)
+ .Case("event", MessageType::event)
+ .Default(std::nullopt);
+ if (!type) {
+P.report("unexpected value, expected 'request', 'response' or 'event'");
+return false;
+ }
+ M = *type;
+ return true;
+}
+
+json::Value toJSON(const Request &R) {
+ json::Object Result{
+ {"type", "request"},
+ {"seq", R.seq},
+ {"command", R.command},
+ };
+
+ if (R.rawArguments)
+Result.insert({"arguments", R.rawArguments});
+
+ return std::move(Result);
+}
+
+bool fromJSON(json::Value const &Params, Request &R, json::Path P) {
+ json::ObjectMapper O(Params, P);
+ if (!O)
+return false;
+
+ MessageType type;
+ if (!O.map("type", type) || !O.map("command", R.command) ||
+ !O.map("seq", R.seq))
+return false;
+
+ if (type != MessageType::request) {
+P.field("type").report("expected to be 'request'");
+return false;
+ }
+
+ if (R.command.empty()) {
+P.field("command").report("expected to not be ''");
+return false;
+ }
+
+ if (!R.seq) {
+P.field("seq").report("expected to not be '0'");
+return false;
+ }
+
+ return mapRaw(Params, "arguments", R.rawArguments, P);
+}
+
+json::Value toJSON(const Response &R) {
+ json::Object Result{{"type", "response"},
+ {"seq", 0},
+ {"command", R.command},
+ {"request_seq", R.request_seq},
+ {"success", R.success}};
+
+ if (R.message) {
+assert(!R.success && "message can only be used if success is false");
+if (const auto *messageEnum = std::get_if(&*R.message))
{
+ switch (*messageEnum) {
+ case Response::Message::cancelled:
+Result.insert({"message", "cancelled"});
+break;
+
[Lldb-commits] [lldb] [lldb-dap] Updating the logging of lldb-dap to use existing LLDBLog. (PR #129294)
@@ -0,0 +1,30 @@
+//===-- DAPLog.cpp
===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "DAPLog.h"
+
+using namespace lldb_private;
+using namespace lldb_dap;
+
+static constexpr Log::Category g_categories[] = {
+{{"transport"}, {"log DAP transport"}, DAPLog::Transport},
+{{"protocol"}, {"log protocol handling"}, DAPLog::Protocol},
+{{"connection"}, {"log connection handling"}, DAPLog::Connection},
+};
+
+static Log::Channel g_log_channel(g_categories, DAPLog::Transport |
+DAPLog::Protocol |
+DAPLog::Connection);
+
+template <> Log::Channel &lldb_private::LogChannelFor() {
+ return g_log_channel;
+}
+
+void lldb_dap::InitializeDAPChannel() {
+ Log::Register("lldb-dap", g_log_channel);
ashgti wrote:
I think given this issue and some other issues related to using the LLDBLog API
I think I may not go forward with this patch and revisit at a later date. I
think lldb-dap could use some improvements to logging, instead of just passing
a `std::ofstream *` around, but I am not sure this is the best approach, also
see https://github.com/llvm/llvm-project/pull/129294#issuecomment-2699922582
https://github.com/llvm/llvm-project/pull/129294
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Minidump]Update MinidumpFileBuilder to read and write in chunks (PR #129307)
https://github.com/Jlalond updated
https://github.com/llvm/llvm-project/pull/129307
>From 2f77beefb752ffdae908101d75381268203311d6 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde
Date: Fri, 28 Feb 2025 11:38:35 -0800
Subject: [PATCH 1/4] Update MinidumpFileBuilder to read and write in chunks
---
.../Minidump/MinidumpFileBuilder.cpp | 130 --
.../ObjectFile/Minidump/MinidumpFileBuilder.h | 7 +
2 files changed, 97 insertions(+), 40 deletions(-)
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index c5013ea5e3be4..e88b606f279cd 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -969,12 +969,82 @@ Status MinidumpFileBuilder::DumpDirectories() const {
return error;
}
-static uint64_t
-GetLargestRangeSize(const std::vector &ranges) {
- uint64_t max_size = 0;
- for (const auto &core_range : ranges)
-max_size = std::max(max_size, core_range.range.size());
- return max_size;
+Status MinidumpFileBuilder::ReadWriteMemoryInChunks(
+const lldb_private::CoreFileMemoryRange &range, uint64_t *bytes_read) {
+ Log *log = GetLog(LLDBLog::Object);
+ lldb::addr_t addr = range.range.start();
+ lldb::addr_t size = range.range.size();
+ // First we set the byte tally to 0, so if we do exit gracefully
+ // the caller doesn't think the random garbage on the stack is a
+ // success.
+ if (bytes_read)
+*bytes_read = 0;
+
+ uint64_t bytes_remaining = size;
+ uint64_t total_bytes_read = 0;
+ auto data_up = std::make_unique(
+ std::min(bytes_remaining, MAX_WRITE_CHUNK_SIZE), 0);
+ Status error;
+ while (bytes_remaining > 0) {
+// Get the next read chunk size as the minimum of the remaining bytes and
+// the write chunk max size.
+const size_t bytes_to_read =
+std::min(bytes_remaining, MAX_WRITE_CHUNK_SIZE);
+const size_t bytes_read_for_chunk =
+m_process_sp->ReadMemory(range.range.start() + total_bytes_read,
+ data_up->GetBytes(), bytes_to_read, error);
+if (error.Fail() || bytes_read_for_chunk == 0) {
+ LLDB_LOGF(log,
+"Failed to read memory region at: %" PRIx64
+". Bytes read: %zu, error: %s",
+addr, bytes_read_for_chunk, error.AsCString());
+ // If we've only read one byte we can just give up and return
+ if (total_bytes_read == 0)
+return Status();
+
+ // If we've read some bytes, we stop trying to read more and return
+ // this best effort attempt
+ bytes_remaining = 0;
+} else if (bytes_read_for_chunk != bytes_to_read) {
+ LLDB_LOGF(
+ log, "Memory region at: %" PRIx64 " failed to read %" PRIx64 "
bytes",
+ addr, size);
+
+ // If we've read some bytes, we stop trying to read more and return
+ // this best effort attempt
+ bytes_remaining = 0;
+}
+
+// Write to the minidump file with the chunk potentially flushing to disk.
+// this is the only place we want to return a true error, so that we can
+// fail. If we get an error writing to disk we can't easily gaurauntee
+// that we won't corrupt the minidump.
+error = AddData(data_up->GetBytes(), bytes_read_for_chunk);
+if (error.Fail())
+ return error;
+
+// This check is so we don't overflow when the error code above sets the
+// bytes to read to 0 (the graceful exit condition).
+if (bytes_remaining > 0)
+ bytes_remaining -= bytes_read_for_chunk;
+
+total_bytes_read += bytes_read_for_chunk;
+// If the caller wants a tally back of the bytes_read, update it as we
+// write. We do this in the loop so if we encounter an error we can
+// report the accurate total.
+if (bytes_read)
+ *bytes_read += bytes_read_for_chunk;
+
+// We clear the heap per loop, without checking if we
+// read the expected bytes this is so we don't allocate
+// more than the MAX_WRITE_CHUNK_SIZE. But we do check if
+// this is even worth clearing before we return and
+// destruct the heap.
+if (bytes_remaining > 0)
+ data_up->Clear();
+ }
+
+ return error;
}
Status
@@ -987,8 +1057,6 @@
MinidumpFileBuilder::AddMemoryList_32(std::vector &ranges,
Log *log = GetLog(LLDBLog::Object);
size_t region_index = 0;
- auto data_up =
- std::make_unique(GetLargestRangeSize(ranges), 0);
for (const auto &core_range : ranges) {
// Take the offset before we write.
const offset_t offset_for_data = GetCurrentDataEndOffset();
@@ -1003,18 +1071,15 @@
MinidumpFileBuilder::AddMemoryList_32(std::vector &ranges,
++region_index;
progress.Increment(1, "Adding Memory Range " + core_range.Dump());
-const size_t bytes_read =
-m_process_sp->ReadMemory(addr, data_up->GetBytes(), size, error);
-if (error.Fail() || bytes_read == 0) {
[Lldb-commits] [lldb] [lldb-dap] Replace Get{Signed, Unsigned} with GetInteger (NFC) (PR #129823)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Jonas Devlieghere (JDevlieghere)
Changes
Replace Get{Signed,Unsigned} with GetInteger and return std::optional
so you can distinguish between the value not being present and it being
explicitly set to the previous fail_value. All existing uses are replaced by
calling value_or(fail_value).
Continuation of #129818
---
Patch is 20.25 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/129823.diff
18 Files Affected:
- (modified) lldb/tools/lldb-dap/DAP.cpp (+5-3)
- (modified) lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp (+4-3)
- (modified) lldb/tools/lldb-dap/Handler/BreakpointLocationsHandler.cpp (+7-5)
- (modified) lldb/tools/lldb-dap/Handler/CompletionsHandler.cpp (+3-2)
- (modified) lldb/tools/lldb-dap/Handler/DataBreakpointInfoRequestHandler.cpp
(+1-1)
- (modified) lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp (+3-2)
- (modified) lldb/tools/lldb-dap/Handler/LocationsRequestHandler.cpp (+2-1)
- (modified) lldb/tools/lldb-dap/Handler/ReadMemoryRequestHandler.cpp (+3-2)
- (modified) lldb/tools/lldb-dap/Handler/RequestHandler.cpp (+2-1)
- (modified) lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp (+3-2)
- (modified) lldb/tools/lldb-dap/Handler/SourceRequestHandler.cpp (+4-2)
- (modified) lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp (+3-2)
- (modified) lldb/tools/lldb-dap/Handler/StepInRequestHandler.cpp (+2-1)
- (modified) lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp (+3-3)
- (modified) lldb/tools/lldb-dap/InstructionBreakpoint.cpp (+1-1)
- (modified) lldb/tools/lldb-dap/JSONUtils.cpp (+1-31)
- (modified) lldb/tools/lldb-dap/JSONUtils.h (+19-27)
- (modified) lldb/tools/lldb-dap/SourceBreakpoint.cpp (+2-2)
``diff
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 3dc9d6f5ca0a4..32e94ff9f2bb6 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -526,12 +526,14 @@ ExceptionBreakpoint
*DAP::GetExceptionBPFromStopReason(lldb::SBThread &thread) {
}
lldb::SBThread DAP::GetLLDBThread(const llvm::json::Object &arguments) {
- auto tid = GetSigned(arguments, "threadId", LLDB_INVALID_THREAD_ID);
+ auto tid = GetInteger(arguments, "threadId")
+ .value_or(LLDB_INVALID_THREAD_ID);
return target.GetProcess().GetThreadByID(tid);
}
lldb::SBFrame DAP::GetLLDBFrame(const llvm::json::Object &arguments) {
- const uint64_t frame_id = GetUnsigned(arguments, "frameId", UINT64_MAX);
+ const uint64_t frame_id =
+ GetInteger(arguments, "frameId").value_or(UINT64_MAX);
lldb::SBProcess process = target.GetProcess();
// Upper 32 bits is the thread index ID
lldb::SBThread thread =
@@ -771,7 +773,7 @@ bool DAP::HandleObject(const llvm::json::Object &object) {
}
if (packet_type == "response") {
-auto id = GetSigned(object, "request_seq", 0);
+auto id = GetInteger(object, "request_seq").value_or(0);
std::unique_ptr response_handler;
{
diff --git a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp
b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp
index 8b203e0392066..4c0970bf941e8 100644
--- a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp
@@ -54,9 +54,9 @@ void AttachRequestHandler::operator()(const
llvm::json::Object &request) const {
const int invalid_port = 0;
const auto *arguments = request.getObject("arguments");
const lldb::pid_t pid =
- GetUnsigned(arguments, "pid", LLDB_INVALID_PROCESS_ID);
+ GetInteger(arguments, "pid").value_or(LLDB_INVALID_PROCESS_ID);
const auto gdb_remote_port =
- GetUnsigned(arguments, "gdb-remote-port", invalid_port);
+ GetInteger(arguments,
"gdb-remote-port").value_or(invalid_port);
const auto gdb_remote_hostname =
GetString(arguments, "gdb-remote-hostname", "localhost");
if (pid != LLDB_INVALID_PROCESS_ID)
@@ -70,7 +70,8 @@ void AttachRequestHandler::operator()(const
llvm::json::Object &request) const {
dap.terminate_commands = GetStrings(arguments, "terminateCommands");
auto attachCommands = GetStrings(arguments, "attachCommands");
llvm::StringRef core_file = GetString(arguments, "coreFile");
- const uint64_t timeout_seconds = GetUnsigned(arguments, "timeout", 30);
+ const auto timeout_seconds =
+ GetInteger(arguments, "timeout").value_or(30);
dap.stop_at_entry =
core_file.empty() ? GetBoolean(arguments, "stopOnEntry", false) : true;
dap.post_run_commands = GetStrings(arguments, "postRunCommands");
diff --git a/lldb/tools/lldb-dap/Handler/BreakpointLocationsHandler.cpp
b/lldb/tools/lldb-dap/Handler/BreakpointLocationsHandler.cpp
index 1b5c8ba307dcb..468dacfe6737e 100644
--- a/lldb/tools/lldb-dap/Handler/BreakpointLocationsHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/BreakpointLocationsHandler.cpp
@@ -131,11 +131,13 @@ void BreakpointLocationsReques
[Lldb-commits] [lldb] [lldb-dap] Replace Get{Signed, Unsigned} with GetInteger (NFC) (PR #129823)
https://github.com/ashgti approved this pull request. https://github.com/llvm/llvm-project/pull/129823 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Updating the logging of lldb-dap to use existing LLDBLog. (PR #129294)
ashgti wrote: > The more I think about it, the less I like the idea of using `LLDBLog` in > `lldb-dap`. Initially, I liked the idea of reusing our existing logging > implementation, although I have expressed my concern about relying on > `liblldb` in other PRs. But the issues and workarounds we're talking about > here sound to me like signs that we're taking it too far. I think I also ran into this issue when using some of the Host types in lldb-dap where we had two different 'global' instances of a value due to the static linking. Specifically this was the in the `lldb_private::FileSystem` API and its when I was trying to handling some defaults correctly across different platforms. I ended up re-writing the patch to not use that API specifically. > > I think if we want to do this right, we have two options, and they're both > pretty drastic: > > 1. We expose `LLDBLog` from the SB API. This is something that has come up in > the past in the context of people writing LLDB scripts. Similar to exposing > the ability to generate progress events from the SB API, I think we need to > be careful about this and clearly distinguish between unvetted logs, coming > from scripts and vetted logs, coming from LLDB itself. > 2. We give up on trying to build lldb-dap on top the SB API and move the DAP > functionality into liblldb, allowing us to use `lldb_private` and exposing a > simple interface that gets set up in the `lldb-dap` binary, similar to the > command line driver. On the one hand I think it'd be sad to give up on > building on top of the SB API. I always liked the idea that in theory, you > could use `lldb-dap` with a different version of `liblldb`. In practice, I > don't know if anyone does. We've already been cheating in that regard but > we've gotten away with it because of static linking and I have a feeling this > is the direction we'll likely keep on going. It's just too hard to pass up on > reusing existing functionality. At Google, we do actually do this to support building lldb-dap at head with the LLDB.framework linked from Xcode. This is a pretty specific use case because we are doing that so we can support mobile devices. And every once in a while when lldb-dap uses a new SB API that isn't in the internally supported Xcode release I end up writing either a back port or a fallback implementation. > There is of course the third option which is designing a dedicated logging > framework for `lldb-dap`. Although `LLDBLog` has is upsides, it also has its > drawbacks, most notably that everything is global. If you change something in > one `(SB)Debugger` instance, it is reflected in all the other ones. We'd have > a similar problem for `lldb-dap` where someone changing the logging settings > in one instance will affect all instances in the same process, which will be > especially problematic in server mode. I think there are some improvements we could make to the logging incrementally, instead of just passing a `std::ofstream *` around. Thinking about these issues, I agree that we may not want to use the LLDBLog api and instead maybe we can make some incremental improvements to the existing system instead. I may revisit this after I get my other patch in to refactor the IOStream into a Transport abstraction and then I had two other follow ups for better type handling and cancel request support. For now I think I'll just close this PR. https://github.com/llvm/llvm-project/pull/129294 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Replace Get{Signed, Unsigned} with GetInteger (NFC) (PR #129823)
@@ -131,11 +131,13 @@ void BreakpointLocationsRequestHandler::operator()(
auto *arguments = request.getObject("arguments");
auto *source = arguments->getObject("source");
std::string path = GetString(source, "path").str();
- uint64_t start_line = GetUnsigned(arguments, "line", 0);
- uint64_t start_column = GetUnsigned(arguments, "column", 0);
- uint64_t end_line = GetUnsigned(arguments, "endLine", start_line);
- uint64_t end_column =
- GetUnsigned(arguments, "endColumn",
std::numeric_limits::max());
+ const auto start_line = GetInteger(arguments, "line").value_or(0);
+ const auto start_column =
+ GetInteger(arguments, "column").value_or(0);
+ const auto end_line =
+ GetInteger(arguments, "endLine").value_or(start_line);
+ const auto end_column = GetInteger(arguments, "endColumn")
+ .value_or(std::numeric_limits::max());
JDevlieghere wrote:
I think it's a good idea, especially since we have code comparing against
`LLDB_INVALID_LINE_NUMBER`. I'll do that in a follow-up though to keep this one
NFC.
https://github.com/llvm/llvm-project/pull/129823
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Implement a MemoryMonitor for macOS & Linux (PR #129332)
https://github.com/JDevlieghere updated
https://github.com/llvm/llvm-project/pull/129332
>From c63350db79ac6bcc6f180cc84a37e829d1c8b2a8 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere
Date: Fri, 28 Feb 2025 14:54:42 -0600
Subject: [PATCH 1/5] [lldb-dap] Implement a MemoryMonitor for macOS & Linux
This implements a memory monitor for macOS & Linux, and registers a
callback that invokes SBDebugger::MemoryPressureDetected() when a low
memory event is detected.
---
lldb/tools/lldb-dap/CMakeLists.txt| 1 +
lldb/tools/lldb-dap/MemoryMonitor.cpp | 114 ++
lldb/tools/lldb-dap/MemoryMonitor.h | 41 +
lldb/tools/lldb-dap/lldb-dap.cpp | 17 +++-
4 files changed, 171 insertions(+), 2 deletions(-)
create mode 100644 lldb/tools/lldb-dap/MemoryMonitor.cpp
create mode 100644 lldb/tools/lldb-dap/MemoryMonitor.h
diff --git a/lldb/tools/lldb-dap/CMakeLists.txt
b/lldb/tools/lldb-dap/CMakeLists.txt
index 8b3c520ec4360..8db377e31c3c6 100644
--- a/lldb/tools/lldb-dap/CMakeLists.txt
+++ b/lldb/tools/lldb-dap/CMakeLists.txt
@@ -36,6 +36,7 @@ add_lldb_tool(lldb-dap
RunInTerminal.cpp
SourceBreakpoint.cpp
Watchpoint.cpp
+ MemoryMonitor.cpp
Handler/ResponseHandler.cpp
Handler/AttachRequestHandler.cpp
diff --git a/lldb/tools/lldb-dap/MemoryMonitor.cpp
b/lldb/tools/lldb-dap/MemoryMonitor.cpp
new file mode 100644
index 0..da3da42fe9b0f
--- /dev/null
+++ b/lldb/tools/lldb-dap/MemoryMonitor.cpp
@@ -0,0 +1,114 @@
+//===-- MemoryMonitor.cpp
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MemoryMonitor.h"
+#include "llvm/ADT/ScopeExit.h"
+#include
+#include
+#include
+#include
+
+#if defined(__APPLE__)
+#include
+#endif
+
+#if defined(__linux__)
+#include
+#include
+#include
+#endif
+
+using namespace lldb_dap;
+
+#if defined(__APPLE__)
+class MemoryMonitorDarwin : public MemoryMonitor {
+ using MemoryMonitor::MemoryMonitor;
+ void Start() override {
+m_memory_pressure_source = dispatch_source_create(
+DISPATCH_SOURCE_TYPE_MEMORYPRESSURE,
+0, // system-wide monitoring
+DISPATCH_MEMORYPRESSURE_WARN | DISPATCH_MEMORYPRESSURE_CRITICAL,
+dispatch_get_main_queue());
+
+dispatch_source_set_event_handler(m_memory_pressure_source, ^{
+ dispatch_source_memorypressure_flags_t pressureLevel =
+ dispatch_source_get_data(m_memory_pressure_source);
+ if (pressureLevel &
+ (DISPATCH_MEMORYPRESSURE_WARN | DISPATCH_MEMORYPRESSURE_CRITICAL)) {
+m_callback();
+ }
+});
+ }
+
+ void Stop() override { dispatch_source_cancel(m_memory_pressure_source); }
+
+private:
+ dispatch_source_t m_memory_pressure_source;
+};
+#endif
+
+#if defined(__linux__)
+static void MonitorThread(std::atomic &done,
+ MemoryMonitor::Callback callback) {
+ struct pollfd fds;
+ fds.fd = open("/proc/pressure/memory", O_RDWR | O_NONBLOCK);
+ if (fds.fd < 0)
+return;
+ fds.events = POLLPRI;
+
+ auto cleanup = llvm::make_scope_exit([&]() { close(fds.fd); });
+
+ // Detect a 50ms stall in a 2 second time window.
+ const char trig[] = "some 5 200";
+ if (write(fds.fd, trig, strlen(trig) + 1) < 0)
+return;
+
+ while (!done) {
+int n = poll(&fds, 1, 1000);
+if (n > 0) {
+ if (fds.revents & POLLERR)
+return;
+ if (fds.revents & POLLPRI)
+callback();
+}
+ }
+}
+
+class MemoryMonitorLinux : public MemoryMonitor {
+public:
+ using MemoryMonitor::MemoryMonitor;
+
+ void Start() override {
+m_memory_pressure_thread =
+std::thread(MonitorThread, std::ref(m_done), m_callback);
+ }
+
+ void Stop() override {
+if (m_memory_pressure_thread.joinable()) {
+ m_done = true;
+ m_memory_pressure_thread.join();
+}
+ }
+
+private:
+ std::atomic m_done = false;
+ std::thread m_memory_pressure_thread;
+};
+#endif
+
+std::unique_ptr MemoryMonitor::Create(Callback callback) {
+#if defined(__APPLE__)
+ return std::make_unique(callback);
+#endif
+
+#if defined(__linux__)
+ return std::make_unique(callback);
+#endif
+
+ return nullptr;
+}
diff --git a/lldb/tools/lldb-dap/MemoryMonitor.h
b/lldb/tools/lldb-dap/MemoryMonitor.h
new file mode 100644
index 0..e07c3bde9e85c
--- /dev/null
+++ b/lldb/tools/lldb-dap/MemoryMonitor.h
@@ -0,0 +1,41 @@
+//===-- MemoryMonitor.h
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===-
[Lldb-commits] [lldb] [lldb-dap] Implement a MemoryMonitor for macOS & Linux (PR #129332)
https://github.com/JDevlieghere updated
https://github.com/llvm/llvm-project/pull/129332
>From c63350db79ac6bcc6f180cc84a37e829d1c8b2a8 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere
Date: Fri, 28 Feb 2025 14:54:42 -0600
Subject: [PATCH 1/6] [lldb-dap] Implement a MemoryMonitor for macOS & Linux
This implements a memory monitor for macOS & Linux, and registers a
callback that invokes SBDebugger::MemoryPressureDetected() when a low
memory event is detected.
---
lldb/tools/lldb-dap/CMakeLists.txt| 1 +
lldb/tools/lldb-dap/MemoryMonitor.cpp | 114 ++
lldb/tools/lldb-dap/MemoryMonitor.h | 41 +
lldb/tools/lldb-dap/lldb-dap.cpp | 17 +++-
4 files changed, 171 insertions(+), 2 deletions(-)
create mode 100644 lldb/tools/lldb-dap/MemoryMonitor.cpp
create mode 100644 lldb/tools/lldb-dap/MemoryMonitor.h
diff --git a/lldb/tools/lldb-dap/CMakeLists.txt
b/lldb/tools/lldb-dap/CMakeLists.txt
index 8b3c520ec4360..8db377e31c3c6 100644
--- a/lldb/tools/lldb-dap/CMakeLists.txt
+++ b/lldb/tools/lldb-dap/CMakeLists.txt
@@ -36,6 +36,7 @@ add_lldb_tool(lldb-dap
RunInTerminal.cpp
SourceBreakpoint.cpp
Watchpoint.cpp
+ MemoryMonitor.cpp
Handler/ResponseHandler.cpp
Handler/AttachRequestHandler.cpp
diff --git a/lldb/tools/lldb-dap/MemoryMonitor.cpp
b/lldb/tools/lldb-dap/MemoryMonitor.cpp
new file mode 100644
index 0..da3da42fe9b0f
--- /dev/null
+++ b/lldb/tools/lldb-dap/MemoryMonitor.cpp
@@ -0,0 +1,114 @@
+//===-- MemoryMonitor.cpp
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MemoryMonitor.h"
+#include "llvm/ADT/ScopeExit.h"
+#include
+#include
+#include
+#include
+
+#if defined(__APPLE__)
+#include
+#endif
+
+#if defined(__linux__)
+#include
+#include
+#include
+#endif
+
+using namespace lldb_dap;
+
+#if defined(__APPLE__)
+class MemoryMonitorDarwin : public MemoryMonitor {
+ using MemoryMonitor::MemoryMonitor;
+ void Start() override {
+m_memory_pressure_source = dispatch_source_create(
+DISPATCH_SOURCE_TYPE_MEMORYPRESSURE,
+0, // system-wide monitoring
+DISPATCH_MEMORYPRESSURE_WARN | DISPATCH_MEMORYPRESSURE_CRITICAL,
+dispatch_get_main_queue());
+
+dispatch_source_set_event_handler(m_memory_pressure_source, ^{
+ dispatch_source_memorypressure_flags_t pressureLevel =
+ dispatch_source_get_data(m_memory_pressure_source);
+ if (pressureLevel &
+ (DISPATCH_MEMORYPRESSURE_WARN | DISPATCH_MEMORYPRESSURE_CRITICAL)) {
+m_callback();
+ }
+});
+ }
+
+ void Stop() override { dispatch_source_cancel(m_memory_pressure_source); }
+
+private:
+ dispatch_source_t m_memory_pressure_source;
+};
+#endif
+
+#if defined(__linux__)
+static void MonitorThread(std::atomic &done,
+ MemoryMonitor::Callback callback) {
+ struct pollfd fds;
+ fds.fd = open("/proc/pressure/memory", O_RDWR | O_NONBLOCK);
+ if (fds.fd < 0)
+return;
+ fds.events = POLLPRI;
+
+ auto cleanup = llvm::make_scope_exit([&]() { close(fds.fd); });
+
+ // Detect a 50ms stall in a 2 second time window.
+ const char trig[] = "some 5 200";
+ if (write(fds.fd, trig, strlen(trig) + 1) < 0)
+return;
+
+ while (!done) {
+int n = poll(&fds, 1, 1000);
+if (n > 0) {
+ if (fds.revents & POLLERR)
+return;
+ if (fds.revents & POLLPRI)
+callback();
+}
+ }
+}
+
+class MemoryMonitorLinux : public MemoryMonitor {
+public:
+ using MemoryMonitor::MemoryMonitor;
+
+ void Start() override {
+m_memory_pressure_thread =
+std::thread(MonitorThread, std::ref(m_done), m_callback);
+ }
+
+ void Stop() override {
+if (m_memory_pressure_thread.joinable()) {
+ m_done = true;
+ m_memory_pressure_thread.join();
+}
+ }
+
+private:
+ std::atomic m_done = false;
+ std::thread m_memory_pressure_thread;
+};
+#endif
+
+std::unique_ptr MemoryMonitor::Create(Callback callback) {
+#if defined(__APPLE__)
+ return std::make_unique(callback);
+#endif
+
+#if defined(__linux__)
+ return std::make_unique(callback);
+#endif
+
+ return nullptr;
+}
diff --git a/lldb/tools/lldb-dap/MemoryMonitor.h
b/lldb/tools/lldb-dap/MemoryMonitor.h
new file mode 100644
index 0..e07c3bde9e85c
--- /dev/null
+++ b/lldb/tools/lldb-dap/MemoryMonitor.h
@@ -0,0 +1,41 @@
+//===-- MemoryMonitor.h
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===-
[Lldb-commits] [lldb] [lldb-dap] Implement a MemoryMonitor for macOS & Linux (PR #129332)
JDevlieghere wrote: > This looks very nice. Just out of curiosity, is something printed on the > terminal/console whenever one of these low memory events happen? No, this should be transparent to the user. Removing the orphaned modules might not be enough to get out of this situation, so the callback might be called repeatedly. I've added a log line so we can see when this happens that way. https://github.com/llvm/llvm-project/pull/129332 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Implement a MemoryMonitor (PR #129332)
https://github.com/JDevlieghere edited https://github.com/llvm/llvm-project/pull/129332 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Return a std::optional from GetBoolean (NFC) (PR #129818)
https://github.com/JDevlieghere closed https://github.com/llvm/llvm-project/pull/129818 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] c7dbf20 - [lldb-dap] Test: disable children return test for all arm architectures (#129409)
Author: Da-Viper Date: 2025-03-04T15:46:26+05:00 New Revision: c7dbf20e66606e7e26a28ad567ff75f3a493d3bd URL: https://github.com/llvm/llvm-project/commit/c7dbf20e66606e7e26a28ad567ff75f3a493d3bd DIFF: https://github.com/llvm/llvm-project/commit/c7dbf20e66606e7e26a28ad567ff75f3a493d3bd.diff LOG: [lldb-dap] Test: disable children return test for all arm architectures (#129409) amd64 and aarch64 are treated differently Follows up #106907 Added: Modified: lldb/test/API/tools/lldb-dap/variables/children/TestDAP_variables_children.py Removed: diff --git a/lldb/test/API/tools/lldb-dap/variables/children/TestDAP_variables_children.py b/lldb/test/API/tools/lldb-dap/variables/children/TestDAP_variables_children.py index f0e1e51365d9b..a9371e5c5fe68 100644 --- a/lldb/test/API/tools/lldb-dap/variables/children/TestDAP_variables_children.py +++ b/lldb/test/API/tools/lldb-dap/variables/children/TestDAP_variables_children.py @@ -41,7 +41,7 @@ def test_get_num_children(self): )["body"]["result"], ) -@skipIf(archs=["arm64"]) +@skipIf(archs=["arm", "arm64", "aarch64"]) def test_return_variable_with_children(self): """ Test the stepping out of a function with return value show the children correctly ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 77a8770 - Reland "[lldb][HostInfoMacOSX] Try to use DW_AT_LLVM_sysroot instead of xcrun when looking up SDK" (#129621)"
Author: Michael Buch
Date: 2025-03-04T10:24:08Z
New Revision: 77a8770d4976e086f36004a6b8bf09e76d981451
URL:
https://github.com/llvm/llvm-project/commit/77a8770d4976e086f36004a6b8bf09e76d981451
DIFF:
https://github.com/llvm/llvm-project/commit/77a8770d4976e086f36004a6b8bf09e76d981451.diff
LOG: Reland "[lldb][HostInfoMacOSX] Try to use DW_AT_LLVM_sysroot instead of
xcrun when looking up SDK" (#129621)"
This reverts commit 6041c745f32e8fd60ed24e29e7d919d8d1c87ca6.
Relands the original patch with the test-case data fixed. Weirldy the PR CI
didn't seem to run the unit-tests? In any case, the problem was an
incorrect expectation in the test-case data. Since we have both public
and internal SDK in that test-case, we should `expect_mismatch` to be
`true`.
Added:
Modified:
lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
lldb/include/lldb/Utility/XcodeSDK.h
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Utility/XcodeSDK.cpp
lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
lldb/unittests/Utility/XcodeSDKTest.cpp
Removed:
diff --git a/lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
b/lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
index 8eb2ede382c22..9034c80fdefa4 100644
--- a/lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
+++ b/lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
@@ -32,6 +32,9 @@ class HostInfoMacOSX : public HostInfoPosix {
static FileSpec GetXcodeDeveloperDirectory();
/// Query xcrun to find an Xcode SDK directory.
+ ///
+ /// Note, this is an expensive operation if the SDK we're querying
+ /// does not exist in an Xcode installation path on the host.
static llvm::Expected GetSDKRoot(SDKOptions options);
static llvm::Expected FindSDKTool(XcodeSDK sdk,
llvm::StringRef tool);
diff --git a/lldb/include/lldb/Utility/XcodeSDK.h
b/lldb/include/lldb/Utility/XcodeSDK.h
index 2720d0d8a44a1..3a6cbb9c98f6b 100644
--- a/lldb/include/lldb/Utility/XcodeSDK.h
+++ b/lldb/include/lldb/Utility/XcodeSDK.h
@@ -9,6 +9,7 @@
#ifndef LLDB_UTILITY_SDK_H
#define LLDB_UTILITY_SDK_H
+#include "lldb/Utility/FileSpec.h"
#include "lldb/lldb-forward.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/VersionTuple.h"
@@ -23,6 +24,7 @@ namespace lldb_private {
/// An abstraction for Xcode-style SDKs that works like \ref ArchSpec.
class XcodeSDK {
std::string m_name;
+ FileSpec m_sysroot;
public:
/// Different types of Xcode SDKs.
@@ -62,6 +64,8 @@ class XcodeSDK {
/// directory component of a path one would pass to clang's -isysroot
/// parameter. For example, "MacOSX.10.14.sdk".
XcodeSDK(std::string &&name) : m_name(std::move(name)) {}
+ XcodeSDK(std::string name, FileSpec sysroot)
+ : m_name(std::move(name)), m_sysroot(std::move(sysroot)) {}
static XcodeSDK GetAnyMacOS() { return XcodeSDK("MacOSX.sdk"); }
/// The merge function follows a strict order to maintain monotonicity:
@@ -79,6 +83,7 @@ class XcodeSDK {
llvm::VersionTuple GetVersion() const;
Type GetType() const;
llvm::StringRef GetString() const;
+ const FileSpec &GetSysroot() const;
/// Whether this Xcode SDK supports Swift.
bool SupportsSwift() const;
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
index 665500f23e95d..ee8e256748cea 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1425,6 +1425,9 @@ PlatformDarwin::ResolveSDKPathFromDebugInfo(Module
&module) {
auto [sdk, _] = std::move(*sdk_or_err);
+ if (FileSystem::Instance().Exists(sdk.GetSysroot()))
+return sdk.GetSysroot().GetPath();
+
auto path_or_err = HostInfo::GetSDKRoot(HostInfo::SDKOptions{sdk});
if (!path_or_err)
return llvm::createStringError(
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 58b544a9a137b..d562de84db94f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -993,21 +993,26 @@ XcodeSDK SymbolFileDWARF::ParseXcodeSDK(CompileUnit
&comp_unit) {
const char *sdk = cu_die.GetAttributeValueAsString(DW_AT_APPLE_sdk, nullptr);
if (!sdk)
return {};
- const char *sysroot =
+ std::string sysroot =
cu_die.GetAttributeValueAsString(DW_AT_LLVM_sysroot, "");
- // Register the sysroot path remapping with the module belonging to
- // the CU as well as the one belonging to the symbol file. The two
- // would be
diff erent if this is an OSO object and module is the
- // corresponding debug map, in which case both should be updated.
- ModuleSP module_sp = comp_unit.GetModule();
- if (mod
[Lldb-commits] [lldb] [LLDB][Telemetry]Defind telemetry::CommandInfo (PR #129354)
https://github.com/oontvoo updated
https://github.com/llvm/llvm-project/pull/129354
>From 5a992aff351a93ff820d15ace3ebc2bea59dd5fc Mon Sep 17 00:00:00 2001
From: Vy Nguyen
Date: Fri, 28 Feb 2025 23:03:35 -0500
Subject: [PATCH 01/15] [LLDB][Telemetry]Defind telemetry::CommandInfo and
collect telemetry about a command's execution.
---
lldb/include/lldb/Core/Telemetry.h| 127 +-
lldb/source/Core/Telemetry.cpp| 14 ++
.../source/Interpreter/CommandInterpreter.cpp | 20 +++
3 files changed, 158 insertions(+), 3 deletions(-)
diff --git a/lldb/include/lldb/Core/Telemetry.h
b/lldb/include/lldb/Core/Telemetry.h
index b72556ecaf3c9..30b8474156124 100644
--- a/lldb/include/lldb/Core/Telemetry.h
+++ b/lldb/include/lldb/Core/Telemetry.h
@@ -12,11 +12,14 @@
#include "lldb/Core/StructuredDataImpl.h"
#include "lldb/Interpreter/CommandReturnObject.h"
#include "lldb/Utility/StructuredData.h"
+#include "lldb/Utility/LLDBLog.h"
#include "lldb/lldb-forward.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/FunctionExtras.h"
#include "llvm/Support/JSON.h"
#include "llvm/Telemetry/Telemetry.h"
+#include
#include
#include
#include
@@ -27,8 +30,16 @@
namespace lldb_private {
namespace telemetry {
+struct LLDBConfig : public ::llvm::telemetry::Config {
+ const bool m_collect_original_command;
+
+ explicit LLDBConfig(bool enable_telemetry, bool collect_original_command)
+ : ::llvm::telemetry::Config(enable_telemetry),
m_collect_original_command(collect_original_command) {}
+};
+
struct LLDBEntryKind : public ::llvm::telemetry::EntryKind {
- static const llvm::telemetry::KindType BaseInfo = 0b11000;
+ static const llvm::telemetry::KindType BaseInfo = 0b1100;
+ static const llvm::telemetry::KindType CommandInfo = 0b1101;
};
/// Defines a convenient type for timestamp of various events.
@@ -41,6 +52,7 @@ struct LLDBBaseTelemetryInfo : public
llvm::telemetry::TelemetryInfo {
std::optional end_time;
// TBD: could add some memory stats here too?
+ lldb::user_id_t debugger_id = LLDB_INVALID_UID;
Debugger *debugger;
// For dyn_cast, isa, etc operations.
@@ -56,6 +68,42 @@ struct LLDBBaseTelemetryInfo : public
llvm::telemetry::TelemetryInfo {
void serialize(llvm::telemetry::Serializer &serializer) const override;
};
+
+struct CommandInfo : public LLDBBaseTelemetryInfo {
+
+ // If the command is/can be associated with a target entry this field
contains
+ // that target's UUID. otherwise.
+ std::string target_uuid;
+ // A unique ID for a command so the manager can match the start entry with
+ // its end entry. These values only need to be unique within the same
session.
+ // Necessary because we'd send off an entry right before a command's
execution
+ // and another right after. This is to avoid losing telemetry if the command
+ // does not execute successfully.
+ int command_id;
+
+ // Eg., "breakpoint set"
+ std::string command_name;
+
+ // !!NOTE!! These two fields are not collected (upstream) due to PII risks.
+ // (Downstream impl may add them if needed).
+ // std::string original_command;
+ // std::string args;
+
+ lldb::ReturnStatus ret_status;
+ std::string error_data;
+
+
+ CommandInfo() = default;
+
+ llvm::telemetry::KindType getKind() const override { return
LLDBEntryKind::CommandInfo; }
+
+ static bool classof(const llvm::telemetry::TelemetryInfo *T) {
+return (T->getKind() & LLDBEntryKind::CommandInfo) ==
LLDBEntryKind::CommandInfo;
+ }
+
+ void serialize(Serializer &serializer) const override;
+};
+
/// The base Telemetry manager instance in LLDB.
/// This class declares additional instrumentation points
/// applicable to LLDB.
@@ -63,19 +111,92 @@ class TelemetryManager : public llvm::telemetry::Manager {
public:
llvm::Error preDispatch(llvm::telemetry::TelemetryInfo *entry) override;
+ int MakeNextCommandId();
+
+ LLDBConfig* GetConfig() { return m_config.get(); }
+
virtual llvm::StringRef GetInstanceName() const = 0;
static TelemetryManager *getInstance();
protected:
- TelemetryManager(std::unique_ptr config);
+ TelemetryManager(std::unique_ptr config);
static void setInstance(std::unique_ptr manger);
private:
- std::unique_ptr m_config;
+ std::unique_ptr m_config;
+ const std::string m_id;
+ // We assign each command (in the same session) a unique id so that their
+ // "start" and "end" entries can be matched up.
+ // These values don't need to be unique across runs (because they are
+ // secondary-key), hence a simple counter is sufficent.
+ std::atomic command_id_seed = 0;
static std::unique_ptr g_instance;
};
+/// Helper RAII class for collecting telemetry.
+template struct ScopedDispatcher {
+ // The debugger pointer is optional because we may not have a debugger yet.
+ // In that case, caller must set the debugger later.
+ ScopedDispatcher(Debugger *debugger = nullptr) {
+// Sta
[Lldb-commits] [lldb] [lldb] Avoid force loading symbol files in statistics collection (PR #129593)
https://github.com/dmpots updated
https://github.com/llvm/llvm-project/pull/129593
>From bca07d83152df179f7784d0003262fa54834 Mon Sep 17 00:00:00 2001
From: David Peixotto
Date: Wed, 26 Feb 2025 13:55:35 -0800
Subject: [PATCH 1/3] Avoid force loading symbol files in statistics collection
This commit modifies the `DebuggerStats::ReportStatistics` implementation
to avoid loading symbol files for unloaded symbols. We collect stats
on debugger shutdown and without this change it can cause the debugger
to hang for a long while on shutdown if they symbols were not previously
loaded (e.g. `settings set target.preload-symbols false`).
The implementation is done by adding an optional parameter to
`Module::GetSymtab` to control if the corresponding symbol file will
be loaded in the same way that can control it for `Module::GetSymbolFile`.
---
lldb/include/lldb/Core/Module.h | 6 +++-
lldb/source/Core/Module.cpp | 4 +--
lldb/source/Target/Statistics.cpp | 4 +--
.../Commands/command-statistics-dump.test | 31 +++
4 files changed, 40 insertions(+), 5 deletions(-)
create mode 100644 lldb/test/Shell/Commands/command-statistics-dump.test
diff --git a/lldb/include/lldb/Core/Module.h b/lldb/include/lldb/Core/Module.h
index 90e0f4b6a3aac..9aa05ed3eb074 100644
--- a/lldb/include/lldb/Core/Module.h
+++ b/lldb/include/lldb/Core/Module.h
@@ -608,7 +608,11 @@ class Module : public std::enable_shared_from_this,
virtual SymbolFile *GetSymbolFile(bool can_create = true,
Stream *feedback_strm = nullptr);
- Symtab *GetSymtab();
+ /// Get the module's symbol table
+ ///
+ /// If the symbol table has already been loaded, this function returns it.
+ /// Otherwise, it will only be loaded when can_create is true.
+ Symtab *GetSymtab(bool can_create = true);
/// Get a reference to the UUID value contained in this object.
///
diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index 33668c5d20dde..d70f292abaea4 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -1022,8 +1022,8 @@ SymbolFile *Module::GetSymbolFile(bool can_create, Stream
*feedback_strm) {
return m_symfile_up ? m_symfile_up->GetSymbolFile() : nullptr;
}
-Symtab *Module::GetSymtab() {
- if (SymbolFile *symbols = GetSymbolFile())
+Symtab *Module::GetSymtab(bool can_create) {
+ if (SymbolFile *symbols = GetSymbolFile(can_create))
return symbols->GetSymtab();
return nullptr;
}
diff --git a/lldb/source/Target/Statistics.cpp
b/lldb/source/Target/Statistics.cpp
index 8173d20457e21..b5d2e7bda1edf 100644
--- a/lldb/source/Target/Statistics.cpp
+++ b/lldb/source/Target/Statistics.cpp
@@ -316,7 +316,7 @@ llvm::json::Value DebuggerStats::ReportStatistics(
ModuleStats module_stat;
module_stat.symtab_parse_time = module->GetSymtabParseTime().get().count();
module_stat.symtab_index_time = module->GetSymtabIndexTime().get().count();
-Symtab *symtab = module->GetSymtab();
+Symtab *symtab = module->GetSymtab(/*can_create=*/false);
if (symtab) {
module_stat.symtab_loaded_from_cache = symtab->GetWasLoadedFromCache();
if (module_stat.symtab_loaded_from_cache)
@@ -325,7 +325,7 @@ llvm::json::Value DebuggerStats::ReportStatistics(
if (module_stat.symtab_saved_to_cache)
++symtabs_saved;
}
-SymbolFile *sym_file = module->GetSymbolFile();
+SymbolFile *sym_file = module->GetSymbolFile(/*can_create=*/false);
if (sym_file) {
if (!summary_only) {
if (sym_file->GetObjectFile() != module->GetObjectFile())
diff --git a/lldb/test/Shell/Commands/command-statistics-dump.test
b/lldb/test/Shell/Commands/command-statistics-dump.test
new file mode 100644
index 0..d490a0c374057
--- /dev/null
+++ b/lldb/test/Shell/Commands/command-statistics-dump.test
@@ -0,0 +1,31 @@
+# RUN: %clang_host -g %S/Inputs/main.c -o %t-main.exe
+
+# When we enable symbol preload and dump stats there should be a non-zero
+# time for parsing symbol tables for the main module.
+# RUN: %lldb %t-main.exe \
+# RUN: -O "settings set plugin.jit-loader.gdb.enable off" \
+# RUN: -O "settings set target.preload-symbols true" \
+# RUN: -o "statistics dump" \
+# RUN: -o "q" \
+# RUN: | FileCheck %s -check-prefixes=CHECK,PRELOAD_TRUE
+
+# Find the module stats for the main executable and make sure
+# we are looking at the symbol parse time for that module.
+# CHECK: "modules": [
+# CHECK: {
+# CHECK: "path": {{.*}}-main.exe
+# CHECK-NOT: }
+
+# PRELOAD_TRUE: "symbolTableParseTime":
+# PRELOAD_TRUE-SAME: {{[1-9]+}}
+
+# When we disable symbol preload and dump stats the symbol table
+# for main should not be parsed and have a time of 0.
+# RUN: %lldb %t-main.exe \
+# RUN: -O "settings set plugin.jit-loader.gdb.enable off" \
+# RUN: -O "settings set target.preload-symbols false" \
+# RUN: -o "statistics dump
[Lldb-commits] [lldb] [lldb] Support expectedFlakey decorator in dotest (PR #129817)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: David Peixotto (dmpots)
Changes
The dotest framework had an existing decorator to mark flakey tests. It was not
implemented and would simply run the underlying test. This commit modifies the
decorator to run the test multiple times in the case of failure and only raises
the test failure when all attempts fail.
---
Full diff: https://github.com/llvm/llvm-project/pull/129817.diff
1 Files Affected:
- (modified) lldb/packages/Python/lldbsuite/test/decorators.py (+16-3)
``diff
diff --git a/lldb/packages/Python/lldbsuite/test/decorators.py
b/lldb/packages/Python/lldbsuite/test/decorators.py
index c48c0b2f77944..c4de3f8f10751 100644
--- a/lldb/packages/Python/lldbsuite/test/decorators.py
+++ b/lldb/packages/Python/lldbsuite/test/decorators.py
@@ -3,6 +3,7 @@
from packaging import version
import ctypes
import locale
+import logging
import os
import platform
import re
@@ -525,12 +526,24 @@ def expectedFailureWindows(bugnumber=None):
return expectedFailureOS(["windows"], bugnumber)
-# TODO: This decorator does not do anything. Remove it.
-def expectedFlakey(expected_fn, bugnumber=None):
+# This decorator can be used to mark a test that can fail
non-deterministically.
+# The decorator causes the underlying test to be re-run if it encounters a
+# failure. After `num_retries` attempts the test will be marked as a failure
+# if it has not yet passed.
+def expectedFlakey(expected_fn, bugnumber=None, num_retries=3):
def expectedFailure_impl(func):
@wraps(func)
def wrapper(*args, **kwargs):
-func(*args, **kwargs)
+for i in range(1, num_retries+1):
+try:
+return func(*args, **kwargs)
+except Exception:
+logging.warning(
+f"expectedFlakey: test {func} failed attempt
({i}/{num_retries})"
+)
+# If the last attempt fails then re-raise the original
failure.
+if i == num_retries:
+raise
return wrapper
``
https://github.com/llvm/llvm-project/pull/129817
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Support expectedFlakey decorator in dotest (PR #129817)
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
11b9466c04db4da7439fc1d9d8ba7241a9d68705...1d1c3a6bf52ba47a4ff4c13e99209a6ae3d983b3
lldb/packages/Python/lldbsuite/test/decorators.py
``
View the diff from darker here.
``diff
--- decorators.py 2025-03-05 02:34:40.00 +
+++ decorators.py 2025-03-05 02:42:34.414204 +
@@ -532,11 +532,11 @@
# if it has not yet passed.
def expectedFlakey(expected_fn, bugnumber=None, num_retries=3):
def expectedFailure_impl(func):
@wraps(func)
def wrapper(*args, **kwargs):
-for i in range(1, num_retries+1):
+for i in range(1, num_retries + 1):
try:
return func(*args, **kwargs)
except Exception:
logging.warning(
f"expectedFlakey: test {func} failed attempt
({i}/{num_retries})"
``
https://github.com/llvm/llvm-project/pull/129817
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Return a std::optional from GetBoolean (NFC) (PR #129818)
https://github.com/JDevlieghere created
https://github.com/llvm/llvm-project/pull/129818
Return a std::optional from GetBoolean so you can distinguish between the
value not being present and it being explicitly set to true or false. All
existing uses are replaced by calling `value_or(fail_value`).
>From eea8a23427e576b762e0c628ce8a5b6fe80863ba Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere
Date: Tue, 4 Mar 2025 20:14:51 -0800
Subject: [PATCH] [lldb-dap] Return a std::optional from GetBoolean (NFC)
Return a std::optional from GetBoolean so you can distinguish
between the value not being present and it being explicitly set to true
or false. All existing uses are replaced by calling
`value_or(fail_value`).
---
lldb/tools/lldb-dap/DAP.cpp | 2 +-
.../lldb-dap/Handler/AttachRequestHandler.cpp| 13 +++--
.../Handler/DisassembleRequestHandler.cpp| 3 ++-
.../Handler/DisconnectRequestHandler.cpp | 4 ++--
.../Handler/InitializeRequestHandler.cpp | 5 +++--
.../lldb-dap/Handler/LaunchRequestHandler.cpp| 8
lldb/tools/lldb-dap/Handler/RequestHandler.cpp | 11 ++-
.../lldb-dap/Handler/StepInRequestHandler.cpp| 3 ++-
.../lldb-dap/Handler/VariablesRequestHandler.cpp | 2 +-
lldb/tools/lldb-dap/JSONUtils.cpp| 16
lldb/tools/lldb-dap/JSONUtils.h | 12 +++-
11 files changed, 43 insertions(+), 36 deletions(-)
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 3dc9d6f5ca0a4..c4790414f64f9 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -787,7 +787,7 @@ bool DAP::HandleObject(const llvm::json::Object &object) {
response_handler = std::make_unique("", id);
// Result should be given, use null if not.
-if (GetBoolean(object, "success", false)) {
+if (GetBoolean(object, "success").value_or(false)) {
llvm::json::Value Result = nullptr;
if (auto *B = object.get("body")) {
Result = std::move(*B);
diff --git a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp
b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp
index 8b203e0392066..2733f58b74683 100644
--- a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp
@@ -61,7 +61,7 @@ void AttachRequestHandler::operator()(const
llvm::json::Object &request) const {
GetString(arguments, "gdb-remote-hostname", "localhost");
if (pid != LLDB_INVALID_PROCESS_ID)
attach_info.SetProcessID(pid);
- const auto wait_for = GetBoolean(arguments, "waitFor", false);
+ const auto wait_for = GetBoolean(arguments, "waitFor").value_or(false);
attach_info.SetWaitForLaunch(wait_for, false /*async*/);
dap.init_commands = GetStrings(arguments, "initCommands");
dap.pre_run_commands = GetStrings(arguments, "preRunCommands");
@@ -71,16 +71,17 @@ void AttachRequestHandler::operator()(const
llvm::json::Object &request) const {
auto attachCommands = GetStrings(arguments, "attachCommands");
llvm::StringRef core_file = GetString(arguments, "coreFile");
const uint64_t timeout_seconds = GetUnsigned(arguments, "timeout", 30);
- dap.stop_at_entry =
- core_file.empty() ? GetBoolean(arguments, "stopOnEntry", false) : true;
+ dap.stop_at_entry = core_file.empty()
+ ? GetBoolean(arguments,
"stopOnEntry").value_or(false)
+ : true;
dap.post_run_commands = GetStrings(arguments, "postRunCommands");
const llvm::StringRef debuggerRoot = GetString(arguments, "debuggerRoot");
dap.enable_auto_variable_summaries =
- GetBoolean(arguments, "enableAutoVariableSummaries", false);
+ GetBoolean(arguments, "enableAutoVariableSummaries").value_or(false);
dap.enable_synthetic_child_debugging =
- GetBoolean(arguments, "enableSyntheticChildDebugging", false);
+ GetBoolean(arguments, "enableSyntheticChildDebugging").value_or(false);
dap.display_extended_backtrace =
- GetBoolean(arguments, "displayExtendedBacktrace", false);
+ GetBoolean(arguments, "displayExtendedBacktrace").value_or(false);
dap.command_escape_prefix = GetString(arguments, "commandEscapePrefix", "`");
dap.SetFrameFormat(GetString(arguments, "customFrameFormat"));
dap.SetThreadFormat(GetString(arguments, "customThreadFormat"));
diff --git a/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp
b/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp
index 4c2690d32d3b2..d6ff2a7fd8880 100644
--- a/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp
@@ -123,7 +123,8 @@ void DisassembleRequestHandler::operator()(
return;
}
- const bool resolveSymbols = GetBoolean(arguments, "resolveSymbols", false);
+ const bool resolveSymbols =
+ GetBoolean(arguments, "resolveSymbols").value_or(false);
llvm::json::Array instructions;
co
[Lldb-commits] [lldb] [lldb-dap] Return a std::optional from GetBoolean (NFC) (PR #129818)
JDevlieghere wrote: CC @Da-Viper (I can't add you as a reviewer because you're not part of the LLVM organization) https://github.com/llvm/llvm-project/pull/129818 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Avoid force loading symbol files in statistics collection (PR #129593)
@@ -0,0 +1,31 @@ +# RUN: %clang_host -g %S/Inputs/main.c -o %t-main.exe + +# When we enable symbol preload and dump stats there should be a non-zero +# time for parsing symbol tables for the main module. +# RUN: %lldb %t-main.exe \ +# RUN: -O "settings set plugin.jit-loader.gdb.enable off" \ dmpots wrote: Thanks for the feedback. I updated the test to create the target as suggested. Originally I had the test include a '-o "run"' flag as well, but that ran into symbol loading issues as you pointed out. The ASAN/TSAN runtime plugins are loaded automatically and there is no way to disable them. Those plugins force symbol loading by looking for some specific symbols in all executable modules. Is it a reasonable to expect that we should have a way to turn off new plugins that cause the the test to break if they force symbol loading? Or is there a better way to test this feature? https://github.com/llvm/llvm-project/pull/129593 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Support expectedFlakey decorator in dotest (PR #129817)
https://github.com/dmpots updated
https://github.com/llvm/llvm-project/pull/129817
>From 1d1c3a6bf52ba47a4ff4c13e99209a6ae3d983b3 Mon Sep 17 00:00:00 2001
From: David Peixotto
Date: Tue, 4 Mar 2025 11:39:03 -0800
Subject: [PATCH 1/2] [lldb] Support expectedFlakey decorator in dotest
The dotest framework had an existing decorator to mark flakey tests. It
was not implemented and would simply run the underlying test. This
commit modifies the decorator to run the test multiple times in the case
of failure and only raises the test failure when all attempts fail.
---
.../Python/lldbsuite/test/decorators.py | 19 ---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/lldb/packages/Python/lldbsuite/test/decorators.py
b/lldb/packages/Python/lldbsuite/test/decorators.py
index c48c0b2f77944..c4de3f8f10751 100644
--- a/lldb/packages/Python/lldbsuite/test/decorators.py
+++ b/lldb/packages/Python/lldbsuite/test/decorators.py
@@ -3,6 +3,7 @@
from packaging import version
import ctypes
import locale
+import logging
import os
import platform
import re
@@ -525,12 +526,24 @@ def expectedFailureWindows(bugnumber=None):
return expectedFailureOS(["windows"], bugnumber)
-# TODO: This decorator does not do anything. Remove it.
-def expectedFlakey(expected_fn, bugnumber=None):
+# This decorator can be used to mark a test that can fail
non-deterministically.
+# The decorator causes the underlying test to be re-run if it encounters a
+# failure. After `num_retries` attempts the test will be marked as a failure
+# if it has not yet passed.
+def expectedFlakey(expected_fn, bugnumber=None, num_retries=3):
def expectedFailure_impl(func):
@wraps(func)
def wrapper(*args, **kwargs):
-func(*args, **kwargs)
+for i in range(1, num_retries+1):
+try:
+return func(*args, **kwargs)
+except Exception:
+logging.warning(
+f"expectedFlakey: test {func} failed attempt
({i}/{num_retries})"
+)
+# If the last attempt fails then re-raise the original
failure.
+if i == num_retries:
+raise
return wrapper
>From 909f3055c5a684ecd9572d71c654f5f19eda384b Mon Sep 17 00:00:00 2001
From: David Peixotto
Date: Tue, 4 Mar 2025 18:48:36 -0800
Subject: [PATCH 2/2] Formatting
---
lldb/packages/Python/lldbsuite/test/decorators.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/packages/Python/lldbsuite/test/decorators.py
b/lldb/packages/Python/lldbsuite/test/decorators.py
index c4de3f8f10751..71c9420ad07ac 100644
--- a/lldb/packages/Python/lldbsuite/test/decorators.py
+++ b/lldb/packages/Python/lldbsuite/test/decorators.py
@@ -534,7 +534,7 @@ def expectedFlakey(expected_fn, bugnumber=None,
num_retries=3):
def expectedFailure_impl(func):
@wraps(func)
def wrapper(*args, **kwargs):
-for i in range(1, num_retries+1):
+for i in range(1, num_retries + 1):
try:
return func(*args, **kwargs)
except Exception:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Support expectedFlakey decorator in dotest (PR #129817)
https://github.com/dmpots created
https://github.com/llvm/llvm-project/pull/129817
The dotest framework had an existing decorator to mark flakey tests. It was not
implemented and would simply run the underlying test. This commit modifies the
decorator to run the test multiple times in the case of failure and only raises
the test failure when all attempts fail.
>From 1d1c3a6bf52ba47a4ff4c13e99209a6ae3d983b3 Mon Sep 17 00:00:00 2001
From: David Peixotto
Date: Tue, 4 Mar 2025 11:39:03 -0800
Subject: [PATCH] [lldb] Support expectedFlakey decorator in dotest
The dotest framework had an existing decorator to mark flakey tests. It
was not implemented and would simply run the underlying test. This
commit modifies the decorator to run the test multiple times in the case
of failure and only raises the test failure when all attempts fail.
---
.../Python/lldbsuite/test/decorators.py | 19 ---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/lldb/packages/Python/lldbsuite/test/decorators.py
b/lldb/packages/Python/lldbsuite/test/decorators.py
index c48c0b2f77944..c4de3f8f10751 100644
--- a/lldb/packages/Python/lldbsuite/test/decorators.py
+++ b/lldb/packages/Python/lldbsuite/test/decorators.py
@@ -3,6 +3,7 @@
from packaging import version
import ctypes
import locale
+import logging
import os
import platform
import re
@@ -525,12 +526,24 @@ def expectedFailureWindows(bugnumber=None):
return expectedFailureOS(["windows"], bugnumber)
-# TODO: This decorator does not do anything. Remove it.
-def expectedFlakey(expected_fn, bugnumber=None):
+# This decorator can be used to mark a test that can fail
non-deterministically.
+# The decorator causes the underlying test to be re-run if it encounters a
+# failure. After `num_retries` attempts the test will be marked as a failure
+# if it has not yet passed.
+def expectedFlakey(expected_fn, bugnumber=None, num_retries=3):
def expectedFailure_impl(func):
@wraps(func)
def wrapper(*args, **kwargs):
-func(*args, **kwargs)
+for i in range(1, num_retries+1):
+try:
+return func(*args, **kwargs)
+except Exception:
+logging.warning(
+f"expectedFlakey: test {func} failed attempt
({i}/{num_retries})"
+)
+# If the last attempt fails then re-raise the original
failure.
+if i == num_retries:
+raise
return wrapper
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Return a std::optional from GetBoolean (NFC) (PR #129818)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) Changes Return a std::optionalfrom GetBoolean so you can distinguish between the value not being present and it being explicitly set to true or false. All existing uses are replaced by calling `value_or(fail_value`). --- Full diff: https://github.com/llvm/llvm-project/pull/129818.diff 11 Files Affected: - (modified) lldb/tools/lldb-dap/DAP.cpp (+1-1) - (modified) lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp (+7-6) - (modified) lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp (+2-1) - (modified) lldb/tools/lldb-dap/Handler/DisconnectRequestHandler.cpp (+2-2) - (modified) lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp (+3-2) - (modified) lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp (+4-4) - (modified) lldb/tools/lldb-dap/Handler/RequestHandler.cpp (+6-5) - (modified) lldb/tools/lldb-dap/Handler/StepInRequestHandler.cpp (+2-1) - (modified) lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp (+1-1) - (modified) lldb/tools/lldb-dap/JSONUtils.cpp (+8-8) - (modified) lldb/tools/lldb-dap/JSONUtils.h (+7-5) ``diff diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index 3dc9d6f5ca0a4..c4790414f64f9 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -787,7 +787,7 @@ bool DAP::HandleObject(const llvm::json::Object &object) { response_handler = std::make_unique("", id); // Result should be given, use null if not. -if (GetBoolean(object, "success", false)) { +if (GetBoolean(object, "success").value_or(false)) { llvm::json::Value Result = nullptr; if (auto *B = object.get("body")) { Result = std::move(*B); diff --git a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp index 8b203e0392066..2733f58b74683 100644 --- a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp @@ -61,7 +61,7 @@ void AttachRequestHandler::operator()(const llvm::json::Object &request) const { GetString(arguments, "gdb-remote-hostname", "localhost"); if (pid != LLDB_INVALID_PROCESS_ID) attach_info.SetProcessID(pid); - const auto wait_for = GetBoolean(arguments, "waitFor", false); + const auto wait_for = GetBoolean(arguments, "waitFor").value_or(false); attach_info.SetWaitForLaunch(wait_for, false /*async*/); dap.init_commands = GetStrings(arguments, "initCommands"); dap.pre_run_commands = GetStrings(arguments, "preRunCommands"); @@ -71,16 +71,17 @@ void AttachRequestHandler::operator()(const llvm::json::Object &request) const { auto attachCommands = GetStrings(arguments, "attachCommands"); llvm::StringRef core_file = GetString(arguments, "coreFile"); const uint64_t timeout_seconds = GetUnsigned(arguments, "timeout", 30); - dap.stop_at_entry = - core_file.empty() ? GetBoolean(arguments, "stopOnEntry", false) : true; + dap.stop_at_entry = core_file.empty() + ? GetBoolean(arguments, "stopOnEntry").value_or(false) + : true; dap.post_run_commands = GetStrings(arguments, "postRunCommands"); const llvm::StringRef debuggerRoot = GetString(arguments, "debuggerRoot"); dap.enable_auto_variable_summaries = - GetBoolean(arguments, "enableAutoVariableSummaries", false); + GetBoolean(arguments, "enableAutoVariableSummaries").value_or(false); dap.enable_synthetic_child_debugging = - GetBoolean(arguments, "enableSyntheticChildDebugging", false); + GetBoolean(arguments, "enableSyntheticChildDebugging").value_or(false); dap.display_extended_backtrace = - GetBoolean(arguments, "displayExtendedBacktrace", false); + GetBoolean(arguments, "displayExtendedBacktrace").value_or(false); dap.command_escape_prefix = GetString(arguments, "commandEscapePrefix", "`"); dap.SetFrameFormat(GetString(arguments, "customFrameFormat")); dap.SetThreadFormat(GetString(arguments, "customThreadFormat")); diff --git a/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp index 4c2690d32d3b2..d6ff2a7fd8880 100644 --- a/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp @@ -123,7 +123,8 @@ void DisassembleRequestHandler::operator()( return; } - const bool resolveSymbols = GetBoolean(arguments, "resolveSymbols", false); + const bool resolveSymbols = + GetBoolean(arguments, "resolveSymbols").value_or(false); llvm::json::Array instructions; const auto num_insts = insts.GetSize(); for (size_t i = 0; i < num_insts; ++i) { diff --git a/lldb/tools/lldb-dap/Handler/DisconnectRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/DisconnectRequestHandler.cpp index 1d6ff0d039405..b8f3404874e91 100644 --- a/lldb/tools/lldb-dap/Handler/Dis
[Lldb-commits] [lldb] [lldb-dap] Return a std::optional from GetBoolean (NFC) (PR #129818)
https://github.com/JDevlieghere edited https://github.com/llvm/llvm-project/pull/129818 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
