[Lldb-commits] [PATCH] D30981: Remove HostThreadLinux/Free/NetBSD
labath added a comment. In https://reviews.llvm.org/D30981#701902, @krytarowski wrote: > Currently these calls for set/get thread name were moved to LLVM. I haven't > checked other systems but the NetBSD one must be called for the current > process. > > The original NetBSD version was ported from FreeBSD... and it inherited its > oddness that used to bite me. Another important note - the NetBSD LLVM > version works only for pthread_t (POSIX threads), not native threads. There > is also no interface exposed for users to translate native thread (LWP) <-> > POSIX thread (pthread_t), it's not a lack of feature but the design that > pthread_t is fully opaque. I'm not sure what's the purpose of the LLVM > interface, whether to use pthread_t for the current process or any process & > native thread like what might happen on Linux. I verified that other > interfaces use pthread_t API out there and I accepted that revision. Yes, the versions in LLVM are for the current process (in fact, current thread) only, which is fine for usage as a debuggee. In the debugger we need another set of functions which will read the names from the process we are debugging, and may be implemented using completely different means. > Should I implement an interface to investigate tracee's thread name [on > NetBSD] in ptrace(2)? I didn't want to introduce a ptrace(2) calls for > set/get thread name, as thread name stored in the kernel is shadowed in > pthread_t in our libpthread(3) (this field is called pt_name). It might be a > remnant from the M:N thread model from the past (pre-5.0 NetBSD release)... > If there is a use-case from a debugger point of view to set a new name, I > will add dedicated calls. Displaying the thread name in the debugger is definitely useful, but not all that important on the grand scale of things. If the kernel already knows about the name, then I'd recommend reading it out from there, although for non-critical functionality like this you could make the case for keeping the kernel interface simple (although, then I don't know why is the kernel storing the name in the first place). I don't know what a real use case for *setting* a thread name from a debugger would be. I suppose someone might want to do that eventually, but it's probably not that useful, so it may be better to just leave it out for now. > I think it's cleaner and simpler to use the ptrace(2) API on FreeBSD. I am not familiar with the FreeBSD APIs so I'd leave that to the FreeBSD maintainer. https://reviews.llvm.org/D30981 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D31031: Move GetAuxvData from Host to relevant process plugins
labath created this revision.
Herald added subscribers: mgorny, srhines.
GetAuxvData was causing dependencies from host to target and linux
process modules. It also does not fit netbsd use case, as there we can
only read the auxiliary vector with ptrace, which is better done in the
process plugin, with the other ptrace calls.
I resolve these issues by moving the freebsd and linux versions into the
relevant process plugins. In case of linux, this required adding an
interface in NativeProcessProtocol. The empty definitions on other
platforms can simply be removed.
To get the code compiling I had to add ProcessGdbRemote -> ProcessLinux
dependency, which was not caught before because we depended on it
transitively.
https://reviews.llvm.org/D31031
Files:
include/lldb/Host/Host.h
include/lldb/Host/common/NativeProcessProtocol.h
source/Host/CMakeLists.txt
source/Host/freebsd/Host.cpp
source/Host/linux/Host.cpp
source/Host/macosx/Host.mm
source/Host/netbsd/Host.cpp
source/Host/windows/Host.cpp
source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp
source/Plugins/Process/Linux/NativeProcessLinux.h
source/Plugins/Process/gdb-remote/CMakeLists.txt
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -117,7 +117,7 @@
MainLoop::ReadHandleUP m_stdio_handle_up;
lldb::StateType m_inferior_prev_state;
- lldb::DataBufferSP m_active_auxv_buffer_sp;
+ std::unique_ptr m_active_auxv_buffer_up;
std::mutex m_saved_registers_mutex;
std::unordered_map m_saved_registers_map;
uint32_t m_next_saved_registers_id;
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -81,7 +81,6 @@
m_continue_tid(LLDB_INVALID_THREAD_ID), m_debugged_process_mutex(),
m_debugged_process_sp(), m_stdio_communication("process.stdio"),
m_inferior_prev_state(StateType::eStateInvalid),
- m_active_auxv_buffer_sp(), m_saved_registers_mutex(),
m_saved_registers_map(), m_next_saved_registers_id(1),
m_handshake_completed(false) {
RegisterPacketHandlers();
@@ -2694,7 +2693,7 @@
"qXfer:auxv:read:: packet missing length");
// Grab the auxv data if we need it.
- if (!m_active_auxv_buffer_sp) {
+ if (!m_active_auxv_buffer_up) {
// Make sure we have a valid process.
if (!m_debugged_process_sp ||
(m_debugged_process_sp->GetID() == LLDB_INVALID_PROCESS_ID)) {
@@ -2706,55 +2705,45 @@
}
// Grab the auxv data.
-m_active_auxv_buffer_sp = Host::GetAuxvData(m_debugged_process_sp->GetID());
-if (!m_active_auxv_buffer_sp ||
-m_active_auxv_buffer_sp->GetByteSize() == 0) {
- // Hmm, no auxv data, call that an error.
- if (log)
-log->Printf("GDBRemoteCommunicationServerLLGS::%s failed, no auxv data "
-"retrieved",
-__FUNCTION__);
- m_active_auxv_buffer_sp.reset();
- return SendErrorResponse(0x11);
+auto buffer_or_error = m_debugged_process_sp->GetAuxvData();
+if (!buffer_or_error) {
+ std::error_code ec = buffer_or_error.getError();
+ LLDB_LOG(log, "no auxv data retrieved: {0}", ec.message());
+ return SendErrorResponse(ec.value());
}
+m_active_auxv_buffer_up = std::move(*buffer_or_error);
}
- // FIXME find out if/how I lock the stream here.
-
StreamGDBRemote response;
bool done_with_buffer = false;
- if (auxv_offset >= m_active_auxv_buffer_sp->GetByteSize()) {
+ llvm::StringRef buffer = m_active_auxv_buffer_up->getBuffer();
+ if (auxv_offset >= buffer.size()) {
// We have nothing left to send. Mark the buffer as complete.
response.PutChar('l');
done_with_buffer = true;
} else {
// Figure out how many bytes are available starting at the given offset.
-const uint64_t bytes_remaining =
-m_active_auxv_buffer_sp->GetByteSize() - auxv_offset;
-
-// Figure out how many bytes we're going to read.
-const uint64_t bytes_to_read =
-(auxv_length > bytes_remaining) ? bytes_remaining : auxv_length;
+buffer = buffer.drop_front(auxv_offset);
// Mark the response type according to whether we're reading the remainder
// of the auxv data.
-if (bytes_to_read >= bytes_remaining) {
+if (auxv_length >= buffer.size()) {
// There will be nothing left to read after this
response.PutChar('l');
done_with_buffer = t
[Lldb-commits] [PATCH] D31031: Move GetAuxvData from Host to relevant process plugins
zturner added a comment. You mention that this was the cause of dependencies from `Host` to `Target`, but I don't see any `#include "lldb/Target/*.h"` statements removed. Is this an oversight or not possible yet? https://reviews.llvm.org/D31031 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D31031: Move GetAuxvData from Host to relevant process plugins
labath added a comment. In https://reviews.llvm.org/D31031#702747, @zturner wrote: > You mention that this was the cause of dependencies from `Host` to `Target`, > but I don't see any `#include "lldb/Target/*.h"` statements removed. Is this > an oversight or not possible yet? I guess I wasn't clear about that. One of the GetAuxvData overloads was taking a `Process*` argument, but this is by no means the last dependency on stuff from the Target module. https://reviews.llvm.org/D31031 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D31031: Move GetAuxvData from Host to relevant process plugins
It's fine, just checking :) lgtm On Thu, Mar 16, 2017 at 7:49 AM Pavel Labath via Phabricator < revi...@reviews.llvm.org> wrote: > labath added a comment. > > In https://reviews.llvm.org/D31031#702747, @zturner wrote: > > > You mention that this was the cause of dependencies from `Host` to > `Target`, but I don't see any `#include "lldb/Target/*.h"` statements > removed. Is this an oversight or not possible yet? > > > I guess I wasn't clear about that. One of the GetAuxvData overloads was > taking a `Process*` argument, but this is by no means the last dependency > on stuff from the Target module. > > > https://reviews.llvm.org/D31031 > > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D30981: Remove HostThreadLinux/Free/NetBSD
krytarowski accepted this revision. krytarowski added a comment. This revision is now accepted and ready to land. Thank you for your explanation. I will use approach similar to FreeBSD on NetBSD. I agree that leaving the FreeBSD code is best for the maintainers - it's good enough. https://reviews.llvm.org/D30981 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D31031: Move GetAuxvData from Host to relevant process plugins
krytarowski added a comment. Thank you for this. https://reviews.llvm.org/D31031 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D30981: Remove HostThreadLinux/Free/NetBSD
krytarowski added inline comments. Comment at: packages/Python/lldbsuite/test/tools/lldb-server/thread-name/main.cpp:6 +#if defined(__linux__) || defined(__NetBSD__) + ::pthread_setname_np(::pthread_self(), name); +#elif defined(__FreeBSD__) I overlooked it. ``` int pthread_setname_np(pthread_t thread, const char *name, void *arg); ``` It should be: ``` ::pthread_setname_np(::pthread_self(), "%s", name); ``` https://reviews.llvm.org/D30981 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r298004 - [Support] Support both Windows and Posix paths on both platforms.
Author: zturner
Date: Thu Mar 16 17:28:04 2017
New Revision: 298004
URL: http://llvm.org/viewvc/llvm-project?rev=298004&view=rev
Log:
[Support] Support both Windows and Posix paths on both platforms.
Previously which path syntax we supported dependend on what
platform we were compiling LLVM on. While this is normally
desirable, there are situations where we need to be able to
handle a path that we know was generated on a remote host.
Remote debugging, for example, or parsing debug info.
99% of the code in LLVM for handling paths was platform
agnostic and literally just a few branches were gated behind
pre-processor checks, so this changes those sites to use
runtime checks instead, and adds a flag to every path
API that allows one to override the host native syntax.
Differential Revision: https://reviews.llvm.org/D30858
Modified:
lldb/trunk/source/Commands/CommandCompletions.cpp
lldb/trunk/source/Utility/TildeExpressionResolver.cpp
Modified: lldb/trunk/source/Commands/CommandCompletions.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandCompletions.cpp?rev=298004&r1=298003&r2=298004&view=diff
==
--- lldb/trunk/source/Commands/CommandCompletions.cpp (original)
+++ lldb/trunk/source/Commands/CommandCompletions.cpp Thu Mar 16 17:28:04 2017
@@ -123,7 +123,8 @@ static int DiskFilesOrDirectories(const
if (CompletionBuffer.startswith("~")) {
llvm::StringRef Buffer(CompletionBuffer);
-size_t FirstSep = Buffer.find_if(path::is_separator);
+size_t FirstSep =
+Buffer.find_if([](char c) { return path::is_separator(c); });
llvm::StringRef Username = Buffer.take_front(FirstSep);
llvm::StringRef Remainder;
Modified: lldb/trunk/source/Utility/TildeExpressionResolver.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/TildeExpressionResolver.cpp?rev=298004&r1=298003&r2=298004&view=diff
==
--- lldb/trunk/source/Utility/TildeExpressionResolver.cpp (original)
+++ lldb/trunk/source/Utility/TildeExpressionResolver.cpp Thu Mar 16 17:28:04
2017
@@ -28,9 +28,8 @@ TildeExpressionResolver::~TildeExpressio
bool StandardTildeExpressionResolver::ResolveExact(
StringRef Expr, SmallVectorImpl &Output) {
// We expect the tilde expression to be ONLY the expression itself, and
- // contain
- // no separators.
- assert(!llvm::any_of(Expr, path::is_separator));
+ // contain no separators.
+ assert(!llvm::any_of(Expr, [](char c) { return path::is_separator(c); }));
assert(Expr.empty() || Expr[0] == '~');
return !fs::real_path(Expr, Output, true);
@@ -40,7 +39,7 @@ bool StandardTildeExpressionResolver::Re
StringSet<> &Output) {
// We expect the tilde expression to be ONLY the expression itself, and
// contain no separators.
- assert(!llvm::any_of(Expr, path::is_separator));
+ assert(!llvm::any_of(Expr, [](char c) { return path::is_separator(c); }));
assert(Expr.empty() || Expr[0] == '~');
Output.clear();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r298011 - Update for LLVM API removal of Function::getArgumentList()
Author: rnk
Date: Thu Mar 16 18:13:49 2017
New Revision: 298011
URL: http://llvm.org/viewvc/llvm-project?rev=298011&view=rev
Log:
Update for LLVM API removal of Function::getArgumentList()
Modified:
lldb/trunk/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
Modified: lldb/trunk/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp?rev=298011&r1=298010&r2=298011&view=diff
==
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp (original)
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp Thu Mar 16
18:13:49 2017
@@ -1853,9 +1853,9 @@ bool IRForTarget::ReplaceVariables(Funct
if (!m_decl_map->GetStructInfo(num_elements, size, alignment))
return false;
- Function::arg_iterator iter(llvm_function.getArgumentList().begin());
+ Function::arg_iterator iter(llvm_function.arg_begin());
- if (iter == llvm_function.getArgumentList().end()) {
+ if (iter == llvm_function.arg_end()) {
m_error_stream.Printf("Internal error [IRForTarget]: Wrapper takes no "
"arguments (should take at least a struct pointer)");
@@ -1867,7 +1867,7 @@ bool IRForTarget::ReplaceVariables(Funct
if (argument->getName().equals("this")) {
++iter;
-if (iter == llvm_function.getArgumentList().end()) {
+if (iter == llvm_function.arg_end()) {
m_error_stream.Printf("Internal error [IRForTarget]: Wrapper takes only "
"'this' argument (should take a struct pointer "
"too)");
@@ -1879,7 +1879,7 @@ bool IRForTarget::ReplaceVariables(Funct
} else if (argument->getName().equals("self")) {
++iter;
-if (iter == llvm_function.getArgumentList().end()) {
+if (iter == llvm_function.arg_end()) {
m_error_stream.Printf("Internal error [IRForTarget]: Wrapper takes only "
"'self' argument (should take '_cmd' and a struct "
"pointer too)");
@@ -1897,7 +1897,7 @@ bool IRForTarget::ReplaceVariables(Funct
++iter;
-if (iter == llvm_function.getArgumentList().end()) {
+if (iter == llvm_function.arg_end()) {
m_error_stream.Printf("Internal error [IRForTarget]: Wrapper takes only "
"'self' and '_cmd' arguments (should take a struct
"
"pointer too)");
Modified:
lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp?rev=298011&r1=298010&r2=298011&view=diff
==
---
lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
(original)
+++
lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
Thu Mar 16 18:13:49 2017
@@ -267,8 +267,7 @@ bool fixupRSAllocationStructByValCalls(l
// for all called function decls
for (auto func : rs_functions) {
// inspect all of the arguments in the call
-llvm::SymbolTableList &arg_list = func->getArgumentList();
-for (auto &arg : arg_list) {
+for (auto &arg : func->args()) {
if (arg.hasByValAttr()) {
arg.removeAttr(attr_byval);
changed = true;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D30454: [LLDB][MIPS] Check if memory_info.GetName() is empty before finding corresponding module.
clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Looks good, sorry for the delay. https://reviews.llvm.org/D30454 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
