[Lldb-commits] [PATCH] D68658: LLDB: Use LLVM's type for minidump ExceptionStream [NFC]

[Joseph Tremoulet via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd094d97d0223: LLDB: Use LLVMs type for minidump 
ExceptionStream [NFC] (authored by JosephTremoulet).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68658/new/

https://reviews.llvm.org/D68658

Files:
  lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
  lldb/source/Plugins/Process/minidump/MinidumpParser.h
  lldb/source/Plugins/Process/minidump/MinidumpTypes.cpp
  lldb/source/Plugins/Process/minidump/MinidumpTypes.h
  lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
  lldb/source/Plugins/Process/minidump/ProcessMinidump.h
  lldb/unittests/Process/minidump/MinidumpParserTest.cpp

Index: lldb/unittests/Process/minidump/MinidumpParserTest.cpp
===
--- lldb/unittests/Process/minidump/MinidumpParserTest.cpp
+++ lldb/unittests/Process/minidump/MinidumpParserTest.cpp
@@ -252,10 +252,10 @@
 
 TEST_F(MinidumpParserTest, GetExceptionStream) {
   SetUpData("linux-x86_64.dmp");
-  const MinidumpExceptionStream *exception_stream =
+  const llvm::minidump::ExceptionStream *exception_stream =
   parser->GetExceptionStream();
   ASSERT_NE(nullptr, exception_stream);
-  ASSERT_EQ(11UL, exception_stream->exception_record.exception_code);
+  ASSERT_EQ(11UL, exception_stream->ExceptionRecord.ExceptionCode);
 }
 
 void check_mem_range_exists(MinidumpParser , const uint64_t range_start,
Index: lldb/source/Plugins/Process/minidump/ProcessMinidump.h
===
--- lldb/source/Plugins/Process/minidump/ProcessMinidump.h
+++ lldb/source/Plugins/Process/minidump/ProcessMinidump.h
@@ -108,7 +108,7 @@
   FileSpec m_core_file;
   lldb::DataBufferSP m_core_data;
   llvm::ArrayRef m_thread_list;
-  const MinidumpExceptionStream *m_active_exception;
+  const minidump::ExceptionStream *m_active_exception;
   lldb::CommandObjectSP m_command_sp;
   bool m_is_wow64;
 };
Index: lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
===
--- lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -241,36 +241,45 @@
   if (!m_active_exception)
 return;
 
-  if (m_active_exception->exception_record.exception_code ==
-  MinidumpException::DumpRequested) {
+  constexpr uint32_t BreakpadDumpRequested = 0x;
+  if (m_active_exception->ExceptionRecord.ExceptionCode ==
+  BreakpadDumpRequested) {
+// This "ExceptionCode" value is a sentinel that is sometimes used
+// when generating a dump for a process that hasn't crashed.
+
+// TODO: The definition and use of this "dump requested" constant
+// in Breakpad are actually Linux-specific, and for similar use
+// cases on Mac/Windows it defines differnt constants, referring
+// to them as "simulated" exceptions; consider moving this check
+// down to the OS-specific paths and checking each OS for its own
+// constant.
 return;
   }
 
   lldb::StopInfoSP stop_info;
   lldb::ThreadSP stop_thread;
 
-  Process::m_thread_list.SetSelectedThreadByID(m_active_exception->thread_id);
+  Process::m_thread_list.SetSelectedThreadByID(m_active_exception->ThreadId);
   stop_thread = Process::m_thread_list.GetSelectedThread();
   ArchSpec arch = GetArchitecture();
 
   if (arch.GetTriple().getOS() == llvm::Triple::Linux) {
 stop_info = StopInfo::CreateStopReasonWithSignal(
-*stop_thread, m_active_exception->exception_record.exception_code);
+*stop_thread, m_active_exception->ExceptionRecord.ExceptionCode);
   } else if (arch.GetTriple().getVendor() == llvm::Triple::Apple) {
 stop_info = StopInfoMachException::CreateStopReasonWithMachException(
-*stop_thread, m_active_exception->exception_record.exception_code, 2,
-m_active_exception->exception_record.exception_flags,
-m_active_exception->exception_record.exception_address, 0);
+*stop_thread, m_active_exception->ExceptionRecord.ExceptionCode, 2,
+m_active_exception->ExceptionRecord.ExceptionFlags,
+m_active_exception->ExceptionRecord.ExceptionAddress, 0);
   } else {
 std::string desc;
 llvm::raw_string_ostream desc_stream(desc);
 desc_stream << "Exception "
 << llvm::format_hex(
-   m_active_exception->exception_record.exception_code, 8)
+   m_active_exception->ExceptionRecord.ExceptionCode, 8)
 << " encountered at address "
 << llvm::format_hex(
-   m_active_exception->exception_record.exception_address,
-   8);
+   m_active_exception->ExceptionRecord.ExceptionAddress, 8);
 stop_info = StopInfo::CreateStopReasonWithException(
 *stop_thread, desc_stream.str().c_str());
   }
@@ 

