Make sure code that is in the middle of figuring out the correct architecture

on attach uses the architecture it has figured out, rather than the Target's
architecture, which may not have been updated to the correct value yet.

<rdar://problem/24632895>

llvm-svn: 261279

4 files changed, 95 insertions, 22 deletions

diff --git a/lldb/packages/Python/lldbsuite/test/macosx/universal/TestUniversal.py b/lldb/packages/Python/lldbsuite/test/macosx/universal/TestUniversal.py
index 64f0cca9095..70a83ea9079 100644
--- a/lldb/packages/Python/lldbsuite/test/macosx/universal/TestUniversal.py
+++ b/lldb/packages/Python/lldbsuite/test/macosx/universal/TestUniversal.py
@@ -19,7 +19,7 @@ class UniversalTestCase(TestBase):
         # Call super's setUp().
         TestBase.setUp(self)
         # Find the line number to break inside main().
-        self.line = line_number('main.c', '// Set break point at this line.')
+        self.line = line_number('main.c',  '// Set break point at this line.')
 
     @add_test_categories(['pyapi'])
     @skipUnlessDarwin
@@ -106,3 +106,51 @@ class UniversalTestCase(TestBase):
             substrs = ['Name: eax'])
 
         self.runCmd("continue")
+
+        
+    @skipUnlessDarwin
+    @unittest2.skipUnless(hasattr(os, "uname") and os.uname()[4] in ['i386', 'x86_64'],
+            "requires i386 or x86_64")
+    def test_process_attach_with_wrong_arch(self):
+        """Test that when we attach to a binary from the wrong fork of a universal binary, we fix up the ABI correctly."""
+        # Now keep the architecture at 32 bit, but switch the binary we launch to
+        # 64 bit, and make sure on attach we switch to the correct architecture.
+
+        # Invoke the default build rule.
+        self.build()
+
+        # Note that "testit" is a universal binary.
+        exe = os.path.join(os.getcwd(), "testit")
+
+
+        # Create a target by the debugger.
+        target = self.dbg.CreateTargetWithFileAndTargetTriple(exe, "i386-apple-macosx")
+        self.assertTrue(target, VALID_TARGET)
+        pointer_size = target.GetAddressByteSize()
+        self.assertTrue(pointer_size == 4, "Initially we were 32 bit.")
+
+        bkpt = target.BreakpointCreateBySourceRegex("sleep", lldb.SBFileSpec("main.c"))
+        self.assertTrue (bkpt.IsValid(), "Valid breakpoint")
+        self.assertTrue(bkpt.GetNumLocations() >= 1, "Our main breakpoint has locations.")
+
+        popen = self.spawnSubprocess(exe, ["keep_waiting"])
+        self.addTearDownHook(self.cleanupSubprocesses)
+
+        error = lldb.SBError()
+        empty_listener = lldb.SBListener()
+        process = target.AttachToProcessWithID(empty_listener, popen.pid, error)
+        self.assertTrue(error.Success(), "Attached to process.")
+
+        pointer_size = target.GetAddressByteSize()
+        self.assertTrue(pointer_size == 8, "We switched to 64 bit.")
+
+        # It may seem odd that I am checking the number of frames, but the bug that
+        # motivated this test was that we eventually fixed the architecture, but we
+        # left the ABI set to the original value.  In that case, if you asked the
+        # process for its architecture, it would look right, but since the ABI was
+        # wrong, backtracing failed.
+
+        threads = lldbutil.continue_to_breakpoint(process, bkpt)
+        self.assertTrue(len(threads) == 1)
+        thread = threads[0]
+        self.assertTrue(thread.GetNumFrames() > 1, "We were able to backtrace.")
diff --git a/lldb/packages/Python/lldbsuite/test/macosx/universal/main.c b/lldb/packages/Python/lldbsuite/test/macosx/universal/main.c
index 9351c77f714..3edab51b1f6 100644
--- a/lldb/packages/Python/lldbsuite/test/macosx/universal/main.c
+++ b/lldb/packages/Python/lldbsuite/test/macosx/universal/main.c
@@ -1,7 +1,21 @@
 #include <stdio.h>
+#include <unistd.h>
+#include <string.h>
+
+void
+call_me()
+{
+  sleep(1);
+}
+
 int
 main (int argc, char **argv)
 {
   printf ("Hello there!\n"); // Set break point at this line.
+  if (argc == 2 && strcmp(argv[1], "keep_waiting") == 0)
+    while (1)
+      {
+        call_me();
+      }
   return 0;
 }
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 263639904f1..eeeba4272da 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -500,7 +500,21 @@ ProcessGDBRemote::BuildDynamicRegisterInfo (bool force)
         }
     }
 
