[llvm-branch-commits] [lldb] r270116 - Make sure code that is in the middle of figuring out the correct architecture

Francis Ricci via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Thu May 19 13:41:15 PDT 2016


Author: fjricci
Date: Thu May 19 15:41:14 2016
New Revision: 270116

URL: http://llvm.org/viewvc/llvm-project?rev=270116&view=rev
Log:
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>

Author:    Jim Ingham <jingham at apple.com>
Date:      Thu Feb 18 23:58:45 2016 +0000

This is a cherry-pick of r261279

Modified:
    lldb/branches/release_38/packages/Python/lldbsuite/test/macosx/universal/TestUniversal.py
    lldb/branches/release_38/packages/Python/lldbsuite/test/macosx/universal/main.c
    lldb/branches/release_38/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
    lldb/branches/release_38/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h

Modified: lldb/branches/release_38/packages/Python/lldbsuite/test/macosx/universal/TestUniversal.py
URL: http://llvm.org/viewvc/llvm-project/lldb/branches/release_38/packages/Python/lldbsuite/test/macosx/universal/TestUniversal.py?rev=270116&r1=270115&r2=270116&view=diff
==============================================================================
--- lldb/branches/release_38/packages/Python/lldbsuite/test/macosx/universal/TestUniversal.py (original)
+++ lldb/branches/release_38/packages/Python/lldbsuite/test/macosx/universal/TestUniversal.py Thu May 19 15:41:14 2016
@@ -18,7 +18,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
@@ -105,3 +105,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.")

Modified: lldb/branches/release_38/packages/Python/lldbsuite/test/macosx/universal/main.c
URL: http://llvm.org/viewvc/llvm-project/lldb/branches/release_38/packages/Python/lldbsuite/test/macosx/universal/main.c?rev=270116&r1=270115&r2=270116&view=diff
==============================================================================
--- lldb/branches/release_38/packages/Python/lldbsuite/test/macosx/universal/main.c (original)
+++ lldb/branches/release_38/packages/Python/lldbsuite/test/macosx/universal/main.c Thu May 19 15:41:14 2016
@@ -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;
 }

Modified: lldb/branches/release_38/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/branches/release_38/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp?rev=270116&r1=270115&r2=270116&view=diff
==============================================================================
--- lldb/branches/release_38/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (original)
+++ lldb/branches/release_38/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp Thu May 19 15:41:14 2016
@@ -500,7 +500,21 @@ ProcessGDBRemote::BuildDynamicRegisterIn
         }
     }
 
-    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::BuildDynamicRegisterIn
                     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::BuildDynamicRegisterIn
     // 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
@@ -4520,7 +4528,7 @@ ParseRegisters (XMLNode feature_node, Gd
 // 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())
@@ -4599,9 +4607,12 @@ ProcessGDBRemote::GetGDBServerRegisterIn
                 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)
@@ -4619,10 +4630,10 @@ ProcessGDBRemote::GetGDBServerRegisterIn
                 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);
         }
     }
 

Modified: lldb/branches/release_38/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
URL: http://llvm.org/viewvc/llvm-project/lldb/branches/release_38/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h?rev=270116&r1=270115&r2=270116&view=diff
==============================================================================
--- lldb/branches/release_38/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h (original)
+++ lldb/branches/release_38/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h Thu May 19 15:41:14 2016
@@ -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




More information about the llvm-branch-commits mailing list