[Lldb-commits] [lldb] r219145 - Create a ConnectionGenericFile class for Windows.
Abid, Hafiz
Hafiz_Abid at mentor.com
Wed Oct 22 09:30:08 PDT 2014
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: lldb-commits-bounces at cs.uiuc.edu [mailto:lldb-commits-
> bounces at cs.uiuc.edu] On Behalf Of Zachary Turner
> Sent: 06 October 2014 22:23
> To: lldb-commits at cs.uiuc.edu
> 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
> lldb-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
-------------- next part --------------
A non-text attachment was scrubbed...
Name: goto.patch
Type: application/octet-stream
Size: 2576 bytes
Desc: goto.patch
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20141022/4bdc5413/attachment.obj>
More information about the lldb-commits
mailing list