[Lldb-commits] [PATCH] D68658: LLDB: Use LLVM's type for minidump ExceptionStream [NFC]

[Joseph Tremoulet via Phabricator via lldb-commits]

JosephTremoulet updated this revision to Diff 224356.
JosephTremoulet added a comment.

- Define DumpRequested constant in-line, per feedback in D68656 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68658/new/

https://reviews.llvm.org/D68658

Files:
  lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
  lldb/source/Plugins/Process/minidump/MinidumpParser.h
  lldb/source/Plugins/Process/minidump/MinidumpTypes.cpp
  lldb/source/Plugins/Process/minidump/MinidumpTypes.h
  lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
  lldb/source/Plugins/Process/minidump/ProcessMinidump.h
  lldb/unittests/Process/minidump/MinidumpParserTest.cpp

Index: lldb/unittests/Process/minidump/MinidumpParserTest.cpp
===
--- lldb/unittests/Process/minidump/MinidumpParserTest.cpp
+++ lldb/unittests/Process/minidump/MinidumpParserTest.cpp
@@ -252,10 +252,10 @@
 
 TEST_F(MinidumpParserTest, GetExceptionStream) {
   SetUpData("linux-x86_64.dmp");
-  const MinidumpExceptionStream *exception_stream =
+  const llvm::minidump::ExceptionStream *exception_stream =
   parser->GetExceptionStream();
   ASSERT_NE(nullptr, exception_stream);
-  ASSERT_EQ(11UL, exception_stream->exception_record.exception_code);
+  ASSERT_EQ(11UL, exception_stream->ExceptionRecord.ExceptionCode);
 }
 
 void check_mem_range_exists(MinidumpParser , const uint64_t range_start,
Index: lldb/source/Plugins/Process/minidump/ProcessMinidump.h
===
--- lldb/source/Plugins/Process/minidump/ProcessMinidump.h
+++ lldb/source/Plugins/Process/minidump/ProcessMinidump.h
@@ -108,7 +108,7 @@
   FileSpec m_core_file;
   lldb::DataBufferSP m_core_data;
   llvm::ArrayRef m_thread_list;
-  const MinidumpExceptionStream *m_active_exception;
+  const minidump::ExceptionStream *m_active_exception;
   lldb::CommandObjectSP m_command_sp;
   bool m_is_wow64;
 };
Index: lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
===
--- lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -241,36 +241,45 @@
   if (!m_active_exception)
 return;
 
-  if (m_active_exception->exception_record.exception_code ==
-  MinidumpException::DumpRequested) {
+  constexpr uint32_t BreakpadDumpRequested = 0x;
+  if (m_active_exception->ExceptionRecord.ExceptionCode ==
+  BreakpadDumpRequested) {
+// This "ExceptionCode" value is a sentinel that is sometimes used
+// when generating a dump for a process that hasn't crashed.
+
+// TODO: The definition and use of this "dump requested" constant
+// in Breakpad are actually Linux-specific, and for similar use
+// cases on Mac/Windows it defines differnt constants, referring
+// to them as "simulated" exceptions; consider moving this check
+// down to the OS-specific paths and checking each OS for its own
+// constant.
 return;
   }
 
   lldb::StopInfoSP stop_info;
   lldb::ThreadSP stop_thread;
 
-  Process::m_thread_list.SetSelectedThreadByID(m_active_exception->thread_id);
+  Process::m_thread_list.SetSelectedThreadByID(m_active_exception->ThreadId);
   stop_thread = Process::m_thread_list.GetSelectedThread();
   ArchSpec arch = GetArchitecture();
 
   if (arch.GetTriple().getOS() == llvm::Triple::Linux) {
 stop_info = StopInfo::CreateStopReasonWithSignal(
-*stop_thread, m_active_exception->exception_record.exception_code);
+*stop_thread, m_active_exception->ExceptionRecord.ExceptionCode);
   } else if (arch.GetTriple().getVendor() == llvm::Triple::Apple) {
 stop_info = StopInfoMachException::CreateStopReasonWithMachException(
-*stop_thread, m_active_exception->exception_record.exception_code, 2,
-m_active_exception->exception_record.exception_flags,
-m_active_exception->exception_record.exception_address, 0);
+*stop_thread, m_active_exception->ExceptionRecord.ExceptionCode, 2,
+m_active_exception->ExceptionRecord.ExceptionFlags,
+m_active_exception->ExceptionRecord.ExceptionAddress, 0);
   } else {
 std::string desc;
 llvm::raw_string_ostream desc_stream(desc);
 desc_stream << "Exception "
 << llvm::format_hex(
-   m_active_exception->exception_record.exception_code, 8)
+   m_active_exception->ExceptionRecord.ExceptionCode, 8)
 << " encountered at address "
 << llvm::format_hex(
-   m_active_exception->exception_record.exception_address,
-   8);
+   m_active_exception->ExceptionRecord.ExceptionAddress, 8);
 stop_info = StopInfo::CreateStopReasonWithException(
 *stop_thread, desc_stream.str().c_str());
   }
@@ -335,8 

