hhellyer created this revision.
hhellyer added a subscriber: lldb-commits.

This patch changes the way ProcessElfCore.cpp handles signal information.
The patch changes ProcessElfCore.cpp to use the signal from si_signo in SIGINFO 
notes in preference to the value of cursig in PRSTATUS notes. The value from 
SIGINFO seems to be more thread specific. The value from PRSTATUS is usually 
the same for all threads even if only one thread received a signal.
If it cannot find any SIGINFO blocks it reverts to the old behaviour and uses 
the value from cursig in PRSTATUS. If after that no thread appears to have been 
stopped it forces the status of the first thread to be SIGSTOP to prevent lldb 
hanging waiting for any thread from the core file to change state.

The order is:

- If one or more threads have a non-zero si_signo in SIGINFO that will be used.
- If no threads had a SIGINFO block with a non-zero si_signo set all threads 
signals to the value in cursig in their PRSTATUS notes.
- If no thread has a signal set to a non-zero value set the signal for only the 
first thread to SIGSTOP.

This resolves two issues. The first was identified in bug 26322, the second 
became apparent while investigating this problem and looking at the signal 
values reported for each thread via “thread list”.

Firstly lldb is able to load core dumps generated by gcore where each thread 
has a SIGINFO note containing a signal number but cursig in the PRSTATUS block 
for each thread is 0.

Secondly if a SIGINFO note was found the “thread list” command will no longer 
show the same signal number for all threads. At the moment if a process 
crashes, for example with SIGILL, all threads will show “stop reason = signal 
SIGILL”. With this patch only the thread that executed the illegal instruction 
shows that stop reason. The other threads show “stop reason = signal 0”.


https://reviews.llvm.org/D26676

Files:
  source/Plugins/Process/elf-core/ProcessElfCore.cpp
  source/Plugins/Process/elf-core/ThreadElfCore.cpp
  source/Plugins/Process/elf-core/ThreadElfCore.h

Index: source/Plugins/Process/elf-core/ThreadElfCore.h
===================================================================
--- source/Plugins/Process/elf-core/ThreadElfCore.h
+++ source/Plugins/Process/elf-core/ThreadElfCore.h
@@ -82,6 +82,38 @@
 static_assert(sizeof(ELFLinuxPrStatus) == 112,
               "sizeof ELFLinuxPrStatus is not correct!");
 
+struct ELFLinuxSigInfo {
+  int32_t si_signo;
+  int32_t si_code;
+  int32_t si_errno;
+
+  ELFLinuxSigInfo();
+
+  lldb_private::Error Parse(lldb_private::DataExtractor &data,
+                            lldb_private::ArchSpec &arch);
+
+  // Return the bytesize of the structure
+  // 64 bit - just sizeof
+  // 32 bit - hardcoded because we are reusing the struct, but some of the
+  // members are smaller -
+  // so the layout is not the same
+  static size_t GetSize(lldb_private::ArchSpec &arch) {
+    switch (arch.GetCore()) {
+    case lldb_private::ArchSpec::eCore_x86_64_x86_64:
+      return sizeof(ELFLinuxSigInfo);
+    case lldb_private::ArchSpec::eCore_s390x_generic:
+    case lldb_private::ArchSpec::eCore_x86_32_i386:
+    case lldb_private::ArchSpec::eCore_x86_32_i486:
+      return 12;
+    default:
+      return 0;
+    }
+  }
+};
+
+static_assert(sizeof(ELFLinuxSigInfo) == 12,
+              "sizeof ELFLinuxSigInfo is not correct!");
+
 // PRPSINFO structure's size differs based on architecture.
 // This is the layout in the x86-64 arch case.
 // In the i386 case we parse it manually and fill it again
@@ -133,7 +165,8 @@
   lldb_private::DataExtractor fpregset;
   lldb_private::DataExtractor vregset;
   lldb::tid_t tid;
-  int signo;
+  int signo = 0;
+  int prstatus_sig = 0;
   std::string name;
 };
 
Index: source/Plugins/Process/elf-core/ThreadElfCore.cpp
===================================================================
--- source/Plugins/Process/elf-core/ThreadElfCore.cpp
+++ source/Plugins/Process/elf-core/ThreadElfCore.cpp
@@ -320,3 +320,45 @@
 
   return error;
 }
