[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)
@@ -5028,36 +5218,52 @@ int main(int argc, char *argv[]) {
#endif
// Initialize LLDB first before we do anything.
- lldb::SBDebugger::Initialize();
+ lldb::SBError error = lldb::SBDebugger::InitializeWithErrorHandling();
+ if (error.Fail()) {
+llvm::errs() << "Failed to initialize LLDB: " << error.GetCString() <<
"\n";
+return EXIT_FAILURE;
+ }
// Terminate the debugger before the C++ destructor chain kicks in.
auto terminate_debugger =
llvm::make_scope_exit([] { lldb::SBDebugger::Terminate(); });
- DAP dap = DAP(program_path.str(), default_repl_mode);
+ if (portno != -1) {
+llvm::errs() << llvm::format("Listening on port %i...\n", portno);
+return AcceptConnection(program_path.str(), pre_init_commands, log,
+default_repl_mode, portno);
+ }
+
+ if (!unix_socket_path.empty()) {
+return AcceptConnection(program_path.str(), pre_init_commands, log,
+default_repl_mode, unix_socket_path);
+ }
+
+ DAP dap = DAP(program_path.str(), log, default_repl_mode, pre_init_commands);
ashgti wrote:
Done, made this a uniform callback for both stdin/stdout and connections.
https://github.com/llvm/llvm-project/pull/116392
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)
@@ -1196,6 +1202,62 @@ def terminate(self): self.process.wait() self.process = None +@classmethod +def launch( +cls, executable: str, /, connection=None, log_file=None, env=None +) -> tuple[subprocess.Popen, str]: +adaptor_env = os.environ.copy() +if env: +adaptor_env.update(env) + +if log_file: +adaptor_env["LLDBDAP_LOG"] = log_file + +if os.uname().sysname == "Darwin": +adaptor_env["NSUnbufferedIO"] = "YES" ashgti wrote: Removed, having a flush seems to fix fix this. https://github.com/llvm/llvm-project/pull/116392 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)
@@ -0,0 +1,67 @@
+"""
+Test lldb-dap server integration.
+"""
+
+import os
+import tempfile
+
+import dap_server
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+import lldbdap_testcase
+
+
+class TestDAP_server(lldbdap_testcase.DAPTestCaseBase):
+def do_test_server(self, connection):
+log_file_path = self.getBuildArtifact("dap.txt")
+server, connection = dap_server.DebugAdaptorServer.launch(
+self.lldbDAPExec, connection, log_file=log_file_path
+)
+
+def cleanup():
+server.terminate()
+server.wait()
+
+self.addTearDownHook(cleanup)
+
+self.build()
+program = self.getBuildArtifact("a.out")
+source = "main.c"
+breakpoint_line = line_number(source, "// breakpoint")
+
+# Initial connection over the port.
+self.create_debug_adaptor(launch=False, connection=connection)
+self.launch(
+program,
+disconnectAutomatically=False,
+)
+self.set_source_breakpoints(source, [breakpoint_line])
+self.continue_to_next_stop()
+self.continue_to_exit()
+output = self.get_stdout()
+self.assertEquals(output, "hello world!\r\n")
+self.dap_server.request_disconnect()
+
+# Second connection over the port.
+self.create_debug_adaptor(launch=False, connection=connection)
+self.launch(program)
+self.set_source_breakpoints(source, [breakpoint_line])
+self.continue_to_next_stop()
+self.continue_to_exit()
+output = self.get_stdout()
+self.assertEquals(output, "hello world!\r\n")
+
+def test_server_port(self):
+"""
+Test launching a binary with a lldb-dap in server mode on a specific
port.
+"""
+self.do_test_server(connection="tcp://localhost:0")
+
+def test_server_unix_socket(self):
ashgti wrote:
Done.
https://github.com/llvm/llvm-project/pull/116392
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)
@@ -0,0 +1,67 @@
+"""
+Test lldb-dap server integration.
+"""
+
+import os
+import tempfile
+
+import dap_server
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+import lldbdap_testcase
+
+
+class TestDAP_server(lldbdap_testcase.DAPTestCaseBase):
+def do_test_server(self, connection):
+log_file_path = self.getBuildArtifact("dap.txt")
+server, connection = dap_server.DebugAdaptorServer.launch(
+self.lldbDAPExec, connection, log_file=log_file_path
+)
+
+def cleanup():
+server.terminate()
+server.wait()
+
+self.addTearDownHook(cleanup)
+
+self.build()
+program = self.getBuildArtifact("a.out")
+source = "main.c"
+breakpoint_line = line_number(source, "// breakpoint")
+
+# Initial connection over the port.
+self.create_debug_adaptor(launch=False, connection=connection)
+self.launch(
+program,
+disconnectAutomatically=False,
+)
+self.set_source_breakpoints(source, [breakpoint_line])
+self.continue_to_next_stop()
+self.continue_to_exit()
+output = self.get_stdout()
+self.assertEquals(output, "hello world!\r\n")
+self.dap_server.request_disconnect()
+
+# Second connection over the port.
+self.create_debug_adaptor(launch=False, connection=connection)
+self.launch(program)
+self.set_source_breakpoints(source, [breakpoint_line])
+self.continue_to_next_stop()
+self.continue_to_exit()
+output = self.get_stdout()
+self.assertEquals(output, "hello world!\r\n")
+
+def test_server_port(self):
+"""
+Test launching a binary with a lldb-dap in server mode on a specific
port.
+"""
+self.do_test_server(connection="tcp://localhost:0")
+
+def test_server_unix_socket(self):
+"""
+Test launching a binary with a lldb-dap in server mode on a unix
socket.
+"""
+dir = tempfile.gettempdir()
ashgti wrote:
Done.
https://github.com/llvm/llvm-project/pull/116392
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)
@@ -1196,6 +1202,62 @@ def terminate(self):
self.process.wait()
self.process = None
+@classmethod
+def launch(
+cls, executable: str, /, connection=None, log_file=None, env=None
+) -> tuple[subprocess.Popen, str]:
+adaptor_env = os.environ.copy()
+if env:
+adaptor_env.update(env)
+
+if log_file:
+adaptor_env["LLDBDAP_LOG"] = log_file
+
+if os.uname().sysname == "Darwin":
+adaptor_env["NSUnbufferedIO"] = "YES"
+
+args = [executable]
+if connection:
+args.append("--connection")
+args.append(connection)
+
+proc = subprocess.Popen(
+args,
+stdin=subprocess.PIPE,
+stdout=subprocess.PIPE,
+stderr=sys.stdout,
+env=adaptor_env,
+)
+
+if connection:
+# If a conneciton is specified, lldb-dap will print the listening
+# address once the listener is made to stdout. The listener is
+# formatted like `tcp://host:port` or `unix:///path`.
+with selectors.DefaultSelector() as sel:
+print("Reading stdout for the listening connection")
+os.set_blocking(proc.stdout.fileno(), False)
+stdout_key = sel.register(proc.stdout, selectors.EVENT_READ)
+rdy_fds = sel.select(timeout=10.0)
ashgti wrote:
I refactored this to not use a select and instead adjusted the script to not
buffer when using a connection. This lets us read a single line at a time
without reaching EOF first.
https://github.com/llvm/llvm-project/pull/116392
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Fix crash in TypeSystemClang::GetIndexofChildMemberWithName. (PR #117808)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: None (cmtice)
Changes
LLDB can crash in TypeSystemClang::GetIndexOfChildMemberWithName, at a point
where it pushes an index onto the child_indexes vector, tries to call itself
recursively, then tries to pop the entry from child_indexes. The problem is
that the recursive call can clear child_indexes, so that this code ends up
trying to pop an already empty vector. This change saves the old vector before
the push, then restores the saved vector rather than trying to pop.
---
Full diff: https://github.com/llvm/llvm-project/pull/117808.diff
1 Files Affected:
- (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp (+2-2)
``diff
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 1a77c7cf9161a0..16eca7700d9fff 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -6754,12 +6754,12 @@ size_t TypeSystemClang::GetIndexOfChildMemberWithName(
llvm::StringRef field_name = field->getName();
if (field_name.empty()) {
CompilerType field_type = GetType(field->getType());
+std::vector save_indices = child_indexes;
child_indexes.push_back(child_idx);
if (field_type.GetIndexOfChildMemberWithName(
name, omit_empty_base_classes, child_indexes))
return child_indexes.size();
-child_indexes.pop_back();
-
+child_indexes = save_indices;
} else if (field_name == name) {
// We have to add on the number of base classes to this index!
child_indexes.push_back(
``
https://github.com/llvm/llvm-project/pull/117808
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Fix crash in TypeSystemClang::GetIndexofChildMemberWithName. (PR #117808)
https://github.com/cmtice created
https://github.com/llvm/llvm-project/pull/117808
LLDB can crash in TypeSystemClang::GetIndexOfChildMemberWithName, at a point
where it pushes an index onto the child_indexes vector, tries to call itself
recursively, then tries to pop the entry from child_indexes. The problem is
that the recursive call can clear child_indexes, so that this code ends up
trying to pop an already empty vector. This change saves the old vector before
the push, then restores the saved vector rather than trying to pop.
>From b8c64e227b8f9f82b420cc5c2f24fbd3f75f67f5 Mon Sep 17 00:00:00 2001
From: Caroline Tice
Date: Tue, 26 Nov 2024 15:08:32 -0800
Subject: [PATCH] [lLDB] Fix crash in
TypeSystemClang::GetIndexofChildMemberWithName.
LLDB can crash in TypeSystemClang::GetIndexOfChildMemberWithName, at a
point where it pushes an index onto the child_indexes vector, tries to call
itself recursively, then tries to pop the entry from child_indexes.
The problem is that the recursive call can clear child_indexes, so that
this code ends up trying to pop an already empty vector. This change saves
the old vector before the push, then restores the saved vector rather than
trying to pop.
---
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 1a77c7cf9161a0..16eca7700d9fff 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -6754,12 +6754,12 @@ size_t TypeSystemClang::GetIndexOfChildMemberWithName(
llvm::StringRef field_name = field->getName();
if (field_name.empty()) {
CompilerType field_type = GetType(field->getType());
+std::vector save_indices = child_indexes;
child_indexes.push_back(child_idx);
if (field_type.GetIndexOfChildMemberWithName(
name, omit_empty_base_classes, child_indexes))
return child_indexes.size();
-child_indexes.pop_back();
-
+child_indexes = save_indices;
} else if (field_name == name) {
// We have to add on the number of base classes to this index!
child_indexes.push_back(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)
@@ -32,35 +34,44 @@ using namespace lldb_dap;
namespace lldb_dap {
-DAP::DAP(llvm::StringRef path, ReplMode repl_mode)
-: debug_adaptor_path(path), broadcaster("lldb-dap"),
- exception_breakpoints(), focus_tid(LLDB_INVALID_THREAD_ID),
- stop_at_entry(false), is_attach(false),
+DAP::DAP(llvm::StringRef path, llvm::raw_ostream *log, ReplMode repl_mode,
+ std::vector pre_init_commands)
+: debug_adaptor_path(path), broadcaster("lldb-dap"), log(log),
+ exception_breakpoints(), pre_init_commands(pre_init_commands),
+ focus_tid(LLDB_INVALID_THREAD_ID), stop_at_entry(false),
is_attach(false),
enable_auto_variable_summaries(false),
enable_synthetic_child_debugging(false),
display_extended_backtrace(false),
restarting_process_id(LLDB_INVALID_PROCESS_ID),
configuration_done_sent(false), waiting_for_run_in_terminal(false),
progress_event_reporter(
[&](const ProgressEvent &event) { SendJSON(event.ToJSON()); }),
- reverse_request_seq(0), repl_mode(repl_mode) {
- const char *log_file_path = getenv("LLDBDAP_LOG");
-#if defined(_WIN32)
- // Windows opens stdout and stdin in text mode which converts \n to 13,10
- // while the value is just 10 on Darwin/Linux. Setting the file mode to
binary
- // fixes this.
- int result = _setmode(fileno(stdout), _O_BINARY);
- assert(result);
- result = _setmode(fileno(stdin), _O_BINARY);
- UNUSED_IF_ASSERT_DISABLED(result);
- assert(result);
-#endif
- if (log_file_path)
-log.reset(new std::ofstream(log_file_path));
-}
+ reverse_request_seq(0), repl_mode(repl_mode) {}
DAP::~DAP() = default;
+llvm::Error DAP::ConfigureIO(int out_fd, int err_fd) {
+ llvm::Expected new_stdout_fd =
+ RedirectFd(out_fd, [this](llvm::StringRef data) {
+SendOutput(OutputType::Stdout, data);
+ });
ashgti wrote:
Adjusted the RedirectFD to clean up once the DAP object is not used anymore.
This includes making a best effort to use `dup2` to revert the FD redirects.
As for stderr handling, in server mode it no longer setups a redirect on stderr
to the current connection. When I allocate the SBDebugger I also configure the
OutputFile and ErrorFile handles to pipes that forward to that specific
connection.
This does mean that if we are running in a server mode and some part of lldb
(or llvm/clang/etc.) print directly to stderr then it wouldn't be associated
with the debug session, but I think that would be an issue for other users of
the SBDebugger API.
I know that Xcode also has a helper `lldb-rpc-server` that is a similar
configuration to the lldb-dap server mode. When a debug session starts in Xcode
it starts an `lldb-rpc-server` that hosts all the SBDebugger instances. So a
single version of Xcode will have 1 instance of `lldb-rpc-server` running at a
time and that instance is used to run each debug session for each Xcode window.
But my overall point with this is that if there are parts of the stack that are
not respecting the SBDebugger's OutputFile and ErrorFile it would also show up
in Xcode.
https://github.com/llvm/llvm-project/pull/116392
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [Clang] Improve Sema diagnostic performance for __builtin_counted_by_ref (PR #116719)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/116719 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [Clang] Improve Sema diagnostic performance for __builtin_counted_by_ref (PR #116719)
@@ -14690,6 +14690,13 @@ void Sema::FinalizeDeclaration(Decl *ThisDecl) {
}
}
+ // The result of __builtin_counted_by_ref cannot be assigned to a variable.
+ // It allows leaking and modification of bounds safety information.
+ if (IsBuiltinCountedByRef(VD->getInit()))
+Diag(VD->getInit()->getExprLoc(),
+ diag::err_builtin_counted_by_ref_cannot_leak_reference)
+<< VD->getInit()->getSourceRange();
AaronBallman wrote:
Should we split this off into a helper function like `bool
CheckInvalidBuiltinCountedByRef(const Expr *E);` ?
https://github.com/llvm/llvm-project/pull/116719
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [Clang] Improve Sema diagnostic performance for __builtin_counted_by_ref (PR #116719)
https://github.com/AaronBallman commented: Is there a branch up for this on https://llvm-compile-time-tracker.com/ so that we've verified that this actually improves performance? https://github.com/llvm/llvm-project/pull/116719 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 86f7f08 - Fix return value of 'PluginManager::RegisterPlugin()'. (#114120)
Author: Miro Bucko
Date: 2024-11-26T11:29:24-05:00
New Revision: 86f7f089ee6bcf01bf082ca802220b1143a3ade9
URL:
https://github.com/llvm/llvm-project/commit/86f7f089ee6bcf01bf082ca802220b1143a3ade9
DIFF:
https://github.com/llvm/llvm-project/commit/86f7f089ee6bcf01bf082ca802220b1143a3ade9.diff
LOG: Fix return value of 'PluginManager::RegisterPlugin()'. (#114120)
Added:
Modified:
lldb/source/Core/PluginManager.cpp
Removed:
diff --git a/lldb/source/Core/PluginManager.cpp
b/lldb/source/Core/PluginManager.cpp
index a5219025495a91..80c9465f9af721 100644
--- a/lldb/source/Core/PluginManager.cpp
+++ b/lldb/source/Core/PluginManager.cpp
@@ -206,10 +206,9 @@ template class PluginInstances {
if (!callback)
return false;
assert(!name.empty());
-Instance instance =
-Instance(name, description, callback, std::forward(args)...);
-m_instances.push_back(instance);
-return false;
+m_instances.emplace_back(name, description, callback,
+ std::forward(args)...);
+return true;
}
bool UnregisterPlugin(typename Instance::CallbackType callback) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix return value of 'PluginManager::RegisterPlugin()'. (PR #114120)
https://github.com/mbucko closed https://github.com/llvm/llvm-project/pull/114120 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Bugfix: Not showing the synthetic children of values behind pointers (PR #117755)
@@ -4028,6 +4032,9 @@ void request_variables(DAP &dap, const llvm::json::Object
&request) {
dap.enable_synthetic_child_debugging,
/*is_name_duplicated=*/false, custom_name));
};
+ if (variable.GetType().IsPointerType() ||
variable.GetType().IsReferenceType()) {
+variable = variable.Dereference();
+ }
walter-erquinigo wrote:
Do you know why you need this change and the one above is not enough?
https://github.com/llvm/llvm-project/pull/117755
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] build: enable CONFIG mode search for LibXml2 for LLDB (PR #117615)
https://github.com/compnerd updated
https://github.com/llvm/llvm-project/pull/117615
>From 16271991a157b7892cb3d0e914aac63f7659d41b Mon Sep 17 00:00:00 2001
From: Saleem Abdulrasool
Date: Mon, 25 Nov 2024 11:21:17 -0800
Subject: [PATCH] build: clean up extraneous include paths
Clean up some unnecessary include paths. The use of `LibXml2::LibXml2`
with `target_link_libraries` on `libLLDBHost` ensures that the header
search path is properly propagated.
---
lldb/cmake/modules/LLDBConfig.cmake | 5 -
1 file changed, 5 deletions(-)
diff --git a/lldb/cmake/modules/LLDBConfig.cmake
b/lldb/cmake/modules/LLDBConfig.cmake
index 93ccd9c479c2b8..ee4c2630d32e25 100644
--- a/lldb/cmake/modules/LLDBConfig.cmake
+++ b/lldb/cmake/modules/LLDBConfig.cmake
@@ -239,10 +239,6 @@ if (LLDB_ENABLE_LZMA)
include_directories(${LIBLZMA_INCLUDE_DIRS})
endif()
-if (LLDB_ENABLE_LIBXML2)
- include_directories(${LIBXML2_INCLUDE_DIR})
-endif()
-
include_directories(BEFORE
${CMAKE_CURRENT_BINARY_DIR}/include
${CMAKE_CURRENT_SOURCE_DIR}/include
@@ -283,7 +279,6 @@ if (APPLE)
find_library(FOUNDATION_LIBRARY Foundation)
find_library(CORE_FOUNDATION_LIBRARY CoreFoundation)
find_library(SECURITY_LIBRARY Security)
- include_directories(${LIBXML2_INCLUDE_DIR})
endif()
if( WIN32 AND NOT CYGWIN )
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Bugfix: Not showing the synthetic children of values behind pointers (PR #117755)
https://github.com/clayborg commented:
None of this should be needed. If we have a child value that is a pointer to a
synthetic value, and we expand that value, we should be showing the synthetic
children via the standard LLDB APIs.
In LLDB way back in the beginning, when we would expand a pointer, we would
show the first child as the dereferenced value and its name would be
"*". And then if you expanded that, then you would see the contents.
So if we had this code:
```
struct Point { int x,y; };
int my_int = 12
Point pt = {1,2};
Point *pt_ptr = &pt;
int *my_int_ptr = &i;
```
Old LLDB would show this as:
```
pt_ptr
\_ *pt_ptr
|- x = 1
\- y = 2
my_int_ptr
\_ 12
```
Current LLDB, when you expand a pointer, will look at the type and see if the
type has children, and if it does, it will auto dereference the type:
```
pt_ptr
|- x = 1
\- y = 2
my_int_ptr
\_ 12
```
And if the dereferenced value has a synthetic child provider, it should be
grabbing that synthetic value and asking it for its children.
So this isn't the right fix for this. It works for C++. We need to figure out
why it isn't working for your language
https://github.com/user-attachments/assets/7a548b5b-9b94-4dbe-9060-9fb0e0416730";>
https://github.com/llvm/llvm-project/pull/117755
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add 'FindFirstSymbolWithNameAndType()' to ModuleList. (PR #117777)
https://github.com/mbucko created
https://github.com/llvm/llvm-project/pull/11
None
>From 7f36a8b8672c9a75aa7dfe6a123401ae6d789a11 Mon Sep 17 00:00:00 2001
From: Miro Bucko
Date: Tue, 26 Nov 2024 12:07:57 -0800
Subject: [PATCH] Add 'FindFirstSymbolWithNameAndType()' to ModuleList.
---
lldb/include/lldb/Core/ModuleList.h | 4
lldb/source/Core/ModuleList.cpp | 13 +
2 files changed, 17 insertions(+)
diff --git a/lldb/include/lldb/Core/ModuleList.h
b/lldb/include/lldb/Core/ModuleList.h
index 43d931a8447406..29881a967b7a61 100644
--- a/lldb/include/lldb/Core/ModuleList.h
+++ b/lldb/include/lldb/Core/ModuleList.h
@@ -353,6 +353,10 @@ class ModuleList {
lldb::ModuleSP FindFirstModule(const ModuleSpec &module_spec) const;
+ const Symbol *
+ FindFirstSymbolWithNameAndType(ConstString name,
+ lldb::SymbolType symbol_type) const;
+
void FindSymbolsWithNameAndType(ConstString name,
lldb::SymbolType symbol_type,
SymbolContextList &sc_list) const;
diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index 2b8ccab2406c6b..3b0ee6bd87b334 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -524,6 +524,19 @@ void ModuleList::FindGlobalVariables(const
RegularExpression ®ex,
module_sp->FindGlobalVariables(regex, max_matches, variable_list);
}
+const Symbol *
+ModuleList::FindFirstSymbolWithNameAndType(ConstString name,
+ lldb::SymbolType symbol_type) const
{
+ std::lock_guard guard(m_modules_mutex);
+ for (const ModuleSP &module_sp : m_modules) {
+const Symbol *symbol =
+module_sp->FindFirstSymbolWithNameAndType(name, symbol_type);
+if (symbol)
+ return symbol;
+ }
+ return nullptr;
+}
+
void ModuleList::FindSymbolsWithNameAndType(ConstString name,
SymbolType symbol_type,
SymbolContextList &sc_list) const {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add 'FindFirstSymbolWithNameAndType()' to ModuleList. (PR #117777)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Miro Bucko (mbucko)
Changes
---
Full diff: https://github.com/llvm/llvm-project/pull/11.diff
2 Files Affected:
- (modified) lldb/include/lldb/Core/ModuleList.h (+4)
- (modified) lldb/source/Core/ModuleList.cpp (+13)
``diff
diff --git a/lldb/include/lldb/Core/ModuleList.h
b/lldb/include/lldb/Core/ModuleList.h
index 43d931a8447406..29881a967b7a61 100644
--- a/lldb/include/lldb/Core/ModuleList.h
+++ b/lldb/include/lldb/Core/ModuleList.h
@@ -353,6 +353,10 @@ class ModuleList {
lldb::ModuleSP FindFirstModule(const ModuleSpec &module_spec) const;
+ const Symbol *
+ FindFirstSymbolWithNameAndType(ConstString name,
+ lldb::SymbolType symbol_type) const;
+
void FindSymbolsWithNameAndType(ConstString name,
lldb::SymbolType symbol_type,
SymbolContextList &sc_list) const;
diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index 2b8ccab2406c6b..3b0ee6bd87b334 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -524,6 +524,19 @@ void ModuleList::FindGlobalVariables(const
RegularExpression ®ex,
module_sp->FindGlobalVariables(regex, max_matches, variable_list);
}
+const Symbol *
+ModuleList::FindFirstSymbolWithNameAndType(ConstString name,
+ lldb::SymbolType symbol_type) const
{
+ std::lock_guard guard(m_modules_mutex);
+ for (const ModuleSP &module_sp : m_modules) {
+const Symbol *symbol =
+module_sp->FindFirstSymbolWithNameAndType(name, symbol_type);
+if (symbol)
+ return symbol;
+ }
+ return nullptr;
+}
+
void ModuleList::FindSymbolsWithNameAndType(ConstString name,
SymbolType symbol_type,
SymbolContextList &sc_list) const {
``
https://github.com/llvm/llvm-project/pull/11
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)
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 c71418574f1bb9e4678428901775c8b633cded09...35035cd4b8bfdbe2a0d196be7b4aa74a144f160c lldb/test/API/tools/lldb-dap/server/TestDAP_server.py lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py `` View the diff from darker here. ``diff --- test/API/tools/lldb-dap/server/TestDAP_server.py2024-11-26 22:01:42.00 + +++ test/API/tools/lldb-dap/server/TestDAP_server.py2024-11-26 22:04:43.964689 + @@ -62,9 +62,11 @@ """ Test launching a binary with a lldb-dap in server mode on a unix socket. """ dir = tempfile.gettempdir() name = dir + "/dap-connection-" + str(os.getpid()) + def cleanup(): os.unlink(name) + self.addTearDownHook(cleanup) self.do_test_server(connection="unix://" + name) `` https://github.com/llvm/llvm-project/pull/116392 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [Clang] Improve Sema diagnostic performance for __builtin_counted_by_ref (PR #116719)
nikic wrote: > Is there a branch up for this on https://llvm-compile-time-tracker.com/ so > that we've verified that this actually improves performance? Compile-time: https://llvm-compile-time-tracker.com/compare.php?from=105b7803ea22823a2fca2a82ee843d0884e9cbf3&to=dcb3e2b6b4333556fbfa3d7a5a45c145972b466f&stat=instructions:u Yes, this does recover the regression. https://github.com/llvm/llvm-project/pull/116719 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Bugfix: Not showing the synthetic children of values behind pointers (PR #117755)
@@ -4028,6 +4032,9 @@ void request_variables(DAP &dap, const llvm::json::Object
&request) {
dap.enable_synthetic_child_debugging,
/*is_name_duplicated=*/false, custom_name));
};
+ if (variable.GetType().IsPointerType() ||
variable.GetType().IsReferenceType()) {
+variable = variable.Dereference();
+ }
skuznetsov wrote:
The first change is more than enough. To be on the safe side, I overcomplicated
it a little.
https://github.com/llvm/llvm-project/pull/117755
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Bugfix: Not showing the synthetic children of values behind pointers (PR #117755)
https://github.com/skuznetsov edited https://github.com/llvm/llvm-project/pull/117755 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Bugfix: Not showing the synthetic children of values behind pointers (PR #117755)
skuznetsov wrote: @clayborg This is what puzzles me, too. As I provided the examples in the Discourse thread, it works in the CLI lldb but not when used in lldb-dap. It works only if it is dereferenced. https://github.com/llvm/llvm-project/pull/117755 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)
@@ -250,6 +251,13 @@ def which(program):
return None
+def pickrandomport():
+"""Returns a random open port."""
+with socket.socket() as sock:
+sock.bind(("", 0))
+return sock.getsockname()[1]
ashgti wrote:
Reverted changes to this file.
https://github.com/llvm/llvm-project/pull/116392
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)
@@ -52,11 +51,11 @@ struct StreamDescriptor {
struct InputStream {
StreamDescriptor descriptor;
- bool read_full(std::ofstream *log, size_t length, std::string &text);
+ bool read_full(llvm::raw_ostream *log, size_t length, std::string &text);
- bool read_line(std::ofstream *log, std::string &line);
+ bool read_line(llvm::raw_ostream *log, std::string &line);
- bool read_expected(std::ofstream *log, llvm::StringRef expected);
+ bool read_expected(llvm::raw_ostream *log, llvm::StringRef expected);
ashgti wrote:
Reverted this change.
https://github.com/llvm/llvm-project/pull/116392
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)
@@ -0,0 +1,182 @@ +//===-- Socket.h *- C++ -*-===// ashgti wrote: I reworked this now to use the `lldb/Host/Socket.h` and associated types. https://github.com/llvm/llvm-project/pull/116392 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)
https://github.com/ashgti edited https://github.com/llvm/llvm-project/pull/116392 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix return value of 'PluginManager::RegisterPlugin()'. (PR #114120)
https://github.com/mbucko reopened https://github.com/llvm/llvm-project/pull/114120 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] build: enable CONFIG mode search for LibXml2 for LLDB (PR #117615)
https://github.com/compnerd updated
https://github.com/llvm/llvm-project/pull/117615
>From 0b7645656a9a79a496438f49c7906dead1319fe7 Mon Sep 17 00:00:00 2001
From: Saleem Abdulrasool
Date: Mon, 25 Nov 2024 11:21:17 -0800
Subject: [PATCH] build: enable CONFIG mode search for LibXml2 for LLDB
The `find_package(LibXml2 ...)` invocation that we are currently using
precludes the use of "CONFIG mode" for libxml2. This is important to
allow dependencies to flow through the build with static builds on
Windows, which depends on Bcrypt and conditionally on Ws2_32 (in
development - current releases are unconditionally dependent on it). If
libxml2 is built statically, this dependency would need to be replicated
into LLDB's build configuration to ensure that the dependencies are met.
Add a workaround to support CONFIG mode search which avoids this need.
Additionally, clean up some unnecessary include paths. The use of
`LibXml2::LibXml2` with `target_link_libraries` on `libLLDBHost` ensures
that the header search path is properly propagated.
---
lldb/cmake/modules/LLDBConfig.cmake | 17 +++--
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/lldb/cmake/modules/LLDBConfig.cmake
b/lldb/cmake/modules/LLDBConfig.cmake
index 93ccd9c479c2b8..bf659c1648b867 100644
--- a/lldb/cmake/modules/LLDBConfig.cmake
+++ b/lldb/cmake/modules/LLDBConfig.cmake
@@ -57,7 +57,17 @@ add_optional_dependency(LLDB_ENABLE_CURSES "Enable curses
support in LLDB" Curse
add_optional_dependency(LLDB_ENABLE_LZMA "Enable LZMA compression support in
LLDB" LibLZMA LIBLZMA_FOUND)
add_optional_dependency(LLDB_ENABLE_LUA "Enable Lua scripting support in LLDB"
LuaAndSwig LUAANDSWIG_FOUND)
add_optional_dependency(LLDB_ENABLE_PYTHON "Enable Python scripting support in
LLDB" PythonAndSwig PYTHONANDSWIG_FOUND)
-add_optional_dependency(LLDB_ENABLE_LIBXML2 "Enable Libxml 2 support in LLDB"
LibXml2 LIBXML2_FOUND VERSION 2.8)
+# The FindLibXml2.cmake module from CMake does not seem to search for the
+# package in both CONFIG and MODULE mode as the `find_package` documentation
+# states. Workaround this by initially doing an optional search in CONFIG mode
+# and fallback to the default search, if needed, which currently only does the
+# MODULE mode search.
+if(LLDB_ENABLE_LIBXML2 OR "${LLDB_ENABLE_LIBXML2}" STREQUAL "AUTO")
+ find_package(LibXml2 CONFIG)
+ if(NOT TARGET LibXml2::LibXml2)
+add_optional_dependency(LLDB_ENABLE_LIBXML2 "Enable Libxml 2 support in
LLDB" LibXml2 LIBXML2_FOUND VERSION 2.8)
+ endif()
+endif()
add_optional_dependency(LLDB_ENABLE_FBSDVMCORE "Enable libfbsdvmcore support
in LLDB" FBSDVMCore FBSDVMCore_FOUND QUIET)
option(LLDB_USE_ENTITLEMENTS "When codesigning, use entitlements if available"
ON)
@@ -239,10 +249,6 @@ if (LLDB_ENABLE_LZMA)
include_directories(${LIBLZMA_INCLUDE_DIRS})
endif()
-if (LLDB_ENABLE_LIBXML2)
- include_directories(${LIBXML2_INCLUDE_DIR})
-endif()
-
include_directories(BEFORE
${CMAKE_CURRENT_BINARY_DIR}/include
${CMAKE_CURRENT_SOURCE_DIR}/include
@@ -283,7 +289,6 @@ if (APPLE)
find_library(FOUNDATION_LIBRARY Foundation)
find_library(CORE_FOUNDATION_LIBRARY CoreFoundation)
find_library(SECURITY_LIBRARY Security)
- include_directories(${LIBXML2_INCLUDE_DIR})
endif()
if( WIN32 AND NOT CYGWIN )
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [Clang] Improve Sema diagnostic performance for __builtin_counted_by_ref (PR #116719)
@@ -14690,6 +14690,13 @@ void Sema::FinalizeDeclaration(Decl *ThisDecl) {
}
}
+ // The result of __builtin_counted_by_ref cannot be assigned to a variable.
+ // It allows leaking and modification of bounds safety information.
+ if (IsBuiltinCountedByRef(VD->getInit()))
+Diag(VD->getInit()->getExprLoc(),
+ diag::err_builtin_counted_by_ref_cannot_leak_reference)
+<< VD->getInit()->getSourceRange();
Sirraide wrote:
I was going to comment on that too, but ended up forgetting about it; it’s used
in like 3–4 places, so I personally would prefer making this a helper function.
https://github.com/llvm/llvm-project/pull/116719
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Bugfix: Not showing the synthetic children of values behind pointers (PR #117755)
https://github.com/skuznetsov created
https://github.com/llvm/llvm-project/pull/117755
This bug fix for the situation that was extensively discussed at LLVM Discourse
thread: https://discourse.llvm.org/t/synthetic-data-providers-and-lldb-dap/
>From 82a3bc3aaf57f25a1041cda4082b24bd8cdf8919 Mon Sep 17 00:00:00 2001
From: Sergey Kuznetsov
Date: Tue, 26 Nov 2024 12:54:30 -0500
Subject: [PATCH] Bugfix: Not showing the synthetic children of values behind
pointers
---
lldb/tools/lldb-dap/lldb-dap.cpp | 7 +++
1 file changed, 7 insertions(+)
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 3bfc578806021e..b86994f60f04e5 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -4020,6 +4020,10 @@ void request_variables(DAP &dap, const
llvm::json::Object &request) {
std::optional custom_name = {}) {
if (!child.IsValid())
return;
+if (child.IsSynthetic() && (child.GetType().IsPointerType() ||
child.GetType().IsReferenceType())) {
+ // Dereference to access synthetic children behind
pointers/references
+ child = child.Dereference();
+}
bool is_permanent =
dap.variables.IsPermanentVariableReference(variablesReference);
int64_t var_ref = dap.variables.InsertVariable(child, is_permanent);
@@ -4028,6 +4032,9 @@ void request_variables(DAP &dap, const llvm::json::Object
&request) {
dap.enable_synthetic_child_debugging,
/*is_name_duplicated=*/false, custom_name));
};
+ if (variable.GetType().IsPointerType() ||
variable.GetType().IsReferenceType()) {
+variable = variable.Dereference();
+ }
const int64_t num_children = variable.GetNumChildren();
int64_t end_idx = start + ((count == 0) ? num_children : count);
int64_t i = start;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Bugfix: Not showing the synthetic children of values behind pointers (PR #117755)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Sergey Kuznetsov (skuznetsov)
Changes
This bug fix for the situation that was extensively discussed at LLVM Discourse
thread: https://discourse.llvm.org/t/synthetic-data-providers-and-lldb-dap/
---
Full diff: https://github.com/llvm/llvm-project/pull/117755.diff
1 Files Affected:
- (modified) lldb/tools/lldb-dap/lldb-dap.cpp (+7)
``diff
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 3bfc578806021e..b86994f60f04e5 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -4020,6 +4020,10 @@ void request_variables(DAP &dap, const
llvm::json::Object &request) {
std::optional custom_name = {}) {
if (!child.IsValid())
return;
+if (child.IsSynthetic() && (child.GetType().IsPointerType() ||
child.GetType().IsReferenceType())) {
+ // Dereference to access synthetic children behind
pointers/references
+ child = child.Dereference();
+}
bool is_permanent =
dap.variables.IsPermanentVariableReference(variablesReference);
int64_t var_ref = dap.variables.InsertVariable(child, is_permanent);
@@ -4028,6 +4032,9 @@ void request_variables(DAP &dap, const llvm::json::Object
&request) {
dap.enable_synthetic_child_debugging,
/*is_name_duplicated=*/false, custom_name));
};
+ if (variable.GetType().IsPointerType() ||
variable.GetType().IsReferenceType()) {
+variable = variable.Dereference();
+ }
const int64_t num_children = variable.GetNumChildren();
int64_t end_idx = start + ((count == 0) ? num_children : count);
int64_t i = start;
``
https://github.com/llvm/llvm-project/pull/117755
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Handle improperly nested blocks differently (PR #117725)
https://github.com/labath created
https://github.com/llvm/llvm-project/pull/117725
In 6c7f56192fa6e689ef14d32e43a785de5692e9c0 (Sep 2011) we added code to extend
the range of the parent block if the child block is not contained within it.
Currently, this code doesn't work for (at least) two reasons:
- we don't re-sort the ranges of the parent block, which means that the lookup
may not find the range we added (and possibly other ranges as well)
- The main address lookup mechanism
(SymbolFileDWARF::ResolveSymbolContext(Address)) performs the lookup using
DWARF DIE information, which doesn't contain this extra range.
The motivation for the change was bad compiler output. The commit message does
not go into details, so it's hard to say if this has been fixed, but given that
was 13 years ago, I think we can assume that to be the case. In fact, as far as
I can tell (I haven't tried building an lldb this old) this code stopped
working in
ea3e7d5ccf4f00741e4b106978bd8dab5cece3a1 (~two weeks later), when we added the
requirement for the ranges to be sorted.
Given that this isn't working, and that not changing the ranges of other blocks
has other nice properties (the ranges can be immutable after construction), I'm
removing the parent range changing code. I'm adding a test case that makes sure
we don't do something egregious (e.g., crash) in this case. Having a child
block not be a subset of the parent block doesn't seem to cause problems now,
but if that ever turns out to be an issue, we can always intersect the child
range with that of the parent.
I'm also upgrading this message to a proper warning so we can see if this ever
occurs in practice, and simplifying it so it doesn't get in the way of
understanding the function.
>From 12ddfe8273339608e00946bd30218b8a407d8524 Mon Sep 17 00:00:00 2001
From: Pavel Labath
Date: Tue, 26 Nov 2024 16:12:40 +0100
Subject: [PATCH] [lldb] Handle improperly nested blocks differently
In 6c7f56192fa6e689ef14d32e43a785de5692e9c0 (Sep 2011) we added code to
extend the range of the parent block if the child block is not contained
within it. Currently, this code doesn't work for (at least) two
reasons:
- we don't re-sort the ranges of the parent block, which means that the
lookup may not find the range we added (and possibly other ranges as
well)
- The main address lookup mechanism
(SymbolFileDWARF::ResolveSymbolContext(Address)) performs the lookup
using DWARF DIE information, which doesn't contain this extra range.
The motivation for the change was bad compiler output. The commit
message does not go into details, so it's hard to say if this has been
fixed, but given that was 13 years ago, I think we can assume that to be
the case. In fact, as far as I can tell (I haven't tried building an
lldb this old) this code stopped working in
ea3e7d5ccf4f00741e4b106978bd8dab5cece3a1 (~two weeks later), when we
added the requirement for the ranges to be sorted.
Given that this isn't working, and that not changing the ranges of other
blocks has other nice properties (the ranges can be immutable after
construction), I'm removing the parent range changing code. I'm adding a
test case that makes sure we don't do something egregious (e.g., crash)
in this case. Having a child block not be a subset of the parent block
doesn't seem to cause problems now, but if that ever turns out to be an
issue, we can always intersect the child range with that of the parent.
I'm also upgrading this message to a proper warning so we can see if
this ever occurs in practice, and simplifying it so it doesn't get in
the way of understanding the function.
---
lldb/source/Symbol/Block.cpp | 38 +--
.../DWARF/x86/improperly_nested_blocks.s | 100 ++
2 files changed, 105 insertions(+), 33 deletions(-)
create mode 100644
lldb/test/Shell/SymbolFile/DWARF/x86/improperly_nested_blocks.s
diff --git a/lldb/source/Symbol/Block.cpp b/lldb/source/Symbol/Block.cpp
index 8746a25e3fad5a..27fce043dfc5ed 100644
--- a/lldb/source/Symbol/Block.cpp
+++ b/lldb/source/Symbol/Block.cpp
@@ -352,39 +352,11 @@ void Block::FinalizeRanges() {
void Block::AddRange(const Range &range) {
Block *parent_block = GetParent();
if (parent_block && !parent_block->Contains(range)) {
-Log *log = GetLog(LLDBLog::Symbols);
-if (log) {
- ModuleSP module_sp(m_parent_scope->CalculateSymbolContextModule());
- Function *function = m_parent_scope->CalculateSymbolContextFunction();
- const addr_t function_file_addr =
- function->GetAddressRange().GetBaseAddress().GetFileAddress();
- const addr_t block_start_addr = function_file_addr +
range.GetRangeBase();
- const addr_t block_end_addr = function_file_addr + range.GetRangeEnd();
- Type *func_type = function->GetType();
-
- const Declaration &func_decl = func_type->GetDeclaration();
- if (func_decl.GetLine()) {
-LLDB_LOGF(log,
- "warning: %s:%u block
[Lldb-commits] [lldb] [lldb] Handle improperly nested blocks differently (PR #117725)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Pavel Labath (labath)
Changes
In 6c7f56192fa6e689ef14d32e43a785de5692e9c0 (Sep 2011) we added code to extend
the range of the parent block if the child block is not contained within it.
Currently, this code doesn't work for (at least) two reasons:
- we don't re-sort the ranges of the parent block, which means that the lookup
may not find the range we added (and possibly other ranges as well)
- The main address lookup mechanism
(SymbolFileDWARF::ResolveSymbolContext(Address)) performs the lookup using
DWARF DIE information, which doesn't contain this extra range.
The motivation for the change was bad compiler output. The commit message does
not go into details, so it's hard to say if this has been fixed, but given that
was 13 years ago, I think we can assume that to be the case. In fact, as far as
I can tell (I haven't tried building an lldb this old) this code stopped
working in
ea3e7d5ccf4f00741e4b106978bd8dab5cece3a1 (~two weeks later), when we added the
requirement for the ranges to be sorted.
Given that this isn't working, and that not changing the ranges of other blocks
has other nice properties (the ranges can be immutable after construction), I'm
removing the parent range changing code. I'm adding a test case that makes sure
we don't do something egregious (e.g., crash) in this case. Having a child
block not be a subset of the parent block doesn't seem to cause problems now,
but if that ever turns out to be an issue, we can always intersect the child
range with that of the parent.
I'm also upgrading this message to a proper warning so we can see if this ever
occurs in practice, and simplifying it so it doesn't get in the way of
understanding the function.
---
Full diff: https://github.com/llvm/llvm-project/pull/117725.diff
2 Files Affected:
- (modified) lldb/source/Symbol/Block.cpp (+5-33)
- (added) lldb/test/Shell/SymbolFile/DWARF/x86/improperly_nested_blocks.s
(+100)
``diff
diff --git a/lldb/source/Symbol/Block.cpp b/lldb/source/Symbol/Block.cpp
index 8746a25e3fad5a..27fce043dfc5ed 100644
--- a/lldb/source/Symbol/Block.cpp
+++ b/lldb/source/Symbol/Block.cpp
@@ -352,39 +352,11 @@ void Block::FinalizeRanges() {
void Block::AddRange(const Range &range) {
Block *parent_block = GetParent();
if (parent_block && !parent_block->Contains(range)) {
-Log *log = GetLog(LLDBLog::Symbols);
-if (log) {
- ModuleSP module_sp(m_parent_scope->CalculateSymbolContextModule());
- Function *function = m_parent_scope->CalculateSymbolContextFunction();
- const addr_t function_file_addr =
- function->GetAddressRange().GetBaseAddress().GetFileAddress();
- const addr_t block_start_addr = function_file_addr +
range.GetRangeBase();
- const addr_t block_end_addr = function_file_addr + range.GetRangeEnd();
- Type *func_type = function->GetType();
-
- const Declaration &func_decl = func_type->GetDeclaration();
- if (func_decl.GetLine()) {
-LLDB_LOGF(log,
- "warning: %s:%u block {0x%8.8" PRIx64
- "} has range[%u] [0x%" PRIx64 " - 0x%" PRIx64
- ") which is not contained in parent block {0x%8.8" PRIx64
- "} in function {0x%8.8" PRIx64 "} from %s",
- func_decl.GetFile().GetPath().c_str(), func_decl.GetLine(),
- GetID(), (uint32_t)m_ranges.GetSize(), block_start_addr,
- block_end_addr, parent_block->GetID(), function->GetID(),
- module_sp->GetFileSpec().GetPath().c_str());
- } else {
-LLDB_LOGF(log,
- "warning: block {0x%8.8" PRIx64 "} has range[%u] [0x%" PRIx64
- " - 0x%" PRIx64
- ") which is not contained in parent block {0x%8.8" PRIx64
- "} in function {0x%8.8" PRIx64 "} from %s",
- GetID(), (uint32_t)m_ranges.GetSize(), block_start_addr,
- block_end_addr, parent_block->GetID(), function->GetID(),
- module_sp->GetFileSpec().GetPath().c_str());
- }
-}
-parent_block->AddRange(range);
+m_parent_scope->CalculateSymbolContextModule()->ReportWarning(
+"block {0:x} has a range [{1:x}, {2:x}) which is not contained in the "
+"parent block {3:x}",
+GetID(), range.GetRangeBase(), range.GetRangeEnd(),
+parent_block->GetID());
}
m_ranges.Append(range);
}
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/improperly_nested_blocks.s
b/lldb/test/Shell/SymbolFile/DWARF/x86/improperly_nested_blocks.s
new file mode 100644
index 00..af744385993f28
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/improperly_nested_blocks.s
@@ -0,0 +1,100 @@
+## This test checks that lldb handles (corrupt?) debug info which has
improperly
+## nested blocks. The behavior here is not prescriptive. We only want to check
+## that we do something "reasonable".
+
+
+# R
[Lldb-commits] [lldb] [lldb] Add timed callbacks to the MainLoop class (PR #112895)
@@ -223,6 +220,61 @@ TEST_F(MainLoopTest, ManyPendingCallbacks) {
ASSERT_TRUE(loop.Run().Success());
}
+TEST_F(MainLoopTest, CallbackWithTimeout) {
+ MainLoop loop;
+ loop.AddCallback([](MainLoopBase &loop) { loop.RequestTermination(); },
+ std::chrono::seconds(2));
+ auto start = std::chrono::steady_clock::now();
+ ASSERT_THAT_ERROR(loop.Run().takeError(), llvm::Succeeded());
+ EXPECT_GE(std::chrono::steady_clock::now() - start, std::chrono::seconds(2));
labath wrote:
The representation is "an integer" but to interpret it, you need to know the
resolution of the steady_clock on windows (arm). That appears to be
nanoseconds, which means this value is `0x77354edc/1e9 = 1.8231`
seconds, which means that the callback runs sooner than it should have. I think
I know the reason. Lemme whip up a patch real quick.
https://github.com/llvm/llvm-project/pull/112895
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix premature MainLoop wakeup on windows (PR #117756)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Pavel Labath (labath)
Changes
The windows system APIs only take milliseconds. Make sure we round the sleep
interval (in nanoseconds) upwards.
---
Full diff: https://github.com/llvm/llvm-project/pull/117756.diff
1 Files Affected:
- (modified) lldb/source/Host/windows/MainLoopWindows.cpp (+1-1)
``diff
diff --git a/lldb/source/Host/windows/MainLoopWindows.cpp
b/lldb/source/Host/windows/MainLoopWindows.cpp
index 0a5a35e9db9dde..f3ab2a710cd014 100644
--- a/lldb/source/Host/windows/MainLoopWindows.cpp
+++ b/lldb/source/Host/windows/MainLoopWindows.cpp
@@ -28,7 +28,7 @@ static DWORD
ToTimeout(std::optional point) {
return WSA_INFINITE;
nanoseconds dur = (std::max)(*point - steady_clock::now(), nanoseconds(0));
- return duration_cast(dur).count();
+ return ceil(dur).count();
}
MainLoopWindows::MainLoopWindows() {
``
https://github.com/llvm/llvm-project/pull/117756
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix premature MainLoop wakeup on windows (PR #117756)
https://github.com/labath created
https://github.com/llvm/llvm-project/pull/117756
The windows system APIs only take milliseconds. Make sure we round the sleep
interval (in nanoseconds) upwards.
>From 153338ddae6f6442ffbe195a98e27e812a3f3ed6 Mon Sep 17 00:00:00 2001
From: Pavel Labath
Date: Tue, 26 Nov 2024 19:11:17 +0100
Subject: [PATCH] [lldb] Fix premature MainLoop wakeup on windows
The windows system APIs only take milliseconds. Make sure we round the
sleep interval (in nanoseconds) upwards.
---
lldb/source/Host/windows/MainLoopWindows.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/source/Host/windows/MainLoopWindows.cpp
b/lldb/source/Host/windows/MainLoopWindows.cpp
index 0a5a35e9db9dde..f3ab2a710cd014 100644
--- a/lldb/source/Host/windows/MainLoopWindows.cpp
+++ b/lldb/source/Host/windows/MainLoopWindows.cpp
@@ -28,7 +28,7 @@ static DWORD
ToTimeout(std::optional point) {
return WSA_INFINITE;
nanoseconds dur = (std::max)(*point - steady_clock::now(), nanoseconds(0));
- return duration_cast(dur).count();
+ return ceil(dur).count();
}
MainLoopWindows::MainLoopWindows() {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix premature MainLoop wakeup on windows (PR #117756)
labath wrote: (I haven't tested it, but I'm fairly certain it should fix the problem in https://github.com/llvm/llvm-project/pull/112895#discussion_r1858817258) https://github.com/llvm/llvm-project/pull/117756 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Bugfix: Not showing the synthetic children of values behind pointers (PR #117755)
github-actions[bot] wrote: Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using `@` followed by their GitHub username. If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the [LLVM GitHub User Guide](https://llvm.org/docs/GitHub.html). You can also ask questions in a comment on this PR, on the [LLVM Discord](https://discord.com/invite/xS7Z362) or on the [forums](https://discourse.llvm.org/). https://github.com/llvm/llvm-project/pull/117755 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 4ab298b - [LLDB][ThreadELFCore] Set all the properties of ELFLinuxSigInfo to a non build dependent size (#117604)
Author: Jacob Lalonde
Date: 2024-11-26T10:20:52-08:00
New Revision: 4ab298b5fbc8f48387062b2dd99ea07127c02e6b
URL:
https://github.com/llvm/llvm-project/commit/4ab298b5fbc8f48387062b2dd99ea07127c02e6b
DIFF:
https://github.com/llvm/llvm-project/commit/4ab298b5fbc8f48387062b2dd99ea07127c02e6b.diff
LOG: [LLDB][ThreadELFCore] Set all the properties of ELFLinuxSigInfo to a non
build dependent size (#117604)
On #110065 the changes to LinuxSigInfo Struct introduced some variables
that will differ in size on 32b or 64b. I've rectified this by setting
them all to build independent types.
Added:
Modified:
lldb/source/Plugins/Process/elf-core/ThreadElfCore.h
Removed:
diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h
b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h
index 4ebbaadebe9f90..6f8d41351a6bfb 100644
--- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h
+++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h
@@ -83,10 +83,10 @@ struct ELFLinuxSigInfo {
int32_t si_errno;
int32_t si_code;
// Copied from siginfo_t so we don't have to include signal.h on non 'Nix
- // builds.
- struct {
-lldb::addr_t si_addr; /* faulting insn/memory ref. */
-short int si_addr_lsb; /* Valid LSB of the reported address. */
+ // builds. Slight modifications to ensure no 32b vs 64b
diff erences.
+ struct alignas(8) {
+lldb::addr_t si_addr; /* faulting insn/memory ref. */
+int16_t si_addr_lsb; /* Valid LSB of the reported address. */
union {
/* used when si_code=SEGV_BNDERR */
struct {
@@ -98,7 +98,8 @@ struct ELFLinuxSigInfo {
} bounds;
} sigfault;
- enum { eUnspecified, eNT_SIGINFO } note_type;
+ enum SigInfoNoteType : uint8_t { eUnspecified, eNT_SIGINFO };
+ SigInfoNoteType note_type;
ELFLinuxSigInfo();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][ThreadELFCore] Set all the properties of ELFLinuxSigInfo to a non build dependent size (PR #117604)
Jlalond wrote: > The build succeeded! > > The source comment at line 82 caught my attention, though. It may imply that > data layout is important in this struct, i.e. it should match `siginfo_t` > returned by the OS. But this change is not guaranteeing that. > > Actually, the order of the first 3 integers do change on different systems. > > With that said, this LGTM considering that data layout doesn't need to match > the OS' `siginfo_t`. I'll look into fixing the comment, but I'm going to merge to unbreak everyone's builds! https://github.com/llvm/llvm-project/pull/117604 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] build: enable CONFIG mode search for LibXml2 for LLDB (PR #117615)
@@ -57,7 +57,17 @@ add_optional_dependency(LLDB_ENABLE_CURSES "Enable curses support in LLDB" Curse add_optional_dependency(LLDB_ENABLE_LZMA "Enable LZMA compression support in LLDB" LibLZMA LIBLZMA_FOUND) add_optional_dependency(LLDB_ENABLE_LUA "Enable Lua scripting support in LLDB" LuaAndSwig LUAANDSWIG_FOUND) add_optional_dependency(LLDB_ENABLE_PYTHON "Enable Python scripting support in LLDB" PythonAndSwig PYTHONANDSWIG_FOUND) -add_optional_dependency(LLDB_ENABLE_LIBXML2 "Enable Libxml 2 support in LLDB" LibXml2 LIBXML2_FOUND VERSION 2.8) +# LibXml2 is required if LLDB_ENABLE_LIBXML2 is ON or AUTO. The LibXml2 package +# search is not setup properly to support the CONFIG mode search. Furthermore, compnerd wrote: I believe, the issue is on the CMake side :( The documentation states that a `find_package` is supposed to search for the `CONFIG` and `MODULE` mode. This is now what I'm seeing with libxml2. It only searches in `MODULE` mode. The CONFIG mode works completely once invoked. The problem is I'm worried that not everyone is building libxml2 with CMake and the `CONFIG` mode works only for those that are using CMake. I can rewrite the comment. https://github.com/llvm/llvm-project/pull/117615 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add timed callbacks to the MainLoop class (PR #112895)
@@ -223,6 +220,61 @@ TEST_F(MainLoopTest, ManyPendingCallbacks) {
ASSERT_TRUE(loop.Run().Success());
}
+TEST_F(MainLoopTest, CallbackWithTimeout) {
+ MainLoop loop;
+ loop.AddCallback([](MainLoopBase &loop) { loop.RequestTermination(); },
+ std::chrono::seconds(2));
+ auto start = std::chrono::steady_clock::now();
+ ASSERT_THAT_ERROR(loop.Run().takeError(), llvm::Succeeded());
+ EXPECT_GE(std::chrono::steady_clock::now() - start, std::chrono::seconds(2));
DavidSpickett wrote:
FYI this test is flaky on Windows on Arm:
https://lab.llvm.org/buildbot/#/builders/141/builds/4179
```
TEST 'lldb-unit :: Host/./HostTests.exe/0/9' FAILED
Script(shard):
--
GTEST_OUTPUT=json:C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\unittests\Host\.\HostTests.exe-lldb-unit-16696-0-9.json
GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=9 GTEST_SHARD_INDEX=0
C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\unittests\Host\.\HostTests.exe
--
Script:
--
C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\unittests\Host\.\HostTests.exe
--gtest_filter=MainLoopTest.CallbackWithTimeout
--
C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\unittests\Host\MainLoopTest.cpp(229):
error: Expected: (std::chrono::steady_clock::now() - start) >=
(std::chrono::seconds(2)), actual: 8-byte object vs
8-byte object <02-00 00-00 00-00 00-00>
C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\unittests\Host\MainLoopTest.cpp:229
Expected: (std::chrono::steady_clock::now() - start) >=
(std::chrono::seconds(2)), actual: 8-byte object vs
8-byte object <02-00 00-00 00-00 00-00>
```
If anything I would think our bot might take way longer than 2 seconds but this
test would pass if that were the case. The expected object is clearly looks
like 2 seconds but the one we got I've no idea of the layout.
I don't see it failing in any other build and it could be some bug in MSVC's
STL, but letting you know in case you can spot something obvious in the code.
https://github.com/llvm/llvm-project/pull/112895
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add timed callbacks to the MainLoop class (PR #112895)
@@ -223,6 +220,61 @@ TEST_F(MainLoopTest, ManyPendingCallbacks) {
ASSERT_TRUE(loop.Run().Success());
}
+TEST_F(MainLoopTest, CallbackWithTimeout) {
+ MainLoop loop;
+ loop.AddCallback([](MainLoopBase &loop) { loop.RequestTermination(); },
+ std::chrono::seconds(2));
+ auto start = std::chrono::steady_clock::now();
+ ASSERT_THAT_ERROR(loop.Run().takeError(), llvm::Succeeded());
+ EXPECT_GE(std::chrono::steady_clock::now() - start, std::chrono::seconds(2));
DavidSpickett wrote:
Or on Windows the definition of 2 seconds is a little more fuzzy than we'd like
it to be :)
https://github.com/llvm/llvm-project/pull/112895
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] build: enable CONFIG mode search for LibXml2 for LLDB (PR #117615)
@@ -57,7 +57,17 @@ add_optional_dependency(LLDB_ENABLE_CURSES "Enable curses support in LLDB" Curse add_optional_dependency(LLDB_ENABLE_LZMA "Enable LZMA compression support in LLDB" LibLZMA LIBLZMA_FOUND) add_optional_dependency(LLDB_ENABLE_LUA "Enable Lua scripting support in LLDB" LuaAndSwig LUAANDSWIG_FOUND) add_optional_dependency(LLDB_ENABLE_PYTHON "Enable Python scripting support in LLDB" PythonAndSwig PYTHONANDSWIG_FOUND) -add_optional_dependency(LLDB_ENABLE_LIBXML2 "Enable Libxml 2 support in LLDB" LibXml2 LIBXML2_FOUND VERSION 2.8) +# LibXml2 is required if LLDB_ENABLE_LIBXML2 is ON or AUTO. The LibXml2 package +# search is not setup properly to support the CONFIG mode search. Furthermore, etcwilde wrote: > In this case, by default, CMake first tries Module mode by searching for a > Find.cmake module. If it fails, CMake then searches for the package > using Config mode. - https://cmake.org/cmake/help/latest/variable/CMAKE_FIND_PACKAGE_PREFER_CONFIG.html The `FindLibXml2.cmake` module exists, so it's using that. If you use `CMAKE_FIND_PACKAGE_PREFER_CONFIG=YES`, it should go with your config file. https://github.com/llvm/llvm-project/pull/117615 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][ThreadELFCore] Set all the properties of ELFLinuxSigInfo to a non build dependent size (PR #117604)
https://github.com/Jlalond closed https://github.com/llvm/llvm-project/pull/117604 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Bugfix: Not showing the synthetic children of values behind pointers (PR #117755)
@@ -4020,6 +4020,10 @@ void request_variables(DAP &dap, const
llvm::json::Object &request) {
std::optional custom_name = {}) {
if (!child.IsValid())
return;
+if (child.IsSynthetic() && (child.GetType().IsPointerType() ||
child.GetType().IsReferenceType())) {
walter-erquinigo wrote:
@clayborg , what do you think about this? This pretty much matches the
experience from CLI, i.e. the variable name is prepended with `*`, but the
synthetic value is displayed right away.
https://github.com/llvm/llvm-project/pull/117755
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Remove child_process_inherit from the socket classes (PR #117699)
https://github.com/ashgti approved this pull request. https://github.com/llvm/llvm-project/pull/117699 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] build: enable CONFIG mode search for LibXml2 for LLDB (PR #117615)
@@ -57,7 +57,17 @@ add_optional_dependency(LLDB_ENABLE_CURSES "Enable curses support in LLDB" Curse add_optional_dependency(LLDB_ENABLE_LZMA "Enable LZMA compression support in LLDB" LibLZMA LIBLZMA_FOUND) add_optional_dependency(LLDB_ENABLE_LUA "Enable Lua scripting support in LLDB" LuaAndSwig LUAANDSWIG_FOUND) add_optional_dependency(LLDB_ENABLE_PYTHON "Enable Python scripting support in LLDB" PythonAndSwig PYTHONANDSWIG_FOUND) -add_optional_dependency(LLDB_ENABLE_LIBXML2 "Enable Libxml 2 support in LLDB" LibXml2 LIBXML2_FOUND VERSION 2.8) +# LibXml2 is required if LLDB_ENABLE_LIBXML2 is ON or AUTO. The LibXml2 package +# search is not setup properly to support the CONFIG mode search. Furthermore, compnerd wrote: That seems reasonable. I'll trim this down to the removal of the additional include paths then. https://github.com/llvm/llvm-project/pull/117615 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement basic support for reverse-continue (PR #112079)
https://github.com/rocallahan updated
https://github.com/llvm/llvm-project/pull/112079
>From 6d8f3ccf7b30334853e07e141f41545d68870bb8 Mon Sep 17 00:00:00 2001
From: Robert O'Callahan
Date: Fri, 19 Jul 2024 22:46:42 +1200
Subject: [PATCH] [lldb] Implement basic support for reverse-continue
This commit only adds support for the
`SBProcess::ContinueInDirection()` API. A user-accessible command
for this will follow in a later commit.
This feature depends on a gdbserver implementation (e.g. `rr`)
providing support for the `bc` and `bs` packets. `lldb-server`
does not support those packets, and there is no plan to change that.
with a Python implementation of *very limited* record-and-replay
functionality.
---
lldb/include/lldb/API/SBProcess.h | 1 +
lldb/include/lldb/Target/Process.h| 18 +-
lldb/include/lldb/Target/StopInfo.h | 7 +
lldb/include/lldb/Target/Thread.h | 12 +-
lldb/include/lldb/Target/ThreadList.h | 6 +-
lldb/include/lldb/Target/ThreadPlan.h | 2 +
lldb/include/lldb/Target/ThreadPlanBase.h | 2 +
lldb/include/lldb/lldb-enumerations.h | 6 +
.../Python/lldbsuite/test/gdbclientutils.py | 5 +-
.../Python/lldbsuite/test/lldbgdbproxy.py | 184 +++
.../Python/lldbsuite/test/lldbreverse.py | 480 ++
.../Python/lldbsuite/test/lldbtest.py | 2 +
.../tools/lldb-server/lldbgdbserverutils.py | 15 +-
lldb/source/API/SBProcess.cpp | 8 +
lldb/source/API/SBThread.cpp | 2 +
.../source/Interpreter/CommandInterpreter.cpp | 3 +-
.../Process/Linux/NativeThreadLinux.cpp | 3 +
.../Process/MacOSX-Kernel/ProcessKDP.cpp | 9 +-
.../Process/MacOSX-Kernel/ProcessKDP.h| 2 +-
.../Process/Windows/Common/ProcessWindows.cpp | 9 +-
.../Process/Windows/Common/ProcessWindows.h | 2 +-
.../GDBRemoteCommunicationClient.cpp | 22 +
.../gdb-remote/GDBRemoteCommunicationClient.h | 6 +
.../GDBRemoteCommunicationServerLLGS.cpp | 1 +
.../Process/gdb-remote/ProcessGDBRemote.cpp | 88 +++-
.../Process/gdb-remote/ProcessGDBRemote.h | 2 +-
.../Process/scripted/ScriptedProcess.cpp | 10 +-
.../Process/scripted/ScriptedProcess.h| 2 +-
lldb/source/Target/Process.cpp| 33 +-
lldb/source/Target/StopInfo.cpp | 29 ++
lldb/source/Target/Thread.cpp | 13 +-
lldb/source/Target/ThreadList.cpp | 62 ++-
lldb/source/Target/ThreadPlanBase.cpp | 4 +
.../reverse-execution/Makefile| 3 +
.../TestReverseContinueBreakpoints.py | 151 ++
.../TestReverseContinueNotSupported.py| 30 ++
.../functionalities/reverse-execution/main.c | 14 +
lldb/tools/lldb-dap/JSONUtils.cpp | 3 +
lldb/tools/lldb-dap/LLDBUtils.cpp | 1 +
39 files changed, 1176 insertions(+), 76 deletions(-)
create mode 100644 lldb/packages/Python/lldbsuite/test/lldbgdbproxy.py
create mode 100644 lldb/packages/Python/lldbsuite/test/lldbreverse.py
create mode 100644 lldb/test/API/functionalities/reverse-execution/Makefile
create mode 100644
lldb/test/API/functionalities/reverse-execution/TestReverseContinueBreakpoints.py
create mode 100644
lldb/test/API/functionalities/reverse-execution/TestReverseContinueNotSupported.py
create mode 100644 lldb/test/API/functionalities/reverse-execution/main.c
diff --git a/lldb/include/lldb/API/SBProcess.h
b/lldb/include/lldb/API/SBProcess.h
index 1624e02070b1b2..882b8bd837131d 100644
--- a/lldb/include/lldb/API/SBProcess.h
+++ b/lldb/include/lldb/API/SBProcess.h
@@ -159,6 +159,7 @@ class LLDB_API SBProcess {
lldb::SBError Destroy();
lldb::SBError Continue();
+ lldb::SBError ContinueInDirection(lldb::RunDirection direction);
lldb::SBError Stop();
diff --git a/lldb/include/lldb/Target/Process.h
b/lldb/include/lldb/Target/Process.h
index b8c53a474ba6b9..7b8ec7ae39f2b7 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -1104,9 +1104,15 @@ class Process : public
std::enable_shared_from_this,
/// \see Thread:Resume()
/// \see Thread:Step()
/// \see Thread:Suspend()
- virtual Status DoResume() {
-return Status::FromErrorStringWithFormatv(
-"error: {0} does not support resuming processes", GetPluginName());
+ virtual Status DoResume(lldb::RunDirection direction) {
+if (direction == lldb::RunDirection::eRunForward) {
+ return Status::FromErrorStringWithFormatv(
+ "error: {0} does not support resuming processes", GetPluginName());
+} else {
+ return Status::FromErrorStringWithFormatv(
+ "error: {0} does not support reverse execution of processes",
+ GetPluginName());
+}
}
/// Called after resuming a process.
@@ -2674,6 +2680,11 @@ void PruneThreadPlans();
const AddressRange &range, size_t alignment,
[Lldb-commits] [clang] [lldb] [Clang] Improve Sema diagnostic performance for __builtin_counted_by_ref (PR #116719)
@@ -14690,6 +14690,13 @@ void Sema::FinalizeDeclaration(Decl *ThisDecl) {
}
}
+ // The result of __builtin_counted_by_ref cannot be assigned to a variable.
+ // It allows leaking and modification of bounds safety information.
+ if (IsBuiltinCountedByRef(VD->getInit()))
+Diag(VD->getInit()->getExprLoc(),
+ diag::err_builtin_counted_by_ref_cannot_leak_reference)
+<< VD->getInit()->getSourceRange();
bwendling wrote:
I'm not sure I agree. The diagnostic isn't the same in all places. And one of
them takes arguments not available from the `Expr`. I would say that if we were
adding several more diagnostics then a separate function would make sense. But
for four, it's probably a bit more trouble than it's worth.
https://github.com/llvm/llvm-project/pull/116719
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] build: remove extraneous search paths for LibXml2 (PR #117615)
https://github.com/compnerd edited https://github.com/llvm/llvm-project/pull/117615 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement basic support for reverse-continue (PR #112079)
https://github.com/rocallahan updated
https://github.com/llvm/llvm-project/pull/112079
>From 91d40f5e331b97a202208c221ef6b11784dc8ca2 Mon Sep 17 00:00:00 2001
From: Robert O'Callahan
Date: Fri, 19 Jul 2024 22:46:42 +1200
Subject: [PATCH] [lldb] Implement basic support for reverse-continue
This commit only adds support for the
`SBProcess::ContinueInDirection()` API. A user-accessible command
for this will follow in a later commit.
This feature depends on a gdbserver implementation (e.g. `rr`)
providing support for the `bc` and `bs` packets. `lldb-server`
does not support those packets, and there is no plan to change that.
with a Python implementation of *very limited* record-and-replay
functionality.
---
lldb/include/lldb/API/SBProcess.h | 1 +
lldb/include/lldb/Target/Process.h| 16 +-
lldb/include/lldb/Target/StopInfo.h | 7 +
lldb/include/lldb/Target/Thread.h | 12 +-
lldb/include/lldb/Target/ThreadList.h | 6 +-
lldb/include/lldb/Target/ThreadPlan.h | 4 +
lldb/include/lldb/Target/ThreadPlanBase.h | 2 +
lldb/include/lldb/lldb-enumerations.h | 6 +
.../Python/lldbsuite/test/gdbclientutils.py | 5 +-
.../Python/lldbsuite/test/lldbgdbproxy.py | 184 +++
.../Python/lldbsuite/test/lldbreverse.py | 480 ++
.../Python/lldbsuite/test/lldbtest.py | 2 +
.../tools/lldb-server/lldbgdbserverutils.py | 15 +-
lldb/source/API/SBProcess.cpp | 8 +
lldb/source/API/SBThread.cpp | 2 +
.../source/Interpreter/CommandInterpreter.cpp | 3 +-
.../Process/Linux/NativeThreadLinux.cpp | 3 +
.../Process/MacOSX-Kernel/ProcessKDP.cpp | 9 +-
.../Process/MacOSX-Kernel/ProcessKDP.h| 2 +-
.../Process/Windows/Common/ProcessWindows.cpp | 9 +-
.../Process/Windows/Common/ProcessWindows.h | 2 +-
.../GDBRemoteCommunicationClient.cpp | 22 +
.../gdb-remote/GDBRemoteCommunicationClient.h | 6 +
.../GDBRemoteCommunicationServerLLGS.cpp | 1 +
.../Process/gdb-remote/ProcessGDBRemote.cpp | 88 +++-
.../Process/gdb-remote/ProcessGDBRemote.h | 2 +-
.../Process/scripted/ScriptedProcess.cpp | 10 +-
.../Process/scripted/ScriptedProcess.h| 2 +-
lldb/source/Target/Process.cpp| 25 +-
lldb/source/Target/StopInfo.cpp | 29 ++
lldb/source/Target/Thread.cpp | 13 +-
lldb/source/Target/ThreadList.cpp | 62 ++-
lldb/source/Target/ThreadPlanBase.cpp | 4 +
.../reverse-execution/Makefile| 3 +
.../TestReverseContinueBreakpoints.py | 151 ++
.../TestReverseContinueNotSupported.py| 30 ++
.../functionalities/reverse-execution/main.c | 14 +
lldb/tools/lldb-dap/JSONUtils.cpp | 3 +
lldb/tools/lldb-dap/LLDBUtils.cpp | 1 +
39 files changed, 1173 insertions(+), 71 deletions(-)
create mode 100644 lldb/packages/Python/lldbsuite/test/lldbgdbproxy.py
create mode 100644 lldb/packages/Python/lldbsuite/test/lldbreverse.py
create mode 100644 lldb/test/API/functionalities/reverse-execution/Makefile
create mode 100644
lldb/test/API/functionalities/reverse-execution/TestReverseContinueBreakpoints.py
create mode 100644
lldb/test/API/functionalities/reverse-execution/TestReverseContinueNotSupported.py
create mode 100644 lldb/test/API/functionalities/reverse-execution/main.c
diff --git a/lldb/include/lldb/API/SBProcess.h
b/lldb/include/lldb/API/SBProcess.h
index 1624e02070b1b2..882b8bd837131d 100644
--- a/lldb/include/lldb/API/SBProcess.h
+++ b/lldb/include/lldb/API/SBProcess.h
@@ -159,6 +159,7 @@ class LLDB_API SBProcess {
lldb::SBError Destroy();
lldb::SBError Continue();
+ lldb::SBError ContinueInDirection(lldb::RunDirection direction);
lldb::SBError Stop();
diff --git a/lldb/include/lldb/Target/Process.h
b/lldb/include/lldb/Target/Process.h
index b8c53a474ba6b9..80663d6b1fddcb 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -1104,9 +1104,15 @@ class Process : public
std::enable_shared_from_this,
/// \see Thread:Resume()
/// \see Thread:Step()
/// \see Thread:Suspend()
- virtual Status DoResume() {
-return Status::FromErrorStringWithFormatv(
-"error: {0} does not support resuming processes", GetPluginName());
+ virtual Status DoResume(lldb::RunDirection direction) {
+if (direction == lldb::RunDirection::eRunForward) {
+ return Status::FromErrorStringWithFormatv(
+ "error: {0} does not support resuming processes", GetPluginName());
+} else {
+ return Status::FromErrorStringWithFormatv(
+ "error: {0} does not support reverse execution of processes",
+ GetPluginName());
+}
}
/// Called after resuming a process.
@@ -2674,6 +2680,9 @@ void PruneThreadPlans();
const AddressRange &range, size_t alignment,
[Lldb-commits] [lldb] [lldb] Implement basic support for reverse-continue (PR #112079)
https://github.com/rocallahan updated
https://github.com/llvm/llvm-project/pull/112079
>From c0904fed59e42d957e2b40698c4343fdb5582b21 Mon Sep 17 00:00:00 2001
From: Robert O'Callahan
Date: Fri, 19 Jul 2024 22:46:42 +1200
Subject: [PATCH] [lldb] Implement basic support for reverse-continue
This commit only adds support for the
`SBProcess::ContinueInDirection()` API. A user-accessible command
for this will follow in a later commit.
This feature depends on a gdbserver implementation (e.g. `rr`)
providing support for the `bc` and `bs` packets. `lldb-server`
does not support those packets, and there is no plan to change that.
with a Python implementation of *very limited* record-and-replay
functionality.
---
lldb/include/lldb/API/SBProcess.h | 1 +
lldb/include/lldb/Target/Process.h| 16 +-
lldb/include/lldb/Target/StopInfo.h | 7 +
lldb/include/lldb/Target/Thread.h | 12 +-
lldb/include/lldb/Target/ThreadList.h | 6 +-
lldb/include/lldb/Target/ThreadPlan.h | 4 +
lldb/include/lldb/Target/ThreadPlanBase.h | 2 +
lldb/include/lldb/lldb-enumerations.h | 6 +
.../Python/lldbsuite/test/gdbclientutils.py | 5 +-
.../Python/lldbsuite/test/lldbgdbproxy.py | 185 +++
.../Python/lldbsuite/test/lldbreverse.py | 489 ++
.../Python/lldbsuite/test/lldbtest.py | 2 +
.../tools/lldb-server/lldbgdbserverutils.py | 15 +-
lldb/source/API/SBProcess.cpp | 8 +
lldb/source/API/SBThread.cpp | 2 +
.../source/Interpreter/CommandInterpreter.cpp | 3 +-
.../Process/Linux/NativeThreadLinux.cpp | 3 +
.../Process/MacOSX-Kernel/ProcessKDP.cpp | 9 +-
.../Process/MacOSX-Kernel/ProcessKDP.h| 2 +-
.../Process/Windows/Common/ProcessWindows.cpp | 9 +-
.../Process/Windows/Common/ProcessWindows.h | 2 +-
.../GDBRemoteCommunicationClient.cpp | 22 +
.../gdb-remote/GDBRemoteCommunicationClient.h | 6 +
.../GDBRemoteCommunicationServerLLGS.cpp | 1 +
.../Process/gdb-remote/ProcessGDBRemote.cpp | 88 +++-
.../Process/gdb-remote/ProcessGDBRemote.h | 2 +-
.../Process/scripted/ScriptedProcess.cpp | 10 +-
.../Process/scripted/ScriptedProcess.h| 2 +-
lldb/source/Target/Process.cpp| 25 +-
lldb/source/Target/StopInfo.cpp | 29 ++
lldb/source/Target/Thread.cpp | 13 +-
lldb/source/Target/ThreadList.cpp | 62 ++-
lldb/source/Target/ThreadPlanBase.cpp | 4 +
.../reverse-execution/Makefile| 3 +
.../TestReverseContinueBreakpoints.py | 151 ++
.../TestReverseContinueNotSupported.py| 30 ++
.../functionalities/reverse-execution/main.c | 14 +
lldb/tools/lldb-dap/JSONUtils.cpp | 3 +
lldb/tools/lldb-dap/LLDBUtils.cpp | 1 +
39 files changed, 1183 insertions(+), 71 deletions(-)
create mode 100644 lldb/packages/Python/lldbsuite/test/lldbgdbproxy.py
create mode 100644 lldb/packages/Python/lldbsuite/test/lldbreverse.py
create mode 100644 lldb/test/API/functionalities/reverse-execution/Makefile
create mode 100644
lldb/test/API/functionalities/reverse-execution/TestReverseContinueBreakpoints.py
create mode 100644
lldb/test/API/functionalities/reverse-execution/TestReverseContinueNotSupported.py
create mode 100644 lldb/test/API/functionalities/reverse-execution/main.c
diff --git a/lldb/include/lldb/API/SBProcess.h
b/lldb/include/lldb/API/SBProcess.h
index 1624e02070b1b2..882b8bd837131d 100644
--- a/lldb/include/lldb/API/SBProcess.h
+++ b/lldb/include/lldb/API/SBProcess.h
@@ -159,6 +159,7 @@ class LLDB_API SBProcess {
lldb::SBError Destroy();
lldb::SBError Continue();
+ lldb::SBError ContinueInDirection(lldb::RunDirection direction);
lldb::SBError Stop();
diff --git a/lldb/include/lldb/Target/Process.h
b/lldb/include/lldb/Target/Process.h
index b8c53a474ba6b9..80663d6b1fddcb 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -1104,9 +1104,15 @@ class Process : public
std::enable_shared_from_this,
/// \see Thread:Resume()
/// \see Thread:Step()
/// \see Thread:Suspend()
- virtual Status DoResume() {
-return Status::FromErrorStringWithFormatv(
-"error: {0} does not support resuming processes", GetPluginName());
+ virtual Status DoResume(lldb::RunDirection direction) {
+if (direction == lldb::RunDirection::eRunForward) {
+ return Status::FromErrorStringWithFormatv(
+ "error: {0} does not support resuming processes", GetPluginName());
+} else {
+ return Status::FromErrorStringWithFormatv(
+ "error: {0} does not support reverse execution of processes",
+ GetPluginName());
+}
}
/// Called after resuming a process.
@@ -2674,6 +2680,9 @@ void PruneThreadPlans();
const AddressRange &range, size_t alignment,
[Lldb-commits] [lldb] [lldb][Mach-O] Handle shared cache binaries correctly (PR #117832)
https://github.com/jasonmolenda created
https://github.com/llvm/llvm-project/pull/117832
The Mach-O load commands have an LC_SYMTAB / struct symtab_command which
represents the offset of the symbol table (nlist records) and string table for
this binary. In a mach-o binary on disk, these are file offsets. If a mach-o
binary is loaded in memory with all segments consecutive, the `symoff` and
`stroff` are the offsets from the TEXT segment (aka the mach-o header) virtual
address to the virtual address of the start of these tables.
However, if a Mach-O binary is a part of the shared cache, then the segments
will be separated -- they will have different slide values. And it is possible
for the LINKEDIT segment to be greater than 4GB away from the TEXT segment in
the virtual address space, so these 32-bit offsets cannot express the offset
from TEXT segment to these tables.
Create separate uint64_t variables to track the offset to the symbol table and
string table, instead of reusing the 32-bit ones in the symtab_command
structure.
rdar://140432279
>From 00a429c14d159ebc42ac7c3a7e98a91851ece236 Mon Sep 17 00:00:00 2001
From: Jason Molenda
Date: Tue, 26 Nov 2024 17:56:06 -0800
Subject: [PATCH] [lldb][Mach-O] Handle shared cache binaries correctly
The Mach-O load commands have an LC_SYMTAB / struct symtab_command
which represents the offset of the symbol table (nlist records) and
string table for this binary. In a mach-o binary on disk, these are
file offsets. If a mach-o binary is loaded in memory with all
segments consecutive, the `symoff` and `stroff` are the offsets from
the TEXT segment (aka the mach-o header) virtual address to the
virtual address of the start of these tables.
However, if a Mach-O binary is a part of the shared cache, then the
segments will be separated -- they will have different slide values.
And it is possible for the LINKEDIT segment to be greater than 4GB
away from the TEXT segment in the virtual address space, so these
32-bit offsets cannot express the offset from TEXT segment to these
tables.
Create separate uint64_t variables to track the offset to the
symbol table and string table, instead of reusing the 32-bit ones
in the symtab_command structure.
rdar://140432279
---
.../ObjectFile/Mach-O/ObjectFileMachO.cpp | 26 ++-
1 file changed, 20 insertions(+), 6 deletions(-)
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 079fd905037d45..5f047d84d53e73 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -2244,6 +2244,18 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
// code.
typedef AddressDataArray FunctionStarts;
+ // The virtual address offset from TEXT to the symbol/string tables
+ // in the LINKEDIT section. The LC_SYMTAB symtab_command `symoff` and
+ // `stroff` are uint32_t's that give the file offset in the binary.
+ // If the binary is laid down in memory with all segments consecutive,
+ // then these are the offsets from the mach-o header aka TEXT segment
+ // to the tables' virtual addresses.
+ // But if the binary is loaded in virtual address space with different
+ // slides for the segments (e.g. a shared cache), the LINKEDIT may be
+ // more than 4GB away from TEXT, and a 32-bit offset is not sufficient.
+ offset_t symbol_table_offset_from_TEXT = 0;
+ offset_t string_table_offset_from_TEXT = 0;
+
// Record the address of every function/data that we add to the symtab.
// We add symbols to the table in the order of most information (nlist
// records) to least (function starts), and avoid duplicating symbols
@@ -2282,6 +2294,8 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
if (m_data.GetU32(&offset, &symtab_load_command.symoff, 4) ==
nullptr) // fill in symoff, nsyms, stroff, strsize fields
return;
+ string_table_offset_from_TEXT = symtab_load_command.stroff;
+ symbol_table_offset_from_TEXT = symtab_load_command.symoff;
break;
case LC_DYLD_INFO:
@@ -2403,9 +2417,9 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
const addr_t linkedit_file_offset = linkedit_section_sp->GetFileOffset();
const addr_t symoff_addr = linkedit_load_addr +
- symtab_load_command.symoff -
+ symbol_table_offset_from_TEXT -
linkedit_file_offset;
- strtab_addr = linkedit_load_addr + symtab_load_command.stroff -
+ strtab_addr = linkedit_load_addr + string_table_offset_from_TEXT -
linkedit_file_offset;
// Always load dyld - the dynamic linker - from memory if we didn't
@@ -2473,17 +2487,17 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
lldb::addr_t linkedit_offset = linkedit_section_sp->GetFileOffset();
lldb::offset_t
[Lldb-commits] [lldb] [lldb][Mach-O] Handle shared cache binaries correctly (PR #117832)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Jason Molenda (jasonmolenda)
Changes
The Mach-O load commands have an LC_SYMTAB / struct symtab_command which
represents the offset of the symbol table (nlist records) and string table for
this binary. In a mach-o binary on disk, these are file offsets. If a mach-o
binary is loaded in memory with all segments consecutive, the `symoff` and
`stroff` are the offsets from the TEXT segment (aka the mach-o header) virtual
address to the virtual address of the start of these tables.
However, if a Mach-O binary is a part of the shared cache, then the segments
will be separated -- they will have different slide values. And it is possible
for the LINKEDIT segment to be greater than 4GB away from the TEXT segment in
the virtual address space, so these 32-bit offsets cannot express the offset
from TEXT segment to these tables.
Create separate uint64_t variables to track the offset to the symbol table and
string table, instead of reusing the 32-bit ones in the symtab_command
structure.
rdar://140432279
---
Full diff: https://github.com/llvm/llvm-project/pull/117832.diff
1 Files Affected:
- (modified) lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp (+20-6)
``diff
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 079fd905037d45..5f047d84d53e73 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -2244,6 +2244,18 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
// code.
typedef AddressDataArray FunctionStarts;
+ // The virtual address offset from TEXT to the symbol/string tables
+ // in the LINKEDIT section. The LC_SYMTAB symtab_command `symoff` and
+ // `stroff` are uint32_t's that give the file offset in the binary.
+ // If the binary is laid down in memory with all segments consecutive,
+ // then these are the offsets from the mach-o header aka TEXT segment
+ // to the tables' virtual addresses.
+ // But if the binary is loaded in virtual address space with different
+ // slides for the segments (e.g. a shared cache), the LINKEDIT may be
+ // more than 4GB away from TEXT, and a 32-bit offset is not sufficient.
+ offset_t symbol_table_offset_from_TEXT = 0;
+ offset_t string_table_offset_from_TEXT = 0;
+
// Record the address of every function/data that we add to the symtab.
// We add symbols to the table in the order of most information (nlist
// records) to least (function starts), and avoid duplicating symbols
@@ -2282,6 +2294,8 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
if (m_data.GetU32(&offset, &symtab_load_command.symoff, 4) ==
nullptr) // fill in symoff, nsyms, stroff, strsize fields
return;
+ string_table_offset_from_TEXT = symtab_load_command.stroff;
+ symbol_table_offset_from_TEXT = symtab_load_command.symoff;
break;
case LC_DYLD_INFO:
@@ -2403,9 +2417,9 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
const addr_t linkedit_file_offset = linkedit_section_sp->GetFileOffset();
const addr_t symoff_addr = linkedit_load_addr +
- symtab_load_command.symoff -
+ symbol_table_offset_from_TEXT -
linkedit_file_offset;
- strtab_addr = linkedit_load_addr + symtab_load_command.stroff -
+ strtab_addr = linkedit_load_addr + string_table_offset_from_TEXT -
linkedit_file_offset;
// Always load dyld - the dynamic linker - from memory if we didn't
@@ -2473,17 +2487,17 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
lldb::addr_t linkedit_offset = linkedit_section_sp->GetFileOffset();
lldb::offset_t linkedit_slide =
linkedit_offset - m_linkedit_original_offset;
- symtab_load_command.symoff += linkedit_slide;
- symtab_load_command.stroff += linkedit_slide;
+ symbol_table_offset_from_TEXT += linkedit_slide;
+ string_table_offset_from_TEXT += linkedit_slide;
dyld_info.export_off += linkedit_slide;
dysymtab.indirectsymoff += linkedit_slide;
function_starts_load_command.dataoff += linkedit_slide;
exports_trie_load_command.dataoff += linkedit_slide;
}
-nlist_data.SetData(m_data, symtab_load_command.symoff,
+nlist_data.SetData(m_data, symbol_table_offset_from_TEXT,
nlist_data_byte_size);
-strtab_data.SetData(m_data, symtab_load_command.stroff,
+strtab_data.SetData(m_data, string_table_offset_from_TEXT,
strtab_data_byte_size);
// We shouldn't have exports data from both the LC_DYLD_INFO command
``
https://github.com/llvm/llvm-project/pull/117832
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https
[Lldb-commits] [lldb] [lldb][Mach-O] Handle shared cache binaries correctly (PR #117832)
jasonmolenda wrote: I have two criticisms of the patch as-is. We read the LC_SYMTAB load command into `symtab_load_command` which has six fields, the first two being the usual `cmd` and `cmdsize`. We use all of the next four: `ncmds`, `strsize`, `symoff`, `stroff`. I'm taking the uses of two of these fields out of `symtab_load_command`, and putting them in 64-bit locals. But I think the smarter move is probably to read the load command into a temporary object, and then create four local variables with the resized fields as needed, or have a locally defined struct with the resized fields. The second criticism is that I don't have a good way to test it. I have a request to have ProcessMachCore treat an LC_SEGMENT that has a virtual address & size, but no file size as an all-zeroes segment. In which case it would be possible to create a mach-o corefile that is larger than 4GB in size, but actually only uses a couple hundred kb on disk (and doesn't fill the CI filesystems), and then we'd have to hand-write a mach-o file with a LINKEDIT 4GB away from the TEXT segment. There's a couple pieces that don't exist to do all of this right now, though. https://github.com/llvm/llvm-project/pull/117832 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] f2129ca - [lldb][NFC] Whitespace fix for mis-indented block
Author: Jason Molenda
Date: 2024-11-26T17:00:35-08:00
New Revision: f2129ca94c47875b5f915abc9d7dfb02a7445fe6
URL:
https://github.com/llvm/llvm-project/commit/f2129ca94c47875b5f915abc9d7dfb02a7445fe6
DIFF:
https://github.com/llvm/llvm-project/commit/f2129ca94c47875b5f915abc9d7dfb02a7445fe6.diff
LOG: [lldb][NFC] Whitespace fix for mis-indented block
This mis-indented block makes a FC change I'm about
to propose look larger than it is when clang-formatted.
Added:
Modified:
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
Removed:
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 85edf4bd5494ae..079fd905037d45 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -2408,45 +2408,44 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
strtab_addr = linkedit_load_addr + symtab_load_command.stroff -
linkedit_file_offset;
-// Always load dyld - the dynamic linker - from memory if we didn't
-// find a binary anywhere else. lldb will not register
-// dylib/framework/bundle loads/unloads if we don't have the dyld
-// symbols, we force dyld to load from memory despite the user's
-// target.memory-module-load-level setting.
-if (memory_module_load_level == eMemoryModuleLoadLevelComplete ||
-m_header.filetype == llvm::MachO::MH_DYLINKER) {
- DataBufferSP nlist_data_sp(
- ReadMemory(process_sp, symoff_addr, nlist_data_byte_size));
- if (nlist_data_sp)
-nlist_data.SetData(nlist_data_sp, 0, nlist_data_sp->GetByteSize());
- if (dysymtab.nindirectsyms != 0) {
-const addr_t indirect_syms_addr = linkedit_load_addr +
- dysymtab.indirectsymoff -
- linkedit_file_offset;
-DataBufferSP indirect_syms_data_sp(ReadMemory(
-process_sp, indirect_syms_addr, dysymtab.nindirectsyms * 4));
-if (indirect_syms_data_sp)
- indirect_symbol_index_data.SetData(
- indirect_syms_data_sp, 0,
- indirect_syms_data_sp->GetByteSize());
-// If this binary is outside the shared cache,
-// cache the string table.
-// Binaries in the shared cache all share a giant string table,
-// and we can't share the string tables across multiple
-// ObjectFileMachO's, so we'd end up re-reading this mega-strtab
-// for every binary in the shared cache - it would be a big perf
-// problem. For binaries outside the shared cache, it's faster to
-// read the entire strtab at once instead of piece-by-piece as we
-// process the nlist records.
-if (!is_shared_cache_image) {
- DataBufferSP strtab_data_sp(
- ReadMemory(process_sp, strtab_addr, strtab_data_byte_size));
- if (strtab_data_sp) {
-strtab_data.SetData(strtab_data_sp, 0,
-strtab_data_sp->GetByteSize());
- }
+ // Always load dyld - the dynamic linker - from memory if we didn't
+ // find a binary anywhere else. lldb will not register
+ // dylib/framework/bundle loads/unloads if we don't have the dyld
+ // symbols, we force dyld to load from memory despite the user's
+ // target.memory-module-load-level setting.
+ if (memory_module_load_level == eMemoryModuleLoadLevelComplete ||
+ m_header.filetype == llvm::MachO::MH_DYLINKER) {
+DataBufferSP nlist_data_sp(
+ReadMemory(process_sp, symoff_addr, nlist_data_byte_size));
+if (nlist_data_sp)
+ nlist_data.SetData(nlist_data_sp, 0, nlist_data_sp->GetByteSize());
+if (dysymtab.nindirectsyms != 0) {
+ const addr_t indirect_syms_addr = linkedit_load_addr +
+dysymtab.indirectsymoff -
+linkedit_file_offset;
+ DataBufferSP indirect_syms_data_sp(ReadMemory(
+ process_sp, indirect_syms_addr, dysymtab.nindirectsyms * 4));
+ if (indirect_syms_data_sp)
+indirect_symbol_index_data.SetData(
+indirect_syms_data_sp, 0,
indirect_syms_data_sp->GetByteSize());
+ // If this binary is outside the shared cache,
+ // cache the string table.
+ // Binaries in the shared cache all share a giant string table,
+ // and we can't share the string tables across multiple
+ // ObjectFileMachO's, so we'd end up re-reading this mega-strtab
+ // for every binary in the shared cache - it wo
[Lldb-commits] [lldb] [lldb] Make sure Blocks always have a parent (PR #117683)
https://github.com/labath created
https://github.com/llvm/llvm-project/pull/117683
It's basically true already (except for a brief time during construction). This
patch makes sure the objects are constructed with a valid parent and enforces
this in the type system, which allows us to get rid of some nullptr checks.
>From 458e98135c15550ba6cd140aee8582c19e6a8c8d Mon Sep 17 00:00:00 2001
From: Pavel Labath
Date: Tue, 26 Nov 2024 09:03:01 +0100
Subject: [PATCH] [lldb] Make sure Blocks always have a parent
It's basically true already (except for a brief time during
construction). This patch makes sure the objects are constructed with a
valid parent and enforces this in the type system, which allows us to
get rid of some nullptr checks.
---
lldb/include/lldb/Symbol/Block.h | 34 +-
.../Breakpad/SymbolFileBreakpad.cpp | 4 +-
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 4 +-
.../NativePDB/SymbolFileNativePDB.cpp | 6 +--
.../Plugins/SymbolFile/PDB/SymbolFilePDB.cpp | 4 +-
lldb/source/Symbol/Block.cpp | 47 +++
lldb/source/Symbol/Function.cpp | 3 +-
7 files changed, 36 insertions(+), 66 deletions(-)
diff --git a/lldb/include/lldb/Symbol/Block.h b/lldb/include/lldb/Symbol/Block.h
index c9c4d5ad767d7e..7c7a66de831998 100644
--- a/lldb/include/lldb/Symbol/Block.h
+++ b/lldb/include/lldb/Symbol/Block.h
@@ -43,11 +43,13 @@ class Block : public UserID, public SymbolContextScope {
typedef RangeVector RangeList;
typedef RangeList::Entry Range;
- /// Construct with a User ID \a uid, \a depth.
- ///
- /// Initialize this block with the specified UID \a uid. The \a depth in the
- /// \a block_list is used to represent the parent, sibling, and child block
- /// information and also allows for partial parsing at the block level.
+ // Creates a block representing the whole function. Only meant to be used
from
+ // the Function class.
+ Block(Function &function, lldb::user_id_t function_uid);
+
+ ~Block() override;
+
+ /// Creates a block with the specified UID \a uid.
///
/// \param[in] uid
/// The UID for a given block. This value is given by the
@@ -56,19 +58,7 @@ class Block : public UserID, public SymbolContextScope {
/// information data that it parses for further or more in
/// depth parsing. Common values would be the index into a
/// table, or an offset into the debug information.
- ///
- /// \see BlockList
- Block(lldb::user_id_t uid);
-
- /// Destructor.
- ~Block() override;
-
- /// Add a child to this object.
- ///
- /// \param[in] child_block_sp
- /// A shared pointer to a child block that will get added to
- /// this block.
- void AddChild(const lldb::BlockSP &child_block_sp);
+ lldb::BlockSP CreateChild(lldb::user_id_t uid);
/// Add a new offset range to this block.
void AddRange(const Range &range);
@@ -317,10 +307,6 @@ class Block : public UserID, public SymbolContextScope {
const Declaration *decl_ptr,
const Declaration *call_decl_ptr);
- void SetParentScope(SymbolContextScope *parent_scope) {
-m_parent_scope = parent_scope;
- }
-
/// Set accessor for the variable list.
///
/// Called by the SymbolFile plug-ins after they have parsed the variable
@@ -364,7 +350,7 @@ class Block : public UserID, public SymbolContextScope {
protected:
typedef std::vector collection;
// Member variables.
- SymbolContextScope *m_parent_scope;
+ SymbolContextScope &m_parent_scope;
collection m_children;
RangeList m_ranges;
lldb::InlineFunctionInfoSP m_inlineInfoSP; ///< Inlined function information.
@@ -382,6 +368,8 @@ class Block : public UserID, public SymbolContextScope {
private:
Block(const Block &) = delete;
const Block &operator=(const Block &) = delete;
+
+ Block(lldb::user_id_t uid, SymbolContextScope &parent_scope);
};
} // namespace lldb_private
diff --git a/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
b/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
index fadc19676609bf..df3bf157278daf 100644
--- a/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
+++ b/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
@@ -315,7 +315,8 @@ size_t SymbolFileBreakpad::ParseBlocksRecursive(Function
&func) {
if (record->InlineNestLevel == 0 ||
record->InlineNestLevel <= last_added_nest_level + 1) {
last_added_nest_level = record->InlineNestLevel;
-BlockSP block_sp = std::make_shared(It.GetBookmark().offset);
+BlockSP block_sp = blocks[record->InlineNestLevel]->CreateChild(
+It.GetBookmark().offset);
FileSpec callsite_file;
if (record->CallSiteFileNum < m_files->size())
callsite_file = (*m_files)[record->CallSiteFileNum];
@@ -333,7 +334,6 @@ size_t SymbolFileBreakpad::ParseBlocksRecursive(Function
&fun
[Lldb-commits] [lldb] [lldb] Make sure Blocks always have a parent (PR #117683)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Pavel Labath (labath)
Changes
It's basically true already (except for a brief time during construction). This
patch makes sure the objects are constructed with a valid parent and enforces
this in the type system, which allows us to get rid of some nullptr checks.
---
Full diff: https://github.com/llvm/llvm-project/pull/117683.diff
7 Files Affected:
- (modified) lldb/include/lldb/Symbol/Block.h (+11-23)
- (modified) lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
(+2-2)
- (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (+1-3)
- (modified) lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
(+2-4)
- (modified) lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp (+1-3)
- (modified) lldb/source/Symbol/Block.cpp (+18-29)
- (modified) lldb/source/Symbol/Function.cpp (+1-2)
``diff
diff --git a/lldb/include/lldb/Symbol/Block.h b/lldb/include/lldb/Symbol/Block.h
index c9c4d5ad767d7e..7c7a66de831998 100644
--- a/lldb/include/lldb/Symbol/Block.h
+++ b/lldb/include/lldb/Symbol/Block.h
@@ -43,11 +43,13 @@ class Block : public UserID, public SymbolContextScope {
typedef RangeVector RangeList;
typedef RangeList::Entry Range;
- /// Construct with a User ID \a uid, \a depth.
- ///
- /// Initialize this block with the specified UID \a uid. The \a depth in the
- /// \a block_list is used to represent the parent, sibling, and child block
- /// information and also allows for partial parsing at the block level.
+ // Creates a block representing the whole function. Only meant to be used
from
+ // the Function class.
+ Block(Function &function, lldb::user_id_t function_uid);
+
+ ~Block() override;
+
+ /// Creates a block with the specified UID \a uid.
///
/// \param[in] uid
/// The UID for a given block. This value is given by the
@@ -56,19 +58,7 @@ class Block : public UserID, public SymbolContextScope {
/// information data that it parses for further or more in
/// depth parsing. Common values would be the index into a
/// table, or an offset into the debug information.
- ///
- /// \see BlockList
- Block(lldb::user_id_t uid);
-
- /// Destructor.
- ~Block() override;
-
- /// Add a child to this object.
- ///
- /// \param[in] child_block_sp
- /// A shared pointer to a child block that will get added to
- /// this block.
- void AddChild(const lldb::BlockSP &child_block_sp);
+ lldb::BlockSP CreateChild(lldb::user_id_t uid);
/// Add a new offset range to this block.
void AddRange(const Range &range);
@@ -317,10 +307,6 @@ class Block : public UserID, public SymbolContextScope {
const Declaration *decl_ptr,
const Declaration *call_decl_ptr);
- void SetParentScope(SymbolContextScope *parent_scope) {
-m_parent_scope = parent_scope;
- }
-
/// Set accessor for the variable list.
///
/// Called by the SymbolFile plug-ins after they have parsed the variable
@@ -364,7 +350,7 @@ class Block : public UserID, public SymbolContextScope {
protected:
typedef std::vector collection;
// Member variables.
- SymbolContextScope *m_parent_scope;
+ SymbolContextScope &m_parent_scope;
collection m_children;
RangeList m_ranges;
lldb::InlineFunctionInfoSP m_inlineInfoSP; ///< Inlined function information.
@@ -382,6 +368,8 @@ class Block : public UserID, public SymbolContextScope {
private:
Block(const Block &) = delete;
const Block &operator=(const Block &) = delete;
+
+ Block(lldb::user_id_t uid, SymbolContextScope &parent_scope);
};
} // namespace lldb_private
diff --git a/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
b/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
index fadc19676609bf..df3bf157278daf 100644
--- a/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
+++ b/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
@@ -315,7 +315,8 @@ size_t SymbolFileBreakpad::ParseBlocksRecursive(Function
&func) {
if (record->InlineNestLevel == 0 ||
record->InlineNestLevel <= last_added_nest_level + 1) {
last_added_nest_level = record->InlineNestLevel;
-BlockSP block_sp = std::make_shared(It.GetBookmark().offset);
+BlockSP block_sp = blocks[record->InlineNestLevel]->CreateChild(
+It.GetBookmark().offset);
FileSpec callsite_file;
if (record->CallSiteFileNum < m_files->size())
callsite_file = (*m_files)[record->CallSiteFileNum];
@@ -333,7 +334,6 @@ size_t SymbolFileBreakpad::ParseBlocksRecursive(Function
&func) {
}
block_sp->FinalizeRanges();
-blocks[record->InlineNestLevel]->AddChild(block_sp);
if (record->InlineNestLevel + 1 >= blocks.size()) {
blocks.resize(blocks.size() + 1);
}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
[Lldb-commits] [clang] [lldb] [llvm] Extending LLDB to work on AIX (PR #102601)
DhruvSrivastavaX wrote: > BTW, the main loop changes are something that you should be able to upstream > quite easily. It sounds like the main problem is that you don't have a ppoll > syscall, but the last round of changes means that ppoll is not actually > necessary for its operation (the only reason to use it is the increased > precision) Hmm. That is good idea too. I think we can work on that. Thanks!! https://github.com/llvm/llvm-project/pull/102601 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make sure Blocks always have a parent (PR #117683)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/117683 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make sure Blocks always have a parent (PR #117683)
@@ -399,11 +392,9 @@ size_t Block::MemorySize() const {
return mem_size;
}
-void Block::AddChild(const BlockSP &child_block_sp) {
- if (child_block_sp) {
-child_block_sp->SetParentScope(this);
-m_children.push_back(child_block_sp);
- }
+BlockSP Block::CreateChild(user_id_t uid) {
+ m_children.push_back(std::shared_ptr(new Block(uid, *this)));
DavidSpickett wrote:
std::make_shared to save a few characters?
https://github.com/llvm/llvm-project/pull/117683
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make sure Blocks always have a parent (PR #117683)
https://github.com/DavidSpickett approved this pull request. Some small things you can address if you want but otherwise LGTM. https://github.com/llvm/llvm-project/pull/117683 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make sure Blocks always have a parent (PR #117683)
@@ -43,11 +43,13 @@ class Block : public UserID, public SymbolContextScope {
typedef RangeVector RangeList;
typedef RangeList::Entry Range;
- /// Construct with a User ID \a uid, \a depth.
- ///
- /// Initialize this block with the specified UID \a uid. The \a depth in the
- /// \a block_list is used to represent the parent, sibling, and child block
- /// information and also allows for partial parsing at the block level.
+ // Creates a block representing the whole function. Only meant to be used
from
+ // the Function class.
DavidSpickett wrote:
Can you enforce that restriction using `friend`, or does it get into a mess of
circular includes?
https://github.com/llvm/llvm-project/pull/117683
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] build: enable CONFIG mode search for LibXml2 for LLDB (PR #117615)
@@ -57,7 +57,17 @@ add_optional_dependency(LLDB_ENABLE_CURSES "Enable curses support in LLDB" Curse add_optional_dependency(LLDB_ENABLE_LZMA "Enable LZMA compression support in LLDB" LibLZMA LIBLZMA_FOUND) add_optional_dependency(LLDB_ENABLE_LUA "Enable Lua scripting support in LLDB" LuaAndSwig LUAANDSWIG_FOUND) add_optional_dependency(LLDB_ENABLE_PYTHON "Enable Python scripting support in LLDB" PythonAndSwig PYTHONANDSWIG_FOUND) -add_optional_dependency(LLDB_ENABLE_LIBXML2 "Enable Libxml 2 support in LLDB" LibXml2 LIBXML2_FOUND VERSION 2.8) +# LibXml2 is required if LLDB_ENABLE_LIBXML2 is ON or AUTO. The LibXml2 package +# search is not setup properly to support the CONFIG mode search. Furthermore, DavidSpickett wrote: "The LibXml2 package search is not setup properly to support the CONFIG mode search." Could you clarify who has not set this up properly? LLDB's CMake configuration or libxml2 itself? https://github.com/llvm/llvm-project/pull/117615 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] build: enable CONFIG mode search for LibXml2 for LLDB (PR #117615)
@@ -57,7 +57,17 @@ add_optional_dependency(LLDB_ENABLE_CURSES "Enable curses support in LLDB" Curse add_optional_dependency(LLDB_ENABLE_LZMA "Enable LZMA compression support in LLDB" LibLZMA LIBLZMA_FOUND) add_optional_dependency(LLDB_ENABLE_LUA "Enable Lua scripting support in LLDB" LuaAndSwig LUAANDSWIG_FOUND) add_optional_dependency(LLDB_ENABLE_PYTHON "Enable Python scripting support in LLDB" PythonAndSwig PYTHONANDSWIG_FOUND) -add_optional_dependency(LLDB_ENABLE_LIBXML2 "Enable Libxml 2 support in LLDB" LibXml2 LIBXML2_FOUND VERSION 2.8) +# LibXml2 is required if LLDB_ENABLE_LIBXML2 is ON or AUTO. The LibXml2 package +# search is not setup properly to support the CONFIG mode search. Furthermore, DavidSpickett wrote: Also perhaps this comment would be better if it started with "ideally we would do X" so that in future when whatever the problem is is maybe fixed, we have some idea of what we can change this to. It sounds like this CONFIG mode *kinda* works with libxml2 right now but not totally? https://github.com/llvm/llvm-project/pull/117615 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add timeout argument to Socket::Accept (PR #117691)
https://github.com/labath created
https://github.com/llvm/llvm-project/pull/117691
Allows us to stop waiting for a connection if it doesn't come in a certain
amount of time. Right now, I'm keeping the status quo (infitnite wait) in the
"production" code, but using smaller (finite) values in tests. (A lot of these
tests create "loopback" connections, where a really short wait is sufficient:
on linux at least even a poll (0s wait) is sufficient if the other end has
connect()ed already, but this doesn't seem to be the case on Windows, so I'm
using a 1s wait in these cases).
>From 62887d675847c6ce827a4a7ec959aa747ad52329 Mon Sep 17 00:00:00 2001
From: Pavel Labath
Date: Tue, 19 Nov 2024 13:52:31 +0100
Subject: [PATCH] [lldb] Add timeout argument to Socket::Accept
Allows us to stop waiting for a connection if it doesn't come in a
certain amount of time. Right now, I'm keeping the status quo (infitnite
wait) in the "production" code, but using smaller (finite) values in
tests. (A lot of these tests create "loopback" connections, where a
really short wait is sufficient: on linux at least even a poll (0s wait)
is sufficient if the other end has connect()ed already, but this doesn't
seem to be the case on Windows, so I'm using a 1s wait in these cases).
---
lldb/include/lldb/Host/Socket.h | 3 ++-
lldb/source/Host/common/Socket.cpp| 13 +--
.../posix/ConnectionFileDescriptorPosix.cpp | 2 +-
.../gdb-remote/GDBRemoteCommunication.cpp | 15 ++--
lldb/unittests/Host/MainLoopTest.cpp | 3 ++-
lldb/unittests/Host/SocketTest.cpp| 23 +++
.../Host/SocketTestUtilities.cpp | 19 +--
.../tools/lldb-server/tests/TestClient.cpp| 11 +++--
8 files changed, 62 insertions(+), 27 deletions(-)
diff --git a/lldb/include/lldb/Host/Socket.h b/lldb/include/lldb/Host/Socket.h
index 14468c98ac5a3a..0e542b05a085c6 100644
--- a/lldb/include/lldb/Host/Socket.h
+++ b/lldb/include/lldb/Host/Socket.h
@@ -13,6 +13,7 @@
#include
#include "lldb/Host/MainLoopBase.h"
+#include "lldb/Utility/Timeout.h"
#include "lldb/lldb-private.h"
#include "lldb/Host/SocketAddress.h"
@@ -108,7 +109,7 @@ class Socket : public IOObject {
// Accept a single connection and "return" it in the pointer argument. This
// function blocks until the connection arrives.
- virtual Status Accept(Socket *&socket);
+ virtual Status Accept(const Timeout &timeout, Socket *&socket);
// Initialize a Tcp Socket object in listening mode. listen and accept are
// implemented separately because the caller may wish to manipulate or query
diff --git a/lldb/source/Host/common/Socket.cpp
b/lldb/source/Host/common/Socket.cpp
index d69eb608204033..63396f7b4abc9c 100644
--- a/lldb/source/Host/common/Socket.cpp
+++ b/lldb/source/Host/common/Socket.cpp
@@ -460,7 +460,8 @@ NativeSocket Socket::CreateSocket(const int domain, const
int type,
return sock;
}
-Status Socket::Accept(Socket *&socket) {
+Status Socket::Accept(const Timeout &timeout, Socket *&socket) {
+ socket = nullptr;
MainLoop accept_loop;
llvm::Expected> expected_handles =
Accept(accept_loop,
@@ -470,7 +471,15 @@ Status Socket::Accept(Socket *&socket) {
});
if (!expected_handles)
return Status::FromError(expected_handles.takeError());
- return accept_loop.Run();
+ if (timeout) {
+accept_loop.AddCallback(
+[](MainLoopBase &loop) { loop.RequestTermination(); }, *timeout);
+ }
+ if (Status status = accept_loop.Run(); status.Fail())
+return status;
+ if (socket)
+return Status();
+ return Status(std::make_error_code(std::errc::timed_out));
}
NativeSocket Socket::AcceptSocket(NativeSocket sockfd, struct sockaddr *addr,
diff --git a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
index 8a03e47ef3d900..903bfc50def3aa 100644
--- a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -543,7 +543,7 @@ lldb::ConnectionStatus
ConnectionFileDescriptor::AcceptSocket(
if (!error.Fail()) {
post_listen_callback(*listening_socket);
-error = listening_socket->Accept(accepted_socket);
+error = listening_socket->Accept(/*timeout=*/std::nullopt,
accepted_socket);
}
if (!error.Fail()) {
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
index 7eacd605362e7c..67b41b1e77a533 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -1223,10 +1223,6 @@
GDBRemoteCommunication::ConnectLocally(GDBRemoteCommunication &client,
listen_socket.Listen("localhost:0", backlog).ToError())
return error;
- Socket *accept_socket = nullptr;
- std::future accept_status =
[Lldb-commits] [lldb] [lldb] Add timeout argument to Socket::Accept (PR #117691)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Pavel Labath (labath)
Changes
Allows us to stop waiting for a connection if it doesn't come in a certain
amount of time. Right now, I'm keeping the status quo (infitnite wait) in the
"production" code, but using smaller (finite) values in tests. (A lot of these
tests create "loopback" connections, where a really short wait is sufficient:
on linux at least even a poll (0s wait) is sufficient if the other end has
connect()ed already, but this doesn't seem to be the case on Windows, so I'm
using a 1s wait in these cases).
---
Full diff: https://github.com/llvm/llvm-project/pull/117691.diff
8 Files Affected:
- (modified) lldb/include/lldb/Host/Socket.h (+2-1)
- (modified) lldb/source/Host/common/Socket.cpp (+11-2)
- (modified) lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp (+1-1)
- (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
(+8-7)
- (modified) lldb/unittests/Host/MainLoopTest.cpp (+2-1)
- (modified) lldb/unittests/Host/SocketTest.cpp (+23)
- (modified) lldb/unittests/TestingSupport/Host/SocketTestUtilities.cpp (+6-13)
- (modified) lldb/unittests/tools/lldb-server/tests/TestClient.cpp (+9-2)
``diff
diff --git a/lldb/include/lldb/Host/Socket.h b/lldb/include/lldb/Host/Socket.h
index 14468c98ac5a3a..0e542b05a085c6 100644
--- a/lldb/include/lldb/Host/Socket.h
+++ b/lldb/include/lldb/Host/Socket.h
@@ -13,6 +13,7 @@
#include
#include "lldb/Host/MainLoopBase.h"
+#include "lldb/Utility/Timeout.h"
#include "lldb/lldb-private.h"
#include "lldb/Host/SocketAddress.h"
@@ -108,7 +109,7 @@ class Socket : public IOObject {
// Accept a single connection and "return" it in the pointer argument. This
// function blocks until the connection arrives.
- virtual Status Accept(Socket *&socket);
+ virtual Status Accept(const Timeout &timeout, Socket *&socket);
// Initialize a Tcp Socket object in listening mode. listen and accept are
// implemented separately because the caller may wish to manipulate or query
diff --git a/lldb/source/Host/common/Socket.cpp
b/lldb/source/Host/common/Socket.cpp
index d69eb608204033..63396f7b4abc9c 100644
--- a/lldb/source/Host/common/Socket.cpp
+++ b/lldb/source/Host/common/Socket.cpp
@@ -460,7 +460,8 @@ NativeSocket Socket::CreateSocket(const int domain, const
int type,
return sock;
}
-Status Socket::Accept(Socket *&socket) {
+Status Socket::Accept(const Timeout &timeout, Socket *&socket) {
+ socket = nullptr;
MainLoop accept_loop;
llvm::Expected> expected_handles =
Accept(accept_loop,
@@ -470,7 +471,15 @@ Status Socket::Accept(Socket *&socket) {
});
if (!expected_handles)
return Status::FromError(expected_handles.takeError());
- return accept_loop.Run();
+ if (timeout) {
+accept_loop.AddCallback(
+[](MainLoopBase &loop) { loop.RequestTermination(); }, *timeout);
+ }
+ if (Status status = accept_loop.Run(); status.Fail())
+return status;
+ if (socket)
+return Status();
+ return Status(std::make_error_code(std::errc::timed_out));
}
NativeSocket Socket::AcceptSocket(NativeSocket sockfd, struct sockaddr *addr,
diff --git a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
index 8a03e47ef3d900..903bfc50def3aa 100644
--- a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -543,7 +543,7 @@ lldb::ConnectionStatus
ConnectionFileDescriptor::AcceptSocket(
if (!error.Fail()) {
post_listen_callback(*listening_socket);
-error = listening_socket->Accept(accepted_socket);
+error = listening_socket->Accept(/*timeout=*/std::nullopt,
accepted_socket);
}
if (!error.Fail()) {
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
index 7eacd605362e7c..67b41b1e77a533 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -1223,10 +1223,6 @@
GDBRemoteCommunication::ConnectLocally(GDBRemoteCommunication &client,
listen_socket.Listen("localhost:0", backlog).ToError())
return error;
- Socket *accept_socket = nullptr;
- std::future accept_status = std::async(
- std::launch::async, [&] { return listen_socket.Accept(accept_socket); });
-
llvm::SmallString<32> remote_addr;
llvm::raw_svector_ostream(remote_addr)
<< "connect://localhost:" << listen_socket.GetLocalPortNumber();
@@ -1238,10 +1234,15 @@
GDBRemoteCommunication::ConnectLocally(GDBRemoteCommunication &client,
return llvm::createStringError(llvm::inconvertibleErrorCode(),
"Unable to connect: %s",
status.AsCString());
- client.SetConnection(std::move(conn_up));
- if (llvm::Error error = accept_status.get().T
[Lldb-commits] [lldb] [lldb] Add timeout argument to Socket::Accept (PR #117691)
@@ -543,7 +543,7 @@ lldb::ConnectionStatus
ConnectionFileDescriptor::AcceptSocket(
if (!error.Fail()) {
post_listen_callback(*listening_socket);
-error = listening_socket->Accept(accepted_socket);
+error = listening_socket->Accept(/*timeout=*/std::nullopt,
accepted_socket);
labath wrote:
@tedwoodward I think this is the timeout you want to reduce. I'm leaving it up
to you to figure out how to do that (whether to use a fixed value, setting,
whatever...).
https://github.com/llvm/llvm-project/pull/117691
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make sure Blocks always have a parent (PR #117683)
@@ -399,11 +392,9 @@ size_t Block::MemorySize() const {
return mem_size;
}
-void Block::AddChild(const BlockSP &child_block_sp) {
- if (child_block_sp) {
-child_block_sp->SetParentScope(this);
-m_children.push_back(child_block_sp);
- }
+BlockSP Block::CreateChild(user_id_t uid) {
+ m_children.push_back(std::shared_ptr(new Block(uid, *this)));
labath wrote:
make_shared won't work because that constructor is private. I think protecting
that one is important as it ensures the integrity of the parent-child links.
That said, the tagged constructor approach does make it possible to use
`make_shared`, so this could be another argument in favour of it.
https://github.com/llvm/llvm-project/pull/117683
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add timeout argument to Socket::Accept (PR #117691)
labath wrote: FWIW, I think of MainLoop::Run as an equivalent to [asyncio.loop.run_forever](https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.loop.run_forever), which also doesn't take a timeout argument. The library does have a `run_until_complete` method though, which could probably be used to implement timeout based running, but I don't think that's the use case they had in mind when they were adding that function. https://github.com/llvm/llvm-project/pull/117691 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Remove child_process_inherit from the socket classes (PR #117699)
https://github.com/labath created
https://github.com/llvm/llvm-project/pull/117699
It's never set to true. Also, using inheritable FDs in a multithreaded process
pretty much guarantees descriptor leaks. It's better to explicitly pass a
specific FD to a specific subprocess, which we already mostly can do using the
ProcessLaunchInfo FileActions.
>From 927b60e0f613c60fb3b4dd4ef238574f3b666cbf Mon Sep 17 00:00:00 2001
From: Pavel Labath
Date: Sat, 19 Oct 2024 18:17:23 +0200
Subject: [PATCH] [lldb] Remove child_process_inherit from the socket classes
It's never set to true. Also, using inheritable FDs in a multithreaded
process pretty much guarantees descriptor leaks. It's better to
explicitly pass a specific FD to a specific subprocess, which we already
mostly can do using the ProcessLaunchInfo FileActions.
---
lldb/include/lldb/Host/Socket.h | 18 +++
lldb/include/lldb/Host/common/TCPSocket.h | 5 +-
lldb/include/lldb/Host/common/UDPSocket.h | 4 +-
lldb/include/lldb/Host/linux/AbstractSocket.h | 2 +-
lldb/include/lldb/Host/posix/DomainSocket.h | 7 ++-
lldb/source/Host/common/Socket.cpp| 53 +++
lldb/source/Host/common/TCPSocket.cpp | 25 -
lldb/source/Host/common/UDPSocket.cpp | 14 ++---
lldb/source/Host/linux/AbstractSocket.cpp | 3 +-
.../posix/ConnectionFileDescriptorPosix.cpp | 10 ++--
lldb/source/Host/posix/DomainSocket.cpp | 28 --
.../gdb-remote/GDBRemoteCommunication.cpp | 3 +-
lldb/tools/lldb-server/lldb-platform.cpp | 12 ++---
lldb/unittests/Host/MainLoopTest.cpp | 7 +--
lldb/unittests/Host/SocketTest.cpp| 15 +++---
.../Host/SocketTestUtilities.cpp | 17 +++---
lldb/unittests/debugserver/RNBSocketTest.cpp | 2 +-
.../tools/lldb-server/tests/TestClient.cpp| 2 +-
18 files changed, 86 insertions(+), 141 deletions(-)
diff --git a/lldb/include/lldb/Host/Socket.h b/lldb/include/lldb/Host/Socket.h
index 14468c98ac5a3a..06bd929818c559 100644
--- a/lldb/include/lldb/Host/Socket.h
+++ b/lldb/include/lldb/Host/Socket.h
@@ -93,7 +93,6 @@ class Socket : public IOObject {
static void Terminate();
static std::unique_ptr Create(const SocketProtocol protocol,
-bool child_processes_inherit,
Status &error);
virtual Status Connect(llvm::StringRef name) = 0;
@@ -114,14 +113,13 @@ class Socket : public IOObject {
// implemented separately because the caller may wish to manipulate or query
// the socket after it is initialized, but before entering a blocking accept.
static llvm::Expected>
- TcpListen(llvm::StringRef host_and_port, bool child_processes_inherit,
-int backlog = 5);
+ TcpListen(llvm::StringRef host_and_port, int backlog = 5);
static llvm::Expected>
- TcpConnect(llvm::StringRef host_and_port, bool child_processes_inherit);
+ TcpConnect(llvm::StringRef host_and_port);
static llvm::Expected>
- UdpConnect(llvm::StringRef host_and_port, bool child_processes_inherit);
+ UdpConnect(llvm::StringRef host_and_port);
static int GetOption(NativeSocket sockfd, int level, int option_name,
int &option_value);
@@ -153,8 +151,7 @@ class Socket : public IOObject {
virtual std::string GetRemoteConnectionURI() const { return ""; };
protected:
- Socket(SocketProtocol protocol, bool should_close,
- bool m_child_process_inherit);
+ Socket(SocketProtocol protocol, bool should_close);
virtual size_t Send(const void *buf, const size_t num_bytes);
@@ -162,15 +159,12 @@ class Socket : public IOObject {
static Status GetLastError();
static void SetLastError(Status &error);
static NativeSocket CreateSocket(const int domain, const int type,
- const int protocol,
- bool child_processes_inherit, Status
&error);
+ const int protocol, Status &error);
static NativeSocket AcceptSocket(NativeSocket sockfd, struct sockaddr *addr,
- socklen_t *addrlen,
- bool child_processes_inherit, Status
&error);
+ socklen_t *addrlen, Status &error);
SocketProtocol m_protocol;
NativeSocket m_socket;
- bool m_child_processes_inherit;
bool m_should_close_fd;
};
diff --git a/lldb/include/lldb/Host/common/TCPSocket.h
b/lldb/include/lldb/Host/common/TCPSocket.h
index eefe0240fe4a95..ca36622691fe9a 100644
--- a/lldb/include/lldb/Host/common/TCPSocket.h
+++ b/lldb/include/lldb/Host/common/TCPSocket.h
@@ -17,9 +17,8 @@
namespace lldb_private {
class TCPSocket : public Socket {
public:
- TCPSocket(bool should_close, bool child_processes_inherit);
- TCPSocket(NativeSocket socket, bool should_close,
-bool child_processes_inherit);
+ explicit TCPSocket(bool should_close);
[Lldb-commits] [lldb] [lldb] Add timeout argument to Socket::Accept (PR #117691)
https://github.com/mgorny approved this pull request. Well, I don't see anything obviously wrong with it. https://github.com/llvm/llvm-project/pull/117691 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make sure Blocks always have a parent (PR #117683)
@@ -399,11 +392,9 @@ size_t Block::MemorySize() const {
return mem_size;
}
-void Block::AddChild(const BlockSP &child_block_sp) {
- if (child_block_sp) {
-child_block_sp->SetParentScope(this);
-m_children.push_back(child_block_sp);
- }
+BlockSP Block::CreateChild(user_id_t uid) {
+ m_children.push_back(std::shared_ptr(new Block(uid, *this)));
DavidSpickett wrote:
Understood.
https://github.com/llvm/llvm-project/pull/117683
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Remove child_process_inherit from the socket classes (PR #117699)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Pavel Labath (labath)
Changes
It's never set to true. Also, using inheritable FDs in a multithreaded process
pretty much guarantees descriptor leaks. It's better to explicitly pass a
specific FD to a specific subprocess, which we already mostly can do using the
ProcessLaunchInfo FileActions.
---
Patch is 31.92 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/117699.diff
18 Files Affected:
- (modified) lldb/include/lldb/Host/Socket.h (+6-12)
- (modified) lldb/include/lldb/Host/common/TCPSocket.h (+2-3)
- (modified) lldb/include/lldb/Host/common/UDPSocket.h (+2-2)
- (modified) lldb/include/lldb/Host/linux/AbstractSocket.h (+1-1)
- (modified) lldb/include/lldb/Host/posix/DomainSocket.h (+3-4)
- (modified) lldb/source/Host/common/Socket.cpp (+18-35)
- (modified) lldb/source/Host/common/TCPSocket.cpp (+10-15)
- (modified) lldb/source/Host/common/UDPSocket.cpp (+7-7)
- (modified) lldb/source/Host/linux/AbstractSocket.cpp (+1-2)
- (modified) lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp (+4-6)
- (modified) lldb/source/Host/posix/DomainSocket.cpp (+11-17)
- (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
(+1-2)
- (modified) lldb/tools/lldb-server/lldb-platform.cpp (+4-8)
- (modified) lldb/unittests/Host/MainLoopTest.cpp (+2-5)
- (modified) lldb/unittests/Host/SocketTest.cpp (+6-9)
- (modified) lldb/unittests/TestingSupport/Host/SocketTestUtilities.cpp (+6-11)
- (modified) lldb/unittests/debugserver/RNBSocketTest.cpp (+1-1)
- (modified) lldb/unittests/tools/lldb-server/tests/TestClient.cpp (+1-1)
``diff
diff --git a/lldb/include/lldb/Host/Socket.h b/lldb/include/lldb/Host/Socket.h
index 14468c98ac5a3a..06bd929818c559 100644
--- a/lldb/include/lldb/Host/Socket.h
+++ b/lldb/include/lldb/Host/Socket.h
@@ -93,7 +93,6 @@ class Socket : public IOObject {
static void Terminate();
static std::unique_ptr Create(const SocketProtocol protocol,
-bool child_processes_inherit,
Status &error);
virtual Status Connect(llvm::StringRef name) = 0;
@@ -114,14 +113,13 @@ class Socket : public IOObject {
// implemented separately because the caller may wish to manipulate or query
// the socket after it is initialized, but before entering a blocking accept.
static llvm::Expected>
- TcpListen(llvm::StringRef host_and_port, bool child_processes_inherit,
-int backlog = 5);
+ TcpListen(llvm::StringRef host_and_port, int backlog = 5);
static llvm::Expected>
- TcpConnect(llvm::StringRef host_and_port, bool child_processes_inherit);
+ TcpConnect(llvm::StringRef host_and_port);
static llvm::Expected>
- UdpConnect(llvm::StringRef host_and_port, bool child_processes_inherit);
+ UdpConnect(llvm::StringRef host_and_port);
static int GetOption(NativeSocket sockfd, int level, int option_name,
int &option_value);
@@ -153,8 +151,7 @@ class Socket : public IOObject {
virtual std::string GetRemoteConnectionURI() const { return ""; };
protected:
- Socket(SocketProtocol protocol, bool should_close,
- bool m_child_process_inherit);
+ Socket(SocketProtocol protocol, bool should_close);
virtual size_t Send(const void *buf, const size_t num_bytes);
@@ -162,15 +159,12 @@ class Socket : public IOObject {
static Status GetLastError();
static void SetLastError(Status &error);
static NativeSocket CreateSocket(const int domain, const int type,
- const int protocol,
- bool child_processes_inherit, Status
&error);
+ const int protocol, Status &error);
static NativeSocket AcceptSocket(NativeSocket sockfd, struct sockaddr *addr,
- socklen_t *addrlen,
- bool child_processes_inherit, Status
&error);
+ socklen_t *addrlen, Status &error);
SocketProtocol m_protocol;
NativeSocket m_socket;
- bool m_child_processes_inherit;
bool m_should_close_fd;
};
diff --git a/lldb/include/lldb/Host/common/TCPSocket.h
b/lldb/include/lldb/Host/common/TCPSocket.h
index eefe0240fe4a95..ca36622691fe9a 100644
--- a/lldb/include/lldb/Host/common/TCPSocket.h
+++ b/lldb/include/lldb/Host/common/TCPSocket.h
@@ -17,9 +17,8 @@
namespace lldb_private {
class TCPSocket : public Socket {
public:
- TCPSocket(bool should_close, bool child_processes_inherit);
- TCPSocket(NativeSocket socket, bool should_close,
-bool child_processes_inherit);
+ explicit TCPSocket(bool should_close);
+ TCPSocket(NativeSocket socket, bool should_close);
~TCPSocket() override;
// returns port number or 0 if error
diff --git a/lldb/include/lldb/Host/common/UDPSocket.h
b/lldb/include/lldb/Host/common/UDPSocket.h
index 73
[Lldb-commits] [lldb] [lldb] Make sure Blocks always have a parent (PR #117683)
@@ -43,11 +43,13 @@ class Block : public UserID, public SymbolContextScope {
typedef RangeVector RangeList;
typedef RangeList::Entry Range;
- /// Construct with a User ID \a uid, \a depth.
- ///
- /// Initialize this block with the specified UID \a uid. The \a depth in the
- /// \a block_list is used to represent the parent, sibling, and child block
- /// information and also allows for partial parsing at the block level.
+ // Creates a block representing the whole function. Only meant to be used
from
+ // the Function class.
DavidSpickett wrote:
Make sense, leave it as is then.
https://github.com/llvm/llvm-project/pull/117683
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add timeout argument to Socket::Accept (PR #117691)
mgorny wrote: I'm not complaining, but wanted to point out it feels a bit backwards to be adding a callback to timeout, rather than having `Run()` accept a timeout. https://github.com/llvm/llvm-project/pull/117691 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make sure Blocks always have a parent (PR #117683)
@@ -43,11 +43,13 @@ class Block : public UserID, public SymbolContextScope {
typedef RangeVector RangeList;
typedef RangeList::Entry Range;
- /// Construct with a User ID \a uid, \a depth.
- ///
- /// Initialize this block with the specified UID \a uid. The \a depth in the
- /// \a block_list is used to represent the parent, sibling, and child block
- /// information and also allows for partial parsing at the block level.
+ // Creates a block representing the whole function. Only meant to be used
from
+ // the Function class.
labath wrote:
I've thought about this for quite a while. I could befriend the whole class
easily enough, but I think that would do more harm than good (it would give the
Function class free reign over Block's innards. Befriending just the
constructor is not possible because of it would cause a dependency loop between
the classes, but even if it weren't, it would produce tighter coupling than I'd
like (basically, you have to [match the constructor
signature](https://godbolt.org/z/f95dnfjdh)). It is possible to befriend a
class for construction without exposing all internals using the [tagged
constructor pattern](https://godbolt.org/z/Ex367Wz68) -- I rejected that one
because it seemed unnecessarily complicated.
I figured a constructor explicitly taking a Function pointer is going to be
sufficiently un-useful, so it wouldn't be likely to be misused, but I could be
easily convinced to do the constructor tag thingy.
https://github.com/llvm/llvm-project/pull/117683
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add timeout argument to Socket::Accept (PR #117691)
labath wrote: I can sort of see your point, but I also disagree with it. :) It's true that in this case (the blocking version of Socket::Accept), a timeout argument to MainLoop::Run would be sufficient (and probably look cleaner), but it's not very generic. Like, imagine you had a "proper" long-lived MainLoop object (like the one we have in lldb-server), and you wanted to use that to wait for a connection. You couldn't pass a timeout to the Run function there. Instead, you'd probably schedule a callback to unregister the accept handle after a certain amount of time. In this mindset, I view the blocking Accept function as a sort of a convenience wrapper on top of that functionality and its goal is to provide a simple interface for the cases where all you want to do wait for a connection. https://github.com/llvm/llvm-project/pull/117691 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][ThreadELFCore] Set all the properties of ELFLinuxSigInfo to a non build dependent size (PR #117604)
https://github.com/tuliom approved this pull request. The build succeeded! The source comment at line 82 caught my attention, though. It may imply that data layout is important in this struct, i.e. it should match `siginfo_t` returned by the OS. But this change is not guaranteeing that. Actually, the order of the first 3 integers do change on different systems. With that said, this LGTM considering that data layout doesn't need to match the OS' `siginfo_t`. https://github.com/llvm/llvm-project/pull/117604 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