[Lldb-commits] [PATCH] D68658: LLDB: Use LLVM's type for minidump ExceptionStream

[Joseph Tremoulet via Phabricator via lldb-commits]

JosephTremoulet created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
JosephTremoulet added a parent revision: D68657: Update MinidumpYAML to use 
minidump::Exception for exception stream.
JosephTremoulet added reviewers: labath, clayborg.

The types defined for it in LLDB are now redundant with core types.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68658

Files:
  lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
  lldb/source/Plugins/Process/minidump/MinidumpParser.h
  lldb/source/Plugins/Process/minidump/MinidumpTypes.cpp
  lldb/source/Plugins/Process/minidump/MinidumpTypes.h
  lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
  lldb/source/Plugins/Process/minidump/ProcessMinidump.h
  lldb/unittests/Process/minidump/MinidumpParserTest.cpp

Index: lldb/unittests/Process/minidump/MinidumpParserTest.cpp
===
--- lldb/unittests/Process/minidump/MinidumpParserTest.cpp
+++ lldb/unittests/Process/minidump/MinidumpParserTest.cpp
@@ -252,10 +252,10 @@
 
 TEST_F(MinidumpParserTest, GetExceptionStream) {
   SetUpData("linux-x86_64.dmp");
-  const MinidumpExceptionStream *exception_stream =
+  const llvm::minidump::ExceptionStream *exception_stream =
   parser->GetExceptionStream();
   ASSERT_NE(nullptr, exception_stream);
-  ASSERT_EQ(11UL, exception_stream->exception_record.exception_code);
+  ASSERT_EQ(11UL, exception_stream->ExceptionRecord.ExceptionCode);
 }
 
 void check_mem_range_exists(MinidumpParser , const uint64_t range_start,
Index: lldb/source/Plugins/Process/minidump/ProcessMinidump.h
===
--- lldb/source/Plugins/Process/minidump/ProcessMinidump.h
+++ lldb/source/Plugins/Process/minidump/ProcessMinidump.h
@@ -108,7 +108,7 @@
   FileSpec m_core_file;
   lldb::DataBufferSP m_core_data;
   llvm::ArrayRef m_thread_list;
-  const MinidumpExceptionStream *m_active_exception;
+  const minidump::ExceptionStream *m_active_exception;
   lldb::CommandObjectSP m_command_sp;
   bool m_is_wow64;
 };
Index: lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
===
--- lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -237,36 +237,37 @@
   if (!m_active_exception)
 return;
 
-  if (m_active_exception->exception_record.exception_code ==
-  MinidumpException::DumpRequested) {
+  if (m_active_exception->ExceptionRecord.ExceptionCode ==
+  static_cast(minidump::ExceptionCode::Linux_DumpRequested)) {
+// TODO: Consider moving this down to the linux-specific path and checking
+// the Simulated cases for Mac and Windows.
 return;
   }
 
   lldb::StopInfoSP stop_info;
   lldb::ThreadSP stop_thread;
 
-  Process::m_thread_list.SetSelectedThreadByID(m_active_exception->thread_id);
+  Process::m_thread_list.SetSelectedThreadByID(m_active_exception->ThreadId);
   stop_thread = Process::m_thread_list.GetSelectedThread();
   ArchSpec arch = GetArchitecture();
 
   if (arch.GetTriple().getOS() == llvm::Triple::Linux) {
 stop_info = StopInfo::CreateStopReasonWithSignal(
-*stop_thread, m_active_exception->exception_record.exception_code);
+*stop_thread, m_active_exception->ExceptionRecord.ExceptionCode);
   } else if (arch.GetTriple().getVendor() == llvm::Triple::Apple) {
 stop_info = StopInfoMachException::CreateStopReasonWithMachException(
-*stop_thread, m_active_exception->exception_record.exception_code, 2,
-m_active_exception->exception_record.exception_flags,
-m_active_exception->exception_record.exception_address, 0);
+*stop_thread, m_active_exception->ExceptionRecord.ExceptionCode, 2,
+m_active_exception->ExceptionRecord.ExceptionFlags,
+m_active_exception->ExceptionRecord.ExceptionAddress, 0);
   } else {
 std::string desc;
 llvm::raw_string_ostream desc_stream(desc);
 desc_stream << "Exception "
 << llvm::format_hex(
-   m_active_exception->exception_record.exception_code, 8)
+   m_active_exception->ExceptionRecord.ExceptionCode, 8)
 << " encountered at address "
 << llvm::format_hex(
-   m_active_exception->exception_record.exception_address,
-   8);
+   m_active_exception->ExceptionRecord.ExceptionAddress, 8);
 stop_info = StopInfo::CreateStopReasonWithException(
 *stop_thread, desc_stream.str().c_str());
   }
@@ -331,8 +332,8 @@
 
 // If the minidump contains an exception context, use it
 if (m_active_exception != nullptr &&
-m_active_exception->thread_id == thread.ThreadId) {
-  context_location = m_active_exception->thread_context;
+m_active_exception->ThreadId == thread.ThreadId) {
+