-    if (GetGDBServerRegisterInfo ())
+    const ArchSpec &target_arch = GetTarget().GetArchitecture();
+    const ArchSpec &remote_host_arch = m_gdb_comm.GetHostArchitecture();
+    const ArchSpec &remote_process_arch = m_gdb_comm.GetProcessArchitecture();
+
+    // Use the process' architecture instead of the host arch, if available
+    ArchSpec arch_to_use;
+    if (remote_process_arch.IsValid ())
+        arch_to_use = remote_process_arch;
+    else
+        arch_to_use = remote_host_arch;
+    
+    if (!arch_to_use.IsValid())
+        arch_to_use = target_arch;
+
+    if (GetGDBServerRegisterInfo (arch_to_use))
         return;
 
     char packet[128];
@@ -640,7 +654,12 @@ ProcessGDBRemote::BuildDynamicRegisterInfo (bool force)
                     reg_info.invalidate_regs = invalidate_regs.data();
                 }
 
-                AugmentRegisterInfoViaABI (reg_info, reg_name, GetABI ());
+                // We have to make a temporary ABI here, and not use the GetABI because this code
+                // gets called in DidAttach, when the target architecture (and consequently the ABI we'll get from
+                // the process) may be wrong.
+                ABISP abi_to_use = ABI::FindPlugin(arch_to_use);
+
+                AugmentRegisterInfoViaABI (reg_info, reg_name, abi_to_use);
 
                 m_register_info.AddRegister(reg_info, reg_name, alt_name, set_name);
             }
@@ -668,22 +687,11 @@ ProcessGDBRemote::BuildDynamicRegisterInfo (bool force)
     // add composite registers to the existing primordial ones.
     bool from_scratch = (m_register_info.GetNumRegisters() == 0);
 
-    const ArchSpec &target_arch = GetTarget().GetArchitecture();
-    const ArchSpec &remote_host_arch = m_gdb_comm.GetHostArchitecture();
-    const ArchSpec &remote_process_arch = m_gdb_comm.GetProcessArchitecture();
-
-    // Use the process' architecture instead of the host arch, if available
-    ArchSpec remote_arch;
-    if (remote_process_arch.IsValid ())
-        remote_arch = remote_process_arch;
-    else
-        remote_arch = remote_host_arch;
-
     if (!target_arch.IsValid())
     {
-        if (remote_arch.IsValid()
-              && (remote_arch.GetMachine() == llvm::Triple::arm || remote_arch.GetMachine() == llvm::Triple::thumb)
-              && remote_arch.GetTriple().getVendor() == llvm::Triple::Apple)
+        if (arch_to_use.IsValid()
+              && (arch_to_use.GetMachine() == llvm::Triple::arm || arch_to_use.GetMachine() == llvm::Triple::thumb)
+              && arch_to_use.GetTriple().getVendor() == llvm::Triple::Apple)
             m_register_info.HardcodeARMRegisters(from_scratch);
     }
     else if (target_arch.GetMachine() == llvm::Triple::arm
@@ -4531,7 +4539,7 @@ ParseRegisters (XMLNode feature_node, GdbServerTargetInfo &target_info, GDBRemot
 // return:  'true'  on success
 //          'false' on failure
 bool
-ProcessGDBRemote::GetGDBServerRegisterInfo ()
+ProcessGDBRemote::GetGDBServerRegisterInfo (ArchSpec &arch_to_use)
 {
     // Make sure LLDB has an XML parser it can use first
     if (!XMLDocument::XMLEnabled())
@@ -4610,9 +4618,12 @@ ProcessGDBRemote::GetGDBServerRegisterInfo ()
                 return true; // Keep iterating through all children of the target_node
             });
 
+            // Don't use Process::GetABI, this code gets called from DidAttach, and in that context we haven't
+            // set the Target's architecture yet, so the ABI is also potentially incorrect.
+            ABISP abi_to_use_sp = ABI::FindPlugin(arch_to_use);
             if (feature_node)
             {
-                ParseRegisters(feature_node, target_info, this->m_register_info, GetABI());
+                ParseRegisters(feature_node, target_info, this->m_register_info, abi_to_use_sp);
             }
 
             for (const auto &include : target_info.includes)
@@ -4630,10 +4641,10 @@ ProcessGDBRemote::GetGDBServerRegisterInfo ()
                 XMLNode include_feature_node = include_xml_document.GetRootElement("feature");
                 if (include_feature_node)
                 {
-                    ParseRegisters(include_feature_node, target_info, this->m_register_info, GetABI());
+                    ParseRegisters(include_feature_node, target_info, this->m_register_info, abi_to_use_sp);
                 }
             }
-            this->m_register_info.Finalize(GetTarget().GetArchitecture());
+            this->m_register_info.Finalize(arch_to_use);
         }
     }
 
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
index b48edd836a7..70633e71307 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -461,7 +461,7 @@ protected:
 
     // Query remote GDBServer for register information
     bool
-    GetGDBServerRegisterInfo ();
+    GetGDBServerRegisterInfo (ArchSpec &arch);
 
     // Query remote GDBServer for a detailed loaded library list
     Error