Improve ReadRegister for RegisterContextWindowsx86
authorAdrian McCarthy <amccarth@google.com>
Thu, 11 Feb 2016 18:24:57 +0000 (18:24 +0000)
committerAdrian McCarthy <amccarth@google.com>
Thu, 11 Feb 2016 18:24:57 +0000 (18:24 +0000)
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.

Differential Review: http://reviews.llvm.org/D17152

llvm-svn: 260559

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

index e57e1ef..a95561d 100644 (file)
@@ -131,48 +131,42 @@ RegisterContextWindows_x86::ReadRegister(const RegisterInfo *reg_info, RegisterV
     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;
 }
index 7d854ef..6c29d54 100644 (file)
@@ -41,6 +41,9 @@ class RegisterContextWindows_x86 : public RegisterContextWindows
 
     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;
 };
 
 }