Improve ReadRegister for RegisterContextWindowsx86

[amccarth]

Summary

In some circumstances (notably, certain minidumps), the thread CONTEXT does not have values for the control registers (EIP, ESP, EBP, EFLAGS). There are flags in the CONTEXT which indicate which portions are valid, but those flags weren't checked. The old code would not detect this and give a garbage value for the register. The new code will log the problem and return an error.

I consolidated the error checking and logging into a helper function, which makes the big switch statement easier to read and verify.

Ran tests to ensure this doesn't break anything. Manually verified that a minidump without info on the control registers now indicates the problem instead of giving bad information.

[Eugene.Zelenko]

Committed in r260559.

source/Plugins/Process/Windows/Common/x86/RegisterContextWindows_x86.h

public:
     const RegisterInfo *GetRegisterInfoAtIndex(size_t reg) override;
 
     size_t GetRegisterSetCount() override;
 
     const RegisterSet *GetRegisterSet(size_t reg_set) override;
 
     bool ReadRegister(const RegisterInfo *reg_info, RegisterValue &reg_value) override;
 
+private:
+    bool
+    ReadRegisterHelper(DWORD flags_required, const char *reg_name, DWORD value, RegisterValue &reg_value) const;
 };
 
 }
 
 #endif // #ifndef liblldb_RegisterContextWindows_x86_H_

source/Plugins/Process/Windows/Common/x86/RegisterContextWindows_x86.cpp

 {
     if (!CacheAllRegisterValues())
         return false;
 
     uint32_t reg = reg_info->kinds[eRegisterKindLLDB];
     switch (reg)
     {
         case lldb_eax_i386:
-            WINLOG_IFALL(WINDOWS_LOG_REGISTERS, "Read value 0x%x from EAX", m_context.Eax);
-            reg_value.SetUInt32(m_context.Eax);
-            break;
+            return ReadRegisterHelper(CONTEXT_INTEGER, "EAX", m_context.Eax, reg_value);
         case lldb_ebx_i386:
-            WINLOG_IFALL(WINDOWS_LOG_REGISTERS, "Read value 0x%x from EBX", m_context.Ebx);
-            reg_value.SetUInt32(m_context.Ebx);
-            break;
+            return ReadRegisterHelper(CONTEXT_INTEGER, "EBX", m_context.Ebx, reg_value);
         case lldb_ecx_i386:
-            WINLOG_IFALL(WINDOWS_LOG_REGISTERS, "Read value 0x%x from ECX", m_context.Ecx);
-            reg_value.SetUInt32(m_context.Ecx);
-            break;
+            return ReadRegisterHelper(CONTEXT_INTEGER, "ECX", m_context.Ecx, reg_value);
         case lldb_edx_i386:
-            WINLOG_IFALL(WINDOWS_LOG_REGISTERS, "Read value 0x%x from EDX", m_context.Edx);
-            reg_value.SetUInt32(m_context.Edx);
-            break;
+            return ReadRegisterHelper(CONTEXT_INTEGER, "EDX", m_context.Edx, reg_value);
         case lldb_edi_i386:
-            WINLOG_IFALL(WINDOWS_LOG_REGISTERS, "Read value 0x%x from EDI", m_context.Edi);
-            reg_value.SetUInt32(m_context.Edi);
-            break;
+            return ReadRegisterHelper(CONTEXT_INTEGER, "EDI", m_context.Edi, reg_value);
         case lldb_esi_i386:
-            WINLOG_IFALL(WINDOWS_LOG_REGISTERS, "Read value 0x%x from ESI", m_context.Esi);
-            reg_value.SetUInt32(m_context.Esi);
-            break;
+            return ReadRegisterHelper(CONTEXT_INTEGER, "ESI", m_context.Esi, reg_value);
         case lldb_ebp_i386:
-            WINLOG_IFALL(WINDOWS_LOG_REGISTERS, "Read value 0x%x from EBP", m_context.Ebp);
-            reg_value.SetUInt32(m_context.Ebp);
-            break;
+            return ReadRegisterHelper(CONTEXT_CONTROL, "EBP", m_context.Ebp, reg_value);
         case lldb_esp_i386:
-            WINLOG_IFALL(WINDOWS_LOG_REGISTERS, "Read value 0x%x from ESP", m_context.Esp);
-            reg_value.SetUInt32(m_context.Esp);
-            break;
+            return ReadRegisterHelper(CONTEXT_CONTROL, "ESP", m_context.Esp, reg_value);
         case lldb_eip_i386:
-            WINLOG_IFALL(WINDOWS_LOG_REGISTERS, "Read value 0x%x from EIP", m_context.Eip);
-            reg_value.SetUInt32(m_context.Eip);
-            break;
+            return ReadRegisterHelper(CONTEXT_CONTROL, "EIP", m_context.Eip, reg_value);
         case lldb_eflags_i386:
-            WINLOG_IFALL(WINDOWS_LOG_REGISTERS, "Read value 0x%x from EFLAGS", m_context.EFlags);
-            reg_value.SetUInt32(m_context.EFlags);
-            break;
+            return ReadRegisterHelper(CONTEXT_CONTROL, "EFLAGS", m_context.EFlags, reg_value);
         default:
             WINWARN_IFALL(WINDOWS_LOG_REGISTERS, "Requested unknown register %u", reg);
             break;
     }
+    return false;
+}
+
+bool
+RegisterContextWindows_x86::ReadRegisterHelper(DWORD flags_required, const char *reg_name, DWORD value,
+                                               RegisterValue &reg_value) const
+{
+    if ((m_context.ContextFlags & flags_required) != flags_required)
+    {
+        WINLOG_IFALL(WINDOWS_LOG_REGISTERS, "Thread context doesn't have %s", reg_name);
+        return false;
+    }
+    WINLOG_IFALL(WINDOWS_LOG_REGISTERS, "Read value 0x%x from %s", value, reg_name);
+    reg_value.SetUInt32(value);
     return true;
 }