jingham created this revision.
jingham added reviewers: jasonmolenda, aprantl.
Herald added subscribers: lldb-commits, abidh.

The test "TestExec.py" has been on and off flakey on the GreenDragon MacOS bots 
for a while now.  I'm trying to fix that.  It adds a lot of noise to the bots.

For background:

The job of detecting an exec used to be handled by the DynamicLoaderMacOSX 
plugin.  But when dyld switched over to a remote API approach rather than data 
structures lldb inspects, that job became hard.  About the only clue remaining 
for the new dyld API loader (DynamicLoaderMacOS) is whether the program is back 
at dyld_start when it gets a SIGTRAP.  But if dyld has moved in the course of 
the exec then the new dyld_start address is not the one we knew it as before 
the exec, so this detection will fail.  To make this work on the lldb side, 
we'd have to refresh the shared library state every time we got a SIGTRAP.  But 
that's expensive.

Fortunately, newer debugservers figure this out on their end and annotate the 
stop packet with "reason:exec".  So there's no need to do it on the lldb side.  
But because we didn't want to deal with code signing on the bots, they 
sometimes run older debugservers.  And indeed, when Adrian and I were 
investigating the failures by looking at the packet log we only saw failures 
when the stop reason didn't include reason:exec.

Also fortunately, in order to support other tools that haven't updated to the 
new dyld API's, for now dyld is still maintaining the data structures lldb used 
to look at.

So this patch adds back the check for the dyld info data structure moving that 
the DynamicLoaderMacOSX used.  For debugservers that send reason:exec, this 
code won't ever get run since ProcessGDBRemote will immediately turn the 
SIGTRAP into an exec.  And it will work correctly with older debugservers until 
dyld actually removes this structure.  At that point, the info address will be 
LLDB_INVALID_ADDRESS, and this code won't help anymore.  But presumably by then 
we won't have any debugservers we care about that don't send "reason:exec".


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D55399

Files:
  source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
  source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.h


Index: source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.h
===================================================================
--- source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.h
+++ source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.h
@@ -104,6 +104,7 @@
                                   // loaded/unloaded images
   lldb::user_id_t m_break_id;
   mutable std::recursive_mutex m_mutex;
+  lldb::addr_t m_maybe_image_infos_address;
 
 private:
   DISALLOW_COPY_AND_ASSIGN(DynamicLoaderMacOS);
Index: source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
===================================================================
--- source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
+++ source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
@@ -79,7 +79,8 @@
 //----------------------------------------------------------------------
 DynamicLoaderMacOS::DynamicLoaderMacOS(Process *process)
     : DynamicLoaderDarwin(process), m_image_infos_stop_id(UINT32_MAX),
-      m_break_id(LLDB_INVALID_BREAK_ID), m_mutex() {}
+      m_break_id(LLDB_INVALID_BREAK_ID), m_mutex()
+      , m_maybe_image_infos_address(LLDB_INVALID_ADDRESS) {}
 
 //----------------------------------------------------------------------
 // Destructor
