[windows] Fix out-of-memory failure in some of the tests

[stella.stamenova]

Summary

When ReadProcessMemory fails, bytes_read is sometimes set to a large garbage value. In that case, we need to set it back to zero before returning or the garbage value will be used to allocate memory later causing LLDB to crash with an out of memory error.

[zturner]

Any idea Why does the call fail?

[stella.stamenova]

The memory address that we are reading from is invalid in the test that is failing – it has not been initialized, so it points to an invalid location. ReadProcessMemory can fail for a number of reasons and in this case I believe it’s because it doesn’t have access to read from that particular memory location.

Thanks,
-Stella

From: Zachary Turner <[e-mail]>
Sent: Tuesday, July 10, 2018 3:38 PM
To: [e-mail]
Cc: Stella Stamenova <[e-mail]>; aaron.lee.smith <[e-mail]>; [e-mail]
Subject: Re: [PATCH] D49159: [windows] Fix out-of-memory failure in some of the tests

Any idea Why does the call fail?

[jingham]

The comment for ReadMemory (in Process.h) says it returns 0 on error, but DoReadMemory doesn't say that. Also the comments are out of date and don't mention the error parameter. Could you fix that so newer plugin authors will know to do the right thing too?

lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp

Status ProcessWindows::DoLaunch(Module *exe_module,
   Log *log = ProcessWindowsLog::GetLogIfAny(WINDOWS_LOG_PROCESS);
   Status result;
 
   FileSpec working_dir = launch_info.GetWorkingDirectory();
   namespace fs = llvm::sys::fs;
   if (working_dir && (!working_dir.ResolvePath() ||
                       !fs::is_directory(working_dir.GetPath()))) {
     result.SetErrorStringWithFormat("No such file or directory: %s",
                                     working_dir.GetCString());
     return result;
   }
 
   if (!launch_info.GetFlags().Test(eLaunchFlagDebug)) {
     StreamString stream;
     stream.Printf("ProcessWindows unable to launch '%s'.  ProcessWindows can "
                   "only be used for debug launches.",
                   launch_info.GetExecutableFile().GetPath().c_str());
if (private_state == eStateStopped || private_state == eStateCrashed) {
 
     bool failed = false;
     for (uint32_t i = 0; i < m_thread_list.GetSize(); ++i) {
       auto thread = std::static_pointer_cast<TargetThreadWindows>(
           m_thread_list.GetThreadAtIndex(i));
       Status result = thread->DoResume();
       if (result.Fail()) {
         failed = true;
-        LLDB_LOG(log, "Trying to resume thread at index {0}, but failed with error {1}.", i, result);
+        LLDB_LOG(
+            log,
+            "Trying to resume thread at index {0}, but failed with error {1}.",
+            i, result);
       }
     }
 
     if (failed) {
       error.SetErrorString("ProcessWindows::DoResume failed");
       return error;
     } else {
       SetPrivateState(eStateRunning);
void ProcessWindows::RefreshStateAfterStop() {
   }
 
   m_thread_list.RefreshStateAfterStop();
 
   std::weak_ptr<ExceptionRecord> exception_record =
       m_session_data->m_debugger->GetActiveException();
   ExceptionRecordSP active_exception = exception_record.lock();
   if (!active_exception) {
-    LLDB_LOG(log, "there is no active exception in process {0}.  Why is the "
-                  "process stopped?",
+    LLDB_LOG(log,
+             "there is no active exception in process {0}.  Why is the "
              m_session_data->m_debugger->GetProcess().GetProcessId());
     return;
   }
 
   StopInfoSP stop_info;
   m_thread_list.SetSelectedThreadByID(active_exception->GetThreadID());
   ThreadSP stop_thread = m_thread_list.GetSelectedThread();
   if (!stop_thread)
     return;
 
   switch (active_exception->GetExceptionCode()) {
   case EXCEPTION_SINGLE_STEP: {
     RegisterContextSP register_context = stop_thread->GetRegisterContext();
     const uint64_t pc = register_context->GetPC();
     BreakpointSiteSP site(GetBreakpointSiteList().FindByAddress(pc));
     if (site && site->ValidForThisThread(stop_thread.get())) {
-      LLDB_LOG(log, "Single-stepped onto a breakpoint in process {0} at "
-                    "address {1:x} with breakpoint site {2}",
+      LLDB_LOG(log,
+               "Single-stepped onto a breakpoint in process {0} at "
                m_session_data->m_debugger->GetProcess().GetProcessId(), pc,
                site->GetID());
       stop_info = StopInfo::CreateStopReasonWithBreakpointSiteID(*stop_thread,
                                                                  site->GetID());
       stop_thread->SetStopInfo(stop_info);
     } else {
       LLDB_LOG(log, "single stepping thread {0}", stop_thread->GetID());
       stop_info = StopInfo::CreateStopReasonToTrace(*stop_thread);
       stop_thread->SetStopInfo(stop_info);
     }
     return;
   }
 
   case EXCEPTION_BREAKPOINT: {
     RegisterContextSP register_context = stop_thread->GetRegisterContext();
 
     // The current EIP is AFTER the BP opcode, which is one byte.
     uint64_t pc = register_context->GetPC() - 1;
 
     BreakpointSiteSP site(GetBreakpointSiteList().FindByAddress(pc));
     if (site) {
-      LLDB_LOG(log, "detected breakpoint in process {0} at address {1:x} with "
-                    "breakpoint site {2}",
+      LLDB_LOG(log,
+               "detected breakpoint in process {0} at address {1:x} with "
                m_session_data->m_debugger->GetProcess().GetProcessId(), pc,
                site->GetID());
 
       if (site->ValidForThisThread(stop_thread.get())) {
-        LLDB_LOG(log, "Breakpoint site {0} is valid for this thread ({1:x}), "
-                      "creating stop info.",
+        LLDB_LOG(log,
+                 "Breakpoint site {0} is valid for this thread ({1:x}), "
                  site->GetID(), stop_thread->GetID());
 
         stop_info = StopInfo::CreateStopReasonWithBreakpointSiteID(
             *stop_thread, site->GetID());
         register_context->SetPC(pc);
       } else {
-        LLDB_LOG(log, "Breakpoint site {0} is not valid for this thread, "
-                      "creating empty stop info.",
+        LLDB_LOG(log,
+                 "Breakpoint site {0} is not valid for this thread, "
                  site->GetID());
       }
       stop_thread->SetStopInfo(stop_info);
       return;
     } else {
       // The thread hit a hard-coded breakpoint like an `int 3` or
       // `__debugbreak()`.
       LLDB_LOG(log,
size_t ProcessWindows::DoReadMemory(lldb::addr_t vm_addr, void *buf,
   LLDB_LOG(log, "attempting to read {0} bytes from address {1:x}", size,
            vm_addr);
 
   HostProcess process = m_session_data->m_debugger->GetProcess();
   void *addr = reinterpret_cast<void *>(vm_addr);
   SIZE_T bytes_read = 0;
   if (!ReadProcessMemory(process.GetNativeProcess().GetSystemHandle(), addr,
                          buf, size, &bytes_read)) {
+    // Reading from the process can fail for a number of reasons - set the
+    // error code and make sure that the number of bytes read is set back to 0
+    // because in some scenarios the value of bytes_read returned from the API
+    // is garbage.
     error.SetError(GetLastError(), eErrorTypeWin32);
     LLDB_LOG(log, "reading failed with error: {0}", error);
+    bytes_read = 0;
   }
   return bytes_read;
 }
 
 size_t ProcessWindows::DoWriteMemory(lldb::addr_t vm_addr, const void *buf,
                                      size_t size, Status &error) {
   Log *log = ProcessWindowsLog::GetLogIfAny(WINDOWS_LOG_MEMORY);
   llvm::sys::ScopedLock lock(m_mutex);
if (::GetLastError() == ERROR_INVALID_PARAMETER) {
       info.GetRange().SetRangeEnd(LLDB_INVALID_ADDRESS);
       info.SetReadable(MemoryRegionInfo::eNo);
       info.SetExecutable(MemoryRegionInfo::eNo);
       info.SetWritable(MemoryRegionInfo::eNo);
       info.SetMapped(MemoryRegionInfo::eNo);
       return error;
     } else {
       error.SetError(::GetLastError(), eErrorTypeWin32);
-      LLDB_LOG(log, "VirtualQueryEx returned error {0} while getting memory "
-                    "region info for address {1:x}",
+      LLDB_LOG(log,
+               "VirtualQueryEx returned error {0} while getting memory "
                error, vm_addr);
       return error;
     }
   }
 
   // Protect bits are only valid for MEM_COMMIT regions.
   if (mem_info.State == MEM_COMMIT) {
     const bool readable = IsPageReadable(mem_info.Protect);
if (mem_info.State != MEM_FREE) {
     GetSystemInfo(&data);
     DWORD page_offset = vm_addr % data.dwPageSize;
     info.GetRange().SetRangeBase(vm_addr);
     info.GetRange().SetByteSize(mem_info.RegionSize - page_offset);
     info.SetMapped(MemoryRegionInfo::eNo);
   }
 
   error.SetError(::GetLastError(), eErrorTypeWin32);
-  LLDB_LOGV(log, "Memory region info for address {0}: readable={1}, "
-                 "executable={2}, writable={3}",
+  LLDB_LOGV(log,
+            "Memory region info for address {0}: readable={1}, "
             vm_addr, info.GetReadable(), info.GetExecutable(),
             info.GetWritable());
   return error;
 }
 
 lldb::addr_t ProcessWindows::GetImageInfoAddress() {
   Target &target = GetTarget();
   ObjectFile *obj_file = target.GetExecutableModule()->GetObjectFile();
ProcessWindows::OnDebugException(bool first_chance,
   // FIXME: Without this check, occasionally when running the test suite there
   // is
   // an issue where m_session_data can be null.  It's not clear how this could
   // happen but it only surfaces while running the test suite.  In order to
   // properly diagnose this, we probably need to first figure allow the test
   // suite to print out full lldb logs, and then add logging to the process
   // plugin.
   if (!m_session_data) {
-    LLDB_LOG(log, "Debugger thread reported exception {0:x} at address {1:x}, "
-                  "but there is no session.",
+    LLDB_LOG(log,
+             "Debugger thread reported exception {0:x} at address {1:x}, "
              record.GetExceptionCode(), record.GetExceptionAddress());
     return ExceptionResult::SendToApplication;
   }
 
   if (!first_chance) {
     // Any second chance exception is an application crash by definition.
     SetPrivateState(eStateCrashed);
   }
case EXCEPTION_BREAKPOINT:
     }
     SetPrivateState(eStateStopped);
     break;
   case EXCEPTION_SINGLE_STEP:
     result = ExceptionResult::BreakInDebugger;
     SetPrivateState(eStateStopped);
     break;
   default:
-    LLDB_LOG(log, "Debugger thread reported exception {0:x} at address {1:x} "
-                  "(first_chance={2})",
+    LLDB_LOG(log,
+             "Debugger thread reported exception {0:x} at address {1:x} "
              record.GetExceptionCode(), record.GetExceptionAddress(),
              first_chance);
     // For non-breakpoints, give the application a chance to handle the
     // exception first.
     if (first_chance)
       result = ExceptionResult::SendToApplication;
     else
       result = ExceptionResult::BreakInDebugger;

 
 void ProcessWindows::OnDebuggerError(const Status &error, uint32_t type) {
   llvm::sys::ScopedLock lock(m_mutex);
   Log *log = ProcessWindowsLog::GetLogIfAny(WINDOWS_LOG_PROCESS);
 
   if (m_session_data->m_initial_stop_received) {
     // This happened while debugging.  Do we shutdown the debugging session,
     // try to continue, or do something else?
-    LLDB_LOG(log, "Error {0} occurred during debugging.  Unexpected behavior "
-                  "may result.  {1}",
+    LLDB_LOG(log,
+             "Error {0} occurred during debugging.  Unexpected behavior "
              error.GetError(), error);
   } else {
     // If we haven't actually launched the process yet, this was an error
     // launching the process.  Set the internal error and signal the initial
     // stop event so that the DoLaunch method wakes up and returns a failure.
     m_session_data->m_launch_error = error;
     ::SetEvent(m_session_data->m_initial_stop_event);
     LLDB_LOG(