[Lldb-commits] [lldb] r261279 - Make sure code that is in the middle of figuring out the correct architecture

Jim Ingham via lldb-commits lldb-commits at lists.llvm.org
Thu Feb 18 15:58:45 PST 2016


Author: jingham
Date: Thu Feb 18 17:58:45 2016
New Revision: 261279

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

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

Modified: lldb/trunk/packages/Python/lldbsuite/test/macosx/universal/TestUniversal.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/macosx/universal/TestUniversal.py?rev=261279&r1=261278&r2=261279&view=diff
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/macosx/universal/TestUniversal.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/macosx/universal/TestUniversal.py Thu Feb 18 17:58:45 2016
@@ -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.")

Modified: lldb/trunk/packages/Python/lldbsuite/test/macosx/universal/main.c
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/macosx/universal/main.c?rev=261279&r1=261278&r2=261279&view=diff
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/macosx/universal/main.c (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/macosx/universal/main.c Thu Feb 18 17:58:45 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/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp?rev=261279&r1=261278&r2=261279&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp Thu Feb 18 17:58:45 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
@@ -4531,7 +4539,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())
@@ -4610,9 +4618,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)
@@ -4630,10 +4641,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/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h?rev=261279&r1=261278&r2=261279&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h Thu Feb 18 17:58:45 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 lldb-commits mailing list