+
+//----------------------------------------------------------------
+// Parse SIGINFO from NOTE entry
+//----------------------------------------------------------------
+ELFLinuxSigInfo::ELFLinuxSigInfo() {
+  memset(this, 0, sizeof(ELFLinuxSigInfo));
+}
+
+Error ELFLinuxSigInfo::Parse(DataExtractor &data, ArchSpec &arch) {
+  Error error;
+  ByteOrder byteorder = data.GetByteOrder();
+  if (GetSize(arch) > data.GetByteSize()) {
+    error.SetErrorStringWithFormat(
+        "NT_SIGINFO size should be %zu, but the remaining bytes are: %" PRIu64,
+        GetSize(arch), data.GetByteSize());
+    return error;
+  }
+
+  switch (arch.GetCore()) {
+  case ArchSpec::eCore_x86_64_x86_64:
+    data.ExtractBytes(0, sizeof(ELFLinuxPrStatus), byteorder, this);
+    break;
+  case ArchSpec::eCore_s390x_generic:
+  case ArchSpec::eCore_x86_32_i386:
+  case ArchSpec::eCore_x86_32_i486: {
+    // Parsing from a 32 bit ELF core file, and populating/reusing the structure
+    // properly, because the struct is for the 64 bit version
+    offset_t offset = 0;
+    si_signo = data.GetU32(&offset);
+    si_code = data.GetU32(&offset);
+    si_errno = data.GetU32(&offset);
+
+    break;
+  }
+  default:
+    error.SetErrorStringWithFormat("ELFLinuxSigInfo::%s Unknown architecture",
+                                   __FUNCTION__);
+    break;
+  }
+
+  return error;
+}
Index: source/Plugins/Process/elf-core/ProcessElfCore.cpp
===================================================================
--- source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -201,6 +201,30 @@
     }
   }
 
+  // Ensure we found at least one thread that was stopped on a signal.
+  bool siginfo_signal_found = false;
+  bool prstatus_signal_found = false;
+  // Check we found a signal in a SIGINFO note.
+  for (std::vector<ThreadData>::iterator it = m_thread_data.begin();
+      it != m_thread_data.end(); ++it) {
+    if (it->signo != 0)
+      siginfo_signal_found = true;
+    if (it->prstatus_sig != 0)
+      prstatus_signal_found = true;
+  }
+  if (siginfo_signal_found == false) {
+    // If we don't have signal from SIGINFO use the signal from each threads
+    // PRSTATUS note.
+    if( prstatus_signal_found == true) {
+      for (std::vector<ThreadData>::iterator it = m_thread_data.begin();
+          it != m_thread_data.end(); ++it)
+        it->signo = it->prstatus_sig;
+    } else if(m_thread_data.size() > 0 ){
+      // If all else fails force the first thread to be SIGSTOP
+      m_thread_data.begin()->signo = SIGSTOP;
+    }
+  }
+
   if (!ranges_are_sorted) {
     m_core_aranges.Sort();
     m_core_range_infos.Sort();
@@ -400,7 +424,8 @@
   NT_TASKSTRUCT,
   NT_PLATFORM,
   NT_AUXV,
-  NT_FILE = 0x46494c45
+  NT_FILE = 0x46494c45,
+  NT_SIGINFO = 0x53494749
 };
 
 namespace FREEBSD {
@@ -484,6 +509,7 @@
   ArchSpec arch = GetArchitecture();
   ELFLinuxPrPsInfo prpsinfo;
   ELFLinuxPrStatus prstatus;
+  ELFLinuxSigInfo siginfo;
   size_t header_size;
   size_t len;
   Error error;
@@ -545,7 +571,7 @@
         error = prstatus.Parse(note_data, arch);
         if (error.Fail())
           return error;
-        thread_data->signo = prstatus.pr_cursig;
+        thread_data->prstatus_sig = prstatus.pr_cursig;
         thread_data->tid = prstatus.pr_pid;
         header_size = ELFLinuxPrStatus::GetSize(arch);
         len = note_data.GetByteSize() - header_size;
@@ -583,6 +609,12 @@
             m_nt_file_entries[i].path.SetCString(path);
         }
       } break;
+      case NT_SIGINFO: {
+          error = siginfo.Parse(note_data, arch);
+          if (error.Fail())
+              return error;
+          thread_data->signo = siginfo.si_signo;
+      } break;
       default:
         break;
       }
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to