@@ -95,16 +96,26 @@
   if (m_process) {
     // If we are stopped after an exec, we will have only one thread...
     if (m_process->GetThreadList().GetSize() == 1) {
-      // See if we are stopped at '_dyld_start'
-      ThreadSP thread_sp(m_process->GetThreadList().GetThreadAtIndex(0));
-      if (thread_sp) {
-        lldb::StackFrameSP frame_sp(thread_sp->GetStackFrameAtIndex(0));
-        if (frame_sp) {
-          const Symbol *symbol =
-              frame_sp->GetSymbolContext(eSymbolContextSymbol).symbol;
-          if (symbol) {
-            if (symbol->GetName() == ConstString("_dyld_start"))
-              did_exec = true;
+      // Maybe we still have an image infos address around?  If so see
+      // if that has changed, and if so we have exec'ed:
+      if (m_maybe_image_infos_address != LLDB_INVALID_ADDRESS) {
+        lldb::addr_t  image_infos_address = m_process->GetImageInfoAddress();
+        if (image_infos_address != m_maybe_image_infos_address)
+          did_exec = true;
+      }
+
+      if (!did_exec) {
+        // See if we are stopped at '_dyld_start'
+        ThreadSP thread_sp(m_process->GetThreadList().GetThreadAtIndex(0));
+        if (thread_sp) {
+          lldb::StackFrameSP frame_sp(thread_sp->GetStackFrameAtIndex(0));
+          if (frame_sp) {
+            const Symbol *symbol =
+                frame_sp->GetSymbolContext(eSymbolContextSymbol).symbol;
+            if (symbol) {
+              if (symbol->GetName() == ConstString("_dyld_start"))
+                did_exec = true;
+            }
           }
         }
       }
@@ -180,6 +191,7 @@
   }
 
   m_dyld_image_infos_stop_id = m_process->GetStopID();
+  m_maybe_image_infos_address = m_process->GetImageInfoAddress();
 }
 
 bool DynamicLoaderMacOS::NeedToDoInitialImageFetch() { return true; }


Index: source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.h
===================================================================
--- source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.h
+++ source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.h
@@ -104,6 +104,7 @@
                                   // loaded/unloaded images
   lldb::user_id_t m_break_id;
   mutable std::recursive_mutex m_mutex;
+  lldb::addr_t m_maybe_image_infos_address;
 
 private:
   DISALLOW_COPY_AND_ASSIGN(DynamicLoaderMacOS);
Index: source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
===================================================================
--- source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
+++ source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
@@ -79,7 +79,8 @@
 //----------------------------------------------------------------------
 DynamicLoaderMacOS::DynamicLoaderMacOS(Process *process)
     : DynamicLoaderDarwin(process), m_image_infos_stop_id(UINT32_MAX),
-      m_break_id(LLDB_INVALID_BREAK_ID), m_mutex() {}
+      m_break_id(LLDB_INVALID_BREAK_ID), m_mutex()
+      , m_maybe_image_infos_address(LLDB_INVALID_ADDRESS) {}
 
 //----------------------------------------------------------------------
 // Destructor
@@ -95,16 +96,26 @@
   if (m_process) {
     // If we are stopped after an exec, we will have only one thread...
     if (m_process->GetThreadList().GetSize() == 1) {
-      // See if we are stopped at '_dyld_start'
-      ThreadSP thread_sp(m_process->GetThreadList().GetThreadAtIndex(0));
-      if (thread_sp) {
-        lldb::StackFrameSP frame_sp(thread_sp->GetStackFrameAtIndex(0));
-        if (frame_sp) {
-          const Symbol *symbol =
-              frame_sp->GetSymbolContext(eSymbolContextSymbol).symbol;
-          if (symbol) {
-            if (symbol->GetName() == ConstString("_dyld_start"))
-              did_exec = true;
+      // Maybe we still have an image infos address around?  If so see
+      // if that has changed, and if so we have exec'ed:
+      if (m_maybe_image_infos_address != LLDB_INVALID_ADDRESS) {
+        lldb::addr_t  image_infos_address = m_process->GetImageInfoAddress();
+        if (image_infos_address != m_maybe_image_infos_address)
+          did_exec = true;
+      }
+
+      if (!did_exec) {
+        // See if we are stopped at '_dyld_start'
+        ThreadSP thread_sp(m_process->GetThreadList().GetThreadAtIndex(0));
+        if (thread_sp) {
+          lldb::StackFrameSP frame_sp(thread_sp->GetStackFrameAtIndex(0));
+          if (frame_sp) {
+            const Symbol *symbol =
+                frame_sp->GetSymbolContext(eSymbolContextSymbol).symbol;
+            if (symbol) {
+              if (symbol->GetName() == ConstString("_dyld_start"))
+                did_exec = true;
+            }
           }
         }
       }
@@ -180,6 +191,7 @@
   }
 
   m_dyld_image_infos_stop_id = m_process->GetStopID();
+  m_maybe_image_infos_address = m_process->GetImageInfoAddress();
 }
 
 bool DynamicLoaderMacOS::NeedToDoInitialImageFetch() { return true; }
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to