[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

teemperor added a comment. This commit seems to have broken the windows bot: http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/7780 Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63165/new/ https://reviews.llvm.org/D63165

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

labath accepted this revision. labath added a comment. This looks great to me. Thank you for your patience. Comment at: lldb/source/Plugins/Process/Utility/RegisterContextWindows_x86_64.cpp:10 +#include "RegisterContextWindows_x86_64.h" +#include

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

asmith added a comment. This is done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63165/new/ https://reviews.llvm.org/D63165 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

labath added a comment. I am still waiting for the `g_private_reg_interface` thingy to be removed, or for an explanation of why it cannot be done... CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63165/new/ https://reviews.llvm.org/D63165

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

asmith added a comment. Any last comments before this is committed? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63165/new/ https://reviews.llvm.org/D63165 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

labath added a comment. In D63165#1611924 , @asmith wrote: > I can make the one simplification but Im not sure what you are asking as far > as the refactoring. Provide an example and I will try to accommodate your > request. I'm not going to be able to

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

asmith added a comment. I can make the one simplification but Im not sure what you are asking as far as the refactoring. Provide an example and I will try to accommodate your request. I'm not going to be able to spend much more time on lldb patches though. CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

labath added a comment. > Are you accepting this review? No I would still like to see the two comments I made in https://reviews.llvm.org/D63165#1607877>> to be addressed. The last comment was just acknowledging the fact that you (or is it @Hui? I'm not sure who actually writes these

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

asmith marked an inline comment as done. asmith added inline comments. Comment at: lldb/source/Plugins/Process/Utility/RegisterContextWindows_x86_64.cpp:141 +GetRegisterInfo_WoW64(const lldb_private::ArchSpec ) { + // A WoW64 register info is the same as the i386's. +

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

labath added inline comments. Comment at: lldb/source/Plugins/Process/Utility/RegisterContextWindows_x86_64.cpp:141 +GetRegisterInfo_WoW64(const lldb_private::ArchSpec ) { + // A WoW64 register info is the same as the i386's. + std::vector _register_infos =

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

asmith marked 11 inline comments as done. asmith added inline comments. Comment at: lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp:95 + +// Resume the debug loop. +ExceptionRecordSP active_exception = amccarth wrote: > This code

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

labath added inline comments. Comment at: lldb/source/Plugins/Process/Utility/RegisterContextWindows_x86_64.cpp:141 +GetRegisterInfo_WoW64(const lldb_private::ArchSpec ) { + // A WoW64 register info is the same as the i386's. + std::vector _register_infos =

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

Hui added inline comments. Comment at: lldb/source/Plugins/Process/Utility/RegisterContextWindows_x86_64.cpp:141 +GetRegisterInfo_WoW64(const lldb_private::ArchSpec ) { + // A WoW64 register info is the same as the i386's. + std::vector _register_infos =

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

Hui added inline comments. Comment at: lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_x86_64.cpp:104-109 + if (::SuspendThread(thread_handle) == -1) { +error.SetError(GetLastError(), eErrorTypeWin32); +LLDB_LOG(log, "{0} ResumeThread failed

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

Hui added inline comments. Comment at: lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.h:45-49 + NativeProcessWindows(ProcessLaunchInfo _info, NativeDelegate , + llvm::Error ); + + NativeProcessWindows(lldb::pid_t pid, int terminal_fd, +

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

labath added a comment. In D63165#1591665 , @amccarth wrote: > I'm trying to think through the implications of this > always-use-an-lldb-server approach on cross-platform postmortem debugging. > I'll have to do some studying, but I guess this patch

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

amccarth requested changes to this revision. amccarth added a comment. This revision now requires changes to proceed. In D63165#1541944 , @clayborg wrote: > In D63165#1540606 , @Hui wrote: > > > > In D63165#1539118

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

labath added a comment. I am sorry, but the code still seems a lot more verbose to me than it should be needed to implement the given functionality. I'd like to understand why/if it's that necessary.. Comment at:

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

labath added inline comments. Comment at: lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows.cpp:31-36 + if (!data_sp) { +error.SetErrorStringWithFormat( +"failed to allocate DataBufferHeap instance of size %" PRIu64, +data_size); +

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

asmith added inline comments. Comment at: lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows.cpp:31-36 + if (!data_sp) { +error.SetErrorStringWithFormat( +"failed to allocate DataBufferHeap instance of size %" PRIu64, +data_size); +

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

labath added inline comments. Comment at: lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.h:45-49 + NativeProcessWindows(ProcessLaunchInfo _info, NativeDelegate , + llvm::Error ); + + NativeProcessWindows(lldb::pid_t pid, int

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

Hui added inline comments. Comment at: source/Plugins/Process/Windows/Common/NativeProcessWindows.h:31 +class NativeProcessWindows : public NativeProcessProtocol, + public ProcessDebugger { + labath wrote: > Hui wrote: > > labath

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

labath added inline comments. Comment at: source/Plugins/Process/Windows/Common/NativeProcessWindows.h:31 +class NativeProcessWindows : public NativeProcessProtocol, + public ProcessDebugger { + Hui wrote: > labath wrote: > > I'm not

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

Hui added inline comments. Comment at: source/Plugins/Process/Windows/Common/NativeProcessWindows.h:31 +class NativeProcessWindows : public NativeProcessProtocol, + public ProcessDebugger { + labath wrote: > I'm not sure this multiple

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

labath added inline comments. Comment at: source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp:315 + lldb::addr_t _addr) { + return GetProcessBaseAddress(m_pid, load_addr); +} This doesn't look right. You're

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

Hui added inline comments. Comment at: source/Plugins/Process/Windows/Common/NativeRegisterContextWindows.h:31 +protected: + Status ReadAllRegisterValues(lldb::DataBufferSP _sp, + const size_t data_size); labath wrote: > Is this

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

clayborg added a comment. In D63165#1540606 , @Hui wrote: > > In D63165#1539118 , @amccarth > > wrote: > > > >> Sorry for the stupid question, but ... > >> > >> What exactly is meant here by "Native"? How is a

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

Hui added a comment. > In D63165#1539118 , @amccarth wrote: > >> Sorry for the stupid question, but ... >> >> What exactly is meant here by "Native"? How is a NativeProcessWindows >> different from ProcessWindows? > > > The Native*** classes are meant

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

labath added a reviewer: amccarth. labath added a comment. I take it that D63166 is a prerequisite for this patch. Is that all, or is there something else that we ought to look at first? Overall, this patch is slightly larger that would be ideal for a proper

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

amccarth added a comment. Sorry for the stupid question, but ... What exactly is meant here by "Native"? How is a NativeProcessWindows different from ProcessWindows? Comment at: source/Plugins/Process/Windows/Common/NativeProcessWindows.h:16 +#include "IDebugDelegate.h"

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

compnerd added inline comments. Comment at: source/Plugins/Process/Utility/RegisterContextWindows_i386.cpp:40 + +// clang-format off +#define DEFINE_GPR(reg, alt, kind1, kind2, kind3, kind4) \ I believe that this bounds the range, and

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

clayborg added a comment. I don't see much I would change here as long as this works and gets tested by the generic GDB remote protocol testing? Any others have comments? Comment at: source/Plugins/Process/Windows/Common/DebuggerThread.cpp:350 +