Hi Zachary, Some 'goto' jumps over local variable with initialization in ConnectionGenericFileWindows.cpp. I think this makes the code ill-formed as per C++ standard 6.7.3. I have attached a simple fix for it. Please let me know if you see a problem with it. Otherwise I will apply it later today.
Thanks, Abid > -----Original Message----- > From: [email protected] [mailto:lldb-commits- > [email protected]] On Behalf Of Zachary Turner > Sent: 06 October 2014 22:23 > To: [email protected] > Subject: [Lldb-commits] [lldb] r219145 - Create a ConnectionGenericFile class > for Windows. > > Author: zturner > Date: Mon Oct 6 16:23:09 2014 > New Revision: 219145 > > URL: http://llvm.org/viewvc/llvm-project?rev=219145&view=rev > Log: > Create a ConnectionGenericFile class for Windows. > > This is the first step in getting ConnectionFileDescriptor ported to Windows. > It implements a connection against a disk file for windows. This supports > connection strings of the form file://PATH which are currently supported > only on posix platforms in ConnectionFileDescriptor. > > Reviewed by: Greg Clayton > Differential Revision: http://reviews.llvm.org/D5608 > > Added: > lldb/trunk/include/lldb/Host/windows/ConnectionGenericFileWindows.h > lldb/trunk/source/Host/windows/ConnectionGenericFileWindows.cpp > Modified: > lldb/trunk/include/lldb/Core/Connection.h > lldb/trunk/include/lldb/lldb-types.h > lldb/trunk/source/API/SBCommunication.cpp > lldb/trunk/source/Core/Connection.cpp > lldb/trunk/source/Host/CMakeLists.txt > > Modified: lldb/trunk/include/lldb/Core/Connection.h > URL: http://llvm.org/viewvc/llvm- > project/lldb/trunk/include/lldb/Core/Connection.h?rev=219145&r1=219144 > &r2=219145&view=diff > ================================================================ > ============== > --- lldb/trunk/include/lldb/Core/Connection.h (original) > +++ lldb/trunk/include/lldb/Core/Connection.h Mon Oct 6 16:23:09 2014 > @@ -46,6 +46,8 @@ public: > virtual > ~Connection (); > > + static Connection *CreateDefaultConnection(const char *url); > + > //------------------------------------------------------------------ > /// Connect using the connect string \a url. > /// > > Added: > lldb/trunk/include/lldb/Host/windows/ConnectionGenericFileWindows.h > URL: http://llvm.org/viewvc/llvm- > project/lldb/trunk/include/lldb/Host/windows/ConnectionGenericFileWindo > ws.h?rev=219145&view=auto > ================================================================ > ============== > --- lldb/trunk/include/lldb/Host/windows/ConnectionGenericFileWindows.h > (added) > +++ > lldb/trunk/include/lldb/Host/windows/ConnectionGenericFileWindows.h > +++ Mon Oct 6 16:23:09 2014 > @@ -0,0 +1,64 @@ > +//===-- ConnectionGenericFileWindows.h --------------------------*- C++ > +-*-===// // > +// The LLVM Compiler Infrastructure > +// > +// This file is distributed under the University of Illinois Open > +Source // License. See LICENSE.TXT for details. > +// > +//===------------------------------------------------------------------ > +----===// > + > +#ifndef liblldb_Host_windows_ConnectionGenericFileWindows_h_ > +#define liblldb_Host_windows_ConnectionGenericFileWindows_h_ > + > +#include "lldb/Core/Connection.h" > +#include "lldb/Host/windows/windows.h" > +#include "lldb/lldb-types.h" > + > +namespace lldb_private > +{ > + > +class Error; > + > +class ConnectionGenericFile : public lldb_private::Connection { > + public: > + ConnectionGenericFile(); > + > + ConnectionGenericFile(lldb::file_t file, bool owns_file); > + > + virtual ~ConnectionGenericFile(); > + > + virtual bool IsConnected() const; > + > + virtual lldb::ConnectionStatus Connect(const char *s, Error > + *error_ptr); > + > + virtual lldb::ConnectionStatus Disconnect(Error *error_ptr); > + > + virtual size_t Read(void *dst, size_t dst_len, uint32_t > + timeout_usec, lldb::ConnectionStatus &status, Error *error_ptr); > + > + virtual size_t Write(const void *src, size_t src_len, > + lldb::ConnectionStatus &status, Error *error_ptr); > + > + bool InterruptRead(); > + > + protected: > + OVERLAPPED m_overlapped; > + HANDLE m_file; > + HANDLE m_event_handles[2]; > + bool m_owns_file; > + LARGE_INTEGER m_file_position; > + > + enum > + { > + kBytesAvailableEvent, > + kInterruptEvent > + }; > + > + private: > + void InitializeEventHandles(); > + void IncrementFilePointer(DWORD amount); > + > + DISALLOW_COPY_AND_ASSIGN(ConnectionGenericFile); > +}; > +} > + > +#endif > > Modified: lldb/trunk/include/lldb/lldb-types.h > URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/lldb- > types.h?rev=219145&r1=219144&r2=219145&view=diff > ================================================================ > ============== > --- lldb/trunk/include/lldb/lldb-types.h (original) > +++ lldb/trunk/include/lldb/lldb-types.h Mon Oct 6 16:23:09 2014 > @@ -51,6 +51,9 @@ namespace lldb > typedef void* rwlock_t; > typedef void* process_t; // Process type > is HANDLE > typedef void* thread_t; // Host thread > type > + typedef void* file_t; // Host file type > + typedef void* pipe_t; // Host pipe type > + typedef unsigned int __w64 socket_t; // Host socket > type > typedef uint32_t thread_key_t; > typedef void* thread_arg_t; // Host thread > argument > type > typedef unsigned thread_result_t; // Host thread > result type > @@ -71,6 +74,9 @@ namespace lldb > typedef pthread_rwlock_t rwlock_t; > typedef uint64_t process_t; // Process type > is just a pid. > typedef pthread_t thread_t; // Host thread > type > + typedef int file_t; // Host file type > + typedef int pipe_t; // Host pipe type > + typedef int socket_t; // Host socket > type > typedef pthread_key_t thread_key_t; > typedef void * thread_arg_t; // Host thread > argument > type > typedef void * thread_result_t; // Host thread > result type > > Modified: lldb/trunk/source/API/SBCommunication.cpp > URL: http://llvm.org/viewvc/llvm- > project/lldb/trunk/source/API/SBCommunication.cpp?rev=219145&r1=21914 > 4&r2=219145&view=diff > ================================================================ > ============== > --- lldb/trunk/source/API/SBCommunication.cpp (original) > +++ lldb/trunk/source/API/SBCommunication.cpp Mon Oct 6 16:23:09 2014 > @@ -71,7 +71,7 @@ SBCommunication::Connect (const char *ur > if (m_opaque) > { > if (!m_opaque->HasConnection ()) > - m_opaque->SetConnection (new ConnectionFileDescriptor()); > + > + m_opaque->SetConnection(Connection::CreateDefaultConnection(url)); > return m_opaque->Connect (url, NULL); > } > return eConnectionStatusNoConnection; > > Modified: lldb/trunk/source/Core/Connection.cpp > URL: http://llvm.org/viewvc/llvm- > project/lldb/trunk/source/Core/Connection.cpp?rev=219145&r1=219144&r2 > =219145&view=diff > ================================================================ > ============== > --- lldb/trunk/source/Core/Connection.cpp (original) > +++ lldb/trunk/source/Core/Connection.cpp Mon Oct 6 16:23:09 2014 > @@ -13,6 +13,12 @@ > // Project includes > #include "lldb/Core/Connection.h" > > +#if defined(_WIN32) > +#include "lldb/Host/windows/ConnectionGenericFileWindows.h" > +#endif > + > +#include "lldb/Host/ConnectionFileDescriptor.h" > + > using namespace lldb_private; > > Connection::Connection () > @@ -22,3 +28,13 @@ Connection::Connection () Connection::~Connection () > { } > + > +Connection * > +Connection::CreateDefaultConnection(const char *url) { #if > +defined(_WIN32) > + if (strstr(url, "file://") == url) > + return new ConnectionGenericFile(); #endif > + return new ConnectionFileDescriptor(); } > > Modified: lldb/trunk/source/Host/CMakeLists.txt > URL: http://llvm.org/viewvc/llvm- > project/lldb/trunk/source/Host/CMakeLists.txt?rev=219145&r1=219144&r2= > 219145&view=diff > ================================================================ > ============== > --- lldb/trunk/source/Host/CMakeLists.txt (original) > +++ lldb/trunk/source/Host/CMakeLists.txt Mon Oct 6 16:23:09 2014 > @@ -40,6 +40,7 @@ add_host_subdirectory(posix if > (CMAKE_SYSTEM_NAME MATCHES "Windows") > add_host_subdirectory(windows > windows/Condition.cpp > + windows/ConnectionGenericFileWindows.cpp > windows/EditLineWin.cpp > windows/FileSystem.cpp > windows/Host.cpp > > Added: > lldb/trunk/source/Host/windows/ConnectionGenericFileWindows.cpp > URL: http://llvm.org/viewvc/llvm- > project/lldb/trunk/source/Host/windows/ConnectionGenericFileWindows.cp > p?rev=219145&view=auto > ================================================================ > ============== > --- lldb/trunk/source/Host/windows/ConnectionGenericFileWindows.cpp > (added) > +++ lldb/trunk/source/Host/windows/ConnectionGenericFileWindows.cpp > Mon > +++ Oct 6 16:23:09 2014 > @@ -0,0 +1,337 @@ > +//===-- ConnectionGenericFileWindows.cpp ------------------------*- C++ > +-*-===// // > +// The LLVM Compiler Infrastructure > +// > +// This file is distributed under the University of Illinois Open > +Source // License. See LICENSE.TXT for details. > +// > +//===------------------------------------------------------------------ > +----===// > + > +#include "lldb/Core/Error.h" > +#include "lldb/Core/Log.h" > +#include "lldb/Host/TimeValue.h" > +#include "lldb/Host/windows/ConnectionGenericFileWindows.h" > + > +#include "llvm/ADT/STLExtras.h" > +#include "llvm/ADT/StringRef.h" > + > +using namespace lldb; > +using namespace lldb_private; > + > +namespace > +{ > +// This is a simple helper class to package up the information needed > +to return from a Read/Write // operation function. Since there is alot > +of code to be run before exit regardless of whether the // operation > +succeeded or failed, combined with many possible return paths, this is the > cleanest // way to represent it. > +class ReturnInfo > +{ > + public: > + void > + Set(size_t bytes, ConnectionStatus status, DWORD error_code) > + { > + m_error.SetError(error_code, eErrorTypeWin32); > + m_bytes = bytes; > + m_status = status; > + } > + > + void > + Set(size_t bytes, ConnectionStatus status, llvm::StringRef error_msg) > + { > + m_error.SetErrorString(error_msg.data()); > + m_bytes = bytes; > + m_status = status; > + } > + > + size_t > + GetBytes() const > + { > + return m_bytes; > + } > + ConnectionStatus > + GetStatus() const > + { > + return m_status; > + } > + const Error & > + GetError() const > + { > + return m_error; > + } > + > + private: > + Error m_error; > + size_t m_bytes; > + ConnectionStatus m_status; > +}; > +} > + > +ConnectionGenericFile::ConnectionGenericFile() > + : m_file(INVALID_HANDLE_VALUE) > + , m_owns_file(false) > +{ > + ::ZeroMemory(&m_overlapped, sizeof(m_overlapped)); > + ::ZeroMemory(&m_file_position, sizeof(m_file_position)); > + InitializeEventHandles(); > +} > + > +ConnectionGenericFile::ConnectionGenericFile(lldb::file_t file, bool > owns_file) > + : m_file(file) > + , m_owns_file(owns_file) > +{ > + ::ZeroMemory(&m_overlapped, sizeof(m_overlapped)); > + ::ZeroMemory(&m_file_position, sizeof(m_file_position)); > + InitializeEventHandles(); > +} > + > +ConnectionGenericFile::~ConnectionGenericFile() > +{ > + if (m_owns_file && IsConnected()) > + ::CloseHandle(m_file); > + > + ::CloseHandle(m_event_handles[kBytesAvailableEvent]); > + ::CloseHandle(m_event_handles[kInterruptEvent]); > +} > + > +void > +ConnectionGenericFile::InitializeEventHandles() > +{ > + m_event_handles[kInterruptEvent] = CreateEvent(NULL, FALSE, FALSE, > +NULL); > + > + // Note, we should use a manual reset event for the hEvent argument of > the OVERLAPPED. This > + // is because both WaitForMultipleObjects and GetOverlappedResult (if > you set the bWait > + // argument to TRUE) will wait for the event to be signalled. If we use > an > auto-reset event, > + // WaitForMultipleObjects will reset the event, return successfully, and > then > + // GetOverlappedResult will block since the event is no longer signalled. > + m_event_handles[kBytesAvailableEvent] = ::CreateEvent(NULL, TRUE, > +FALSE, NULL); } > + > +bool > +ConnectionGenericFile::IsConnected() const { > + return m_file && (m_file != INVALID_HANDLE_VALUE); } > + > +lldb::ConnectionStatus > +ConnectionGenericFile::Connect(const char *s, Error *error_ptr) { > + Log > *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION)); > + if (log) > + log->Printf("%p ConnectionGenericFile::Connect (url = '%s')", > +static_cast<void *>(this), s); > + > + if (strstr(s, "file://") != s) > + { > + if (error_ptr) > + error_ptr->SetErrorStringWithFormat("unsupported connection URL: > '%s'", s); > + return eConnectionStatusError; > + } > + > + if (IsConnected()) > + { > + ConnectionStatus status = Disconnect(error_ptr); > + if (status != eConnectionStatusSuccess) > + return status; > + } > + > + // file://PATH > + const char *path = s + strlen("file://"); > + // Open the file for overlapped access. If it does not exist, create > it. We > open it overlapped > + // so that we can issue asynchronous reads and then use > WaitForMultipleObjects to allow the read > + // to be interrupted by an event object. > + m_file = ::CreateFile(path, GENERIC_READ | GENERIC_WRITE, > FILE_SHARE_READ, NULL, OPEN_ALWAYS, FILE_FLAG_OVERLAPPED, NULL); > + if (m_file == INVALID_HANDLE_VALUE) > + { > + if (error_ptr) > + error_ptr->SetError(::GetLastError(), eErrorTypeWin32); > + return eConnectionStatusError; > + } > + > + m_owns_file = true; > + return eConnectionStatusSuccess; > +} > + > +lldb::ConnectionStatus > +ConnectionGenericFile::Disconnect(Error *error_ptr) { > + Log > *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION)); > + if (log) > + log->Printf("%p ConnectionGenericFile::Disconnect (url = > +'%s')", static_cast<void *>(this), s); > + > + if (!IsConnected()) > + return eConnectionStatusSuccess; > + > + // Reset the handle so that after we unblock any pending reads, > subsequent calls to Read() will > + // see a disconnected state. > + HANDLE old_file = m_file; > + m_file = INVALID_HANDLE_VALUE; > + > + // Set the disconnect event so that any blocking reads unblock, then > cancel any pending IO operations. > + ::CancelIoEx(old_file, &m_overlapped); > + > + // Close the file handle if we owned it, but don't close the event > handles. > We could always > + // reconnect with the same Connection instance. > + if (m_owns_file) > + ::CloseHandle(old_file); > + > + ::ZeroMemory(&m_file_position, sizeof(m_file_position)); > + m_owns_file = false; > + return eConnectionStatusSuccess; > +} > + > +size_t > +ConnectionGenericFile::Read(void *dst, size_t dst_len, uint32_t > +timeout_usec, lldb::ConnectionStatus &status, Error *error_ptr) { > + ReturnInfo return_info; > + > + if (error_ptr) > + error_ptr->Clear(); > + > + if (!IsConnected()) > + { > + return_info.Set(0, eConnectionStatusNoConnection, > ERROR_INVALID_HANDLE); > + goto finish; > + } > + > + m_overlapped.hEvent = m_event_handles[kBytesAvailableEvent]; > + > + BOOL result = ::ReadFile(m_file, dst, dst_len, NULL, &m_overlapped); > + if (result || ::GetLastError() == ERROR_IO_PENDING) > + { > + if (!result) > + { > + // The expected return path. The operation is pending. Wait for > the operation to complete > + // or be interrupted. > + TimeValue time_value; > + time_value.OffsetWithMicroSeconds(timeout_usec); > + DWORD milliseconds = time_value.milliseconds(); > + result = > ::WaitForMultipleObjects(llvm::array_lengthof(m_event_handles), > m_event_handles, FALSE, milliseconds); > + // All of the events are manual reset events, so make sure we > reset > them to non-signalled. > + switch (result) > + { > + case WAIT_OBJECT_0 + kBytesAvailableEvent: > + break; > + case WAIT_OBJECT_0 + kInterruptEvent: > + return_info.Set(0, eConnectionStatusInterrupted, 0); > + goto finish; > + case WAIT_TIMEOUT: > + return_info.Set(0, eConnectionStatusTimedOut, 0); > + goto finish; > + case WAIT_FAILED: > + return_info.Set(0, eConnectionStatusError, > ::GetLastError()); > + goto finish; > + } > + } > + // The data is ready. Figure out how much was read and return; > + DWORD bytes_read = 0; > + if (!::GetOverlappedResult(m_file, &m_overlapped, &bytes_read, > FALSE)) > + { > + DWORD result_error = ::GetLastError(); > + // ERROR_OPERATION_ABORTED occurs when someone calls > Disconnect() during a blocking read. > + // This triggers a call to CancelIoEx, which causes the > operation to > complete and the > + // result to be ERROR_OPERATION_ABORTED. > + if (result_error == ERROR_HANDLE_EOF || result_error == > ERROR_OPERATION_ABORTED) > + return_info.Set(bytes_read, eConnectionStatusEndOfFile, 0); > + else > + return_info.Set(bytes_read, eConnectionStatusError, > result_error); > + } > + else if (bytes_read == 0) > + return_info.Set(bytes_read, eConnectionStatusEndOfFile, 0); > + else > + return_info.Set(bytes_read, eConnectionStatusSuccess, 0); > + > + goto finish; > + } > + > + // An unknown error occured. Fail out. > + return_info.Set(0, eConnectionStatusError, ::GetLastError()); > + goto finish; > + > +finish: > + status = return_info.GetStatus(); > + if (error_ptr) > + *error_ptr = return_info.GetError(); > + > + // kBytesAvailableEvent is a manual reset event. Make sure it gets reset > here so that any > + // subsequent operations don't immediately see bytes available. > + ResetEvent(m_event_handles[kBytesAvailableEvent]); > + > + IncrementFilePointer(return_info.GetBytes()); > + Log > *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION)); > + if (log) > + { > + log->Printf("%" PRIxPTR " ConnectionGenericFile::Read() handle = %" > PRIxPTR ", dst = %" PRIxPTR ", dst_len = %" PRIu64 > + ") => %" PRIu64 ", error = %s", > + this, m_file, dst, static_cast<uint64_t>(dst_len), > static_cast<uint64_t>(return_info.GetBytes()), > + return_info.GetError().AsCString()); > + } > + > + return return_info.GetBytes(); > +} > + > +size_t > +ConnectionGenericFile::Write(const void *src, size_t src_len, > +lldb::ConnectionStatus &status, Error *error_ptr) { > + ReturnInfo return_info; > + > + if (error_ptr) > + error_ptr->Clear(); > + > + if (!IsConnected()) > + { > + return_info.Set(0, eConnectionStatusNoConnection, > ERROR_INVALID_HANDLE); > + goto finish; > + } > + > + m_overlapped.hEvent = NULL; > + > + // Writes are not interruptible like reads are, so just block until it's > done. > + BOOL result = ::WriteFile(m_file, src, src_len, NULL, &m_overlapped); > + if (!result && ::GetLastError() != ERROR_IO_PENDING) > + { > + return_info.Set(0, eConnectionStatusError, ::GetLastError()); > + goto finish; > + } > + > + DWORD bytes_written = 0; > + if (!::GetOverlappedResult(m_file, &m_overlapped, &bytes_written, > TRUE)) > + { > + return_info.Set(bytes_written, eConnectionStatusError, > ::GetLastError()); > + goto finish; > + } > + > + return_info.Set(bytes_written, eConnectionStatusSuccess, 0); > + goto finish; > + > +finish: > + status = return_info.GetStatus(); > + if (error_ptr) > + *error_ptr = return_info.GetError(); > + > + IncrementFilePointer(return_info.GetBytes()); > + Log > *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION)); > + if (log) > + { > + log->Printf("%" PRIxPTR " ConnectionGenericFile::Write() handle = %" > PRIxPTR ", src = %" PRIxPTR ", src_len = %" PRIu64 > + ") => %" PRIu64 ", error = %s", > + this, m_file, src, static_cast<uint64_t>(src_len), > static_cast<uint64_t>(return_info.GetBytes()), > + return_info.GetError().AsCString()); > + } > + return return_info.GetBytes(); > +} > + > +bool > +ConnectionGenericFile::InterruptRead() > +{ > + return ::SetEvent(m_event_handles[kInterruptEvent]); > +} > + > +void > +ConnectionGenericFile::IncrementFilePointer(DWORD amount) { > + LARGE_INTEGER old_pos; > + old_pos.HighPart = m_overlapped.OffsetHigh; > + old_pos.LowPart = m_overlapped.Offset; > + old_pos.QuadPart += amount; > + m_overlapped.Offset = old_pos.LowPart; > + m_overlapped.OffsetHigh = old_pos.HighPart; } > \ No newline at end of file > > > _______________________________________________ > lldb-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
goto.patch
Description: goto.patch
_______________________________________________ lldb-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
